feat(mcp): record and alert on a governance-file rewrite (#97) #98

Merged
mathias merged 1 commits from agent/97-observe-brain-edit-root into main 2026-08-22 06:09:53 +00:00
Owner

Closes #97.

brain_edit_root was gated but unobserved — on success it emitted a slog.Warn and nothing else. That proves a rewrite was allowed, not that one happened, and brain-sync commits it within 15 minutes authored brain-sync, so git history cannot distinguish a CLAUDE.md rewrite from routine note traffic.

What changed

  • ntfy on every successful brain_edit_root — reuses the existing audit.NewNtfyNotifier wiring and topic (BRAIN_NTFY_URL/BRAIN_NTFY_TOKEN). Extracted buildNotifier() so the capture sink and the governance observer share one construction path instead of two.
  • Audit record through the existing capture.AuditEntry/AuditSink — Items: [path], SecurityEvents: ["governance file rewritten: <path> before=<sha> after=<sha>"], principal from the authenticated context.
  • notes.Result.PrevContentHash — the before-hash. Result previously carried only the resulting hash, so a record could say what a file became but not what it stopped being.
  • Wired in cmd/server/main.go alongside WithCapture, which is what re-derives the static principal the gate checks.

Decisions worth reviewing

Path and hashes only in the notification body. It leaves the cluster for a phone through a third-party topic. A hash identifies the rewrite; the bytes would leak whatever was written into an instruction file. Asserted by a test that fails if old_str/new_str appear in the message.

Reserved at Internal, not Confidential. DegradingSink refuses a Confidential record when loki is unreachable rather than buffering it — that would drop exactly the record that matters most. Internal buffers and reconciles.

Observation is best-effort and never fails the call. The bytes are on disk and brain-sync will commit them, so returning an error would report a non-rewrite to the caller while the rewrite stands. A failure to observe logs at ERROR — the one case where the trail itself is what broke. Same reasoning the reindex hook already uses.

Ordinary brain_edit is deliberately NOT audited — the third checkbox on #97, decided rather than left open. Git history already carries the same before/after bytes with a diff, and the asymmetry is that governance files change what later agents obey, not that note edits are harder to reconstruct. Routine note traffic in the same trail would also bury the rare governance record, which is how an alert stops being read. Pinned by a test, and the reasoning is in the brainEdit doc comment so it is not silently reversed.

Tests

Red-first. The two positive tests failed on an empty sink before the observer existed:

--- FAIL: TestBrainEditRoot_RecordsAnAuditEntryNamingPrincipalPathAndBothHashes
    "[]" should have 1 item(s), but has 0
--- FAIL: TestBrainEditRoot_NotifiesWithPathAndHashesButNeverContent
    "[]" should have 1 item(s), but has 0

Both hashes are asserted as literal sha256 values computed in the test, not just "non-empty".

Three negative tests passed from the start and pin behaviour that must not change: a refused call (no principal) writes nothing and emits neither; an authorised call with no match emits neither; an ordinary brain_edit on wiki/ emits neither.

task check exit 0 (build, vet, go test ./..., lint, gitleaks).

Not verified here

No live brain_edit_root was fired against the running service — that would rewrite the real CLAUDE.md to test a notification. Worth one deliberate live call after merge and deploy to confirm the ntfy topic actually delivers, since the test only proves Notify is called with the right body.

Risk: LOW — additive observability. The gate's behaviour is unchanged, and every new path is best-effort.

Closes #97. `brain_edit_root` was gated but unobserved — on success it emitted a `slog.Warn` and nothing else. That proves a rewrite was *allowed*, not that one *happened*, and brain-sync commits it within 15 minutes authored `brain-sync`, so git history cannot distinguish a `CLAUDE.md` rewrite from routine note traffic. ## What changed - **ntfy on every successful `brain_edit_root`** — reuses the existing `audit.NewNtfyNotifier` wiring and topic (`BRAIN_NTFY_URL`/`BRAIN_NTFY_TOKEN`). Extracted `buildNotifier()` so the capture sink and the governance observer share one construction path instead of two. - **Audit record** through the existing `capture.AuditEntry`/`AuditSink` — `Items: [path]`, `SecurityEvents: ["governance file rewritten: <path> before=<sha> after=<sha>"]`, principal from the authenticated context. - **`notes.Result.PrevContentHash`** — the before-hash. `Result` previously carried only the resulting hash, so a record could say what a file became but not what it stopped being. - Wired in `cmd/server/main.go` alongside `WithCapture`, which is what re-derives the static principal the gate checks. ## Decisions worth reviewing **Path and hashes only in the notification body.** It leaves the cluster for a phone through a third-party topic. A hash identifies the rewrite; the bytes would leak whatever was written into an instruction file. Asserted by a test that fails if `old_str`/`new_str` appear in the message. **Reserved at `Internal`, not `Confidential`.** `DegradingSink` *refuses* a Confidential record when loki is unreachable rather than buffering it — that would drop exactly the record that matters most. Internal buffers and reconciles. **Observation is best-effort and never fails the call.** The bytes are on disk and brain-sync will commit them, so returning an error would report a non-rewrite to the caller while the rewrite stands. A failure to observe logs at ERROR — the one case where the trail itself is what broke. Same reasoning the reindex hook already uses. **Ordinary `brain_edit` is deliberately NOT audited** — the third checkbox on #97, decided rather than left open. Git history already carries the same before/after bytes with a diff, and the asymmetry is that governance files change what later agents obey, not that note edits are harder to reconstruct. Routine note traffic in the same trail would also bury the rare governance record, which is how an alert stops being read. Pinned by a test, and the reasoning is in the `brainEdit` doc comment so it is not silently reversed. ## Tests Red-first. The two positive tests failed on an empty sink before the observer existed: ``` --- FAIL: TestBrainEditRoot_RecordsAnAuditEntryNamingPrincipalPathAndBothHashes "[]" should have 1 item(s), but has 0 --- FAIL: TestBrainEditRoot_NotifiesWithPathAndHashesButNeverContent "[]" should have 1 item(s), but has 0 ``` Both hashes are asserted as literal `sha256` values computed in the test, not just "non-empty". Three negative tests passed from the start and pin behaviour that must not change: a refused call (no principal) writes nothing and emits neither; an authorised call with no match emits neither; an ordinary `brain_edit` on `wiki/` emits neither. `task check` exit 0 (build, vet, `go test ./...`, lint, gitleaks). ## Not verified here No live `brain_edit_root` was fired against the running service — that would rewrite the real `CLAUDE.md` to test a notification. Worth one deliberate live call after merge and deploy to confirm the ntfy topic actually delivers, since the test only proves `Notify` is called with the right body. **Risk:** LOW — additive observability. The gate's behaviour is unchanged, and every new path is best-effort.
mathias added 1 commit 2026-08-20 15:30:51 +00:00
feat(mcp): record and alert on a governance-file rewrite (#97)
CI / Lint / Test / Vet (pull_request) Successful in 41s
CI / Mirror to GitHub (pull_request) Skipped
c2cdcb07f1
brain_edit_root was gated but unobserved: on success it emitted a
slog.Warn and nothing else. That proves a rewrite was allowed, not that
one happened — and brain-sync commits it within 15 minutes authored
`brain-sync`, so git history cannot tell a CLAUDE.md rewrite apart from
routine note traffic. CLAUDE.md is what the next agent loads as
instructions.

A successful brain_edit_root now emits an ntfy notification and an
AuditEntry through the same sink a capture already uses. notes.Result
gains PrevContentHash so both ends of the change are identifiable.

Path and hashes only in the notification body. It leaves the cluster for
a phone via a third-party topic, and a hash identifies the rewrite while
the bytes would leak whatever was written into an instruction file.

Reserved at Internal, not Confidential: Confidential makes DegradingSink
refuse to record while loki is down rather than buffer, which would drop
exactly the record that matters most.

Observation is best-effort and never fails the call. The bytes are
already on disk and will be committed, so returning an error would report
a non-rewrite to the caller while the rewrite stands. A failure to
observe logs at ERROR — that is the case where the trail itself broke.

Ordinary brain_edit on knowledge//wiki/ is deliberately NOT audited. Git
history already carries the same before/after bytes with a diff; the
asymmetry is that governance files change what later agents obey. Routine
note traffic in the same trail would also bury the rare governance record,
which is how an alert stops being read.

Red-first: the audit-contents and notification tests failed on an empty
sink before the observer existed. The three negative tests (refused call,
failed edit, ordinary note edit) passed from the start, pinning behaviour
that must not change.

Refs #97, #96, brain#4.
mathias merged commit 36a45068ab into main 2026-08-22 06:09:53 +00:00
Sign in to join this conversation.