SECURITY: ingestion REST routes have no in-process auth; brain_ingest/ingest-path read any server file #149
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mathias/hyperguild#149
Reference in New Issue
Block a user
Found by the PR #147 gatekeeper review, 2026-10-01. The public half was confirmed live and is mitigated in infra by restricting the ingress paths (infra#517). This issue is the durable code fix.
1. REST mux has no auth
ingestion/cmd/server/main.gowraps only/mcpinchassisauth.BearerMiddleware./query,/write,/index,/ingest,/ingest-path,/ingest-raw,/backfill-refs,/promote,/backfill-embeddings,/pending,/pass-rateand/metricshave none. After infra#517 they are no longer public, but they are still open to the tailnet (Tailscale Serve koala:8443 → NodePort) and to every pod the NetworkPolicy admits.Fix: wrap the REST routes in the same
BearerMiddleware(mcpToken, jwtValidator, ...)./metricscan stay open in-cluster for Prometheus, or move to its own port. In-cluster callers already send a token (brain-uiBRAIN_MCP_TOKEN, brain-ingest-ntfyBRAIN_TOKEN). Check the watcher and any CronJobs.2. Arbitrary file read
MCP
brain_ingestwithpathand RESTPOST /ingest-pathcallextract.Text(path)on a caller-chosen path. Any file the pod can read (e.g./var/run/secrets/kubernetes.io/serviceaccount/token) can be ingested into the brain and read back throughbrain_query.Fix: resolve the path against an allowlisted root (an inbox directory) with symlink-checked containment, as
notes.Resolvedoes, and refuse everything else. Test with../and symlink escapes.Done when
POST /queryreturns 401 on the NodePort, with a test.brain_ingestwithpath: /etc/hostnameis refused, with a test.risk/high: control code (rule 8), needs an adversarial review.
Created via git-mcp on behalf of @mathias
Pre-push review (infra#470 lane): mathias/infra 7aceee5, verdict PASS
This covers the unpushed commit
7aceee5onmathias/inframain ("fix(brain-ingest-ntfy): source the brain token from ESO, same item as ingestion"). It is prep for hyperguild#152. The reviewer did not push it.What I checked, all read-only and by running commands:
kubectl kustomize k3s/apps/brain-ingest-ntfyexits 0 and renders 5 objects.kubectl diff -kshows exactly two changes: the new ExternalSecretbrain-ingest-ntfy/brain-mcp-token, and the Deployment'sBRAIN_TOKENsecretKeyRef moving frombrain-ingest-ntfy-secrets/BRAIN_TOKENtobrain-mcp-token/BRAIN_MCP_TOKEN. Nothing else changes.secretStoreRef,targetanddata[].remoteRef(key: BRAIN_MCP_TOKEN,property: password) matchk3s/apps/supervisor/brain-mcp-token-externalsecret.yamlfield for field. ClusterSecretStoreonepasswordisReady=True("store validated"), and itsspec.conditionsis empty, so no namespace restriction applies. The same item already syncs into 4 other namespaces (supervisor, agent-runner, agentsquad-serve, brain-ui), and the existingbrain-ingest-ntfy/gitea-registryExternalSecret on this store isSecretSynced.CreateContainerConfigErrorand the old pod keeps serving. The kubelet retries, so the rollout finishes once the Secret exists. The secretKeyRef is non-optional, so the pod cannot start with an empty token. The worst case is a brief overlap of two subscribers (possible duplicate ingest), not a gap. That overlap happens on every rollout today and this change does not cause it. Fluxappshaswaitunset, so a stuck rollout would not turnappsNotReady.AGENTS/brain-ingest-ntfyat 65f4572 (the deployed image tag),BRAIN_TOKENis read incmd/brain-ingest-ntfy/main.go:32and used only asAuthorization: Bearerininternal/brain/client.go:45.brain-ingest-ntfy-secretsstill holdsNTFY_TOKEN(32 bytes), and the deployment still references it. That Secret has no labels or annotations (nokustomize.toolkit.fluxcd.io/*) and no manifest in the repo, so Flux does not own it and prune will leave it alone. ItsBRAIN_TOKENkey becomes unused. Deleting that key is a separate decision.supervisor/allow-callersalready admits namespacebrain-ingest-ntfyon 3300/TCP.describeand key byte lengths. No values were printed.task checkexits 0.Flip test. In a scratch worktree I broke both the Deployment's secret name (
brain-mcp-tokn) and the remoteRef key (BRAIN_MCP_TOKN) and rantask check: exit 0. No gate in this repo checks that a secretKeyRef points to a Secret that some manifest or ExternalSecret produces, or that a remoteRef names a real 1Password item. The commit has no test, which is acceptable because the change is declarative. But if it regressed, the result would be a silently stuck rollout. Nothing would page, because Fluxwaitis unset. task check is green, and that green covers none of the properties above.What I did not check. I did not compare the old hand-made
BRAIN_TOKENvalue with the 1Password item, because that would mean reading values. It doesn't matter here, because after this change the app's token comes from the same item ingestion uses. I could not observe the live sync of the new ExternalSecret, because it only exists after push. After the push, confirm withkubectl -n brain-ingest-ntfy get externalsecret brain-mcp-token(expect SecretSynced) and check that the rollout completed. Do both before hyperguild#152 deploys. The shared checkout also has an unrelated uncommitted change tok3s/apps/tapir/deployment.yaml. It is not in this commit and was not reviewed.Gatekeeper verdict: BLOCK (head
21a53c4)I reviewed this by running it, not by reading it. I built the real
ingestion/cmd/serverbinary, started it in an isolated env (sentinel token, OAuth plus capture routes enabled) and attacked it over HTTP and MCP.What held (verified against the live binary)
Bearer,Bearer(empty),Bearer tok(two spaces), lowercasebearerandBasic, for all methods (GET/HEAD/OPTIONS/PUT/DELETE/PATCH/CONNECT/TRACE). The same holds for unregistered paths and/. These paths return 401 directly:/query/,/%2Fquery,/%71uery,/query;x,/query%00,/QUERY,/mcp%2F..%2Fquery, the absolute-formPOST http://evil/query. These get a 307 to the clean path, which then needs auth://query,/./query,/mcp/../query,/capture/../query,/metrics/../query,/.well-known/../query./oauth/tokenrefuses empty or wrong credentials./capturewith no auth returns 401./metricsexposes only per-route latency and status-class counts./ingest-pathand MCPbrain_ingestboth. Refused:.., absolute paths outside raw/,/etc/hostname,/proc/self/environ, abs and rel symlinked files, a symlinked dir and paths through it, a sibling-prefix dirraw-evil/rawx. In a walk, symlinked files are skipped and symlinked dirs are not followed (sentinel not read). A missing raw/ fails closed.Blocking
main.gowiring has no test, and the done-when asks for a test of 401 on the NodePort.routes_test.gobuilds its own replica router. Two flips both stayed green ongo test ./cmd/server:main.go:528withreturn nextmux.HandleFunc("POST /query", h.Query)back onto the outer mux. That is the more specific pattern, so/querygoes public again.Either regression ships green. Suggested fix: pull the outer-mux construction out of
main()into a function and test that function. Or assert that every pattern on the outer mux is in an explicit public allowlist.TestMetricsStaysOpenForPrometheusonly checks a handler the test defines itself (rule 9). I wrapped the real/metricsinmain.gowith BearerMiddleware and the test stayed green.hyperguild brain pass-ratenow gets 401.cmd/hyperguild/http.goPassRate()never callsc.authorize(req). I reproduced it: withBRAIN_MCP_TOKENset, it sends noAuthorizationheader and getsstatus 401.TestBrainClientSendsBrainMCPTokenonly exercises Query and Write, so its name claims more than it checks and hides this (rule 9). The PR's caller audit also leaves out/pass-rate.Should fix / follow-up (not the sole reason for the block)
ingestpath_root.goputs the EvalSymlinks error into the response.../../../../../../../etc/passwdgives the generic refusal,.../etc/nonexistent-zzgiveslstat /etc/nonexistent-zz: no such file or directory, and.../root/.sshgivespermission denied. It also leaks the absolute brain dir. Return the generic error and log the detail.raw/x.md -> <outside file>is refused by/ingest-path. YetGET /pendingreturned the outside file's contents in its excerpt (sentinel found 2 times, bytes not printed).brain_pendinguses the same code.brain_promote(promote.go:108) would copy the target into wiki/, and brain-sync would push it. raw/ is writable by a remote party:/var/lib/supervisor/brainis a Syncthingsendreceivefolder shared with flamingo, raw/ is not in.stignore, and Syncthing syncs symlinks. It is also writable by uid 1000 on koala and by the brain-* CronJobs that mount the hostPath.ResolveIngestPathandextract.Text(path), which reopens the file by name. Exploiting it needs write access to raw/ (see 5) plus a race, so I rate it low. The proper fix is to open beneath the root (os.OpenRoot/ openat2 RESOLVE_BENEATH) and extract from that file handle.raw -> /,ResolveIngestPath(d, "/etc/hostname")succeeds. That's by design, but nothing documents or tests it.AGENTS.md(~l.489) says the REST API "remains available" with no mention of auth.docs/runbooks/exercising-pass-rate-traffic.md:27says/pass-rateneeds auth "none (read-only)".Flip results
mountRESTmountsrestwithout authmain.goauth closure becomes identitymain.gore-registers publicPOST /querymain.go/metricswrapped in authResolveIngestPathdrops EvalSymlinksbrain_ingestconfinement removedWritestops sending the tokeninternal/brain.Querystops sending the tokenGates
CI=1 task checkin a clean clone: exit 0, 0 FAIL. CI run 8859 (pull_request,21a53c4) succeeded. CI is green, but green covers none of findings 1-3.Callers checked: brain-ui (
/query) and brain-ingest-ntfy (/ingest) sendBearer <token>when the token is set. Infra 7aceee5 points the ntfy token at the ESO item. agentsquad, dispatch and agent-runner use only/mcp. Watcher and claudewatcher run in-process.ai-sessions/ingest_sessions.pysends the token. Not checked: brain-gardener (no local checkout), and whether the live tokens are equal.Gatekeeper verdict, round 2: PASS (head
b5c2271). No blocking findings remain.I rebuilt the real
ingestion/cmd/serverbinary fromb5c2271, started it in an isolated env (invented sentinel token, OAuth plus capture enabled) and re-ran the whole round-1 attack set.Results against the real binary
/return 401 for all methods and all auth-header variants: none, wrong,Bearer,Bearer, two spaces, lowercase,Basic. Encoded, dot-segment and absolute-form paths also return 401. Non-clean paths get a 307 to/query, which still needs the token. A valid token returns 200./metricsand the OAuth metadata stay open./oauth/tokenrefuses bad credentials./mcpand/capturereturn 401 without auth..., absolute paths,/etc/hostname,/proc/self/environ, abs and rel symlinked files, a symlinked dir. A walk skips the inner symlink (outside sentinel count 0)..../etc/passwd,.../etc/nonexistent-zzand.../root/.sshnow return identical text, with no brain dir in it.Flip table (round 2)
public["POST /query"]build router: buildMux: "POST /query" is not an allowed public route. The suite stays green; CI can't catch this, the startup check does. The deployment uses the default RollingUpdate, so the old pod keeps serving.return nextPOST /queryadded topublicRoutesmountRESTwithout authdo()drops the Authorization headerPOST /querymcpSrvtopublic["/mcp"]unwrappedResiduals (not blocking; recorded here so they are not lost)
buildMuxandListenAndServestill ships green. That leaves one untested line of wiring, which is a reasonable bar, but the wording claims slightly more than the test proves.publicRoutessays each public route "authenticates itself". Nothing asserts that for/mcp: its BearerMiddleware is still applied in main.go, and the test router uses stubs. The wiring is the same as on main today. This deserves a follow-up test that/mcpwith no token returns 401 through the production assembly.Gates
CI=1 task checkin a clean clone atb5c2271: exit 0, 0 FAIL, 44 packages ok.b5c2271) succeeded. The PR is mergeable.Done-when
POST /queryreturns 401: verified on the real binary, and asserted throughbuildMux, the function production calls.brain_ingestwith/etc/hostnameis refused: verified over MCP, and covered by a test.Note on the merge lane: this is
risk/high. Under the gatekeeper procedure, a risk/high merge is Mathias's call. This PASS is the review evidence, not the merge authorisation.Deployed and verified live. #152 squash-merged as
cfe3f24; CD rolled outingestion:cfe3f24. Mathias approved the merge after the round-2 PASS.10.0.1.20:30330koala:8443Done-when:
POST /query→ 401: verified on both, and asserted throughbuildMux, which production calls.brain_ingestwith/etc/hostnamerefused: verified by the gatekeeper on the real binary, and covered by a test.Callers: brain-ingest-ntfy now uses the ESO token (infra
7aceee5, synced and rolled out); brain-ui unchanged; the hyperguild CLI sends the token; ai-sessions repointed (b590f6e). After the deploy, the 4xx counter on the REST routes equals my 4 probes, so no other caller has been refused so far.Follow-ups: #153 (raw/ symlink readers, Syncthing write path, TOCTOU), #154 (
/mcpauth test, re-wrap gap), #155 (chassis v0.2.0 hides rejection logs; metrics label). Closing.Created via git-mcp on behalf of @mathias