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

brain_write follows a symlinked wing directory, including one that points outside the brain #188

New Issue
Closed
opened 2026-10-05 19:06:00 +00:00 by mathias · 3 comments
mathias commented 2026-10-05 19:06:00 +00:00
Owner
Copy Link
Copy Source

Source: PR #187 review (N4), 2026-10-05. Verified by execution through the MCP brain_write handler. This predates #187 and is outside #173's scope; it is a write path, like #158.

The new-note write path in internal/api/handler.go (around lines 230–233) does os.MkdirAll + os.WriteFile by path:

  • If a wing directory under wiki/ is a symlink to another place in the brain, the note lands there.
  • If it points outside the brain, the note is written outside the brain.

Bounds: it needs a planted directory symlink. Syncthing syncs symlinks, so a peer could plant one. The symlinkguard tripwire removes symlinks within 30 s, so the window is short but real.

Fix

  • Create the note through the brain root, never through a symlink:
    • realDir on the parent chain
    • os.Root.MkdirAll-style creation
    • an O_CREATE|O_EXCL|O_NOFOLLOW open through the root, with a SameFile check
  • Add a brainfs CreateRegular beside WriteRegular (which never creates).
  • Then audit the other creating writes: wiki.RebuildIndex, brain.BuildWingIndex's _index.md, pipeline.buildAndWrite, wiki.AppendLog, and logFuzzyCandidates.

Done when: a test where wiki/<wing> is a symlink (in-brain target and outside target) has brain_write refuse with nothing created at the target, and the test is red before the fix.

Created via git-mcp on behalf of @mathias

**Source:** PR #187 review (N4), 2026-10-05. Verified by execution through the MCP `brain_write` handler. This predates #187 and is outside #173's scope; it is a write path, like #158. The new-note write path in `internal/api/handler.go` (around lines 230–233) does `os.MkdirAll` + `os.WriteFile` by path: - If a wing directory under `wiki/` is a symlink to another place in the brain, the note lands there. - If it points outside the brain, the note is written outside the brain. **Bounds:** it needs a planted directory symlink. Syncthing syncs symlinks, so a peer could plant one. The symlinkguard tripwire removes symlinks within 30 s, so the window is short but real. ## Fix - Create the note through the brain root, never through a symlink: - `realDir` on the parent chain - `os.Root.MkdirAll`-style creation - an O_CREATE|O_EXCL|O_NOFOLLOW open through the root, with a SameFile check - Add a brainfs `CreateRegular` beside `WriteRegular` (which never creates). - Then audit the other creating writes: `wiki.RebuildIndex`, `brain.BuildWingIndex`'s `_index.md`, `pipeline.buildAndWrite`, `wiki.AppendLog`, and `logFuzzyCandidates`. **Done when:** a test where `wiki/<wing>` is a symlink (in-brain target and outside target) has brain_write refuse with nothing created at the target, and the test is red before the fix. --- _Created via git-mcp on behalf of @mathias_
mathias added the risk/high label 2026-10-05 19:06:00 +00:00
mathias commented 2026-10-05 20:54:03 +00:00
Author
Owner
Copy Link
Copy Source

PR #189 review (2026-10-05): BLOCKED. Full evidence: #189 (comment)

  • Done-when is met. TestWriteNoteRefusesASymlinkedWing covers in-brain and outside targets, and it goes red when writeHallNote is reverted to path I/O.
  • Every converted entry point refused every planted shape when run end to end: nothing was created or changed at a link target, and nothing hung.
  • Blocked on three tests that cannot go red:
    • TestEditRefusesANoteBehindASymlinkedDirectory refuses only because the temp brain has no .git.
    • The AppendRegular unclean-path case is masked by a symlinked final file, so cleanRel there has no test.
    • Brain.Remove's real-parent check has no test.
  • Pre-existing, needs its own issue: brain_edit's FIFO hang in the snapshot read, the by-path rollback write, and the by-path lock file (internal/mcp/tools_notes.go, internal/notes/edit.go). None of these is in #158 part B's list.
**PR #189 review (2026-10-05): BLOCKED.** Full evidence: https://git.d-ma.be/mathias/hyperguild/pulls/189#issuecomment-15753 - **Done-when is met.** `TestWriteNoteRefusesASymlinkedWing` covers in-brain and outside targets, and it goes red when writeHallNote is reverted to path I/O. - **Every converted entry point refused every planted shape** when run end to end: nothing was created or changed at a link target, and nothing hung. - **Blocked on three tests that cannot go red:** - `TestEditRefusesANoteBehindASymlinkedDirectory` refuses only because the temp brain has no `.git`. - The `AppendRegular` unclean-path case is masked by a symlinked final file, so `cleanRel` there has no test. - `Brain.Remove`'s real-parent check has no test. - **Pre-existing, needs its own issue:** brain_edit's FIFO hang in the snapshot read, the by-path rollback write, and the by-path lock file (`internal/mcp/tools_notes.go`, `internal/notes/edit.go`). None of these is in #158 part B's list.
mathias commented 2026-10-05 21:01:59 +00:00
Author
Owner
Copy Link
Copy Source

PR #189 round 2 (db18785): BLOCKED. Full review: see the latest review comment on PR #189.

  • Round-1 blockers: B1–B3 are fixed, each confirmed by a flip.
  • N1, new: CreateRegular (now promote/reject's only write path) has no test. If its real-directory parent check is replaced with root.MkdirAll, the suite stays green, while promote into an in-brain symlinked wing writes at the link target and deletes the raw source. Verified through MCP.
  • N2: CI run 9497 is red because of the #193 symlinkguard flake (unrelated package). It needs a green re-run.
**PR #189 round 2 (db18785): BLOCKED.** Full review: see the latest review comment on PR #189. - **Round-1 blockers:** B1–B3 are fixed, each confirmed by a flip. - **N1, new:** `CreateRegular` (now promote/reject's only write path) has no test. If its real-directory parent check is replaced with `root.MkdirAll`, the suite stays green, while promote into an in-brain symlinked wing writes at the link target and deletes the raw source. Verified through MCP. - **N2:** CI run 9497 is red because of the #193 symlinkguard flake (unrelated package). It needs a green re-run.
mathias commented 2026-10-05 21:23:35 +00:00
Author
Owner
Copy Link
Copy Source

PR #189 round 3 (ac84ddf): PASS. Full review: see the latest review comment on PR #189.

  • Round-2 findings: both are fixed. The CreateRegular and in-brain promote/reject tests go red under each flip.
  • CI: run 9506 passed.
  • Merge: awaiting Mathias, because merging deploys ingestion.
  • Residual, race-only: if promote/reject call PutRegular instead of CreateRegular, no test notices. The collision check covers the static case.
**PR #189 round 3 (ac84ddf): PASS.** Full review: see the latest review comment on PR #189. - **Round-2 findings:** both are fixed. The `CreateRegular` and in-brain promote/reject tests go red under each flip. - **CI:** run 9506 passed. - **Merge:** awaiting Mathias, because merging deploys ingestion. - **Residual, race-only:** if promote/reject call `PutRegular` instead of `CreateRegular`, no test notices. The collision check covers the static case.
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/high
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#188