Compare commits

..
2 Commits
Author SHA1 Message Date
mathiasandClaude Opus 5.5 87fa28e2c8 fix(update,session): brain_update and session_log confine caller paths (#169)
CI / Lint / Test / Vet (pull_request) Successful in 45s
CI / Mirror to GitHub (pull_request) Skipped
Review round 1 on PR #170 found two more handlers with the #169 class of
bug (verified by execution):

- brain_update resolved its caller path with resolveWithin, a lexical
  containment check: any brain file could be superseded
  (classification.yaml, raw/, sessions/), and a final symlink was
  written through. UpdateNote now resolves like brain_edit
  (notes.Resolve, ScopeNotes: knowledge/ and wiki/ .md notes), and reads
  and rewrites through brainfs (ReadRegular, WriteRegular).
- session_log joined the caller's session_id onto the sessions dir and
  opened it O_CREATE|O_APPEND: an id with path segments created or
  appended a .jsonl file anywhere the pod could write. The id must now
  be a plain name (^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$, which every live
  session file matches), and the open adds O_NOFOLLOW|O_NONBLOCK. Read
  checks the id too.
- Tunnel tests: the .txt case now names an existing file (it failed on
  "missing" before), the symlink test adds an in-brain target, and the
  FIFO comment no longer claims the handle check is the only guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 22:38:08 +02:00
mathiasandClaude Opus 5.5 9eb3d2456d fix(tunnel): brain_tunnel cannot write outside the wiki or through symlinks
CI / Lint / Test / Vet (pull_request) Successful in 52s
CI / Mirror to GitHub (pull_request) Skipped
brain_tunnel passes caller-supplied source and target paths to
WriteTunnel, which checked only the first segments ("wiki/<wing>/...")
and joined them onto the brain dir. "wiki/b/x/../../../../tmp/f.md"
resolved outside the brain, and appendSeeAlso read and rewrote that file
(verified by execution): any MCP token holder could append to any
existing file the pod user can write, inside the brain (raw/, sessions/,
classification.yaml) or outside it.

- checkNotePath: only a clean, local wiki/.../<slug>.md path, with no
  ".", ".." or empty segments.
- Both notes are read and rewritten through brainfs. New
  brainfs.WriteRegular: never creates; Lstat through the root, open with
  O_NOFOLLOW|O_NONBLOCK, and require the opened handle to be the same
  regular file (os.SameFile). os.Root.OpenFile follows an in-root final
  symlink even with O_NOFOLLOW (verified on go1.27), so the flag alone
  does not refuse it.
- The source is no longer rewritten when the target is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 22:23:45 +02:00
14 changed files with 514 additions and 409 deletions
+18 -6
View File
@@ -3,15 +3,18 @@ package api
import ( import (
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"errors"
"fmt" "fmt"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain" "github.com/mathiasbq/hyperguild/ingestion/internal/brain"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"github.com/mathiasbq/hyperguild/ingestion/internal/claim" "github.com/mathiasbq/hyperguild/ingestion/internal/claim"
"github.com/mathiasbq/hyperguild/ingestion/internal/notes"
) )
// ContentHash returns the lowercase hex sha256 of b. It is the note's // ContentHash returns the lowercase hex sha256 of b. It is the note's
@@ -91,14 +94,23 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
rel = filepath.ToSlash(rel) rel = filepath.ToSlash(rel)
} }
rel, abs, err := resolveWithin(brainDir, rel) // The path is caller-supplied (brain_update): resolve it as brain_edit
// does, knowledge/ and wiki/ notes only, and read and rewrite it through
// the brain root, never through a symlink (hyperguild#169).
t, err := notes.Resolve(brainDir, rel, notes.ScopeNotes)
if err != nil { if err != nil {
return "", "", "", err return "", "", "", err
} }
rel = t.Rel
prior, err := os.ReadFile(abs) bfs, err := brainfs.Open(brainDir)
if err != nil { if err != nil {
if os.IsNotExist(err) { return "", "", "", fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
prior, err := bfs.ReadRegular(rel)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return "", "", "", fmt.Errorf("note %q does not exist: use brain_write to create", rel) return "", "", "", fmt.Errorf("note %q does not exist: use brain_write to create", rel)
} }
return "", "", "", fmt.Errorf("read target: %w", err) return "", "", "", fmt.Errorf("read target: %w", err)
@@ -164,7 +176,7 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
return "", "", "", err return "", "", "", err
} }
out := []byte(stamped) out := []byte(stamped)
if err := os.WriteFile(abs, out, 0o644); err != nil { if err := bfs.WriteRegular(rel, out); err != nil {
return "", "", "", fmt.Errorf("write: %w", err) return "", "", "", fmt.Errorf("write: %w", err)
} }
return rel, ContentHash(out), priorHash, nil return rel, ContentHash(out), priorHash, nil
+112
View File
@@ -0,0 +1,112 @@
package api_test
import (
"os"
"path/filepath"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
)
// brain_update passed the caller's path through resolveWithin, a lexical
// containment check only: any file in the brain could be superseded
// (classification.yaml, raw/, sessions/), and a final symlink was written
// through. It now resolves like brain_edit: knowledge/ and wiki/ notes only,
// read and rewritten through the brain root.
func TestUpdateNoteRefusesPathsOutsideKnowledgeAndWikiNotes(t *testing.T) {
brain := t.TempDir()
files := map[string]string{
"classification.yaml": "levels: {}\n",
"raw/r.md": "raw\n",
"sessions/s.jsonl": "{}\n",
"wiki/w/facts/ok.md": "---\ntitle: ok\n---\nold\n",
"knowledge/k.md": "---\ntitle: k\n---\nold\n",
"wiki/w/facts/x.txt": "txt\n",
}
for rel, body := range files {
p := filepath.Join(brain, filepath.FromSlash(rel))
require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755))
require.NoError(t, os.WriteFile(p, []byte(body), 0o644))
}
for _, rel := range []string{"classification.yaml", "raw/r.md", "sessions/s.jsonl", "wiki/w/facts/x.txt", "wiki/w/../../raw/r.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "replaced"})
assert.Error(t, err, rel)
}
for rel, body := range files {
if rel == "wiki/w/facts/ok.md" || rel == "knowledge/k.md" {
continue
}
got, err := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
require.NoError(t, err)
assert.Equal(t, body, string(got), rel)
}
for _, rel := range []string{"wiki/w/facts/ok.md", "knowledge/k.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "new body"})
require.NoError(t, err, "positive control: %s", rel)
got, _ := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
assert.Contains(t, string(got), "new body")
}
}
func TestUpdateNoteDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), filepath.Join(facts, "l.md")))
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/l.md", Content: "replaced"})
assert.Error(t, err)
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
// The note is swapped between a regular file and an in-brain symlink to a
// raw/ file while UpdateNote runs: the read and the write each refuse a
// symlink, so the raw/ file is never rewritten.
func TestUpdateNoteNeverRewritesASymlinkSwappedInForTheNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
note := filepath.Join(facts, "n.md")
body := "---\ntitle: n\n---\nold\n"
require.NoError(t, os.WriteFile(note, []byte(body), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := note + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte(body), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, note)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if _, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/n.md", Content: "new"}); err == nil {
ok++
}
}
stop.Store(true)
wg.Wait()
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
assert.Positive(t, ok, "positive control: some updates reach the regular note")
}
+54 -20
View File
@@ -4,9 +4,12 @@ import (
"bufio" "bufio"
"fmt" "fmt"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// seeAlsoHeader is the markdown heading used to group cross-wing links. // seeAlsoHeader is the markdown heading used to group cross-wing links.
@@ -197,6 +200,12 @@ func fileExists(p string) bool {
// section headers. Rejects same-wing pairs (a tunnel is by definition // section headers. Rejects same-wing pairs (a tunnel is by definition
// cross-wing) and missing notes. // cross-wing) and missing notes.
func WriteTunnel(brainDir, sourcePath, targetPath string) error { func WriteTunnel(brainDir, sourcePath, targetPath string) error {
if err := checkNotePath(sourcePath); err != nil {
return fmt.Errorf("source: %w", err)
}
if err := checkNotePath(targetPath); err != nil {
return fmt.Errorf("target: %w", err)
}
srcWing, err := wingOf(sourcePath) srcWing, err := wingOf(sourcePath)
if err != nil { if err != nil {
return fmt.Errorf("source: %w", err) return fmt.Errorf("source: %w", err)
@@ -209,20 +218,50 @@ func WriteTunnel(brainDir, sourcePath, targetPath string) error {
return fmt.Errorf("tunnel must cross wings; got both in %q", srcWing) return fmt.Errorf("tunnel must cross wings; got both in %q", srcWing)
} }
srcFull := filepath.Join(brainDir, filepath.FromSlash(sourcePath)) // Both notes are read and rewritten through the brain root, regular
tgtFull := filepath.Join(brainDir, filepath.FromSlash(targetPath)) // files only: brain_tunnel passes caller-supplied paths, and the brain is
if _, err := os.Stat(srcFull); err != nil { // a Syncthing folder that can hold symlinks.
bfs, err := brainfs.Open(brainDir)
if err != nil {
return fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
srcContent, err := bfs.ReadRegular(sourcePath)
if err != nil {
return fmt.Errorf("source note: %w", err) return fmt.Errorf("source note: %w", err)
} }
if _, err := os.Stat(tgtFull); err != nil { tgtContent, err := bfs.ReadRegular(targetPath)
if err != nil {
return fmt.Errorf("target note: %w", err) return fmt.Errorf("target note: %w", err)
} }
if err := appendSeeAlso(srcFull, wikilinkOf(targetPath)); err != nil { if out, changed, err := withSeeAlso(srcContent, wikilinkOf(targetPath)); err != nil {
return fmt.Errorf("update source: %w", err)
} else if changed {
if err := bfs.WriteRegular(sourcePath, out); err != nil {
return fmt.Errorf("update source: %w", err) return fmt.Errorf("update source: %w", err)
} }
if err := appendSeeAlso(tgtFull, wikilinkOf(sourcePath)); err != nil { }
if out, changed, err := withSeeAlso(tgtContent, wikilinkOf(sourcePath)); err != nil {
return fmt.Errorf("update target: %w", err) return fmt.Errorf("update target: %w", err)
} else if changed {
if err := bfs.WriteRegular(targetPath, out); err != nil {
return fmt.Errorf("update target: %w", err)
}
}
return nil
}
// checkNotePath accepts only a clean, local wiki/<wing>/<hall>/.../<slug>.md
// path: no "." or ".." segments, no empty segments, no leading slash.
func checkNotePath(p string) error {
if p == "" || path.Clean(p) != p || !filepath.IsLocal(p) || !strings.HasSuffix(p, ".md") {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
for _, part := range strings.Split(p, "/") {
if part == "" || part == "." || part == ".." {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
} }
return nil return nil
} }
@@ -248,16 +287,13 @@ func wingOf(relPath string) (string, error) {
return parts[1], nil return parts[1], nil
} }
// appendSeeAlso inserts `- [[link]]` under the file's See also section, // withSeeAlso returns content with `- [[link]]` under its See also section,
// creating the section if absent. No-op when the link is already present. // creating the section if absent. changed is false when the link is already
func appendSeeAlso(filePath, link string) error { // present.
content, err := os.ReadFile(filePath) func withSeeAlso(content []byte, link string) (out []byte, changed bool, err error) {
if err != nil {
return err
}
wikilink := "[[" + link + "]]" wikilink := "[[" + link + "]]"
if strings.Contains(string(content), wikilink) { if strings.Contains(string(content), wikilink) {
return nil return content, false, nil
} }
bullet := "- " + wikilink bullet := "- " + wikilink
@@ -266,8 +302,7 @@ func appendSeeAlso(filePath, link string) error {
// No section yet — append a fresh one. Always emit a trailing // No section yet — append a fresh one. Always emit a trailing
// newline so subsequent appends don't merge into the previous line. // newline so subsequent appends don't merge into the previous line.
trimmed := strings.TrimRight(string(content), "\n") trimmed := strings.TrimRight(string(content), "\n")
out := trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n" return []byte(trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n"), true, nil
return os.WriteFile(filePath, []byte(out), 0o644)
} }
// Section exists — splice the bullet in just before the next `## ` // Section exists — splice the bullet in just before the next `## `
@@ -293,12 +328,11 @@ func appendSeeAlso(filePath, link string) error {
b.WriteByte('\n') b.WriteByte('\n')
} }
if err := scanner.Err(); err != nil { if err := scanner.Err(); err != nil {
return err return nil, false, err
} }
if !inserted { if !inserted {
// section was the last thing in the file — just append bullet // section was the last thing in the file — just append bullet
out := strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n" return []byte(strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n"), true, nil
return os.WriteFile(filePath, []byte(out), 0o644)
} }
return os.WriteFile(filePath, []byte(b.String()), 0o644) return []byte(b.String()), true, nil
} }
@@ -0,0 +1,141 @@
package brain_test
import (
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
)
// brain_tunnel passes caller-supplied source and target paths to WriteTunnel,
// which checked only "wiki/<wing>/..." and then joined them onto the brain
// dir: ".." segments resolved outside the brain and appendSeeAlso rewrote the
// file there.
func TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("---\ntitle: S\n---\nbody\n"), 0o644))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
up := strings.Repeat("../", strings.Count(filepath.Clean(brain0), "/")+3)
target := "wiki/b/x/" + up + strings.TrimPrefix(victim, "/")
err := brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target)
assert.Error(t, err)
got, rerr := os.ReadFile(victim)
require.NoError(t, rerr)
assert.Equal(t, "untouched\n", string(got), "a file outside the brain was modified")
}
func TestWriteTunnelRefusesUncleanOrNonWikiPaths(t *testing.T) {
brain0 := t.TempDir()
for _, p := range []string{"wiki/a/facts/s.md", "wiki/b/facts/t.md", "wiki/b/facts/t.txt", "raw/r.md"} {
require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(brain0, p)), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain0, p), []byte("x\n"), 0o644))
}
for _, target := range []string{
"wiki/b/facts/../../../raw/r.md",
"wiki/b/./facts/t.md",
"wiki/b//facts/t.md",
"/wiki/b/facts/t.md",
"wiki/b/facts/t.txt",
"wiki/b/facts/",
} {
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target), target)
}
for _, p := range []string{"raw/r.md", "wiki/b/facts/t.txt"} {
got, err := os.ReadFile(filepath.Join(brain0, p))
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), p)
}
require.NoError(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"), "positive control")
got, err := os.ReadFile(filepath.Join(brain0, "wiki", "b", "facts", "t.md"))
require.NoError(t, err)
assert.Contains(t, string(got), "[[a/facts/s]]")
}
func TestWriteTunnelDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(victim, filepath.Join(tdir, "t.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"))
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
// The same with a link to a file elsewhere in the brain, which os.Root
// would follow.
inVictim := filepath.Join(brain0, "raw", "v.md")
require.NoError(t, os.MkdirAll(filepath.Dir(inVictim), 0o755))
require.NoError(t, os.WriteFile(inVictim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "v.md"), filepath.Join(tdir, "u.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/u.md"))
got, err = os.ReadFile(inVictim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
got, err = os.ReadFile(src)
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), "the source is not touched when the target is refused")
}
// The target is swapped between a regular note and a symlink to an in-brain
// file outside wiki/ while WriteTunnel runs: the read and the write each
// refuse a symlink, so the other file is never rewritten.
func TestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(brain0, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
tgt := filepath.Join(tdir, "t.md")
require.NoError(t, os.WriteFile(tgt, []byte("t\n"), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := tgt + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte("t\n"), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, tgt)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md") == nil {
ok++
}
_ = os.WriteFile(src, []byte("x\n"), 0o644)
}
stop.Store(true)
wg.Wait()
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got), "the swapped-in symlink's target was rewritten")
assert.Positive(t, ok, "positive control: some tunnels were written to the regular target")
}
+36 -27
View File
@@ -82,26 +82,42 @@ func (b *Brain) realDir(rel string) error {
return nil return nil
} }
// ReadDirRegular lists the regular files directly in a brain-relative // WriteRegular replaces the contents of an existing regular file at a
// directory, by name. Every component of relDir must be a real directory // brain-relative path. It never creates a file and never writes through a
// (ErrNotRegular otherwise); symlinks, FIFOs, devices and subdirectories in // final symlink: os.Root.OpenFile follows an in-root final symlink even with
// it are left out. // O_NOFOLLOW (verified on go1.27), so the path is Lstat-ed through the root
func (b *Brain) ReadDirRegular(relDir string) ([]string, error) { // first and the opened handle must be that same file (os.SameFile), which
relDir = filepath.ToSlash(relDir) // also catches a swap between the two. O_NONBLOCK keeps a FIFO from blocking
if err := b.realDir(relDir); err != nil { // the open. Intermediate in-brain symlinks are still followed (os.Root keeps
return nil, err // the write inside the brain; hyperguild#167).
} func (b *Brain) WriteRegular(rel string, data []byte) error {
ents, err := fs.ReadDir(b.root.FS(), relDir) rel = filepath.ToSlash(rel)
lfi, err := b.root.Lstat(rel)
if err != nil { if err != nil {
return nil, err return err
} }
var names []string if !lfi.Mode().IsRegular() {
for _, e := range ents { return fmt.Errorf("%s: %w", rel, ErrNotRegular)
if e.Type().IsRegular() {
names = append(names, e.Name())
} }
f, err := b.root.OpenFile(rel, os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
if err != nil {
return err
} }
return names, nil defer func() { _ = f.Close() }()
fi, err := f.Stat()
if err != nil {
return err
}
if !fi.Mode().IsRegular() || !os.SameFile(lfi, fi) {
return fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
}
if err := f.Truncate(0); err != nil {
return err
}
if _, err := f.Write(data); err != nil {
return err
}
return f.Close()
} }
// ReadRegular reads a brain-relative path, refusing anything that is not a // ReadRegular reads a brain-relative path, refusing anything that is not a
@@ -114,7 +130,7 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) {
if !fi.Mode().IsRegular() { if !fi.Mode().IsRegular() {
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
} }
return b.readOpenedMax(rel, -1, fi) return b.readOpened(rel)
} }
// readOpened opens rel non-blocking and reads it only if the OPENED handle is // readOpened opens rel non-blocking and reads it only if the OPENED handle is
@@ -128,7 +144,7 @@ func (b *Brain) ReadRegular(rel string) ([]byte, error) {
// cannot create one; fs.protected_hardlinks=1 and the pod's secrets are on a // cannot create one; fs.protected_hardlinks=1 and the pod's secrets are on a
// different filesystem from the brain volume, so a local caller cannot link // different filesystem from the brain volume, so a local caller cannot link
// them in. Refusing Nlink > 1 is the next step if either changes. // them in. Refusing Nlink > 1 is the next step if either changes.
func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1, nil) } func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(rel, -1) }
// ReadRegularMax is ReadRegular with a size limit, checked on the opened // ReadRegularMax is ReadRegular with a size limit, checked on the opened
// handle before reading (max < 0 means no limit). // handle before reading (max < 0 means no limit).
@@ -140,14 +156,10 @@ func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) {
if !fi.Mode().IsRegular() { if !fi.Mode().IsRegular() {
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
} }
return b.readOpenedMax(rel, max, fi) return b.readOpenedMax(rel, max)
} }
// readOpenedMax opens rel and reads it if the opened handle is a regular file func (b *Brain) readOpenedMax(rel string, max int64) ([]byte, error) {
// and, when lfi is set, the same file the caller Lstat-ed: os.Root follows an
// in-root final symlink even with O_NOFOLLOW, so a symlink to another brain
// file swapped in between the Lstat and the open is refused only here.
func (b *Brain) readOpenedMax(rel string, max int64, lfi fs.FileInfo) ([]byte, error) {
f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0) f, err := b.root.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -160,9 +172,6 @@ func (b *Brain) readOpenedMax(rel string, max int64, lfi fs.FileInfo) ([]byte, e
if !hfi.Mode().IsRegular() { if !hfi.Mode().IsRegular() {
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular) return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
} }
if lfi != nil && !os.SameFile(lfi, hfi) {
return nil, fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
}
if max >= 0 && hfi.Size() > max { if max >= 0 && hfi.Size() > max {
return nil, fmt.Errorf("%s is %d bytes (limit %d): %w", rel, hfi.Size(), max, ErrTooLarge) return nil, fmt.Errorf("%s is %d bytes (limit %d): %w", rel, hfi.Size(), max, ErrTooLarge)
} }
+36 -63
View File
@@ -6,8 +6,6 @@ import (
"io/fs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"sync"
"sync/atomic"
"syscall" "syscall"
"testing" "testing"
"time" "time"
@@ -192,73 +190,48 @@ func TestWalkRegularReportsASymlinkedIntermediateComponentOfTheWalkRoot(t *testi
assert.ErrorIs(t, errs[0], ErrNotRegular) assert.ErrorIs(t, errs[0], ErrNotRegular)
} }
func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *testing.T) { func TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(dir, "a", "b", "sub"), 0o755)) outside := filepath.Join(t.TempDir(), "o.md")
require.NoError(t, os.WriteFile(filepath.Join(dir, "a", "b", "n.md"), []byte("n"), 0o644)) require.NoError(t, os.WriteFile(outside, []byte("untouched"), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(dir, "other.md"), []byte("o"), 0o644)) require.NoError(t, os.Symlink(outside, filepath.Join(dir, "l.md")))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "other.md"), filepath.Join(dir, "a", "b", "l.md"))) require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "f.md"), 0o644))
require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "a", "b", "f.md"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join(dir, "n.md"), []byte("old content"), 0o644))
require.NoError(t, os.Symlink("a", filepath.Join(dir, "alias")))
require.NoError(t, os.MkdirAll(filepath.Join(dir, "x"), 0o755))
require.NoError(t, os.Symlink(filepath.Join("..", "a"), filepath.Join(dir, "x", "mid")))
b, err := Open(dir) b, err := Open(dir)
require.NoError(t, err) require.NoError(t, err)
defer func() { _ = b.Close() }() defer func() { _ = b.Close() }()
names, err := b.ReadDirRegular("a/b") assert.Error(t, b.WriteRegular("l.md", []byte("x")))
require.NoError(t, err) got, _ := os.ReadFile(outside)
assert.Equal(t, []string{"n.md"}, names) assert.Equal(t, "untouched", string(got))
for _, rel := range []string{"alias/b", "x/mid/b"} {
_, err := b.ReadDirRegular(rel)
assert.ErrorIs(t, err, ErrNotRegular, rel)
}
}
// A final component swapped for a symlink to another regular file in the // A symlink to another file inside the brain: os.Root would allow it,
// brain between the Lstat and the open: os.Root follows in-root links, so // only O_NOFOLLOW refuses it.
// only comparing the two (os.SameFile) refuses it. require.NoError(t, os.WriteFile(filepath.Join(dir, "in.md"), []byte("inside"), 0o644))
func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.T) { require.NoError(t, os.Symlink("in.md", filepath.Join(dir, "il.md")))
dir := t.TempDir() assert.Error(t, b.WriteRegular("il.md", []byte("x")))
require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644)) got, _ = os.ReadFile(filepath.Join(dir, "in.md"))
p := filepath.Join(dir, "r.md") assert.Equal(t, "inside", string(got))
require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644))
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
var stop atomic.Bool // A reader held open, so a non-blocking write open would succeed. The
var wg sync.WaitGroup // Lstat type check refuses the FIFO first; the handle check and
wg.Add(1) // O_NONBLOCK matter only if it is swapped in after the Lstat.
go func() { r, err := os.OpenFile(filepath.Join(dir, "f.md"), os.O_RDONLY|syscall.O_NONBLOCK, 0)
defer wg.Done() require.NoError(t, err)
tmp := p + ".swap" defer func() { _ = r.Close() }()
for i := 0; !stop.Load(); i++ { done := make(chan error, 1)
_ = os.Remove(tmp) go func() { done <- b.WriteRegular("f.md", []byte("x")) }()
if i%2 == 0 { select {
_ = os.WriteFile(tmp, []byte("inside"), 0o644) case err := <-done:
} else { assert.Error(t, err)
_ = os.Symlink("s.md", tmp) case <-time.After(3 * time.Second):
t.Fatal("WriteRegular hung on a FIFO")
} }
_ = os.Rename(tmp, p)
} assert.Error(t, b.WriteRegular("../"+filepath.Base(filepath.Dir(outside))+"/o.md", []byte("x")))
}() assert.Error(t, b.WriteRegular("missing.md", []byte("x")), "does not create files")
leaks, ok := 0, 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); { require.NoError(t, b.WriteRegular("n.md", []byte("new")))
got, err := b.ReadRegular("r.md") got, _ = os.ReadFile(filepath.Join(dir, "n.md"))
if err == nil { assert.Equal(t, "new", string(got), "truncated and rewritten")
ok++
}
if string(got) == "zqsamefilesentinel" {
leaks++
}
got, _ = b.ReadRegularMax("r.md", 1<<20)
if string(got) == "zqsamefilesentinel" {
leaks++
}
}
stop.Store(true)
wg.Wait()
assert.Zero(t, leaks)
assert.Positive(t, ok, "positive control: the regular file is read")
} }
@@ -7,7 +7,6 @@
package brainfstest package brainfstest
import ( import (
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -21,23 +20,16 @@ import (
// Sentinel marks the outside file's contents. // Sentinel marks the outside file's contents.
const Sentinel = "zqbrainfssentinelonefiftyseven" const Sentinel = "zqbrainfssentinelonefiftyseven"
// outsideContent carries Sentinel in every field a reader might extract: the
// frontmatter title and aliases, a heading, a wikilink and the body.
const outsideContent = "---\ntitle: " + Sentinel + "\naliases:\n - " + Sentinel + "\n---\n# " + Sentinel +
"\n\n" + Sentinel + " [[" + Sentinel + "]]\n"
// Outside writes a file outside any brain dir containing Sentinel. // Outside writes a file outside any brain dir containing Sentinel.
func Outside(t *testing.T) string { func Outside(t *testing.T) string {
t.Helper() t.Helper()
p := filepath.Join(t.TempDir(), "secret.md") p := filepath.Join(t.TempDir(), "secret.md")
if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { if err := os.WriteFile(p, []byte("# secret\n\n"+Sentinel), 0o644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
return p return p
} }
var swapSeq atomic.Int64
// StartSwap renames, in a loop, a regular note (content inside) and either // StartSwap renames, in a loop, a regular note (content inside) and either
// a symlink to outside or a FIFO into path, until the test ends. FIFOs get a // a symlink to outside or a FIFO into path, until the test ends. FIFOs get a
// writer held open until the swapper exits, so a reader that skips the // writer held open until the swapper exits, so a reader that skips the
@@ -50,15 +42,7 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
wg.Add(1) wg.Add(1)
go func() { go func() {
defer wg.Done() defer wg.Done()
// A name of its own: swappers on one path share a test, and one tmp := path + ".swap"
// opening another's FIFO for write would block forever.
tmp := fmt.Sprintf("%s.swap%d", path, swapSeq.Add(1))
// A relative link: os.Root refuses every absolute link on its own, so
// an absolute in-brain target would test nothing.
link := outside
if rel, err := filepath.Rel(filepath.Dir(path), outside); err == nil {
link = rel
}
var writers []*os.File var writers []*os.File
defer func() { defer func() {
for _, w := range writers { for _, w := range writers {
@@ -77,7 +61,7 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
} }
} }
default: default:
_ = os.Symlink(link, tmp) _ = os.Symlink(outside, tmp)
} }
_ = os.Rename(tmp, path) _ = os.Rename(tmp, path)
} }
@@ -85,33 +69,11 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
t.Cleanup(func() { stop.Store(true); wg.Wait() }) t.Cleanup(func() { stop.Store(true); wg.Wait() })
} }
// WriteSentinelAt writes a file carrying Sentinel at p (creating its
// directory), for an in-brain symlink target.
func WriteSentinelAt(t *testing.T, p string) string {
t.Helper()
if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil {
t.Fatal(err)
}
return p
}
// AssertNoLeak loops read for d while a symlink swapper runs on path, and // AssertNoLeak loops read for d while a symlink swapper runs on path, and
// fails if any output contains Sentinel. The symlink points outside the // fails if any output contains Sentinel.
// brain; os.Root refuses that on its own, so pair it with AssertNoLeakFrom
// and an in-brain target.
func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func() string) { func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func() string) {
t.Helper() t.Helper()
AssertNoLeakFrom(t, path, inside, Outside(t), d, read) StartSwap(t, path, inside, Outside(t), false)
}
// AssertNoLeakFrom is AssertNoLeak with the symlink pointing at target, e.g.
// a file elsewhere in the brain written with WriteSentinelAt.
func AssertNoLeakFrom(t *testing.T, path, inside, target string, d time.Duration, read func() string) {
t.Helper()
StartSwap(t, path, inside, target, false)
leaks, iters := 0, 0 leaks, iters := 0, 0
for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ { for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ {
leaks += strings.Count(read(), Sentinel) leaks += strings.Count(read(), Sentinel)
+21 -43
View File
@@ -12,14 +12,10 @@ package graphsync
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs" "os"
"path"
"path/filepath" "path/filepath"
"strings"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"github.com/mathiasbq/hyperguild/ingestion/internal/graph" "github.com/mathiasbq/hyperguild/ingestion/internal/graph"
"github.com/mathiasbq/hyperguild/ingestion/internal/graphstore" "github.com/mathiasbq/hyperguild/ingestion/internal/graphstore"
) )
@@ -50,25 +46,8 @@ func IndexDoc(ctx context.Context, store Store, brainDir, relPath string) error
if relPath == "" { if relPath == "" {
return nil return nil
} }
// Callers pass caller-supplied paths (brain_tunnel, brain_update): index abs := filepath.Join(brainDir, filepath.FromSlash(relPath))
// only a clean path under wiki/ or knowledge/. content, err := os.ReadFile(abs)
if path.Clean(relPath) != relPath || !filepath.IsLocal(relPath) ||
(!strings.HasPrefix(relPath, "wiki/") && !strings.HasPrefix(relPath, "knowledge/")) {
return fmt.Errorf("index %q: not a clean wiki/ or knowledge/ path", relPath)
}
bfs, err := brainfs.Open(brainDir)
if err != nil {
return fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
return indexDoc(ctx, store, bfs, relPath)
}
// indexDoc reads relPath through the brain root, regular files only
// (hyperguild#157): the brain is a Syncthing folder, and os.ReadFile
// followed a synced symlink into the graph and blocked on a FIFO.
func indexDoc(ctx context.Context, store Store, bfs *brainfs.Brain, relPath string) error {
content, err := bfs.ReadRegular(relPath)
if err != nil { if err != nil {
return fmt.Errorf("read %q: %w", relPath, err) return fmt.Errorf("read %q: %w", relPath, err)
} }
@@ -98,34 +77,33 @@ func BackfillFromBrainDir(ctx context.Context, store Store, brainDir string) (in
if store == nil { if store == nil {
return 0, nil return 0, nil
} }
bfs, err := brainfs.Open(brainDir) roots := []string{"wiki", "knowledge"}
if err != nil { for _, root := range roots {
return 0, fmt.Errorf("open brain: %w", err) base := filepath.Join(brainDir, root)
} if _, err := os.Stat(base); os.IsNotExist(err) {
defer func() { _ = bfs.Close() }()
for _, root := range []string{"wiki", "knowledge"} {
if _, err := bfs.Lstat(root); errors.Is(err, fs.ErrNotExist) {
continue continue
} }
// Any walk error, including a root that is a symlink, aborts. err := filepath.WalkDir(base, func(path string, d os.DirEntry, walkErr error) error {
var walkErr error if walkErr != nil {
err := bfs.WalkRegular(root, func(rel string) error { return walkErr
if path.Ext(rel) != ".md" { }
if d.IsDir() {
return nil return nil
} }
if err := indexDoc(ctx, store, bfs, rel); err != nil { if filepath.Ext(path) != ".md" {
return nil
}
rel, relErr := filepath.Rel(brainDir, path)
if relErr != nil {
return fmt.Errorf("rel %q: %w", path, relErr)
}
rel = filepath.ToSlash(rel)
if err := IndexDoc(ctx, store, brainDir, rel); err != nil {
return fmt.Errorf("index %q: %w", rel, err) return fmt.Errorf("index %q: %w", rel, err)
} }
indexed++ indexed++
return nil return nil
}, func(rel string, err error) {
if walkErr == nil {
walkErr = fmt.Errorf("%s: %w", rel, err)
}
}) })
if err == nil {
err = walkErr
}
if err != nil { if err != nil {
return indexed, fmt.Errorf("walk %s: %w", root, err) return indexed, fmt.Errorf("walk %s: %w", root, err)
} }
@@ -1,92 +0,0 @@
package graphsync
import (
"context"
"fmt"
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
)
func (f *fakeStore) dump() string {
f.mu.Lock()
defer f.mu.Unlock()
return fmt.Sprintf("%+v %+v", f.upserts, f.replaces)
}
// hyperguild#157 slice 4: IndexDoc and BackfillFromBrainDir read notes with
// os.ReadFile, so a symlinked note put the target's title and wikilinks into
// the graph, and a FIFO hung the read.
func TestIndexDocRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: zqinside\n---\nbody [[other]]\n"), 0o644))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md")))
store := newFakeStore()
require.NoError(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/ok.md"))
assert.Contains(t, store.dump(), "zqinside", "positive control: a regular note is indexed")
assert.Error(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/leak.md"))
// A link to a file elsewhere in the brain: os.Root allows it, only the
// regular-file check refuses it.
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
assert.Error(t, IndexDoc(context.Background(), store, brain, "wiki/w/facts/inlink.md"))
assert.NotContains(t, store.dump(), brainfstest.Sentinel)
p := filepath.Join(facts, "r.md")
read := func() string {
s := newFakeStore()
_ = IndexDoc(context.Background(), s, brain, "wiki/w/facts/r.md")
return s.dump()
}
brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read)
brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: zqinside\n---\n", inBrain, 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}
func TestBackfillSkipsSymlinkedNotesAndAbortsOnASymlinkedRoot(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md")))
require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "r.md"), filepath.Join(facts, "inlink.md")))
require.NoError(t, os.Symlink(".", filepath.Join(brain, "knowledge")))
store := newFakeStore()
_, err := BackfillFromBrainDir(context.Background(), store, brain)
assert.Error(t, err, "a symlinked root aborts the backfill")
assert.NotContains(t, store.dump(), brainfstest.Sentinel)
assert.NotContains(t, store.dump(), "zqrawonly")
require.NoError(t, os.Remove(filepath.Join(brain, "knowledge")))
store = newFakeStore()
n, err := BackfillFromBrainDir(context.Background(), store, brain)
require.NoError(t, err)
assert.Equal(t, 1, n, "only the regular note is indexed")
assert.Contains(t, store.dump(), "zqinside")
assert.NotContains(t, store.dump(), brainfstest.Sentinel)
}
// IndexDoc gets caller-supplied paths (brain_tunnel, brain_update): only a
// clean path under wiki/ or knowledge/ is indexed.
func TestIndexDocRefusesPathsOutsideWikiAndKnowledgeAndUncleanPaths(t *testing.T) {
brain := t.TempDir()
brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "secret.md"))
require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w"), 0o755))
for _, p := range []string{"raw/secret.md", "wiki/w/../../raw/secret.md", "./raw/secret.md", "/raw/secret.md"} {
store := newFakeStore()
assert.Error(t, IndexDoc(context.Background(), store, brain, p), p)
assert.NotContains(t, store.dump(), brainfstest.Sentinel, p)
}
}
+49
View File
@@ -0,0 +1,49 @@
package session
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// session_log passed the caller's session_id straight into
// filepath.Join(sessionsDir, id+".jsonl") and opened it O_CREATE|O_APPEND:
// path segments in the id created or appended files anywhere the pod could
// write. The id is now a plain name, and the open does not follow a final
// symlink.
func TestAppendRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
for _, id := range []string{"../x", "a/b", "/abs", ".", "..", ".hidden", "", "a\\b", "a\x00b"} {
assert.Error(t, Append(dir, id, Entry{SessionID: id}), "%q", id)
}
ents, _ := os.ReadDir(root)
for _, e := range ents {
assert.Equal(t, "sessions", e.Name(), "nothing created beside the sessions dir")
}
for _, id := range []string{"1557b2e1-bf0d-5c4f-bbe3-0754d087e356", "_routing", "2026-05-05-infra.plan", "0"} {
require.NoError(t, Append(dir, id, Entry{SessionID: id}), "positive control: %q", id)
}
}
func TestAppendDoesNotWriteThroughASymlinkedLog(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
victim := filepath.Join(root, "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "victim.md"), filepath.Join(dir, "s.jsonl")))
assert.Error(t, Append(dir, "s", Entry{SessionID: "s"}))
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
func TestReadRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(root, "x.jsonl"), []byte(`{"session_id":"x"}`+"\n"), 0o644))
_, err := Read(filepath.Join(root, "sessions"), "../x")
assert.Error(t, err)
}
+22 -1
View File
@@ -9,6 +9,8 @@ import (
"io/fs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"regexp"
"syscall"
"time" "time"
) )
@@ -42,13 +44,29 @@ type Attempt struct {
RunnerOutput string `json:"runner_output,omitempty"` RunnerOutput string `json:"runner_output,omitempty"`
} }
// idPattern is a plain file name: session_log takes the id from the caller,
// and a path segment in it created or appended files anywhere the pod could
// write (hyperguild#169).
var idPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
func checkID(sessionID string) error {
if !idPattern.MatchString(sessionID) {
return fmt.Errorf("session id %q is not a plain name", sessionID)
}
return nil
}
// Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl. // Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl.
// The open does not follow a final symlink.
func Append(sessionsDir, sessionID string, entry Entry) error { func Append(sessionsDir, sessionID string, entry Entry) error {
if err := checkID(sessionID); err != nil {
return err
}
if err := os.MkdirAll(sessionsDir, 0o755); err != nil { if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
return fmt.Errorf("create sessions dir: %w", err) return fmt.Errorf("create sessions dir: %w", err)
} }
path := filepath.Join(sessionsDir, sessionID+".jsonl") path := filepath.Join(sessionsDir, sessionID+".jsonl")
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
if err != nil { if err != nil {
return fmt.Errorf("open session log: %w", err) return fmt.Errorf("open session log: %w", err)
} }
@@ -70,6 +88,9 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
// Read returns all entries for sessionID. Returns empty slice if no log exists. // Read returns all entries for sessionID. Returns empty slice if no log exists.
func Read(sessionsDir, sessionID string) ([]Entry, error) { func Read(sessionsDir, sessionID string) ([]Entry, error) {
if err := checkID(sessionID); err != nil {
return nil, err
}
path := filepath.Join(sessionsDir, sessionID+".jsonl") path := filepath.Join(sessionsDir, sessionID+".jsonl")
f, err := os.Open(path) f, err := os.Open(path)
if errors.Is(err, fs.ErrNotExist) { if errors.Is(err, fs.ErrNotExist) {
+4 -11
View File
@@ -7,7 +7,6 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"github.com/mathiasbq/hyperguild/ingestion/internal/textutil" "github.com/mathiasbq/hyperguild/ingestion/internal/textutil"
) )
@@ -17,11 +16,6 @@ func RebuildIndex(brainDir, date string) error {
if err != nil { if err != nil {
return fmt.Errorf("load inventory: %w", err) return fmt.Errorf("load inventory: %w", err)
} }
bfs, err := brainfs.Open(brainDir)
if err != nil {
return fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
total := len(inv[PageTypeConcept]) + len(inv[PageTypeEntity]) + len(inv[PageTypeSource]) total := len(inv[PageTypeConcept]) + len(inv[PageTypeEntity]) + len(inv[PageTypeSource])
var sb strings.Builder var sb strings.Builder
@@ -40,7 +34,7 @@ func RebuildIndex(brainDir, date string) error {
label := strings.ToUpper(string(pt)[:1]) + string(pt)[1:] label := strings.ToUpper(string(pt)[:1]) + string(pt)[1:]
fmt.Fprintf(&sb, "## %s\n\n", label) fmt.Fprintf(&sb, "## %s\n\n", label)
for _, e := range entries { for _, e := range entries {
summary := pageFirstSentence(bfs, e) summary := pageFirstSentence(brainDir, e)
if summary != "" { if summary != "" {
fmt.Fprintf(&sb, "- [[%s|%s]] — %s\n", e.Slug, e.Title, summary) fmt.Fprintf(&sb, "- [[%s|%s]] — %s\n", e.Slug, e.Title, summary)
} else { } else {
@@ -54,10 +48,9 @@ func RebuildIndex(brainDir, date string) error {
return os.WriteFile(dest, []byte(sb.String()), 0o644) return os.WriteFile(dest, []byte(sb.String()), 0o644)
} }
// pageFirstSentence reads the page through the brain root, regular files func pageFirstSentence(brainDir string, e Entry) string {
// only (hyperguild#157). path := filepath.Join(brainDir, "wiki", string(e.Type), e.Slug+".md")
func pageFirstSentence(bfs *brainfs.Brain, e Entry) string { content, err := os.ReadFile(path)
content, err := bfs.ReadRegular("wiki/" + string(e.Type) + "/" + e.Slug + ".md")
if err != nil { if err != nil {
return "" return ""
} }
+15 -23
View File
@@ -3,45 +3,36 @@ package wiki
import ( import (
"bufio" "bufio"
"bytes"
"errors"
"fmt" "fmt"
"io/fs" "os"
"path/filepath"
"strings" "strings"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// LoadInventory walks brain/wiki/ and returns all pages grouped by type. // LoadInventory walks brain/wiki/ and returns all pages grouped by type.
// Missing subdirectories are silently skipped. Pages are listed and read // Missing subdirectories are silently skipped.
// through the brain root, regular files only (hyperguild#157); a type
// directory that is a symlink is an error.
func LoadInventory(brainDir string) (map[PageType][]Entry, error) { func LoadInventory(brainDir string) (map[PageType][]Entry, error) {
result := map[PageType][]Entry{ result := map[PageType][]Entry{
PageTypeConcept: {}, PageTypeConcept: {},
PageTypeEntity: {}, PageTypeEntity: {},
PageTypeSource: {}, PageTypeSource: {},
} }
bfs, err := brainfs.Open(brainDir)
if err != nil {
return nil, fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
for pt := range result { for pt := range result {
dir := "wiki/" + string(pt) dir := filepath.Join(brainDir, "wiki", string(pt))
names, err := bfs.ReadDirRegular(dir) entries, err := os.ReadDir(dir)
if errors.Is(err, fs.ErrNotExist) { if os.IsNotExist(err) {
continue continue
} }
if err != nil { if err != nil {
return nil, fmt.Errorf("read dir %s: %w", dir, err) return nil, fmt.Errorf("read dir %s: %w", dir, err)
} }
for _, name := range names { for _, e := range entries {
if !strings.HasSuffix(name, ".md") { if e.IsDir() || !strings.HasSuffix(e.Name(), ".md") {
continue continue
} }
slug := strings.TrimSuffix(name, ".md") slug := strings.TrimSuffix(e.Name(), ".md")
title, aliases := readFrontmatter(bfs, dir+"/"+name, slug) path := filepath.Join(dir, e.Name())
title, aliases := readFrontmatter(path, slug)
result[pt] = append(result[pt], Entry{Slug: slug, Title: title, Aliases: aliases, Type: pt}) result[pt] = append(result[pt], Entry{Slug: slug, Title: title, Aliases: aliases, Type: pt})
} }
} }
@@ -50,14 +41,15 @@ func LoadInventory(brainDir string) (map[PageType][]Entry, error) {
// readFrontmatter extracts title and aliases from YAML frontmatter. // readFrontmatter extracts title and aliases from YAML frontmatter.
// Falls back to slug for title and empty aliases on any error. // Falls back to slug for title and empty aliases on any error.
func readFrontmatter(bfs *brainfs.Brain, rel, fallbackSlug string) (title string, aliases []string) { func readFrontmatter(path, fallbackSlug string) (title string, aliases []string) {
title = fallbackSlug title = fallbackSlug
content, err := bfs.ReadRegular(rel) f, err := os.Open(path)
if err != nil { if err != nil {
return return
} }
defer f.Close() //nolint:errcheck
scanner := bufio.NewScanner(bytes.NewReader(content)) scanner := bufio.NewScanner(f)
inFM := false inFM := false
inAliases := false inAliases := false
for scanner.Scan() { for scanner.Scan() {
-79
View File
@@ -1,79 +0,0 @@
package wiki
import (
"fmt"
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
)
// hyperguild#157 slice 4: LoadInventory listed wiki/<type>/ with os.ReadDir
// and opened each .md by path, so a symlinked page put the target's title and
// aliases into the inventory (and the LLM prompts built from it), and a FIFO
// hung it. RebuildIndex read first sentences the same way.
func TestLoadInventoryRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
dir := filepath.Join(brain, "wiki", "concepts")
require.NoError(t, os.MkdirAll(dir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(dir, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(dir, "leak.md")))
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "s.md"), filepath.Join(dir, "inlink.md")))
inv, err := LoadInventory(brain)
require.NoError(t, err)
got := fmt.Sprintf("%+v", inv)
assert.Contains(t, got, "zqinside", "positive control")
assert.NotContains(t, got, brainfstest.Sentinel)
assert.NotContains(t, got, "leak", "a symlinked page is not listed")
assert.NotContains(t, got, "inlink", "a page linked to another brain file is not listed")
p := filepath.Join(dir, "r.md")
read := func() string {
inv, _ := LoadInventory(brain)
return fmt.Sprintf("%+v", inv)
}
brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read)
brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: zqinside\n---\n", inBrain, 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}
func TestLoadInventoryReturnsErrNotRegularForASymlinkedTypeDir(t *testing.T) {
brain := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755))
require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(brain, "wiki", "concepts")))
inv, err := LoadInventory(brain)
require.ErrorIs(t, err, brainfs.ErrNotRegular)
assert.NotContains(t, fmt.Sprintf("%+v", inv), "zqrawonly")
}
func TestRebuildIndexDoesNotReadAPageSwappedForASymlink(t *testing.T) {
brain := t.TempDir()
dir := filepath.Join(brain, "wiki", "concepts")
require.NoError(t, os.MkdirAll(dir, 0o755))
p := filepath.Join(dir, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\ntitle: R\n---\nzqinside sentence.\n"), 0o644))
require.NoError(t, RebuildIndex(brain, "2026-10-04"))
idx, err := os.ReadFile(filepath.Join(brain, "wiki", "index.md"))
require.NoError(t, err)
require.Contains(t, string(idx), "zqinside", "positive control: the summary is read")
read := func() string {
_ = RebuildIndex(brain, "2026-10-04")
b, _ := os.ReadFile(filepath.Join(brain, "wiki", "index.md"))
return string(b)
}
brainfstest.AssertNoLeak(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, read)
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", inBrain, 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "---\ntitle: R\n---\nzqinside sentence.\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}