brain_update bypasses claim-envelope validation — a note can be superseded into a bad envelope #111

New Issue

2026-08-23T05:37:57Z

mathias commented

2026-08-23 05:37:57 +00:00

Found while checking close-session compatibility, 2026-08-23. An asymmetry introduced by #102 and not noticed at the time.

The gap

claim.ValidateEnvelope is called from writeHallNote (internal/api/handler.go), so it guards the create path:

capture → brainstore.Write → api.WriteNote → writeHallNote → ValidateEnvelope ✓

The supersede path does not go through writeHallNote:

brain_update → brainstore.Update → api.UpdateNote → (no validation) ✗

So half_life: forever is rejected on create and accepted on supersede. Worse, an envelope field can be superseded in without claim_provenance, which is the exact laundering the provenance requirement exists to prevent — a value whose author is unknown, indistinguishable from one a human asserted.

Why it matters more than it looks

brain_update is a whole-note body replace, and the agents that use it regenerate whole notes. So the supersede path is the more likely one to carry model-authored frontmatter, not the less likely one. Validating only the create path guards the direction that needs it least.

Fix

Call ValidateEnvelope in UpdateNote too, before the write, with the same fail-closed posture — reject and write nothing.

caveman: same rules when note change as when note born
Red-first: brain_update with half_life: forever is rejected and the note is unchanged on disk
Red-first: brain_update adding applies: without claim_provenance is rejected
An update carrying no envelope fields is byte-identical to today
Check whether any other write path reaches disk without passing through writeHallNote — the lesson here is that the validator was attached to a function rather than to the boundary

That last box is the real one. Attaching validation to writeHallNote assumed it was the only door. It isn't.

Risk: LOW to fix, additive. The current gap is a hole in a control shipped one day earlier, which is the kind that ages badly precisely because everyone believes it is closed.

Refs: #102, brain#21.

Found while checking `close-session` compatibility, 2026-08-23. An asymmetry introduced by #102 and not noticed at the time. ## The gap `claim.ValidateEnvelope` is called from `writeHallNote` (`internal/api/handler.go`), so it guards the **create** path: ``` capture → brainstore.Write → api.WriteNote → writeHallNote → ValidateEnvelope ✓ ``` The **supersede** path does not go through `writeHallNote`: ``` brain_update → brainstore.Update → api.UpdateNote → (no validation) ✗ ``` So `half_life: forever` is rejected on create and accepted on supersede. Worse, an envelope field can be superseded in *without* `claim_provenance`, which is the exact laundering the provenance requirement exists to prevent — a value whose author is unknown, indistinguishable from one a human asserted. ## Why it matters more than it looks `brain_update` is a whole-note body replace, and the agents that use it regenerate whole notes. So the supersede path is the *more* likely one to carry model-authored frontmatter, not the less likely one. Validating only the create path guards the direction that needs it least. ## Fix Call `ValidateEnvelope` in `UpdateNote` too, before the write, with the same fail-closed posture — reject and write nothing. - [ ] `caveman: same rules when note change as when note born` - [ ] Red-first: `brain_update` with `half_life: forever` is rejected and the note is unchanged on disk - [ ] Red-first: `brain_update` adding `applies:` without `claim_provenance` is rejected - [ ] An update carrying no envelope fields is byte-identical to today - [ ] Check whether any other write path reaches disk without passing through `writeHallNote` — the lesson here is that the validator was attached to a function rather than to the boundary That last box is the real one. Attaching validation to `writeHallNote` assumed it was the only door. It isn't. **Risk:** LOW to fix, additive. The current gap is a hole in a control shipped one day earlier, which is the kind that ages badly precisely because everyone believes it is closed. Refs: #102, brain#21.

mathias added the risk/medium size/s type/fix labels 2026-08-27 05:25:42 +00:00

mathias referenced this issue

2026-09-22 21:20:52 +00:00

Audit remaining write paths that reach disk without claim-envelope validation (brain_edit, ingestion pipeline) #123

mathias referenced a pull request that will close this issue

2026-09-22 21:21:05 +00:00

Brain-quality sweep, batch 1: brain_edit/brain_update correctness + close-session docs #124

mathias referenced this issue from a commit

2026-09-22 21:24:53 +00:00

fix(api): validate the merged envelope on supersede, not the caller's fragment

mathias closed this issue

2026-09-22 21:27:11 +00:00

mathias commented

2026-09-22 21:27:31 +00:00

Verified and merged in mathias/hyperguild#124, commit 5a5d938e8e.

Second-round fix (e2cb315) builds the claim envelope map from fm (post-merge, pre-write) instead of the caller's raw contentFM fragment, in ingestion/internal/api/update.go. Independently re-verified before merging:

Flip-tested TestUpdateNoteRejectsClearingProvenanceWhileEnvelopeSurvives myself: reverted the envelope map to build from contentFM again, confirmed the test goes red ("An error is expected but got nil"); restored the fix, confirmed green.
Adversarial case (mine, not in the PR's own tests): prior note with no envelope at all, update sets half_life: durable alone -> correctly rejected, file unchanged.
Confirmed updated_at/supersedes cannot be spoofed via the caller's fragment: both are unconditionally re-set after the merge loop.
Noted, non-blocking: supersede_reason set via the caller's own fragment survives when opts.Reason is empty -- same-actor two ways to set the same field, not a cross-update provenance leak like the envelope bug, so not treated as blocking.

CI green on the merged head SHA (run 7795, e2cb315, conclusion success).

Verified and merged in mathias/hyperguild#124, commit 5a5d938e8e1d0f9d356e9d0d1d2bfed80dc506ad. Second-round fix (e2cb315) builds the claim envelope map from `fm` (post-merge, pre-write) instead of the caller's raw `contentFM` fragment, in ingestion/internal/api/update.go. Independently re-verified before merging: - Flip-tested TestUpdateNoteRejectsClearingProvenanceWhileEnvelopeSurvives myself: reverted the envelope map to build from `contentFM` again, confirmed the test goes red ("An error is expected but got nil"); restored the fix, confirmed green. - Adversarial case (mine, not in the PR's own tests): prior note with no envelope at all, update sets `half_life: durable` alone -> correctly rejected, file unchanged. - Confirmed `updated_at`/`supersedes` cannot be spoofed via the caller's fragment: both are unconditionally re-set after the merge loop. - Noted, non-blocking: `supersede_reason` set via the caller's own fragment survives when `opts.Reason` is empty -- same-actor two ways to set the same field, not a cross-update provenance leak like the envelope bug, so not treated as blocking. CI green on the merged head SHA (run 7795, e2cb315, conclusion success).

mathias referenced this issue

2026-09-22 21:28:34 +00:00

UpdateNote: supersede_reason settable via caller's own frontmatter fragment, bypassing the Reason option #125

mathias referenced this issue from a commit

2026-09-23 08:14:10 +00:00

fix(mcp): validate the claim envelope before brain_edit writes too

mathias referenced this issue

2026-09-23 08:14:18 +00:00

fix(mcp): validate the claim envelope before brain_edit writes too #136

mathias referenced this issue from a commit

2026-09-23 08:16:27 +00:00

fix(api): UpdateNote reserves updated_at/supersedes/supersede_reason from the caller's fragment