Fixes#173. risk/high: control code in brainfs (every reader and writer), adversarial review required.
Problem
os.Root follows an intermediate directory symlink as long as it stays inside the brain:
A dir symlink inside wiki/ that points elsewhere in the brain let brain_update and brain_tunnel rewrite an existing .md there (for example under raw/).
session_log followed a symlinked sessions/ dir.
Change
WriteRegular, ReadRegular and ReadRegularMax require every directory above the file to be real (realDir(path.Dir(rel))).
Why reads too:
brain_tunnel read the redirected target, then rewrote the source before the target's write was refused (the new tunnel test caught this).
brain_read and brain_get could read another part of the brain through such a path.
session.Append refuses a sessions dir that is not a real directory.
Remaining: a directory swapped in after the check is the #167 residual.
Fixes #173. risk/high: control code in brainfs (every reader and writer), adversarial review required.
## Problem
`os.Root` follows an intermediate directory symlink as long as it stays inside the brain:
- A dir symlink inside `wiki/` that points elsewhere in the brain let **brain_update** and **brain_tunnel** rewrite an existing `.md` there (for example under `raw/`).
- **session_log** followed a symlinked `sessions/` dir.
## Change
- `WriteRegular`, `ReadRegular` and `ReadRegularMax` require **every directory above the file** to be real (`realDir(path.Dir(rel))`).
- **Why reads too:**
- brain_tunnel read the redirected target, then rewrote the source before the target's write was refused (the new tunnel test caught this).
- brain_read and brain_get could read another part of the brain through such a path.
- `session.Append` refuses a sessions dir that is not a real directory.
- **Remaining:** a directory swapped in after the check is the #167 residual.
## Tests (each red before the fix)
- `TestUpdateNoteRefusesANoteBehindASymlinkedDirectory`
- `TestWriteTunnelRefusesATargetBehindASymlinkedDirectory`: the target is refused and the source is not rewritten.
- `TestAppendRefusesASymlinkedSessionsDir`
- `TestReadAndWriteRegularRefuseAFileBehindASymlinkedDirectory`: positive control included.
## Flips (red)
- realDir removed from WriteRegular
- realDir removed from ReadRegular
- the session dir check off
**Cost:** one Lstat per path component on every read and write.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
os.Root follows an intermediate directory symlink that stays inside the
brain. A dir symlink inside wiki/ pointing elsewhere in the brain let
brain_update and brain_tunnel rewrite an existing .md there (e.g. under
raw/), and session_log followed a symlinked sessions dir.
- WriteRegular, ReadRegular and ReadRegularMax require every directory
above the file to be a real directory (realDir). Reads too: brain_tunnel
read the redirected target, then rewrote the source before the target's
write was refused; and brain_read/brain_get could read another part of
the brain through such a path.
- session.Append refuses a sessions dir that is not a real directory.
- A directory swapped in after the check remains the #167 residual.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
I ran everything below in a scratch copy of the branch. The tests were run through the real MCP handlers (NewServer + tools/call) and checked against origin/main.
Blocking
B1. brainfs.ReadRegular, ReadRegularMax, WriteRegular: the new check is lexical, but the open is not.
realDir(path.Dir(rel)) checks the cleaned parent directory. The Lstat and the open then use the rawrel through os.Root.
os.Root resolves a .. component after it follows a symlink. So suppose a non-clean rel has a .. that comes after a symlinked directory. The check passes on the lexical parent, and the read or write goes through the symlink.
I ran this as a brainfs unit probe. WriteRegular replaced a file in raw/ through a symlinked directory under wiki/. ReadRegular and ReadRegularMax returned that file's bytes. The new doc comment says "every directory above the file must be a real directory", and that is false for this input.
No current entry point reaches it.notes.Resolve and resolveWithin clean the path, and checkNotePath and graphsync refuse non-clean paths. I confirmed that brain_get, brain_read, brain_edit, brain_tunnel and brain_update all refuse such a path. The hydrate and excerpt paths come from stores that only receive clean or walked paths.
That means the property is carried by every caller's upstream filter, not by the primitive this PR hardens. Any later caller that passes a store-sourced or constructed path gets the false guarantee. That includes #158 moving the raw/ writes onto WriteRegular.
Fix: in all three functions, refuse a rel that is not path.Clean-clean and filepath.IsLocal, with ErrNotRegular. Add a brainfs test that uses a ..-after-symlink path for all three. It must be red without the guard.
Non-blocking (please record)
N1. session.Append check-then-open race. This is not the #167 residual.
File: internal/session/session.go:73-79.
#167 is bounded by os.Root: a swap stays inside the brain. Append opens by plain path, so if a symlink is swapped in for sessions/ after the Lstat, the .jsonl is created wherever that link points, including outside the brain.
I measured it with a local atomic-exchange swapper. Most of the appends that passed the check landed in the outside directory.
A local writer can hit this; a Syncthing peer probably cannot. Main has the same write statically, so this PR is still a strict improvement.
The code comment records no residual. Either do the open through an os.Root on the brain dir (that bounds it to #167), or file the residual with its true bound.
N2. The api and brain endpoint tests pin neither layer individually.
Flipping only the read-side realDir, or only the write-side one, leaves TestUpdateNoteRefusesANoteBehindASymlinkedDirectory green. The other layer still refuses.
The brainfs unit test does pin each layer, so the coverage exists.
Both endpoint tests assert assert.Error with any error and have no in-test positive control. If an unrelated earlier refusal appeared later, they would stay green vacuously. Prefer ErrorIs(..., brainfs.ErrNotRegular) plus the same call on a real path succeeding.
N3. Two behaviour changes are shipped without assertions.
Search hydrate and brain_context's readExcerpt now refuse a stored path behind a symlinked directory. On main both returned the redirected file's content; I verified both on main and on the PR.
No test in the PR pins this. #171 tracks these per-caller tests.
N4.brain_write creates notes with os.WriteFile and is not covered by this PR. I found no open issue that covers its directory handling. Reported separately; no details here.
N5. Cosmetic.
The realDir error text says "walk root component" for file reads and writes.
The ReadRegular doc does not mention the new directory rule.
Flip tests (each one made a test go red)
Flip
Red
drop the WriteRegular realDir
brainfs test only
drop the ReadRegular realDir
brainfs test + tunnel test (the source got rewritten)
drop the ReadRegularMax realDir
brainfs test
drop all three
brainfs + UpdateNote + tunnel tests
Append: Lstat changed to Stat / dir check dropped
TestAppendRefusesASymlinkedSessionsDir
realDir checking only the first component, or skipping the last
red (several)
Append without MkdirAll
the existing session tests (a missing dir is still created)
Static redirect, end to end (MCP handlers, PR vs main)
I set up a directory symlink to raw/facts at each of wiki/<link>, wiki/w/<link>, wiki/w/<hall-name>, knowledge/<link> and knowledge/deep/<link>, with the brain dir both direct and behind a mount symlink. Through it I called brain_update, brain_get, brain_read, brain_edit (with .git present so the edit is reachable) and brain_tunnel, as both source and target.
PR: every call was refused, nothing leaked, and the victim was intact.
main: update, get, read and edit succeeded at every depth. Tunnel succeeded at every depth where its wing/hall shape applies.
session_log: a symlinked sessions/ is refused, and a missing one is still created, both with and without a mount symlink.
Positive controls: legitimate get, read, update, edit and tunnel calls all succeed, including through the mount symlink.
Regression check against a copy of the real brain
Over 1670 regular .md files, opened through a mount symlink:
ReadRegular and ReadRegularMax: 1670/1670 succeed on main and 1670/1670 on the PR.
WriteRegular (identical content, on a copy): 1670/1670 on the PR.
The copy has no symlinks, so this only shows that nothing legitimate is refused.
Gates
go test -race -count=5 over brainfs, api, brain and session passed.
CI=1 task check exited 0: 0 FAIL, golangci-lint reported 0 issues for both modules, gitleaks was clean.
CI run 9451: Lint/Test/Vet passed.
Green, and green covers none of B1 or N1.
The PR is labelled risk/high, so it needs Mathias's approval to merge regardless of this review.
## pr-gatekeeper: BLOCKED (head 87dace3)
I ran everything below in a scratch copy of the branch. The tests were run through the real MCP handlers (`NewServer` + `tools/call`) and checked against `origin/main`.
### Blocking
**B1. `brainfs.ReadRegular`, `ReadRegularMax`, `WriteRegular`: the new check is lexical, but the open is not.**
- File: `internal/brainfs/brainfs.go:131`, `:160`, `:274`.
- `realDir(path.Dir(rel))` checks the *cleaned* parent directory. The `Lstat` and the open then use the *raw* `rel` through `os.Root`.
- `os.Root` resolves a `..` component *after* it follows a symlink. So suppose a non-clean `rel` has a `..` that comes after a symlinked directory. The check passes on the lexical parent, and the read or write goes through the symlink.
- I ran this as a brainfs unit probe. `WriteRegular` replaced a file in `raw/` through a symlinked directory under `wiki/`. `ReadRegular` and `ReadRegularMax` returned that file's bytes. The new doc comment says "every directory above the file must be a real directory", and that is false for this input.
- **No current entry point reaches it.** `notes.Resolve` and `resolveWithin` clean the path, and `checkNotePath` and graphsync refuse non-clean paths. I confirmed that brain_get, brain_read, brain_edit, brain_tunnel and brain_update all refuse such a path. The hydrate and excerpt paths come from stores that only receive clean or walked paths.
- That means the property is carried by every caller's upstream filter, not by the primitive this PR hardens. Any later caller that passes a store-sourced or constructed path gets the false guarantee. That includes #158 moving the raw/ writes onto `WriteRegular`.
- **Fix:** in all three functions, refuse a `rel` that is not `path.Clean`-clean and `filepath.IsLocal`, with `ErrNotRegular`. Add a brainfs test that uses a `..`-after-symlink path for all three. It must be red without the guard.
### Non-blocking (please record)
**N1. `session.Append` check-then-open race. This is not the #167 residual.**
- File: `internal/session/session.go:73-79`.
- #167 is bounded by `os.Root`: a swap stays inside the brain. `Append` opens by plain path, so if a symlink is swapped in for `sessions/` after the `Lstat`, the `.jsonl` is created wherever that link points, including outside the brain.
- I measured it with a local atomic-exchange swapper. Most of the appends that passed the check landed in the outside directory.
- A local writer can hit this; a Syncthing peer probably cannot. Main has the same write *statically*, so this PR is still a strict improvement.
- The code comment records no residual. Either do the open through an `os.Root` on the brain dir (that bounds it to #167), or file the residual with its true bound.
**N2. The api and brain endpoint tests pin neither layer individually.**
- Flipping only the read-side `realDir`, or only the write-side one, leaves `TestUpdateNoteRefusesANoteBehindASymlinkedDirectory` green. The other layer still refuses.
- The brainfs unit test does pin each layer, so the coverage exists.
- Both endpoint tests assert `assert.Error` with any error and have no in-test positive control. If an unrelated earlier refusal appeared later, they would stay green vacuously. Prefer `ErrorIs(..., brainfs.ErrNotRegular)` plus the same call on a real path succeeding.
**N3. Two behaviour changes are shipped without assertions.**
- Search `hydrate` and `brain_context`'s `readExcerpt` now refuse a stored path behind a symlinked directory. On main both returned the redirected file's content; I verified both on main and on the PR.
- No test in the PR pins this. #171 tracks these per-caller tests.
**N4.** `brain_write` creates notes with `os.WriteFile` and is not covered by this PR. I found no open issue that covers its directory handling. Reported separately; no details here.
**N5. Cosmetic.**
- The `realDir` error text says "walk root component" for file reads and writes.
- The `ReadRegular` doc does not mention the new directory rule.
### Flip tests (each one made a test go red)
| Flip | Red |
|---|---|
| drop the `WriteRegular` realDir | brainfs test only |
| drop the `ReadRegular` realDir | brainfs test + tunnel test (the source got rewritten) |
| drop the `ReadRegularMax` realDir | brainfs test |
| drop all three | brainfs + UpdateNote + tunnel tests |
| `Append`: `Lstat` changed to `Stat` / dir check dropped | `TestAppendRefusesASymlinkedSessionsDir` |
| realDir checking only the first component, or skipping the last | red (several) |
| `Append` without `MkdirAll` | the existing session tests (a missing dir is still created) |
### Static redirect, end to end (MCP handlers, PR vs main)
- I set up a directory symlink to `raw/facts` at each of `wiki/<link>`, `wiki/w/<link>`, `wiki/w/<hall-name>`, `knowledge/<link>` and `knowledge/deep/<link>`, with the brain dir both direct and behind a mount symlink. Through it I called brain_update, brain_get, brain_read, brain_edit (with `.git` present so the edit is reachable) and brain_tunnel, as both source and target.
- **PR:** every call was refused, nothing leaked, and the victim was intact.
- **main:** update, get, read and edit succeeded at every depth. Tunnel succeeded at every depth where its wing/hall shape applies.
- **session_log:** a symlinked `sessions/` is refused, and a missing one is still created, both with and without a mount symlink.
- **Positive controls:** legitimate get, read, update, edit and tunnel calls all succeed, including through the mount symlink.
### Regression check against a copy of the real brain
- Over 1670 regular `.md` files, opened through a mount symlink:
- `ReadRegular` and `ReadRegularMax`: 1670/1670 succeed on main and 1670/1670 on the PR.
- `WriteRegular` (identical content, on a copy): 1670/1670 on the PR.
- The copy has no symlinks, so this only shows that nothing legitimate is refused.
### Gates
- `go test -race -count=5` over brainfs, api, brain and session passed.
- `CI=1 task check` exited 0: 0 FAIL, golangci-lint reported 0 issues for both modules, gitleaks was clean.
- CI run 9451: Lint/Test/Vet passed.
- Green, and green covers none of B1 or N1.
The PR is labelled `risk/high`, so it needs Mathias's approval to merge regardless of this review.
Review round 1 on PR #187:
- realDir checked the cleaned parent while Lstat/OpenFile used the raw
path; os.Root resolves ".." after following a symlink, so a ".." placed
after a symlinked directory passed the check and redirected the read or
write. ReadRegular, ReadRegularMax and WriteRegular now refuse a path
that is not clean and local (ErrNotRegular). No current entry point
reached it (callers clean first); the primitive no longer relies on that.
- session.Append checked the sessions dir with Lstat, then opened by plain
path: a symlink swapped in between created logs outside the brain
(29k in the reviewer's RENAME_EXCHANGE race). It now opens through an
os.Root on the sessions dir's parent, so a swap can only redirect inside
the brain (#167), and, since os.Root follows an in-root final symlink
even with O_NOFOLLOW, writes only after confirming the path is the
regular file it opened (Lstat + os.SameFile).
- Endpoint tests assert ErrorIs(brainfs.ErrNotRegular) and have positive
controls.
golang.org/x/sys moves from indirect to direct (unix.Renameat2 in the
session race test); it was already in the module graph.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Flip tests (each run separately; all red unless noted)
Flip
Red
cleanRel made a no-op
...RefuseAPathThatIsNotClean
cleanRel dropped from ReadRegular / ReadRegularMax / WriteRegular, one at a time
the same test, each time
IsLocal clause dropped
the same test
plain-path open instead of root.OpenFile (3 runs)
...UnderADirSwapRace, 3/3
SameFile off on its own (3 runs)
...LogSwappedForASymlink, 3/3
IsDir check off
TestAppendRefusesASymlinkedSessionsDir
root.Mkdir dropped
existing session tests
backslash clause dropped
stays green (N-b)
empty-string clause dropped
stays green; redundant, because path.Clean("") is .
IsRegular on the log dropped
stays green; redundant, because SameFile already refuses a symlink
End-to-end and regression re-run
The round-1 MCP probe: directory symlinks at five depths under wiki/ and knowledge/, direct and behind a mount symlink, through update, get, read, edit and tunnel (source and target).
All refused, nothing leaked, the victim was intact.
Positive controls pass. session_log refuses a symlinked dir and creates a missing one.
hydrate and readExcerpt refuse the redirected paths and read the real ones.
Copy of the real brain, opened through a mount symlink (1670 .md files): ReadRegular, ReadRegularMax and WriteRegular each succeed on 1670/1670.
Non-blocking
N-a. Dangling in-brain log symlink.Append creates the log with O_CREATE before its SameFile check. A dangling in-brain final symlink at sessions/<id>.jsonl therefore makes Append create an empty file at the link's in-brain target before it refuses. I measured this: 0 bytes, with an error returned.
There is no privilege gain, because planting the link already needs brain write access.
Still, that file then gets indexed. An Lstat of the log before the open, or an O_EXCL create, would close it.
N-b. Untested backslash clause. The backslash clause in cleanRel has no test. On Linux it is a policy choice, not a traversal guard, so either test it or say why it is there.
N-c. Race tests have no floor. The two new race tests have no floor showing the swapper actually ran (a swap count above zero, or at least one refusal observed). That is the #177 pattern: on a slow runner they could pass without proving anything. Each one is red under its flip locally (3/3), so this is hardening, not a hole.
N5: the error text is unchanged and cosmetic.
Gates
go test -race -count=5 over brainfs, api, brain and session passed.
CI=1 task check exited 0: 0 FAIL, golangci-lint reported 0 issues in both modules, gitleaks was clean.
go mod tidy is clean with x/sys as a direct dependency.
CI run 9459: Lint/Test/Vet passed.
## pr-gatekeeper round 2: PASS (head 9a288be). No merge: `risk/high`, so Mathias approves.
I ran everything in a fresh scratch copy of 9a288be.
### Round-1 findings
- **B1: closed.**
- `cleanRel` runs first in `ReadRegular`, `ReadRegularMax` and `WriteRegular`.
- My round-1 brainfs probe is now refused by all three, and the target is untouched.
- The same probe through brain_get, brain_read, brain_edit, brain_tunnel and brain_update is still refused.
- **N1: closed to the #167 bound.**
- A `sessions/` symlink that points outside the brain is refused, and nothing is created outside.
- A dangling final log symlink that points outside creates nothing.
- A missing sessions dir is still created, including under a mount-symlinked brain dir, and a second append adds a second line.
- **N2: closed.** Both endpoint tests now use `ErrorIs(brainfs.ErrNotRegular)` with real positive controls.
- **N4:** filed as #188.
- **N3:** stays with #171.
### Flip tests (each run separately; all red unless noted)
| Flip | Red |
|---|---|
| `cleanRel` made a no-op | `...RefuseAPathThatIsNotClean` |
| `cleanRel` dropped from ReadRegular / ReadRegularMax / WriteRegular, one at a time | the same test, each time |
| `IsLocal` clause dropped | the same test |
| plain-path open instead of `root.OpenFile` (3 runs) | `...UnderADirSwapRace`, 3/3 |
| `SameFile` off on its own (3 runs) | `...LogSwappedForASymlink`, 3/3 |
| `IsDir` check off | `TestAppendRefusesASymlinkedSessionsDir` |
| `root.Mkdir` dropped | existing session tests |
| backslash clause dropped | **stays green** (N-b) |
| empty-string clause dropped | stays green; redundant, because `path.Clean("")` is `.` |
| `IsRegular` on the log dropped | stays green; redundant, because `SameFile` already refuses a symlink |
### End-to-end and regression re-run
- The round-1 MCP probe: directory symlinks at five depths under wiki/ and knowledge/, direct and behind a mount symlink, through update, get, read, edit and tunnel (source and target).
- All refused, nothing leaked, the victim was intact.
- Positive controls pass. `session_log` refuses a symlinked dir and creates a missing one.
- hydrate and readExcerpt refuse the redirected paths and read the real ones.
- Copy of the real brain, opened through a mount symlink (1670 `.md` files): ReadRegular, ReadRegularMax and WriteRegular each succeed on 1670/1670.
### Non-blocking
- **N-a. Dangling in-brain log symlink.** `Append` creates the log with `O_CREATE` before its `SameFile` check. A dangling *in-brain* final symlink at `sessions/<id>.jsonl` therefore makes `Append` create an empty file at the link's in-brain target before it refuses. I measured this: 0 bytes, with an error returned.
- There is no privilege gain, because planting the link already needs brain write access.
- Still, that file then gets indexed. An `Lstat` of the log before the open, or an `O_EXCL` create, would close it.
- **N-b. Untested backslash clause.** The backslash clause in `cleanRel` has no test. On Linux it is a policy choice, not a traversal guard, so either test it or say why it is there.
- **N-c. Race tests have no floor.** The two new race tests have no floor showing the swapper actually ran (a swap count above zero, or at least one refusal observed). That is the #177 pattern: on a slow runner they could pass without proving anything. Each one is red under its flip locally (3/3), so this is hardening, not a hole.
- **N5:** the error text is unchanged and cosmetic.
### Gates
- `go test -race -count=5` over brainfs, api, brain and session passed.
- `CI=1 task check` exited 0: 0 FAIL, golangci-lint reported 0 issues in both modules, gitleaks was clean.
- `go mod tidy` is clean with x/sys as a direct dependency.
- CI run 9459: Lint/Test/Vet passed.
mathias
merged commit ddd4bdc2ac into main2026-10-05 20:17:37 +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.
mathias
Fixes #173. risk/high: control code in brainfs (every reader and writer), adversarial review required.
Problem
os.Rootfollows an intermediate directory symlink as long as it stays inside the brain:wiki/that points elsewhere in the brain let brain_update and brain_tunnel rewrite an existing.mdthere (for example underraw/).sessions/dir.Change
WriteRegular,ReadRegularandReadRegularMaxrequire every directory above the file to be real (realDir(path.Dir(rel))).session.Appendrefuses a sessions dir that is not a real directory.Tests (each red before the fix)
TestUpdateNoteRefusesANoteBehindASymlinkedDirectoryTestWriteTunnelRefusesATargetBehindASymlinkedDirectory: the target is refused and the source is not rewritten.TestAppendRefusesASymlinkedSessionsDirTestReadAndWriteRegularRefuseAFileBehindASymlinkedDirectory: positive control included.Flips (red)
Cost: one Lstat per path component on every read and write.
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
pr-gatekeeper: BLOCKED (head
87dace3)I ran everything below in a scratch copy of the branch. The tests were run through the real MCP handlers (
NewServer+tools/call) and checked againstorigin/main.Blocking
B1.
brainfs.ReadRegular,ReadRegularMax,WriteRegular: the new check is lexical, but the open is not.internal/brainfs/brainfs.go:131,:160,:274.realDir(path.Dir(rel))checks the cleaned parent directory. TheLstatand the open then use the rawrelthroughos.Root.os.Rootresolves a..component after it follows a symlink. So suppose a non-cleanrelhas a..that comes after a symlinked directory. The check passes on the lexical parent, and the read or write goes through the symlink.WriteRegularreplaced a file inraw/through a symlinked directory underwiki/.ReadRegularandReadRegularMaxreturned that file's bytes. The new doc comment says "every directory above the file must be a real directory", and that is false for this input.notes.ResolveandresolveWithinclean the path, andcheckNotePathand graphsync refuse non-clean paths. I confirmed that brain_get, brain_read, brain_edit, brain_tunnel and brain_update all refuse such a path. The hydrate and excerpt paths come from stores that only receive clean or walked paths.WriteRegular.relthat is notpath.Clean-clean andfilepath.IsLocal, withErrNotRegular. Add a brainfs test that uses a..-after-symlink path for all three. It must be red without the guard.Non-blocking (please record)
N1.
session.Appendcheck-then-open race. This is not the #167 residual.internal/session/session.go:73-79.os.Root: a swap stays inside the brain.Appendopens by plain path, so if a symlink is swapped in forsessions/after theLstat, the.jsonlis created wherever that link points, including outside the brain.os.Rooton the brain dir (that bounds it to #167), or file the residual with its true bound.N2. The api and brain endpoint tests pin neither layer individually.
realDir, or only the write-side one, leavesTestUpdateNoteRefusesANoteBehindASymlinkedDirectorygreen. The other layer still refuses.assert.Errorwith any error and have no in-test positive control. If an unrelated earlier refusal appeared later, they would stay green vacuously. PreferErrorIs(..., brainfs.ErrNotRegular)plus the same call on a real path succeeding.N3. Two behaviour changes are shipped without assertions.
hydrateandbrain_context'sreadExcerptnow refuse a stored path behind a symlinked directory. On main both returned the redirected file's content; I verified both on main and on the PR.N4.
brain_writecreates notes withos.WriteFileand is not covered by this PR. I found no open issue that covers its directory handling. Reported separately; no details here.N5. Cosmetic.
realDirerror text says "walk root component" for file reads and writes.ReadRegulardoc does not mention the new directory rule.Flip tests (each one made a test go red)
WriteRegularrealDirReadRegularrealDirReadRegularMaxrealDirAppend:Lstatchanged toStat/ dir check droppedTestAppendRefusesASymlinkedSessionsDirAppendwithoutMkdirAllStatic redirect, end to end (MCP handlers, PR vs main)
raw/factsat each ofwiki/<link>,wiki/w/<link>,wiki/w/<hall-name>,knowledge/<link>andknowledge/deep/<link>, with the brain dir both direct and behind a mount symlink. Through it I called brain_update, brain_get, brain_read, brain_edit (with.gitpresent so the edit is reachable) and brain_tunnel, as both source and target.sessions/is refused, and a missing one is still created, both with and without a mount symlink.Regression check against a copy of the real brain
.mdfiles, opened through a mount symlink:ReadRegularandReadRegularMax: 1670/1670 succeed on main and 1670/1670 on the PR.WriteRegular(identical content, on a copy): 1670/1670 on the PR.Gates
go test -race -count=5over brainfs, api, brain and session passed.CI=1 task checkexited 0: 0 FAIL, golangci-lint reported 0 issues for both modules, gitleaks was clean.The PR is labelled
risk/high, so it needs Mathias's approval to merge regardless of this review.pr-gatekeeper round 2: PASS (head
9a288be). No merge:risk/high, so Mathias approves.I ran everything in a fresh scratch copy of
9a288be.Round-1 findings
cleanRelruns first inReadRegular,ReadRegularMaxandWriteRegular.sessions/symlink that points outside the brain is refused, and nothing is created outside.ErrorIs(brainfs.ErrNotRegular)with real positive controls.Flip tests (each run separately; all red unless noted)
cleanRelmade a no-op...RefuseAPathThatIsNotCleancleanReldropped from ReadRegular / ReadRegularMax / WriteRegular, one at a timeIsLocalclause droppedroot.OpenFile(3 runs)...UnderADirSwapRace, 3/3SameFileoff on its own (3 runs)...LogSwappedForASymlink, 3/3IsDircheck offTestAppendRefusesASymlinkedSessionsDirroot.Mkdirdroppedpath.Clean("")is.IsRegularon the log droppedSameFilealready refuses a symlinkEnd-to-end and regression re-run
session_logrefuses a symlinked dir and creates a missing one..mdfiles): ReadRegular, ReadRegularMax and WriteRegular each succeed on 1670/1670.Non-blocking
Appendcreates the log withO_CREATEbefore itsSameFilecheck. A dangling in-brain final symlink atsessions/<id>.jsonltherefore makesAppendcreate an empty file at the link's in-brain target before it refuses. I measured this: 0 bytes, with an error returned.Lstatof the log before the open, or anO_EXCLcreate, would close it.cleanRelhas no test. On Linux it is a policy choice, not a traversal guard, so either test it or say why it is there.Gates
go test -race -count=5over brainfs, api, brain and session passed.CI=1 task checkexited 0: 0 FAIL, golangci-lint reported 0 issues in both modules, gitleaks was clean.go mod tidyis clean with x/sys as a direct dependency.