fix(ingestion): close the PR #156 review findings on raw/ reads

- Reverting the ingest call sites to extract.Text(path) stayed green;
  a test now fails if anything outside internal/extract calls
  extract.Text( (only TextIn, through an os.Root, may be used).
- The watcher symlink test passed for a watcher that ingested nothing,
  or stopped at the first symlink (SkipAll: one synced link halting all
  ingestion). It now plants a real note after the symlink and asserts
  exactly one ingest, and the symlinked-raw refusal has its own test.
- A FIFO swapped in between Lstat and Open hung open(2) forever.
  extract.OpenRegular opens non-blocking and checks the type on the
  handle; readRawNote, TextIn and the watcher's archive copy use it.
- The symlinked-root test covers pending, promote, reject, resolve and
  ReadIngestFile, and is named for exactly those.
- The hard-link residual is documented where it is accepted.

Out of scope, filed: #157 (symlinks elsewhere in the Syncthing-shared
brain dir reach search and other readers), #158 (writes under raw/
follow symlinks).

Refs #153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ingestion/internal/api/raw_symlink_test.go

@@ -58,7 +58,7 @@ func TestRejectNoteRefusesASymlinkedSource(t *testing.T) {
 }
 
 // Item 4: if raw/ itself is a symlink, containment inside it means nothing.
-func TestRawReadersRefuseASymlinkedRawRoot(t *testing.T) {
+func TestResolvePendingPromoteRejectAndReadIngestRefuseASymlinkedRawRoot(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))
@@ -71,4 +71,10 @@ func TestRawReadersRefuseASymlinkedRawRoot(t *testing.T) {
 	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/")
+	_, err = api.RejectNote(brain, api.RejectOptions{Filename: "x.md"})
+	assert.Error(t, err, "reject must refuse a symlinked raw/")
+	text, err := api.ReadIngestFile(brain, filepath.Join(elsewhere, "x.md"))
+	assert.Error(t, err, "ReadIngestFile must refuse a symlinked raw/")
+	assert.NotContains(t, text, sentinel153)
+	// The watcher's refusal is tested in internal/watcher.
 }

ingestion/internal/api/rawroot.go

@@ -6,6 +6,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+
+	"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
 )
 
 // errNotANote is returned for a raw/ entry that is not a regular file — in
@@ -38,6 +40,12 @@ func OpenRawRoot(brainDir string) (*os.Root, string, error) {
 
 // readRawNote reads one raw/ entry by basename through the root, refusing
 // anything that is not a regular file.
+//
+// Accepted residual: a HARD link to an outside file is a regular file and is
+// read. Syncthing syncs contents, not links, so a peer cannot create one;
+// fs.protected_hardlinks=1 and the pod's secrets live on a different
+// filesystem from the brain volume, so a local caller cannot link them in.
+// Refusing Nlink > 1 is the next step if either assumption changes.
 func readRawNote(r *os.Root, base string) ([]byte, os.FileInfo, error) {
 	fi, err := r.Lstat(base)
 	if err != nil {
@@ -46,7 +54,7 @@ func readRawNote(r *os.Root, base string) ([]byte, os.FileInfo, error) {
 	if !fi.Mode().IsRegular() {
 		return nil, nil, errNotANote
 	}
-	f, err := r.Open(base)
+	f, _, err := extract.OpenRegular(r, base)
 	if err != nil {
 		return nil, nil, err
 	}

ingestion/internal/extract/callsites_test.go

@@ -0,0 +1,43 @@
+package extract
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// PR #156 review: putting extract.Text(path) back at the ingest call sites
+// stayed green, because only TextIn itself was tested. Reading a caller-chosen
+// path by name reopens it after the containment check (hyperguild#153 item 3),
+// so outside this package only TextIn may be used.
+func TestNothingOutsideExtractCallsTextByPath(t *testing.T) {
+	root := filepath.Join("..", "..")
+	var offenders []string
+	err := filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() && (d.Name() == "vendor" || d.Name() == ".git") {
+			return filepath.SkipDir
+		}
+		if d.IsDir() || !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") ||
+			strings.Contains(filepath.ToSlash(p), "internal/extract/") {
+			return nil
+		}
+		b, err := os.ReadFile(p)
+		if err != nil {
+			return err
+		}
+		if strings.Contains(string(b), "extract.Text(") {
+			offenders = append(offenders, p)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(offenders) > 0 {
+		t.Fatalf("extract.Text( called outside internal/extract (use TextIn through an os.Root): %v", offenders)
+	}
+}

ingestion/internal/extract/extract.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"strings"
+	"syscall"
 )
 
 // Text reads the file at path and returns its plain-text content.
@@ -56,7 +57,7 @@ func TextIn(r *os.Root, rel string) (string, error) {
 	if !fi.Mode().IsRegular() {
 		return "", fmt.Errorf("%s: not a regular file", rel)
 	}
-	f, err := r.Open(rel)
+	f, _, err := OpenRegular(r, rel)
 	if err != nil {
 		return "", fmt.Errorf("open %s: %w", rel, err)
 	}
@@ -87,3 +88,25 @@ func TextIn(r *os.Root, rel string) (string, error) {
 		return "", fmt.Errorf("unsupported file extension: %s", ext)
 	}
 }
+
+// OpenRegular opens rel inside r and returns it only if the OPENED handle is
+// a regular file. The open is non-blocking, so a FIFO swapped in after an
+// Lstat (PR #156 review: it hung open(2) forever) is refused promptly rather
+// than stalling the reader. Callers keep their Lstat check, which refuses
+// symlinks outright; this closes the race between that check and the open.
+func OpenRegular(r *os.Root, rel string) (*os.File, os.FileInfo, error) {
+	f, err := r.OpenFile(rel, os.O_RDONLY|syscall.O_NONBLOCK, 0)
+	if err != nil {
+		return nil, nil, err
+	}
+	fi, err := f.Stat()
+	if err != nil {
+		_ = f.Close()
+		return nil, nil, err
+	}
+	if !fi.Mode().IsRegular() {
+		_ = f.Close()
+		return nil, nil, fmt.Errorf("%s: not a regular file", rel)
+	}
+	return f, fi, nil
+}

ingestion/internal/extract/root_test.go

@@ -3,7 +3,9 @@ package extract
 import (
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -34,3 +36,29 @@ func TestTextInReadsInsideTheRootAndRefusesEscapes(t *testing.T) {
 	_, err = TextIn(r, "../"+filepath.Base(outside))
 	assert.Error(t, err)
 }
+
+// PR #156 review: a regular file swapped for a FIFO between Lstat and Open
+// blocked open(2) forever. OpenRegular opens non-blocking and checks the type
+// on the opened handle, so a FIFO is refused promptly instead of hanging.
+func TestOpenRegularRefusesAFIFOWithoutBlocking(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "pipe.md"), 0o644))
+	r, err := os.OpenRoot(dir)
+	require.NoError(t, err)
+	defer func() { _ = r.Close() }()
+
+	done := make(chan error, 1)
+	go func() {
+		f, _, err := OpenRegular(r, "pipe.md")
+		if f != nil {
+			_ = f.Close()
+		}
+		done <- err
+	}()
+	select {
+	case err := <-done:
+		assert.Error(t, err)
+	case <-time.After(2 * time.Second):
+		t.Fatal("OpenRegular blocked on a FIFO")
+	}
+}

ingestion/internal/watcher/watcher.go

@@ -238,7 +238,7 @@ func copyFromRoot(root *os.Root, rel, dst string) error {
 	if !fi.Mode().IsRegular() {
 		return fmt.Errorf("%s: not a regular file", rel)
 	}
-	in, err := root.Open(rel)
+	in, _, err := extract.OpenRegular(root, rel)
 	if err != nil {
 		return err
 	}

ingestion/internal/watcher/watcher_test.go

@@ -289,11 +289,15 @@ func TestProcessDir_SkipsTunnelCandidateFiles(t *testing.T) {
 // files, and extract.Text followed them, so a symlink to any pod-readable
 // file would be extracted into the brain. The watcher now ingests only
 // regular files, read through an os.Root on raw/.
-func TestProcessDir_DoesNotIngestSymlinks(t *testing.T) {
+func TestProcessDir_IngestsTheRealNoteAndSkipsTheSymlinkBesideIt(t *testing.T) {
 	brainDir := setupBrainDir(t)
 	outside := filepath.Join(t.TempDir(), "secret.md")
 	require.NoError(t, os.WriteFile(outside, []byte("SENTINEL-153-WATCHER"), 0o644))
-	require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "leak.md")))
+	// "a-" sorts before "real": a walk that stopped at the first symlink
+	// (filepath.SkipAll — one synced link halting all ingestion) would fail
+	// the call count below (PR #156 review).
+	require.NoError(t, os.Symlink(outside, filepath.Join(brainDir, "raw", "a-leak.md")))
+	require.NoError(t, os.WriteFile(filepath.Join(brainDir, "raw", "real.md"), []byte("real note"), 0o644))
 
 	var sawSentinel bool
 	calls := 0
@@ -308,7 +312,25 @@ func TestProcessDir_DoesNotIngestSymlinks(t *testing.T) {
 		},
 		Schema: "# Schema",
 	}}
-	_ = processDir(context.Background(), cfg, time.Now().UTC().Format("2006-01-02"))
+	errs := processDir(context.Background(), cfg, time.Now().UTC().Format("2006-01-02"))
+	assert.Empty(t, errs)
 	assert.False(t, sawSentinel, "the symlink target must never reach the pipeline")
-	assert.Zero(t, calls, "a symlink is not a note")
+	assert.Equal(t, 1, calls, "exactly the real note is ingested; the symlink is not a note")
+}
+
+// A symlinked raw/ is refused outright: os.OpenRoot would otherwise follow
+// it and confine reads to wherever it points.
+func TestProcessDir_RefusesASymlinkedRawDir(t *testing.T) {
+	brainDir := t.TempDir()
+	elsewhere := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(elsewhere, "x.md"), []byte("SENTINEL-153-RAWLINK"), 0o644))
+	require.NoError(t, os.Symlink(elsewhere, filepath.Join(brainDir, "raw")))
+	calls := 0
+	cfg := Config{BrainDir: brainDir, Interval: time.Hour, Pipeline: pipeline.Config{
+		Complete: func(context.Context, string, string) (string, error) { calls++; return "[]", nil },
+		Schema:   "# Schema",
+	}}
+	errs := processDir(context.Background(), cfg, time.Now().UTC().Format("2006-01-02"))
+	assert.NotEmpty(t, errs, "a symlinked raw/ must be reported, not silently ingested")
+	assert.Zero(t, calls)
 }