fix(tunnel): brain_tunnel cannot write outside the wiki or through symlinks
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>
This commit is contained in:
@@ -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)
|
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)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
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,128 @@
|
|||||||
|
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", "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)
|
||||||
|
}
|
||||||
|
got, err := os.ReadFile(filepath.Join(brain0, "raw", "r.md"))
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, "x\n", string(got))
|
||||||
|
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))
|
||||||
|
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")
|
||||||
|
}
|
||||||
@@ -228,3 +228,41 @@ func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func
|
|||||||
return fn(p)
|
return fn(p)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WriteRegular replaces the contents of an existing regular file at a
|
||||||
|
// brain-relative path. It never creates a file and never writes through a
|
||||||
|
// final symlink: os.Root.OpenFile follows an in-root final symlink even with
|
||||||
|
// O_NOFOLLOW (verified on go1.27), so the path is Lstat-ed through the root
|
||||||
|
// first and the opened handle must be that same file (os.SameFile), which
|
||||||
|
// also catches a swap between the two. O_NONBLOCK keeps a FIFO from blocking
|
||||||
|
// the open. Intermediate in-brain symlinks are still followed (os.Root keeps
|
||||||
|
// the write inside the brain; hyperguild#167).
|
||||||
|
func (b *Brain) WriteRegular(rel string, data []byte) error {
|
||||||
|
rel = filepath.ToSlash(rel)
|
||||||
|
lfi, err := b.root.Lstat(rel)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if !lfi.Mode().IsRegular() {
|
||||||
|
return fmt.Errorf("%s: %w", rel, ErrNotRegular)
|
||||||
|
}
|
||||||
|
f, err := b.root.OpenFile(rel, os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
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()
|
||||||
|
}
|
||||||
|
|||||||
@@ -262,3 +262,48 @@ func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.
|
|||||||
assert.Zero(t, leaks)
|
assert.Zero(t, leaks)
|
||||||
assert.Positive(t, ok, "positive control: the regular file is read")
|
assert.Positive(t, ok, "positive control: the regular file is read")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
outside := filepath.Join(t.TempDir(), "o.md")
|
||||||
|
require.NoError(t, os.WriteFile(outside, []byte("untouched"), 0o644))
|
||||||
|
require.NoError(t, os.Symlink(outside, filepath.Join(dir, "l.md")))
|
||||||
|
require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "f.md"), 0o644))
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "n.md"), []byte("old content"), 0o644))
|
||||||
|
b, err := Open(dir)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer func() { _ = b.Close() }()
|
||||||
|
|
||||||
|
assert.Error(t, b.WriteRegular("l.md", []byte("x")))
|
||||||
|
got, _ := os.ReadFile(outside)
|
||||||
|
assert.Equal(t, "untouched", string(got))
|
||||||
|
|
||||||
|
// A symlink to another file inside the brain: os.Root would allow it,
|
||||||
|
// only O_NOFOLLOW refuses it.
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(dir, "in.md"), []byte("inside"), 0o644))
|
||||||
|
require.NoError(t, os.Symlink("in.md", filepath.Join(dir, "il.md")))
|
||||||
|
assert.Error(t, b.WriteRegular("il.md", []byte("x")))
|
||||||
|
got, _ = os.ReadFile(filepath.Join(dir, "in.md"))
|
||||||
|
assert.Equal(t, "inside", string(got))
|
||||||
|
|
||||||
|
// A reader held open, so a non-blocking write open would succeed and
|
||||||
|
// only the type check on the handle refuses it.
|
||||||
|
r, err := os.OpenFile(filepath.Join(dir, "f.md"), os.O_RDONLY|syscall.O_NONBLOCK, 0)
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer func() { _ = r.Close() }()
|
||||||
|
done := make(chan error, 1)
|
||||||
|
go func() { done <- b.WriteRegular("f.md", []byte("x")) }()
|
||||||
|
select {
|
||||||
|
case err := <-done:
|
||||||
|
assert.Error(t, err)
|
||||||
|
case <-time.After(3 * time.Second):
|
||||||
|
t.Fatal("WriteRegular hung on a FIFO")
|
||||||
|
}
|
||||||
|
|
||||||
|
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")
|
||||||
|
|
||||||
|
require.NoError(t, b.WriteRegular("n.md", []byte("new")))
|
||||||
|
got, _ = os.ReadFile(filepath.Join(dir, "n.md"))
|
||||||
|
assert.Equal(t, "new", string(got), "truncated and rewritten")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user