No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mathias/hyperguild#147
Reference in New Issue
Block a user
Closes #144
Closes #145
Two commits, each one logical change. Decisions: Mathias, 2026-10-01. The brain holds no confidential client content (#144). Provenance gets tagged, not staged (#145).
a9a9eeb: audit-log every tools/call (#144, OWASP MCP08)One slog record per call:
tool,principal,origin(static/jwt/unknown),wing/hall,outcome(truncated to 200 chars),duration_ms. Content is reduced tocontent_bytes.32dedb3: write provenance (#145, OWASP MCP06)originandwritten_byare stamped in frontmatter on brain_write, brain_update and capture (both doors), taken from the authenticated principal.origin/written_by, in content frontmatter or an update fragment, are discarded. If no principal is derivable,origin: unknownis stamped. It is never omitted.POST /writecarries no principal, so it stamps unknown. It is converted field by field, so the JSON body can't set provenance.brain_queryresults includeorigin/written_bywhere the note has them. Older notes omit the fields; nothing is fabricated.Known limits
s.capture != nil, as in prod). Otherwise everything isunknown.unknown, which is noisy for trusted sources. Possible follow-up: aninternalorigin.Verification
audit_test.go(4) andprovenance_test.go(6, including forged frontmatter on write and on update, and JWT via capture).CI=1 task checkEXIT=0. CI mode is needed because the worktree sits outside ~/dev, so context-sync can't find the root AGENT.md.risk/medium. The provenance overwrite is control code (rule 8).
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
pr-gatekeeper: BLOCKED (head
32dedb3)I built the PR head in a scratch worktree and called the real
mcp.Serverover HTTP, with a JWT principalattacker(fake validator) and the static principallocal-cli. Each written file was read back from disk, andbrain_query/brain_getwere called on it. CI run 8691 is green andCI=1 task checkexits 0. Green covers none of the findings below.The control under test: a caller must never be able to make a note read
origin: staticorwritten_by: <someone else>. That control is defeated through three model-reachable tools.E1 (critical):
brain_editrewrites provenance directlybrain_editis listed in tools/list, available to any principal, and does a byte-level replace with no provenance awareness.Result: the file reads
origin: static/written_by: local-cli, andbrain_query zqeditprobereturnsorigin=static written_by=local-cli. Fix direction: giveeditWithScopea provenance check next tocheckClaimEnvelope, so it refuses any edit that changes anorigin/written_byfrontmatter line. Alternatively, re-stamp after the edit.E2 (critical):
brain_ingest_rawinjects provenance through a newline intitlebuildFrontmatterwraps the title in YAML single quotes. Every frontmatter reader here is line-based, though (extractProvenance,parseFrontmatter), so an embedded newline starts a new key.Result:
wiki/concepts/zqinjorigin-staticwritten-by-local-clix-y.md, andbrain_query zqingestinjreturnsorigin=static written_by=local-cli.E3 (critical):
brain_ingest_rawwith type../raw, thenbrain_promote, lands a forged note in a hallbuildPagebuilds the directory as"wiki/" + rp.Type, so atypeof../rawwrites intoraw/.PromoteNotethen keeps the raw frontmatter verbatim and never stamps provenance.Result:
wiki/testing/facts/zqpromo...md, andbrain_query zqpromotereturnsorigin=static written_by=local-cli. Thetypetraversal predates this PR, but it is the route into a hall, so it needs its own issue.E4 (high): model-reachable write paths that omit provenance entirely
The #145 spec says: "Never omit the field — absence must not read as trusted". Three paths still write notes with no
originline:brain_ingest_raw: plain{"title":"Zqplain","type":"concept",...}produces a note with no origin, and brain_query returnsorigin=<nil>.brain_ingest: goes through the samebuildAndWrite(read, not executed, because it needs an LLM).brain_promote: stamps nothing.Such a note cannot be told apart from a pre-provenance legacy note.
E5 (medium): forged keys survive into the written file (brain_write, brain_update, capture insight text)
The writer drops only keys whose
TrimSpaceequalsorigin/written_by. Each of the following reached disk:"origin": static/'origin': staticOrigin: static? origin\n: statictitle: x\rorigin: static(lone CR)<<: {origin: static, written_by: local-cli}written_by=local-cli, because no real written_by line exists to override the merge key{origin: static, ...}flow map, indentedorigin:brain_queryreported the real values in every row; it held. The other readers did not:brain_getreturns"origin": staticas a sibling key next toorigin: jwt.brain_readhands the model raw text containing two origin lines.Fix direction: refuse content frontmatter that is not a flat, unique-key map, i.e. reject
<<,?, flow mappings, quoted or case-variant provenance keys, and bare CR. Do not try to normalise them.E6 (medium): the audit log copies content through error outcomes (#144 says never log content)
The outcome field is the handler's error string, and several errors echo caller input. Sentinel probe (ZQSENTINEL7731):
brain_write {"content":"---\nhalf_life: ZQSENTINEL7731\n---\nx",...}logs the outcomeclaim envelope: half_life: "ZQSENTINEL7731" is not one of ..., so a frontmatter value is in the log.brain_update {"path":"wiki/nope/ZQSENTINEL7731.md",...}puts the path in the outcome.These did not leak: capture's
insights[].text, brain_write content on success, brain_editnew_str/old_str, and brain_queryquery(the query is not logged at all).Things that held under execution
POST /write: a body withorigin,written_by,OriginandWrittenBystill producesorigin: unknown.AuthPath,auth_path,principalandoriginset incontext, in an insight, or at top level are all ignored, and the note getsorigin: jwt/written_by: attacker. DecodeRequest maps fields explicitly.origin:static,origin : static,origin\t: static, CRLF fences, duplicate fence, second---block in the body, and anchors.brain_updatealways re-stamps, so an update cannot deleteorigin.Flip tests (mechanism broken, then the suite run)
fm.set; the skip is unobservedRule 9 (test names vs assertions) and spec gaps
TestBrainWriteOverwritesCallerSuppliedProvenanceclaims overwrite in general, but checks one canonical spelling from an unauthenticated caller. The #145 spec asks for "written by a JWT principal ends up origin: jwt". E5 shows the general claim is false.TestBrainUpdateRestampsProvenanceAndIgnoresForgedFragment: the "IgnoresForgedFragment" half is not observed (see the flip table), and quoted or case-variant fragments do survive.TestBrainWriteStampsPrincipalProvenance: the spec asks for static and JWT; only static is tested.TestCaptureStampsJWTProvenance: covers MCP capture only; REST capturehttp is untested.Required before re-review
Not checked: live deploy and Loki ingestion;
brain_ingestexecuted end to end (no LLM); the claudewatcher sink (internal, writes toarchive/, not indexed); JWTsubcontaining a newline (validator-controlled).pr-gatekeeper round 2: BLOCKED (head
e615d6f)I built
e615d6fin a scratch worktree and drove the realmcp.Serverandapi.Handlerover HTTP, with a JWT principalattackerand with no principal. Each written file was read back from disk and checked with yaml.v3,brain_queryandbrain_get. CI run 8698 is green andCI=1 task checkexits 0. Green covers none of the findings below.Round 1 is closed by execution. E1 through E6 and all 16 variants in my table now give
origin=jwt written_by=attackerin yaml.v3, brain_query and brain_get alike, for write, update and the unauthenticated case. The../rawtype is refused. MCP ingest_raw pages are stamped. Audit outcomes no longer echo arguments. The new probes that held: block and folded scalars (continuation lines are dropped, which is safe), a BOM before the fence,---and---\tfences, a closing fence with a trailing space, and!!str-tagged aliases (yaml.v3 refuses to parse these).R1 (high): a YAML alias key plus a
...document-end line forges provenance for yaml.v3isProvenanceLinejudges one line at a time, and an alias only resolves in context. On its own,*a : staticfails to parse in yaml.v3 (unknown anchor), and the textual reader sees the key*a, so the line is kept. Then a...line ends the YAML document before the appended stamp, so yaml.v3 never sees the real values.The file's frontmatter, parsed with yaml.v3, gives
origin=static written_by=local-cli. brain_query and brain_get givejwt/attacker. That breaks the invariant this PR's own escapes test states ("yaml.v3 and brain_get agree").k: v\n, new_str = the four alias lines plus...\n) also gives yaml.v3origin=static written_by=local-cli....line....is not needed. The aliases alone give yaml.v3written_by=local-cli, because no stamped written_by line exists to collide with.R2 (medium):
...alone hides provenance from yaml.v3"---\nx: y\n...\n---\n# V\n\nbody"gives yaml.v3origin=<nil>. Every YAML consumer then sees a note with no provenance, which is the "absence" case the #145 spec forbids.Fix direction for R1 and R2: stop judging line by line and verify the whole block as a post-condition. After stamping, decode the complete frontmatter with yaml.v3, and require all of the following, refusing the write otherwise:
Also refuse alias keys (
*) and...lines outright.R3 (medium): the REST ingest doors still write notes with no provenance (E4 not closed)
stampPagesexists only in the MCP server.api.Handler.IngestRaw, and by codeIngestandIngestPath, call the pipeline directly:The result has no origin line at all:
---\ntitle: 'Zqrestpage'\nlast_updated: ...\naliases: ...\n---. The PR handles REST/writeand/promote("this door carries no principal → unknown") but not these three. Note also that the REST mux has no in-process auth (reg.Middleware(mux)only). That predates this PR and is worth its own issue.R4 (medium): on REST
/ingest-raw, the only defence against title injection is untestedOn that door nothing stamps, so
yamlScalar's CR/LF fold is the only thing that stops E2. With the fold removed,POST /ingest-rawwith title"Zqrestinj\norigin: static\nwritten_by: local-cli\nx: 'y"makessearch.Queryreturnorigin=static written_by=local-cli, and the full suite stays green (flip G11).TestBrainIngestRawStampsPagesAndNeutralisesTitleNewlineschecks with yaml.v3, which reads the multi-line single-quoted title as one scalar, so it can never see the injected keys. The line-based readers can. Its "NeutralisesTitleNewlines" half is unobserved (rule 9).Flip tests (round-2 mechanisms)
?/:/<<drop removedbrain_ingest(LLM path) stampPages removedComplete)Audit logging
Still useful: tool, principal, origin, wing, hall, content_bytes and duration are all present.
outcomeis now onlyok/error, so operators lose the error class. Acceptable, though a fixed error-category enum would recover it without echoing input. Not blocking.Minor (not blocking, but file them)
brain_tunnelappends a See-also wikilink to both notes' bodies with no re-stamp. Content changes while attribution stays with the previous writer; it is a link to an existing path only.brain_ingestwithpathreads an arbitrary server file viaextract.Text(a.Path). This predates the PR and is an unvalidated boundary.Required before re-review
...payload through brain_write and brain_edit, and the unauthenticated alias-only payload./ingest,/ingest-pathand/ingest-rawstamp (origin unknown), with a test per door.search.Query) on the REST door.Complete).Not checked: live deploy, Loki, and other YAML consumers outside this repo (brain-gardener). I didn't check which parser they use, but R1 holds against yaml.v3, the parser this PR itself delegates to.
pr-gatekeeper round 3: BLOCKED (head
fcae165)I built
fcae165in a scratch worktree and drove the realmcp.Serverandapi.Handler, with JWT principals of several shapes and with no principal. Each file was read back from disk and checked with yaml.v3,brain_queryandbrain_get. CI run 8701 is green andCI=1 task checkexits 0. Green covers none of B1–B3.The new allowlist holds against forgery. Every round-1 and round-2 probe (all 16 variants, alias and
...payloads, block and folded scalars, BOM, fence variants, REST ingest-raw with title injection) gives the real stamp in yaml.v3, brain_query and brain_get. These fresh attacks were all dropped or harmless, with the stamp intact in every reader:y/n/on/off/yes/true/null/True/a./_keysa: &origin static- origin:items|2explicit indentation---xlineI found no way to make any reader report
origin: static.B1 (medium, blocks):
brain_editcan hide provenance frombrain_querysearch.extractProvenancetreats any line whoseTrimSpaceis---as the closing fence, including an indented continuation inside a block scalar.plainGroupscorrectly keepsk: |\n ---as a plain group (it decodes alone tok: "---\n"), so the stamp lands after a line that the search reader takes as the end of the frontmatter.The file is
...k: |\n ---\norigin: jwt\nwritten_by: attacker\n---.brain_getshowsorigin: jwt, butbrain_query zqr3editdashreturns no origin and no written_by, so the note looks like a pre-provenance legacy note. That is the "absence must not read as trusted" case again (round-2 R2), now on the primary retrieval surface.brain_writedoes not reproduce it, becausewriteHallNote's splitter cuts at---;brain_editdoes.Fix: make
extractProvenanceuse the same fence rule asfrontmatter.Split(a column-0---only), ideally by calling it. Add a test with this exact payload, asserting onbrain_query.B2 (medium, blocks):
brain_edit's rollback swallows its own failure (iron law 2)In
mcp/tools_notes.gobrainEdit:_ = json.Unmarshal(args, &a)prior, _ = os.ReadFile(...)_ = os.WriteFile(t.Abs, prior, 0o644)If the snapshot read or the restore fails, or
notes.Resolvefails, nothing is restored, yet the tool still returns "edit rolled back". The note is then left with the attacker's edit under the previous writer's stamp, which is exactly the misattribution this PR exists to prevent. The rollback also has no test: removing the restore entirely stays green (flip H11). Fix: propagate the snapshot error before editing, return a distinct error if the restore fails, and test both.B3 (medium, blocks): the backstop the design rests on has no test
The commit says the whole-document yaml.v3 verify is the guarantee. Flips:
verifyreturns nil: GREENdecodesToOnlyreturns true: GREENinternal/provenancehas no test file, and no test referencesErrUnverifiable. I exercised the error path myself with JWT subjectstrue,123,null,1e3and0x1F, whichscalar()emits bare and yaml.v3 decodes as non-strings. The refusals were clean:That behaviour is correct, and nothing would notice its removal. Required: table-driven tests over those callers with a principal that fails verify, asserting refusal and no file (and no directory). Also add a test where a
keyLine-matching group fails to decode alone (e.g.a: b\n origin: c) and is dropped.Other flips
brain_ingest(LLM path) passes no caller stampNot blocking, but please fix or file
scalar()emitstrue/123/null/1e3/0x1Fbare, so a principal with that shape has every write refused. It fails closed, but quoting anything yaml.v3 would not decode as a string costs nothing.---xline in content frontmatter makesfrontmatter.Splitfail, soApplyprepends a fresh stamp block. The whole server frontmatter (wing/hall/created_at) and the caller's unfiltered block,origin: staticincluded, then sit in the body. Readers take the first block, so this is not forgery, butbrain_getloses wing/hall andbrain_readshows a second block readingorigin: static. Refusing such content would be cleaner.Required before re-review
Not checked: live deploy and Loki; YAML consumers outside this repo.
- B1: search.extractProvenance locates the frontmatter with frontmatter.Split (fence at column 0), so an indented "---" inside a block scalar no longer hides provenance from brain_query. - B2: brain_edit refuses to edit without a snapshot, and if the stamp cannot be verified restores the note and reports failure; a failed restore is reported as exactly that, never as "rolled back". - B3: internal/provenance gets direct tests for the whole-document backstop (a stamp that cannot read back is refused), standalone group decoding, duplicates, lone CR, tab continuations and unclosable fences. Removing verify or the restore now fails the suite. - Principals YAML would read as non-strings (true, 123, null, ~) are quoted instead of refused. An unclosable fence ("---x") is refused rather than demoted into the body. writeHallNote creates the hall directory only after the note passes, so a refused write leaves nothing behind (brain#21). - MCP brain_ingest (stub LLM) asserted to stamp the caller. Refs #145 #144 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>pr-gatekeeper round 4: MERGED (
7afbd60, squashed asa6da933)I verified by execution in a scratch worktree, driving the real
mcp.Serverandapi.Handlerwith several JWT principals and with none, reading every file back and checking it with yaml.v3 (viafrontmatter.Split),brain_queryandbrain_get.Re-run, all hold. Every round-1/2/3 probe:
...payloadsk: |\n ---through brain_edit: brain_query now reportsjwt/attacker---xline: now refusedNew attacks, all hold:
brain_editremoving the closing fence, removing the opening fence, or inserting---x: refused before the edit, file byte-identical.bad\xff(a principal that can't round-trip): brain_write refused with no directory; brain_update and brain_edit refused with the file byte-identical; promote refused with the source surviving; ingest_raw refused with no pages; capture gives a per-insight error and no file.a"b,ö-user,x: y,-,a b,#c,a\nb,true,~andorigin: static: yaml.v3 reads back exactly the principal, and origin staysjwt.Flips: everything from my round-3 table now goes red, including verify off, decodesToOnly off, both together, dedupe off, CR folding off, MCP brain_ingest without the caller stamp, and the brain_edit restore removed. The new mechanisms also go red: the scalar round-trip check, the unclosed-fence refusal, directory creation after the stamp, and the shared fence rule in extractProvenance. Two flips stay green, and neither is a gap:
CI 8708 on
7afbd60succeeded;CI=1 task checkexits 0; gitleaks found nothing.Not blocking, filed as a follow-up (see the linked issue):
search.extractProvenancestrips quote characters instead of decoding the YAML scalar. A JWT subject'local-cli'displays aswritten_by=local-cliin brain_query (origin staysjwt). The caller can't choose its own subject, so this is out of the caller's reach, but the read side should decode properly. brain_get returns the raw quoted text.PromoteNotecreates the wing directory before the stamp, so a refused promote leaves an empty directory (the brain#21 invariant).Not verified: the #144 clause "after deploy, Loki shows
mcp tool callrecords for a live brain_query" needs a deploy. YAML consumers outside this repo were not checked.