feat: lg-handoff, distil and privately share an agent session - #3
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 36480686 | Triggered | Bearer Token | af7e0c1 | src/handoff/scan.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
A handoff bundle is four files with index.html at the root, because that is what enclave serves. checkEnclaveConstraints encodes the published push limits (13 allowed extensions, 2 MB per file, 50 files, 10 MB total) so a bad bundle fails locally with the limit named instead of as an opaque server refusal. The subtree is import-clean by design: nothing here reaches into src/core/ or src/adapters/, so it can be extracted to a sibling package with a git mv.
Pure parsers that narrow a transcript to turns, session id, cwd, model and file paths. They never throw on malformed input; a bad line becomes a warning. The drop list is the point: codex base_instructions, world_state, inter_agent_communication, claude file-history-snapshot records, tool-result blobs and attachments never reach the output. A transcript is untrusted, secret-bearing input and the reader is the narrowing boundary. The opencode export shape is unverified against a real invocation, so that parser is defensive and marked experimental per AGENTS.md.
Ten named rules, each with a positive fixture and a deliberate near-miss that must not fire: a bare eyJ is not a JWT and task-list is not an sk- key. Finding excerpts are masked, so a report can never itself reproduce the secret. The scanner only reports. Fail-closed policy belongs to the CLI.
Distillation is extractive and mechanical: turns are quoted under fixed headings, with a banner saying so. No model calls, so pack works offline and the no-LLM-SDK rule holds. index.html is the security boundary. Its input is derived from an untrusted transcript, so every interpolated value is escaped and the page makes zero external requests.
A second bin, not an lg subcommand: it shares no state with a run and owns its own exit codes (0 ok, 1 usage or not found, 2 findings or push refused). push is fail-closed and ordered. A non-private visibility is refused first, then the scanner, then the enclave constraint check - and a failure at either gate means enclave is never invoked at all. Tests assert that with a fake Exec recording zero calls. The share url is printed and written to SHARE-URL.txt in the same step, because enclave prints it once and keeps only its hash: lose the line and the link is unrecoverable. Every spawn - git, opencode export, enclave - goes through an injected Exec seam, so no test touches a real binary or the network. The enclave share-create json parser is lenient on purpose; that stdout shape has not been captured from a real invocation yet.
States what handoff deliberately does not do, in the voice of the existing non-goals: no pull (enclave has no fetch and share urls are print-once), no raw transcript upload (.jsonl is not an allowed extension and a real transcript exceeds the 2 MB file cap), no cross-CLI replay, no session transplant, private visibility only, and no signal bus or daemon - that would contradict 'Not a workflow server'. AGENTS.md gains the Exec-seam test rule and the src/handoff/ import rule.
All three were verified by running the exploit input, not by reading the code.
1. The git remote was published verbatim and never passed through
redactSession, so a credential-bearing remote
(https://oauth2:<token>@host/repo.git) landed in meta.json, handoff.md and
index.html. It is now stripped at the point it is gathered, and the strip is
reported as a warning. A bare user@host is deliberately left alone -
ssh://git@github.com is the commonest remote there is and rewriting it would
destroy the one fact the reader needs; a token used AS the username is caught
by the vendor rules instead.
2. The scanner was a narrow vendor allowlist. These five inputs all wrote
through with zero findings and would have been published:
https://oauth2:<glpat>@gitlab.com/org/repo.git
https://user:hunter2@host/repo.git
sk_live_<stripe> (underscore, so the sk- rule could not match)
database_password=hunter2 (rule was upper-case only)
postgres://admin:pw@db:5432/app
Adds url-credentials, stripe-key, gitlab-token, npm-token and sendgrid-key;
widens github-token to ghu_/ghr_, makes env-assignment case-insensitive and
able to match a JSON key, and accepts a lower-case Windows drive letter. All
five now fire.
3. A re-pack into an existing --out directory left the previous
SHARE-URL.txt in place, and nothing downstream caught it: .txt is an allowed
extension and no rule matches an enclave share url, so the new artifact
served the old share link. writeBundle now purges it.
Also gitignores handoff-bundle/ and SHARE-URL.txt - the default output path is
inside the repo, so a routine git add -A committed a live capability url.
The scanner is still an allowlist of shapes, not a proof, and the README now
says so rather than implying coverage it does not have.
Two silent-success paths, both verified against the built cli before and after. Given one bundle containing an aws key, the scanner reported: file readable -> 1 scan finding, exit 2 file chmod 000 -> scan clean, exit 0 An empty result means "clean, safe to publish" to every caller, so a file skipped because it was unreadable was indistinguishable from a file that was read and found innocent. Unreadable files, unlistable directories and a non-directory path now each produce an unreadable-file finding, so the gate fails closed. walk() is wrapped too - it could previously throw straight out of scanBundleDir while the surrounding code advertised that it never would. Separately, `lg-handoff scan /typo` printed "scan clean" and exited 0. It now reports the missing directory and exits 1, keeping "cannot find it" and "found something in it" distinguishable at the exit code. Also documents --visibility in the README command table (it was only mentioned in the non-goals list) and notes that line 0 on a finding means the file itself rather than a line in it.
The section explained what the feature is before it explained what to do with it, listed exit codes with no matching action, and had no picture of the output - so a reader could not tell what a colleague would actually receive. Now leads with the two commands and the handover story, then shows the rendered brief (captured from a real run, not invented), then reference material. Adds what was missing rather than rewording what was there: - what the recipient sees, and that they install nothing - an exit-code table with a Do this column, and the fact that exit 2 from push means enclave was never invoked - where each adapter's session is discovered, and why discovery is a guess - a troubleshooting table keyed on the actual message strings, each verified to exist in the shipped source - the share url being print-once, and how to hand over again or revoke --help now carries the same workflow, exit codes and caveats, so an agent that runs lg-handoff --help learns the tool without opening the README. pack and push get their own help notes for session discovery and the fail-closed refusal. AGENTS.md gains the module map and the two rules that are the point of the subtree: readers drop rather than carry, and the gates fail closed.
datj9-reader
left a comment
datj9-reader
left a comment
There was a problem hiding this comment.
Review — feat: lg-handoff, distil and privately share an agent session
Reviewed 4556106 against main. Verified locally: npm run typecheck exits 0, npm test = 23 files / 303 tests pass, npm run build emits dist/handoff/cli.js. CI green on check (22), check (24), GitGuardian. Enclave contract cross-checked against the installed enclave CLI's own push --help and share create --help.
Verdict: fix-first. Two blocking issues, both in the security boundary this feature exists to be. Everything else is small.
Blocking
H1 — push can republish the previous share URL as a served file (src/handoff/commands.ts:224-228)
writeBundle deliberately scrubs SHARE-URL.txt on every pack (bundle.ts:38-43, with a comment explaining exactly this risk). pushCommand writes that file at the end but never scrubs it at the start. So pack → push is safe, but a second push on the same directory uploads the prior print-once link as a page in the new artifact.
Verified on a bundle dir containing the four files plus SHARE-URL.txt:
scan findings: []
constraint violations: []
Neither gate catches it: .txt is in the 13-extension allowlist, and no scan rule matches an enclave /s/… URL — which bundle.ts:41-42 already acknowledges as the reason the pack-time scrub exists. The consequence is that a link the sender believes is separately scoped becomes readable to everyone holding the new artifact's link, and there is no test for a second push.
Fix: rmSync(join(bundleDir, SHARE_URL_FILE), { force: true }) at the top of pushCommand, before scanBundleDir. Or treat the file's presence as a constraint violation. Either way, add a test that pushes twice.
H2 — rewritePaths replaces the bare username everywhere, corrupting content (src/handoff/scan.ts:290)
After the three home-path shapes are handled, replaceLiteral(out, opts.username, "user") runs against the whole document as an unanchored substring. Short or common usernames appear inside ordinary words:
| username | input | output |
|---|---|---|
dat |
the dataset in /srv/data was validated; datadog alerts fired |
the useraset in /srv/usera was valiusered; useradog alerts fired |
ec |
ec2 instance ready, second attempt |
user2 instance ready, suserond attempt |
This is worse than cosmetic. The brief is the artifact a colleague acts on, and this silently rewrites file paths, commands and identifiers inside the quoted turns — with no warning emitted. It also hurts the security goal it serves: the mangling is loud enough that a reader may stop trusting the redaction, and abs-home-path already catches the residue this line is trying to pre-empt.
Fix: bound the replacement to path and delimiter contexts — require a separator or word boundary ((?<=[/\\@:\s"']) … (?=[/\\@:\s"']|$)), or drop the bare-username pass entirely and rely on the three explicit home shapes plus the abs-home-path rule. Add the dat/dataset case as a test.
Non-blocking
M1 — README overclaims that exit 2 means enclave was never invoked (README.md ~348-350, echoed at cli.ts:69 and cli.ts:136-140)
pushCommand also returns 2 after a failed enclave push (commands.ts:183-187) and after a failed enclave share create (commands.ts:210-214). In the share-failure path the private artifact is already published — viewUrl is logged one line earlier. An operator reading exit 2 as "nothing left the machine" will be wrong on any auth error, network blip, or rejected --expires. Either split local-refusal from remote-failure exit codes, or reword to "exit 2 at a local gate means enclave was never invoked; exit 2 after that means see stderr, a partial publish is possible".
M2 — --expires is forwarded to enclave unvalidated (cli.ts:131, enclave.ts:45-46)
Enclave accepts 7d|12h|2w, a date, a datetime, or a zoned ISO instant; anything else is an InvalidInputError. --expires forever therefore publishes the artifact and then fails share-create, landing in the H2-adjacent partial state above with no share URL persisted. Validate the shape locally before the first spawn — the same "fail with a named limit instead of an opaque server refusal" argument that justifies checkEnclaveConstraints.
M3 — the scanner's coverage is narrower than the docs imply
The PR body says 10 rules; SCAN_RULES has 15 plus unreadable-file. That direction is fine, but the gaps matter more than the count. Probed against the real rule set, these are missed:
| Shape | Result |
|---|---|
| AWS secret access key (40-char) | missed |
Authorization: Bearer … / Basic … |
missed |
Stripe restricted (rk_live_) |
missed — sk_live_ is caught |
Google OAuth client secret (GOCSPX-) |
missed |
HuggingFace token (hf_) |
missed |
| Hex-32/hex-64 client secrets | missed |
| PEM body without its header line | missed |
Absolute non-home paths (/srv/clients/acme-bank/…, D:\clients\…) |
missed |
Controls all fire correctly (sk-ant-, ghp_, glpat-, env assignment, URL credentials, /home/<user>/, PEM header, and the JWT/task-list near-misses stay quiet). Not a merge blocker — a prefix scanner is honest work and the masking is correct at 4 chars — but README/PR should describe it as best-effort with a named gap list rather than as the gate. Authorization: headers and rk_live_/hf_/GOCSPX- are cheap adds.
L1 — .svg is skipped by the scanner but allowed by enclave (scan.ts:124-131 vs types.ts:50-64)
BINARY_EXTENSIONS includes .svg, so it is never read; .svg is in enclave's 13-extension allowlist, so it is uploaded. A v1 bundle only ever contains four files, so this is unreachable today — but scanBundleDir walks recursively and checkEnclaveConstraints accepts any allowlisted file, so a hand-added SVG ships unscanned. Verified: an SVG containing sk-ant-api03-… produces zero findings. SVG is text; drop it from the binary set.
L2 — err as Error without narrowing (cli.ts:34) — a non-Error throw prints error: undefined. err instanceof Error ? err.message : String(err) matches what scan.ts already does.
L3 — stale test count — PR body says 289, suite is 303.
What holds up
Checked and clean, so it does not get re-litigated later:
- Ordering is right.
pushCommandruns visibility → scan → constraints → spawn (commands.ts:154-174). No bypass flag exists, and--visibilityrefuses anything butprivatebefore any spawn. - Fail-closed is real. An unreadable dir, an unlistable dir, and an unreadable file each become an
unreadable-filefinding rather than a skip (scan.ts:205-233). Reachable empty findings genuinely mean "scanned and clean" — except through the two paths in H1 and L1. - The renderer does not inject. Probed with hostile transcript turns:
</title><script>in the title,</pre><img src=x onerror=…><pre>in a turn,<script>in a warning. All escaped, zero live<script>, and the deliberate absence of a markdown engine holds. Section spoofing also fails — a turn containing## Warningsstays inside its<pre>; the document still has exactly one<h2>Warnings</h2>.render.test.ts:111-137already tests this class. - The readers narrow rather than convert.
DistilledSessionhas no field for tool results, attachments, snapshots,base_instructions, or permission modes, and each reader drops at the boundary:claude.ts:23-36+:72-89,codex.ts:18-28+:119-124,opencode.ts:73-93. - Layering and purity hold. No
src/handoff/**file importssrc/core/**orsrc/adapters/**(only prose in comments).scanBundleDiris the single I/O function inscan.ts;render.tsand the readers are pure;commands.tsis the only spawner, via the injectedExec;execais confined tocli.ts. ThebuildEnclavePushArgsduplication is intact and commented as intentional. Zeroanyin non-test sources. - The plan-change rationale checks out against the real CLI.
index.htmlrequired, exactly 13 extensions, 2 MB/file, 50 files, 10 MB total, no fetch subcommand, share URL print-once — all confirmed fromenclave push --helpandenclave share create --help, and all encoded intypes.ts/checkEnclaveConstraints. - No test spawns a real binary, and the nine commits are conventional, lowercase, imperative, one logical change each.
H1 and H2 are both a handful of lines. Fix those two plus M1's wording and this is good to merge.
writeBundle already deletes SHARE-URL.txt so pack cannot republish a print-once link. push wrote the file and never removed it, so a second push on the same directory uploaded the previous /s/… url as a .txt page.
An unanchored replace of the local username turned dataset/datadog into useraset/useradog and silently corrupted the brief a colleague acts on.
A rejected expiry used to publish the private artifact and then fail at share create, leaving a live view url and no persisted share link.
push also returns 2 after a failed enclave push or share create. In the share-create path the private artifact is already published.
svg is text and on enclave's allowlist, so skipping it was a hole. Authorization headers, stripe rk_, hf_, and GOCSPX- are cheap prefix adds. The scanner is still an allowlist of shapes, not a proof.
The new local expiry check is a usage error, same as a refused visibility. It must not be described as the scan/constraint exit 2.
|
Addressed the review on H1: H2: bare-username rewrite is delimiter-bounded ( M1: exit 2 docs now distinguish a local gate (nothing uploaded) from a remote failure (partial publish possible). M2: M3 / L1 / L2: scanner described as an allowlist with a named gap list. Cheap prefixes added ( L3: test count in the PR body is now 312. Left out of this round, as the review marked them non-blocking: AWS secret access keys, PEM bodies without a header, hex client secrets, non-home absolute paths. |
datj9-reader
left a comment
datj9-reader
left a comment
There was a problem hiding this comment.
Code Review
Verdict: Approve
Re-checked 876c1c5 against the previous CHANGES_REQUESTED review. Local evidence: npm run typecheck exits 0, npm test = 23 files / 312 tests pass.
Fixed since last review
- H1:
pushCommandremovesSHARE-URL.txtbeforescanBundleDir. Test asserts the leftover file is gone at theenclave pushspawn. - H2: username rewrite is delimiter-bounded.
dataset/datadog/ec2/secondare unchanged. Isolatedauthor dat pushedstill becomesauthor user pushed. - M1: README and CLI help no longer claim every exit 2 means nothing was uploaded.
- M2:
isValidExpiresrejectsforeverbefore any spawn. Invalid expiry is exit 1. - L1:
.svgdropped fromBINARY_EXTENSIONS; planted AWS key inlogo.svgis now a finding. - L2: handoff
finishnarrows withinstanceof Error. - M3 cheap prefixes:
rk_live_/rk_test_,hf_,GOCSPX-,Authorization: Bearer|Basic. Docs name the remaining gaps.
Notes
No new blocking issues. AWS 40-char secrets, PEM bodies without a header, hex client secrets, and non-home absolute paths remain out of the scanner, as documented.
A complete Authorization Bearer literal in the source tripped the repo secret scanner. Same split used for the other fabricated shapes.
datj9-reader
left a comment
datj9-reader
left a comment
There was a problem hiding this comment.
Re-verify — prior review against 08265fc
Prior review: #3 (review) (4556106). Current HEAD: 08265fc (+7 commits). Local gates: npm run typecheck exits 0, npm test = 23 files / 312 tests pass.
| ID | Prior | Verdict | Evidence |
|---|---|---|---|
| H1 | SHARE-URL re-push | fixed | commands.ts:174 rmSync(SHARE_URL_FILE) before scan; test at commands.test.ts:155; probe: after scrub, file gone, scan+constraints clean |
| H2 | bare-username rewrite | fixed | replaceUsernameToken (scan.ts) bounds on path/whitespace/@/:/"'; dataset/datadog/ec2 preserved, path-context dat still → user; test at scan.test.ts:304-313 |
| M1 | exit-2 overclaim | fixed | README ~346-349 and cli.ts:139-141 now distinguish local-gate 2 from post-enclave 2 / partial publish |
| M2 | --expires unvalidated |
fixed | isValidExpires (enclave.ts:54-59) + gate at commands.ts:163-169 returns 1 before any spawn; forever/1m/yes/"" → false; good shapes → true |
| M3 | scanner gaps | partially addressed | Added huggingface-token, google-oauth-secret, auth-header; stripe rule now covers rk_; Bearer/Basic/hf_/GOCSPX-/rk_live_ all DETECTED in probe. Still missed (disclosed-class residual): AWS secret access key body, PEM body without header, bare hex-32/64. Not a merge blocker |
| L1 | .svg unscanned |
fixed | .svg dropped from BINARY_EXTENSIONS (scan.ts:141-147); probe: SVG with sk-ant-… → anthropic-key finding |
| L2 | err as Error |
fixed | cli.ts:34 now err instanceof Error ? err.message : String(err) |
| L3 | PR body "289 tests" | still valid | PR body still says 289 pass (23 files); suite is 312. Cosmetic only |
Count
| Verdict | n |
|---|---|
| fixed | 6 (H1, H2, M1, M2, L1, L2) |
| partially addressed | 1 (M3) |
| still valid | 1 (L3) |
| overstated / stale | 0 |
Decision
Both blockers are gone, with tests. Residual M3 gaps are the hard shapes a prefix scanner honestly leaves; L3 is a one-line PR-body refresh. Approve from my side — L3 is not worth holding the merge.
2 participants
Implements Feature 1 of the "Team fabric for agent CLIs" brainstorm: hand a
colleague enough context to continue your work without handing them your
transcript.
lg-handoffis a second bin. It distils one claude/codex/opencode session into afour-file bundle, scans it for secrets, and publishes it privately to enclave
behind a time-boxed share link.
Why the plan changed before any code was written
The brainstorm was reviewed against the actual
enclaveCLI contract and thisrepo. Three of its Feature-1 designs turned out to be unbuildable as written:
index.html.enclave pushrequires it at theroot - it is the page served. The push would have been refused outright.
bundle.tarworkaround is impossible..taris not in enclave's13-extension allowlist. Not awkward - refused.
raw/transcript upload is dead on arrival..jsonlis not an allowedextension, and the plan's own 3.8 MB example transcript exceeds the 2 MB
per-file cap. This is why v1 is distilled-only: raw mostly cannot ride
enclave.
Also dropped:
handoff pull, because enclave has no fetch subcommand and ashare URL is print-once (the server keeps only its hash).
checkEnclaveConstraintsnow encodes that whole contract locally, so a badbundle fails with the limit named instead of as an opaque server refusal.
What is deliberately not here
All of Feature 2 - the daemon, Redis streams, Ed25519 envelopes, the approval
gate, crossagent integration. It directly contradicts
README.md: "Not aworkflow server. No daemon, no web UI, no cloud." An inbox that can start an
agent on someone else's laptop is a different product with a much harder threat
model, and it belongs in a sibling repo.
To keep that option cheap,
src/handoff/**imports nothing fromsrc/core/**or
src/adapters/**. Extracting it later is agit mv. That is also whybuildEnclavePushArgsexists twice on purpose -AGENTS.mdnow says not todeduplicate it.
Security posture
A transcript is production data, so the code treats it that way:
pushis fail-closed and ordered. Non-private visibility refused first,then the secret scan, then the enclave constraint check. A failure at either
gate means
enclaveis never invoked - asserted with a fakeExecrecordingzero calls.
base_instructions,world_state,inter_agent_communication*, Claudefile-history-snapshot, tool-resultblobs and attachments never reach the output. A test serialises the result and
asserts
base_instructionsappears nowhere.eyJis not a JWT,
task-listis not ansk-key. Findings are masked, so areport cannot reproduce the secret it found.
index.htmlescapes everything. Its content derives from an untrustedtranscript;
<script>alert(1)</script>in a turn renders as literal text, andthe page makes zero external requests.
privatevisibility only.organdpublicare refused.Test plan
npm run typecheck- clean.npm test- 312 pass (23 files), up from 197.npm run build- emitsdist/handoff/cli.js;lgsmoke test(
node dist/cli.js run examples/hello.yaml) still passes.claude,codex,opencode,enclaveorgit, and nonetouches the network. Every spawn goes through an injected
Execseam.AGENTS.mdupdated to say so.warnings surfaced (unknown record type, 16 tool-result blocks dropped), scan
clean, and zero residual
/Users/<name>paths and zero external resourceURLs in the output.
Known gaps, stated rather than hidden
enclave share create --jsonparser is lenient because that stdout shapehas not been captured from a real invocation.
AGENTS.mdrequires a fixture -one real run should tighten it.
marked experimental.
encoding.
packalways prints which file it chose, and--session-fileoverrides a wrong pick.