From 432e09d122afef58308dc8adf0e7346ffa19e070 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 06:57:45 +0200 Subject: [PATCH 1/4] test(ingestion): OIDC discovery 502s then 200 against a real issuer (#83) The retry landed in 9a232ad but was tested only through startupretry with fake funcs; nothing exercised chassisauth.NewJWTValidator's real 502 path. Extracts the main() closure into buildJWTValidator (behaviour unchanged) and tests it against an httptest issuer: 3x502 then 200 succeeds with 4 discoveries; a persistent 502 stops at exactly the 8-attempt budget with the last error; cancelling ctx mid-backoff returns context.Canceled. Flip-verified: Attempts=1 turns all three red. Lane: risk/medium issue, committed to main because the behaviour change already shipped in 9a232ad; this is a test plus a pure extraction. Closes #83 Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/jwt_validator.go | 25 +++++++ ingestion/cmd/server/jwt_validator_test.go | 86 ++++++++++++++++++++++ ingestion/cmd/server/main.go | 12 +-- 3 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 ingestion/cmd/server/jwt_validator.go create mode 100644 ingestion/cmd/server/jwt_validator_test.go diff --git a/ingestion/cmd/server/jwt_validator.go b/ingestion/cmd/server/jwt_validator.go new file mode 100644 index 0000000..002eeb6 --- /dev/null +++ b/ingestion/cmd/server/jwt_validator.go @@ -0,0 +1,25 @@ +package main + +import ( + "context" + "log/slog" + + chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" + + "github.com/mathiasbq/hyperguild/ingestion/internal/startupretry" +) + +// buildJWTValidator runs OIDC discovery under policy, so a 502 from an auth +// upstream that is not serving yet on a cold boot is retried instead of +// exiting (#83). It honours ctx between attempts. +func buildJWTValidator( + ctx context.Context, logger *slog.Logger, issuer, audience string, policy startupretry.Policy, +) (*chassisauth.JWTValidator, error) { + return startupretry.Do(ctx, policy, func(ctx context.Context) (*chassisauth.JWTValidator, error) { + v, err := chassisauth.NewJWTValidator(ctx, issuer, audience) + if err != nil { + logger.Warn("oidc discovery not ready, retrying", "err", err) + } + return v, err + }) +} diff --git a/ingestion/cmd/server/jwt_validator_test.go b/ingestion/cmd/server/jwt_validator_test.go new file mode 100644 index 0000000..1066f04 --- /dev/null +++ b/ingestion/cmd/server/jwt_validator_test.go @@ -0,0 +1,86 @@ +package main + +import ( + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/startupretry" +) + +// fakeIssuer answers OIDC discovery with 502 for the first fail requests, as +// ingress does while dex is not yet serving on a cold boot (#83), then with a +// real discovery document and an empty JWKS. +func fakeIssuer(t *testing.T, fail int32) (*httptest.Server, *atomic.Int32) { + t.Helper() + var discoveries atomic.Int32 + mux := http.NewServeMux() + var srv *httptest.Server + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { + if discoveries.Add(1) <= fail { + w.WriteHeader(http.StatusBadGateway) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"issuer":"`+srv.URL+`","jwks_uri":"`+srv.URL+`/keys"}`) + }) + mux.HandleFunc("/keys", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"keys":[]}`) + }) + srv = httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv, &discoveries +} + +var fastPolicy = startupretry.Policy{Attempts: 8, Base: time.Millisecond, Max: 5 * time.Millisecond} + +func TestBuildJWTValidatorRetriesDiscovery502sThenSucceeds(t *testing.T) { + srv, discoveries := fakeIssuer(t, 3) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + v, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy) + require.NoError(t, err) + assert.NotNil(t, v) + assert.Equal(t, int32(4), discoveries.Load(), "three 502s, then the successful discovery") +} + +func TestBuildJWTValidatorGivesUpAfterTheBudgetWithTheLastError(t *testing.T) { + srv, discoveries := fakeIssuer(t, 1000) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + _, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy) + require.Error(t, err) + assert.Contains(t, err.Error(), "status 502") + assert.Equal(t, int32(8), discoveries.Load(), "bounded: exactly the attempt budget") +} + +func TestBuildJWTValidatorStopsRetryingWhenTheContextIsCancelled(t *testing.T) { + srv, _ := fakeIssuer(t, 1000) + ctx, cancel := context.WithCancel(context.Background()) + slow := startupretry.Policy{Attempts: 8, Base: time.Hour, Max: time.Hour} + + done := make(chan error, 1) + go func() { + _, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", slow) + done <- err + }() + time.Sleep(50 * time.Millisecond) + cancel() + select { + case err := <-done: + assert.ErrorIs(t, err, context.Canceled) + case <-time.After(3 * time.Second): + t.Fatal("did not return after the context was cancelled") + } +} diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index f0fc95d..6baafc1 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -381,15 +381,9 @@ func main() { // // The budget stays bounded: retrying forever would turn a misconfiguration // into a process that looks alive and never serves. - jwtValidator, err := startupretry.Do(ctx, - startupretry.Policy{Attempts: 8, Base: time.Second, Max: 30 * time.Second}, - func(ctx context.Context) (*chassisauth.JWTValidator, error) { - v, err := chassisauth.NewJWTValidator(ctx, os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE")) - if err != nil { - logger.Warn("oidc discovery not ready, retrying", "err", err) - } - return v, err - }) + jwtValidator, err := buildJWTValidator(ctx, logger, + os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"), + startupretry.Policy{Attempts: 8, Base: time.Second, Max: 30 * time.Second}) if err != nil { logger.Error("build jwt validator", "err", err) os.Exit(1) From b6ef11600be44f74a190ddcc03df3366ab86cece Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 06:59:31 +0200 Subject: [PATCH 2/4] fix(deps): bump x/net and x/text past called vulns govulncheck 2026-10-04 found called vulns in ingestion (brain-mcp, public), reached via audit.LokiCentral.Push: golang.org/x/net v0.26.0 (GO-2026-4918 HTTP/2 infinite loop, GO-2026-5026) and golang.org/x/text v0.29.0 (GO-2026-5970). x/net -> v0.55.0, x/text -> v0.39.0; tidy also moved x/sync, x/sys, x/term. govulncheck ./... now: 0 called findings (19 in required modules, none reachable). Refs: govulncheck 2026-10-04 Co-Authored-By: Claude Opus 5.5 --- ingestion/go.mod | 12 ++++++------ ingestion/go.sum | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/ingestion/go.mod b/ingestion/go.mod index edd9b46..eabbfe0 100644 --- a/ingestion/go.mod +++ b/ingestion/go.mod @@ -34,9 +34,9 @@ require ( github.com/rogpeppe/go-internal v1.15.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/x448/float16 v0.8.4 // indirect - golang.org/x/net v0.26.0 // indirect + golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.21.0 // indirect - golang.org/x/term v0.28.0 // indirect + golang.org/x/term v0.43.0 // indirect golang.org/x/time v0.3.0 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect @@ -66,9 +66,9 @@ require ( github.com/lestrrat-go/option v1.0.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/segmentio/asm v1.2.0 // indirect - golang.org/x/crypto v0.32.0 // indirect - golang.org/x/sync v0.17.0 // indirect - golang.org/x/sys v0.31.0 // indirect - golang.org/x/text v0.29.0 // indirect + golang.org/x/crypto v0.51.0 // indirect + golang.org/x/sync v0.21.0 // indirect + golang.org/x/sys v0.45.0 // indirect + golang.org/x/text v0.39.0 // indirect gopkg.in/yaml.v3 v3.0.1 ) diff --git a/ingestion/go.sum b/ingestion/go.sum index 88aea44..290649c 100644 --- a/ingestion/go.sum +++ b/ingestion/go.sum @@ -117,42 +117,42 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.32.0 h1:euUpcYgM8WcP71gNpTqQCn6rC2t6ULUPiOzfWaXVVfc= -golang.org/x/crypto v0.32.0/go.mod h1:ZnnJkOaASj8g0AjIduWNlq2NRxL0PlBrbKVyZ6V/Ugc= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ= -golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/oauth2 v0.21.0 h1:tsimM75w1tF/uws5rbeHzIWxEqElMehnc+iW793zsZs= golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug= -golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= +golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik= -golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= -golang.org/x/term v0.28.0 h1:/Ts8HFuMR2E6IP/jlo7QVLZHggjKQbhu/7H0LJFr3Gg= -golang.org/x/term v0.28.0/go.mod h1:Sw/lC2IAUZ92udQNf3WodGtn4k/XoLyZoh8v/8uiwek= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= -golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg= -golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s= +golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= +golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From ef394352526a724c165fc39a8a04d3703210ae5b Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 07:26:12 +0000 Subject: [PATCH 3/4] fix(brain): wing indexes and the link check read through brainfs (#157, slice 5a) (#175) BuildWingIndex/BuildAllWingIndexes list wings and halls with the new brainfs.ReadDirDirs and ReadDirRegular and read titles with ReadRegular: a symlinked note no longer reaches _index.md, a symlinked wing is refused (nothing written into its target), a symlinked hall is skipped. linkcheck entryIndex walks with WalkRegular, so a symlinked .md cannot make a link resolve, and a linked tree that is not a real directory is skipped with a warning (brain_write still lands). Refs #157. Adversarial review: BLOCKED, PASS. Follow-up: #178. Co-Authored-By: Claude Opus 5.5 Co-authored-by: Mathias --- ingestion/internal/brain/index.go | 104 +++++++++------ ingestion/internal/brain/index_root_test.go | 132 ++++++++++++++++++++ ingestion/internal/brain/linkcheck.go | 49 +++++--- ingestion/internal/brainfs/brainfs.go | 21 ++++ ingestion/internal/brainfs/brainfs_test.go | 18 +++ 5 files changed, 270 insertions(+), 54 deletions(-) create mode 100644 ingestion/internal/brain/index_root_test.go diff --git a/ingestion/internal/brain/index.go b/ingestion/internal/brain/index.go index 3317bf7..4c029e1 100644 --- a/ingestion/internal/brain/index.go +++ b/ingestion/internal/brain/index.go @@ -2,12 +2,18 @@ package brain import ( "bufio" + "bytes" + "errors" "fmt" + "io" + "io/fs" "os" "path/filepath" "sort" "strings" "time" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" ) // noteEntry is one row in a Wing _index.md. @@ -27,13 +33,18 @@ func BuildWingIndex(brainDir, wing string) error { return fmt.Errorf("invalid wing %q", wing) } wingDir := filepath.Join(brainDir, "wiki", w) - if _, err := os.Stat(wingDir); os.IsNotExist(err) { + // Listed and read through the brain root (hyperguild#157): a wing or + // hall that is a symlink is not followed, and notes are regular files. + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + if _, err := bfs.Lstat("wiki/" + w); errors.Is(err, fs.ErrNotExist) { return nil - } else if err != nil { - return fmt.Errorf("stat wing: %w", err) } - entries, err := collectWingEntries(wingDir) + entries, err := collectWingEntries(bfs, "wiki/"+w) if err != nil { return err } @@ -58,51 +69,47 @@ func BuildWingIndex(brainDir, wing string) error { // BuildAllWingIndexes regenerates _index.md for every wing under brain/wiki/. func BuildAllWingIndexes(brainDir string) error { - wikiDir := filepath.Join(brainDir, "wiki") - ents, err := os.ReadDir(wikiDir) - if os.IsNotExist(err) { + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + wings, err := bfs.ReadDirDirs("wiki") + _ = bfs.Close() + if errors.Is(err, fs.ErrNotExist) { return nil } if err != nil { return fmt.Errorf("read wiki: %w", err) } - for _, e := range ents { - if !e.IsDir() { - continue - } - if err := BuildWingIndex(brainDir, e.Name()); err != nil { - return fmt.Errorf("index %s: %w", e.Name(), err) + for _, w := range wings { + if err := BuildWingIndex(brainDir, w); err != nil { + return fmt.Errorf("index %s: %w", w, err) } } return nil } -func collectWingEntries(wingDir string) ([]noteEntry, error) { +func collectWingEntries(bfs *brainfs.Brain, wingRel string) ([]noteEntry, error) { var out []noteEntry - ents, err := os.ReadDir(wingDir) + halls, err := bfs.ReadDirDirs(wingRel) if err != nil { return nil, fmt.Errorf("read wing: %w", err) } - for _, hallEnt := range ents { - if !hallEnt.IsDir() { - continue - } - hall := hallEnt.Name() + for _, hall := range halls { if !IsValidHall(hall) { continue } - hallDir := filepath.Join(wingDir, hall) - notes, err := os.ReadDir(hallDir) + hallRel := wingRel + "/" + hall + names, err := bfs.ReadDirRegular(hallRel) if err != nil { return nil, fmt.Errorf("read hall %s: %w", hall, err) } - for _, n := range notes { - if n.IsDir() || !strings.HasSuffix(n.Name(), ".md") || n.Name() == "_index.md" { + for _, name := range names { + if !strings.HasSuffix(name, ".md") || name == "_index.md" { continue } - slug := strings.TrimSuffix(n.Name(), ".md") - full := filepath.Join(hallDir, n.Name()) - title, created := readTitleAndCreated(full, slug) + slug := strings.TrimSuffix(name, ".md") + title, created := readTitleAndCreatedIn(bfs, hallRel+"/"+name, slug) out = append(out, noteEntry{Hall: hall, Slug: slug, Title: title, Created: created}) } } @@ -117,9 +124,40 @@ func readTitleAndCreated(path, slug string) (string, string) { return slug, "" } defer func() { _ = f.Close() }() + title, created := parseTitleAndCreated(f) + if created == "" { + if info, err := os.Stat(path); err == nil { + created = info.ModTime().UTC().Format("2006-01-02") + } + } + return titleOrSlug(title, slug), created +} - title, created := "", "" - scanner := bufio.NewScanner(f) +// readTitleAndCreatedIn is readTitleAndCreated through the brain root, +// regular files only (hyperguild#157). +func readTitleAndCreatedIn(bfs *brainfs.Brain, rel, slug string) (string, string) { + content, err := bfs.ReadRegular(rel) + if err != nil { + return slug, "" + } + title, created := parseTitleAndCreated(bytes.NewReader(content)) + if created == "" { + if info, err := bfs.Lstat(rel); err == nil { + created = info.ModTime().UTC().Format("2006-01-02") + } + } + return titleOrSlug(title, slug), created +} + +func titleOrSlug(title, slug string) string { + if title == "" { + return strings.ReplaceAll(slug, "-", " ") + } + return title +} + +func parseTitleAndCreated(r io.Reader) (title, created string) { + scanner := bufio.NewScanner(r) inFrontmatter := false for scanner.Scan() { line := scanner.Text() @@ -149,13 +187,5 @@ func readTitleAndCreated(path, slug string) (string, string) { } } } - if title == "" { - title = strings.ReplaceAll(slug, "-", " ") - } - if created == "" { - if info, err := os.Stat(path); err == nil { - created = info.ModTime().UTC().Format("2006-01-02") - } - } return title, created } diff --git a/ingestion/internal/brain/index_root_test.go b/ingestion/internal/brain/index_root_test.go new file mode 100644 index 0000000..6efb2cc --- /dev/null +++ b/ingestion/internal/brain/index_root_test.go @@ -0,0 +1,132 @@ +package brain_test + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brain" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest" +) + +func readIndex(t *testing.T, brainDir, wing string) string { + t.Helper() + b, _ := os.ReadFile(filepath.Join(brainDir, "wiki", wing, "_index.md")) + return string(b) +} + +// hyperguild#157 slice 5: BuildWingIndex listed wiki/// with +// os.ReadDir and opened each note by path, so a symlinked note put the +// target's title into _index.md, and a FIFO hung the index build. +func TestBuildWingIndexRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) { + b := t.TempDir() + facts := filepath.Join(b, "wiki", "w", "facts") + require.NoError(t, os.MkdirAll(facts, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: zqinside\n---\n"), 0o644)) + require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) + inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(b, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md"))) + + require.NoError(t, brain.BuildWingIndex(b, "w")) + idx := readIndex(t, b, "w") + assert.Contains(t, idx, "zqinside", "positive control") + assert.NotContains(t, idx, brainfstest.Sentinel) + assert.NotContains(t, idx, "leak") + assert.NotContains(t, idx, "inlink") + + p := filepath.Join(facts, "r.md") + read := func() string { + _ = brain.BuildWingIndex(b, "w") + return readIndex(t, b, "w") + } + brainfstest.AssertNoLeak(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, read) + brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: zqinside\n---\n", inBrain, 1500*time.Millisecond, read) + brainfstest.AssertNoHang(t, p, "---\ntitle: zqinside\n---\n", 1500*time.Millisecond, 6*time.Second, func() { _ = read() }) +} + +// A symlinked hall is skipped while a real hall next to it is indexed; a +// symlinked wing is refused and nothing is written into its target. +func TestBuildWingIndexSkipsASymlinkedHallAndRefusesASymlinkedWing(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "raw", "facts", "r.md"), []byte("---\ntitle: zqrawonly\n---\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "decisions"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "decisions", "d.md"), []byte("---\ntitle: zqrealhall\n---\n"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "facts"), filepath.Join(b, "wiki", "w", "facts"))) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(b, "wiki", "alias"))) + + require.NoError(t, brain.BuildWingIndex(b, "w")) + idx, err := os.ReadFile(filepath.Join(b, "wiki", "w", "_index.md")) + require.NoError(t, err, "the wing index is written") + assert.Contains(t, string(idx), "zqrealhall", "the real hall is indexed") + assert.NotContains(t, string(idx), "zqrawonly", "the symlinked hall is not") + + assert.Error(t, brain.BuildWingIndex(b, "alias")) + _, err = os.Stat(filepath.Join(b, "raw", "_index.md")) + assert.True(t, os.IsNotExist(err), "an index was written into the symlinked wing's target") +} + +// BuildAllWingIndexes indexes the real wings and skips a symlinked one. +func TestBuildAllWingIndexesSkipsASymlinkedWing(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "facts", "n.md"), []byte("---\ntitle: zqrealwing\n---\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "facts"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(b, "wiki", "alias"))) + + require.NoError(t, brain.BuildAllWingIndexes(b)) + idx, err := os.ReadFile(filepath.Join(b, "wiki", "w", "_index.md")) + require.NoError(t, err) + assert.Contains(t, string(idx), "zqrealwing") + _, err = os.Stat(filepath.Join(b, "raw", "_index.md")) + assert.True(t, os.IsNotExist(err)) +} + +// A link to a symlinked note does not resolve: only regular files count as +// entries, so StripUnresolvedLinks unwraps it. +func TestStripUnresolvedLinksDoesNotCountASymlinkedNoteAsAnEntry(t *testing.T) { + b := t.TempDir() + facts := filepath.Join(b, "wiki", "w", "facts") + require.NoError(t, os.MkdirAll(facts, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(facts, "real.md"), []byte("x"), 0o644)) + brainfstest.WriteSentinelAt(t, filepath.Join(b, "raw", "s.md")) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "ghost.md"))) + out, unlinked, err := brain.StripUnresolvedLinks(b, "see [[real]] and [[ghost]]") + require.NoError(t, err) + assert.Contains(t, out, "[[real]]", "positive control") + assert.NotContains(t, out, "[[ghost]]") + assert.Contains(t, unlinked, "ghost") +} + +// A linked tree (knowledge/ or wiki/) that is not a real directory is +// skipped, not an error: StripUnresolvedLinks runs before every brain_write, +// and an error there loses the learning. +func TestStripUnresolvedLinksSkipsALinkedTreeThatIsNotARealDirectory(t *testing.T) { + for name, mk := range map[string]func(t *testing.T, b string){ + "symlink to an in-brain dir": func(t *testing.T, b string) { + require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "k"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("raw", "k"), filepath.Join(b, "knowledge"))) + }, + "dangling symlink": func(t *testing.T, b string) { + require.NoError(t, os.Symlink("nowhere", filepath.Join(b, "knowledge"))) + }, + "regular file": func(t *testing.T, b string) { + require.NoError(t, os.WriteFile(filepath.Join(b, "knowledge"), []byte("x"), 0o644)) + }, + } { + t.Run(name, func(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "facts", "real.md"), []byte("x"), 0o644)) + mk(t, b) + out, _, err := brain.StripUnresolvedLinks(b, "learning [[real]] and [[gone]]") + require.NoError(t, err) + assert.Contains(t, out, "[[real]]", "links into wiki/ still resolve") + assert.NotContains(t, out, "[[gone]]") + }) + } +} diff --git a/ingestion/internal/brain/linkcheck.go b/ingestion/internal/brain/linkcheck.go index d81d08b..1553015 100644 --- a/ingestion/internal/brain/linkcheck.go +++ b/ingestion/internal/brain/linkcheck.go @@ -1,12 +1,16 @@ package brain import ( + "errors" "fmt" - "os" - "path/filepath" + "io/fs" + "log/slog" + "path" "regexp" "sort" "strings" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" ) // wikilinkRE matches [[target]] and [[target|Display Text]]. @@ -98,29 +102,40 @@ var linkedTrees = []string{"knowledge", "wiki"} // one the audit will also resolve. func entryIndex(brainDir string) (map[string]struct{}, error) { index := map[string]struct{}{} + // Walked through the brain root (hyperguild#157): only regular files + // count as entries, so a symlinked .md name cannot make a link resolve. + bfs, err := brainfs.Open(brainDir) + if err != nil { + return nil, fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() for _, dir := range linkedTrees { - root := filepath.Join(brainDir, dir) - if _, err := os.Stat(root); os.IsNotExist(err) { + if _, err := bfs.Lstat(dir); errors.Is(err, fs.ErrNotExist) { continue - } else if err != nil { - return nil, fmt.Errorf("stat %s: %w", dir, err) } - err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - if d.IsDir() || !strings.HasSuffix(path, ".md") { + var walkErr error + err := bfs.WalkRegular(dir, func(rel string) error { + if !strings.HasSuffix(rel, ".md") { return nil } - rel, err := filepath.Rel(brainDir, path) - if err != nil { - return err - } - rel = filepath.ToSlash(rel) - index[strings.TrimSuffix(filepath.Base(rel), ".md")] = struct{}{} + index[strings.TrimSuffix(path.Base(rel), ".md")] = struct{}{} index[strings.TrimPrefix(strings.TrimSuffix(rel, ".md"), "wiki/")] = struct{}{} return nil + }, func(rel string, err error) { + // A linked tree that is not a real directory (a symlink, a + // file) is skipped, as filepath.WalkDir did: this runs before + // every brain_write, and an error here loses the learning. + if rel == dir && errors.Is(err, brainfs.ErrNotRegular) { + slog.Warn("link check: skipping a linked tree that is not a real directory", "dir", dir) + return + } + if walkErr == nil { + walkErr = fmt.Errorf("%s: %w", rel, err) + } }) + if err == nil { + err = walkErr + } if err != nil { return nil, fmt.Errorf("walk %s: %w", dir, err) } diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index fb1b76c..4feca7c 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -104,6 +104,27 @@ func (b *Brain) ReadDirRegular(relDir string) ([]string, error) { return names, nil } +// ReadDirDirs lists the real subdirectories directly in a brain-relative +// directory, by name. Every component of relDir must be a real directory +// (ErrNotRegular otherwise); symlinks to directories are left out. +func (b *Brain) ReadDirDirs(relDir string) ([]string, error) { + relDir = filepath.ToSlash(relDir) + if err := b.realDir(relDir); err != nil { + return nil, err + } + ents, err := fs.ReadDir(b.root.FS(), relDir) + if err != nil { + return nil, err + } + var names []string + for _, e := range ents { + if e.Type().IsDir() { + names = append(names, e.Name()) + } + } + return names, nil +} + // ReadRegular reads a brain-relative path, refusing anything that is not a // regular file, both before the open and on the opened handle. func (b *Brain) ReadRegular(rel string) ([]byte, error) { diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 7146af8..76ea7d1 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -262,3 +262,21 @@ func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing. assert.Zero(t, leaks) assert.Positive(t, ok, "positive control: the regular file is read") } + +func TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "a", "real"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "other"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "other"), filepath.Join(dir, "a", "linked"))) + require.NoError(t, os.WriteFile(filepath.Join(dir, "a", "f.md"), []byte("f"), 0o644)) + require.NoError(t, os.Symlink("a", filepath.Join(dir, "alias"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + names, err := b.ReadDirDirs("a") + require.NoError(t, err) + assert.Equal(t, []string{"real"}, names) + _, err = b.ReadDirDirs("alias") + assert.ErrorIs(t, err, ErrNotRegular) +} From 0bd0fbf03eff9a79d61fde62d7b0593051d76741 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 09:32:25 +0200 Subject: [PATCH 4/4] test(brainfstest): swap-race asserts fail when nothing was tested AssertNoLeakFrom and the search Query race test passed vacuously when every symlink swap failed or every read errored. Both now count successful symlink swaps and reads that returned the inside note and fail if either is zero; the Query race reader no longer swallows Query's error. Verified by flipping: a swapper whose os.Symlink always fails turns notes, wiki and search race tests red; Query on a missing brain dir turns the search race test red. Closes #177 Co-Authored-By: Claude Opus 5.5 --- .../brainfs/brainfstest/brainfstest.go | 50 +++++++++--- .../brainfs/brainfstest/brainfstest_test.go | 79 +++++++++++++++++++ ingestion/internal/search/brainroot_test.go | 35 +++++--- 3 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 ingestion/internal/brainfs/brainfstest/brainfstest_test.go diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest.go b/ingestion/internal/brainfs/brainfstest/brainfstest.go index fdfe968..c352194 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -27,7 +27,7 @@ const outsideContent = "---\ntitle: " + Sentinel + "\naliases:\n - " + Sentinel "\n\n" + Sentinel + " [[" + Sentinel + "]]\n" // Outside writes a file outside any brain dir containing Sentinel. -func Outside(t *testing.T) string { +func Outside(t testing.TB) string { t.Helper() p := filepath.Join(t.TempDir(), "secret.md") if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { @@ -42,9 +42,12 @@ var swapSeq atomic.Int64 // a symlink to outside or a FIFO into path, until the test ends. FIFOs get a // writer held open until the swapper exits, so a reader that skips the // handle check parks forever instead of reading EOF. The swapper is stopped -// and awaited before t.TempDir cleanup. -func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { +// and awaited before t.TempDir cleanup. It returns the number of symlinks +// or FIFOs actually renamed into path so far, so a caller can tell a race +// that ran from one whose every swap failed (hyperguild#177). +func StartSwap(t testing.TB, path, inside, outside string, fifo bool) *atomic.Int64 { t.Helper() + var swaps atomic.Int64 var stop atomic.Bool var wg sync.WaitGroup wg.Add(1) @@ -67,22 +70,26 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { }() for i := 0; !stop.Load(); i++ { _ = os.Remove(tmp) + var made error switch { case i%2 == 0: _ = os.WriteFile(tmp, []byte(inside), 0o644) case fifo: - if err := syscall.Mkfifo(tmp, 0o644); err == nil { + if made = syscall.Mkfifo(tmp, 0o644); made == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { writers = append(writers, w) } } default: - _ = os.Symlink(link, tmp) + made = os.Symlink(link, tmp) + } + if err := os.Rename(tmp, path); err == nil && made == nil && i%2 == 1 { + swaps.Add(1) } - _ = os.Rename(tmp, path) } }() t.Cleanup(func() { stop.Store(true); wg.Wait() }) + return &swaps } // WriteSentinelAt writes a file carrying Sentinel at p (creating its @@ -107,18 +114,41 @@ func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func( AssertNoLeakFrom(t, path, inside, Outside(t), d, read) } +// InsideMarker must appear in the inside note's content and in read's +// output whenever read saw that note: the floor below counts such reads. +const InsideMarker = "inside" + // AssertNoLeakFrom is AssertNoLeak with the symlink pointing at target, e.g. // a file elsewhere in the brain written with WriteSentinelAt. -func AssertNoLeakFrom(t *testing.T, path, inside, target string, d time.Duration, read func() string) { +// +// Beyond "no Sentinel in any output" it sets two floors, because without +// them the assertion passes when nothing was tested (hyperguild#177): at +// least one symlink must have been swapped into path, and at least one read +// must have returned InsideMarker (a reader that always errors returns +// nothing, and so never leaks). +func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration, read func() string) { t.Helper() - StartSwap(t, path, inside, target, false) - leaks, iters := 0, 0 + if !strings.Contains(inside, InsideMarker) { + t.Fatalf("inside content must contain %q", InsideMarker) + } + swaps := StartSwap(t, path, inside, target, false) + leaks, iters, insideReads := 0, 0, 0 for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ { - leaks += strings.Count(read(), Sentinel) + out := read() + leaks += strings.Count(out, Sentinel) + if strings.Contains(out, InsideMarker) { + insideReads++ + } } if leaks > 0 { t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) } + if n := swaps.Load(); n == 0 { + t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters) + } + if insideReads == 0 { + t.Fatalf("no read in %d iterations returned the inside note (%q): the reader read nothing", iters, InsideMarker) + } } // AssertNoHang loops read for d while a FIFO swapper (held writers) runs on diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest_test.go b/ingestion/internal/brainfs/brainfstest/brainfstest_test.go new file mode 100644 index 0000000..7dfc857 --- /dev/null +++ b/ingestion/internal/brainfs/brainfstest/brainfstest_test.go @@ -0,0 +1,79 @@ +package brainfstest + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "syscall" + "testing" + "time" +) + +// recordingTB stands in for the test under AssertNoLeakFrom so a fatal +// there can be observed instead of failing this test. Fatalf ends the +// calling goroutine the way testing.T does. +type recordingTB struct { + testing.TB + failed bool + msg string +} + +func (r *recordingTB) Fatalf(format string, args ...any) { + r.failed, r.msg = true, fmt.Sprintf(format, args...) + runtime.Goexit() +} + +func runAssert(t *testing.T, path, target string, read func() string) *recordingTB { + t.Helper() + r := &recordingTB{TB: t} + done := make(chan struct{}) + go func() { + defer close(done) + AssertNoLeakFrom(r, path, "inside", target, 200*time.Millisecond, read) + }() + <-done + return r +} + +// hyperguild#177: with every swap failing, no leak can occur, so the leak +// assertion passed without testing anything. +func TestAssertNoLeakFromFailsWhenNoSymlinkSwapSucceeds(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing-dir", "r.md") + r := runAssert(t, path, Outside(t), func() string { return "inside" }) + if !r.failed { + t.Fatal("AssertNoLeakFrom passed with zero successful symlink swaps") + } +} + +// hyperguild#177: a reader that always errors returns nothing, so it never +// leaks either. +func TestAssertNoLeakFromFailsWhenNoReadReturnsTheInsideNote(t *testing.T) { + path := filepath.Join(t.TempDir(), "r.md") + r := runAssert(t, path, Outside(t), func() string { return "" }) + if !r.failed { + t.Fatal("AssertNoLeakFrom passed with zero reads returning the inside note") + } +} + +// Positive control: a reader that refuses symlinks and reads regular files +// passes, so the floors do not fail a working reader. +func TestAssertNoLeakFromPassesAReaderThatRefusesSymlinks(t *testing.T) { + path := filepath.Join(t.TempDir(), "r.md") + read := func() string { + f, err := os.OpenFile(path, os.O_RDONLY|noFollow, 0) + if err != nil { + return "" + } + defer func() { _ = f.Close() }() + b := make([]byte, 64) + n, _ := f.Read(b) + return string(b[:n]) + } + r := runAssert(t, path, Outside(t), read) + if r.failed { + t.Fatalf("a symlink-refusing reader failed: %s", r.msg) + } +} + +const noFollow = syscall.O_NOFOLLOW diff --git a/ingestion/internal/search/brainroot_test.go b/ingestion/internal/search/brainroot_test.go index 6acb636..7c70ad0 100644 --- a/ingestion/internal/search/brainroot_test.go +++ b/ingestion/internal/search/brainroot_test.go @@ -93,16 +93,20 @@ func TestQueryDoesNotFollowASymlinkedWingUnderAWingAndHallRoot(t *testing.T) { // bypass of readOpened hung Query after 5 iterations). // startSwap runs swapInto until the test ends and waits for it before // t.TempDir's cleanup (hyperguild#162). -func startSwap(t *testing.T, path, outside string, fifo bool) { +// It returns the count of symlinks or FIFOs actually renamed into path +// (hyperguild#177: a swapper whose every swap fails tests nothing). +func startSwap(t *testing.T, path, outside string, fifo bool) *atomic.Int64 { t.Helper() var stop atomic.Bool + var swaps atomic.Int64 var wg sync.WaitGroup wg.Add(1) - go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop) }() + go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop, &swaps) }() t.Cleanup(func() { stop.Store(true); wg.Wait() }) + return &swaps } -func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { +func swapInto(path, outside string, fifo bool, stop *atomic.Bool, swaps *atomic.Int64) { tmp := path + ".swap" // Writers stay open until the swapper exits, so a reader parked on a // FIFO stays parked (closing one would hand the reader EOF and hide the @@ -115,33 +119,46 @@ func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { }() for i := 0; !stop.Load(); i++ { _ = os.Remove(tmp) + var made error switch { case i%2 == 0: _ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644) case fifo: - if err := syscall.Mkfifo(tmp, 0o644); err == nil { + if made = syscall.Mkfifo(tmp, 0o644); made == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { writers = append(writers, w) } } default: - _ = os.Symlink(outside, tmp) + made = os.Symlink(outside, tmp) + } + if err := os.Rename(tmp, path); err == nil && made == nil && i%2 == 1 { + swaps.Add(1) } - _ = os.Rename(tmp, path) } } func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) { brain, facts, outside := searchBrain(t) - startSwap(t, filepath.Join(facts, "r.md"), outside, false) - leaks, iters := 0, 0 + swaps := startSwap(t, filepath.Join(facts, "r.md"), outside, false) + leaks, iters, insideReads := 0, 0, 0 for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ { - rs, _ := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) + rs, err := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) + require.NoError(t, err) leaks += leaked(rs) + // Positive control (hyperguild#177): a Query that always errors + // returns nothing and so never leaks. + rs, err = Query(brain, QueryOptions{Query: "zqracenote", Limit: 10}) + require.NoError(t, err) + if len(rs) > 0 { + insideReads++ + } } if leaks > 0 { t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) } + require.Positive(t, swaps.Load(), "no symlink was swapped in: the race did not run") + require.Positive(t, insideReads, "no Query returned the inside note") } func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) {