Refs #157 (slice 6). risk/high: control code, adversarial review required.
Problem
These read with os.ReadFile / os.ReadDir, so a symlinked file's content reached the place listed, and a FIFO hung the ingest or startup:
readSourcePage, loadSchema → the LLM prompt (1.6M leaks in the swap race)
reviseOrCreate, injectSourceRefs, BackfillRefs → the pages the pipeline writes
classification.Load → the I1 sovereignty levels. A FIFO hung startup.
Change
pipeline:
readSourcePage, loadSchema, reviseOrCreate and injectSourceRefs read via readBrainFile (brainfs ReadRegular).
reviseOrCreate(brainDir, page) reads page.Path. A missing page is created; a non-regular page is an error.
BackfillRefs lists with ReadDirRegular and reads with ReadRegular.
classification.Load:
reads through brainfs; a symlinked or special file is a hard error, as a malformed file is
an absent file or brain dir still gives the default config
At startup a load error exits the process. The startup symlink tripwire removes symlinks first, so a planted link does not crash-loop the pod. A FIFO is not a symlink, though, so it would now exit instead of hang.
Tests
All have outside and in-brain targets and positive controls. All four pipeline tests are red against the old pipeline code, and the classification test is red (hang) against the old Load.
Refs #157 (slice 6). risk/high: control code, adversarial review required.
## Problem
These read with `os.ReadFile` / `os.ReadDir`, so a symlinked file's content reached the place listed, and a FIFO hung the ingest or startup:
- **`readSourcePage`, `loadSchema`** → the **LLM prompt** (1.6M leaks in the swap race)
- **`reviseOrCreate`, `injectSourceRefs`, `BackfillRefs`** → the **pages the pipeline writes**
- **`classification.Load`** → the **I1 sovereignty levels**. A FIFO hung startup.
## Change
- **pipeline:**
- `readSourcePage`, `loadSchema`, `reviseOrCreate` and `injectSourceRefs` read via `readBrainFile` (brainfs `ReadRegular`).
- `reviseOrCreate(brainDir, page)` reads `page.Path`. A missing page is created; a non-regular page is an error.
- `BackfillRefs` lists with `ReadDirRegular` and reads with `ReadRegular`.
- **classification.Load:**
- reads through brainfs; a symlinked or special file is a hard error, as a malformed file is
- an absent file or brain dir still gives the default config
- At startup a load error exits the process. The startup symlink tripwire removes symlinks first, so a planted link does not crash-loop the pod. A FIFO is not a symlink, though, so it would now exit instead of hang.
## Tests
All have outside **and in-brain** targets and positive controls. All four pipeline tests are red against the old pipeline code, and the classification test is red (hang) against the old Load.
- `TestReadSourcePageAndSchemaRefuseSymlinksAndSurviveSwapRaces`
- `TestReviseOrCreateRefusesASymlinkedExistingPage`
- `TestInjectSourceRefsDoesNotReadASymlinkedPage`
- `TestBackfillRefsSkipsSymlinkedSourcesAndPages`
- `TestBackfillRefsDoesNotWriteARefFromASourceSwappedInBehindASymlink`
- `TestLoadRefusesASymlinkedOrFIFOClassificationFile`
A first draft used bare `[[slug]]` links, which the pipeline ignores (it only follows `[[slug|title]]`). Its positive controls failed and caught it.
## Flips (red)
- `readBrainFile` reverted to `os.ReadFile` (3 tests)
- the BackfillRefs source read via `os.ReadFile` (race test)
- `classification.Load` via `os.ReadFile`
## Not covered
- **Pipeline writes** (`os.WriteFile` of pages, BackfillRefs writes): #158.
- **`provenance.ApplyFile`:** a read-modify-write that needs `brainfs.WriteRegular` from #170, so it moves to #158 after #170.
- **Intermediate in-brain dir redirects:** #167.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
Gatekeeper review of PR #176 (slice 6: pipeline readers and classification.Load through brainfs; head 70df6db): BLOCKED. Not merged, verdict only.
Blocking
1. ingestion/internal/pipeline/backfill.go:82: the BackfillRefs concept-page read has no test. Reverting it to os.ReadFile keeps the suite green.
Flip: bfs.ReadRegular(relPath) → os.ReadFile(filepath.Join(brainDir, relPath)). Every test passes (rc=0).
Why: the page half of TestBackfillRefsSkipsSymlinkedSourcesAndPages places a static symlinked concept page. wiki.LoadInventory (ReadDirRegular) never lists that page, so findInInventory drops the slug before the changed line runs. The assertion holds because of slice 4's inventory filter, not this read. The test name is true of BackfillRefs end to end, but nothing in the suite pins the line this PR changed.
The line is reachable. A page that is regular when the inventory is listed and is swapped for an in-brain symlink before the read gets there. I ran a probe in a scratch copy that does exactly that: one source links a concept page, and the swapper alternates the page between a regular file and a relative symlink to another brain file carrying the sentinel. The concept page is opened with O_NOFOLLOW after each BackfillRefs run.
PR head, 3 runs × 3 s: 0 sentinel copies in the page.
With the flip, 3 of 3 runs: sentinel copied into the concept page, i.e. other-brain content rewritten into a wiki page.
Writing that probe as a per-caller test closes this. Slice 4 round 1 blocked on the same pattern (a caller-level Lstat bypass stayed green), and #171 tracks it for slices 1 to 3. A new slice should not add another instance.
Non-blocking (record; not this PR's to fix)
#158, write-through under race, observed by execution. The same probe on PR head overwrote the in-brain symlink target about 5,000 times in 3 s. BackfillRefs reads the regular page, the swapper puts a symlink in its place, and then os.WriteFile(dest) follows the symlink. buildAndWrite (MkdirAll + WriteFile) and wiki.AppendLog have the same shape. This PR does not make it worse, and reviseOrCreate refusing a static symlink now stops the static final-link write-through that main had. A target outside the brain would get the same write. Please make sure #158 includes BackfillRefs explicitly.
Abort, not skip.reviseOrCreate on a symlinked existing page returns an error, which aborts buildAndWrite partway: earlier pages are written, and the index rebuild and log append are skipped. That is better than writing through the link, but the watcher moves the file to failed/. Consider skip-with-warning in #158.
(c) classification.Load fail-hard is correct. An absent file falls back to defaults, so fail-open on a refused file would drop explicit tags, some of which may be stricter than the default. I checked these by execution: a no-writer FIFO, a FIFO with a writer, a directory and a dangling link each return "not a regular file" promptly with no hang; a missing brainDir returns defaults. A symlinked file cannot reach line 537, because startupSymlinkCheck exits first. A FIFO passes the tripwire (symlinkguard only reports symlinks). On main it hung startup forever in open(2). Now it is a fast, logged os.Exit(1) that names the file. That is still a crash-loop for whoever can create special files in the pod volume, but Syncthing does not sync FIFOs, so it needs local write access. Optional: have symlinkguard report special files too, so the tripwire message names them.
TestInjectSourceRefsDoesNotReadASymlinkedPage uses a hand-built inventory that includes the symlink. Production LoadInventory would filter it, so in production that read is reachable only by a race. The unit test is still valid, and its name does not overclaim.
Nit: the brainfs import sits inside the stdlib import group in 3 files. Lint passes.
Checked and held (by execution)
(a) Prompt inputs. No os.ReadFile/ReadDir/Open/Walk remains in internal/pipeline, internal/classification or internal/wiki apart from the AppendLog write. The prompt is built from LoadInventory (slice 4), loadSchema and readSourcePage, and the latter two are pinned by flips. The content comes from callers that went through ResolveIngestPath/ReadIngestFile/extract.OpenRegular in earlier slices. I did not re-attack those here.
(b) LLM-controlled paths. Dry-run RunRaw over 80 type × title combinations (dot-dot segments, slashes, backslashes, absolute paths, U+2215, fullwidth dots, NUL, upper case) and 3 hostile source values. Every produced path was wiki/<type>/<slug>.md with exactly 2 slashes, so none could escape. Two things enforce this: validPageType (^[a-z][a-z0-9-]*$), and wiki.Slug, which keeps only letters, digits and hyphens. Resolve substitutes an inventory slug, which is a stem of a regular file name and so cannot contain /. A file named ...md gives slug .., but the path becomes wiki/<t>/...md, which is a file name, not a traversal. So I found no write traversal through filepath.Join(brainDir, page.Path).
Flips that went red: readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs, the BackfillRefs source read (swap test), the BackfillRefs list and read together (static test), classification read to os.ReadFile, classification fail-open on any error, and brainfs SameFile removal.
(d) Rule 9. Apart from finding 1, each name matches what its assertions check.
go test -race -count=5 on pipeline and classification: green. CI=1 task check: exit 0 (lint, race tests, gitleaks), and the worktree stayed clean. CI run 9323 (ci.yml, pull_request) succeeded at 70df6db: green, and green covers none of finding 1.
Not checked
Hard links and in-brain intermediate dir redirects (accepted, #167). The write path beyond what the probe observed (#158). The callers that produce content (earlier slices). Behaviour on a non-Linux FS. The live deploy.
The PR is labelled risk/high, so the merge is Mathias's call even after a PASS.
Gatekeeper review of PR #176 (slice 6: pipeline readers and classification.Load through brainfs; head 70df6db): **BLOCKED**. Not merged, verdict only.
## Blocking
**1. `ingestion/internal/pipeline/backfill.go:82`: the BackfillRefs concept-page read has no test. Reverting it to `os.ReadFile` keeps the suite green.**
- Flip: `bfs.ReadRegular(relPath)` → `os.ReadFile(filepath.Join(brainDir, relPath))`. Every test passes (rc=0).
- Why: the page half of `TestBackfillRefsSkipsSymlinkedSourcesAndPages` places a *static* symlinked concept page. `wiki.LoadInventory` (ReadDirRegular) never lists that page, so `findInInventory` drops the slug before the changed line runs. The assertion holds because of slice 4's inventory filter, not this read. The test name is true of BackfillRefs end to end, but nothing in the suite pins the line this PR changed.
- The line is reachable. A page that is regular when the inventory is listed and is swapped for an in-brain symlink before the read gets there. I ran a probe in a scratch copy that does exactly that: one source links a concept page, and the swapper alternates the page between a regular file and a relative symlink to another brain file carrying the sentinel. The concept page is opened with O_NOFOLLOW after each BackfillRefs run.
- PR head, 3 runs × 3 s: **0** sentinel copies in the page.
- With the flip, 3 of 3 runs: sentinel copied into the concept page, i.e. other-brain content rewritten into a wiki page.
- Writing that probe as a per-caller test closes this. Slice 4 round 1 blocked on the same pattern (a caller-level Lstat bypass stayed green), and #171 tracks it for slices 1 to 3. A new slice should not add another instance.
## Non-blocking (record; not this PR's to fix)
- **#158, write-through under race, observed by execution.** The same probe on PR head overwrote the in-brain symlink *target* about 5,000 times in 3 s. BackfillRefs reads the regular page, the swapper puts a symlink in its place, and then `os.WriteFile(dest)` follows the symlink. `buildAndWrite` (`MkdirAll` + `WriteFile`) and `wiki.AppendLog` have the same shape. This PR does not make it worse, and `reviseOrCreate` refusing a static symlink now stops the static final-link write-through that main had. A target outside the brain would get the same write. Please make sure #158 includes BackfillRefs explicitly.
- **Abort, not skip.** `reviseOrCreate` on a symlinked existing page returns an error, which aborts `buildAndWrite` partway: earlier pages are written, and the index rebuild and log append are skipped. That is better than writing through the link, but the watcher moves the file to `failed/`. Consider skip-with-warning in #158.
- **(c) classification.Load fail-hard is correct.** An absent file falls back to defaults, so fail-open on a refused file would drop explicit tags, some of which may be stricter than the default. I checked these by execution: a no-writer FIFO, a FIFO with a writer, a directory and a dangling link each return "not a regular file" promptly with no hang; a missing brainDir returns defaults. A symlinked file cannot reach line 537, because `startupSymlinkCheck` exits first. A **FIFO** passes the tripwire (symlinkguard only reports symlinks). On main it hung startup forever in `open(2)`. Now it is a fast, logged `os.Exit(1)` that names the file. That is still a crash-loop for whoever can create special files in the pod volume, but Syncthing does not sync FIFOs, so it needs local write access. Optional: have symlinkguard report special files too, so the tripwire message names them.
- `TestInjectSourceRefsDoesNotReadASymlinkedPage` uses a hand-built inventory that includes the symlink. Production `LoadInventory` would filter it, so in production that read is reachable only by a race. The unit test is still valid, and its name does not overclaim.
- Nit: the `brainfs` import sits inside the stdlib import group in 3 files. Lint passes.
## Checked and held (by execution)
- **(a) Prompt inputs.** No `os.ReadFile`/`ReadDir`/`Open`/`Walk` remains in `internal/pipeline`, `internal/classification` or `internal/wiki` apart from the AppendLog write. The prompt is built from `LoadInventory` (slice 4), `loadSchema` and `readSourcePage`, and the latter two are pinned by flips. The content comes from callers that went through `ResolveIngestPath`/`ReadIngestFile`/`extract.OpenRegular` in earlier slices. I did not re-attack those here.
- **(b) LLM-controlled paths.** Dry-run `RunRaw` over 80 type × title combinations (dot-dot segments, slashes, backslashes, absolute paths, U+2215, fullwidth dots, NUL, upper case) and 3 hostile `source` values. Every produced path was `wiki/<type>/<slug>.md` with exactly 2 slashes, so none could escape. Two things enforce this: `validPageType` (`^[a-z][a-z0-9-]*$`), and `wiki.Slug`, which keeps only letters, digits and hyphens. `Resolve` substitutes an inventory slug, which is a stem of a regular file name and so cannot contain `/`. A file named `...md` gives slug `..`, but the path becomes `wiki/<t>/...md`, which is a file name, not a traversal. So I found no write traversal through `filepath.Join(brainDir, page.Path)`.
- Flips that went red: readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs, the BackfillRefs source read (swap test), the BackfillRefs list and read together (static test), classification read to os.ReadFile, classification fail-open on any error, and brainfs SameFile removal.
- **(d) Rule 9.** Apart from finding 1, each name matches what its assertions check.
- `go test -race -count=5` on pipeline and classification: green. `CI=1 task check`: exit 0 (lint, race tests, gitleaks), and the worktree stayed clean. CI run 9323 (ci.yml, pull_request) succeeded at 70df6db: **green, and green covers none of finding 1.**
## Not checked
Hard links and in-brain intermediate dir redirects (accepted, #167). The write path beyond what the probe observed (#158). The callers that produce `content` (earlier slices). Behaviour on a non-Linux FS. The live deploy.
The PR is labelled `risk/high`, so the merge is Mathias's call even after a PASS.
Gatekeeper round 2 on PR #176 (slice 6, head 3cfe345): BLOCKED. Not merged, verdict only. One small fix is needed.
Blocking
1. ingestion/internal/pipeline/root_test.go:165: the new test's positive control does not check what its message says (rule 9). As a result the test stays green when BackfillRefs reads or writes nothing.
require.Contains(t,read(),"zqinside","positive control: the regular page gets the ref")
zqinside is already in the page before BackfillRefs runs, so this assertion holds whether or not a ref was added. I ran TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink alone with each of these flips in a scratch copy:
flip in backfill.go
new test
page read → os.ReadFile
red (3 of 3; e.g. 4145 leaks in 87k iterations)
page read → always fs.ErrNotExist (no page is ever read)
green
return 0, nil right after LoadInventory (BackfillRefs does nothing)
green
Other backfill tests do catch the last two, so the suite as a whole goes red. The problem is this test alone. A refusal test whose positive control cannot tell "refused the swapped link" from "never read anything" passes for the wrong reason, and the message makes it look as if it proves more.
Fix (verified): assert on the ref instead, e.g. require.Contains(t, read(), "[[s|S]]", ...). With that change the test is green on head, red when no page is read, and still red under the os.ReadFile flip.
Verified, held
Round 1's blocker is closed. The page-read flip now turns the new test red.
Non-blocking items are recorded: #158 has a comment naming the BackfillRefs/buildAndWrite/AppendLog write-through, and #179 (symlinkguard ignores FIFOs) is open.
go test -race -count=5 on pipeline and classification passes. CI=1 task check exits 0, and the worktree is clean.
CI run 9347 succeeded at 3cfe345. It is green, but it does not cover finding 1.
The round-1 checks were not re-run: the diff since 70df6db only adds this test.
Not checked
Same as round 1: #167 residuals, write paths (#158), upstream content callers, live deploy. The PR is risk/high, so the merge is Mathias's call.
Gatekeeper round 2 on PR #176 (slice 6, head 3cfe345): **BLOCKED**. Not merged, verdict only. One small fix is needed.
## Blocking
**1. `ingestion/internal/pipeline/root_test.go:165`: the new test's positive control does not check what its message says (rule 9). As a result the test stays green when BackfillRefs reads or writes nothing.**
```go
require.Contains(t, read(), "zqinside", "positive control: the regular page gets the ref")
```
`zqinside` is already in the page before BackfillRefs runs, so this assertion holds whether or not a ref was added. I ran `TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink` alone with each of these flips in a scratch copy:
| flip in backfill.go | new test |
|---|---|
| page read → `os.ReadFile` | **red** (3 of 3; e.g. 4145 leaks in 87k iterations) |
| page read → always `fs.ErrNotExist` (no page is ever read) | **green** |
| return `0, nil` right after LoadInventory (BackfillRefs does nothing) | **green** |
Other backfill tests do catch the last two, so the suite as a whole goes red. The problem is this test alone. A refusal test whose positive control cannot tell "refused the swapped link" from "never read anything" passes for the wrong reason, and the message makes it look as if it proves more.
**Fix (verified):** assert on the ref instead, e.g. `require.Contains(t, read(), "[[s|S]]", ...)`. With that change the test is green on head, red when no page is read, and still red under the `os.ReadFile` flip.
## Verified, held
- Round 1's blocker is closed. The page-read flip now turns the new test red.
- Non-blocking items are recorded: #158 has a comment naming the BackfillRefs/buildAndWrite/AppendLog write-through, and #179 (symlinkguard ignores FIFOs) is open.
- `go test -race -count=5` on pipeline and classification passes. `CI=1 task check` exits 0, and the worktree is clean.
- CI run 9347 succeeded at 3cfe345. It is green, but it does not cover finding 1.
- The round-1 checks were not re-run: the diff since 70df6db only adds this test.
## Not checked
Same as round 1: #167 residuals, write paths (#158), upstream content callers, live deploy. The PR is `risk/high`, so the merge is Mathias's call.
The ingest pipeline read wiki/sources/<slug>.md and schema.md (both go
into the LLM prompt), the pages it revises, and the pages it adds
source refs to, with os.ReadFile; BackfillRefs listed wiki/sources with
os.ReadDir. A symlinked file's content reached the prompt and the
written pages (1.6M leaks in the race), and a FIFO hung the ingest.
classification.Load (the I1 sovereignty levels) read
classification.yaml the same way and hung startup on a FIFO.
- pipeline: readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs
read through readBrainFile (brainfs ReadRegular); reviseOrCreate takes
the brain dir and reads page.Path, a missing page is still created and
a non-regular one is an error. BackfillRefs lists with ReadDirRegular
and reads with ReadRegular.
- classification.Load: read through brainfs; a symlinked or special
classification.yaml is a hard error (as a malformed one is); an absent
file or brain dir still yields the default config.
Tests use in-brain targets and the [[slug|title]] link form the pipeline
actually follows (bare [[slug]] links are ignored by it, which made a
first draft of the backfill test vacuous).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 1 on PR #176: reverting the BackfillRefs concept-page read
to os.ReadFile left every test green. The static symlinked page never
reaches that read, because LoadInventory does not list it; the read is
reachable only when a page listed as regular is swapped for a symlink
before it is read. New test does that against an in-brain target and
reads the page back with O_NOFOLLOW; red under the flip (2/2), green
3/3 on head.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 2 on PR #176: the control checked for text that was in the
page before BackfillRefs ran, so the test stayed green when no page was
read at all. It now asserts the [[s|S]] ref BackfillRefs adds; red when
the page read always fails, still red when the read follows the link.
Branch rebased onto main.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gatekeeper round 3 on PR #176 (slice 6, head b8ea201): PASS. Not merged, verdict only. The PR is risk/high, so the merge is Mathias's call.
Rebase scope (verified)
The branch sits on current main (febab3e). git range-diff shows the two round-2 commits are patch-identical (=) after the rebase. The only new commit is b8ea201, which changes one line in pipeline/root_test.go.
git diff origin/main...b8ea201 touches only the same 6 files as before: classification/{classification.go,root_test.go} and pipeline/{backfill.go,pipeline.go,refs.go,root_test.go}. The rebase brought nothing else in.
Main's own changes to brainfs since the PR's original base (ReadDirDirs, WriteRegular, and the #177 floors in AssertNoLeakFrom) are not in this PR's diff. The floors now apply to this PR's swap tests, which require a swap to have happened and an inside read to have returned. All of them pass.
Round-2 blocker closed (by execution)
TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink, run alone against flips in a scratch copy:
flip in backfill.go
round 2
round 3
page read → os.ReadFile
red
red (4173 leaks in 83k iterations)
page read → always fs.ErrNotExist
green
red (positive control, line 165)
return right after LoadInventory
green
red (positive control)
Round-1 flips, re-run on the rebased tree, all red
readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs, the BackfillRefs source read, classification read → os.ReadFile, and brainfs SameFile removal (which turns 3 swap tests red, including the new one).
Gates
go test -race -count=5 on pipeline and classification: green.
CI run 9444 (pull_request) succeeded at b8ea201. Gitea reports the PR as mergeable.
Carried forward, not blocking
Write-through on BackfillRefs, buildAndWrite and AppendLog, and skip-vs-abort, are tracked in #158. The FIFO classification.yaml crash-loop is tracked in #179. Hard links and intermediate redirects are tracked in #167.
Not checked
The upstream content callers (earlier slices), write paths beyond the round-1 probe, and the live deploy and post-merge smoke test.
Gatekeeper round 3 on PR #176 (slice 6, head b8ea201): **PASS**. Not merged, verdict only. The PR is `risk/high`, so the merge is Mathias's call.
## Rebase scope (verified)
- The branch sits on current main (febab3e). `git range-diff` shows the two round-2 commits are patch-identical (`=`) after the rebase. The only new commit is b8ea201, which changes one line in `pipeline/root_test.go`.
- `git diff origin/main...b8ea201` touches only the same 6 files as before: `classification/{classification.go,root_test.go}` and `pipeline/{backfill.go,pipeline.go,refs.go,root_test.go}`. The rebase brought nothing else in.
- Main's own changes to brainfs since the PR's original base (ReadDirDirs, WriteRegular, and the #177 floors in `AssertNoLeakFrom`) are not in this PR's diff. The floors now apply to this PR's swap tests, which require a swap to have happened and an inside read to have returned. All of them pass.
## Round-2 blocker closed (by execution)
`TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink`, run alone against flips in a scratch copy:
| flip in backfill.go | round 2 | round 3 |
|---|---|---|
| page read → `os.ReadFile` | red | **red** (4173 leaks in 83k iterations) |
| page read → always `fs.ErrNotExist` | green | **red** (positive control, line 165) |
| return right after LoadInventory | green | **red** (positive control) |
## Round-1 flips, re-run on the rebased tree, all red
readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs, the BackfillRefs source read, classification read → os.ReadFile, and brainfs SameFile removal (which turns 3 swap tests red, including the new one).
## Gates
- `go test -race -count=5` on pipeline and classification: green.
- `CI=1 task check` at b8ea201: exit 0, 0 FAIL lines, worktree clean.
- CI run 9444 (pull_request) succeeded at b8ea201. Gitea reports the PR as mergeable.
## Carried forward, not blocking
Write-through on BackfillRefs, buildAndWrite and AppendLog, and skip-vs-abort, are tracked in #158. The FIFO `classification.yaml` crash-loop is tracked in #179. Hard links and intermediate redirects are tracked in #167.
## Not checked
The upstream `content` callers (earlier slices), write paths beyond the round-1 probe, and the live deploy and post-merge smoke test.
mathias
merged commit eedb110be5 into main2026-10-05 18:54:42 +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
Refs #157 (slice 6). risk/high: control code, adversarial review required.
Problem
These read with
os.ReadFile/os.ReadDir, so a symlinked file's content reached the place listed, and a FIFO hung the ingest or startup:readSourcePage,loadSchema→ the LLM prompt (1.6M leaks in the swap race)reviseOrCreate,injectSourceRefs,BackfillRefs→ the pages the pipeline writesclassification.Load→ the I1 sovereignty levels. A FIFO hung startup.Change
readSourcePage,loadSchema,reviseOrCreateandinjectSourceRefsread viareadBrainFile(brainfsReadRegular).reviseOrCreate(brainDir, page)readspage.Path. A missing page is created; a non-regular page is an error.BackfillRefslists withReadDirRegularand reads withReadRegular.Tests
All have outside and in-brain targets and positive controls. All four pipeline tests are red against the old pipeline code, and the classification test is red (hang) against the old Load.
TestReadSourcePageAndSchemaRefuseSymlinksAndSurviveSwapRacesTestReviseOrCreateRefusesASymlinkedExistingPageTestInjectSourceRefsDoesNotReadASymlinkedPageTestBackfillRefsSkipsSymlinkedSourcesAndPagesTestBackfillRefsDoesNotWriteARefFromASourceSwappedInBehindASymlinkTestLoadRefusesASymlinkedOrFIFOClassificationFileA first draft used bare
[[slug]]links, which the pipeline ignores (it only follows[[slug|title]]). Its positive controls failed and caught it.Flips (red)
readBrainFilereverted toos.ReadFile(3 tests)os.ReadFile(race test)classification.Loadviaos.ReadFileNot covered
os.WriteFileof pages, BackfillRefs writes): #158.provenance.ApplyFile: a read-modify-write that needsbrainfs.WriteRegularfrom #170, so it moves to #158 after #170.🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
Gatekeeper review of PR #176 (slice 6: pipeline readers and classification.Load through brainfs; head
70df6db): BLOCKED. Not merged, verdict only.Blocking
1.
ingestion/internal/pipeline/backfill.go:82: the BackfillRefs concept-page read has no test. Reverting it toos.ReadFilekeeps the suite green.bfs.ReadRegular(relPath)→os.ReadFile(filepath.Join(brainDir, relPath)). Every test passes (rc=0).TestBackfillRefsSkipsSymlinkedSourcesAndPagesplaces a static symlinked concept page.wiki.LoadInventory(ReadDirRegular) never lists that page, sofindInInventorydrops the slug before the changed line runs. The assertion holds because of slice 4's inventory filter, not this read. The test name is true of BackfillRefs end to end, but nothing in the suite pins the line this PR changed.Non-blocking (record; not this PR's to fix)
os.WriteFile(dest)follows the symlink.buildAndWrite(MkdirAll+WriteFile) andwiki.AppendLoghave the same shape. This PR does not make it worse, andreviseOrCreaterefusing a static symlink now stops the static final-link write-through that main had. A target outside the brain would get the same write. Please make sure #158 includes BackfillRefs explicitly.reviseOrCreateon a symlinked existing page returns an error, which abortsbuildAndWritepartway: earlier pages are written, and the index rebuild and log append are skipped. That is better than writing through the link, but the watcher moves the file tofailed/. Consider skip-with-warning in #158.startupSymlinkCheckexits first. A FIFO passes the tripwire (symlinkguard only reports symlinks). On main it hung startup forever inopen(2). Now it is a fast, loggedos.Exit(1)that names the file. That is still a crash-loop for whoever can create special files in the pod volume, but Syncthing does not sync FIFOs, so it needs local write access. Optional: have symlinkguard report special files too, so the tripwire message names them.TestInjectSourceRefsDoesNotReadASymlinkedPageuses a hand-built inventory that includes the symlink. ProductionLoadInventorywould filter it, so in production that read is reachable only by a race. The unit test is still valid, and its name does not overclaim.brainfsimport sits inside the stdlib import group in 3 files. Lint passes.Checked and held (by execution)
os.ReadFile/ReadDir/Open/Walkremains ininternal/pipeline,internal/classificationorinternal/wikiapart from the AppendLog write. The prompt is built fromLoadInventory(slice 4),loadSchemaandreadSourcePage, and the latter two are pinned by flips. The content comes from callers that went throughResolveIngestPath/ReadIngestFile/extract.OpenRegularin earlier slices. I did not re-attack those here.RunRawover 80 type × title combinations (dot-dot segments, slashes, backslashes, absolute paths, U+2215, fullwidth dots, NUL, upper case) and 3 hostilesourcevalues. Every produced path waswiki/<type>/<slug>.mdwith exactly 2 slashes, so none could escape. Two things enforce this:validPageType(^[a-z][a-z0-9-]*$), andwiki.Slug, which keeps only letters, digits and hyphens.Resolvesubstitutes an inventory slug, which is a stem of a regular file name and so cannot contain/. A file named...mdgives slug.., but the path becomeswiki/<t>/...md, which is a file name, not a traversal. So I found no write traversal throughfilepath.Join(brainDir, page.Path).go test -race -count=5on pipeline and classification: green.CI=1 task check: exit 0 (lint, race tests, gitleaks), and the worktree stayed clean. CI run 9323 (ci.yml, pull_request) succeeded at70df6db: green, and green covers none of finding 1.Not checked
Hard links and in-brain intermediate dir redirects (accepted, #167). The write path beyond what the probe observed (#158). The callers that produce
content(earlier slices). Behaviour on a non-Linux FS. The live deploy.The PR is labelled
risk/high, so the merge is Mathias's call even after a PASS.Gatekeeper round 2 on PR #176 (slice 6, head
3cfe345): BLOCKED. Not merged, verdict only. One small fix is needed.Blocking
1.
ingestion/internal/pipeline/root_test.go:165: the new test's positive control does not check what its message says (rule 9). As a result the test stays green when BackfillRefs reads or writes nothing.zqinsideis already in the page before BackfillRefs runs, so this assertion holds whether or not a ref was added. I ranTestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlinkalone with each of these flips in a scratch copy:os.ReadFilefs.ErrNotExist(no page is ever read)0, nilright after LoadInventory (BackfillRefs does nothing)Other backfill tests do catch the last two, so the suite as a whole goes red. The problem is this test alone. A refusal test whose positive control cannot tell "refused the swapped link" from "never read anything" passes for the wrong reason, and the message makes it look as if it proves more.
Fix (verified): assert on the ref instead, e.g.
require.Contains(t, read(), "[[s|S]]", ...). With that change the test is green on head, red when no page is read, and still red under theos.ReadFileflip.Verified, held
go test -race -count=5on pipeline and classification passes.CI=1 task checkexits 0, and the worktree is clean.3cfe345. It is green, but it does not cover finding 1.70df6dbonly adds this test.Not checked
Same as round 1: #167 residuals, write paths (#158), upstream content callers, live deploy. The PR is
risk/high, so the merge is Mathias's call.3cfe345415tob8ea201bd3Gatekeeper round 3 on PR #176 (slice 6, head
b8ea201): PASS. Not merged, verdict only. The PR isrisk/high, so the merge is Mathias's call.Rebase scope (verified)
febab3e).git range-diffshows the two round-2 commits are patch-identical (=) after the rebase. The only new commit isb8ea201, which changes one line inpipeline/root_test.go.git diff origin/main...b8ea201touches only the same 6 files as before:classification/{classification.go,root_test.go}andpipeline/{backfill.go,pipeline.go,refs.go,root_test.go}. The rebase brought nothing else in.AssertNoLeakFrom) are not in this PR's diff. The floors now apply to this PR's swap tests, which require a swap to have happened and an inside read to have returned. All of them pass.Round-2 blocker closed (by execution)
TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink, run alone against flips in a scratch copy:os.ReadFilefs.ErrNotExistRound-1 flips, re-run on the rebased tree, all red
readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs, the BackfillRefs source read, classification read → os.ReadFile, and brainfs SameFile removal (which turns 3 swap tests red, including the new one).
Gates
go test -race -count=5on pipeline and classification: green.CI=1 task checkatb8ea201: exit 0, 0 FAIL lines, worktree clean.b8ea201. Gitea reports the PR as mergeable.Carried forward, not blocking
Write-through on BackfillRefs, buildAndWrite and AppendLog, and skip-vs-abort, are tracked in #158. The FIFO
classification.yamlcrash-loop is tracked in #179. Hard links and intermediate redirects are tracked in #167.Not checked
The upstream
contentcallers (earlier slices), write paths beyond the round-1 probe, and the live deploy and post-merge smoke test.