internal/brain at 7c24528488, reviewed by claude-sonnet-5, verified by gemini-3.6-flash.
mission_analytics's ad-hoc-SQL branch (internal/brain/analytics.go) gates on opts.isAdmin instead of opts.isHumanAdmin, so a delegation token minted for a subagent under a superuser (which the project's own tests prove fails isHumanAdmin) can run arbitrary read SQL over the telemetry store, unlike every other admin-gated read/write in this package.
#!/bin/sh
cat > internal/brain/zzz_repro_analytics_test.go <<'EOF'
package brain
import (
"context"
"net/http"
"net/http/httptest"
"path/filepath"
"testing"
"time"
sdkauth "github.com/modelcontextprotocol/go-sdk/auth"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/pdbethke/corralai/internal/auth"
"github.com/pdbethke/corralai/internal/coord"
"github.com/pdbethke/corralai/internal/principals"
"github.com/pdbethke/corralai/internal/telemetry"
)
func TestMissionAnalyticsSQLGatesOnIsAdminNotHuman(t *testing.T) {
dir := t.TempDir()
cstore, err := coord.Open(filepath.Join(dir, "c.sqlite3"))
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { cstore.Close() })
pstore, err := principals.Open(filepath.Join(dir, "p.sqlite3"))
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { pstore.Close() })
if err := pstore.CreateSuperuser("boss@x.com", "test"); err != nil {
t.Fatal(err)
}
tel, err := telemetry.Open(filepath.Join(dir, "t.duckdb"))
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { tel.Close() })
vf := &auth.Verifier{}
vf.EnableDelegation([]byte("test-delegation-key-that-is-32-bytes!!"))
tok, err := vf.MintDelegation("boss@x.com", "boss@x.com/child", time.Hour)
if err != nil {
t.Fatal(err)
}
srv := NewServer(cstore, nil, Options{Principals: pstore, Telemetry: tel})
mcpHandler := mcp.NewStreamableHTTPHandler(
func(*http.Request) *mcp.Server { return srv },
&mcp.StreamableHTTPOptions{DisableLocalhostProtection: true},
)
handler := sdkauth.RequireBearerToken(vf.VerifyToken, nil)(mcpHandler)
ts := httptest.NewServer(handler)
t.Cleanup(ts.Close)
ctx := context.Background()
cl := mcp.NewClient(&mcp.Implementation{Name: "delegated-subagent", Version: "0"}, nil)
sess, err := cl.Connect(ctx, &mcp.StreamableClientTransport{
Endpoint: ts.URL,
HTTPClient: &http.Client{Transport: bearerRT{token: tok}},
}, nil)
if err != nil {
t.Fatalf("connect delegated subagent: %v", err)
}
t.Cleanup(func() { sess.Close() })
res, err := sess.CallTool(ctx, &mcp.CallToolParams{Name: "mission_analytics", Arguments: map[string]any{
"sql": "SELECT 1",
}})
if err != nil {
t.Fatalf("mission_analytics (delegation token) call: %v", err)
}
if res.IsError {
t.Fatalf("DEFECT NOT REPRODUCED: mission_analytics refused the delegation token: %+v", res)
}
t.Logf("DEFECT REPRODUCED: delegation token (fails isHumanAdmin) ran ad-hoc SQL via mission_analytics, which gates on isAdmin only")
}
EOF
OUT=$(go test -run TestMissionAnalyticsSQLGatesOnIsAdminNotHuman ./internal/brain/... -v 2>&1)
STATUS=$?
echo "$OUT"
if [ $STATUS -eq 0 ]; then
echo "RESULT: DEFECT REPRODUCED (test passed, meaning the delegation token's SQL call succeeded)"
exit 0
else
echo "RESULT: defect not reproduced"
exit 1
fi=== RUN TestMissionAnalyticsSQLGatesOnIsAdminNotHuman
zzz_repro_analytics_test.go:77: DEFECT REPRODUCED: delegation token (fails isHumanAdmin) ran ad-hoc SQL via mission_analytics, which gates on isAdmin only
--- PASS: TestMissionAnalyticsSQLGatesOnIsAdminNotHuman (0.03s)
PASS
ok github.com/pdbethke/corralai/internal/brain 0.039s
RESULT: DEFECT REPRODUCED (test passed, meaning the delegation token's SQL call succeeded)confirmed by pdbethke — real, and the fifth review's shape — a rule enforced at one door and not the other: admin.go says every admin door gates on isHumanAdmin; the ad-hoc SQL door gated on isAdmin, so a delegation token under a superuser could read the whole telemetry store. the reviewer's reproduction is kept as the regression test; fixed
ref 54e55c10537f8bb18f44226779c08879c192e50bf8a98386a6721595ad4abd54#R1
fleet_claims's per-principal rate limiter keys directly on identity(req, "anonymous") instead of the actor()-first pattern ask_fleet uses in the same file to avoid a documented rate-limit bypass via rotating caller identity, so fleet_claims may inherit the same bypass for delegated/subagent callers.
refuted by pdbethke — narrower than claimed: fleet_claims keys the limiter on identity(req, "anonymous") with a FIXED fallback, not a caller-supplied name, so the rotating-name bypass ask_fleet closed is not reachable here; a subagent's key is its delegation name, and minting delegations is itself admin-gated
ref 54e55c10537f8bb18f44226779c08879c192e50bf8a98386a6721595ad4abd54#R2
Recorded so that "found nothing here" can be told apart from "never looked here".
This scope shows a codebase unusually self-aware about one privilege-escalation shape (delegation tokens rolling up to superuser and passing isAdmin while failing isHumanAdmin), and it patched that shape thoroughly across writes and several sensitive reads. Finding 1 is a live, reproduced gap in that same pattern on mission_analytics's ad-hoc SQL branch. Finding 2 is the same class of gap by inference on fleet_claims's rate limiter, which skipped a fix ask_fleet's own comments describe needing. I did not find defects in the heavily-tested arithmetic paths (shard/mutant clamps, decorrelation seating, health/reclaim state machine, browser SSRF guard).