Refs #157. This is the tripwire half (option 2). The brain-wide os.Root readers (option 1) follow later.
What
internal/symlinkguard: Scan lists symlinks in the brain dir. It skips .git, and the brain dir itself may be a mount symlink. Watch rescans on an interval and sends one ntfy alert per new set.
At startup, startupSymlinkCheck refuses to start while any symlink exists, and names them.
The rescan interval is BRAIN_SYMLINK_SCAN_INTERVAL, default 300 s.
Safe to deploy
The live /var/lib/supervisor/brain had 0 symlinks on 2026-10-03, checked before writing this (find -type l), so the rollout won't trip. Re-check right before merging.
Known limits
A window between scans: a symlink planted after a scan is live until the next one (≤5 min) and is reported, not removed. The real fix is option 1.
Refusing to start is a deliberate DoS-over-leak choice. An innocent symlink, say from an Obsidian vault, stops ingestion until it's removed, and the startup log names it.
CI=1 task check EXIT=0. Tests were written red first.
Refs #157. This is the tripwire half (option 2). The brain-wide os.Root readers (option 1) follow later.
## What
- `internal/symlinkguard`: `Scan` lists symlinks in the brain dir. It skips `.git`, and the brain dir itself may be a mount symlink. `Watch` rescans on an interval and sends one ntfy alert per new set.
- At startup, `startupSymlinkCheck` refuses to start while any symlink exists, and names them.
- The rescan interval is `BRAIN_SYMLINK_SCAN_INTERVAL`, default 300 s.
## Safe to deploy
The live `/var/lib/supervisor/brain` had **0 symlinks** on 2026-10-03, checked before writing this (`find -type l`), so the rollout won't trip. Re-check right before merging.
## Known limits
- **A window between scans:** a symlink planted after a scan is live until the next one (≤5 min) and is reported, not removed. The real fix is option 1.
- Refusing to start is a deliberate DoS-over-leak choice. An innocent symlink, say from an Obsidian vault, stops ingestion until it's removed, and the startup log names it.
`CI=1 task check` EXIT=0. Tests were written red first.
risk/medium: control code (rule 8).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
The brain dir is a Syncthing sendreceive folder and Syncthing syncs
symlinks; search, vectorstore, context and graph readers follow one out
of the brain and would index or return what it points to (reproduced
with search.Query). Decision (#157): a tripwire now, brain-wide os.Root
readers next.
- symlinkguard.Scan lists every symlink inside the brain dir (skipping
.git; the brain dir itself may be a mount symlink).
- At startup the server refuses to start while any exists, naming them.
- symlinkguard.Watch rescans every BRAIN_SYMLINK_SCAN_INTERVAL seconds
(default 300) and sends one ntfy alert per new set of symlinks.
The live brain dir had 0 symlinks on 2026-10-03, checked before this
change, so deploying it does not trip.
Refs #157
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Not merged. CI run 8893 is green, CI=1 task check EXIT=0 locally, and neither covers any of the findings below.
Findings (most severe first)
cmd/server/symlink_check.go:16-18: the tripwire fails open on any scan error, not only a missing brain dir.filepath.WalkDir aborts on the first error and Scan throws away everything it had already found, so a single unreadable directory turns off the startup refusal.
Repro (executed): brain with planted -> /tmp/x plus an empty dir aaa/ at mode 000. startupSymlinkCheck returns err=<nil>. In the same tree, search.Query still returns the sentinel from a symlinked wiki/w/facts/leak.md (hits=1), because search logs walk errors and carries on (search.go:128-131).
This is live-relevant: the pod runs as uid 1000 (runAsUser: 1000, no root DAC override), and Syncthing on koala also runs as uid 1000, so a dir a peer syncs with mode 000 is unreadable to the pod. I did not test this against a real Syncthing peer. Even with no attacker, Syncthing deleting a dir in the middle of a walk produces the same silent pass.
The fix is a correction, not a judgement call: return nil only when the root does not exist (errors.Is(err, fs.ErrNotExist) from EvalSymlinks), refuse on every other error, or make Scan record per-entry errors and keep walking.
internal/symlinkguard/symlinkguard.go:68-70: Watch goes permanently dark on a persistent scan error. Each tick logs slog.Warn and skips the notifier, so the same unreadable dir silences the runtime alert too. Nothing pages anyone. Because this control's whole value is that someone finds out, a scan that fails is itself something to alert on.
symlinkguard.go:75-84: a failed alert is never retried.last = key is set beforeNotify, so if ntfy is down at that moment the alert is lost until the symlink set changes. Executed: a failing notifier was called 1 time over ~30 ticks with the symlink still in place. Only move last forward after Notify succeeds (or n is nil).
symlinkguard.go:41: .git is skipped at every depth, not only at the root. Executed: wiki/w/facts/.git/leak.md -> outside gives Scan found=0, while search sentinel hits=1. Syncthing is probably not a route for this, since the live .stignore entry .git (no leading slash) matches at any depth. That leaves it reachable only by a writer local to koala. The real problem is that the PR's stated guarantee ("refuses to start while any symlink exists") is false. Skip only filepath.Join(root, ".git"). I flipped it to root-only and the suite stayed green, so the fix costs nothing.
cmd/server/main.go:369: BRAIN_SYMLINK_SCAN_INTERVAL<=0 panics.time.NewTicker panics with "non-positive interval for NewTicker" (executed), and because it happens in a goroutine it takes the whole server down into a crashloop. Clamp the value or reject it at startup.
Minor: the dedupe key strings.Join(found, "\n") collides for names that contain a newline ({"a\nb"} vs {"a","b"}). alertingWarnings doesn't mention that the tripwire alert goes dark when BRAIN_NTFY_URL is unset (it is set live, so there's no effect today).
Risk label: issue #157 is risk/high, PR is risk/medium. A security control on the brain should carry the issue's label.
Rule 9 (test names)
TestScanFollowsTheRootButNotEntries: the test has no symlinked entries, so "ButNotEntries" is never asserted. Rename it (e.g. ...ResolvesASymlinkedRoot) or add an entry symlink.
TestScanFindsSymlinksAtAnyDepthAndSkipsGit: "any depth" is contradicted by finding 4, and "SkipsGit" passes for a skip at any depth as well as a root-only skip.
TestWatchNotifiesOncePerNewSymlinkSet: uses only one set. Moving last= after the empty-check, which suppresses the alert when a set disappears and reappears, stayed green.
Flip tests
Mutation
Suite
drop .git skip
red
.git skip root-only
green (no test pins depth)
drop root EvalSymlinks
red
drop dedupe
red
never call Notify
red
suppress reappear notification
green
startup: scan error refuses instead of nil
green (fail-open path is untested)
remove both startupSymlinkCheck and go Watch from main.go
green (wiring unasserted)
What holds
The startup check runs before every brain reader (handler, MCP, graph backfill, vectorstore, claudewatcher).
Dangling symlinks, directory symlinks, and unicode/newline names are all reported.
A symlink inside a symlinked dir doesn't matter, because the dir link is reported.
The nil notifier is a true nil interface.
context.Background() is never cancelled, but it is one goroutine for the life of the process, so there is no leak.
Live deploy safety: INGEST_BRAIN_DIR=/app/brain is a hostPath of /var/lib/supervisor/brain, which has 0 symlinks, 0 find errors, 0 nested .git, and 0 dirs unreadable by uid 1000 (read-only check, 2026-10-03).
Not checked
Behaviour against a real Syncthing peer (mode-000 dir propagation, symlink sync).
Hard links (out of scope; Syncthing syncs content, not links).
A TOCTOU plant during a walk.
Readers other than search.Query, which I did not execute (vectorstore, context, graph).
## pr-gatekeeper verdict: **BLOCK** (PR #159 @ 796d226)
Not merged. CI run 8893 is green, `CI=1 task check` EXIT=0 locally, and **neither covers any of the findings below**.
### Findings (most severe first)
1. **`cmd/server/symlink_check.go:16-18`: the tripwire fails open on *any* scan error, not only a missing brain dir.** `filepath.WalkDir` aborts on the first error and `Scan` throws away everything it had already found, so a single unreadable directory turns off the startup refusal.
Repro (executed): brain with `planted -> /tmp/x` plus an empty dir `aaa/` at mode 000. `startupSymlinkCheck` returns `err=<nil>`. In the same tree, `search.Query` still returns the sentinel from a symlinked `wiki/w/facts/leak.md` (hits=1), because search logs walk errors and carries on (`search.go:128-131`).
This is live-relevant: the pod runs as uid 1000 (`runAsUser: 1000`, no root DAC override), and Syncthing on koala also runs as uid 1000, so a dir a peer syncs with mode 000 is unreadable to the pod. *I did not test this against a real Syncthing peer.* Even with no attacker, Syncthing deleting a dir in the middle of a walk produces the same silent pass.
The fix is a correction, not a judgement call: return nil only when the **root** does not exist (`errors.Is(err, fs.ErrNotExist)` from `EvalSymlinks`), refuse on every other error, or make Scan record per-entry errors and keep walking.
2. **`internal/symlinkguard/symlinkguard.go:68-70`: Watch goes permanently dark on a persistent scan error.** Each tick logs `slog.Warn` and skips the notifier, so the same unreadable dir silences the runtime alert too. Nothing pages anyone. Because this control's whole value is that someone finds out, a scan that fails is itself something to alert on.
3. **`symlinkguard.go:75-84`: a failed alert is never retried.** `last = key` is set *before* `Notify`, so if ntfy is down at that moment the alert is lost until the symlink set changes. Executed: a failing notifier was called **1 time over ~30 ticks** with the symlink still in place. Only move `last` forward after Notify succeeds (or n is nil).
4. **`symlinkguard.go:41`: `.git` is skipped at every depth, not only at the root.** Executed: `wiki/w/facts/.git/leak.md -> outside` gives Scan found=0, while search sentinel hits=1. Syncthing is probably *not* a route for this, since the live `.stignore` entry `.git` (no leading slash) matches at any depth. That leaves it reachable only by a writer local to koala. The real problem is that the PR's stated guarantee ("refuses to start while any symlink exists") is false. Skip only `filepath.Join(root, ".git")`. I flipped it to root-only and the suite stayed green, so the fix costs nothing.
5. **`cmd/server/main.go:369`: `BRAIN_SYMLINK_SCAN_INTERVAL<=0` panics.** `time.NewTicker` panics with "non-positive interval for NewTicker" (executed), and because it happens in a goroutine it takes the whole server down into a crashloop. Clamp the value or reject it at startup.
6. Minor: the dedupe key `strings.Join(found, "\n")` collides for names that contain a newline (`{"a\nb"}` vs `{"a","b"}`). `alertingWarnings` doesn't mention that the tripwire alert goes dark when `BRAIN_NTFY_URL` is unset (it is set live, so there's no effect today).
7. **Risk label:** issue #157 is `risk/high`, PR is `risk/medium`. A security control on the brain should carry the issue's label.
### Rule 9 (test names)
- `TestScanFollowsTheRootButNotEntries`: the test has no symlinked entries, so "ButNotEntries" is never asserted. Rename it (e.g. `...ResolvesASymlinkedRoot`) or add an entry symlink.
- `TestScanFindsSymlinksAtAnyDepthAndSkipsGit`: "any depth" is contradicted by finding 4, and "SkipsGit" passes for a skip at any depth as well as a root-only skip.
- `TestWatchNotifiesOncePerNewSymlinkSet`: uses only one set. Moving `last=` after the empty-check, which suppresses the alert when a set disappears and reappears, stayed **green**.
### Flip tests
| Mutation | Suite |
|---|---|
| drop `.git` skip | red |
| `.git` skip root-only | green (no test pins depth) |
| drop root EvalSymlinks | red |
| drop dedupe | red |
| never call Notify | red |
| suppress reappear notification | **green** |
| startup: scan error refuses instead of nil | **green** (fail-open path is untested) |
| remove both `startupSymlinkCheck` and `go Watch` from main.go | **green** (wiring unasserted) |
### What holds
- The startup check runs before every brain reader (handler, MCP, graph backfill, vectorstore, claudewatcher).
- Dangling symlinks, directory symlinks, and unicode/newline names are all reported.
- A symlink inside a symlinked dir doesn't matter, because the dir link is reported.
- The nil notifier is a true nil interface.
- `context.Background()` is never cancelled, but it is one goroutine for the life of the process, so there is no leak.
- Live deploy safety: `INGEST_BRAIN_DIR=/app/brain` is a hostPath of `/var/lib/supervisor/brain`, which has 0 symlinks, 0 find errors, 0 nested `.git`, and 0 dirs unreadable by uid 1000 (read-only check, 2026-10-03).
### Not checked
- Behaviour against a real Syncthing peer (mode-000 dir propagation, symlink sync).
- Hard links (out of scope; Syncthing syncs content, not links).
- A TOCTOU plant during a walk.
- Readers other than `search.Query`, which I did not execute (vectorstore, context, graph).
- Startup passed on ANY scan error: WalkDir stopped at the first
unreadable dir and Scan discarded what it had found, while search
kept walking and served a planted symlink. Scan now records
unreadable dirs and keeps going; startup refuses on symlinks or
unreadable dirs and only lets a missing brain dir pass.
- Watch went silent while a scan error persisted, and marked a set as
reported before Notify, so an ntfy outage lost the alert. It now
alerts on unreadable dirs and scan errors too, and records a set as
reported only after Notify succeeds (retried each tick).
- Only the root .git is skipped; a nested .git is scanned.
- A non-positive BRAIN_SYMLINK_SCAN_INTERVAL fell into time.NewTicker
and crashlooped the server; it now falls back to the default.
- Findings are quoted in alerts, so a newline in a name cannot merge
two sets; alertingWarnings names the tripwire when ntfy is unset.
- Test names match what they assert (two-set dedupe, symlinked entries
inside a symlinked root, nested .git).
Flipped: startup ignoring unreadable dirs, .git skipped at any depth,
and no retry each turn a test red. Watch tests stable over -count=10
and under -race.
Refs #157
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Not merged. CI run 8895 is green and CI=1 task check EXIT=0. -race -count=5 is also green. All of the round-1 findings are fixed, and I verified each one by running it. One fail-open remains, and it is the iron-law pattern itself.
Finding (blocking)
ingestion/internal/symlinkguard/symlinkguard.go:74: _ = filepath.WalkDir(...) throws away the error when the brain root can't be listed, and Scan reports Clean. When ReadDir fails on the root, the callback returns that error (if p == root { return err }). WalkDir passes it back up, and _ = drops it, so Scan returns an empty Result with a nil error.
Set up a brain whose root is mode 0100 (traversable but not listable), with a readable wiki/w/facts/leak.md -> outside.
startupSymlinkCheck returns err=<nil>, and search.Query returns the sentinel (hits=1).
The reason: search never lists the root. It joins knowledge/wiki and walks from there.
A root at mode 000 also passes startup (err=<nil>), though readers then see nothing.
Reachability: I don't think a Syncthing peer can change the folder root's mode (I have not verified that against Syncthing), so this most likely needs a writer local to koala. It is still a control reporting "clean" for a tree it never read. The fix is one line: return the WalkDir error from Scan, and let startup refuse on anything that isn't ErrNotExist.
Flipping startupSymlinkCheck to pass on any scan error stays green. Today the only error Scan can return is ErrNotExist, so the refusal branch is never exercised. The fix above plus a test with the root at 0100 closes both gaps.
Non-blocking
The suite does not check that the same set alerts again after it disappears and reappears. Removing reported = "" on a clean scan stays green. ...AgainForANewSet is honestly named (it tests a different set), so this is a gap, not a rule-9 issue. In round 1 I confirmed by running it that the behaviour works.
main.go wiring is untested (removing both calls stayed green in round 1). I don't consider this blocking here, provided hyperguild#154 covers this wiring class. It's worth a line on #154.
Both risk/high and risk/medium labels are on the PR (gitea-mcp#61).
Round-1 fixes: re-verified by execution
Round-1 finding
Probe
Result
1. A subdir at mode 000 let startup pass
mode-000 dir plus a symlink
Scan reports syml=1 unread=1, not Clean. Startup refuses.
2. Watch stayed silent on scan errors
Watch run on an unreadable dir, and on a missing root
1 alert each
3. A failed alert was never retried
notifier fails, then recovers
9 attempts while failing, then exactly 1 more after recovery, then quiet
4. .git was skipped at every depth
nested wiki/w/facts/.git/leak.md
reported. The root .git is still skipped.
5. An interval of 0 or less panicked
Watch with an interval of 0 and of -5
no panic
6. Newline names
name containing a newline
quoted "wiki/a\nb.md"
new
symlink planted while Watch's scans are running
caught on the next tick (alerts=1). The window between scans is inherent and documented.
Flip tests
Mutation
Suite
unreadable dir not recorded
red (2 tests)
.git skipped at any depth
red
no retry after a failed Notify
red
Watch doesn't clamp the interval
red
no reset on a clean scan (reappear suppressed)
green
startup passes on any scan error
green (see the blocking finding)
Deploy safety (read-only, re-checked now)
INGEST_BRAIN_DIR=/app/brain is a hostPath mount of /var/lib/supervisor/brain.
That dir has 0 symlinks, 0 dirs unreadable by uid 1000, and 0 find errors. The root is mode 755. The pod runs as uid 1000.
Not checked
A real Syncthing peer.
Running readers other than search (vectorstore, context, graph).
Hard links (out of scope).
## pr-gatekeeper verdict, round 2: **BLOCK** (PR #159 @ 956ad8b)
Not merged. CI run 8895 is green and `CI=1 task check` EXIT=0. `-race -count=5` is also green. All of the round-1 findings are fixed, and I verified each one by running it. One fail-open remains, and it is the iron-law pattern itself.
### Finding (blocking)
**`ingestion/internal/symlinkguard/symlinkguard.go:74`: `_ = filepath.WalkDir(...)` throws away the error when the brain root can't be listed, and Scan reports Clean.** When `ReadDir` fails on the root, the callback returns that error (`if p == root { return err }`). WalkDir passes it back up, and `_ =` drops it, so Scan returns an empty Result with a nil error.
Repro (executed at 956ad8b):
- Set up a brain whose root is mode 0100 (traversable but not listable), with a readable `wiki/w/facts/leak.md -> outside`.
- `startupSymlinkCheck` returns `err=<nil>`, and `search.Query` returns the sentinel (hits=1).
- The reason: search never lists the root. It joins `knowledge`/`wiki` and walks from there.
- A root at mode 000 also passes startup (`err=<nil>`), though readers then see nothing.
Reachability: I don't think a Syncthing peer can change the folder root's mode (I have not verified that against Syncthing), so this most likely needs a writer local to koala. It is still a control reporting "clean" for a tree it never read. The fix is one line: return the WalkDir error from Scan, and let startup refuse on anything that isn't `ErrNotExist`.
Flipping `startupSymlinkCheck` to pass on *any* scan error stays **green**. Today the only error Scan can return is ErrNotExist, so the refusal branch is never exercised. The fix above plus a test with the root at 0100 closes both gaps.
### Non-blocking
- The suite does not check that the *same* set alerts again after it disappears and reappears. Removing `reported = ""` on a clean scan stays green. `...AgainForANewSet` is honestly named (it tests a different set), so this is a gap, not a rule-9 issue. In round 1 I confirmed by running it that the behaviour works.
- main.go wiring is untested (removing both calls stayed green in round 1). I don't consider this blocking here, provided hyperguild#154 covers this wiring class. It's worth a line on #154.
- Both `risk/high` and `risk/medium` labels are on the PR (gitea-mcp#61).
### Round-1 fixes: re-verified by execution
| Round-1 finding | Probe | Result |
|---|---|---|
| 1. A subdir at mode 000 let startup pass | mode-000 dir plus a symlink | Scan reports syml=1 unread=1, not Clean. Startup refuses. |
| 2. Watch stayed silent on scan errors | Watch run on an unreadable dir, and on a missing root | 1 alert each |
| 3. A failed alert was never retried | notifier fails, then recovers | 9 attempts while failing, then exactly 1 more after recovery, then quiet |
| 4. `.git` was skipped at every depth | nested `wiki/w/facts/.git/leak.md` | reported. The root `.git` is still skipped. |
| 5. An interval of 0 or less panicked | Watch with an interval of 0 and of -5 | no panic |
| 6. Newline names | name containing a newline | quoted `"wiki/a\nb.md"` |
| new | symlink planted while Watch's scans are running | caught on the next tick (alerts=1). The window between scans is inherent and documented. |
### Flip tests
| Mutation | Suite |
|---|---|
| unreadable dir not recorded | red (2 tests) |
| `.git` skipped at any depth | red |
| no retry after a failed Notify | red |
| Watch doesn't clamp the interval | red |
| no reset on a clean scan (reappear suppressed) | **green** |
| startup passes on any scan error | **green** (see the blocking finding) |
### Deploy safety (read-only, re-checked now)
- `INGEST_BRAIN_DIR=/app/brain` is a hostPath mount of `/var/lib/supervisor/brain`.
- That dir has 0 symlinks, 0 dirs unreadable by uid 1000, and 0 find errors. The root is mode 755. The pod runs as uid 1000.
### Not checked
- A real Syncthing peer.
- Running readers other than search (vectorstore, context, graph).
- Hard links (out of scope).
PR #159 round 2: with the brain root at mode 0100 (enterable, not
listable) WalkDir failed at the root, Scan discarded the error and
reported clean, and startup passed while search, which goes straight
into wiki/, served a planted symlink. Scan now returns the root's walk
error, so startup refuses on anything but a missing root. Also pins
that a cleared set is reported again when it reappears.
Refs #157
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
root at mode 0100 plus a symlinked wiki/w/facts/leak.md (my round-2 repro; search would still serve it, hits=1)
yes
root at mode 000
yes
subdir at mode 000 plus a symlink (search hits=1)
yes
wiki/ at mode 0100 (can be entered, not listed)
yes
symlink under a nested .git
yes
missing brain dir
no (passes, by design)
clean brain
no (passes)
Flip tests: all red
Mutation
Suite
discard the walk error again
red (TestScanOfAnUnlistableRootIsAnError, TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot)
record the root error as Unreadable instead of returning it
red
startup passes on any scan error
red
no reset after a clean scan
red (TestWatchReportsASetAgainAfterItWasCleared)
.git skipped at any depth
red
The round-2 flips (no unreadable recording, no retry, no interval clamp) were red at 956ad8b. The code they cover hasn't changed since.
Rule 9: I read each new test name against its assertions, and they match.
The Geteuid()==0 skips do not hide the tests in CI. The gitea-runner pod runs as uid 1000 with runAsNonRoot: true, so the permission tests actually run there.
INGEST_BRAIN_DIR=/app/brain is a hostPath mount of /var/lib/supervisor/brain. The pod runs as uid 1000.
That dir has 0 symlinks, 0 dirs unreadable by uid 1000, and 0 find errors. The root is mode 755 and listable.
The rollout will not trip the check.
Residual, accepted or tracked
Window between scans: a symlink planted after a scan is live for up to the scan interval. Watch reports it but does not remove it. This is documented; the fix is #157 option 1.
main.go wiring is untested. This is now tracked on hyperguild#154 (comment 2026-10-03T15:15).
Stale risk/medium label is still on the PR (gitea-mcp#61).
Not checked
A real Syncthing peer.
Running readers other than search (vectorstore, context, graph).
Hard links (out of scope).
## pr-gatekeeper verdict, round 3: **PASS** (PR #159 @ cda18e6). Not merged.
This is a verdict only. The PR is `risk/high`, so the merge is Mathias's call (CD deploys on merge).
### Blocking finding from round 2: closed, verified by execution
`Scan` now returns the root's WalkDir error. Deeper errors are still recorded as Unreadable. Startup refuses on any error except ErrNotExist.
| Probe (run at cda18e6, uid 1000) | startup refuses? |
|---|---|
| root at mode 0100 plus a symlinked `wiki/w/facts/leak.md` (my round-2 repro; search would still serve it, hits=1) | **yes** |
| root at mode 000 | yes |
| subdir at mode 000 plus a symlink (search hits=1) | yes |
| `wiki/` at mode 0100 (can be entered, not listed) | yes |
| symlink under a nested `.git` | yes |
| missing brain dir | no (passes, by design) |
| clean brain | no (passes) |
### Flip tests: all red
| Mutation | Suite |
|---|---|
| discard the walk error again | red (`TestScanOfAnUnlistableRootIsAnError`, `TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot`) |
| record the root error as Unreadable instead of returning it | red |
| startup passes on any scan error | red |
| no reset after a clean scan | red (`TestWatchReportsASetAgainAfterItWasCleared`) |
| `.git` skipped at any depth | red |
The round-2 flips (no unreadable recording, no retry, no interval clamp) were red at 956ad8b. The code they cover hasn't changed since.
**Rule 9:** I read each new test name against its assertions, and they match.
**The `Geteuid()==0` skips do not hide the tests in CI.** The gitea-runner pod runs as uid 1000 with `runAsNonRoot: true`, so the permission tests actually run there.
### CI and gate
- Run 8896 on cda18e6: success.
- Local `CI=1 task check`: EXIT=0, 0 FAILs.
- `-race -count=3` on both packages: green.
### Deploy safety (read-only, re-checked now)
- `INGEST_BRAIN_DIR=/app/brain` is a hostPath mount of `/var/lib/supervisor/brain`. The pod runs as uid 1000.
- That dir has 0 symlinks, 0 dirs unreadable by uid 1000, and 0 find errors. The root is mode 755 and listable.
- The rollout will not trip the check.
### Residual, accepted or tracked
- **Window between scans:** a symlink planted after a scan is live for up to the scan interval. Watch reports it but does not remove it. This is documented; the fix is #157 option 1.
- **main.go wiring is untested.** This is now tracked on hyperguild#154 (comment 2026-10-03T15:15).
- **Stale `risk/medium` label** is still on the PR (gitea-mcp#61).
### Not checked
- A real Syncthing peer.
- Running readers other than search (vectorstore, context, graph).
- Hard links (out of scope).
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 #157. This is the tripwire half (option 2). The brain-wide os.Root readers (option 1) follow later.
What
internal/symlinkguard:Scanlists symlinks in the brain dir. It skips.git, and the brain dir itself may be a mount symlink.Watchrescans on an interval and sends one ntfy alert per new set.startupSymlinkCheckrefuses to start while any symlink exists, and names them.BRAIN_SYMLINK_SCAN_INTERVAL, default 300 s.Safe to deploy
The live
/var/lib/supervisor/brainhad 0 symlinks on 2026-10-03, checked before writing this (find -type l), so the rollout won't trip. Re-check right before merging.Known limits
CI=1 task checkEXIT=0. Tests were written red first.risk/medium: control code (rule 8).
🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
pr-gatekeeper verdict: BLOCK (PR #159 @
796d226)Not merged. CI run 8893 is green,
CI=1 task checkEXIT=0 locally, and neither covers any of the findings below.Findings (most severe first)
cmd/server/symlink_check.go:16-18: the tripwire fails open on any scan error, not only a missing brain dir.filepath.WalkDiraborts on the first error andScanthrows away everything it had already found, so a single unreadable directory turns off the startup refusal.Repro (executed): brain with
planted -> /tmp/xplus an empty diraaa/at mode 000.startupSymlinkCheckreturnserr=<nil>. In the same tree,search.Querystill returns the sentinel from a symlinkedwiki/w/facts/leak.md(hits=1), because search logs walk errors and carries on (search.go:128-131).This is live-relevant: the pod runs as uid 1000 (
runAsUser: 1000, no root DAC override), and Syncthing on koala also runs as uid 1000, so a dir a peer syncs with mode 000 is unreadable to the pod. I did not test this against a real Syncthing peer. Even with no attacker, Syncthing deleting a dir in the middle of a walk produces the same silent pass.The fix is a correction, not a judgement call: return nil only when the root does not exist (
errors.Is(err, fs.ErrNotExist)fromEvalSymlinks), refuse on every other error, or make Scan record per-entry errors and keep walking.internal/symlinkguard/symlinkguard.go:68-70: Watch goes permanently dark on a persistent scan error. Each tick logsslog.Warnand skips the notifier, so the same unreadable dir silences the runtime alert too. Nothing pages anyone. Because this control's whole value is that someone finds out, a scan that fails is itself something to alert on.symlinkguard.go:75-84: a failed alert is never retried.last = keyis set beforeNotify, so if ntfy is down at that moment the alert is lost until the symlink set changes. Executed: a failing notifier was called 1 time over ~30 ticks with the symlink still in place. Only movelastforward after Notify succeeds (or n is nil).symlinkguard.go:41:.gitis skipped at every depth, not only at the root. Executed:wiki/w/facts/.git/leak.md -> outsidegives Scan found=0, while search sentinel hits=1. Syncthing is probably not a route for this, since the live.stignoreentry.git(no leading slash) matches at any depth. That leaves it reachable only by a writer local to koala. The real problem is that the PR's stated guarantee ("refuses to start while any symlink exists") is false. Skip onlyfilepath.Join(root, ".git"). I flipped it to root-only and the suite stayed green, so the fix costs nothing.cmd/server/main.go:369:BRAIN_SYMLINK_SCAN_INTERVAL<=0panics.time.NewTickerpanics with "non-positive interval for NewTicker" (executed), and because it happens in a goroutine it takes the whole server down into a crashloop. Clamp the value or reject it at startup.Minor: the dedupe key
strings.Join(found, "\n")collides for names that contain a newline ({"a\nb"}vs{"a","b"}).alertingWarningsdoesn't mention that the tripwire alert goes dark whenBRAIN_NTFY_URLis unset (it is set live, so there's no effect today).Risk label: issue #157 is
risk/high, PR isrisk/medium. A security control on the brain should carry the issue's label.Rule 9 (test names)
TestScanFollowsTheRootButNotEntries: the test has no symlinked entries, so "ButNotEntries" is never asserted. Rename it (e.g....ResolvesASymlinkedRoot) or add an entry symlink.TestScanFindsSymlinksAtAnyDepthAndSkipsGit: "any depth" is contradicted by finding 4, and "SkipsGit" passes for a skip at any depth as well as a root-only skip.TestWatchNotifiesOncePerNewSymlinkSet: uses only one set. Movinglast=after the empty-check, which suppresses the alert when a set disappears and reappears, stayed green.Flip tests
.gitskip.gitskip root-onlystartupSymlinkCheckandgo Watchfrom main.goWhat holds
context.Background()is never cancelled, but it is one goroutine for the life of the process, so there is no leak.INGEST_BRAIN_DIR=/app/brainis a hostPath of/var/lib/supervisor/brain, which has 0 symlinks, 0 find errors, 0 nested.git, and 0 dirs unreadable by uid 1000 (read-only check, 2026-10-03).Not checked
search.Query, which I did not execute (vectorstore, context, graph).pr-gatekeeper verdict, round 2: BLOCK (PR #159 @
956ad8b)Not merged. CI run 8895 is green and
CI=1 task checkEXIT=0.-race -count=5is also green. All of the round-1 findings are fixed, and I verified each one by running it. One fail-open remains, and it is the iron-law pattern itself.Finding (blocking)
ingestion/internal/symlinkguard/symlinkguard.go:74:_ = filepath.WalkDir(...)throws away the error when the brain root can't be listed, and Scan reports Clean. WhenReadDirfails on the root, the callback returns that error (if p == root { return err }). WalkDir passes it back up, and_ =drops it, so Scan returns an empty Result with a nil error.Repro (executed at
956ad8b):wiki/w/facts/leak.md -> outside.startupSymlinkCheckreturnserr=<nil>, andsearch.Queryreturns the sentinel (hits=1).knowledge/wikiand walks from there.err=<nil>), though readers then see nothing.Reachability: I don't think a Syncthing peer can change the folder root's mode (I have not verified that against Syncthing), so this most likely needs a writer local to koala. It is still a control reporting "clean" for a tree it never read. The fix is one line: return the WalkDir error from Scan, and let startup refuse on anything that isn't
ErrNotExist.Flipping
startupSymlinkCheckto pass on any scan error stays green. Today the only error Scan can return is ErrNotExist, so the refusal branch is never exercised. The fix above plus a test with the root at 0100 closes both gaps.Non-blocking
reported = ""on a clean scan stays green....AgainForANewSetis honestly named (it tests a different set), so this is a gap, not a rule-9 issue. In round 1 I confirmed by running it that the behaviour works.risk/highandrisk/mediumlabels are on the PR (gitea-mcp#61).Round-1 fixes: re-verified by execution
.gitwas skipped at every depthwiki/w/facts/.git/leak.md.gitis still skipped."wiki/a\nb.md"Flip tests
.gitskipped at any depthDeploy safety (read-only, re-checked now)
INGEST_BRAIN_DIR=/app/brainis a hostPath mount of/var/lib/supervisor/brain.Not checked
pr-gatekeeper verdict, round 3: PASS (PR #159 @
cda18e6). Not merged.This is a verdict only. The PR is
risk/high, so the merge is Mathias's call (CD deploys on merge).Blocking finding from round 2: closed, verified by execution
Scannow returns the root's WalkDir error. Deeper errors are still recorded as Unreadable. Startup refuses on any error except ErrNotExist.cda18e6, uid 1000)wiki/w/facts/leak.md(my round-2 repro; search would still serve it, hits=1)wiki/at mode 0100 (can be entered, not listed).gitFlip tests: all red
TestScanOfAnUnlistableRootIsAnError,TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot)TestWatchReportsASetAgainAfterItWasCleared).gitskipped at any depthThe round-2 flips (no unreadable recording, no retry, no interval clamp) were red at
956ad8b. The code they cover hasn't changed since.Rule 9: I read each new test name against its assertions, and they match.
The
Geteuid()==0skips do not hide the tests in CI. The gitea-runner pod runs as uid 1000 withrunAsNonRoot: true, so the permission tests actually run there.CI and gate
cda18e6: success.CI=1 task check: EXIT=0, 0 FAILs.-race -count=3on both packages: green.Deploy safety (read-only, re-checked now)
INGEST_BRAIN_DIR=/app/brainis a hostPath mount of/var/lib/supervisor/brain. The pod runs as uid 1000.Residual, accepted or tracked
risk/mediumlabel is still on the PR (gitea-mcp#61).Not checked