Refs #153 (items 1, 3, 4 in code. Item 2, Syncthing, is a decision; see below.)
Why
brain/raw/ is writable from outside the API: it's a Syncthing sendreceive folder with flamingo (symlinks are synced), and brain CronJobs mount it. Readers followed symlinks out of it:
/pending and brain_pending showed a symlink target's contents.
brain_promote would copy it into wiki/, after which brain-sync pushes it to Gitea. brain_reject would copy it into raw/rejected/.
The watcher, which needs no API call at all, ingested symlinked files. That one wasn't in the issue; I found it while fixing the others.
Ingest had a time-of-check/time-of-use gap: it checked the path, then reopened it by name.
What
api.OpenRawRoot opens brain/raw as an os.Root, and refuses a raw/ that is itself a symlink (item 4).
ListPending, PromoteNote and RejectNote read through the root and refuse anything that isn't a regular file.
extract.TextIn(root, rel) reads through a root. pdf and docmark formats are copied to a temp file from the root handle first. /ingest-path, brain_ingest and the watcher all use it, and the watcher archives through the root too (item 3).
Tests (red first)
raw_symlink_test.go: pending skips symlinks and never shows the target; promote and reject refuse a symlinked source; a symlinked raw/ is refused by ingest, pending and promote.
extract/root_test.go: TextIn reads inside the root and refuses symlink and .. escapes.
watcher_test.go: TestProcessDir_DoesNotIngestSymlinks. The target never reaches the pipeline.
CI=1 task check EXIT=0.
Item 2 (Syncthing): needs Mathias
With symlinks inert in every reader, the Syncthing write path can no longer leak outside files. My recommendation: accept it, or additionally exclude symlinks from that folder.
Refs #153 (items 1, 3, 4 in code. Item 2, Syncthing, is a decision; see below.)
## Why
`brain/raw/` is writable from outside the API: it's a Syncthing `sendreceive` folder with flamingo (symlinks are synced), and brain CronJobs mount it. Readers followed symlinks out of it:
- `/pending` and `brain_pending` showed a symlink target's contents.
- `brain_promote` would copy it into `wiki/`, after which brain-sync pushes it to Gitea. `brain_reject` would copy it into `raw/rejected/`.
- **The watcher**, which needs no API call at all, ingested symlinked files. That one wasn't in the issue; I found it while fixing the others.
- Ingest had a time-of-check/time-of-use gap: it checked the path, then reopened it by name.
## What
- `api.OpenRawRoot` opens `brain/raw` as an `os.Root`, and refuses a `raw/` that is itself a symlink (item 4).
- `ListPending`, `PromoteNote` and `RejectNote` read through the root and refuse anything that isn't a regular file.
- `extract.TextIn(root, rel)` reads through a root. pdf and docmark formats are copied to a temp file from the root handle first. `/ingest-path`, `brain_ingest` and the watcher all use it, and the watcher archives through the root too (item 3).
## Tests (red first)
- `raw_symlink_test.go`: pending skips symlinks and never shows the target; promote and reject refuse a symlinked source; a symlinked `raw/` is refused by ingest, pending and promote.
- `extract/root_test.go`: `TextIn` reads inside the root and refuses symlink and `..` escapes.
- `watcher_test.go`: `TestProcessDir_DoesNotIngestSymlinks`. The target never reaches the pipeline.
`CI=1 task check` EXIT=0.
## Item 2 (Syncthing): needs Mathias
With symlinks inert in every reader, the Syncthing write path can no longer leak outside files. My recommendation: accept it, or additionally exclude symlinks from that folder.
risk/medium: control code (rule 8).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
raw/ is writable via Syncthing (a sendreceive folder shared with
flamingo, symlinks synced) and by the brain CronJobs, and several
readers followed symlinks out of it:
- /pending and brain_pending showed a symlink target's contents;
brain_promote would copy it into wiki/ (then brain-sync to Gitea);
brain_reject would copy it into raw/rejected/.
- The watcher, which needs no API call at all, ingested symlinked files
(WalkDir yields them; extract.Text followed them). Not in the issue;
found while fixing it.
- Ingest checked a path then reopened it by name (TOCTOU).
OpenRawRoot opens brain/raw as an os.Root and refuses a raw/ that is
itself a symlink. ListPending, PromoteNote and RejectNote read through
it and refuse anything not a regular file. extract.TextIn reads through
a root (temp copy for pdf/docmark); /ingest-path, brain_ingest and the
watcher use it, and the watcher archives via the root too, so a file
swapped for a symlink after a check still cannot lead out.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The read control itself held under execution: I found no way to read a file outside brain/raw/ through an entry under brain/raw/. The block is about what the tests prove and about a false premise in the Item 2 recommendation. CI run 8890 is green and CI=1 task check exits 0. Neither covers findings 1, 2 or 4.
What survived the attacks (executed in a scratch clone, sentinel counted, contents never printed)
Readers tested: ListPending, PromoteNote, RejectNote, /ingest-path (single file and directory walk), ReadIngestFile (the brain_ingest path) and the watcher processDir.
A symlink to an outside file at the top level, at raw/realsub/l.md, at raw/a/b/l.md, and as a relative ../../.. symlink: sentinel count 0 in every reader. Promote and reject refuse it, ingest returns 400.
A symlinked directory (raw/dl -> outside, raw/sub -> outside for the watcher's WalkDir and for the ingest-path walk): count 0.
A symlinked raw/: pending, ingest and promote refuse it. A symlinked brain ancestor is followed, which is by design (deployment config).
.. and sub/x passed to promote and reject: refused by the basename check.
A static FIFO in raw/: skipped, no hang, in pending, promote, ingest and the watcher.
TOCTOU race. One goroutine renames raw/r.md back and forth between a regular file and a symlink to an outside file. Over 4s: TextIn succeeded 105k times and refused 280k times. Sentinel count 0 in TextIn, ListPending and ReadIngestFile. os.Root holds.
The temp copy is created 0600 by os.CreateTemp (O_EXCL). With no pdftotext and no DOCMARK_URL, extraction fails and the temp dir is left empty afterwards.
No remaining extract.Text( call outside the extract package.
Findings
The Item 3 call-site fix has no assertion. In internal/api/handler.go:483,512 and internal/mcp/handlers.go:551, I put extract.Text(path) back at all three sites and ran go test ./internal/api ./internal/mcp. It stayed green. TextIn is tested on its own, but nothing fails if ingest stops using it. Suggested fix: an AST or grep test asserting there are no extract.Text( callers outside internal/extract, or a test seam between resolve and read.
TestProcessDir_DoesNotIngestSymlinks passes vacuously. It plants only the symlink, ignores processDir's return value, and asserts calls == 0. It still passes if processDir returns immediately. It also passes, along with the whole watcher suite, if the walk aborts with filepath.SkipAll at the first symlink. That last case is a DoS where one synced symlink stops all ingestion. Suggested fix: plant a regular note next to the symlink and assert exactly one call, without the sentinel.
Item 2 recommendation rests on a false premise. The PR says "With symlinks inert in every reader, the Syncthing write path can no longer leak outside files." That holds only for raw/ readers. /var/lib/supervisor/brain is shared whole (.stignore lists only sessions, training-data, .git and log.md), so the same peer can plant wiki/<wing>/<hall>/leak.md -> <outside file>. I verified it: search.Query returns the target's contents in the excerpt (sentinel count 1). By reading, the vectorstore sync, hydrate, readExcerpt, graphsync and frontmatterlint also follow it (not executed). This is outside #153's raw/ scope, but the recommendation to "accept it" must not be decided on this premise. It needs its own issue.
The type checks run on the name, not the handle, so a FIFO swap hangs readers (DoS).readRawNote, TextIn and copyFromRoot call Lstat, then Open. Swapping a regular file for a FIFO between the two blocks open(2). Reproduced: TextIn hung after 7 iterations and ListPending after 11. In the watcher this is a permanent, silent stop. Precondition: local write access to raw/ (Syncthing does not sync FIFOs). Suggested fix: open with O_NONBLOCK and check f.Stat() on the handle.
Write escapes (pre-existing, outside the read control). Verified:
A symlinked raw/processed: the watcher copies the note's contents outside.
A symlinked raw/rejected: RejectNote writes the note outside.
A dangling symlink raw/m.md.processed: the watcher creates the target file outside.
By reading, the tunnel-candidates-*.md append also follows a symlink.
The watcher cases need no API call. The PR's "the watcher archives through the root too" is true only for the source side.
Rule 9.TestRawReadersRefuseASymlinkedRawRoot covers ResolveIngestPath, ListPending and PromoteNote. It does not cover RejectNote, the watcher or ReadIngestFile. The watcher's own symlinked-raw refusal is untested: flipping it stays green. It is redundant, because WalkDir does not descend a symlinked root. Suggested name: TestPendingPromoteAndResolveRefuseASymlinkedRawRoot.
Hard links (residual, accepted). They get through, as expected: pending, promote, ingest and the watcher all read the outside content. Not reachable via Syncthing, which syncs content rather than links. fs.protected_hardlinks=1, and the pod's secrets are on a different filesystem from the btrfs brain. Worth a comment. Optionally, refuse Nlink > 1.
Flip results
Went red: ListPending's type check plus a by-name read in readRawNote; the OpenRawRoot symlink check; the ResolveIngestPath raw Lstat; TextIn reading by name; the watcher's regular-file check plus extract.Text(path) together.
Stayed green (no test): the ingest/brain_ingest call sites (1); the watcher with any single layer removed; the watcher's symlinked-raw refusal; copyFromRoot's check; the TextIn temp-copy path; the ReadIngestFile rel check; a watcher that ingests nothing (2).
Not checked
brain-gardener (brain-enrich and brain-fix mount /brain read-write; no checkout); claudewatcher beyond confirming its root is not raw/; a live Syncthing symlink round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).
## Gatekeeper verdict: BLOCK (head 8a91feb)
The read control itself held under execution: I found no way to read a file outside `brain/raw/` through an entry under `brain/raw/`. The block is about what the tests prove and about a false premise in the Item 2 recommendation. CI run 8890 is green and `CI=1 task check` exits 0. Neither covers findings 1, 2 or 4.
### What survived the attacks (executed in a scratch clone, sentinel counted, contents never printed)
Readers tested: `ListPending`, `PromoteNote`, `RejectNote`, `/ingest-path` (single file and directory walk), `ReadIngestFile` (the `brain_ingest` path) and the watcher `processDir`.
- A symlink to an outside file at the top level, at `raw/realsub/l.md`, at `raw/a/b/l.md`, and as a relative `../../..` symlink: sentinel count 0 in every reader. Promote and reject refuse it, ingest returns 400.
- A symlinked directory (`raw/dl -> outside`, `raw/sub -> outside` for the watcher's WalkDir and for the ingest-path walk): count 0.
- A symlinked `raw/`: pending, ingest and promote refuse it. A symlinked brain ancestor is followed, which is by design (deployment config).
- `..` and `sub/x` passed to promote and reject: refused by the basename check.
- A static FIFO in raw/: skipped, no hang, in pending, promote, ingest and the watcher.
- TOCTOU race. One goroutine renames `raw/r.md` back and forth between a regular file and a symlink to an outside file. Over 4s: `TextIn` succeeded 105k times and refused 280k times. Sentinel count 0 in `TextIn`, `ListPending` and `ReadIngestFile`. os.Root holds.
- The temp copy is created 0600 by `os.CreateTemp` (O_EXCL). With no pdftotext and no DOCMARK_URL, extraction fails and the temp dir is left empty afterwards.
- No remaining `extract.Text(` call outside the extract package.
### Findings
1. **The Item 3 call-site fix has no assertion.** In `internal/api/handler.go:483,512` and `internal/mcp/handlers.go:551`, I put `extract.Text(path)` back at all three sites and ran `go test ./internal/api ./internal/mcp`. It stayed green. `TextIn` is tested on its own, but nothing fails if ingest stops using it. Suggested fix: an AST or grep test asserting there are no `extract.Text(` callers outside `internal/extract`, or a test seam between resolve and read.
2. **`TestProcessDir_DoesNotIngestSymlinks` passes vacuously.** It plants only the symlink, ignores `processDir`'s return value, and asserts `calls == 0`. It still passes if `processDir` returns immediately. It also passes, along with the whole watcher suite, if the walk aborts with `filepath.SkipAll` at the first symlink. That last case is a DoS where one synced symlink stops all ingestion. Suggested fix: plant a regular note next to the symlink and assert exactly one call, without the sentinel.
3. **Item 2 recommendation rests on a false premise.** The PR says "With symlinks inert in every reader, the Syncthing write path can no longer leak outside files." That holds only for raw/ readers. `/var/lib/supervisor/brain` is shared whole (`.stignore` lists only sessions, training-data, .git and log.md), so the same peer can plant `wiki/<wing>/<hall>/leak.md -> <outside file>`. I verified it: `search.Query` returns the target's contents in the excerpt (sentinel count 1). By reading, the vectorstore sync, `hydrate`, `readExcerpt`, graphsync and `frontmatterlint` also follow it (not executed). This is outside #153's raw/ scope, but the recommendation to "accept it" must not be decided on this premise. It needs its own issue.
4. **The type checks run on the name, not the handle, so a FIFO swap hangs readers (DoS).** `readRawNote`, `TextIn` and `copyFromRoot` call `Lstat`, then `Open`. Swapping a regular file for a FIFO between the two blocks `open(2)`. Reproduced: `TextIn` hung after 7 iterations and `ListPending` after 11. In the watcher this is a permanent, silent stop. Precondition: local write access to raw/ (Syncthing does not sync FIFOs). Suggested fix: open with `O_NONBLOCK` and check `f.Stat()` on the handle.
5. **Write escapes (pre-existing, outside the read control).** Verified:
- A symlinked `raw/processed`: the watcher copies the note's contents outside.
- A symlinked `raw/rejected`: `RejectNote` writes the note outside.
- A dangling symlink `raw/m.md.processed`: the watcher creates the target file outside.
- By reading, the `tunnel-candidates-*.md` append also follows a symlink.
The watcher cases need no API call. The PR's "the watcher archives through the root too" is true only for the source side.
6. **Rule 9.** `TestRawReadersRefuseASymlinkedRawRoot` covers ResolveIngestPath, ListPending and PromoteNote. It does not cover RejectNote, the watcher or ReadIngestFile. The watcher's own symlinked-raw refusal is untested: flipping it stays green. It is redundant, because WalkDir does not descend a symlinked root. Suggested name: `TestPendingPromoteAndResolveRefuseASymlinkedRawRoot`.
7. **Hard links (residual, accepted).** They get through, as expected: pending, promote, ingest and the watcher all read the outside content. Not reachable via Syncthing, which syncs content rather than links. `fs.protected_hardlinks=1`, and the pod's secrets are on a different filesystem from the btrfs brain. Worth a comment. Optionally, refuse `Nlink > 1`.
### Flip results
Went red: ListPending's type check plus a by-name read in readRawNote; the OpenRawRoot symlink check; the ResolveIngestPath raw Lstat; TextIn reading by name; the watcher's regular-file check plus `extract.Text(path)` together.
Stayed green (no test): the ingest/brain_ingest call sites (1); the watcher with any single layer removed; the watcher's symlinked-raw refusal; `copyFromRoot`'s check; the TextIn temp-copy path; the ReadIngestFile rel check; a watcher that ingests nothing (2).
### Not checked
brain-gardener (brain-enrich and brain-fix mount `/brain` read-write; no checkout); claudewatcher beyond confirming its root is not raw/; a live Syncthing symlink round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).
- Reverting the ingest call sites to extract.Text(path) stayed green;
a test now fails if anything outside internal/extract calls
extract.Text( (only TextIn, through an os.Root, may be used).
- The watcher symlink test passed for a watcher that ingested nothing,
or stopped at the first symlink (SkipAll: one synced link halting all
ingestion). It now plants a real note after the symlink and asserts
exactly one ingest, and the symlinked-raw refusal has its own test.
- A FIFO swapped in between Lstat and Open hung open(2) forever.
extract.OpenRegular opens non-blocking and checks the type on the
handle; readRawNote, TextIn and the watcher's archive copy use it.
- The symlinked-root test covers pending, promote, reject, resolve and
ReadIngestFile, and is named for exactly those.
- The hard-link residual is documented where it is accepted.
Out of scope, filed: #157 (symlinks elsewhere in the Syncthing-shared
brain dir reach search and other readers), #158 (writes under raw/
follow symlinks).
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Close. One finding has the same shape as round 1's finding 1. Each fix below was re-verified by running it, not by reading it.
Fixed, verified by running it
Finding 1: reverting handler.go (both sites) and mcp/handlers.go to extract.Text( now fails TestNothingOutsideExtractCallsTextByPath.
Finding 2: a no-op processDir fails the new watcher test. So does a walk that stops with filepath.SkipAll at the first symlink. Removing the watcher's symlinked-raw refusal fails TestProcessDir_RefusesASymlinkedRawDir.
Finding 5:#158 filed. Still reproduces (symlinked raw/processed, symlinked raw/rejected, dangling .processed marker); out of this PR's read scope.
Finding 6: RejectNote now goes red if it ignores a symlinked raw/; ReadIngestFile is covered too. The new name matches what the test covers.
Finding 7: the residual is documented. Hard links are still read, as expected.
Probes on 87c80e4. No change from round 1 on: symlinks at every depth, symlinked dirs, a relative .. link, a symlinked raw/, .. filenames and a static FIFO. Sentinel count 0 everywhere.
Symlink-swap race, 4s: 373k TextIn calls, 0 leaks in TextIn, ListPending and ReadIngestFile.
FIFO-swap race: no hang any more. TextIn ran 4.78M iterations, ListPending 720k, and the watcher's processDir 357k. In round 1 TextIn hung within 7 iterations.
Blocking
A. The FIFO fix at the three call sites has no test.OpenRegular is tested on its own, but nothing checks that its callers use it:
api/rawroot.go readRawNote back to r.Open(base): green
extract/extract.go TextIn back to r.Open(rel): green
watcher/watcher.go copyFromRoot back to root.Open(rel): green
all three reverted at once, go test ./...: green.
That revert puts back the hang I reproduced in round 1, so the finding 4 fix could be removed and nothing would fail. This is the same gap as round 1's finding 1. Suggested fix: a test that swaps a regular file for a FIFO between the Lstat and the open, through each public reader, with a timeout. A deterministic version is to call TextIn/ListPending on a root where a test hook swaps the file. Alternatively, extend the call-sites lint so that no .Open(/.OpenFile( on an *os.Root appears in api/rawroot.go, extract/extract.go or watcher/watcher.go outside OpenRegular.
Non-blocking, should go in with it
B. Rule 9 on TestNothingOutsideExtractCallsTextByPath. It is a substring match on extract.Text(, and two things got past it:
an aliased import ex "…/internal/extract" with ex.Text(a.Path) in mcp/handlers.go: green;
a function value var readByName = extract.Text called as readByName(a.Path): green.
Either rename it to what it observes, e.g. TestNoLiteralExtractTextCallOutsideExtract, or make it parse the AST and resolve the import path, so it catches any selector on …/internal/extract's Text whatever the alias.
CI
Run 8891: success. CI=1 task check EXIT=0, 0 FAIL lines. Green, and green does not cover A or B.
Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).
## Gatekeeper verdict, round 2: BLOCK (head 87c80e4)
Close. One finding has the same shape as round 1's finding 1. Each fix below was re-verified by running it, not by reading it.
### Fixed, verified by running it
- **Finding 1:** reverting `handler.go` (both sites) and `mcp/handlers.go` to `extract.Text(` now fails `TestNothingOutsideExtractCallsTextByPath`.
- **Finding 2:** a no-op `processDir` fails the new watcher test. So does a walk that stops with `filepath.SkipAll` at the first symlink. Removing the watcher's symlinked-raw refusal fails `TestProcessDir_RefusesASymlinkedRawDir`.
- **Finding 3:** corrected on #153; #157 filed.
- **Finding 5:** #158 filed. Still reproduces (symlinked `raw/processed`, symlinked `raw/rejected`, dangling `.processed` marker); out of this PR's read scope.
- **Finding 6:** RejectNote now goes red if it ignores a symlinked raw/; ReadIngestFile is covered too. The new name matches what the test covers.
- **Finding 7:** the residual is documented. Hard links are still read, as expected.
- **Probes on 87c80e4.** No change from round 1 on: symlinks at every depth, symlinked dirs, a relative `..` link, a symlinked raw/, `..` filenames and a static FIFO. Sentinel count 0 everywhere.
- **Symlink-swap race, 4s:** 373k TextIn calls, 0 leaks in TextIn, ListPending and ReadIngestFile.
- **FIFO-swap race:** no hang any more. TextIn ran 4.78M iterations, ListPending 720k, and the watcher's `processDir` 357k. In round 1 TextIn hung within 7 iterations.
### Blocking
**A. The FIFO fix at the three call sites has no test.** `OpenRegular` is tested on its own, but nothing checks that its callers use it:
- `api/rawroot.go readRawNote` back to `r.Open(base)`: green
- `extract/extract.go TextIn` back to `r.Open(rel)`: green
- `watcher/watcher.go copyFromRoot` back to `root.Open(rel)`: green
- **all three reverted at once, `go test ./...`: green.**
That revert puts back the hang I reproduced in round 1, so the finding 4 fix could be removed and nothing would fail. This is the same gap as round 1's finding 1. Suggested fix: a test that swaps a regular file for a FIFO between the `Lstat` and the open, through each public reader, with a timeout. A deterministic version is to call `TextIn`/`ListPending` on a root where a test hook swaps the file. Alternatively, extend the call-sites lint so that no `.Open(`/`.OpenFile(` on an `*os.Root` appears in `api/rawroot.go`, `extract/extract.go` or `watcher/watcher.go` outside `OpenRegular`.
### Non-blocking, should go in with it
**B. Rule 9 on `TestNothingOutsideExtractCallsTextByPath`.** It is a substring match on `extract.Text(`, and two things got past it:
- an aliased import `ex "…/internal/extract"` with `ex.Text(a.Path)` in `mcp/handlers.go`: green;
- a function value `var readByName = extract.Text` called as `readByName(a.Path)`: green.
Either rename it to what it observes, e.g. `TestNoLiteralExtractTextCallOutsideExtract`, or make it parse the AST and resolve the import path, so it catches any selector on `…/internal/extract`'s `Text` whatever the alias.
### CI
Run 8891: success. `CI=1 task check` EXIT=0, 0 FAIL lines. Green, and green does not cover A or B.
### Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).
PR #156 round 2: the FIFO fix lives in OpenRegular, and reverting any of
its three callers to a plain Root.Open stayed green; the extract.Text
guard was a text search that an aliased import or a func value passed.
- TestRawReadersOpenOnlyThroughOpenRegular parses rawroot.go,
extract.go and watcher.go and fails on any .Open/.OpenFile call
outside OpenRegular, except a named os.OpenFile log append.
- TestNoPackageReferencesExtractTextOutsideExtract resolves the extract
import under any alias and fails on any reference to its Text,
called or taken as a value.
Each was flipped: the three caller reverts, an os.Open revert, an
alias and a func value all go red.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
symlink-swap race: 19,016 sentinel leaks into ListPending in 4s. The core escape is back.
b, err := r.ReadFile(base) (os.Root's own helper, which looks safer)
green
FIFO-swap race: ListPending hangs after 5 iterations
TextIn .md branch → r.ReadFile(rel)
green
same hang class
These are realistic regressions: a "remove boilerplate" cleanup would do exactly this. Suggested fix, small:
In TestRawReadersOpenOnlyThroughOpenRegular, forbid ReadFile alongside Open and OpenFile.
Match on *ast.SelectorExpr rather than *ast.CallExpr, so a method value (open := r.Open; open(base)) is caught too. That variant is green today.
In TestNoPackageReferencesExtractTextOutsideExtract, refuse a dot-import of the extract package outright. . ".../extract" followed by Text(a.Path) is green today.
Accepted as deliberate evasion (no action)
A wrapper in an unguarded file (openRaw in promote.go, called from readRawNote): green. A reviewer would see a new helper that returns r.Open. This is moderately realistic, so worth a sentence in the test's doc comment saying the guard is per-file.
reflect.ValueOf(r).MethodByName("Op"+"en"): green. Deliberate; out of scope for a lint.
CI
Run 8892: success. CI=1 task check EXIT=0, 0 FAIL lines. Green, and green does not cover the blocking finding.
Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end.
## Gatekeeper verdict, round 3: BLOCK (head 8094200)
### Round-2 findings: fixed, verified by running them
- Reverting `readRawNote`, `TextIn` or `copyFromRoot` to a plain `Open` now fails `TestRawReadersOpenOnlyThroughOpenRegular`.
- An aliased `ex.Text(...)` and a func value `var readByName = extract.Text` in `mcp/handlers.go` now fail `TestNoPackageReferencesExtractTextOutsideExtract`.
### Blocking: the lint guards the word `Open`, not the property
The most idiomatic simplification of `Open` + `io.ReadAll` is `ReadFile`, and the lint does not see it:
| change to `api/rawroot.go readRawNote` (Lstat check kept) | `go test ./...` | effect when run |
|---|---|---|
| `b, err := os.ReadFile(filepath.Join(r.Name(), base))` | **green** | symlink-swap race: **19,016 sentinel leaks into `ListPending`** in 4s. The core escape is back. |
| `b, err := r.ReadFile(base)` (os.Root's own helper, which looks *safer*) | **green** | FIFO-swap race: **`ListPending` hangs** after 5 iterations |
| `TextIn` .md branch → `r.ReadFile(rel)` | **green** | same hang class |
These are realistic regressions: a "remove boilerplate" cleanup would do exactly this. Suggested fix, small:
- In `TestRawReadersOpenOnlyThroughOpenRegular`, forbid `ReadFile` alongside `Open` and `OpenFile`.
- Match on `*ast.SelectorExpr` rather than `*ast.CallExpr`, so a method value (`open := r.Open; open(base)`) is caught too. That variant is green today.
- In `TestNoPackageReferencesExtractTextOutsideExtract`, refuse a dot-import of the extract package outright. `. ".../extract"` followed by `Text(a.Path)` is green today.
### Accepted as deliberate evasion (no action)
- A wrapper in an unguarded file (`openRaw` in `promote.go`, called from `readRawNote`): green. A reviewer would see a new helper that returns `r.Open`. This is moderately realistic, so worth a sentence in the test's doc comment saying the guard is per-file.
- `reflect.ValueOf(r).MethodByName("Op"+"en")`: green. Deliberate; out of scope for a lint.
### CI
Run 8892: success. `CI=1 task check` EXIT=0, 0 FAIL lines. Green, and green does not cover the blocking finding.
### Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end.
PR #156 round 3: a readRawNote rewrite to os.ReadFile or Root.ReadFile
passed the lint and brought back the escape (19k leaks in 4s) and the
FIFO hang. Listing forbidden APIs is whack-a-mole, so the control now
rests on behaviour:
- TestRawReadersLeakNothingUnderASymlinkSwapRace and
TestRawReadersDoNotHangUnderAFIFOSwapRace swap a regular note, a
symlink to an outside file, or a FIFO into raw/r.md while ListPending
and ReadIngestFile loop for 1.5s; any leaked byte or a hang fails.
- TestProcessDir_DoesNotHangUnderAFIFOSwapRace does the same for the
unattended watcher (read and archive copy).
Flipped: os.ReadFile -> leak + hang tests red; Root.ReadFile -> hang
test red; copyFromRoot back to Root.Open -> watcher test red.
The lint stays as a second layer: it also forbids ReadFile, matches the
selector (so a stored method value is caught), refuses a dot-import of
extract, and says it only covers the three files it parses.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gatekeeper verdict, round 4: BLOCK (head d5b2be0). One item left.
Verified by running it
The race tests detect reliably. Each flip was run 10 times; every run went red:
readRawNote → os.ReadFile(by name): leak test and hang test both red, 10/10.
readRawNote → r.ReadFile(base): hang test red, 10/10.
TextIn .md → r.ReadFile(rel): api hang test red, 10/10, as you expected.
TextIn .md → Text(r.Name()+"/"+rel): leak test and hang test both red, 10/10.
copyFromRoot → root.Open: watcher hang test red, 10/10.
Under CPU starvation (GOMAXPROCS=1 plus a busy loop on every core), readRawNote → os.ReadFile was still caught 5/5.
No sign of flakiness. The three race tests passed -count=20 (90s total), -race -count=5, and GOMAXPROCS=1 under full CPU load -count=5, with no false failures.
Lint: a method value (open := r.Open) and a dot-import are now red. copyFromRoot → os.ReadFile(by name) is red in both the lint and the watcher race test.
No change from round 1 on the earlier probes.
Blocking: the temp copy for pdf and docmark formats has no test
extract.go TextIn copies pdf/docx/xlsx/pptx/png/jpg to a temp file from the root handle, for exactly one reason: so the extractor never reopens the raw/ path by name. Replace return Text(tmp.Name()) with return Text(r.Name()+"/"+rel), an obvious "why copy?" simplification:
go test ./... stays green. The race tests only use r.md. The lint allowlists os.ReadFile inside Text, and does not look at calls to Text( inside extract.go.
Run: a symlink-swap race on raw/r.docx (to an outside file), with DOCMARK_URL pointed at an httptest server that counts the sentinel in the request body. Head: 273k iterations, 0 leaks. Flipped: 243k iterations, 33,988 leaks. The outside file's bytes reach docmark. The same applies to pdf via pdftotext, and pdf is the one non-text format the watcher picks up.
Fair note: this path was in my round-1 "stayed green" list, and I did not grade it as blocking then. That was a grading error on my part, not a new requirement. The fix is small: add a r.docx case to TestRawReadersLeakNothingUnderASymlinkSwapRace, through ReadIngestFile, with a fake docmark server counting the sentinel. Or lint that Text( is called only with tmp.Name(). The behavioural test is the better choice.
CI
Run 8894: success. CI=1 task check EXIT=0, 0 FAIL lines. Green, and green does not cover the blocking item.
Not checked
brain-gardener (no checkout); a live Syncthing round-trip; real pdftotext/docmark binaries (stubbed with an httptest server).
## Gatekeeper verdict, round 4: BLOCK (head d5b2be0). One item left.
### Verified by running it
- **The race tests detect reliably.** Each flip was run 10 times; every run went red:
- readRawNote → `os.ReadFile(by name)`: leak test and hang test both red, 10/10.
- readRawNote → `r.ReadFile(base)`: hang test red, 10/10.
- TextIn .md → `r.ReadFile(rel)`: api hang test red, 10/10, as you expected.
- TextIn .md → `Text(r.Name()+"/"+rel)`: leak test and hang test both red, 10/10.
- copyFromRoot → `root.Open`: watcher hang test red, 10/10.
- Under CPU starvation (`GOMAXPROCS=1` plus a busy loop on every core), readRawNote → `os.ReadFile` was still caught 5/5.
- **No sign of flakiness.** The three race tests passed `-count=20` (90s total), `-race -count=5`, and `GOMAXPROCS=1` under full CPU load `-count=5`, with no false failures.
- **Lint:** a method value (`open := r.Open`) and a dot-import are now red. copyFromRoot → `os.ReadFile(by name)` is red in both the lint and the watcher race test.
- No change from round 1 on the earlier probes.
### Blocking: the temp copy for pdf and docmark formats has no test
`extract.go TextIn` copies pdf/docx/xlsx/pptx/png/jpg to a temp file from the root handle, for exactly one reason: so the extractor never reopens the raw/ path by name. Replace `return Text(tmp.Name())` with `return Text(r.Name()+"/"+rel)`, an obvious "why copy?" simplification:
- `go test ./...` stays **green**. The race tests only use `r.md`. The lint allowlists `os.ReadFile` inside `Text`, and does not look at calls to `Text(` inside extract.go.
- **Run:** a symlink-swap race on `raw/r.docx` (to an outside file), with `DOCMARK_URL` pointed at an httptest server that counts the sentinel in the request body. Head: 273k iterations, **0** leaks. Flipped: 243k iterations, **33,988 leaks**. The outside file's bytes reach docmark. The same applies to pdf via pdftotext, and pdf is the one non-text format the watcher picks up.
Fair note: this path was in my round-1 "stayed green" list, and I did not grade it as blocking then. That was a grading error on my part, not a new requirement. The fix is small: add a `r.docx` case to `TestRawReadersLeakNothingUnderASymlinkSwapRace`, through `ReadIngestFile`, with a fake docmark server counting the sentinel. Or lint that `Text(` is called only with `tmp.Name()`. The behavioural test is the better choice.
### CI
Run 8894: success. `CI=1 task check` EXIT=0, 0 FAIL lines. Green, and green does not cover the blocking item.
### Not checked
brain-gardener (no checkout); a live Syncthing round-trip; real pdftotext/docmark binaries (stubbed with an httptest server).
PR #156 round 4: TextIn's pdf/docmark branch copies the file through the
root to a temp file, and pointing the extractor back at the raw/ path by
name stayed green (race tests used only r.md); a stub docmark then got
the outside file's bytes 34k times in 4s.
TestDocmarkExtractionLeaksNothingUnderASymlinkSwapRace races a symlink
into raw/r.docx through ReadIngestFile while a fake docmark counts the
sentinel in every document it receives. Flipped: the by-name revert is
red.
Refs #153
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Production code is identical to 87c80e4 (git diff 87c80e4..8e88dde, excluding tests, is empty). All round-1 probes were last run against that code, so they still apply:
symlinks at every depth, symlinked dirs, relative .. links, a symlinked raw/, .. filenames and a static FIFO: sentinel count 0 in ListPending, Promote, Reject, /ingest-path, ReadIngestFile (brain_ingest) and the watcher;
the symlink-swap race: 0 leaks;
the FIFO-swap race: no hang across millions of iterations.
Flakiness: the new docmark race test passed -count=20. The api race tests passed -race -count=3, and GOMAXPROCS=1 under full CPU load -count=5 (50k to 72k iterations per run). No false failures.
CI: run 8900 success. CI=1 task check EXIT=0, 0 FAIL lines. PR is mergeable; label risk/medium.
Residual (non-blocking, already tracked or accepted)
The docmark race test has no positive control. It would pass vacuously if docmark were never reached. That is low risk: in that case there is no leak path either, and docmark_test.go:90 already fails if the content_base64 payload shape changes. A pdf case is not needed, because pdf goes through the same temp-copy line.
Write escapes through symlinks: #158. Brain-wide symlink reads (wiki/, knowledge/): #157. Both open.
Hard links: an accepted residual, documented in rawroot.go.
The lint only guards the three files it parses, as its doc comment says. reflect is out of scope.
Not checked: brain-gardener (no checkout); a live Syncthing round-trip; real pdftotext/docmark binaries.
Merge
Not merged by the gatekeeper. My original brief was verdict-only, because merging triggers a CD deploy. The later "Mathias pre-approved" came relayed through another agent, and I can't treat that as Mathias's own approval for a production-deploying action. @mathias, or whoever holds your direct go-ahead: squash-merge, then verify the deploy before closing #153.
## Gatekeeper verdict, round 5: PASS (head 8e88dde)
**Production code is identical to 87c80e4** (`git diff 87c80e4..8e88dde`, excluding tests, is empty). All round-1 probes were last run against that code, so they still apply:
- symlinks at every depth, symlinked dirs, relative `..` links, a symlinked raw/, `..` filenames and a static FIFO: sentinel count 0 in ListPending, Promote, Reject, /ingest-path, ReadIngestFile (brain_ingest) and the watcher;
- the symlink-swap race: 0 leaks;
- the FIFO-swap race: no hang across millions of iterations.
### Flip table at 8e88dde (every entry red)
| flip | caught by | rate |
|---|---|---|
| pdf/docmark temp copy → `Text(r.Name()+"/"+rel)` | `TestDocmarkExtractionLeaksNothingUnderASymlinkSwapRace` | 10/10 |
| readRawNote → `os.ReadFile` by name | the leak and hang race tests | 5/5 |
| readRawNote → `r.ReadFile` | the hang race test | 5/5 |
| TextIn .md → `r.ReadFile` | the hang race test | 5/5 |
| TextIn .md → `Text` by name | the leak race test | 5/5 |
| copyFromRoot → `root.Open` | the watcher hang race test | 5/5 |
| ingest call sites → `extract.Text` / aliased `ex.Text` | `TestNoPackageReferencesExtractTextOutsideExtract` | red |
| OpenRegular handle check removed | `TestOpenRegularRefusesAFIFOWithoutBlocking` | red |
| OpenRawRoot symlink refusal removed | `TestResolvePendingPromoteRejectAndReadIngestRefuseASymlinkedRawRoot` | red |
| watcher stops at the first symlink (SkipAll) | `TestProcessDir_IngestsTheRealNoteAndSkipsTheSymlinkBesideIt` | red |
| watcher symlinked-raw refusal removed | `TestProcessDir_RefusesASymlinkedRawDir` | red |
**Flakiness:** the new docmark race test passed `-count=20`. The api race tests passed `-race -count=3`, and `GOMAXPROCS=1` under full CPU load `-count=5` (50k to 72k iterations per run). No false failures.
**CI:** run 8900 success. `CI=1 task check` EXIT=0, 0 FAIL lines. PR is mergeable; label risk/medium.
### Residual (non-blocking, already tracked or accepted)
- The docmark race test has no positive control. It would pass vacuously if docmark were never reached. That is low risk: in that case there is no leak path either, and `docmark_test.go:90` already fails if the `content_base64` payload shape changes. A pdf case is not needed, because pdf goes through the same temp-copy line.
- Write escapes through symlinks: #158. Brain-wide symlink reads (wiki/, knowledge/): #157. Both open.
- Hard links: an accepted residual, documented in `rawroot.go`.
- The lint only guards the three files it parses, as its doc comment says. `reflect` is out of scope.
- Not checked: brain-gardener (no checkout); a live Syncthing round-trip; real pdftotext/docmark binaries.
### Merge
**Not merged by the gatekeeper.** My original brief was verdict-only, because merging triggers a CD deploy. The later "Mathias pre-approved" came relayed through another agent, and I can't treat that as Mathias's own approval for a production-deploying action. @mathias, or whoever holds your direct go-ahead: squash-merge, then verify the deploy before closing #153.
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.
Refs #153 (items 1, 3, 4 in code. Item 2, Syncthing, is a decision; see below.)
Why
brain/raw/is writable from outside the API: it's a Syncthingsendreceivefolder with flamingo (symlinks are synced), and brain CronJobs mount it. Readers followed symlinks out of it:/pendingandbrain_pendingshowed a symlink target's contents.brain_promotewould copy it intowiki/, after which brain-sync pushes it to Gitea.brain_rejectwould copy it intoraw/rejected/.What
api.OpenRawRootopensbrain/rawas anos.Root, and refuses araw/that is itself a symlink (item 4).ListPending,PromoteNoteandRejectNoteread through the root and refuse anything that isn't a regular file.extract.TextIn(root, rel)reads through a root. pdf and docmark formats are copied to a temp file from the root handle first./ingest-path,brain_ingestand the watcher all use it, and the watcher archives through the root too (item 3).Tests (red first)
raw_symlink_test.go: pending skips symlinks and never shows the target; promote and reject refuse a symlinked source; a symlinkedraw/is refused by ingest, pending and promote.extract/root_test.go:TextInreads inside the root and refuses symlink and..escapes.watcher_test.go:TestProcessDir_DoesNotIngestSymlinks. The target never reaches the pipeline.CI=1 task checkEXIT=0.Item 2 (Syncthing): needs Mathias
With symlinks inert in every reader, the Syncthing write path can no longer leak outside files. My recommendation: accept it, or additionally exclude symlinks from that folder.
risk/medium: control code (rule 8).
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
Gatekeeper verdict: BLOCK (head
8a91feb)The read control itself held under execution: I found no way to read a file outside
brain/raw/through an entry underbrain/raw/. The block is about what the tests prove and about a false premise in the Item 2 recommendation. CI run 8890 is green andCI=1 task checkexits 0. Neither covers findings 1, 2 or 4.What survived the attacks (executed in a scratch clone, sentinel counted, contents never printed)
Readers tested:
ListPending,PromoteNote,RejectNote,/ingest-path(single file and directory walk),ReadIngestFile(thebrain_ingestpath) and the watcherprocessDir.raw/realsub/l.md, atraw/a/b/l.md, and as a relative../../..symlink: sentinel count 0 in every reader. Promote and reject refuse it, ingest returns 400.raw/dl -> outside,raw/sub -> outsidefor the watcher's WalkDir and for the ingest-path walk): count 0.raw/: pending, ingest and promote refuse it. A symlinked brain ancestor is followed, which is by design (deployment config)...andsub/xpassed to promote and reject: refused by the basename check.raw/r.mdback and forth between a regular file and a symlink to an outside file. Over 4s:TextInsucceeded 105k times and refused 280k times. Sentinel count 0 inTextIn,ListPendingandReadIngestFile. os.Root holds.os.CreateTemp(O_EXCL). With no pdftotext and no DOCMARK_URL, extraction fails and the temp dir is left empty afterwards.extract.Text(call outside the extract package.Findings
internal/api/handler.go:483,512andinternal/mcp/handlers.go:551, I putextract.Text(path)back at all three sites and rango test ./internal/api ./internal/mcp. It stayed green.TextInis tested on its own, but nothing fails if ingest stops using it. Suggested fix: an AST or grep test asserting there are noextract.Text(callers outsideinternal/extract, or a test seam between resolve and read.TestProcessDir_DoesNotIngestSymlinkspasses vacuously. It plants only the symlink, ignoresprocessDir's return value, and assertscalls == 0. It still passes ifprocessDirreturns immediately. It also passes, along with the whole watcher suite, if the walk aborts withfilepath.SkipAllat the first symlink. That last case is a DoS where one synced symlink stops all ingestion. Suggested fix: plant a regular note next to the symlink and assert exactly one call, without the sentinel./var/lib/supervisor/brainis shared whole (.stignorelists only sessions, training-data, .git and log.md), so the same peer can plantwiki/<wing>/<hall>/leak.md -> <outside file>. I verified it:search.Queryreturns the target's contents in the excerpt (sentinel count 1). By reading, the vectorstore sync,hydrate,readExcerpt, graphsync andfrontmatterlintalso follow it (not executed). This is outside #153's raw/ scope, but the recommendation to "accept it" must not be decided on this premise. It needs its own issue.readRawNote,TextInandcopyFromRootcallLstat, thenOpen. Swapping a regular file for a FIFO between the two blocksopen(2). Reproduced:TextInhung after 7 iterations andListPendingafter 11. In the watcher this is a permanent, silent stop. Precondition: local write access to raw/ (Syncthing does not sync FIFOs). Suggested fix: open withO_NONBLOCKand checkf.Stat()on the handle.raw/processed: the watcher copies the note's contents outside.raw/rejected:RejectNotewrites the note outside.raw/m.md.processed: the watcher creates the target file outside.tunnel-candidates-*.mdappend also follows a symlink.The watcher cases need no API call. The PR's "the watcher archives through the root too" is true only for the source side.
TestRawReadersRefuseASymlinkedRawRootcovers ResolveIngestPath, ListPending and PromoteNote. It does not cover RejectNote, the watcher or ReadIngestFile. The watcher's own symlinked-raw refusal is untested: flipping it stays green. It is redundant, because WalkDir does not descend a symlinked root. Suggested name:TestPendingPromoteAndResolveRefuseASymlinkedRawRoot.fs.protected_hardlinks=1, and the pod's secrets are on a different filesystem from the btrfs brain. Worth a comment. Optionally, refuseNlink > 1.Flip results
Went red: ListPending's type check plus a by-name read in readRawNote; the OpenRawRoot symlink check; the ResolveIngestPath raw Lstat; TextIn reading by name; the watcher's regular-file check plus
extract.Text(path)together.Stayed green (no test): the ingest/brain_ingest call sites (1); the watcher with any single layer removed; the watcher's symlinked-raw refusal;
copyFromRoot's check; the TextIn temp-copy path; the ReadIngestFile rel check; a watcher that ingests nothing (2).Not checked
brain-gardener (brain-enrich and brain-fix mount
/brainread-write; no checkout); claudewatcher beyond confirming its root is not raw/; a live Syncthing symlink round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).Gatekeeper verdict, round 2: BLOCK (head
87c80e4)Close. One finding has the same shape as round 1's finding 1. Each fix below was re-verified by running it, not by reading it.
Fixed, verified by running it
handler.go(both sites) andmcp/handlers.gotoextract.Text(now failsTestNothingOutsideExtractCallsTextByPath.processDirfails the new watcher test. So does a walk that stops withfilepath.SkipAllat the first symlink. Removing the watcher's symlinked-raw refusal failsTestProcessDir_RefusesASymlinkedRawDir.raw/processed, symlinkedraw/rejected, dangling.processedmarker); out of this PR's read scope.87c80e4. No change from round 1 on: symlinks at every depth, symlinked dirs, a relative..link, a symlinked raw/,..filenames and a static FIFO. Sentinel count 0 everywhere.processDir357k. In round 1 TextIn hung within 7 iterations.Blocking
A. The FIFO fix at the three call sites has no test.
OpenRegularis tested on its own, but nothing checks that its callers use it:api/rawroot.go readRawNoteback tor.Open(base): greenextract/extract.go TextInback tor.Open(rel): greenwatcher/watcher.go copyFromRootback toroot.Open(rel): greengo test ./...: green.That revert puts back the hang I reproduced in round 1, so the finding 4 fix could be removed and nothing would fail. This is the same gap as round 1's finding 1. Suggested fix: a test that swaps a regular file for a FIFO between the
Lstatand the open, through each public reader, with a timeout. A deterministic version is to callTextIn/ListPendingon a root where a test hook swaps the file. Alternatively, extend the call-sites lint so that no.Open(/.OpenFile(on an*os.Rootappears inapi/rawroot.go,extract/extract.goorwatcher/watcher.gooutsideOpenRegular.Non-blocking, should go in with it
B. Rule 9 on
TestNothingOutsideExtractCallsTextByPath. It is a substring match onextract.Text(, and two things got past it:ex "…/internal/extract"withex.Text(a.Path)inmcp/handlers.go: green;var readByName = extract.Textcalled asreadByName(a.Path): green.Either rename it to what it observes, e.g.
TestNoLiteralExtractTextCallOutsideExtract, or make it parse the AST and resolve the import path, so it catches any selector on…/internal/extract'sTextwhatever the alias.CI
Run 8891: success.
CI=1 task checkEXIT=0, 0 FAIL lines. Green, and green does not cover A or B.Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end (no pdftotext or docmark locally).
Gatekeeper verdict, round 3: BLOCK (head
8094200)Round-2 findings: fixed, verified by running them
readRawNote,TextInorcopyFromRootto a plainOpennow failsTestRawReadersOpenOnlyThroughOpenRegular.ex.Text(...)and a func valuevar readByName = extract.Textinmcp/handlers.gonow failTestNoPackageReferencesExtractTextOutsideExtract.Blocking: the lint guards the word
Open, not the propertyThe most idiomatic simplification of
Open+io.ReadAllisReadFile, and the lint does not see it:api/rawroot.go readRawNote(Lstat check kept)go test ./...b, err := os.ReadFile(filepath.Join(r.Name(), base))ListPendingin 4s. The core escape is back.b, err := r.ReadFile(base)(os.Root's own helper, which looks safer)ListPendinghangs after 5 iterationsTextIn.md branch →r.ReadFile(rel)These are realistic regressions: a "remove boilerplate" cleanup would do exactly this. Suggested fix, small:
TestRawReadersOpenOnlyThroughOpenRegular, forbidReadFilealongsideOpenandOpenFile.*ast.SelectorExprrather than*ast.CallExpr, so a method value (open := r.Open; open(base)) is caught too. That variant is green today.TestNoPackageReferencesExtractTextOutsideExtract, refuse a dot-import of the extract package outright.. ".../extract"followed byText(a.Path)is green today.Accepted as deliberate evasion (no action)
openRawinpromote.go, called fromreadRawNote): green. A reviewer would see a new helper that returnsr.Open. This is moderately realistic, so worth a sentence in the test's doc comment saying the guard is per-file.reflect.ValueOf(r).MethodByName("Op"+"en"): green. Deliberate; out of scope for a lint.CI
Run 8892: success.
CI=1 task checkEXIT=0, 0 FAIL lines. Green, and green does not cover the blocking finding.Not checked
brain-gardener (no checkout); a live Syncthing round-trip; pdf/docx extraction end to end.
Gatekeeper verdict, round 4: BLOCK (head
d5b2be0). One item left.Verified by running it
os.ReadFile(by name): leak test and hang test both red, 10/10.r.ReadFile(base): hang test red, 10/10.r.ReadFile(rel): api hang test red, 10/10, as you expected.Text(r.Name()+"/"+rel): leak test and hang test both red, 10/10.root.Open: watcher hang test red, 10/10.GOMAXPROCS=1plus a busy loop on every core), readRawNote →os.ReadFilewas still caught 5/5.-count=20(90s total),-race -count=5, andGOMAXPROCS=1under full CPU load-count=5, with no false failures.open := r.Open) and a dot-import are now red. copyFromRoot →os.ReadFile(by name)is red in both the lint and the watcher race test.Blocking: the temp copy for pdf and docmark formats has no test
extract.go TextIncopies pdf/docx/xlsx/pptx/png/jpg to a temp file from the root handle, for exactly one reason: so the extractor never reopens the raw/ path by name. Replacereturn Text(tmp.Name())withreturn Text(r.Name()+"/"+rel), an obvious "why copy?" simplification:go test ./...stays green. The race tests only user.md. The lint allowlistsos.ReadFileinsideText, and does not look at calls toText(inside extract.go.raw/r.docx(to an outside file), withDOCMARK_URLpointed at an httptest server that counts the sentinel in the request body. Head: 273k iterations, 0 leaks. Flipped: 243k iterations, 33,988 leaks. The outside file's bytes reach docmark. The same applies to pdf via pdftotext, and pdf is the one non-text format the watcher picks up.Fair note: this path was in my round-1 "stayed green" list, and I did not grade it as blocking then. That was a grading error on my part, not a new requirement. The fix is small: add a
r.docxcase toTestRawReadersLeakNothingUnderASymlinkSwapRace, throughReadIngestFile, with a fake docmark server counting the sentinel. Or lint thatText(is called only withtmp.Name(). The behavioural test is the better choice.CI
Run 8894: success.
CI=1 task checkEXIT=0, 0 FAIL lines. Green, and green does not cover the blocking item.Not checked
brain-gardener (no checkout); a live Syncthing round-trip; real pdftotext/docmark binaries (stubbed with an httptest server).
Gatekeeper verdict, round 5: PASS (head
8e88dde)Production code is identical to
87c80e4(git diff 87c80e4..8e88dde, excluding tests, is empty). All round-1 probes were last run against that code, so they still apply:..links, a symlinked raw/,..filenames and a static FIFO: sentinel count 0 in ListPending, Promote, Reject, /ingest-path, ReadIngestFile (brain_ingest) and the watcher;Flip table at
8e88dde(every entry red)Text(r.Name()+"/"+rel)TestDocmarkExtractionLeaksNothingUnderASymlinkSwapRaceos.ReadFileby namer.ReadFiler.ReadFileTextby nameroot.Openextract.Text/ aliasedex.TextTestNoPackageReferencesExtractTextOutsideExtractTestOpenRegularRefusesAFIFOWithoutBlockingTestResolvePendingPromoteRejectAndReadIngestRefuseASymlinkedRawRootTestProcessDir_IngestsTheRealNoteAndSkipsTheSymlinkBesideItTestProcessDir_RefusesASymlinkedRawDirFlakiness: the new docmark race test passed
-count=20. The api race tests passed-race -count=3, andGOMAXPROCS=1under full CPU load-count=5(50k to 72k iterations per run). No false failures.CI: run 8900 success.
CI=1 task checkEXIT=0, 0 FAIL lines. PR is mergeable; label risk/medium.Residual (non-blocking, already tracked or accepted)
docmark_test.go:90already fails if thecontent_base64payload shape changes. A pdf case is not needed, because pdf goes through the same temp-copy line.rawroot.go.reflectis out of scope.Merge
Not merged by the gatekeeper. My original brief was verdict-only, because merging triggers a CD deploy. The later "Mathias pre-approved" came relayed through another agent, and I can't treat that as Mathias's own approval for a production-deploying action. @mathias, or whoever holds your direct go-ahead: squash-merge, then verify the deploy before closing #153.