mathias/hyperguild
1
0
Fork 0
Code Issues 30 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

brain/raw/ readers follow symlinks (pending, promote); raw/ is writable via Syncthing #153

New Issue
Closed
opened 2026-10-03 14:32:32 +00:00 by mathias · 8 comments
mathias commented 2026-10-03 14:32:32 +00:00
Owner
Copy Link
Copy Source

Found by the PR #152 gatekeeper review, 2026-10-03. Pre-existing; PR #152 confines /ingest-path and brain_ingest but not the other readers of raw/.

1. Other raw/ readers follow symlinks

  • GET /pending and MCP brain_pending returned the target contents of raw/x.md -> <file outside the brain> in their excerpt. Verified by counting a marker; the bytes were not printed.
  • brain_promote (internal/api/promote.go:108) would copy such a target into wiki/, after which brain-sync pushes it to Gitea.

Fix: treat a symlink anywhere under raw/ as not-a-note. Lstat in ListPending/PromoteNote, or reuse ResolveIngestPath. Better still, open through os.OpenRoot(brain/raw) so reads can't leave the root. That also closes item 3.

2. raw/ is writable from outside the API

/var/lib/supervisor/brain is a Syncthing sendreceive folder shared with flamingo. raw/ is not in .stignore, and Syncthing syncs symlinks. uid 1000 on koala and the brain-* CronJobs that mount the folder can also write there. No API route lets a remote caller plant a file of its choosing in raw/, so for now the exposure is limited to whoever controls flamingo or koala.

Decide: exclude raw/ symlinks from Syncthing (.stignore or the folder's symlink setting), or accept it, given that item 1's fix makes symlinks inert.

3. TOCTOU in ingest

ResolveIngestPath checks the path, then extract.Text reopens it by name. Exploiting that needs write access to raw/ (item 2) plus winning a race, so it's low. The fix is the same as item 1: open inside the root and extract from the handle.

4. Undocumented assumption

If raw/ itself were a symlink to /, confinement would allow everything. On koala today it's a real directory. Assert that at startup (refuse to serve ingest if raw/ is a symlink), or document it.

Created via git-mcp on behalf of @mathias

Found by the PR #152 gatekeeper review, 2026-10-03. Pre-existing; PR #152 confines `/ingest-path` and `brain_ingest` but not the other readers of `raw/`. ## 1. Other raw/ readers follow symlinks - `GET /pending` and MCP `brain_pending` returned the *target* contents of `raw/x.md -> <file outside the brain>` in their excerpt. Verified by counting a marker; the bytes were not printed. - `brain_promote` (`internal/api/promote.go:108`) would copy such a target into `wiki/`, after which brain-sync pushes it to Gitea. **Fix:** treat a symlink anywhere under `raw/` as not-a-note. Lstat in `ListPending`/`PromoteNote`, or reuse `ResolveIngestPath`. Better still, open through `os.OpenRoot(brain/raw)` so reads can't leave the root. That also closes item 3. ## 2. raw/ is writable from outside the API `/var/lib/supervisor/brain` is a Syncthing `sendreceive` folder shared with flamingo. `raw/` is not in `.stignore`, and Syncthing syncs symlinks. uid 1000 on koala and the brain-* CronJobs that mount the folder can also write there. No API route lets a remote caller plant a file of its choosing in raw/, so for now the exposure is limited to whoever controls flamingo or koala. **Decide:** exclude `raw/` symlinks from Syncthing (`.stignore` or the folder's symlink setting), or accept it, given that item 1's fix makes symlinks inert. ## 3. TOCTOU in ingest `ResolveIngestPath` checks the path, then `extract.Text` reopens it by name. Exploiting that needs write access to raw/ (item 2) plus winning a race, so it's low. The fix is the same as item 1: open inside the root and extract from the handle. ## 4. Undocumented assumption If `raw/` itself were a symlink to `/`, confinement would allow everything. On koala today it's a real directory. Assert that at startup (refuse to serve ingest if `raw/` is a symlink), or document it. --- _Created via git-mcp on behalf of @mathias_
mathias added the risk/mediumsize/mtype/fix labels 2026-10-03 14:32:32 +00:00
mathias commented 2026-10-03 14:54:17 +00:00
Author
Owner
Copy Link
Copy Source

Decision (Mathias, 2026-10-03), item 2: accept the Syncthing write path to brain/raw/. No .stignore change and no symlink-sync change.

Reasoning: PR #156 makes symlinks inert for every raw/ reader. pending, promote, reject, /ingest-path, brain_ingest and the watcher all read through an os.Root on raw/ and refuse non-regular files and a symlinked raw/. So a symlink arriving over Syncthing can't lead a read outside raw/. A regular file arriving over Syncthing is legitimate inbox content, which is what the folder is for.

This acceptance depends on #156 merging. If #156 doesn't land, item 2 is open again.

Created via git-mcp on behalf of @mathias

**Decision (Mathias, 2026-10-03), item 2:** accept the Syncthing write path to `brain/raw/`. No `.stignore` change and no symlink-sync change. Reasoning: PR #156 makes symlinks inert for every raw/ reader. pending, promote, reject, `/ingest-path`, `brain_ingest` and the watcher all read through an `os.Root` on raw/ and refuse non-regular files and a symlinked raw/. So a symlink arriving over Syncthing can't lead a read outside raw/. A regular file arriving over Syncthing is legitimate inbox content, which is what the folder is for. This acceptance depends on #156 merging. If #156 doesn't land, item 2 is open again. --- _Created via git-mcp on behalf of @mathias_
mathias commented 2026-10-03 14:56:21 +00:00
Author
Owner
Copy Link
Copy Source

Gatekeeper review of PR #156 (head 8a91feb): BLOCK. Full evidence is on the PR thread (#156).

Summary: under execution the raw/ read control held. Symlinks at every depth, symlinked dirs, a symlinked raw/, .., and a 385k-iteration symlink-swap race all produced sentinel count 0. Blocked because:
(1) the item-3 ingest call sites have no assertion: reverting them to extract.Text stays green;
(2) the watcher symlink test passes vacuously (no positive control);
(3) the item-2 recommendation says symlinks are inert "in every reader", but the whole brain is Syncthing-shared and a wiki/ symlink is followed by brain_search (verified). That needs its own issue before item 2 is decided;
(4) a FIFO swapped in between Lstat and Open hangs TextIn, ListPending and the watcher (local write needed);
(5) pre-existing write escapes through symlinked raw/processed, raw/rejected and *.processed markers.

Gatekeeper review of PR #156 (head 8a91feb): **BLOCK**. Full evidence is on the PR thread (https://git.d-ma.be/mathias/hyperguild/pulls/156). Summary: under execution the raw/ read control held. Symlinks at every depth, symlinked dirs, a symlinked raw/, `..`, and a 385k-iteration symlink-swap race all produced sentinel count 0. Blocked because: (1) the item-3 ingest call sites have no assertion: reverting them to `extract.Text` stays green; (2) the watcher symlink test passes vacuously (no positive control); (3) the item-2 recommendation says symlinks are inert "in every reader", but the whole brain is Syncthing-shared and a `wiki/` symlink is followed by `brain_search` (verified). That needs its own issue before item 2 is decided; (4) a FIFO swapped in between Lstat and Open hangs TextIn, ListPending and the watcher (local write needed); (5) pre-existing write escapes through symlinked `raw/processed`, `raw/rejected` and `*.processed` markers.
mathias commented 2026-10-03 14:57:02 +00:00
Author
Owner
Copy Link
Copy Source

Correction: the premise of the item-2 decision above was false. I told Mathias that once raw/ readers are fixed, the Syncthing write path can't leak outside files. The PR #156 review showed that's only true for raw/. The whole /var/lib/supervisor/brain folder is Syncthing-shared (.stignore lists only sessions, training-data, .git and log.md), so a peer can plant wiki/<wing>/<hall>/leak.md -> <outside file>.

  • search.Query (brain_search) returned the target's contents in the excerpt. Reproduced, sentinel count 1.
  • Read but not run: vectorstore/sync.go, search.hydrate, mcp/tools_context.go readExcerpt, graphsync and frontmatterlint follow such symlinks too.

The "accept" decision is withdrawn pending a re-ask. The wiki/ exposure is tracked separately (issue to follow).

Created via git-mcp on behalf of @mathias

**Correction: the premise of the item-2 decision above was false.** I told Mathias that once raw/ readers are fixed, the Syncthing write path can't leak outside files. The PR #156 review showed that's only true for `raw/`. The whole `/var/lib/supervisor/brain` folder is Syncthing-shared (`.stignore` lists only sessions, training-data, .git and log.md), so a peer can plant `wiki/<wing>/<hall>/leak.md -> <outside file>`. - `search.Query` (brain_search) returned the target's contents in the excerpt. Reproduced, sentinel count 1. - Read but not run: `vectorstore/sync.go`, `search.hydrate`, `mcp/tools_context.go readExcerpt`, graphsync and frontmatterlint follow such symlinks too. The "accept" decision is **withdrawn pending a re-ask**. The wiki/ exposure is tracked separately (issue to follow). --- _Created via git-mcp on behalf of @mathias_
mathias commented 2026-10-03 15:01:42 +00:00
Author
Owner
Copy Link
Copy Source

Gatekeeper round 2 on PR #156 (head 87c80e4): BLOCK. Details are on the PR.

Round-1 findings 1, 2, 3, 6 and 7 are fixed and verified by running them. The FIFO hang is gone under a race of millions of iterations. Still blocking: the FIFO fix at its three call sites (readRawNote, TextIn, copyFromRoot) has no test. Reverting all three to a plain Open leaves go test ./... green, and that revert brings the hang back. Non-blocking: the extract.Text( call-site lint can be bypassed with an aliased import or a function value (rule 9).

Gatekeeper round 2 on PR #156 (head 87c80e4): **BLOCK**. Details are on the PR. Round-1 findings 1, 2, 3, 6 and 7 are fixed and verified by running them. The FIFO hang is gone under a race of millions of iterations. Still blocking: the FIFO fix at its three call sites (readRawNote, TextIn, copyFromRoot) has no test. Reverting all three to a plain `Open` leaves `go test ./...` green, and that revert brings the hang back. Non-blocking: the `extract.Text(` call-site lint can be bypassed with an aliased import or a function value (rule 9).
mathias commented 2026-10-03 15:05:05 +00:00
Author
Owner
Copy Link
Copy Source

Gatekeeper round 3 on PR #156 (head 8094200): BLOCK. Details are on the PR.

The round-2 findings are fixed. New: the parser lint forbids Open/OpenFile but not ReadFile. Changing one line in readRawNote to os.ReadFile(filepath.Join(r.Name(), base)) leaves go test ./... green and brings the TOCTOU escape back (19,016 sentinel leaks into ListPending under the swap race). Changing it to r.ReadFile(base) brings the FIFO hang back.

Gatekeeper round 3 on PR #156 (head 8094200): **BLOCK**. Details are on the PR. The round-2 findings are fixed. New: the parser lint forbids `Open`/`OpenFile` but not `ReadFile`. Changing one line in `readRawNote` to `os.ReadFile(filepath.Join(r.Name(), base))` leaves `go test ./...` green and brings the TOCTOU escape back (19,016 sentinel leaks into ListPending under the swap race). Changing it to `r.ReadFile(base)` brings the FIFO hang back.
mathias commented 2026-10-03 15:20:27 +00:00
Author
Owner
Copy Link
Copy Source

Gatekeeper round 4 on PR #156 (head d5b2be0): BLOCK, one item left. Details are on the PR.

The race tests caught 10/10 of my flips, still caught them under CPU starvation, and showed no flakiness at -count=20 or under -race. Remaining gap: the temp copy for pdf/docmark formats has no test. Replacing Text(tmp.Name()) with a read by name keeps the suite green, and a symlink-swap race on r.docx then leaked 33,988 times to a stub docmark server (0 at head).

Gatekeeper round 4 on PR #156 (head d5b2be0): **BLOCK**, one item left. Details are on the PR. The race tests caught 10/10 of my flips, still caught them under CPU starvation, and showed no flakiness at -count=20 or under -race. Remaining gap: the temp copy for pdf/docmark formats has no test. Replacing `Text(tmp.Name())` with a read by name keeps the suite green, and a symlink-swap race on `r.docx` then leaked 33,988 times to a stub docmark server (0 at head).
mathias commented 2026-10-03 15:30:30 +00:00
Author
Owner
Copy Link
Copy Source

Gatekeeper round 5 on PR #156 (head 8e88dde): PASS. Evidence is on the PR.

Every flip in the table goes red (the race tests caught 10/10 or 5/5 of the flips). No flakiness under -count=20, -race, or CPU starvation. CI run 8900 is green. Not merged by the gatekeeper: merging deploys, and the approval to merge reached me only through another agent. Follow-ups #157 and #158 remain open.

Gatekeeper round 5 on PR #156 (head 8e88dde): **PASS**. Evidence is on the PR. Every flip in the table goes red (the race tests caught 10/10 or 5/5 of the flips). No flakiness under -count=20, -race, or CPU starvation. CI run 8900 is green. Not merged by the gatekeeper: merging deploys, and the approval to merge reached me only through another agent. Follow-ups #157 and #158 remain open.
mathias commented 2026-10-03 15:32:34 +00:00
Author
Owner
Copy Link
Copy Source

Shipped and verified live. PR #156 squash-merged as 687afa0 after 5 adversarial review rounds (Mathias pre-approved merging on PASS). CD rolled out ingestion:687afa0; /pending with the token returns 200 and there are no startup errors.

By item:

  1. Readers follow symlinks: closed. pending, promote, reject, /ingest-path, brain_ingest and the watcher read through an os.Root on raw/ and refuse non-regular files. The watcher was a reader the issue didn't list. Behavioural race tests fail on any leaked byte or hang: symlink swap, FIFO swap, and docx through a fake docmark.
  2. TOCTOU: closed. Reads go through the root and open non-blocking, with a type check on the handle.
  3. raw/ itself a symlink: closed. Refused by every reader.
  4. Syncthing write path, scoped to raw/: the premise "symlinks can't leak" now holds for raw/, because every raw/ reader treats them as inert. The earlier withdrawal stands for the brain as a whole, which is #157 (tripwire live; brain-wide readers pending).

Write escapes under raw/ are #158. Closing.

Created via git-mcp on behalf of @mathias

**Shipped and verified live.** PR #156 squash-merged as `687afa0` after 5 adversarial review rounds (Mathias pre-approved merging on PASS). CD rolled out `ingestion:687afa0`; `/pending` with the token returns 200 and there are no startup errors. By item: 1. **Readers follow symlinks:** closed. pending, promote, reject, `/ingest-path`, `brain_ingest` and the watcher read through an `os.Root` on raw/ and refuse non-regular files. The watcher was a reader the issue didn't list. Behavioural race tests fail on any leaked byte or hang: symlink swap, FIFO swap, and docx through a fake docmark. 3. **TOCTOU:** closed. Reads go through the root and open non-blocking, with a type check on the handle. 4. **raw/ itself a symlink:** closed. Refused by every reader. 2. **Syncthing write path, scoped to raw/:** the premise "symlinks can't leak" now holds for raw/, because every raw/ reader treats them as inert. The earlier withdrawal stands for the brain as a whole, which is #157 (tripwire live; brain-wide readers pending). Write escapes under raw/ are #158. Closing. --- _Created via git-mcp on behalf of @mathias_
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agent/ready
Meets the agent-ready contract; authorizes unattended execution
 blocked/client
Waiting on the client
 blocked/supplier
Waiting on a supplier
 blocked/upstream
Waiting on an upstream project or vendor
 risk/high
Irreversible, security-relevant, or control plane
 risk/low
Reversible, local, test-covered
 risk/medium
Touches shared state, deploys or data
 size/l
Too big - split it
 size/m
Under two days
 size/s
Under half a day
 size/xs
Under an hour
 status:dispatch
status:dispatched
status:review
status:reviewing
type/chore
Maintenance, no behaviour change
 type/decision
Human decision required; never agent/ready
 type/docs
Documentation only
 type/feat
New capability
 type/fix
Corrects broken behaviour
 type/ops
Infrastructure, deploy, runtime
 type/research
Investigate; output is a finding, not code
 type/test
Test coverage only
No labels risk/medium size/m type/fix
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate-bot mathias
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mathias/hyperguild#153