fix(security,test): clear 3 SonarCloud vulnerabilities + flaky pipeline test

.github/workflows/scorecard.yml

@@ -13,9 +13,11 @@ on:
     - cron: "0 6 * * 1"
   workflow_dispatch:
 
-# Restrict the default GITHUB_TOKEN to read-only; the steps below request the
-# narrow scopes they actually need.
-permissions: read-all
+# Workflow-level default: minimum needed for actions/checkout. The analysis
+# job declares the additional narrow scopes (security-events: write, id-token:
+# write, actions: read) it actually needs.
+permissions:
+  contents: read
 
 jobs:
   analysis:

.github/workflows/security.yml

@@ -17,7 +17,11 @@ on:
   schedule:
     - cron: '21 4 * * 1'  # Mondays 04:21 UTC — catch newly-disclosed CVEs
 
-permissions: read-all
+# Workflow-level default: minimum needed for actions/checkout. Each job
+# declares the additional narrow scopes (security-events: write for SARIF
+# uploaders, contents: write for SBOM artifact upload) it actually needs.
+permissions:
+  contents: read
 
 jobs:
   osv-scanner:

internal/ingest/pipeline_test.go

@@ -239,8 +239,11 @@ func TestPipeline_PreservesInsertionOrder(t *testing.T) {
 	if err := p.Submit(healthyBatch()); err != nil {
 		t.Fatalf("submit: %v", err)
 	}
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().Processed == 1 }) {
-		t.Fatalf("worker did not process batch within deadline")
+	// Sync on the assertion target — the per-signal call sequence — rather
+	// than Stats().Processed, which can bump between BatchCreate calls under
+	// the race detector and trip the length check on a partial slice.
+	if !waitFor(t, 5*time.Second, func() bool { return len(w.snapshotOrder()) >= 3 }) {
+		t.Fatalf("worker did not record 3 calls within deadline (got %v)", w.snapshotOrder())
 	}
 
 	got := w.snapshotOrder()
@@ -273,17 +276,18 @@ func TestPipeline_CallbacksFireAfterPersistence(t *testing.T) {
 	if err := p.Submit(b); err != nil {
 		t.Fatalf("submit: %v", err)
 	}
-	if !waitFor(t, 2*time.Second, func() bool { return spanHits.Load() == 1 && logHits.Load() == 1 }) {
+	if !waitFor(t, 5*time.Second, func() bool { return spanHits.Load() == 1 && logHits.Load() == 1 }) {
 		t.Fatalf("callbacks did not fire (span=%d log=%d, want 1/1)", spanHits.Load(), logHits.Load())
 	}
 }
 
-func TestPipeline_FailedSpansSkipsLogs(t *testing.T) {
-	// When BatchCreateSpans fails, BatchCreateLogs must NOT run for that
-	// batch — preserves the invariant that orphan logs aren't persisted
-	// without their span. Mirrors the synchronous path's behavior of
-	// returning the span error before log insert.
-	w := &fakeWriter{spanErr: errors.New("span db down")}
+// runFailureSkipsCheck wires up a 1-worker pipeline with the configured
+// fakeWriter, submits a healthy batch, waits for the failure to surface,
+// then asserts that none of the forbidden BatchCreate* calls fired.
+// Shared by the trace-fails and span-fails skip tests so the boilerplate
+// (pipeline lifecycle + waitFor) lives in one place.
+func runFailureSkipsCheck(t *testing.T, w *fakeWriter, forbidden ...string) {
+	t.Helper()
 	p := NewPipeline(w, nil, PipelineConfig{Capacity: 2, Workers: 1})
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -293,41 +297,33 @@ func TestPipeline_FailedSpansSkipsLogs(t *testing.T) {
 	if err := p.Submit(healthyBatch()); err != nil {
 		t.Fatalf("submit: %v", err)
 	}
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().ProcessFailures > 0 }) {
+	if !waitFor(t, 5*time.Second, func() bool { return p.Stats().ProcessFailures > 0 }) {
 		t.Fatalf("expected ProcessFailures > 0, got %d", p.Stats().ProcessFailures)
 	}
 	calls := w.snapshotOrder()
 	for _, c := range calls {
-		if c == "logs" {
-			t.Fatalf("BatchCreateLogs ran after spans failed — order=%v", calls)
+		for _, f := range forbidden {
+			if c == f {
+				t.Fatalf("%s ran after upstream failure — order=%v", f, calls)
+			}
 		}
 	}
 }
 
+func TestPipeline_FailedSpansSkipsLogs(t *testing.T) {
+	// When BatchCreateSpans fails, BatchCreateLogs must NOT run for that
+	// batch — preserves the invariant that orphan logs aren't persisted
+	// without their span. Mirrors the synchronous path's behavior of
+	// returning the span error before log insert.
+	runFailureSkipsCheck(t, &fakeWriter{spanErr: errors.New("span db down")}, "logs")
+}
+
 func TestPipeline_FailedTracesAbortsBatch(t *testing.T) {
 	// Trace failures roll the entire batch back — atomic batches are the
 	// fix for orphan FK rows when a worker crashes between BatchCreate*
 	// calls. Spans and logs must NOT be persisted when the trace insert
 	// fails. Counterpart of TestPipeline_FailedSpansSkipsLogs.
-	w := &fakeWriter{traceErr: errors.New("transient")}
-	p := NewPipeline(w, nil, PipelineConfig{Capacity: 2, Workers: 1})
-	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-	p.Start(ctx)
-	t.Cleanup(p.Stop)
-
-	if err := p.Submit(healthyBatch()); err != nil {
-		t.Fatalf("submit: %v", err)
-	}
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().ProcessFailures > 0 }) {
-		t.Fatalf("expected ProcessFailures > 0, got %d", p.Stats().ProcessFailures)
-	}
-	calls := w.snapshotOrder()
-	for _, c := range calls {
-		if c == "spans" || c == "logs" {
-			t.Fatalf("spans/logs ran after trace failure — order=%v", calls)
-		}
-	}
+	runFailureSkipsCheck(t, &fakeWriter{traceErr: errors.New("transient")}, "spans", "logs")
 }
 
 func TestPipeline_DrainsOnStop(t *testing.T) {
@@ -493,15 +489,17 @@ func TestPipeline_PerTenantCap_ReleasedAfterProcess(t *testing.T) {
 	if err := p.Submit(mk()); err != nil {
 		t.Fatalf("submit 1: %v", err)
 	}
-	// Wait for the worker to drain it (and release the slot).
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().Processed == 1 }) {
+	// Wait for the worker to drain it (and release the slot). 5s tolerates
+	// the race detector's overhead on slow CI runners — the test passes
+	// locally in milliseconds.
+	if !waitFor(t, 5*time.Second, func() bool { return p.Stats().Processed == 1 }) {
 		t.Fatalf("worker did not process first batch")
 	}
 	// Second batch must succeed because the slot was released.
 	if err := p.Submit(mk()); err != nil {
 		t.Fatalf("submit 2 after release: %v", err)
 	}
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().Processed == 2 }) {
+	if !waitFor(t, 5*time.Second, func() bool { return p.Stats().Processed == 2 }) {
 		t.Fatalf("worker did not process second batch")
 	}
 	if got := p.TenantDropped(); got != 0 {
@@ -542,7 +540,7 @@ func TestPipeline_PanicInCallbackRecovered(t *testing.T) {
 	if err := p.Submit(good); err != nil {
 		t.Fatalf("submit good: %v", err)
 	}
-	if !waitFor(t, 2*time.Second, func() bool { return p.Stats().Processed >= 2 }) {
+	if !waitFor(t, 5*time.Second, func() bool { return p.Stats().Processed >= 2 }) {
 		t.Fatalf("worker did not survive callback panic — Processed=%d", p.Stats().Processed)
 	}
 	if p.Stats().ProcessFailures == 0 {

internal/ui/ui.go

@@ -2,6 +2,7 @@ package ui
 
 import (
 	"embed"
+	"errors"
 	"fmt"
 	"io/fs"
 	"net/http"
@@ -13,6 +14,33 @@ import (
 	"github.com/RandomCodeSpace/otelcontext/internal/vectordb"
 )
 
+// spaFS wraps an fs.FS so http.FileServer transparently serves index.html
+// for any extensionless path that doesn't resolve to a real file — the
+// usual single-page-app routing where the React router owns client-side
+// URLs. Asset-shaped paths (anything with a ".") still 404 normally so a
+// missing /favicon.ico doesn't surprise the browser with an HTML body.
+//
+// Wrapping the FS — rather than calling Open() against r.URL.Path in our
+// own handler — keeps the user-controlled name behind the stdlib
+// http.FileServer boundary, where path.Clean has already happened.
+type spaFS struct{ fs.FS }
+
+func (s spaFS) Open(name string) (fs.File, error) {
+	f, err := s.FS.Open(name)
+	if err == nil {
+		return f, nil
+	}
+	if !errors.Is(err, fs.ErrNotExist) {
+		return nil, err
+	}
+	// SPA fallback only for extensionless paths (treated as client-side
+	// routes); legitimate asset 404s still propagate.
+	if strings.Contains(name, ".") {
+		return nil, err
+	}
+	return s.FS.Open("index.html")
+}
+
 //go:embed static/* dist
 var content embed.FS
 
@@ -46,26 +74,15 @@ func (s *Server) SetMCPConfig(enabled bool, path string) {
 func (s *Server) RegisterRoutes(mux *http.ServeMux) error {
 	mux.Handle("/static/", http.FileServer(http.FS(content)))
 
-	// Serve React SPA from dist/ for all non-API paths.
-	// API routes are registered before this is called, so they take priority.
+	// Serve React SPA from dist/ for all non-API paths. API routes are
+	// registered before this is called, so they take priority. spaFS
+	// converts extensionless 404s into index.html so the React router
+	// can claim them.
 	distFS, err := fs.Sub(content, "dist")
 	if err != nil {
 		return fmt.Errorf("ui: failed to create dist sub-fs: %w", err)
 	}
-	fileServer := http.FileServer(http.FS(distFS))
-	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		// Try the file as-is; if not found, fall back to index.html (SPA routing).
-		f, openErr := distFS.Open(strings.TrimPrefix(r.URL.Path, "/"))
-		if openErr == nil {
-			_ = f.Close()
-			fileServer.ServeHTTP(w, r)
-			return
-		}
-		// SPA fallback — let the React router handle the path.
-		r2 := r.Clone(r.Context())
-		r2.URL.Path = "/"
-		fileServer.ServeHTTP(w, r2)
-	})
+	mux.Handle("/", http.FileServer(http.FS(spaFS{distFS})))
 
 	return nil
 }