test(extract): enforce the raw/ open discipline with the Go parser
CI / Lint / Test / Vet (pull_request) Successful in 35s
CI / Mirror to GitHub (pull_request) Skipped

PR #156 round 2: the FIFO fix lives in OpenRegular, and reverting any of
its three callers to a plain Root.Open stayed green; the extract.Text
guard was a text search that an aliased import or a func value passed.

- TestRawReadersOpenOnlyThroughOpenRegular parses rawroot.go,
  extract.go and watcher.go and fails on any .Open/.OpenFile call
  outside OpenRegular, except a named os.OpenFile log append.
- TestNoPackageReferencesExtractTextOutsideExtract resolves the extract
  import under any alias and fails on any reference to its Text,
  called or taken as a value.

Each was flipped: the three caller reverts, an os.Open revert, an
alias and a func value all go red.

Refs #153

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-03 17:03:09 +02:00
co-authored by Claude Opus 5.5
parent 87c80e4e82
commit 809420070b
+94 -18
View File
@@ -1,43 +1,119 @@
package extract
import (
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"strconv"
"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 {
const extractPkg = "github.com/mathiasbq/hyperguild/ingestion/internal/extract"
// parseNonTest parses every non-test Go file under the module.
func parseNonTest(t *testing.T) map[string]*ast.File {
t.Helper()
fset := token.NewFileSet()
out := map[string]*ast.File{}
err := filepath.WalkDir(filepath.Join("..", ".."), 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/") {
if d.IsDir() || !strings.HasSuffix(p, ".go") || strings.HasSuffix(p, "_test.go") {
return nil
}
b, err := os.ReadFile(p)
if err != nil {
return err
}
if strings.Contains(string(b), "extract.Text(") {
offenders = append(offenders, p)
f, perr := parser.ParseFile(fset, p, nil, 0)
if perr != nil {
return perr
}
out[filepath.ToSlash(p)] = f
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)
return out
}
// PR #156 review: reading a caller-chosen path by name reopens it after the
// containment check (hyperguild#153 item 3). Outside this package, nothing
// may reference extract.Text — called OR taken as a value — under whatever
// name the package is imported (round 2: an alias and a func value slipped
// past a plain text search).
func TestNoPackageReferencesExtractTextOutsideExtract(t *testing.T) {
for path, f := range parseNonTest(t) {
if strings.Contains(path, "internal/extract/") {
continue
}
names := map[string]bool{}
for _, imp := range f.Imports {
if p, _ := strconv.Unquote(imp.Path.Value); p == extractPkg {
name := "extract"
if imp.Name != nil {
name = imp.Name.Name
}
names[name] = true
}
}
if len(names) == 0 {
continue
}
ast.Inspect(f, func(n ast.Node) bool {
if sel, ok := n.(*ast.SelectorExpr); ok {
if id, ok := sel.X.(*ast.Ident); ok && names[id.Name] && sel.Sel.Name == "Text" {
t.Errorf("%s references %s.Text; use extract.TextIn through an os.Root", path, id.Name)
}
}
return true
})
}
}
// PR #156 review round 2: the FIFO fix (open non-blocking, check the handle)
// lives in OpenRegular, and reverting any caller to a plain Root.Open stayed
// green. In the raw/ readers, the only Open/OpenFile on anything is inside
// OpenRegular itself.
// osOpenAllowed names the functions in guarded files that may call os.Open*
// on a path that is not a raw/ read.
var osOpenAllowed = map[string]bool{"internal/watcher/watcher.go:appendWatcherLog": true}
func TestRawReadersOpenOnlyThroughOpenRegular(t *testing.T) {
guarded := []string{"internal/api/rawroot.go", "internal/extract/extract.go", "internal/watcher/watcher.go"}
files := parseNonTest(t)
for _, g := range guarded {
var f *ast.File
for p, af := range files {
if strings.HasSuffix(p, g) {
f = af
}
}
if f == nil {
t.Fatalf("guarded file %s not found", g)
}
for _, decl := range f.Decls {
fn, ok := decl.(*ast.FuncDecl)
if !ok || fn.Name.Name == "OpenRegular" {
continue
}
ast.Inspect(fn, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}
if sel, ok := call.Fun.(*ast.SelectorExpr); ok && (sel.Sel.Name == "Open" || sel.Sel.Name == "OpenFile") {
if id, ok := sel.X.(*ast.Ident); ok && id.Name == "os" && osOpenAllowed[g+":"+fn.Name.Name] {
return true // a named non-raw write (the ingest log append)
}
t.Errorf("%s: %s calls .%s; raw/ reads must go through OpenRegular", g, fn.Name.Name, sel.Sel.Name)
}
return true
})
}
}
}