Compare commits

..
2 Commits
Author SHA1 Message Date
mathiasandClaude Opus 5.5 3cfe345415 test(pipeline): pin BackfillRefs' page read with a swap race (#157)
CI / Lint / Test / Vet (pull_request) Successful in 54s
CI / Mirror to GitHub (pull_request) Skipped
Review round 1 on PR #176: reverting the BackfillRefs concept-page read
to os.ReadFile left every test green. The static symlinked page never
reaches that read, because LoadInventory does not list it; the read is
reachable only when a page listed as regular is swapped for a symlink
before it is read. New test does that against an in-brain target and
reads the page back with O_NOFOLLOW; red under the flip (2/2), green
3/3 on head.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 09:27:38 +02:00
mathiasandClaude Opus 5.5 70df6db7a0 fix(pipeline,classification): read the brain through brainfs (#157, slice 6)
CI / Lint / Test / Vet (pull_request) Successful in 1m3s
CI / Mirror to GitHub (pull_request) Skipped
The ingest pipeline read wiki/sources/<slug>.md and schema.md (both go
into the LLM prompt), the pages it revises, and the pages it adds
source refs to, with os.ReadFile; BackfillRefs listed wiki/sources with
os.ReadDir. A symlinked file's content reached the prompt and the
written pages (1.6M leaks in the race), and a FIFO hung the ingest.
classification.Load (the I1 sovereignty levels) read
classification.yaml the same way and hung startup on a FIFO.

- pipeline: readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs
  read through readBrainFile (brainfs ReadRegular); reviseOrCreate takes
  the brain dir and reads page.Path, a missing page is still created and
  a non-regular one is an error. BackfillRefs lists with ReadDirRegular
  and reads with ReadRegular.
- classification.Load: read through brainfs; a symlinked or special
  classification.yaml is a hard error (as a malformed one is); an absent
  file or brain dir still yields the default config.

Tests use in-brain targets and the [[slug|title]] link form the pipeline
actually follows (bare [[slug]] links are ignored by it, which made a
first draft of the backfill test vacuous).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 07:13:39 +02:00
27 changed files with 437 additions and 1621 deletions
+15 -16
View File
@@ -2,25 +2,24 @@ package main
import ( import (
"context" "context"
"fmt"
"log/slog" "log/slog"
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
) )
// buildJWTValidator builds the JWT validator for issuer (nil when issuer is // buildJWTValidator runs OIDC discovery under policy, so a 502 from an auth
// empty: JWT auth disabled). Since mcp-chassis v0.7 construction no longer // upstream that is not serving yet on a cold boot is retried instead of
// fails when OIDC discovery does (mcp-chassis#4): the issuer resolves lazily, // exiting (#83). It honours ctx between attempts.
// and until it does a JWT is answered 503-and-retry, not 401. That replaces func buildJWTValidator(
// #83's boot-time retry loop (hyperguild#155). An issuer still unresolved at ctx context.Context, logger *slog.Logger, issuer, audience string, policy startupretry.Policy,
// boot is logged, so a misconfigured issuer is visible rather than silent. ) (*chassisauth.JWTValidator, error) {
func buildJWTValidator(ctx context.Context, logger *slog.Logger, issuer, audience string) (*chassisauth.JWTValidator, error) { return startupretry.Do(ctx, policy, func(ctx context.Context) (*chassisauth.JWTValidator, error) {
v, err := chassisauth.NewJWTValidator(ctx, issuer, audience) v, err := chassisauth.NewJWTValidator(ctx, issuer, audience)
if err != nil { if err != nil {
return nil, fmt.Errorf("build jwt validator: %w", err) logger.Warn("oidc discovery not ready, retrying", "err", err)
} }
if ok, rerr := v.Ready(ctx); !ok { return v, err
logger.Warn("jwt issuer not resolved at boot; JWTs answer 503 until it is", "issuer", issuer, "err", rerr) })
}
return v, nil
} }
+36 -61
View File
@@ -1,7 +1,6 @@
package main package main
import ( import (
"bytes"
"context" "context"
"io" "io"
"log/slog" "log/slog"
@@ -9,11 +8,12 @@ import (
"net/http/httptest" "net/http/httptest"
"sync/atomic" "sync/atomic"
"testing" "testing"
"time"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics" "github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
) )
// fakeIssuer answers OIDC discovery with 502 for the first fail requests, as // fakeIssuer answers OIDC discovery with 502 for the first fail requests, as
@@ -41,71 +41,46 @@ func fakeIssuer(t *testing.T, fail int32) (*httptest.Server, *atomic.Int32) {
return srv, &discoveries return srv, &discoveries
} }
// #83 retried discovery at boot because chassis v0.2.0 failed construction on var fastPolicy = startupretry.Policy{Attempts: 8, Base: time.Millisecond, Max: 5 * time.Millisecond}
// a 502. Since chassis v0.7 (mcp-chassis#4) the validator is built anyway and
// resolves the issuer lazily, so boot no longer waits or exits on it func TestBuildJWTValidatorRetriesDiscovery502sThenSucceeds(t *testing.T) {
// (hyperguild#155). An unresolved issuer is logged at boot. srv, discoveries := fakeIssuer(t, 3)
func TestBuildJWTValidatorReturnsAValidatorWhileDiscovery502s(t *testing.T) { ctx, cancel := context.WithCancel(context.Background())
srv, _ := fakeIssuer(t, 1000) defer cancel()
var buf bytes.Buffer
v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain") v, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy)
require.NoError(t, err) require.NoError(t, err)
assert.NotNil(t, v) assert.NotNil(t, v)
assert.Contains(t, buf.String(), "jwt issuer not resolved at boot") assert.Equal(t, int32(4), discoveries.Load(), "three 502s, then the successful discovery")
} }
func TestBuildJWTValidatorLogsNothingAgainstAHealthyIssuer(t *testing.T) { func TestBuildJWTValidatorGivesUpAfterTheBudgetWithTheLastError(t *testing.T) {
srv, _ := fakeIssuer(t, 0) srv, discoveries := fakeIssuer(t, 1000)
var buf bytes.Buffer ctx, cancel := context.WithCancel(context.Background())
v, err := buildJWTValidator(context.Background(), slog.New(slog.NewTextHandler(&buf, nil)), srv.URL, "brain") defer cancel()
require.NoError(t, err)
assert.NotNil(t, v) _, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", fastPolicy)
assert.NotContains(t, buf.String(), "not resolved") require.Error(t, err)
assert.Contains(t, err.Error(), "status 502")
assert.Equal(t, int32(8), discoveries.Load(), "bounded: exactly the attempt budget")
} }
func TestBuildJWTValidatorIsNilWithoutAnIssuer(t *testing.T) { func TestBuildJWTValidatorStopsRetryingWhenTheContextIsCancelled(t *testing.T) {
v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), "", "brain")
require.NoError(t, err)
assert.Nil(t, v)
}
// With the issuer unresolved, a JWT on a REST route answers 503 (retry), not
// 401: the token may be fine, the server just cannot check it yet.
func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(t *testing.T) {
srv, _ := fakeIssuer(t, 1000) srv, _ := fakeIssuer(t, 1000)
v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), srv.URL, "brain") ctx, cancel := context.WithCancel(context.Background())
require.NoError(t, err) slow := startupretry.Policy{Attempts: 8, Base: time.Hour, Max: time.Hour}
mux, err := buildMux(restAuth{token: routesTok, validator: v, realm: "brain"}, testHandler(t),
okHandler("mcp"), metrics.New(), nil)
require.NoError(t, err)
code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc")
assert.Equal(t, http.StatusServiceUnavailable, code)
}
// PR #183 review: /mcp shares REST's bearer, but only the static token was done := make(chan error, 1)
// asserted on it. With the issuer unresolved, a JWT on /mcp answers 503 too, go func() {
// so /mcp's middleware was built with the JWT validator. _, err := buildJWTValidator(ctx, slog.New(slog.DiscardHandler), srv.URL, "brain", slow)
func TestBuildMuxAnswers503ToAJWTOnMCPWhileTheIssuerIsUnresolved(t *testing.T) { done <- err
srv, _ := fakeIssuer(t, 1000) }()
v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), srv.URL, "brain") time.Sleep(50 * time.Millisecond)
require.NoError(t, err) cancel()
mux, err := buildMux(restAuth{token: routesTok, validator: v, realm: "brain"}, testHandler(t), select {
okHandler("mcp"), metrics.New(), nil) case err := <-done:
require.NoError(t, err) assert.ErrorIs(t, err, context.Canceled)
code, _ := do(t, mux, "POST", "/mcp", "Bearer aaa.bbb.ccc") case <-time.After(3 * time.Second):
assert.Equal(t, http.StatusServiceUnavailable, code) t.Fatal("did not return after the context was cancelled")
} }
// PR #183 review: a 401 on /mcp names the protected-resource metadata in
// WWW-Authenticate (what the claude.ai OAuth connector discovers from), so
// /mcp's middleware was built with resourceMetadataURL.
func TestBuildMuxMCP401CarriesResourceMetadataInWWWAuthenticate(t *testing.T) {
const rm = "https://brain.example/.well-known/oauth-protected-resource"
mux, err := buildMux(restAuth{token: routesTok, realm: "brain", resourceMetadataURL: rm}, testHandler(t),
okHandler("mcp"), metrics.New(), nil)
require.NoError(t, err)
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/mcp", nil))
require.Equal(t, http.StatusUnauthorized, rec.Code)
assert.Contains(t, rec.Header().Get("WWW-Authenticate"), `resource_metadata="`+rm+`"`)
} }
+32 -22
View File
@@ -32,6 +32,7 @@ import (
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
"github.com/mathiasbq/hyperguild/ingestion/internal/reranker" "github.com/mathiasbq/hyperguild/ingestion/internal/reranker"
"github.com/mathiasbq/hyperguild/ingestion/internal/search" "github.com/mathiasbq/hyperguild/ingestion/internal/search"
"github.com/mathiasbq/hyperguild/ingestion/internal/startupretry"
"github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard" "github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard"
"github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore" "github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore"
"github.com/mathiasbq/hyperguild/ingestion/internal/watcher" "github.com/mathiasbq/hyperguild/ingestion/internal/watcher"
@@ -368,15 +369,21 @@ func main() {
// hyperguild#157: report a symlink that appears in the brain while running. // hyperguild#157: report a symlink that appears in the brain while running.
go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier()) go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier())
// Never fatal on an unreachable issuer (#83): on a cold cluster boot this // Built before the watcher, the graph backfill and session ingestion, and
// pod can beat dex to serving and get a 502 from ingress, and exiting meant // retried rather than fatal (#83). Two separate reasons:
// CrashLoopBackOff redoing the backfill. Since mcp-chassis v0.7 the //
// validator resolves the issuer lazily and answers JWTs 503-and-retry until // - On a cold cluster boot this pod can beat dex to serving and gets a
// it does (hyperguild#155), so no boot retry is needed. A misconfigured // 502 from ingress. Nothing is misconfigured; it simply started first.
// issuer no longer exits the process: it is logged at boot and every JWT // Exiting there meant CrashLoopBackOff redoing the backfill on every
// gets 503; the static bearer keeps working. // attempt, with exponential backoff stretching the brain outage.
// - Constructing it first means a genuine config error (wrong issuer,
// bad audience) fails in seconds instead of after all that work.
//
// The budget stays bounded: retrying forever would turn a misconfiguration
// into a process that looks alive and never serves.
jwtValidator, err := buildJWTValidator(ctx, logger, jwtValidator, err := buildJWTValidator(ctx, logger,
os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE")) os.Getenv("DEX_ISSUER_URL"), os.Getenv("MCP_AUDIENCE"),
startupretry.Policy{Attempts: 8, Base: time.Second, Max: 30 * time.Second})
if err != nil { if err != nil {
logger.Error("build jwt validator", "err", err) logger.Error("build jwt validator", "err", err)
os.Exit(1) os.Exit(1)
@@ -516,6 +523,8 @@ func main() {
} }
} }
public["/mcp"] = chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, mcpSrv)
// POST /capture (#53/#54): the uniform capture REST door. Needs a ticket // POST /capture (#53/#54): the uniform capture REST door. Needs a ticket
// tracker to file action items, so it only mounts when Gitea is // tracker to file action items, so it only mounts when Gitea is
// configured. It reuses the MCP server's graph-wired brain store (one // configured. It reuses the MCP server's graph-wired brain store (one
@@ -539,7 +548,7 @@ func main() {
public["POST /capture"] = captureH public["POST /capture"] = captureH
// Same use-case behind the MCP `capture` tool (#55 relay) so MCP-native // Same use-case behind the MCP `capture` tool (#55 relay) so MCP-native
// harnesses (claude.ai, Crush, Pi, LLM Council) reach capture through // harnesses (claude.ai, Crush, Pi, LLM Council) reach capture through
// the existing /mcp OAuth connector. buildMux wraps this same mcpSrv; // the existing /mcp OAuth connector. mcpSrv is already wrapped above;
// WithCapture mutates the same instance, so the tool appears live. // WithCapture mutates the same instance, so the tool appears live.
mcpSrv.WithCapture(captureSvc, jwtValidator, mcpToken, "local-cli", resolver) mcpSrv.WithCapture(captureSvc, jwtValidator, mcpToken, "local-cli", resolver)
// hyperguild#97: a governance-file rewrite gets the same trail a // hyperguild#97: a governance-file rewrite gets the same trail a
@@ -581,18 +590,19 @@ func main() {
os.Exit(1) os.Exit(1)
} }
// /metrics — unauthenticated Prometheus endpoint, scraped via the // /metrics — unauthenticated Prometheus endpoint. kube-prometheus-stack
// ServiceMonitor in k3s/apps/supervisor/. buildMux registers it and wraps // scrapes it via the ServiceMonitor in k3s/apps/supervisor/. The metrics
// the whole router in the registry's middleware. // middleware below wraps every other registered handler so it observes
// // real request latency. /metrics itself is excluded from its own
// The whole router, /mcp's bearer included, is built in buildMux so its // observation by registering it on the outer mux (post-wrap).
// tests are tests of production wiring (PR #152 review, hyperguild#154). reg := metrics.New()
// The REST API's credential is the same as /mcp's. Untested here: the public["GET /metrics"] = http.HandlerFunc(reg.Handler())
// restAuth fields and the public map this call passes in (e.g. a nil
// validator would drop JWT auth on /mcp and REST and the suite stays // The whole router, built in one place so the test of buildMux is a test
// green), and ListenAndServe serving the result unchanged (hyperguild#182). // of production wiring (PR #152 review). The REST API's credential is the
handler, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain", // same as /mcp's.
resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public) mux, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
resourceMetadataURL: resourceMetadataURL}, h, public)
if err != nil { if err != nil {
logger.Error("build router", "err", err) logger.Error("build router", "err", err)
os.Exit(1) os.Exit(1)
@@ -613,7 +623,7 @@ func main() {
"watch_interval", watchIntervalLog, "watch_interval", watchIntervalLog,
"mcp_enabled", true, "mcp_enabled", true,
) )
if err := http.ListenAndServe(addr, handler); err != nil { if err := http.ListenAndServe(addr, reg.Middleware(mux)); err != nil {
logger.Error("server stopped", "err", err) logger.Error("server stopped", "err", err)
os.Exit(1) os.Exit(1)
} }
+19 -51
View File
@@ -8,22 +8,22 @@ import (
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth" chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
"github.com/mathiasbq/hyperguild/ingestion/internal/api" "github.com/mathiasbq/hyperguild/ingestion/internal/api"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
) )
// publicRoutes is the complete set of patterns a caller may register on the // publicRoutes is the complete set of patterns allowed on the outer mux,
// outer mux, outside the REST API's auth (hyperguild#149). Each authenticates // outside the REST API's auth (hyperguild#149). Each authenticates itself or is
// itself or is meant to be open: /capture (bearer/JWT), the webhook (HMAC), // meant to be open: /mcp (bearer/JWT), /capture (bearer/JWT), the webhook
// /oauth/token (client secret) and OAuth metadata (open). /mcp and /metrics // (HMAC), /oauth/token (client secret), OAuth metadata and /metrics (open).
// are not here: buildMux builds them itself (hyperguild#154). A REST pattern // A REST pattern added here would beat the authenticated catch-all by
// added here would beat the authenticated catch-all by specificity and ship // specificity and ship open, which is why buildMux refuses anything else.
// open, which is why buildMux refuses anything else.
var publicRoutes = map[string]bool{ var publicRoutes = map[string]bool{
"/mcp": true,
"POST /capture": true, "POST /capture": true,
"POST /webhooks/brain-sync": true, "POST /webhooks/brain-sync": true,
"GET /.well-known/oauth-protected-resource": true, "GET /.well-known/oauth-protected-resource": true,
"GET /.well-known/oauth-authorization-server": true, "GET /.well-known/oauth-authorization-server": true,
"POST /oauth/token": true, "POST /oauth/token": true,
"GET /metrics": true,
} }
// restAuth is the credential the REST API checks: the same as /mcp's. // restAuth is the credential the REST API checks: the same as /mcp's.
@@ -34,25 +34,16 @@ type restAuth struct {
resourceMetadataURL string resourceMetadataURL string
} }
// buildMux builds the handler main passes to ListenAndServe unchanged. The // buildMux is the server's whole router. The REST API is mounted at "/"
// REST API is mounted at "/" and /mcp is registered, both behind bearer auth // behind bearer auth that buildMux constructs itself, so a caller cannot pass
// that buildMux constructs itself from auth (one credential), so a caller // a no-op; public handlers are registered as given, and only for patterns on
// cannot pass a no-op. GET /metrics serves reg, and the whole router is // the publicRoutes allowlist. It returns an http.Handler, not the
// wrapped in reg's middleware. Other public handlers are registered as given, // *ServeMux, so nothing can register a route on the result afterwards and
// and only for patterns on the publicRoutes allowlist. It returns an // bypass the allowlist.
// http.Handler, not the *ServeMux, so nothing can register a route on the func buildMux(auth restAuth, h *api.Handler, public map[string]http.Handler) (http.Handler, error) {
// result. What stays untested is main's call site: the restAuth fields and
// the public map main passes in, and ListenAndServe serving this handler
// unchanged (hyperguild#182). The tests build their own arguments.
func buildMux(
auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler,
) (http.Handler, error) {
if auth.token == "" { if auth.token == "" {
return nil, errors.New("buildMux: REST auth needs a token") return nil, errors.New("buildMux: REST auth needs a token")
} }
if mcpHandler == nil || reg == nil {
return nil, errors.New("buildMux: needs the MCP handler and the metrics registry")
}
mux := http.NewServeMux() mux := http.NewServeMux()
for pattern, handler := range public { for pattern, handler := range public {
if !publicRoutes[pattern] { if !publicRoutes[pattern] {
@@ -60,15 +51,10 @@ func buildMux(
} }
mux.Handle(pattern, handler) mux.Handle(pattern, handler)
} }
bearer := func(next http.Handler) http.Handler { mountREST(mux, h, func(next http.Handler) http.Handler {
return chassisauth.BearerMiddleware(auth.token, auth.validator, auth.realm, auth.resourceMetadataURL, next) return chassisauth.BearerMiddleware(auth.token, auth.validator, auth.realm, auth.resourceMetadataURL, next)
} })
mux.Handle("/mcp", bearer(mcpHandler)) return mux, nil
// /metrics is registered on the mux and so observed by its own
// middleware, like any route.
mux.Handle("GET /metrics", reg.Handler())
mountREST(mux, h, bearer)
return reg.Middleware(mux), nil
} }
// mountREST registers the REST API on its own mux and mounts it at "/" behind // mountREST registers the REST API on its own mux and mounts it at "/" behind
@@ -87,23 +73,5 @@ func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.
rest.HandleFunc("POST /promote", h.Promote) rest.HandleFunc("POST /promote", h.Promote)
rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings) rest.HandleFunc("POST /backfill-embeddings", h.BackfillEmbeddings)
rest.HandleFunc("GET /pass-rate", h.PassRate) rest.HandleFunc("GET /pass-rate", h.PassRate)
mux.Handle("/", labelRESTPattern(rest, auth(rest))) mux.Handle("/", auth(rest))
}
// labelRESTPattern sets the request's Pattern to the REST route it addresses
// before auth runs, so the metrics middleware (which reads Pattern from the
// same *http.Request) labels a refused request by its route rather than by
// the "/" mount (hyperguild#155). A refused request to a path no route
// matches keeps "/", so for refused requests the label set stays bounded by
// the registered routes. That bound does not hold for authenticated
// requests: an unregistered path reaches the REST mux, which 404s with an
// empty Pattern, and the metrics middleware falls back to the raw URL path
// (hyperguild#185).
func labelRESTPattern(rest *http.ServeMux, next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if _, pattern := rest.Handler(r); pattern != "" {
r.Pattern = pattern
}
next.ServeHTTP(w, r)
})
} }
@@ -1,88 +0,0 @@
package main
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// wrongTokSentinel is a presented credential distinctive enough that any
// occurrence in a log line is the raw token, not a coincidence.
const wrongTokSentinel = "wrong-tok-sentinel-q7zx"
// hyperguild#155: since #149 every REST route sits behind BearerMiddleware,
// and chassis v0.2.0 refused without logging, so an unknown caller got
// silent 401s. The chassis now writes an `mcp auth rejected` line, and the
// presented token appears nowhere in the log as a bare value (PR #181
// review: asserting on the "Bearer " header form could never fail, since
// the chassis logs fields, not headers).
func TestBuildMuxRejectionLogLineOmitsTheBarePresentedToken(t *testing.T) {
var buf bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
t.Cleanup(func() { slog.SetDefault(prev) })
code, _ := do(t, testMux(t), "POST", "/query", "Bearer "+wrongTokSentinel)
require.Equal(t, http.StatusUnauthorized, code)
assert.Contains(t, buf.String(), "mcp auth rejected")
assert.Contains(t, buf.String(), "path=/query")
assert.NotContains(t, buf.String(), wrongTokSentinel, "the rejected token is logged as a bare value")
assert.NotContains(t, buf.String(), routesTok, "the valid static token is logged")
}
// The `mcp auth accepted` line leaves out the valid static token.
func TestBuildMuxAcceptedLogLineOmitsTheBareStaticToken(t *testing.T) {
var buf bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
t.Cleanup(func() { slog.SetDefault(prev) })
code, _ := do(t, testMux(t), "GET", "/pending", "Bearer "+routesTok)
require.Equal(t, http.StatusOK, code)
assert.Contains(t, buf.String(), "mcp auth accepted")
assert.NotContains(t, buf.String(), routesTok, "the accepted static token is logged as a bare value")
}
// An authenticated REST request is labelled with the REST mux's pattern:
// the inner mux sets it on the same *http.Request the outer middleware reads.
func TestMetricsLabelAnAuthenticatedRESTRequestWithItsRoutePattern(t *testing.T) {
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "GET", "/pending", "Bearer "+routesTok)
require.Equal(t, http.StatusOK, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="GET /pending"`)
assert.NotContains(t, rec.Body.String(), `path="/"`)
}
// hyperguild#155: a request refused by REST auth never reaches the REST mux,
// so it was labelled path="/" and a 4xx on /query could not be told from one
// on /write. It is now labelled with the REST pattern it was addressed to.
func TestMetricsLabelARejectedRESTRequestWithItsRoutePattern(t *testing.T) {
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "POST", "/query", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="POST /query",status="4xx"`)
}
// A path no REST route matches keeps the catch-all label, so an attacker
// probing random URLs cannot grow the series set.
func TestMetricsLabelARejectedUnknownPathWithTheCatchAll(t *testing.T) {
mux, reg := testMuxAndRegistry(t)
code, _ := do(t, mux, "POST", "/no-such-route-zq", "")
require.Equal(t, http.StatusUnauthorized, code)
rec := httptest.NewRecorder()
reg.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/metrics", nil))
assert.Contains(t, rec.Body.String(), `path="/",status="4xx"`)
assert.NotContains(t, rec.Body.String(), "no-such-route-zq")
}
+12 -67
View File
@@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/api" "github.com/mathiasbq/hyperguild/ingestion/internal/api"
"github.com/mathiasbq/hyperguild/ingestion/internal/metrics"
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
) )
@@ -22,34 +21,20 @@ func okHandler(body string) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) }) return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) })
} }
// testMux builds the router through buildMux, the function main calls, with // testMux builds the router exactly as main does: buildMux with the full
// every allowlisted public pattern (stub handlers), a stub MCP server and a // public set main registers (stub handlers) and real bearer config.
// static token only: no JWT validator and no resourceMetadataURL (the tests // hyperguild#149: the REST API is authenticated as a whole; the PR #152
// in jwt_validator_test.go pass those). It does not use main's own restAuth // review showed a test of a COPY of the router let main.go regress unseen.
// or public map (hyperguild#182). hyperguild#149: the REST API is
// authenticated as a whole; the PR #152 review showed a test of a COPY of
// the router let main.go regress unseen.
func testMux(t *testing.T) http.Handler { func testMux(t *testing.T) http.Handler {
t.Helper() t.Helper()
mux, _ := testMuxAndRegistry(t) h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
return mux
}
func testMuxAndRegistry(t *testing.T) (http.Handler, *metrics.Registry) {
t.Helper()
public := map[string]http.Handler{} public := map[string]http.Handler{}
for p := range publicRoutes { for p := range publicRoutes {
public[p] = okHandler("public " + p) public[p] = okHandler("public " + p)
} }
reg := metrics.New() mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, public)
mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, testHandler(t), okHandler("mcp"), reg, public)
require.NoError(t, err) require.NoError(t, err)
return mux, reg return mux
}
func testHandler(t *testing.T) *api.Handler {
t.Helper()
return api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
} }
func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) { func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) {
@@ -89,9 +74,8 @@ func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) {
} }
// buildMux registers public handlers as given, without adding auth: each // buildMux registers public handlers as given, without adding auth: each
// public route owns its own (bearer/JWT on /capture, HMAC on the webhook, // public route owns its own (bearer on /mcp, HMAC on the webhook, client
// client secret on /oauth/token) or is meant to be open (OAuth metadata). // secret on /oauth/token) or is meant to be open (/metrics, OAuth metadata).
// /mcp and /metrics are not public routes: buildMux builds them itself.
func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) { func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
r := testMux(t) r := testMux(t)
for p := range publicRoutes { for p := range publicRoutes {
@@ -109,53 +93,14 @@ func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
// over the authenticated catch-all (more specific pattern), so it is refused. // over the authenticated catch-all (more specific pattern), so it is refused.
func TestBuildMuxRefusesAPublicPatternOffTheAllowlist(t *testing.T) { func TestBuildMuxRefusesAPublicPatternOffTheAllowlist(t *testing.T) {
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{}) h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
// "/mcp" and "GET /metrics" are built by buildMux itself (hyperguild#154): for _, p := range []string{"POST /query", "GET /pending", "/", "POST /metrics"} {
// passing either as public would register them unauthenticated or twice. _, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, map[string]http.Handler{p: okHandler("x")})
for _, p := range []string{"POST /query", "GET /pending", "/", "POST /metrics", "/mcp", "GET /metrics"} {
_, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, okHandler("mcp"), metrics.New(),
map[string]http.Handler{p: okHandler("x")})
assert.Error(t, err, "public pattern %q must be refused", p) assert.Error(t, err, "public pattern %q must be refused", p)
} }
} }
func TestBuildMuxRefusesAnEmptyToken(t *testing.T) { func TestBuildMuxRefusesAnEmptyToken(t *testing.T) {
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{}) h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
_, err := buildMux(restAuth{token: "", realm: "brain"}, h, okHandler("mcp"), metrics.New(), nil) _, err := buildMux(restAuth{token: "", realm: "brain"}, h, nil)
assert.Error(t, err) assert.Error(t, err)
} }
// hyperguild#154: /mcp's bearer was built in main.go, untested; the buildMux
// tests used stubs. buildMux now wraps the MCP handler itself, with the same
// credential as the REST API.
func TestBuildMuxRefusesMCPWithoutAValidToken(t *testing.T) {
r := testMux(t)
for _, authz := range []string{"", "Bearer wrong"} {
code, body := do(t, r, "POST", "/mcp", authz)
assert.Equal(t, http.StatusUnauthorized, code, "authz %q", authz)
assert.NotEqual(t, "mcp", body)
}
code, body := do(t, r, "POST", "/mcp", "Bearer "+routesTok)
assert.Equal(t, http.StatusOK, code)
assert.Equal(t, "mcp", body)
}
func TestBuildMuxRefusesANilMCPHandlerOrRegistry(t *testing.T) {
auth := restAuth{token: routesTok, realm: "brain"}
_, err := buildMux(auth, testHandler(t), nil, metrics.New(), nil)
assert.Error(t, err)
_, err = buildMux(auth, testHandler(t), okHandler("mcp"), nil, nil)
assert.Error(t, err)
}
// hyperguild#154: main wrapped buildMux's result in the metrics middleware
// before ListenAndServe, untested. buildMux now returns the handler main
// serves as-is, metrics included: a request through it is observed and
// exposed on its own GET /metrics.
func TestBuildMuxObservesRequestsAndServesMetrics(t *testing.T) {
r := testMux(t)
code, _ := do(t, r, "GET", "/pending", "Bearer "+routesTok)
require.Equal(t, http.StatusOK, code)
code, body := do(t, r, "GET", "/metrics", "")
require.Equal(t, http.StatusOK, code)
assert.Contains(t, body, `path="GET /pending"`)
}
+1 -1
View File
@@ -51,7 +51,7 @@ require (
) )
require ( require (
git.d-ma.be/mathias/mcp-chassis v0.7.1 git.d-ma.be/mathias/mcp-chassis v0.2.0
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.0 // indirect
github.com/goccy/go-json v0.10.3 // indirect github.com/goccy/go-json v0.10.3 // indirect
+2 -2
View File
@@ -1,5 +1,5 @@
git.d-ma.be/mathias/mcp-chassis v0.7.1 h1:4h8TOBHTA0L/no4IPcr4tLcQtmKUlhE2/cXyPdNFGmM= git.d-ma.be/mathias/mcp-chassis v0.2.0 h1:6fLmb7xqRa2nNVWsHaUbbfbArgDXJw/gDhb09clBIjo=
git.d-ma.be/mathias/mcp-chassis v0.7.1/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8= git.d-ma.be/mathias/mcp-chassis v0.2.0/go.mod h1:Ks7EK2UnGAN0H3rJjKUxUagX8/ZBdtLrOlcUbv0RwH8=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+5 -17
View File
@@ -3,18 +3,15 @@ package api
import ( import (
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"errors"
"fmt" "fmt"
"io/fs" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain" "github.com/mathiasbq/hyperguild/ingestion/internal/brain"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"github.com/mathiasbq/hyperguild/ingestion/internal/claim" "github.com/mathiasbq/hyperguild/ingestion/internal/claim"
"github.com/mathiasbq/hyperguild/ingestion/internal/notes"
) )
// ContentHash returns the lowercase hex sha256 of b. It is the note's // ContentHash returns the lowercase hex sha256 of b. It is the note's
@@ -94,23 +91,14 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
rel = filepath.ToSlash(rel) rel = filepath.ToSlash(rel)
} }
// The path is caller-supplied (brain_update): resolve it as brain_edit rel, abs, err := resolveWithin(brainDir, rel)
// does, knowledge/ and wiki/ notes only, and read and rewrite it through
// the brain root, never through a symlink (hyperguild#169).
t, err := notes.Resolve(brainDir, rel, notes.ScopeNotes)
if err != nil { if err != nil {
return "", "", "", err return "", "", "", err
} }
rel = t.Rel
bfs, err := brainfs.Open(brainDir)
if err != nil {
return "", "", "", fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
prior, err := bfs.ReadRegular(rel) prior, err := os.ReadFile(abs)
if err != nil { if err != nil {
if errors.Is(err, fs.ErrNotExist) { if os.IsNotExist(err) {
return "", "", "", fmt.Errorf("note %q does not exist: use brain_write to create", rel) return "", "", "", fmt.Errorf("note %q does not exist: use brain_write to create", rel)
} }
return "", "", "", fmt.Errorf("read target: %w", err) return "", "", "", fmt.Errorf("read target: %w", err)
@@ -176,7 +164,7 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
return "", "", "", err return "", "", "", err
} }
out := []byte(stamped) out := []byte(stamped)
if err := bfs.WriteRegular(rel, out); err != nil { if err := os.WriteFile(abs, out, 0o644); err != nil {
return "", "", "", fmt.Errorf("write: %w", err) return "", "", "", fmt.Errorf("write: %w", err)
} }
return rel, ContentHash(out), priorHash, nil return rel, ContentHash(out), priorHash, nil
-112
View File
@@ -1,112 +0,0 @@
package api_test
import (
"os"
"path/filepath"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
)
// brain_update passed the caller's path through resolveWithin, a lexical
// containment check only: any file in the brain could be superseded
// (classification.yaml, raw/, sessions/), and a final symlink was written
// through. It now resolves like brain_edit: knowledge/ and wiki/ notes only,
// read and rewritten through the brain root.
func TestUpdateNoteRefusesPathsOutsideKnowledgeAndWikiNotes(t *testing.T) {
brain := t.TempDir()
files := map[string]string{
"classification.yaml": "levels: {}\n",
"raw/r.md": "raw\n",
"sessions/s.jsonl": "{}\n",
"wiki/w/facts/ok.md": "---\ntitle: ok\n---\nold\n",
"knowledge/k.md": "---\ntitle: k\n---\nold\n",
"wiki/w/facts/x.txt": "txt\n",
}
for rel, body := range files {
p := filepath.Join(brain, filepath.FromSlash(rel))
require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755))
require.NoError(t, os.WriteFile(p, []byte(body), 0o644))
}
for _, rel := range []string{"classification.yaml", "raw/r.md", "sessions/s.jsonl", "wiki/w/facts/x.txt", "wiki/w/../../raw/r.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "replaced"})
assert.Error(t, err, rel)
}
for rel, body := range files {
if rel == "wiki/w/facts/ok.md" || rel == "knowledge/k.md" {
continue
}
got, err := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
require.NoError(t, err)
assert.Equal(t, body, string(got), rel)
}
for _, rel := range []string{"wiki/w/facts/ok.md", "knowledge/k.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "new body"})
require.NoError(t, err, "positive control: %s", rel)
got, _ := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
assert.Contains(t, string(got), "new body")
}
}
func TestUpdateNoteDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), filepath.Join(facts, "l.md")))
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/l.md", Content: "replaced"})
assert.Error(t, err)
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
// The note is swapped between a regular file and an in-brain symlink to a
// raw/ file while UpdateNote runs: the read and the write each refuse a
// symlink, so the raw/ file is never rewritten.
func TestUpdateNoteNeverRewritesASymlinkSwappedInForTheNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
note := filepath.Join(facts, "n.md")
body := "---\ntitle: n\n---\nold\n"
require.NoError(t, os.WriteFile(note, []byte(body), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := note + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte(body), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, note)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if _, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/n.md", Content: "new"}); err == nil {
ok++
}
}
stop.Store(true)
wg.Wait()
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
assert.Positive(t, ok, "positive control: some updates reach the regular note")
}
+37 -67
View File
@@ -2,18 +2,12 @@ package brain
import ( import (
"bufio" "bufio"
"bytes"
"errors"
"fmt" "fmt"
"io"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"sort" "sort"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// noteEntry is one row in a Wing _index.md. // noteEntry is one row in a Wing _index.md.
@@ -33,18 +27,13 @@ func BuildWingIndex(brainDir, wing string) error {
return fmt.Errorf("invalid wing %q", wing) return fmt.Errorf("invalid wing %q", wing)
} }
wingDir := filepath.Join(brainDir, "wiki", w) wingDir := filepath.Join(brainDir, "wiki", w)
// Listed and read through the brain root (hyperguild#157): a wing or if _, err := os.Stat(wingDir); os.IsNotExist(err) {
// 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 return nil
} else if err != nil {
return fmt.Errorf("stat wing: %w", err)
} }
entries, err := collectWingEntries(bfs, "wiki/"+w) entries, err := collectWingEntries(wingDir)
if err != nil { if err != nil {
return err return err
} }
@@ -69,47 +58,51 @@ func BuildWingIndex(brainDir, wing string) error {
// BuildAllWingIndexes regenerates _index.md for every wing under brain/wiki/. // BuildAllWingIndexes regenerates _index.md for every wing under brain/wiki/.
func BuildAllWingIndexes(brainDir string) error { func BuildAllWingIndexes(brainDir string) error {
bfs, err := brainfs.Open(brainDir) wikiDir := filepath.Join(brainDir, "wiki")
if err != nil { ents, err := os.ReadDir(wikiDir)
return fmt.Errorf("open brain: %w", err) if os.IsNotExist(err) {
}
wings, err := bfs.ReadDirDirs("wiki")
_ = bfs.Close()
if errors.Is(err, fs.ErrNotExist) {
return nil return nil
} }
if err != nil { if err != nil {
return fmt.Errorf("read wiki: %w", err) return fmt.Errorf("read wiki: %w", err)
} }
for _, w := range wings { for _, e := range ents {
if err := BuildWingIndex(brainDir, w); err != nil { if !e.IsDir() {
return fmt.Errorf("index %s: %w", w, err) continue
}
if err := BuildWingIndex(brainDir, e.Name()); err != nil {
return fmt.Errorf("index %s: %w", e.Name(), err)
} }
} }
return nil return nil
} }
func collectWingEntries(bfs *brainfs.Brain, wingRel string) ([]noteEntry, error) { func collectWingEntries(wingDir string) ([]noteEntry, error) {
var out []noteEntry var out []noteEntry
halls, err := bfs.ReadDirDirs(wingRel) ents, err := os.ReadDir(wingDir)
if err != nil { if err != nil {
return nil, fmt.Errorf("read wing: %w", err) return nil, fmt.Errorf("read wing: %w", err)
} }
for _, hall := range halls { for _, hallEnt := range ents {
if !hallEnt.IsDir() {
continue
}
hall := hallEnt.Name()
if !IsValidHall(hall) { if !IsValidHall(hall) {
continue continue
} }
hallRel := wingRel + "/" + hall hallDir := filepath.Join(wingDir, hall)
names, err := bfs.ReadDirRegular(hallRel) notes, err := os.ReadDir(hallDir)
if err != nil { if err != nil {
return nil, fmt.Errorf("read hall %s: %w", hall, err) return nil, fmt.Errorf("read hall %s: %w", hall, err)
} }
for _, name := range names { for _, n := range notes {
if !strings.HasSuffix(name, ".md") || name == "_index.md" { if n.IsDir() || !strings.HasSuffix(n.Name(), ".md") || n.Name() == "_index.md" {
continue continue
} }
slug := strings.TrimSuffix(name, ".md") slug := strings.TrimSuffix(n.Name(), ".md")
title, created := readTitleAndCreatedIn(bfs, hallRel+"/"+name, slug) full := filepath.Join(hallDir, n.Name())
title, created := readTitleAndCreated(full, slug)
out = append(out, noteEntry{Hall: hall, Slug: slug, Title: title, Created: created}) out = append(out, noteEntry{Hall: hall, Slug: slug, Title: title, Created: created})
} }
} }
@@ -124,40 +117,9 @@ func readTitleAndCreated(path, slug string) (string, string) {
return slug, "" return slug, ""
} }
defer func() { _ = f.Close() }() 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
}
// readTitleAndCreatedIn is readTitleAndCreated through the brain root, title, created := "", ""
// regular files only (hyperguild#157). scanner := bufio.NewScanner(f)
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 inFrontmatter := false
for scanner.Scan() { for scanner.Scan() {
line := scanner.Text() line := scanner.Text()
@@ -187,5 +149,13 @@ func parseTitleAndCreated(r io.Reader) (title, created 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 return title, created
} }
-132
View File
@@ -1,132 +0,0 @@
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/<wing>/<hall>/ 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]]")
})
}
}
+17 -32
View File
@@ -1,16 +1,12 @@
package brain package brain
import ( import (
"errors"
"fmt" "fmt"
"io/fs" "os"
"log/slog" "path/filepath"
"path"
"regexp" "regexp"
"sort" "sort"
"strings" "strings"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// wikilinkRE matches [[target]] and [[target|Display Text]]. // wikilinkRE matches [[target]] and [[target|Display Text]].
@@ -102,40 +98,29 @@ var linkedTrees = []string{"knowledge", "wiki"}
// one the audit will also resolve. // one the audit will also resolve.
func entryIndex(brainDir string) (map[string]struct{}, error) { func entryIndex(brainDir string) (map[string]struct{}, error) {
index := map[string]struct{}{} 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 { for _, dir := range linkedTrees {
if _, err := bfs.Lstat(dir); errors.Is(err, fs.ErrNotExist) { root := filepath.Join(brainDir, dir)
if _, err := os.Stat(root); os.IsNotExist(err) {
continue continue
} else if err != nil {
return nil, fmt.Errorf("stat %s: %w", dir, err)
} }
var walkErr error err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
err := bfs.WalkRegular(dir, func(rel string) error { if err != nil {
if !strings.HasSuffix(rel, ".md") { return err
}
if d.IsDir() || !strings.HasSuffix(path, ".md") {
return nil return nil
} }
index[strings.TrimSuffix(path.Base(rel), ".md")] = struct{}{} 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.TrimPrefix(strings.TrimSuffix(rel, ".md"), "wiki/")] = struct{}{} index[strings.TrimPrefix(strings.TrimSuffix(rel, ".md"), "wiki/")] = struct{}{}
return nil 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 { if err != nil {
return nil, fmt.Errorf("walk %s: %w", dir, err) return nil, fmt.Errorf("walk %s: %w", dir, err)
} }
+20 -54
View File
@@ -4,12 +4,9 @@ import (
"bufio" "bufio"
"fmt" "fmt"
"os" "os"
"path"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
) )
// seeAlsoHeader is the markdown heading used to group cross-wing links. // seeAlsoHeader is the markdown heading used to group cross-wing links.
@@ -200,12 +197,6 @@ func fileExists(p string) bool {
// section headers. Rejects same-wing pairs (a tunnel is by definition // section headers. Rejects same-wing pairs (a tunnel is by definition
// cross-wing) and missing notes. // cross-wing) and missing notes.
func WriteTunnel(brainDir, sourcePath, targetPath string) error { func WriteTunnel(brainDir, sourcePath, targetPath string) error {
if err := checkNotePath(sourcePath); err != nil {
return fmt.Errorf("source: %w", err)
}
if err := checkNotePath(targetPath); err != nil {
return fmt.Errorf("target: %w", err)
}
srcWing, err := wingOf(sourcePath) srcWing, err := wingOf(sourcePath)
if err != nil { if err != nil {
return fmt.Errorf("source: %w", err) return fmt.Errorf("source: %w", err)
@@ -218,50 +209,20 @@ func WriteTunnel(brainDir, sourcePath, targetPath string) error {
return fmt.Errorf("tunnel must cross wings; got both in %q", srcWing) return fmt.Errorf("tunnel must cross wings; got both in %q", srcWing)
} }
// Both notes are read and rewritten through the brain root, regular srcFull := filepath.Join(brainDir, filepath.FromSlash(sourcePath))
// files only: brain_tunnel passes caller-supplied paths, and the brain is tgtFull := filepath.Join(brainDir, filepath.FromSlash(targetPath))
// a Syncthing folder that can hold symlinks. if _, err := os.Stat(srcFull); err != nil {
bfs, err := brainfs.Open(brainDir)
if err != nil {
return fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
srcContent, err := bfs.ReadRegular(sourcePath)
if err != nil {
return fmt.Errorf("source note: %w", err) return fmt.Errorf("source note: %w", err)
} }
tgtContent, err := bfs.ReadRegular(targetPath) if _, err := os.Stat(tgtFull); err != nil {
if err != nil {
return fmt.Errorf("target note: %w", err) return fmt.Errorf("target note: %w", err)
} }
if out, changed, err := withSeeAlso(srcContent, wikilinkOf(targetPath)); err != nil { if err := appendSeeAlso(srcFull, wikilinkOf(targetPath)); err != nil {
return fmt.Errorf("update source: %w", err) return fmt.Errorf("update source: %w", err)
} else if changed {
if err := bfs.WriteRegular(sourcePath, out); err != nil {
return fmt.Errorf("update source: %w", err)
}
} }
if out, changed, err := withSeeAlso(tgtContent, wikilinkOf(sourcePath)); err != nil { if err := appendSeeAlso(tgtFull, wikilinkOf(sourcePath)); err != nil {
return fmt.Errorf("update target: %w", err) return fmt.Errorf("update target: %w", err)
} else if changed {
if err := bfs.WriteRegular(targetPath, out); err != nil {
return fmt.Errorf("update target: %w", err)
}
}
return nil
}
// checkNotePath accepts only a clean, local wiki/<wing>/<hall>/.../<slug>.md
// path: no "." or ".." segments, no empty segments, no leading slash.
func checkNotePath(p string) error {
if p == "" || path.Clean(p) != p || !filepath.IsLocal(p) || !strings.HasSuffix(p, ".md") {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
for _, part := range strings.Split(p, "/") {
if part == "" || part == "." || part == ".." {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
} }
return nil return nil
} }
@@ -287,13 +248,16 @@ func wingOf(relPath string) (string, error) {
return parts[1], nil return parts[1], nil
} }
// withSeeAlso returns content with `- [[link]]` under its See also section, // appendSeeAlso inserts `- [[link]]` under the file's See also section,
// creating the section if absent. changed is false when the link is already // creating the section if absent. No-op when the link is already present.
// present. func appendSeeAlso(filePath, link string) error {
func withSeeAlso(content []byte, link string) (out []byte, changed bool, err error) { content, err := os.ReadFile(filePath)
if err != nil {
return err
}
wikilink := "[[" + link + "]]" wikilink := "[[" + link + "]]"
if strings.Contains(string(content), wikilink) { if strings.Contains(string(content), wikilink) {
return content, false, nil return nil
} }
bullet := "- " + wikilink bullet := "- " + wikilink
@@ -302,7 +266,8 @@ func withSeeAlso(content []byte, link string) (out []byte, changed bool, err err
// No section yet — append a fresh one. Always emit a trailing // No section yet — append a fresh one. Always emit a trailing
// newline so subsequent appends don't merge into the previous line. // newline so subsequent appends don't merge into the previous line.
trimmed := strings.TrimRight(string(content), "\n") trimmed := strings.TrimRight(string(content), "\n")
return []byte(trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n"), true, nil out := trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n"
return os.WriteFile(filePath, []byte(out), 0o644)
} }
// Section exists — splice the bullet in just before the next `## ` // Section exists — splice the bullet in just before the next `## `
@@ -328,11 +293,12 @@ func withSeeAlso(content []byte, link string) (out []byte, changed bool, err err
b.WriteByte('\n') b.WriteByte('\n')
} }
if err := scanner.Err(); err != nil { if err := scanner.Err(); err != nil {
return nil, false, err return err
} }
if !inserted { if !inserted {
// section was the last thing in the file — just append bullet // section was the last thing in the file — just append bullet
return []byte(strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n"), true, nil out := strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n"
return os.WriteFile(filePath, []byte(out), 0o644)
} }
return []byte(b.String()), true, nil return os.WriteFile(filePath, []byte(b.String()), 0o644)
} }
@@ -1,141 +0,0 @@
package brain_test
import (
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
)
// brain_tunnel passes caller-supplied source and target paths to WriteTunnel,
// which checked only "wiki/<wing>/..." and then joined them onto the brain
// dir: ".." segments resolved outside the brain and appendSeeAlso rewrote the
// file there.
func TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("---\ntitle: S\n---\nbody\n"), 0o644))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
up := strings.Repeat("../", strings.Count(filepath.Clean(brain0), "/")+3)
target := "wiki/b/x/" + up + strings.TrimPrefix(victim, "/")
err := brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target)
assert.Error(t, err)
got, rerr := os.ReadFile(victim)
require.NoError(t, rerr)
assert.Equal(t, "untouched\n", string(got), "a file outside the brain was modified")
}
func TestWriteTunnelRefusesUncleanOrNonWikiPaths(t *testing.T) {
brain0 := t.TempDir()
for _, p := range []string{"wiki/a/facts/s.md", "wiki/b/facts/t.md", "wiki/b/facts/t.txt", "raw/r.md"} {
require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(brain0, p)), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain0, p), []byte("x\n"), 0o644))
}
for _, target := range []string{
"wiki/b/facts/../../../raw/r.md",
"wiki/b/./facts/t.md",
"wiki/b//facts/t.md",
"/wiki/b/facts/t.md",
"wiki/b/facts/t.txt",
"wiki/b/facts/",
} {
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target), target)
}
for _, p := range []string{"raw/r.md", "wiki/b/facts/t.txt"} {
got, err := os.ReadFile(filepath.Join(brain0, p))
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), p)
}
require.NoError(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"), "positive control")
got, err := os.ReadFile(filepath.Join(brain0, "wiki", "b", "facts", "t.md"))
require.NoError(t, err)
assert.Contains(t, string(got), "[[a/facts/s]]")
}
func TestWriteTunnelDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(victim, filepath.Join(tdir, "t.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"))
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
// The same with a link to a file elsewhere in the brain, which os.Root
// would follow.
inVictim := filepath.Join(brain0, "raw", "v.md")
require.NoError(t, os.MkdirAll(filepath.Dir(inVictim), 0o755))
require.NoError(t, os.WriteFile(inVictim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "v.md"), filepath.Join(tdir, "u.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/u.md"))
got, err = os.ReadFile(inVictim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
got, err = os.ReadFile(src)
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), "the source is not touched when the target is refused")
}
// The target is swapped between a regular note and a symlink to an in-brain
// file outside wiki/ while WriteTunnel runs: the read and the write each
// refuse a symlink, so the other file is never rewritten.
func TestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(brain0, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
tgt := filepath.Join(tdir, "t.md")
require.NoError(t, os.WriteFile(tgt, []byte("t\n"), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := tgt + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte("t\n"), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, tgt)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md") == nil {
ok++
}
_ = os.WriteFile(src, []byte("x\n"), 0o644)
}
stop.Store(true)
wg.Wait()
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got), "the swapped-in symlink's target was rewritten")
assert.Positive(t, ok, "positive control: some tunnels were written to the regular target")
}
-59
View File
@@ -104,27 +104,6 @@ func (b *Brain) ReadDirRegular(relDir string) ([]string, error) {
return names, nil 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 // ReadRegular reads a brain-relative path, refusing anything that is not a
// regular file, both before the open and on the opened handle. // regular file, both before the open and on the opened handle.
func (b *Brain) ReadRegular(rel string) ([]byte, error) { func (b *Brain) ReadRegular(rel string) ([]byte, error) {
@@ -249,41 +228,3 @@ func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func
return fn(p) return fn(p)
}) })
} }
// WriteRegular replaces the contents of an existing regular file at a
// brain-relative path. It never creates a file and never writes through a
// final symlink: os.Root.OpenFile follows an in-root final symlink even with
// O_NOFOLLOW (verified on go1.27), so the path is Lstat-ed through the root
// first and the opened handle must be that same file (os.SameFile), which
// also catches a swap between the two. O_NONBLOCK keeps a FIFO from blocking
// the open. Intermediate in-brain symlinks are still followed (os.Root keeps
// the write inside the brain; hyperguild#167).
func (b *Brain) WriteRegular(rel string, data []byte) error {
rel = filepath.ToSlash(rel)
lfi, err := b.root.Lstat(rel)
if err != nil {
return err
}
if !lfi.Mode().IsRegular() {
return fmt.Errorf("%s: %w", rel, ErrNotRegular)
}
f, err := b.root.OpenFile(rel, os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
if err != nil {
return err
}
defer func() { _ = f.Close() }()
fi, err := f.Stat()
if err != nil {
return err
}
if !fi.Mode().IsRegular() || !os.SameFile(lfi, fi) {
return fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
}
if err := f.Truncate(0); err != nil {
return err
}
if _, err := f.Write(data); err != nil {
return err
}
return f.Close()
}
@@ -262,67 +262,3 @@ func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.
assert.Zero(t, leaks) assert.Zero(t, leaks)
assert.Positive(t, ok, "positive control: the regular file is read") assert.Positive(t, ok, "positive control: the regular file is read")
} }
func TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles(t *testing.T) {
dir := t.TempDir()
outside := filepath.Join(t.TempDir(), "o.md")
require.NoError(t, os.WriteFile(outside, []byte("untouched"), 0o644))
require.NoError(t, os.Symlink(outside, filepath.Join(dir, "l.md")))
require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "f.md"), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(dir, "n.md"), []byte("old content"), 0o644))
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
assert.Error(t, b.WriteRegular("l.md", []byte("x")))
got, _ := os.ReadFile(outside)
assert.Equal(t, "untouched", string(got))
// A symlink to another file inside the brain: os.Root would allow it,
// only O_NOFOLLOW refuses it.
require.NoError(t, os.WriteFile(filepath.Join(dir, "in.md"), []byte("inside"), 0o644))
require.NoError(t, os.Symlink("in.md", filepath.Join(dir, "il.md")))
assert.Error(t, b.WriteRegular("il.md", []byte("x")))
got, _ = os.ReadFile(filepath.Join(dir, "in.md"))
assert.Equal(t, "inside", string(got))
// A reader held open, so a non-blocking write open would succeed. The
// Lstat type check refuses the FIFO first; the handle check and
// O_NONBLOCK matter only if it is swapped in after the Lstat.
r, err := os.OpenFile(filepath.Join(dir, "f.md"), os.O_RDONLY|syscall.O_NONBLOCK, 0)
require.NoError(t, err)
defer func() { _ = r.Close() }()
done := make(chan error, 1)
go func() { done <- b.WriteRegular("f.md", []byte("x")) }()
select {
case err := <-done:
assert.Error(t, err)
case <-time.After(3 * time.Second):
t.Fatal("WriteRegular hung on a FIFO")
}
assert.Error(t, b.WriteRegular("../"+filepath.Base(filepath.Dir(outside))+"/o.md", []byte("x")))
assert.Error(t, b.WriteRegular("missing.md", []byte("x")), "does not create files")
require.NoError(t, b.WriteRegular("n.md", []byte("new")))
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)
}
@@ -27,7 +27,7 @@ const outsideContent = "---\ntitle: " + Sentinel + "\naliases:\n - " + Sentinel
"\n\n" + Sentinel + " [[" + Sentinel + "]]\n" "\n\n" + Sentinel + " [[" + Sentinel + "]]\n"
// Outside writes a file outside any brain dir containing Sentinel. // Outside writes a file outside any brain dir containing Sentinel.
func Outside(t testing.TB) string { func Outside(t *testing.T) string {
t.Helper() t.Helper()
p := filepath.Join(t.TempDir(), "secret.md") p := filepath.Join(t.TempDir(), "secret.md")
if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil {
@@ -42,12 +42,9 @@ var swapSeq atomic.Int64
// a symlink to outside or a FIFO into path, until the test ends. FIFOs get a // 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 // 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 // handle check parks forever instead of reading EOF. The swapper is stopped
// and awaited before t.TempDir cleanup. It returns the number of symlinks // and awaited before t.TempDir cleanup.
// or FIFOs actually renamed into path so far, so a caller can tell a race func StartSwap(t *testing.T, path, inside, outside string, fifo bool) {
// 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() t.Helper()
var swaps atomic.Int64
var stop atomic.Bool var stop atomic.Bool
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Add(1)
@@ -70,26 +67,22 @@ func StartSwap(t testing.TB, path, inside, outside string, fifo bool) *atomic.In
}() }()
for i := 0; !stop.Load(); i++ { for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp) _ = os.Remove(tmp)
var made error
switch { switch {
case i%2 == 0: case i%2 == 0:
_ = os.WriteFile(tmp, []byte(inside), 0o644) _ = os.WriteFile(tmp, []byte(inside), 0o644)
case fifo: case fifo:
if made = syscall.Mkfifo(tmp, 0o644); made == nil { if err := syscall.Mkfifo(tmp, 0o644); err == nil {
if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil {
writers = append(writers, w) writers = append(writers, w)
} }
} }
default: default:
made = os.Symlink(link, tmp) _ = 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() }) t.Cleanup(func() { stop.Store(true); wg.Wait() })
return &swaps
} }
// WriteSentinelAt writes a file carrying Sentinel at p (creating its // WriteSentinelAt writes a file carrying Sentinel at p (creating its
@@ -114,41 +107,18 @@ func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func(
AssertNoLeakFrom(t, path, inside, Outside(t), d, read) 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. // AssertNoLeakFrom is AssertNoLeak with the symlink pointing at target, e.g.
// a file elsewhere in the brain written with WriteSentinelAt. // 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() t.Helper()
if !strings.Contains(inside, InsideMarker) { StartSwap(t, path, inside, target, false)
t.Fatalf("inside content must contain %q", InsideMarker) leaks, iters := 0, 0
}
swaps := StartSwap(t, path, inside, target, false)
leaks, iters, insideReads := 0, 0, 0
for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ { for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ {
out := read() leaks += strings.Count(read(), Sentinel)
leaks += strings.Count(out, Sentinel)
if strings.Contains(out, InsideMarker) {
insideReads++
}
} }
if leaks > 0 { if leaks > 0 {
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) 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 // AssertNoHang loops read for d while a FIFO swapper (held writers) runs on
@@ -1,79 +0,0 @@
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
+1 -1
View File
@@ -162,6 +162,6 @@ func TestBackfillRefsDoesNotCopyAPageSwappedInBehindASymlink(t *testing.T) {
return string(b) return string(b)
} }
require.NoError(t, os.WriteFile(page, []byte(inside), 0o644)) require.NoError(t, os.WriteFile(page, []byte(inside), 0o644))
require.Contains(t, read(), "[[s|S]]", "positive control: the regular page gets the source ref") require.Contains(t, read(), "zqinside", "positive control: the regular page gets the ref")
brainfstest.AssertNoLeakFrom(t, page, inside, target, 3*time.Second, read) brainfstest.AssertNoLeakFrom(t, page, inside, target, 3*time.Second, read)
} }
+9 -26
View File
@@ -93,20 +93,16 @@ func TestQueryDoesNotFollowASymlinkedWingUnderAWingAndHallRoot(t *testing.T) {
// bypass of readOpened hung Query after 5 iterations). // bypass of readOpened hung Query after 5 iterations).
// startSwap runs swapInto until the test ends and waits for it before // startSwap runs swapInto until the test ends and waits for it before
// t.TempDir's cleanup (hyperguild#162). // t.TempDir's cleanup (hyperguild#162).
// It returns the count of symlinks or FIFOs actually renamed into path func startSwap(t *testing.T, path, outside string, fifo bool) {
// (hyperguild#177: a swapper whose every swap fails tests nothing).
func startSwap(t *testing.T, path, outside string, fifo bool) *atomic.Int64 {
t.Helper() t.Helper()
var stop atomic.Bool var stop atomic.Bool
var swaps atomic.Int64
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Add(1)
go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop, &swaps) }() go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop) }()
t.Cleanup(func() { stop.Store(true); wg.Wait() }) t.Cleanup(func() { stop.Store(true); wg.Wait() })
return &swaps
} }
func swapInto(path, outside string, fifo bool, stop *atomic.Bool, swaps *atomic.Int64) { func swapInto(path, outside string, fifo bool, stop *atomic.Bool) {
tmp := path + ".swap" tmp := path + ".swap"
// Writers stay open until the swapper exits, so a reader parked on a // 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 // FIFO stays parked (closing one would hand the reader EOF and hide the
@@ -119,46 +115,33 @@ func swapInto(path, outside string, fifo bool, stop *atomic.Bool, swaps *atomic.
}() }()
for i := 0; !stop.Load(); i++ { for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp) _ = os.Remove(tmp)
var made error
switch { switch {
case i%2 == 0: case i%2 == 0:
_ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644) _ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644)
case fifo: case fifo:
if made = syscall.Mkfifo(tmp, 0o644); made == nil { if err := syscall.Mkfifo(tmp, 0o644); err == nil {
if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil {
writers = append(writers, w) writers = append(writers, w)
} }
} }
default: default:
made = os.Symlink(outside, tmp) _ = 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) { func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) {
brain, facts, outside := searchBrain(t) brain, facts, outside := searchBrain(t)
swaps := startSwap(t, filepath.Join(facts, "r.md"), outside, false) startSwap(t, filepath.Join(facts, "r.md"), outside, false)
leaks, iters, insideReads := 0, 0, 0 leaks, iters := 0, 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ { for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ {
rs, err := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) rs, _ := Query(brain, QueryOptions{Query: sentinel157, Limit: 10})
require.NoError(t, err)
leaks += leaked(rs) 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 { if leaks > 0 {
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) 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) { func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) {
-92
View File
@@ -1,92 +0,0 @@
package session
import (
"errors"
"os"
"path/filepath"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// session_log passed the caller's session_id straight into
// filepath.Join(sessionsDir, id+".jsonl") and opened it O_CREATE|O_APPEND:
// path segments in the id created or appended files anywhere the pod could
// write. The id is now a plain name, and the open does not follow a final
// symlink.
func TestAppendRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
for _, id := range []string{"../x", "a/b", "/abs", ".", "..", ".hidden", "", "a\\b", "a\x00b"} {
assert.Error(t, Append(dir, id, Entry{SessionID: id}), "%q", id)
}
ents, _ := os.ReadDir(root)
for _, e := range ents {
assert.Equal(t, "sessions", e.Name(), "nothing created beside the sessions dir")
}
for _, id := range []string{"1557b2e1-bf0d-5c4f-bbe3-0754d087e356", "_routing", "2026-05-05-infra.plan", "0"} {
require.NoError(t, Append(dir, id, Entry{SessionID: id}), "positive control: %q", id)
}
}
func TestAppendDoesNotWriteThroughASymlinkedLog(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
victim := filepath.Join(root, "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "victim.md"), filepath.Join(dir, "s.jsonl")))
assert.Error(t, Append(dir, "s", Entry{SessionID: "s"}))
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
func TestReadRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(root, "x.jsonl"), []byte(`{"session_id":"x"}`+"\n"), 0o644))
_, err := Read(filepath.Join(root, "sessions"), "../x")
assert.Error(t, err)
}
// A name that starts plainly and then climbs out with ".." is exactly what the
// ban on "/" in the id's tail stops: filepath.Join cleans "a/../../x" to a
// path beside the sessions dir without needing sessions/a to exist. The
// assertion is on the refusal itself (ErrInvalidID), not on any error.
func TestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefix(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
for _, id := range []string{"a/../../escape", "a/../../../escape", "a/../b"} {
err := Append(dir, id, Entry{SessionID: id})
assert.ErrorIs(t, err, ErrInvalidID, "%q", id)
}
_, err := os.Lstat(filepath.Join(root, "escape.jsonl"))
assert.True(t, errors.Is(err, os.ErrNotExist), "nothing created beside the sessions dir")
_, err = os.Lstat(filepath.Join(dir, "b.jsonl"))
assert.True(t, errors.Is(err, os.ErrNotExist), "no in-dir alias created either")
}
// Append does no Lstat type check, so O_NONBLOCK is the only thing that keeps
// a FIFO named <id>.jsonl with no reader from blocking session_log forever.
func TestAppendReturnsPromptlyOnAFIFOLogWithNoReader(t *testing.T) {
dir := filepath.Join(t.TempDir(), "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
fifo := filepath.Join(dir, "s.jsonl")
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
done := make(chan error, 1)
go func() { done <- Append(dir, "s", Entry{SessionID: "s"}) }()
select {
case err := <-done:
assert.Error(t, err, "a FIFO with no reader is refused, not written")
case <-time.After(3 * time.Second):
// Unblock the stuck open so the goroutine and TempDir cleanup finish.
if r, err := os.OpenFile(fifo, os.O_RDONLY|syscall.O_NONBLOCK, 0); err == nil {
defer r.Close() //nolint:errcheck
}
<-done
t.Fatal("Append blocked on a FIFO log with no reader")
}
}
+1 -25
View File
@@ -9,8 +9,6 @@ import (
"io/fs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"regexp"
"syscall"
"time" "time"
) )
@@ -44,32 +42,13 @@ type Attempt struct {
RunnerOutput string `json:"runner_output,omitempty"` RunnerOutput string `json:"runner_output,omitempty"`
} }
// idPattern is a plain file name: session_log takes the id from the caller,
// and a path segment in it created or appended files anywhere the pod could
// write (hyperguild#169).
var idPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
// ErrInvalidID is returned when a session id is not a plain file name.
var ErrInvalidID = errors.New("session id is not a plain name")
func checkID(sessionID string) error {
if !idPattern.MatchString(sessionID) {
return fmt.Errorf("%w: %q", ErrInvalidID, sessionID)
}
return nil
}
// Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl. // Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl.
// The open does not follow a final symlink.
func Append(sessionsDir, sessionID string, entry Entry) error { func Append(sessionsDir, sessionID string, entry Entry) error {
if err := checkID(sessionID); err != nil {
return err
}
if err := os.MkdirAll(sessionsDir, 0o755); err != nil { if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
return fmt.Errorf("create sessions dir: %w", err) return fmt.Errorf("create sessions dir: %w", err)
} }
path := filepath.Join(sessionsDir, sessionID+".jsonl") path := filepath.Join(sessionsDir, sessionID+".jsonl")
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644) f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644)
if err != nil { if err != nil {
return fmt.Errorf("open session log: %w", err) return fmt.Errorf("open session log: %w", err)
} }
@@ -91,9 +70,6 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
// Read returns all entries for sessionID. Returns empty slice if no log exists. // Read returns all entries for sessionID. Returns empty slice if no log exists.
func Read(sessionsDir, sessionID string) ([]Entry, error) { func Read(sessionsDir, sessionID string) ([]Entry, error) {
if err := checkID(sessionID); err != nil {
return nil, err
}
path := filepath.Join(sessionsDir, sessionID+".jsonl") path := filepath.Join(sessionsDir, sessionID+".jsonl")
f, err := os.Open(path) f, err := os.Open(path)
if errors.Is(err, fs.ErrNotExist) { if errors.Is(err, fs.ErrNotExist) {
@@ -0,0 +1,75 @@
// Package startupretry retries a startup dependency that may not be serving
// yet, instead of exiting on its first failure.
//
// It exists for the cold-boot ordering race (#83): on a cluster restart the
// ingestion pod can reach OIDC discovery before dex is answering, gets a 502
// from ingress, and dies — after having already connected postgres, backfilled
// the graph and ingested session batches, all of which the restart redoes.
// Nothing is misconfigured in that scenario; the pod simply started first.
//
// The budget is bounded on purpose. Retrying forever would turn a genuine
// misconfiguration (wrong issuer URL, bad audience) into a process that looks
// healthy and never serves, which is harder to diagnose than a crash loop.
package startupretry
import (
"context"
"fmt"
"time"
)
// Policy bounds the retry: at most Attempts calls, with the delay doubling
// from Base and capped at Max.
type Policy struct {
Attempts int // total calls, including the first; < 1 means 1
Base time.Duration // delay before the second attempt
Max time.Duration // ceiling for the doubling delay
}
// Do calls fn until it succeeds, the attempt budget is spent, or ctx ends.
//
// The returned error wraps the last failure, so callers keep the diagnosis
// they would have had without the retry.
func Do[T any](ctx context.Context, p Policy, fn func(context.Context) (T, error)) (T, error) {
var zero T
attempts := p.Attempts
if attempts < 1 {
attempts = 1
}
delay := p.Base
var lastErr error
for i := range attempts {
if err := ctx.Err(); err != nil {
return zero, fmt.Errorf("startupretry: %w", err)
}
v, err := fn(ctx)
if err == nil {
return v, nil
}
lastErr = err
if i == attempts-1 {
break
}
if delay <= 0 {
delay = time.Second
}
if p.Max > 0 && delay > p.Max {
delay = p.Max
}
select {
case <-ctx.Done():
return zero, fmt.Errorf("startupretry: %w", ctx.Err())
case <-time.After(delay):
}
delay *= 2
}
return zero, fmt.Errorf("startupretry: gave up after %d attempts: %w", attempts, lastErr)
}
@@ -0,0 +1,142 @@
// ingestion/internal/startupretry/startupretry_test.go
package startupretry
import (
"context"
"errors"
"testing"
"time"
)
func fastPolicy(attempts int) Policy {
return Policy{Attempts: attempts, Base: time.Millisecond, Max: 2 * time.Millisecond}
}
func TestDo_ReturnsImmediatelyOnFirstSuccess(t *testing.T) {
calls := 0
got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
calls++
return "ready", nil
})
if err != nil {
t.Fatalf("Do: %v", err)
}
if got != "ready" {
t.Errorf("got %q, want %q", got, "ready")
}
if calls != 1 {
t.Errorf("calls = %d, want 1", calls)
}
}
func TestDo_SurvivesTransientFailuresThenSucceeds(t *testing.T) {
// The observed cold-boot race: dex is not serving yet, ingress answers
// 502 for the first few attempts, then discovery works.
calls := 0
got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
calls++
if calls < 4 {
return "", errors.New("oidc discovery: status 502")
}
return "ready", nil
})
if err != nil {
t.Fatalf("Do returned %v — a transient 502 must not be fatal", err)
}
if got != "ready" {
t.Errorf("got %q, want %q", got, "ready")
}
if calls != 4 {
t.Errorf("calls = %d, want 4", calls)
}
}
func TestDo_GivesUpAfterTheAttemptBudgetAndReportsTheLastError(t *testing.T) {
// A genuinely broken issuer must still fail — retrying forever would turn
// a config error into a silent hang, which is worse than crash-looping.
last := errors.New("oidc discovery: status 404")
calls := 0
_, err := Do(context.Background(), fastPolicy(3), func(context.Context) (string, error) {
calls++
return "", last
})
if !errors.Is(err, last) {
t.Fatalf("err = %v, want it to wrap the last failure", err)
}
if calls != 3 {
t.Errorf("calls = %d, want 3 (the attempt budget)", calls)
}
}
func TestDo_StopsPromptlyWhenTheContextIsCancelled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
calls := 0
done := make(chan struct{})
var err error
go func() {
defer close(done)
_, err = Do(ctx, Policy{Attempts: 100, Base: 50 * time.Millisecond, Max: time.Second},
func(context.Context) (string, error) {
calls++
if calls == 1 {
cancel()
}
return "", errors.New("still down")
})
}()
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("Do did not return after context cancellation")
}
if !errors.Is(err, context.Canceled) {
t.Errorf("err = %v, want context.Canceled", err)
}
}
func TestDo_BacksOffAndIsCappedByMax(t *testing.T) {
var delays []time.Duration
p := Policy{Attempts: 5, Base: 10 * time.Millisecond, Max: 25 * time.Millisecond}
last := time.Now()
_, _ = Do(context.Background(), p, func(context.Context) (string, error) {
now := time.Now()
delays = append(delays, now.Sub(last))
last = now
return "", errors.New("down")
})
// delays[0] is the first call (no wait); the rest are the sleeps.
if len(delays) != 5 {
t.Fatalf("attempts = %d, want 5", len(delays))
}
if delays[1] >= delays[2] {
t.Errorf("backoff is not growing: %v then %v", delays[1], delays[2])
}
for i, d := range delays[1:] {
if d > 2*p.Max {
t.Errorf("delay %d = %v, exceeds the Max cap %v with slack", i+1, d, p.Max)
}
}
}
func TestPolicy_ZeroValueStillMakesOneAttempt(t *testing.T) {
// A caller that forgets to configure the policy must not silently get a
// no-op that returns a nil validator and a nil error.
calls := 0
_, err := Do(context.Background(), Policy{}, func(context.Context) (string, error) {
calls++
return "", errors.New("down")
})
if calls != 1 {
t.Errorf("calls = %d, want 1", calls)
}
if err == nil {
t.Error("err = nil, want the failure surfaced")
}
}
@@ -18,14 +18,9 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"sync"
"sync/atomic"
"time" "time"
) )
// shutdownFlushTimeout bounds the final flush when Watch stops.
var shutdownFlushTimeout = 5 * time.Second
// DefaultInterval is used when Watch is given a non-positive interval. // DefaultInterval is used when Watch is given a non-positive interval.
const DefaultInterval = 30 * time.Second const DefaultInterval = 30 * time.Second
@@ -138,32 +133,13 @@ func Scan(brainDir string) (Result, error) {
// queue until Notify succeeds (see alertQueue): an outage delays them, and // queue until Notify succeeds (see alertQueue): an outage delays them, and
// any that do not fit the bounded queue, or are undeliverable, are counted in // any that do not fit the bounded queue, or are undeliverable, are counted in
// a summary that is never dropped, so every incident is delivered or counted. // a summary that is never dropped, so every incident is delivered or counted.
//
// Delivery runs in its own goroutine (hyperguild#164): a hung notifier, with
// a full queue, blocked one flush for several notify timeouts, and while the
// scan waited on it a planted link stayed readable for several intervals.
// The scan hands alerts over without blocking; any that do not fit are
// counted in the summary. When ctx ends, Watch returns after a final flush
// of what was handed over (bounded by shutdownFlushTimeout); alerts that
// flush cannot deliver are counted in one error log line instead. That
// shutdown path only runs for a caller that cancels ctx: cmd/server passes
// context.Background(), so in production the flush on handoff is what keeps
// alerts from waiting on a restart.
func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notifier) { func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notifier) {
if interval <= 0 { if interval <= 0 {
interval = DefaultInterval interval = DefaultInterval
} }
t := time.NewTicker(interval) t := time.NewTicker(interval)
defer t.Stop() defer t.Stop()
alerts := make(chan string, maxPending) q := &alertQueue{}
var overflow atomic.Int64
var wg sync.WaitGroup
wg.Add(1)
go func() { defer wg.Done(); deliver(ctx, interval, n, alerts, &overflow) }()
// The scan is the only sender: closing alerts tells deliver to drain and
// flush once more, and Watch returns only after it has.
defer func() { close(alerts); wg.Wait() }()
q := alertSender{alerts: alerts, overflow: &overflow}
reported := "" reported := ""
for { for {
select { select {
@@ -192,76 +168,7 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
". Readers may follow symlinks out of the brain; fix them.") ". Readers may follow symlinks out of the brain; fix them.")
} }
} }
} q.flush(ctx, n)
}
// alertSender hands alerts to deliver without ever blocking the scan; an
// alert that does not fit is counted in overflow, which deliver adds to the
// summary.
type alertSender struct {
alerts chan<- string
overflow *atomic.Int64
}
func (s alertSender) push(msg string) {
select {
case s.alerts <- msg:
default:
s.overflow.Add(1)
}
}
// deliver owns the alertQueue. It flushes as soon as an alert is handed
// over, and again every interval while any are pending (outage retries).
// When alerts is closed (Watch is stopping) it drains it and flushes once
// more, bounded by shutdownFlushTimeout, so an alert handed over just before
// shutdown is not lost (PR #180 review). Whatever that flush cannot deliver
// is logged as one error with its counts, never dropped silently.
func deliver(ctx context.Context, interval time.Duration, n Notifier, alerts <-chan string, overflow *atomic.Int64) {
t := time.NewTicker(interval)
defer t.Stop()
q := &alertQueue{}
// drain moves everything handed over into the queue; it reports false
// once alerts is closed.
drain := func() bool {
defer func() { q.suppressed += int(overflow.Swap(0)) }()
for {
select {
case msg, ok := <-alerts:
if !ok {
return false
}
q.push(msg)
default:
return true
}
}
}
for {
select {
case msg, ok := <-alerts:
if ok {
q.push(msg)
ok = drain()
}
if !ok {
q.suppressed += int(overflow.Swap(0))
fctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), shutdownFlushTimeout)
q.flush(fctx, n)
cancel()
if len(q.items) > 0 || q.suppressed > 0 {
slog.Error("symlink tripwire alerts not delivered at shutdown",
"undelivered", len(q.items), "suppressed", q.suppressed)
}
return
}
q.flush(ctx, n)
case <-t.C:
if !drain() {
continue // the closed channel is handled by the case above
}
q.flush(ctx, n)
}
} }
} }
@@ -1,12 +1,10 @@
package symlinkguard package symlinkguard
import ( import (
"bytes"
"context" "context"
"errors" "errors"
"fmt" "fmt"
"os" "os"
"log/slog"
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
@@ -347,19 +345,13 @@ func TestWatchDropsAMessageThatNeverSendsSoLaterAlertsFlow(t *testing.T) {
}, 3*time.Second, 10*time.Millisecond, "a later alert is delivered despite a poison message") }, 3*time.Second, 10*time.Millisecond, "a later alert is delivered despite a poison message")
} }
// toggleNotifier fails while down is set. Like the real audit.NtfyNotifier // toggleNotifier fails while down is set.
// (http.NewRequestWithContext), it also fails, without recording, when its
// ctx is already done (PR #180 round 2: notifiers that ignored ctx let a
// final flush on the cancelled ctx pass).
type toggleNotifier struct { type toggleNotifier struct {
recNotifier recNotifier
down atomic.Bool down atomic.Bool
} }
func (r *toggleNotifier) Notify(ctx context.Context, m string) error { func (r *toggleNotifier) Notify(ctx context.Context, m string) error {
if err := ctx.Err(); err != nil {
return err
}
if r.down.Load() { if r.down.Load() {
return errors.New("ntfy down") return errors.New("ntfy down")
} }
@@ -504,271 +496,3 @@ func TestWatchDropsAPoisonMessageAndCountsIt(t *testing.T) {
return false return false
}, 3*time.Second, 5*time.Millisecond, "the dropped poison alert is counted in a summary") }, 3*time.Second, 5*time.Millisecond, "the dropped poison alert is counted in a summary")
} }
// hungNotifier answers every Notify only after latency (or when ctx ends),
// with an outage error: an ntfy that accepts connections but never responds
// until its timeout.
type hungNotifier struct{ latency time.Duration }
func (h hungNotifier) Notify(ctx context.Context, _ string) error {
select {
case <-time.After(h.latency):
case <-ctx.Done():
}
return errors.New("ntfy timeout")
}
func waitGone(t *testing.T, link string) {
t.Helper()
require.Eventually(t, func() bool { _, err := os.Lstat(link); return os.IsNotExist(err) }, 10*time.Second, time.Millisecond)
}
// hyperguild#164: alerts were flushed in the scan goroutine, so with a hung
// ntfy and a full queue one flush (6 x notify timeout) delayed the next scan.
// The notifier here is slower than the interval, as in production (10 s
// timeout vs 30 s interval with 6 messages), so the alert handoff is full:
// a scan that waited on it would keep a link for seconds.
func TestWatchLinkLifetimeStaysNearOneIntervalWithANotifierSlowerThanTheInterval(t *testing.T) {
const interval = 100 * time.Millisecond
brain := t.TempDir()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go Watch(ctx, brain, interval, hungNotifier{latency: 400 * time.Millisecond})
for i := 0; i < 2*maxPending+3; i++ { // overfill queue and handoff
link := filepath.Join(brain, "fill"+strconv.Itoa(i))
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
}
var worst time.Duration
for i := 0; i < 6; i++ {
link := filepath.Join(brain, "probe"+strconv.Itoa(i))
require.NoError(t, os.Symlink(t.TempDir(), link))
planted := time.Now()
waitGone(t, link)
worst = max(worst, time.Since(planted))
}
assert.LessOrEqual(t, worst, interval+interval/2, "worst link lifetime %v with a hung notifier", worst)
}
// Review of PR #180: delivery waited for its own tick, about one interval
// after the removal. An alert is now flushed as soon as it is handed over.
func TestWatchDeliversARemovalAlertWellWithinOneInterval(t *testing.T) {
const interval = 400 * time.Millisecond
brain := t.TempDir()
n := &recNotifier{}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go Watch(ctx, brain, interval, n)
link := filepath.Join(brain, "prompt")
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
removed := time.Now()
require.Eventually(t, func() bool { return n.count() == 1 }, 2*time.Second, time.Millisecond)
assert.Less(t, time.Since(removed), interval/4)
}
// Review of PR #180: on ctx end, deliver returned without draining what the
// scan had handed over, so a removal just before shutdown was neither
// delivered nor counted. Watch now drains and flushes before returning.
func TestWatchDeliversAnAlertHandedOverJustBeforeShutdown(t *testing.T) {
for i := 0; i < 20; i++ {
brain := t.TempDir()
n := &recNotifier{}
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() { Watch(ctx, brain, 5*time.Millisecond, n); close(done) }()
link := filepath.Join(brain, "late")
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
cancel()
<-done
require.Equal(t, 1, n.count(), "iteration %d: the removal alert was lost at shutdown", i)
}
}
// blockingNotifier holds every Notify until its ctx ends, tracking calls in
// flight.
type blockingNotifier struct{ inFlight atomic.Int32 }
func (b *blockingNotifier) Notify(ctx context.Context, _ string) error {
b.inFlight.Add(1)
defer b.inFlight.Add(-1)
<-ctx.Done()
return ctx.Err()
}
// Watch does not return while its delivery goroutine is still notifying.
func TestWatchReturnsOnlyAfterDeliveryHasStopped(t *testing.T) {
prev := shutdownFlushTimeout
shutdownFlushTimeout = 50 * time.Millisecond
t.Cleanup(func() { shutdownFlushTimeout = prev })
brain := t.TempDir()
n := &blockingNotifier{}
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() { Watch(ctx, brain, 5*time.Millisecond, n); close(done) }()
link := filepath.Join(brain, "x")
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
require.Eventually(t, func() bool { return n.inFlight.Load() == 1 }, 2*time.Second, time.Millisecond)
cancel()
select {
case <-done:
case <-time.After(5 * time.Second):
t.Fatal("Watch did not return after cancel")
}
assert.Zero(t, n.inFlight.Load(), "Notify still running after Watch returned")
}
// slowToggleNotifier fails slowly (a hung ntfy) while down, then records.
type slowToggleNotifier struct {
recNotifier
down atomic.Bool
latency time.Duration
}
func (r *slowToggleNotifier) Notify(ctx context.Context, m string) error {
if r.down.Load() {
select {
case <-time.After(r.latency):
case <-ctx.Done():
}
return errors.New("ntfy timeout")
}
return r.recNotifier.Notify(ctx, m)
}
// Review of PR #180: with a slow notifier the scan's handoff overflows, and
// those alerts exist only in the overflow counter. Every removal is still
// delivered or counted in the summary.
func TestWatchCountsHandoffOverflowWithASlowNotifier(t *testing.T) {
brain := t.TempDir()
n := &slowToggleNotifier{latency: 30 * time.Millisecond}
n.down.Store(true)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go Watch(ctx, brain, 2*time.Millisecond, n)
const plants = 40
for i := 0; i < plants; i++ {
link := filepath.Join(brain, "ov"+strconv.Itoa(i))
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
}
n.down.Store(false)
delivered, counted := 0, 0
require.Eventually(t, func() bool {
n.mu.Lock()
defer n.mu.Unlock()
delivered, counted = 0, 0
for _, m := range n.msgs {
if strings.Contains(m, "removed 1 symlink(s)") {
delivered++
}
if i := strings.Index(m, "): "); i >= 0 && strings.Contains(m, "suppressed") {
var c int
_, _ = fmt.Sscanf(m[i+3:], "%d", &c)
counted += c
}
}
return delivered+counted == plants
}, 5*time.Second, 10*time.Millisecond, "delivered %d + counted %d of %d", delivered, counted, plants)
}
// An alert still queued when Watch stops (its send failed, the notifier has
// since recovered, the retry tick has not come) is flushed once more before
// Watch returns, on a ctx that is not the cancelled one: toggleNotifier fails
// on a done ctx, as the real ntfy notifier does.
func TestWatchFinalFlushSendsAQueuedAlertOnACtxThatOutlivesTheCancel(t *testing.T) {
brain := t.TempDir()
n := &toggleNotifier{}
n.down.Store(true)
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() { Watch(ctx, brain, 300*time.Millisecond, n); close(done) }()
link := filepath.Join(brain, "queued")
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
time.Sleep(20 * time.Millisecond) // let the immediate send fail
n.down.Store(false)
cancel()
<-done
assert.Equal(t, 1, n.count(), "the queued alert was not flushed at shutdown")
}
// lockedBuffer is a goroutine-safe log sink: Watch goroutines of earlier
// tests may still log while one test captures.
type lockedBuffer struct {
mu sync.Mutex
buf bytes.Buffer
}
func (b *lockedBuffer) Write(p []byte) (int, error) {
b.mu.Lock()
defer b.mu.Unlock()
return b.buf.Write(p)
}
func (b *lockedBuffer) String() string { b.mu.Lock(); defer b.mu.Unlock(); return b.buf.String() }
func captureLog(t *testing.T) *lockedBuffer {
t.Helper()
b := &lockedBuffer{}
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(b, nil)))
t.Cleanup(func() { slog.SetDefault(prev) })
return b
}
// PR #180 round 2: when the final flush fails (ntfy hung at shutdown) the
// undelivered alerts vanished with only "will retry" warnings. Watch now logs
// one error with both counts.
func TestWatchLogsUndeliveredAndSuppressedCountsWhenTheFinalFlushFails(t *testing.T) {
prev := shutdownFlushTimeout
shutdownFlushTimeout = 50 * time.Millisecond
t.Cleanup(func() { shutdownFlushTimeout = prev })
logs := captureLog(t)
brain := t.TempDir()
ctx, cancel := context.WithCancel(context.Background())
done := make(chan struct{})
go func() { Watch(ctx, brain, 5*time.Millisecond, hungNotifier{latency: time.Hour}); close(done) }()
link := filepath.Join(brain, "lost")
require.NoError(t, os.Symlink(t.TempDir(), link))
waitGone(t, link)
cancel()
<-done
out := logs.String()
assert.Contains(t, out, "level=ERROR")
assert.Contains(t, out, `msg="symlink tripwire alerts not delivered at shutdown"`)
assert.Contains(t, out, "undelivered=1")
assert.Contains(t, out, "suppressed=0")
}
// deliver, given a handoff that already holds alerts and is closed, moves them
// into the queue and runs the final flush, even though the ctx is cancelled:
// the drain-on-close path, without depending on when the close lands.
func TestDeliverFlushesAlertsDrainedFromAClosedHandoffOnACancelledCtx(t *testing.T) {
alerts := make(chan string, 3)
alerts <- "a1"
alerts <- "a2"
close(alerts)
ctx, cancel := context.WithCancel(context.Background())
cancel()
n := &toggleNotifier{}
var overflow atomic.Int64
deliver(ctx, time.Hour, n, alerts, &overflow)
assert.Equal(t, []string{"a1", "a2"}, n.msgs)
}
// PR #180 round 2: when the first receive after close finds the handoff empty,
// overflow counted by the scan was never moved into the summary.
func TestDeliverCountsOverflowWhenTheHandoffIsClosedAndEmpty(t *testing.T) {
alerts := make(chan string)
close(alerts)
n := &recNotifier{}
var overflow atomic.Int64
overflow.Store(3)
deliver(context.Background(), time.Hour, n, alerts, &overflow)
require.Len(t, n.msgs, 1)
assert.Contains(t, n.msgs[0], "3 further alert(s)")
}