Make dejima-audit usable: on PATH, self-locating, and showing what you approve #63

2026-08-03T03:15:47-07:00

branderson commented

2026-08-03 03:15:47 -07:00

Summary generated by LLM agent

Makes dejima-audit usable. It worked, but you had to know it existed, tell it where it was, and then approve changes you could not see.

The three asks

On PATH. install.sh links it alongside the other CLIs. bin/dejima's startup warning already tells you to run dejima-audit ack ... by name; that instruction is now true.

Finds its own workspace. show and ack take an optional path. Without one they try, in order, the enclosing repo's toplevel, the main worktree root when you are in a linked worktree, and $PWD — picking the first that actually has reports. The worktree case matters here: work happens in .worktrees/<branch> but reports are keyed to the parent repo, so a worktree-local lookup would say "no reports" while the startup warning kept firing on the parent's.

Shows what you are approving. ack prompted [a] approve this exact content while displaying only a finding's name. It now shows a unified diff of pre-session against current, or the full content for a surface created during the session.

What that costs, and what bounds it

Diffing needs a "before", and the audit only ever stored hashes. So snapshot now stages each monitored surface's bytes for the session's duration, and compare turns them into bounded diffs in a <report>.d/ sidecar.

The rule is diffs persist, whole files do not:

case	retained
surface modified	`diff -U2`, capped at 200 lines / 64 KiB
surface created	empty `.new` marker — the content is live on disk
surface removed	empty `.removed` marker — a deletion diff is the whole former file
withheld (see below)	a one-line reason, no content

Staging lives under $AUDIT_DIR, never /tmp: it is the one place whole files exist, and /tmp is world-writable and survives an uncleaned exit. compare removes it from its own EXIT trap. A SIGKILLed session leaves it behind, so snapshot reaps orphans by pid + process start time — exact, so a concurrent session against the same workspace is never disturbed, with a 24-hour fallback when the owner record is unreadable.

Content is withheld from the store, with the reason shown at the prompt, when the surface holds credentials by convention (.envrc, .mcp.json, .claude/settings.local.json, and .env/.env.* matched on basename at any depth); when the diff trips a credential screen; when it is binary, over the per-diff cap, or too large to have been staged; when a git config key is multi-valued; and when the store itself sits inside the mounted workspace. In every case ack still shows the live current content — withholding is about the persistent store, not about what you see while deciding. DEJIMA_AUDIT_DIFFS=0 turns retention off entirely.

Exhausting the 1 MiB per-report budget is treated as a signal rather than a limit: both the report and the exit-time summary say so, in the same voice as the existing CRIT_CAP overflow, because that many changed host-persistence surfaces in one session is abnormal.

The boundary

All of this rests on the container not being able to read the store. docker-compose.yml mounts nothing covering ~/.local/share/dejima, and test/static.bats now pins that — including the ${HOME}/.local ancestor — so a future mount breaks a test instead of silently opening the boundary. Where the store is reachable (configured inside the workspace) no content is retained at all, only a note saying why. That check is enforced at snapshot, so nothing is written even transiently.

Known limitations, deliberately scoped out

Multi-valued git config keys. git_config_snapshot sorts, so lexicographic-last is not file-order-last, and git resolves by file order. Rather than guess, the audit withholds the diff and says to compare by hand. Two consequences remain: the live-content fallback shows the first sorted value, and [a] pins that value, so the report sheds its -CRITICAL.txt eviction protection. It is not silent suppression — the sha never matches what git executes, so the finding re-fires next session. Correct file-order resolution is its own change; an ack-time refusal was rejected because include.path is legitimately multi-valued and refusing would make those findings permanently unclearable, which is the alert fatigue this feature exists to end.
The credential screen is best-effort and case-insensitive, which measurably widens false positives — a hook containing TOKEN=$(...) is now withheld. Every false positive costs a withheld diff, never a leak. Narrowing it (exempting shell substitutions per-match) is follow-up work, not a one-liner on the reliable half of a credential defence.
render_finding copies the current file unbounded before display-time truncation — the same shape as a bug fixed here in promote_finding, but operator-initiated rather than on the automatic exit path.
Two compare runs in the same second against one workspace share a sidecar directory. Requires two concurrent sessions exiting together; the underlying same-second report-filename collision predates this branch.
Two of the ack tests do not discriminate the implementation from its absence; each pairs with a sibling that does.

Testing

bats test/audit.bats test/static.bats → 275/275 pass. Whole fast suite (adding project, proxy, remote) → 475 tests, 1 failure: proxy.bats "forwarder tls mode refuses to start without a cert", which fails identically on unmodified main and is unrelated to this branch. runtime.bats was not run (needs Docker). bash -n bin/dejima-audit clean.

Reviewed task by task with an adversarial pass after each, plus a whole-branch review. Findings those caught and fixed here, rather than shipped:

A workspace-controlled git config key reached the terminal unsanitized, so a planted ESC byte could repaint the prompt being answered.
looks_secret was case-sensitive, so {"env":{"GITLAB_TOKEN":"glpat-…"}} in .claude/settings.json reached the store in cleartext — demonstrated end-to-end.
A multi-valued exec key produced a fabricated diff: for core.pager going to aaa,bbb,zzz-evil, the prompt read "bbb removed" and zzz-evil appeared nowhere.
Every size cap applied after the bytes hit disk: a 300 MB file caused 573 MB of peak $TMPDIR writes — on tmpfs, from the automatic exit path — to produce a 44-byte marker.
readlink -f failing on a not-yet-created ~/.local/share/dejima made the fail-closed gate silently disable staging on every fresh install.
Creating the staging directory made resolve_ws treat any snapshotted workspace as having reports, hiding a parent repo's CRITICAL report from a worktree.
A dangling symlink could be displayed as real content, then refused at approval, leaving a CRITICAL report permanently unclearable.

Fixes #57

**Summary generated by LLM agent** Makes `dejima-audit` usable. It worked, but you had to know it existed, tell it where it was, and then approve changes you could not see. ## The three asks **On PATH.** `install.sh` links it alongside the other CLIs. `bin/dejima`'s startup warning already tells you to run `dejima-audit ack ...` by name; that instruction is now true. **Finds its own workspace.** `show` and `ack` take an optional path. Without one they try, in order, the enclosing repo's toplevel, the main worktree root when you are in a linked worktree, and `$PWD` — picking the first that actually has reports. The worktree case matters here: work happens in `.worktrees/<branch>` but reports are keyed to the parent repo, so a worktree-local lookup would say "no reports" while the startup warning kept firing on the parent's. **Shows what you are approving.** `ack` prompted `[a] approve this exact content` while displaying only a finding's name. It now shows a unified diff of pre-session against current, or the full content for a surface created during the session. ## What that costs, and what bounds it Diffing needs a "before", and the audit only ever stored hashes. So `snapshot` now stages each monitored surface's bytes for the session's duration, and `compare` turns them into bounded diffs in a `<report>.d/` sidecar. The rule is **diffs persist, whole files do not**: | case | retained | |---|---| | surface modified | `diff -U2`, capped at 200 lines / 64 KiB | | surface created | empty `.new` marker — the content is live on disk | | surface removed | empty `.removed` marker — a deletion diff *is* the whole former file | | withheld (see below) | a one-line reason, no content | Staging lives under `$AUDIT_DIR`, never `/tmp`: it is the one place whole files exist, and `/tmp` is world-writable and survives an uncleaned exit. `compare` removes it from its own `EXIT` trap. A `SIGKILL`ed session leaves it behind, so `snapshot` reaps orphans by pid + process start time — exact, so a concurrent session against the same workspace is never disturbed, with a 24-hour fallback when the owner record is unreadable. Content is withheld from the store, with the reason shown at the prompt, when the surface holds credentials by convention (`.envrc`, `.mcp.json`, `.claude/settings.local.json`, and `.env`/`.env.*` matched on basename at any depth); when the diff trips a credential screen; when it is binary, over the per-diff cap, or too large to have been staged; when a git config key is multi-valued; and when the store itself sits inside the mounted workspace. In every case `ack` still shows the **live current content** — withholding is about the persistent store, not about what you see while deciding. `DEJIMA_AUDIT_DIFFS=0` turns retention off entirely. Exhausting the 1 MiB per-report budget is treated as a signal rather than a limit: both the report and the exit-time summary say so, in the same voice as the existing `CRIT_CAP` overflow, because that many changed host-persistence surfaces in one session is abnormal. ## The boundary All of this rests on the container not being able to read the store. `docker-compose.yml` mounts nothing covering `~/.local/share/dejima`, and `test/static.bats` now pins that — including the `${HOME}/.local` ancestor — so a future mount breaks a test instead of silently opening the boundary. Where the store *is* reachable (configured inside the workspace) no content is retained at all, only a note saying why. That check is enforced at `snapshot`, so nothing is written even transiently. ## Known limitations, deliberately scoped out - **Multi-valued git config keys.** `git_config_snapshot` sorts, so lexicographic-last is not file-order-last, and git resolves by file order. Rather than guess, the audit withholds the diff and says to compare by hand. Two consequences remain: the live-content fallback shows the first sorted value, and `[a]` pins that value, so the report sheds its `-CRITICAL.txt` eviction protection. It is not silent suppression — the sha never matches what git executes, so the finding re-fires next session. Correct file-order resolution is its own change; an ack-time refusal was rejected because `include.path` is legitimately multi-valued and refusing would make those findings permanently unclearable, which is the alert fatigue this feature exists to end. - **The credential screen is best-effort** and case-insensitive, which measurably widens false positives — a hook containing `TOKEN=$(...)` is now withheld. Every false positive costs a withheld diff, never a leak. Narrowing it (exempting shell substitutions per-match) is follow-up work, not a one-liner on the reliable half of a credential defence. - **`render_finding` copies the current file unbounded** before display-time truncation — the same shape as a bug fixed here in `promote_finding`, but operator-initiated rather than on the automatic exit path. - **Two `compare` runs in the same second** against one workspace share a sidecar directory. Requires two concurrent sessions exiting together; the underlying same-second report-filename collision predates this branch. - Two of the `ack` tests do not discriminate the implementation from its absence; each pairs with a sibling that does. ## Testing `bats test/audit.bats test/static.bats` → 275/275 pass. Whole fast suite (adding `project`, `proxy`, `remote`) → 475 tests, 1 failure: `proxy.bats` "forwarder tls mode refuses to start without a cert", which **fails identically on unmodified `main`** and is unrelated to this branch. `runtime.bats` was not run (needs Docker). `bash -n bin/dejima-audit` clean. Reviewed task by task with an adversarial pass after each, plus a whole-branch review. Findings those caught and fixed here, rather than shipped: - A workspace-controlled git config key reached the terminal unsanitized, so a planted ESC byte could repaint the prompt being answered. - `looks_secret` was case-sensitive, so `{"env":{"GITLAB_TOKEN":"glpat-…"}}` in `.claude/settings.json` reached the store in cleartext — demonstrated end-to-end. - A multi-valued exec key produced a *fabricated* diff: for `core.pager` going to `aaa,bbb,zzz-evil`, the prompt read "`bbb` removed" and `zzz-evil` appeared nowhere. - Every size cap applied *after* the bytes hit disk: a 300 MB file caused 573 MB of peak `$TMPDIR` writes — on tmpfs, from the automatic exit path — to produce a 44-byte marker. - `readlink -f` failing on a not-yet-created `~/.local/share/dejima` made the fail-closed gate silently disable staging on every fresh install. - Creating the staging directory made `resolve_ws` treat any snapshotted workspace as having reports, hiding a parent repo's CRITICAL report from a worktree. - A dangling symlink could be displayed as real content, then refused at approval, leaving a CRITICAL report permanently unclearable. Fixes #57

branderson added 13 commits 2026-08-03 03:15:48 -07:00

install: put dejima-audit on PATH dc4100c89c

audit: default the workspace to the enclosing repo f8238140f7

audit: show the content ack is asking to approve e6b4b23b44

audit: fix review findings in ack content display 9ebf22e3bb

- sanitize the gitconfig key in source_hint's git-config arm; it was the
  only display arm that skipped sanitize(), letting a crafted subsection
  name reach the terminal verbatim
- current_sha_for_id now accepts a dangling symlink (target missing) the
  same way current_content_for_id does, so approving a symlinked finding
  the operator was just shown can no longer be refused as nonexistent
- scope the git-config content test to the block after the
  '--- current content ---' marker, since the finding's own body line
  already carries the value and made the old assertion vacuous

audit: stage pre-session content inside the store 8831b0fab8

audit: fix review findings in staging cc00f15512

audit: use readlink -m in store_reachable so staging works on a fresh install 6fdfad105e

audit: retain bounded diffs beside each report 751a4e198a

audit: close the retention screen's case gap and bound what one report keeps ee92936c1d

audit: render retained diffs at the ack prompt 391f9d9301

docs: describe what the audit retains and why fb1df3b4cb

Document dejima-audit's staging/retention scheme as it actually stands:
bounded per-surface diffs kept under $AUDIT_DIR outside the container's
reach, the withhold cases (denylist by basename, oversized-to-stage,
binary/size, and the best-effort credential screen), the per-report
retention budget and its ABNORMAL alarm on exhaustion, and SIGKILL/reap
semantics keyed on pid + process start time. Pin the store/container
boundary with a regression test in test/static.bats, and fix install.sh's
header comment to mention dejima-audit among the CLIs it puts on PATH.

docs: cover the empty-diff withhold case, widen the boundary guard 730ac26d90

Document write_diff's sz-eq-0 withhold path: a multi-valued git config
key can compare as unchanged because gitconfig_value_for_key reads only
the first matching line while git itself resolves to the last, so a
reported finding can still yield an empty diff - state why, so it reads
as a known limitation rather than a bug. Also add an ${HOME}/.local
pattern to the audit-store boundary test: the existing three patterns
missed that ancestor of $AUDIT_DIR, which a mount there would equally
expose to the container.

audit: withhold on multi-valued config keys, cap the after side before copying

tests / runtime (pull_request) Failing after 6m23s

Details

tests / fast (pull_request) Successful in 7m50s

Details

6ba0c2e658

Two Important findings from the final whole-branch review.

A multi-valued git config exec key made `ack` show a fabricated diff.
promote_gitconfig's before-loop has no break, so it emits every
pre-session value of the key; write_diff's empty-diff withhold therefore
only covered the shape where there was exactly one. With two, a diff was
retained saying the second one had been removed - nothing was removed,
and the value the session appended, the one git will actually execute,
appeared nowhere in it. Approving it pinned the sha of a value the
operator was never shown. Reachable by a one-line append to .git/config,
and ordinary besides: include.path is legitimately multi-valued.

Count both sides instead and withhold when either holds more than one
value. Deliberately no attempt to resolve which value git would use:
git_config_snapshot sorts, so lexicographic-last is not file-order-last,
and a naive first-to-last swap is still wrong. That is its own change.
This leaves write_diff's empty-diff arm reachable only by a config line
whose text changed while its parsed value did not, so its comment now
names that route instead.

Separately, every size cap here applied after the bytes were already on
disk. compare runs from bin/dejima's EXIT trap and the size is the
session's choice, so a .claude/ file grown to 300 MB during a session
cost 573 MB of peak $TMPDIR - frequently a tmpfs, i.e. host RAM - to
produce a 44-byte marker, and a SIGKILL mid-compare left a whole copy of
it behind. Check the source size before copying it, reusing the existing
too-large marker, and bound the generated diff at generation with
head -c rather than after.

Tests cover both multi-valued shapes and the oversized after side (peak
$TMPDIR sampled against an isolated TMPDIR); the existing multi-valued
test is rewritten in place because the reason text it pinned legitimately
changed. 275/275 pass.

branderson added 1 commit 2026-08-03 03:52:30 -07:00

Merge branch 'main' into audit-usability

tests / fast (pull_request) Successful in 7m40s

Details

tests / runtime (pull_request) Successful in 5m0s

Details

13269a9ec0

branderson merged commit 1ee896ccd4 into main

2026-08-03 04:15:09 -07:00

branderson referenced this issue from a commit

2026-08-03 04:15:11 -07:00

Merge pull request 'Make dejima-audit usable: on PATH, self-locating, and showing what you approve' (#63) from audit-usability into main