Skip to content

fix: 3.10.4 — hooks refuse an unhonorable FDEOPS_ENGAGEMENT instead of capturing into another client - #45

Open
devin-ai-integration[bot] wants to merge 3 commits into
Mainfrom
devin/1787266703-hook-resolution
Open

fix: 3.10.4 — hooks refuse an unhonorable FDEOPS_ENGAGEMENT instead of capturing into another client#45
devin-ai-integration[bot] wants to merge 3 commits into
Mainfrom
devin/1787266703-hook-resolution

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

#44 fixed the misroute in bin/fde.js only. The hooks/ layer resolves the engagement itself, in bash, so the same misroute stayed live in the unattended path: with FDEOPS_ENGAGEMENT=client-zzz in a workspace bound to client-b, step 1 failed and step 2 silently fell through to the registry —

session-start  -> injected client-b's context.md
session-stop   -> capture + dashboard against client-b
pre-compact    -> preserve against client-b

All three now stop where the CLI stops, and gain the forms the CLI gained in 3.10.3:

# hooks/{session-start,session-stop,pre-compact}
resolve_engagement_dir() {
  case $raw in
    /*)      [ -d "$raw/.fde" ] && ... ; [ -d "$raw" ] && ... ; return 1 ;;
    */*|.*)  return 1 ;;            # relative resolves against the agent's cwd
  esac
  ... "$root/$(slugify "$raw")/.fde"  # bare slug, like the CLI
}
...
if [ -z "$ENG_DIR" ] && [ -n "${FDEOPS_ENGAGEMENT:-...}" ]; then exit 0; fi   # no registry fallback

Everything else here is the same class of defect, found by fuzzing the resolver and the memory-file shapes rather than by review:

  • Relative override escaped the engagements root. FDEOPS_ENGAGEMENT=.. has no /, so it fell to accept(env) and the parent directory became the engagement — fde log created memory there. The env value is now classified up front: absolute → path forms, bare slug → <root>/<slug>/.fde, anything relative → refuse and name it.
  • A value that slugifies to nothing named a real client. slugify() defaults to the literal engagement, so FDEOPS_ENGAGEMENT='???' resolved onto a client slugged engagement — exit 0, no warning, reads leaked its content too. A name with no slug characters now refuses.
  • A whitespace-only value read as unset and filed the note under the workspace binding (export FDEOPS_ENGAGEMENT="$CLIENT" with CLIENT empty). It refuses.
  • Anything that is not a regular file in a memory slot hung the CLI forever. readEng() now lstats first — but it was not the only path: stakeholdersMemoryHealth() (bin/lib/trust.js), readRegistry(), and the append target each blocked on a fifo. All guarded, and refuseSymlinkWrite() now refuses any non-regular target, not just symlinks.
  • resume --init could exit 0 leaving the workspace unbound. It verifies the binding reads back, and the registry write runs soft inside the lock so an unwritable .registry no longer process.exit()s past the finally and leaves a stale .registry.lock.

Also: bin/check.js names the file the recording was actually found in (it said "README" even when the embed lives in docs/USAGE.md), and the 3.10.3 changelog date is corrected to 2026-08-20 — it was dated before 3.10.2.

Deliberately unchanged: an absolute out-of-root path is still accepted, because in-repo .fde/ is a supported strategy and absolute means the operator meant it.

Tests: 108 pass (11 new, incl. the three hooks executed as bash against a decoy client with byte-identical memory asserted), npm run check green. Runtime verification across three adversarial rounds is in the PR comment.

Link to Devin session: https://app.devin.ai/sessions/f135381c4682413bae73dff38eb6d1a3
Requested by: @suboss87

…f capturing into another client

Co-Authored-By: Subash Natarajan <suboss87@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration Bot and others added 2 commits August 20, 2026 23:17
…reads, registry lock

Co-Authored-By: Subash Natarajan <suboss87@gmail.com>
…her unhonorable value

Co-Authored-By: Subash Natarajan <suboss87@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Runtime verification — hook resolution, containment, and the residual holes (2664cf6, v3.10.4)

Three rounds of the adversarial matrix, shell-only, in a disposable HOME=$(mktemp -d) per case with decoy engagements created first and compared by sorted sha256 manifests before/after, every invocation under timeout, and baseline contrast against #44 (ce451c9) in a separate worktree. Floor: npm run check exit 0, npm test 108/108.

Both headline fixes are proven causal — each old defect reproduces on ce451c9 and is dead here — and the three residual holes the second round found are fixed and could not be reopened.

The #44 hook blocker and the .. escape are closed
check result
all 3 hooks × both env vars, all five strategies armed, FDEOPS_ENGAGEMENT=client-zzz exit 0, whole-root manifest IDENTICAL, no dashboard HTML, 0 leaked context
control, var unset hooks do resolve and mutate client-b → the fall-through was genuinely reachable
baseline ce451c9 reproduces the misroute: manifest CHANGED, dashboard written for client-b
.., ../.., ./x, a/b, ., ../client-b, ..%2f… CLI exit 2 must be an absolute path or a bare engagement slug; hooks exit 0 writing nothing
baseline ce451c9, FDEOPS_ENGAGEMENT=.. reproduces the escape — memory written into the plain parent dir
8 legitimate forms (absolute .fde, engagement folder, bare slug, Client A, CLIENT-A, ~/…, padded, trailing slash) all still capture into client-a only; client-b byte-identical; no .fde/.fde
Residual holes found mid-review, then fixed and re-verified
finding result at 2664cf6
slugify()'s || 'engagement' default let FDEOPS_ENGAGEMENT='???' land in a client slugged engagement (exit 0, no warning, reads leaked too) CLI exit 2 … which is not an engagement name; that client and client-b byte-identical; needle nowhere
fifo hang only partly fixed — readEng() was not the only reader (bin/lib/trust.js, readRegistry(), and the append target) fifo/socket/dir/symlink × all 5 slots, reads and writes: no exit 124 anywhere; each shape reported; hostile write target → exit 1 refused: <f> is not a regular file …; symlink target sha unchanged (no write-through)
symlink/dir .registry exited via older guards, printed no reason, and left a stale .registry.lock symlink/dir/fifo all exit 1 with could not bind this workspace … (ESYMLINK|EIRREGULAR) + the export FDEOPS_ENGAGEMENT=… fallback, no stray lock, nothing written through the symlink (the fifo case was exit 124 before)
FDEOPS_ENGAGEMENT=' ' read as unset and filed into the bound client refuses: set to whitespace - that names no engagement

Privacy floor held throughout: the sealed needle and both fragments appear 0 times across resume, resume --full, triage, prep, receipts, status --all, doctor, capture, dashboard, all three hooks, and the 95 KB hook-written dashboard, with (private - redacted) rendering and the needle confirmed on disk as a control.

Deliberately unchanged: an absolute out-of-root path (incl. ~/..) is still accepted — in-repo .fde/ is a supported strategy, so absolute means "the operator meant it"; chmod 444 .registry still binds (atomic rename only needs the directory writable, and the binding was verified to persist). Still untested: npx skills add suboss87/fdeops — no network egress from the sandbox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant