Fixes#169. risk/high: an authenticated path traversal to arbitrary file rewrite. Control code, adversarial review required.
Change
checkNotePath: only a clean (path.Clean(p) == p), local wiki/.../<slug>.md path is accepted, with no ., .. or empty segments. Checked before anything else.
WriteTunnel: reads both notes with brainfs.ReadRegular and rewrites them with the new brainfs.WriteRegular. appendSeeAlso becomes the pure withSeeAlso.
brainfs.WriteRegular:
never creates a file
Lstats through the root, then opens with O_NOFOLLOW|O_NONBLOCK
requires the opened handle to be the same regular file (os.SameFile)
Why SameFile and not just O_NOFOLLOW:os.Root.OpenFile follows an in-root final symlink even with O_NOFOLLOW. Verified on go1.27.1: os.OpenFile returns ELOOP, Root.OpenFile opens the target. A test using an in-brain symlink caught this; the original outside-symlink test could not, because os.Root refuses escapes anyway.
The source is no longer rewritten when the target is refused.
Tests
All red on main before the fix.
TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain: a file outside the brain was modified.
TestWriteTunnelRefusesUncleanOrNonWikiPaths: 6 bad shapes, plus a positive control.
TestWriteTunnelDoesNotWriteThroughASymlinkedNote
TestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget: the target is swapped between a regular note and an in-brain symlink to raw/victim.md. Positive control: some tunnels succeed.
TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles: in-brain symlink, FIFO with a reader held open, escape, missing file.
Flips
Red:
path check off
target read via os.ReadFile
target write via os.WriteFile
SameFile dropped
Green, by design:
Lstat type check dropped: SameFile still refuses.
O_CREATE added: Lstat fails on a missing file first.
Handle type check dropped: the kernel refuses FIFO truncate and directory write anyway.
Fixes #169. risk/high: an authenticated path traversal to arbitrary file rewrite. Control code, adversarial review required.
## Change
- **`checkNotePath`:** only a clean (`path.Clean(p) == p`), local `wiki/.../<slug>.md` path is accepted, with no `.`, `..` or empty segments. Checked before anything else.
- **WriteTunnel:** reads both notes with `brainfs.ReadRegular` and rewrites them with the new `brainfs.WriteRegular`. `appendSeeAlso` becomes the pure `withSeeAlso`.
- **`brainfs.WriteRegular`:**
- never creates a file
- Lstats through the root, then opens with `O_NOFOLLOW|O_NONBLOCK`
- requires the opened handle to be **the same regular file** (`os.SameFile`)
- **Why SameFile and not just O_NOFOLLOW:** `os.Root.OpenFile` follows an **in-root final symlink even with O_NOFOLLOW**. Verified on go1.27.1: `os.OpenFile` returns ELOOP, `Root.OpenFile` opens the target. A test using an in-brain symlink caught this; the original outside-symlink test could not, because os.Root refuses escapes anyway.
- The source is no longer rewritten when the target is refused.
## Tests
All red on main before the fix.
- `TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain`: a file outside the brain was modified.
- `TestWriteTunnelRefusesUncleanOrNonWikiPaths`: 6 bad shapes, plus a positive control.
- `TestWriteTunnelDoesNotWriteThroughASymlinkedNote`
- `TestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget`: the target is swapped between a regular note and an in-brain symlink to `raw/victim.md`. Positive control: some tunnels succeed.
- `TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles`: in-brain symlink, FIFO with a reader held open, escape, missing file.
## Flips
Red:
- path check off
- target read via `os.ReadFile`
- target write via `os.WriteFile`
- `SameFile` dropped
Green, by design:
- **Lstat type check dropped:** SameFile still refuses.
- **`O_CREATE` added:** Lstat fails on a missing file first.
- **Handle type check dropped:** the kernel refuses FIFO truncate and directory write anyway.
## Not covered
- Intermediate in-brain symlinks (accepted residual, #167).
- `AutoTunnel`'s `logFuzzyCandidates` appends to `raw/tunnel-candidates-*.md` with `os.OpenFile` (write path, #158).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
mathias
changed title from fix(tunnel): brain_tunnel cannot write outside the wiki or through symlinks (#169) to fix: brain_tunnel, brain_update and session_log confine caller-supplied paths (#169)2026-10-04 20:38:18 +00:00
Posted late: the round-1 verdict went to the coordinator but not to this thread. Exploit inputs are left out because the repo is public.
The brain_tunnel fix held. 17 path shapes were refused:
.., ./ and // segments
encoded and look-alike dots, backslashes, NUL
a trailing slash, very long paths, case variants
Two claims checked by running them:
os.Root.OpenFile does follow an in-root final symlink even with O_NOFOLLOW (go1.27.1).
WriteRegular survived a regular-file/FIFO swap race without hanging.
Blocking (the PR said Fixes #169, and #169 said no other handler was affected; that was false):
brain_update: api.UpdateNote's resolveWithin confined the path only lexically. There was no knowledge/wiki scope, no .md rule and no symlink check, so one call could supersede governance files and raw/ and sessions/ files, and write through a final symlink out of the brain.
session_log: session.Append joined the caller's session_id into the path and opened it with O_CREATE, so a token holder could create or append files outside sessions/.
Non-blocking:
Flips that went green:
the .md rule: the .txt case named a file that did not exist
the IsLocal check: redundant
O_NONBLOCK in WriteRegular: needed only in a race, which my probe showed
the Lstat type check
ignored Write/Close errors
Two test names claimed more than their assertions prove.
DetectTunnels (readTitleAndCreated) blocks on a FIFO under wiki/. This was already true on main.
CI run 9178 was green. Green covered none of the blocking findings.
## Adversarial review, round 1 (head 9eb3d24): BLOCKED
Posted late: the round-1 verdict went to the coordinator but not to this thread. Exploit inputs are left out because the repo is public.
**The brain_tunnel fix held.** 17 path shapes were refused:
- `..`, `./` and `//` segments
- encoded and look-alike dots, backslashes, NUL
- a trailing slash, very long paths, case variants
**Two claims checked by running them:**
- `os.Root.OpenFile` does follow an in-root final symlink even with O_NOFOLLOW (go1.27.1).
- WriteRegular survived a regular-file/FIFO swap race without hanging.
**Blocking (the PR said `Fixes #169`, and #169 said no other handler was affected; that was false):**
1. `brain_update`: `api.UpdateNote`'s `resolveWithin` confined the path only lexically. There was no knowledge/wiki scope, no `.md` rule and no symlink check, so one call could supersede governance files and raw/ and sessions/ files, and write through a final symlink out of the brain.
2. `session_log`: `session.Append` joined the caller's `session_id` into the path and opened it with O_CREATE, so a token holder could create or append files outside sessions/.
**Non-blocking:**
- Flips that went green:
- the `.md` rule: the `.txt` case named a file that did not exist
- the IsLocal check: redundant
- O_NONBLOCK in WriteRegular: needed only in a race, which my probe showed
- the Lstat type check
- ignored Write/Close errors
- Two test names claimed more than their assertions prove.
- `DetectTunnels` (`readTitleAndCreated`) blocks on a FIFO under wiki/. This was already true on main.
CI run 9178 was green. Green covered none of the blocking findings.
Both round-1 handlers are fixed in the code, and I could not get past either fix through the MCP handlers. I'm blocking because two of the guards this round adds have no test that fails when they are removed.
Verified by execution (scratch brain, through brainUpdate / sessionLog / brainIngestRaw)
brain_update refused all of these:
governance files, raw/ and sessions/, including via ./, wiki/../ and the slug-with-slash route
final symlinks, whether they point inside or outside the brain
backslashes, NUL, case variants, FIFOs, missing notes
.. wing/slug
brain_update still accepts paths that only look different, such as a leading / or a trailing /.. These resolve to the same note and are harmless.
session_log refused traversal, separators, NUL, newline, dot-leading names, non-ASCII, look-alike dots and over-long ids. It refused a symlinked log (ELOOP), and a FIFO log with no reader returned ENXIO instead of hanging. Nothing was created outside sessions/.
brain_ingest_raw: a page type containing path segments is skipped by validPageType (#147).
Blocking: the flip stayed green on guards this PR ships
session.idPattern's ban on /: allowing / in the tail of the regex leaves every test green. The a/b case fails only because sessions/a/ doesn't exist. No case begins with a plain character and then climbs out with .., and that is exactly the escape the ban stops: with the ban removed, my probe's id created a file beside sessions/. Fix: add such a case, and assert on the refusal itself (a sentinel error, or that nothing exists outside), not on any error.
O_NONBLOCK in session.Append: removing it leaves every test green. Unlike WriteRegular, Append does no Lstat type check, so O_NONBLOCK is the only thing that stops a FIFO named <id>.jsonl from hanging session_log forever. My probe hangs without it. Fix: add a FIFO test with no reader and a timeout.
Flips that went red (good)
UpdateNote: scope off
UpdateNote: write via os.WriteFile
session: id check off
session: a leading-dot id allowed
session: O_NOFOLLOW off
session: the Read id check off
tunnel: the .md rule off
Non-blocking
UpdateNote reading via os.ReadFile instead of ReadRegular is green. That's acceptable: notes.Resolve already refuses a final symlink, the write refuses a swapped one, and the prior content is never returned.
Writes follow intermediate in-brain directory symlinks. A directory symlink inside wiki/ or knowledge/ that points elsewhere in the brain lets brain_update (and brain_tunnel) rewrite an existing .md there, for example in raw/. Governance files and .jsonl are not reachable because of the .md/scope rule. Likewise, if sessions/ itself is a symlink, session_log follows it.
This needs a planted symlink, and the tripwire removes symlinks within 30 s.
But it is a static redirect on a write path, and #167 tracks a read-side race. The WriteRegular doc cites #167 for this, and #167 doesn't cover it. Record it there, or in its own issue, so the acceptance is explicit.
brain_update hangs if there is a FIFO anywhere under wiki/. A successful update runs wikiUpkeep → AutoTunnel → DetectTunnels, which opens every wiki .md with a blocking os.Open. This was already true on main and affects brain_write too. Not tracked in #157/#158 as far as I can see.
Still unchanged:
the WriteRegular O_NONBLOCK/Lstat-type redundancy: fine, because SameFile and the race probe cover it
ignored Write/Close errors: no test, but not swallowed in code
CI and gates
Run 9181 on 87fa28e was green, and it ran api, brain, brainfs and session.
CI=1 task check exited 0.
-race -count=5 on brain, brainfs, api and session: ok.
Green covers neither blocking finding.
Not checked
live pod state
brain_classify, brain_graph and capture internals beyond their argument shapes
REST /promote and /ingest-path, beyond sharing the MCP code paths
## Adversarial review, round 2 (head 87fa28e): BLOCKED
Both round-1 handlers are fixed in the code, and I could not get past either fix through the MCP handlers. I'm blocking because two of the guards this round adds have no test that fails when they are removed.
### Verified by execution (scratch brain, through `brainUpdate` / `sessionLog` / `brainIngestRaw`)
- **brain_update** refused all of these:
- governance files, raw/ and sessions/, including via `./`, `wiki/../` and the slug-with-slash route
- final symlinks, whether they point inside or outside the brain
- backslashes, NUL, case variants, FIFOs, missing notes
- `..` wing/slug
- **brain_update** still accepts paths that only look different, such as a leading `/` or a trailing `/.`. These resolve to the same note and are harmless.
- **session_log** refused traversal, separators, NUL, newline, dot-leading names, non-ASCII, look-alike dots and over-long ids. It refused a symlinked log (ELOOP), and a FIFO log with no reader returned ENXIO instead of hanging. Nothing was created outside sessions/.
- **brain_ingest_raw:** a page `type` containing path segments is skipped by `validPageType` (#147).
### Blocking: the flip stayed green on guards this PR ships
1. **`session.idPattern`'s ban on `/`:** allowing `/` in the tail of the regex leaves every test green. The `a/b` case fails only because `sessions/a/` doesn't exist. No case begins with a plain character and then climbs out with `..`, and that is exactly the escape the ban stops: with the ban removed, my probe's id created a file beside sessions/.
**Fix:** add such a case, and assert on the refusal itself (a sentinel error, or that nothing exists outside), not on any error.
2. **`O_NONBLOCK` in `session.Append`:** removing it leaves every test green. Unlike WriteRegular, Append does no Lstat type check, so O_NONBLOCK is the *only* thing that stops a FIFO named `<id>.jsonl` from hanging `session_log` forever. My probe hangs without it.
**Fix:** add a FIFO test with no reader and a timeout.
### Flips that went red (good)
- UpdateNote: scope off
- UpdateNote: write via `os.WriteFile`
- session: id check off
- session: a leading-dot id allowed
- session: O_NOFOLLOW off
- session: the Read id check off
- tunnel: the `.md` rule off
### Non-blocking
- **UpdateNote reading via `os.ReadFile`** instead of `ReadRegular` is green. That's acceptable: `notes.Resolve` already refuses a final symlink, the write refuses a swapped one, and the prior content is never returned.
- **Writes follow intermediate in-brain directory symlinks.** A directory symlink inside wiki/ or knowledge/ that points elsewhere in the brain lets `brain_update` (and `brain_tunnel`) rewrite an existing `.md` there, for example in raw/. Governance files and `.jsonl` are not reachable because of the `.md`/scope rule. Likewise, if sessions/ itself is a symlink, `session_log` follows it.
- This needs a planted symlink, and the tripwire removes symlinks within 30 s.
- **But it is a static redirect on a write path**, and #167 tracks a read-side *race*. The `WriteRegular` doc cites #167 for this, and #167 doesn't cover it. Record it there, or in its own issue, so the acceptance is explicit.
- **brain_update hangs if there is a FIFO anywhere under wiki/.** A successful update runs `wikiUpkeep → AutoTunnel → DetectTunnels`, which opens every wiki `.md` with a blocking `os.Open`. This was already true on main and affects brain_write too. Not tracked in #157/#158 as far as I can see.
- **Still unchanged:**
- the WriteRegular O_NONBLOCK/Lstat-type redundancy: fine, because SameFile and the race probe cover it
- ignored Write/Close errors: no test, but not swallowed in code
### CI and gates
- Run 9181 on 87fa28e was green, and it ran api, brain, brainfs and session.
- `CI=1 task check` exited 0.
- `-race -count=5` on brain, brainfs, api and session: ok.
- Green covers neither blocking finding.
### Not checked
- live pod state
- brain_classify, brain_graph and capture internals beyond their argument shapes
- REST /promote and /ingest-path, beyond sharing the MCP code paths
brain_tunnel passes caller-supplied source and target paths to
WriteTunnel, which checked only the first segments ("wiki/<wing>/...")
and joined them onto the brain dir. "wiki/b/x/../../../../tmp/f.md"
resolved outside the brain, and appendSeeAlso read and rewrote that file
(verified by execution): any MCP token holder could append to any
existing file the pod user can write, inside the brain (raw/, sessions/,
classification.yaml) or outside it.
- checkNotePath: only a clean, local wiki/.../<slug>.md path, with no
".", ".." or empty segments.
- Both notes are read and rewritten through brainfs. New
brainfs.WriteRegular: never creates; Lstat through the root, open with
O_NOFOLLOW|O_NONBLOCK, and require the opened handle to be the same
regular file (os.SameFile). os.Root.OpenFile follows an in-root final
symlink even with O_NOFOLLOW (verified on go1.27), so the flag alone
does not refuse it.
- The source is no longer rewritten when the target is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 1 on PR #170 found two more handlers with the #169 class of
bug (verified by execution):
- brain_update resolved its caller path with resolveWithin, a lexical
containment check: any brain file could be superseded
(classification.yaml, raw/, sessions/), and a final symlink was
written through. UpdateNote now resolves like brain_edit
(notes.Resolve, ScopeNotes: knowledge/ and wiki/ .md notes), and reads
and rewrites through brainfs (ReadRegular, WriteRegular).
- session_log joined the caller's session_id onto the sessions dir and
opened it O_CREATE|O_APPEND: an id with path segments created or
appended a .jsonl file anywhere the pod could write. The id must now
be a plain name (^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$, which every live
session file matches), and the open adds O_NOFOLLOW|O_NONBLOCK. Read
checks the id too.
- Tunnel tests: the .txt case now names an existing file (it failed on
"missing" before), the symlink test adds an in-brain target, and the
FIFO comment no longer claims the handle check is the only guard.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR #170 round-2 review: both guards stayed green when removed. Adds an
ErrInvalidID sentinel, a plain-prefix-then-'..' escape case asserted on
the sentinel, and a no-reader FIFO case with a timeout. Flip-verified:
allowing '/' in the id tail and dropping O_NONBLOCK each turn the suite red.
Refs #169
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Note for the session holding ~/worktrees/hg-tunnel:fix/tunnel-traversal was rebased onto main 77e32de and force-pushed with lease (expected 87fa28e). The new head is 6325c2b. It adds the two tests round 2 asked for: the ErrInvalidID climb-out case and the session FIFO timeout. Before pushing again from that worktree, run git fetch && git reset --hard origin/fix/tunnel-traversal. The worktree was clean and fully pushed at the time of the rebase. Round-3 review is running now.
**Note for the session holding `~/worktrees/hg-tunnel`:** `fix/tunnel-traversal` was rebased onto main 77e32de and force-pushed with lease (expected 87fa28e). The new head is 6325c2b. It adds the two tests round 2 asked for: the `ErrInvalidID` climb-out case and the session FIFO timeout. Before pushing again from that worktree, run `git fetch && git reset --hard origin/fix/tunnel-traversal`. The worktree was clean and fully pushed at the time of the rebase. Round-3 review is running now.
---
_Created via git-mcp on behalf of @mathias_
I found nothing that blocks. This PR is risk/high, so merging is Mathias's call and I have not merged it. Exploit inputs are left out because the repo is public.
Both round-2 blockers are fixed, and each flip went red
Allowing / in idPattern's tail turns TestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefix red. All three ids return nil where ErrInvalidID was expected, and both "nothing created" asserts fail.
Dropping O_NONBLOCK from session.Append turns TestAppendReturnsPromptlyOnAFIFOLogWithNoReader red ("Append blocked on a FIFO log with no reader", 3.00s). The test's own unblock path let it finish instead of hanging the suite.
Both test names match their assertions.
Rebase resolution (brainfs.go and brainfs_test.go): correct
git range-diff from 87fa28e to 6325c2b shows that WriteRegular and its test were only moved, from before ReadRegular to the end of the file. The bodies are byte-identical.
The diff against main is additions only, so nothing from #168 was dropped: ReadDirRegular and the os.SameFile check in readOpenedMax are both intact.
Traversal probes through the MCP handlers (brain_tunnel, brain_update, session_log)
I ran 192 probes, all through toolCall on mcp.NewServer, in a scratch brain plus an outside directory. Each probe was checked with an Lstat and sha256 snapshot of the whole tree, so nothing depended on the response text. The shapes covered:
.., absolute paths, and file://
final symlinks to outside, relative and absolute
intermediate directory symlinks to outside, at the wing level and at the hall level
%2e%2e, %2f and %2F, backslash, NUL
look-alike dots: U+2025, U+FF0E and U+2024
./, //, and climbing out after a .md segment
governance, raw/ and sessions/ targets
.. in wing or hall
for session_log: symlinked logs, FIFO logs, and ids with newlines or over 200 characters
Result on 6325c2b: 0 filesystem changes, and every probe was refused.
The positive control is the same harness run on origin/main 77e32de. It records 52 escapes (tunnel 13, update 29, session_log 10), and the three legitimate control calls write on both. So the harness can see an escape, and the PR closes all 52.
Gates
CI run 9283 on 6325c2b: success. The job log shows ok for ingestion api, brain, brainfs, mcp and session, with 0 FAIL lines.
CI=1 GOFLAGS=-p=2 task check in a fresh clone: exit 0, 0 FAIL lines, gitleaks clean.
-race -count=10 on session, brainfs and brain: exit 0.
Merge check: main has since moved to b6ef116 (#83 and a deps bump). PR 6325c2b merged onto it passes the api, brain, brainfs, mcp and session tests (exit 0), and Gitea reports the PR as mergeable.
Green alone would not have shown the round-2 gaps. The flips above are the evidence.
Non-blocking
session.Read still uses a blocking os.Open, which follows a final symlink and hangs on a FIFO. It has no non-test caller (only Append is reachable via session_log). It becomes a finding if Read is ever exposed.
Round-2 residuals are now filed as #173 (static intermediate in-brain directory symlink on writes) and #174 (DetectTunnels FIFO hang). The rebase did not change WriteRegular or DetectTunnels, so neither residual got worse.
Not checked
Live pod state and deployment.
The intermediate in-brain directory symlink case (#173), which I did not re-probe because the code is unchanged since round 2.
REST /promote and /ingest-path, beyond sharing these code paths.
brain_classify, brain_graph and capture internals.
## Adversarial review, round 3 (head 6325c2b): PASS
I found nothing that blocks. This PR is `risk/high`, so **merging is Mathias's call** and I have not merged it. Exploit inputs are left out because the repo is public.
### Both round-2 blockers are fixed, and each flip went red
1. **Allowing `/` in `idPattern`'s tail** turns `TestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefix` red. All three ids return nil where `ErrInvalidID` was expected, and both "nothing created" asserts fail.
2. **Dropping `O_NONBLOCK` from `session.Append`** turns `TestAppendReturnsPromptlyOnAFIFOLogWithNoReader` red ("Append blocked on a FIFO log with no reader", 3.00s). The test's own unblock path let it finish instead of hanging the suite.
Both test names match their assertions.
### Rebase resolution (brainfs.go and brainfs_test.go): correct
- `git range-diff` from 87fa28e to 6325c2b shows that `WriteRegular` and its test were only moved, from before `ReadRegular` to the end of the file. The bodies are byte-identical.
- The diff against main is additions only, so nothing from #168 was dropped: `ReadDirRegular` and the `os.SameFile` check in `readOpenedMax` are both intact.
- b3d782b is identical to 87fa28e.
### Traversal probes through the MCP handlers (`brain_tunnel`, `brain_update`, `session_log`)
I ran 192 probes, all through `toolCall` on `mcp.NewServer`, in a scratch brain plus an outside directory. Each probe was checked with an Lstat and sha256 snapshot of the whole tree, so nothing depended on the response text. The shapes covered:
- `..`, absolute paths, and `file://`
- final symlinks to outside, relative and absolute
- intermediate directory symlinks to outside, at the wing level and at the hall level
- `%2e%2e`, `%2f` and `%2F`, backslash, NUL
- look-alike dots: U+2025, U+FF0E and U+2024
- `./`, `//`, and climbing out after a `.md` segment
- governance, `raw/` and `sessions/` targets
- `..` in wing or hall
- for `session_log`: symlinked logs, FIFO logs, and ids with newlines or over 200 characters
**Result on 6325c2b: 0 filesystem changes, and every probe was refused.**
The positive control is the same harness run on origin/main 77e32de. It records **52 escapes** (tunnel 13, update 29, session_log 10), and the three legitimate control calls write on both. So the harness can see an escape, and the PR closes all 52.
### Gates
- **CI run 9283 on 6325c2b:** success. The job log shows `ok` for ingestion api, brain, brainfs, mcp and session, with 0 FAIL lines.
- **`CI=1 GOFLAGS=-p=2 task check`** in a fresh clone: exit 0, 0 FAIL lines, gitleaks clean.
- **`-race -count=10`** on session, brainfs and brain: exit 0.
- **Merge check:** main has since moved to b6ef116 (#83 and a deps bump). PR 6325c2b merged onto it passes the api, brain, brainfs, mcp and session tests (exit 0), and Gitea reports the PR as mergeable.
Green alone would not have shown the round-2 gaps. The flips above are the evidence.
### Non-blocking
- **`session.Read`** still uses a blocking `os.Open`, which follows a final symlink and hangs on a FIFO. It has no non-test caller (only `Append` is reachable via `session_log`). It becomes a finding if `Read` is ever exposed.
- **Round-2 residuals** are now filed as #173 (static intermediate in-brain directory symlink on writes) and #174 (DetectTunnels FIFO hang). The rebase did not change `WriteRegular` or `DetectTunnels`, so neither residual got worse.
### Not checked
- Live pod state and deployment.
- The intermediate in-brain directory symlink case (#173), which I did not re-probe because the code is unchanged since round 2.
- REST `/promote` and `/ingest-path`, beyond sharing these code paths.
- brain_classify, brain_graph and capture internals.
Conflict in ingestion/internal/brainfs/brainfs_test.go only: both sides
appended a test after TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink
(this branch: TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles;
main: TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents).
Kept both, unchanged. No production code touched by the resolution.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
I merged main into this branch: merge commit cce4dc3 (parents 6325c2b and main 0bd0fbf), a normal push with no force. The merge only resolves a conflict; no production code is touched.
Conflict:ingestion/internal/brainfs/brainfs_test.go, one hunk at line ~266. Both sides appended a new test after TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink:
this branch: TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles
git split this into two marker blocks because the two functions share the b, err := Open(dir) ... defer lines and the closing brace. I kept both functions whole and unchanged, one after the other. The resolved file drops 0 lines compared with either parent.
brainfs.go auto-merged cleanly.
Logic unchanged: the PR's diff of non-test files against main after the merge is line-for-line identical to 6325c2b's diff against the old merge-base. The round-3 PASS reviewed that diff. No logic choice was needed.
Gates:
CI=1 GOFLAGS=-p=2 task check exits 0 with 0 FAIL lines.
go test -race -count=3 on brainfs (incl. brainfstest), session, api, mcp and brain: all ok.
Not merged into main. This is risk/high, so merging is Mathias's call.
I merged main into this branch: merge commit **cce4dc3** (parents 6325c2b and main 0bd0fbf), a normal push with no force. The merge only resolves a conflict; no production code is touched.
- **Conflict:** `ingestion/internal/brainfs/brainfs_test.go`, one hunk at line ~266. Both sides appended a new test after `TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink`:
- this branch: `TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles`
- main: `TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents`
git split this into two marker blocks because the two functions share the `b, err := Open(dir) ... defer` lines and the closing brace. I kept both functions whole and unchanged, one after the other. The resolved file drops 0 lines compared with either parent.
- **`brainfs.go`** auto-merged cleanly.
- **Logic unchanged:** the PR's diff of non-test files against main after the merge is line-for-line identical to 6325c2b's diff against the old merge-base. The round-3 PASS reviewed that diff. No logic choice was needed.
- **Gates:**
- `CI=1 GOFLAGS=-p=2 task check` exits 0 with 0 FAIL lines.
- `go test -race -count=3` on brainfs (incl. brainfstest), session, api, mcp and brain: all ok.
Not merged into main. This is `risk/high`, so merging is Mathias's call.
---
_Created via git-mcp on behalf of @mathias_
mathias
merged commit febab3e45d into main2026-10-05 18:47:07 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #169. risk/high: an authenticated path traversal to arbitrary file rewrite. Control code, adversarial review required.
Change
checkNotePath: only a clean (path.Clean(p) == p), localwiki/.../<slug>.mdpath is accepted, with no.,..or empty segments. Checked before anything else.brainfs.ReadRegularand rewrites them with the newbrainfs.WriteRegular.appendSeeAlsobecomes the purewithSeeAlso.brainfs.WriteRegular:O_NOFOLLOW|O_NONBLOCKos.SameFile)os.Root.OpenFilefollows an in-root final symlink even with O_NOFOLLOW. Verified on go1.27.1:os.OpenFilereturns ELOOP,Root.OpenFileopens the target. A test using an in-brain symlink caught this; the original outside-symlink test could not, because os.Root refuses escapes anyway.Tests
All red on main before the fix.
TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain: a file outside the brain was modified.TestWriteTunnelRefusesUncleanOrNonWikiPaths: 6 bad shapes, plus a positive control.TestWriteTunnelDoesNotWriteThroughASymlinkedNoteTestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget: the target is swapped between a regular note and an in-brain symlink toraw/victim.md. Positive control: some tunnels succeed.TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles: in-brain symlink, FIFO with a reader held open, escape, missing file.Flips
Red:
os.ReadFileos.WriteFileSameFiledroppedGreen, by design:
O_CREATEadded: Lstat fails on a missing file first.Not covered
AutoTunnel'slogFuzzyCandidatesappends toraw/tunnel-candidates-*.mdwithos.OpenFile(write path, #158).🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
fix(tunnel): brain_tunnel cannot write outside the wiki or through symlinks (#169)to fix: brain_tunnel, brain_update and session_log confine caller-supplied paths (#169)Adversarial review, round 1 (head
9eb3d24): BLOCKEDPosted late: the round-1 verdict went to the coordinator but not to this thread. Exploit inputs are left out because the repo is public.
The brain_tunnel fix held. 17 path shapes were refused:
..,./and//segmentsTwo claims checked by running them:
os.Root.OpenFiledoes follow an in-root final symlink even with O_NOFOLLOW (go1.27.1).Blocking (the PR said
Fixes #169, and #169 said no other handler was affected; that was false):brain_update:api.UpdateNote'sresolveWithinconfined the path only lexically. There was no knowledge/wiki scope, no.mdrule and no symlink check, so one call could supersede governance files and raw/ and sessions/ files, and write through a final symlink out of the brain.session_log:session.Appendjoined the caller'ssession_idinto the path and opened it with O_CREATE, so a token holder could create or append files outside sessions/.Non-blocking:
.mdrule: the.txtcase named a file that did not existDetectTunnels(readTitleAndCreated) blocks on a FIFO under wiki/. This was already true on main.CI run 9178 was green. Green covered none of the blocking findings.
Adversarial review, round 2 (head
87fa28e): BLOCKEDBoth round-1 handlers are fixed in the code, and I could not get past either fix through the MCP handlers. I'm blocking because two of the guards this round adds have no test that fails when they are removed.
Verified by execution (scratch brain, through
brainUpdate/sessionLog/brainIngestRaw)./,wiki/../and the slug-with-slash route..wing/slug/or a trailing/.. These resolve to the same note and are harmless.typecontaining path segments is skipped byvalidPageType(#147).Blocking: the flip stayed green on guards this PR ships
session.idPattern's ban on/: allowing/in the tail of the regex leaves every test green. Thea/bcase fails only becausesessions/a/doesn't exist. No case begins with a plain character and then climbs out with.., and that is exactly the escape the ban stops: with the ban removed, my probe's id created a file beside sessions/.Fix: add such a case, and assert on the refusal itself (a sentinel error, or that nothing exists outside), not on any error.
O_NONBLOCKinsession.Append: removing it leaves every test green. Unlike WriteRegular, Append does no Lstat type check, so O_NONBLOCK is the only thing that stops a FIFO named<id>.jsonlfrom hangingsession_logforever. My probe hangs without it.Fix: add a FIFO test with no reader and a timeout.
Flips that went red (good)
os.WriteFile.mdrule offNon-blocking
os.ReadFileinstead ofReadRegularis green. That's acceptable:notes.Resolvealready refuses a final symlink, the write refuses a swapped one, and the prior content is never returned.brain_update(andbrain_tunnel) rewrite an existing.mdthere, for example in raw/. Governance files and.jsonlare not reachable because of the.md/scope rule. Likewise, if sessions/ itself is a symlink,session_logfollows it.WriteRegulardoc cites #167 for this, and #167 doesn't cover it. Record it there, or in its own issue, so the acceptance is explicit.wikiUpkeep → AutoTunnel → DetectTunnels, which opens every wiki.mdwith a blockingos.Open. This was already true on main and affects brain_write too. Not tracked in #157/#158 as far as I can see.CI and gates
87fa28ewas green, and it ran api, brain, brainfs and session.CI=1 task checkexited 0.-race -count=5on brain, brainfs, api and session: ok.Not checked
brain_tunnel passes caller-supplied source and target paths to WriteTunnel, which checked only the first segments ("wiki/<wing>/...") and joined them onto the brain dir. "wiki/b/x/../../../../tmp/f.md" resolved outside the brain, and appendSeeAlso read and rewrote that file (verified by execution): any MCP token holder could append to any existing file the pod user can write, inside the brain (raw/, sessions/, classification.yaml) or outside it. - checkNotePath: only a clean, local wiki/.../<slug>.md path, with no ".", ".." or empty segments. - Both notes are read and rewritten through brainfs. New brainfs.WriteRegular: never creates; Lstat through the root, open with O_NOFOLLOW|O_NONBLOCK, and require the opened handle to be the same regular file (os.SameFile). os.Root.OpenFile follows an in-root final symlink even with O_NOFOLLOW (verified on go1.27), so the flag alone does not refuse it. - The source is no longer rewritten when the target is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>87fa28e2c8to6325c2b414Note for the session holding
~/worktrees/hg-tunnel:fix/tunnel-traversalwas rebased onto main77e32deand force-pushed with lease (expected87fa28e). The new head is6325c2b. It adds the two tests round 2 asked for: theErrInvalidIDclimb-out case and the session FIFO timeout. Before pushing again from that worktree, rungit fetch && git reset --hard origin/fix/tunnel-traversal. The worktree was clean and fully pushed at the time of the rebase. Round-3 review is running now.Created via git-mcp on behalf of @mathias
Adversarial review, round 3 (head
6325c2b): PASSI found nothing that blocks. This PR is
risk/high, so merging is Mathias's call and I have not merged it. Exploit inputs are left out because the repo is public.Both round-2 blockers are fixed, and each flip went red
/inidPattern's tail turnsTestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefixred. All three ids return nil whereErrInvalidIDwas expected, and both "nothing created" asserts fail.O_NONBLOCKfromsession.AppendturnsTestAppendReturnsPromptlyOnAFIFOLogWithNoReaderred ("Append blocked on a FIFO log with no reader", 3.00s). The test's own unblock path let it finish instead of hanging the suite.Both test names match their assertions.
Rebase resolution (brainfs.go and brainfs_test.go): correct
git range-difffrom87fa28eto6325c2bshows thatWriteRegularand its test were only moved, from beforeReadRegularto the end of the file. The bodies are byte-identical.ReadDirRegularand theos.SameFilecheck inreadOpenedMaxare both intact.b3d782bis identical to87fa28e.Traversal probes through the MCP handlers (
brain_tunnel,brain_update,session_log)I ran 192 probes, all through
toolCallonmcp.NewServer, in a scratch brain plus an outside directory. Each probe was checked with an Lstat and sha256 snapshot of the whole tree, so nothing depended on the response text. The shapes covered:.., absolute paths, andfile://%2e%2e,%2fand%2F, backslash, NUL./,//, and climbing out after a.mdsegmentraw/andsessions/targets..in wing or hallsession_log: symlinked logs, FIFO logs, and ids with newlines or over 200 charactersResult on
6325c2b: 0 filesystem changes, and every probe was refused.The positive control is the same harness run on origin/main
77e32de. It records 52 escapes (tunnel 13, update 29, session_log 10), and the three legitimate control calls write on both. So the harness can see an escape, and the PR closes all 52.Gates
6325c2b: success. The job log showsokfor ingestion api, brain, brainfs, mcp and session, with 0 FAIL lines.CI=1 GOFLAGS=-p=2 task checkin a fresh clone: exit 0, 0 FAIL lines, gitleaks clean.-race -count=10on session, brainfs and brain: exit 0.b6ef116(#83 and a deps bump). PR6325c2bmerged onto it passes the api, brain, brainfs, mcp and session tests (exit 0), and Gitea reports the PR as mergeable.Green alone would not have shown the round-2 gaps. The flips above are the evidence.
Non-blocking
session.Readstill uses a blockingos.Open, which follows a final symlink and hangs on a FIFO. It has no non-test caller (onlyAppendis reachable viasession_log). It becomes a finding ifReadis ever exposed.WriteRegularorDetectTunnels, so neither residual got worse.Not checked
/promoteand/ingest-path, beyond sharing these code paths.I merged main into this branch: merge commit
cce4dc3(parents6325c2band main0bd0fbf), a normal push with no force. The merge only resolves a conflict; no production code is touched.Conflict:
ingestion/internal/brainfs/brainfs_test.go, one hunk at line ~266. Both sides appended a new test afterTestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink:TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFilesTestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponentsgit split this into two marker blocks because the two functions share the
b, err := Open(dir) ... deferlines and the closing brace. I kept both functions whole and unchanged, one after the other. The resolved file drops 0 lines compared with either parent.brainfs.goauto-merged cleanly.Logic unchanged: the PR's diff of non-test files against main after the merge is line-for-line identical to 6325c2b's diff against the old merge-base. The round-3 PASS reviewed that diff. No logic choice was needed.
Gates:
CI=1 GOFLAGS=-p=2 task checkexits 0 with 0 FAIL lines.go test -race -count=3on brainfs (incl. brainfstest), session, api, mcp and brain: all ok.Not merged into main. This is
risk/high, so merging is Mathias's call.Created via git-mcp on behalf of @mathias