mathias/hyperguild
1
0
Fork 0
Code Issues 30 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

ingestion pins mcp-chassis v0.2.0: auth rejections aren't logged; REST metrics collapse to path="/" #155

New Issue
Closed
opened 2026-10-03 14:42:23 +00:00 by mathias · 2 comments
mathias commented 2026-10-03 14:42:23 +00:00
Owner
Copy Link
Copy Source

Found verifying #149 live, 2026-10-03.

  1. Rejections are invisible. ingestion/go.mod pins git.d-ma.be/mathias/mcp-chassis v0.2.0. The mcp auth rejected audit line (reason, remote, token fingerprint, path) arrived in chassis d5aa5b9 (gitea-mcp#9), after v0.2.0; current is v0.7.1. Since #149 every REST route sits behind BearerMiddleware, so a caller I didn't know about would get silent 401s. Today the only signal is the 4xx counter. Bump the chassis (check the v0.3–v0.7 changelog: Fingerprint, the accepted-path naming, per-issuer JWKS client) and assert in a test that a rejection is logged.
  2. Metrics label. reg.Middleware labels by ServeMux pattern, so since mountREST every REST route reports as path="/". Per-route latency and 4xx breakdown for /query, /write etc. is lost. Label by the inner REST mux's pattern instead (e.g. middleware inside mountREST).

Baseline after deploy cfe3f24: brain_query_duration_seconds_count{path="/",status="4xx"} = 4, all from my verification probes. A rise there with no matching probe means an unknown caller is being refused.

Created via git-mcp on behalf of @mathias

Found verifying #149 live, 2026-10-03. 1. **Rejections are invisible.** `ingestion/go.mod` pins `git.d-ma.be/mathias/mcp-chassis v0.2.0`. The `mcp auth rejected` audit line (reason, remote, token fingerprint, path) arrived in chassis `d5aa5b9` (gitea-mcp#9), after v0.2.0; current is v0.7.1. Since #149 every REST route sits behind `BearerMiddleware`, so a caller I didn't know about would get silent 401s. Today the only signal is the 4xx counter. Bump the chassis (check the v0.3–v0.7 changelog: `Fingerprint`, the accepted-path naming, per-issuer JWKS client) and assert in a test that a rejection is logged. 2. **Metrics label.** `reg.Middleware` labels by ServeMux pattern, so since `mountREST` every REST route reports as `path="/"`. Per-route latency and 4xx breakdown for `/query`, `/write` etc. is lost. Label by the inner REST mux's pattern instead (e.g. middleware inside `mountREST`). Baseline after deploy `cfe3f24`: `brain_query_duration_seconds_count{path="/",status="4xx"}` = 4, all from my verification probes. A rise there with no matching probe means an unknown caller is being refused. --- _Created via git-mcp on behalf of @mathias_
mathias added the risk/lowsize/stype/fix labels 2026-10-03 14:42:23 +00:00
mathias commented 2026-10-05 14:21:57 +00:00
Author
Owner
Copy Link
Copy Source

pr-gatekeeper verdict: BLOCKED (PR #181)

PR #181 (head 474473c) is blocked. The full evidence is at #181 (comment).

In brief: the auth semantics, metrics labels and routing all checked clean by execution, and CI run 9376 is green. The block is one decorative assertion. The new rejection-log test claims "the credential itself is never logged", but it stays green when the chassis logs the raw presented token. No test checks that the mcp auth accepted line leaves out the valid token. Fix: use a sentinel token, assert the bare sentinel is absent on both lines, and re-run the flip. The labelRESTPattern "bounded" comment also overclaims for authenticated unknown paths. That gap predates this PR.

## pr-gatekeeper verdict: BLOCKED (PR #181) PR #181 (head `474473c`) is blocked. The full evidence is at https://git.d-ma.be/mathias/hyperguild/pulls/181#issuecomment-15483. In brief: the auth semantics, metrics labels and routing all checked clean by execution, and CI run 9376 is green. The block is one decorative assertion. The new rejection-log test claims "the credential itself is never logged", but it stays green when the chassis logs the raw presented token. No test checks that the `mcp auth accepted` line leaves out the valid token. Fix: use a sentinel token, assert the bare sentinel is absent on both lines, and re-run the flip. The labelRESTPattern "bounded" comment also overclaims for authenticated unknown paths. That gap predates this PR.
mathias commented 2026-10-05 14:41:11 +00:00
Author
Owner
Copy Link
Copy Source

pr-gatekeeper verdict: APPROVED and MERGED (PR #181, re-review)

PR mode. Head e05a13a, squash-merged as c0be5c4. PR label: risk/medium.

  • Round-1 blocker fixed. I re-ran my flip with a local chassis v0.7.1 swapped in by a replace directive. With deny logging the raw token, the suite went RED. With accept logging the raw token: RED. With deny logging the Authorization header: RED. With deny silenced to Debug: RED. With no change (the control): green. A transformed token in the log, for example uppercased, is not detected. The tests are honestly named "bare token", so that is acceptable.
  • The labelRESTPattern doc comment now states the limit for authenticated requests. That limit is tracked in #185. internal/startupretry is deleted and nothing references it.
  • Local task check (CI=1): exit 0, 0 FAIL lines, golangci-lint 0 issues on both modules. CI run 9409, Lint/Test/Vet: success on e05a13a.
  • The round-1 results on auth semantics, metrics and routing still hold. The code they covered is unchanged apart from the doc comment.

Not checked: live cluster rollout; whether any alert watches ingestion 5xx or jwt_dex_unavailable.

## pr-gatekeeper verdict: APPROVED and MERGED (PR #181, re-review) PR mode. Head `e05a13a`, squash-merged as `c0be5c4`. PR label: risk/medium. - Round-1 blocker fixed. I re-ran my flip with a local chassis v0.7.1 swapped in by a `replace` directive. With `deny` logging the raw token, the suite went RED. With `accept` logging the raw token: RED. With `deny` logging the Authorization header: RED. With `deny` silenced to Debug: RED. With no change (the control): green. A *transformed* token in the log, for example uppercased, is not detected. The tests are honestly named "bare token", so that is acceptable. - The labelRESTPattern doc comment now states the limit for authenticated requests. That limit is tracked in #185. `internal/startupretry` is deleted and nothing references it. - Local `task check` (CI=1): exit 0, 0 FAIL lines, golangci-lint 0 issues on both modules. CI run 9409, Lint/Test/Vet: success on e05a13a. - The round-1 results on auth semantics, metrics and routing still hold. The code they covered is unchanged apart from the doc comment. Not checked: live cluster rollout; whether any alert watches ingestion 5xx or `jwt_dex_unavailable`.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agent/ready
Meets the agent-ready contract; authorizes unattended execution
 blocked/client
Waiting on the client
 blocked/supplier
Waiting on a supplier
 blocked/upstream
Waiting on an upstream project or vendor
 risk/high
Irreversible, security-relevant, or control plane
 risk/low
Reversible, local, test-covered
 risk/medium
Touches shared state, deploys or data
 size/l
Too big - split it
 size/m
Under two days
 size/s
Under half a day
 size/xs
Under an hour
 status:dispatch
status:dispatched
status:review
status:reviewing
type/chore
Maintenance, no behaviour change
 type/decision
Human decision required; never agent/ready
 type/docs
Documentation only
 type/feat
New capability
 type/fix
Corrects broken behaviour
 type/ops
Infrastructure, deploy, runtime
 type/research
Investigate; output is a finding, not code
 type/test
Test coverage only
No labels risk/low size/s type/fix
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate-bot mathias
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mathias/hyperguild#155