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) 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= 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 7fe1180..7496a74 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 09f904a..d964f25 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -308,3 +308,21 @@ func TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesR got, _ = os.ReadFile(filepath.Join(dir, "n.md")) assert.Equal(t, "new", string(got), "truncated and rewritten") } + +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) +} 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) {