PR #156 round 3: a readRawNote rewrite to os.ReadFile or Root.ReadFile passed the lint and brought back the escape (19k leaks in 4s) and the FIFO hang. Listing forbidden APIs is whack-a-mole, so the control now rests on behaviour: - TestRawReadersLeakNothingUnderASymlinkSwapRace and TestRawReadersDoNotHangUnderAFIFOSwapRace swap a regular note, a symlink to an outside file, or a FIFO into raw/r.md while ListPending and ReadIngestFile loop for 1.5s; any leaked byte or a hang fails. - TestProcessDir_DoesNotHangUnderAFIFOSwapRace does the same for the unattended watcher (read and archive copy). Flipped: os.ReadFile -> leak + hang tests red; Root.ReadFile -> hang test red; copyFromRoot back to Root.Open -> watcher test red. The lint stays as a second layer: it also forbids ReadFile, matches the selector (so a stored method value is caught), refuses a dot-import of extract, and says it only covers the three files it parses. Refs #153 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
131 lines
4.0 KiB
Go
131 lines
4.0 KiB
Go
package extract
|
|
|
|
import (
|
|
"go/ast"
|
|
"go/parser"
|
|
"go/token"
|
|
"os"
|
|
"path/filepath"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
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") {
|
|
return nil
|
|
}
|
|
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)
|
|
}
|
|
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
|
|
}
|
|
if name == "." {
|
|
t.Errorf("%s dot-imports extract; import it by name so its use stays checkable", path)
|
|
}
|
|
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 three raw/ reader FILES parsed below (and only those: a
|
|
// wrapper in another file is not seen), no Open/OpenFile/ReadFile selector
|
|
// may appear outside OpenRegular, called or taken as a method value. This is
|
|
// a second layer; the behavioural race tests in internal/api and
|
|
// internal/watcher are what catch a rewrite through any API (round 3).
|
|
// 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, // ingest log append, not a raw/ read
|
|
// Text reads by path; outside this package only TextIn's temp copy reaches
|
|
// it (TestNoPackageReferencesExtractTextOutsideExtract enforces that).
|
|
"internal/extract/extract.go:Text": true,
|
|
}
|
|
|
|
func TestRawReaderFilesUseNoOpenOrReadFileOutsideOpenRegular(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 {
|
|
sel, ok := n.(*ast.SelectorExpr)
|
|
if !ok {
|
|
return true
|
|
}
|
|
if sel.Sel.Name == "Open" || sel.Sel.Name == "OpenFile" || sel.Sel.Name == "ReadFile" {
|
|
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
|
|
})
|
|
}
|
|
}
|
|
}
|