fix(ingestion): every brain/raw/ reader goes through an os.Root; symlinks are inert
raw/ is writable via Syncthing (a sendreceive folder shared with flamingo, symlinks synced) and by the brain CronJobs, and several readers followed symlinks out of it: - /pending and brain_pending showed a symlink target's contents; brain_promote would copy it into wiki/ (then brain-sync to Gitea); brain_reject would copy it into raw/rejected/. - The watcher, which needs no API call at all, ingested symlinked files (WalkDir yields them; extract.Text followed them). Not in the issue; found while fixing it. - Ingest checked a path then reopened it by name (TOCTOU). OpenRawRoot opens brain/raw as an os.Root and refuses a raw/ that is itself a symlink. ListPending, PromoteNote and RejectNote read through it and refuse anything not a regular file. extract.TextIn reads through a root (temp copy for pdf/docmark); /ingest-path, brain_ingest and the watcher use it, and the watcher archives via the root too, so a file swapped for a symlink after a check still cannot lead out. Refs #153 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,7 +13,6 @@ import (
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/claim"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/search"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore"
|
||||
@@ -481,7 +480,7 @@ func (h *Handler) IngestPath(w http.ResponseWriter, r *http.Request) {
|
||||
allWarnings = append(allWarnings, fmt.Sprintf("skipped %s: %v", path, rerr))
|
||||
return nil
|
||||
}
|
||||
content, readErr := extract.Text(path)
|
||||
content, readErr := ReadIngestFile(h.brainDir, path)
|
||||
if readErr != nil {
|
||||
allWarnings = append(allWarnings, fmt.Sprintf("extract %s: %v", path, readErr))
|
||||
return nil
|
||||
@@ -510,7 +509,7 @@ func (h *Handler) IngestPath(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, fmt.Sprintf("unsupported file extension: %s", ext))
|
||||
return
|
||||
}
|
||||
content, readErr := extract.Text(req.Path)
|
||||
content, readErr := ReadIngestFile(h.brainDir, req.Path)
|
||||
if readErr != nil {
|
||||
writeError(w, http.StatusInternalServerError, fmt.Sprintf("extract text: %v", readErr))
|
||||
return
|
||||
|
||||
@@ -2,6 +2,9 @@ package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
)
|
||||
@@ -19,6 +22,11 @@ func ResolveIngestPath(brainDir, p string) (string, error) {
|
||||
if strings.TrimSpace(p) == "" {
|
||||
return "", ErrOutsideIngestRoot
|
||||
}
|
||||
// raw/ itself must be a real directory: if it were a symlink, containment
|
||||
// inside it would mean nothing (hyperguild#153 item 4).
|
||||
if fi, lerr := os.Lstat(filepath.Join(brainDir, "raw")); lerr != nil || fi.Mode()&os.ModeSymlink != 0 || !fi.IsDir() {
|
||||
return "", ErrOutsideIngestRoot
|
||||
}
|
||||
root, err := filepath.EvalSymlinks(filepath.Join(brainDir, "raw"))
|
||||
if err != nil {
|
||||
return "", ErrOutsideIngestRoot
|
||||
@@ -36,3 +44,24 @@ func ResolveIngestPath(brainDir, p string) (string, error) {
|
||||
}
|
||||
return real, nil
|
||||
}
|
||||
|
||||
// ReadIngestFile extracts text from an ingest path already confined by
|
||||
// ResolveIngestPath, reading it through the raw/ root rather than reopening
|
||||
// the path by name (hyperguild#153 item 3): if the file is swapped for a
|
||||
// symlink after the check, the read still cannot leave raw/.
|
||||
func ReadIngestFile(brainDir, resolved string) (string, error) {
|
||||
root, rawPath, err := OpenRawRoot(brainDir)
|
||||
if err != nil {
|
||||
return "", ErrOutsideIngestRoot
|
||||
}
|
||||
defer func() { _ = root.Close() }()
|
||||
realRoot, err := filepath.EvalSymlinks(rawPath)
|
||||
if err != nil {
|
||||
return "", ErrOutsideIngestRoot
|
||||
}
|
||||
rel, err := filepath.Rel(realRoot, resolved)
|
||||
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) {
|
||||
return "", ErrOutsideIngestRoot
|
||||
}
|
||||
return extract.TextIn(root, rel)
|
||||
}
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
@@ -28,25 +30,25 @@ var datePrefix = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}-`)
|
||||
// (natural review order). An absent raw/ dir yields an empty slice, not an
|
||||
// error. Only .md files are listed; tunnel-candidate files are skipped.
|
||||
func ListPending(brainDir string) ([]PendingNote, error) {
|
||||
dir := filepath.Join(brainDir, "raw")
|
||||
entries, err := os.ReadDir(dir)
|
||||
root, _, err := OpenRawRoot(brainDir)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return []PendingNote{}, nil
|
||||
}
|
||||
return nil, fmt.Errorf("read raw dir: %w", err)
|
||||
}
|
||||
defer func() { _ = root.Close() }()
|
||||
entries, err := fs.ReadDir(root.FS(), ".")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read raw dir: %w", err)
|
||||
}
|
||||
|
||||
out := make([]PendingNote, 0, len(entries))
|
||||
for _, e := range entries {
|
||||
if e.IsDir() || !strings.HasSuffix(e.Name(), ".md") || strings.HasPrefix(e.Name(), "tunnel-candidates-") {
|
||||
continue
|
||||
if !e.Type().IsRegular() || !strings.HasSuffix(e.Name(), ".md") || strings.HasPrefix(e.Name(), "tunnel-candidates-") {
|
||||
continue // directories, and symlinks whose target may be anywhere (#153)
|
||||
}
|
||||
info, statErr := e.Info()
|
||||
if statErr != nil {
|
||||
continue
|
||||
}
|
||||
raw, readErr := os.ReadFile(filepath.Join(dir, e.Name()))
|
||||
raw, info, readErr := readRawNote(root, e.Name())
|
||||
if readErr != nil {
|
||||
continue
|
||||
}
|
||||
@@ -105,12 +107,9 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) {
|
||||
}
|
||||
|
||||
src := filepath.Join(brainDir, "raw", base)
|
||||
raw, err := os.ReadFile(src)
|
||||
raw, err := readRawSource(brainDir, base)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return "", fmt.Errorf("pending note %q does not exist in raw/", base)
|
||||
}
|
||||
return "", fmt.Errorf("read source: %w", err)
|
||||
return "", err
|
||||
}
|
||||
|
||||
// Collision: never silently overwrite an existing promoted note.
|
||||
@@ -180,12 +179,9 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) {
|
||||
}
|
||||
|
||||
src := filepath.Join(brainDir, "raw", base)
|
||||
raw, err := os.ReadFile(src)
|
||||
raw, err := readRawSource(brainDir, base)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return "", fmt.Errorf("pending note %q does not exist in raw/", base)
|
||||
}
|
||||
return "", fmt.Errorf("read source: %w", err)
|
||||
return "", err
|
||||
}
|
||||
|
||||
dest := filepath.Join(brainDir, "raw", "rejected", base)
|
||||
@@ -224,3 +220,24 @@ func excerpt(s string, n int) string {
|
||||
}
|
||||
return strings.TrimSpace(string(r))
|
||||
}
|
||||
|
||||
// readRawSource reads a pending note for promote/reject through the raw root,
|
||||
// refusing symlinks and a symlinked raw/ (hyperguild#153).
|
||||
func readRawSource(brainDir, base string) ([]byte, error) {
|
||||
root, _, err := OpenRawRoot(brainDir)
|
||||
if err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return nil, fmt.Errorf("pending note %q does not exist in raw/", base)
|
||||
}
|
||||
return nil, fmt.Errorf("read source: %w", err)
|
||||
}
|
||||
defer func() { _ = root.Close() }()
|
||||
raw, _, err := readRawNote(root, base)
|
||||
if err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return nil, fmt.Errorf("pending note %q does not exist in raw/", base)
|
||||
}
|
||||
return nil, fmt.Errorf("read source: %w", err)
|
||||
}
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
package api_test
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||
)
|
||||
|
||||
// hyperguild#153: readers of brain/raw/ followed symlinks. /pending showed the
|
||||
// target's contents and promote would copy it into wiki/ (and brain-sync to
|
||||
// Gitea). raw/ is writable via Syncthing, so a symlink there is reachable
|
||||
// input. Every raw/ reader now treats a symlink as not-a-note.
|
||||
const sentinel153 = "SENTINEL-153-OUTSIDE"
|
||||
|
||||
func rawWithSymlink(t *testing.T) (brain string) {
|
||||
t.Helper()
|
||||
brain = t.TempDir()
|
||||
raw := filepath.Join(brain, "raw")
|
||||
require.NoError(t, os.MkdirAll(raw, 0o755))
|
||||
outside := filepath.Join(t.TempDir(), "secret.md")
|
||||
require.NoError(t, os.WriteFile(outside, []byte("---\ncreated_at: 2026-01-01T00:00:00Z\n---\n# s\n\n"+sentinel153), 0o644))
|
||||
require.NoError(t, os.Symlink(outside, filepath.Join(raw, "2026-01-01-leak.md")))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(raw, "2026-01-02-real.md"), []byte("# real\n\nreal body"), 0o644))
|
||||
return brain
|
||||
}
|
||||
|
||||
func TestListPendingSkipsSymlinkedNotes(t *testing.T) {
|
||||
notes, err := api.ListPending(rawWithSymlink(t))
|
||||
require.NoError(t, err)
|
||||
var names []string
|
||||
for _, n := range notes {
|
||||
names = append(names, n.Filename)
|
||||
assert.NotContains(t, n.Excerpt, sentinel153)
|
||||
}
|
||||
assert.Equal(t, []string{"2026-01-02-real.md"}, names)
|
||||
}
|
||||
|
||||
func TestPromoteNoteRefusesASymlinkedSource(t *testing.T) {
|
||||
brain := rawWithSymlink(t)
|
||||
_, err := api.PromoteNote(brain, api.PromoteOptions{Filename: "2026-01-01-leak.md", Wing: "testing", Hall: "facts"})
|
||||
require.Error(t, err)
|
||||
_, statErr := os.Stat(filepath.Join(brain, "wiki", "testing", "facts", "leak.md"))
|
||||
assert.True(t, os.IsNotExist(statErr), "nothing may be promoted from a symlink")
|
||||
}
|
||||
|
||||
func TestRejectNoteRefusesASymlinkedSource(t *testing.T) {
|
||||
brain := rawWithSymlink(t)
|
||||
_, err := api.RejectNote(brain, api.RejectOptions{Filename: "2026-01-01-leak.md"})
|
||||
require.Error(t, err)
|
||||
b, _ := os.ReadFile(filepath.Join(brain, "raw", "rejected", "2026-01-01-leak.md"))
|
||||
assert.False(t, strings.Contains(string(b), sentinel153))
|
||||
}
|
||||
|
||||
// Item 4: if raw/ itself is a symlink, containment inside it means nothing.
|
||||
func TestRawReadersRefuseASymlinkedRawRoot(t *testing.T) {
|
||||
brain := t.TempDir()
|
||||
elsewhere := t.TempDir()
|
||||
require.NoError(t, os.WriteFile(filepath.Join(elsewhere, "x.md"), []byte("# x\n\n"+sentinel153), 0o644))
|
||||
require.NoError(t, os.Symlink(elsewhere, filepath.Join(brain, "raw")))
|
||||
|
||||
_, err := api.ResolveIngestPath(brain, "x.md")
|
||||
assert.Error(t, err, "ingest must refuse a symlinked raw/")
|
||||
notes, err := api.ListPending(brain)
|
||||
assert.Error(t, err, "pending must refuse a symlinked raw/")
|
||||
assert.Empty(t, notes)
|
||||
_, err = api.PromoteNote(brain, api.PromoteOptions{Filename: "x.md", Wing: "testing", Hall: "facts"})
|
||||
assert.Error(t, err, "promote must refuse a symlinked raw/")
|
||||
}
|
||||
@@ -0,0 +1,59 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
)
|
||||
|
||||
// errNotANote is returned for a raw/ entry that is not a regular file — in
|
||||
// particular a symlink, whose target may be anywhere (hyperguild#153).
|
||||
var errNotANote = errors.New("not a regular note file in raw/ (symlinks are refused)")
|
||||
|
||||
// errRawNotDir means brain/raw is not a real directory (e.g. a symlink), so
|
||||
// containment inside it would mean nothing (hyperguild#153 item 4).
|
||||
var errRawNotDir = errors.New("brain/raw is not a real directory")
|
||||
|
||||
// OpenRawRoot opens brain/raw as an os.Root, refusing a raw/ that is itself a
|
||||
// symlink. Reads through the root cannot leave it, even if an entry is
|
||||
// swapped for a symlink between a check and the read. It returns the root
|
||||
// and raw/'s path. A missing raw/ returns an error wrapping os.ErrNotExist.
|
||||
func OpenRawRoot(brainDir string) (*os.Root, string, error) {
|
||||
p := filepath.Join(brainDir, "raw")
|
||||
fi, err := os.Lstat(p)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
if fi.Mode()&os.ModeSymlink != 0 || !fi.IsDir() {
|
||||
return nil, "", errRawNotDir
|
||||
}
|
||||
r, err := os.OpenRoot(p)
|
||||
if err != nil {
|
||||
return nil, "", fmt.Errorf("open raw root: %w", err)
|
||||
}
|
||||
return r, p, nil
|
||||
}
|
||||
|
||||
// readRawNote reads one raw/ entry by basename through the root, refusing
|
||||
// anything that is not a regular file.
|
||||
func readRawNote(r *os.Root, base string) ([]byte, os.FileInfo, error) {
|
||||
fi, err := r.Lstat(base)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
if !fi.Mode().IsRegular() {
|
||||
return nil, nil, errNotANote
|
||||
}
|
||||
f, err := r.Open(base)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
defer func() { _ = f.Close() }()
|
||||
b, err := io.ReadAll(f)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
return b, fi, nil
|
||||
}
|
||||
Reference in New Issue
Block a user