|
|
|
|
@@ -0,0 +1,174 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-261106-kt1
|
|
|
|
|
plan: 01
|
|
|
|
|
type: execute
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
files_modified:
|
|
|
|
|
- modules/pier/pier.go
|
|
|
|
|
- modules/pier/tracer_smoke_test.go
|
|
|
|
|
- modules/pier/README.md
|
|
|
|
|
- docs/services/mcp.md
|
|
|
|
|
- .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements:
|
|
|
|
|
- QUICK-261106-kt1
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 35000
|
|
|
|
|
raw_tokens: 35000
|
|
|
|
|
tasks: 2
|
|
|
|
|
confidence: low
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "A request to the MCP endpoint with a bad or missing bearer answers 401 whose WWW-Authenticate is `Bearer resource_metadata=\"<app.url>/.well-known/oauth-protected-resource<mcp.path>\"` and carries no scope parameter"
|
|
|
|
|
- "A spec-following MCP client therefore falls back to the PRM scopes_supported list when it requests authorization"
|
|
|
|
|
- "No 403 insufficient-scope path is introduced: RequireBearerTokenOptions.Scopes stays unset, so the SDK still never enforces a scope on every request"
|
|
|
|
|
- "SSE / streamable GET responses still flush: the SDK handler now receives the original ResponseWriter, which keeps its own http.Flusher"
|
|
|
|
|
- "pier's README, docs/services/mcp.md and the 19.7 security review row T-19.7-10 describe the challenge without a scope parameter"
|
|
|
|
|
artifacts:
|
|
|
|
|
- path: modules/pier/pier.go
|
|
|
|
|
provides: "bearer() wrapping auth.RequireBearerToken directly, no response-writer wrapper"
|
|
|
|
|
- path: modules/pier/tracer_smoke_test.go
|
|
|
|
|
provides: "bad-bearer and missing-bearer 401 assertions: resource_metadata present, no scope parameter"
|
|
|
|
|
key_links:
|
|
|
|
|
- from: modules/pier/pier.go bearer()
|
|
|
|
|
to: go-sdk auth.RequireBearerToken (v1.8.0 auth/auth.go:104-116)
|
|
|
|
|
via: "ResourceMetadataURL set, Scopes empty -> header is `Bearer resource_metadata=\"...\"` only"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
pier's 401 challenge on the MCP endpoint stops naming a fixed scope. Today the
|
|
|
|
|
response-writer wrapper in `modules/pier/pier.go` appends a read-only scope
|
|
|
|
|
parameter to every 401 `WWW-Authenticate`, and the MCP scope-selection rule makes
|
|
|
|
|
a client use that parameter instead of the PRM `scopes_supported`. A client that
|
|
|
|
|
follows the spec therefore asks only for the read scope, never receives a `write`
|
|
|
|
|
grant, and pier's read/write ceiling (`modules/pier/identity.go:80`, `writes()`)
|
|
|
|
|
hides every non-read-only tool with no step-up path. User decision (option 1):
|
|
|
|
|
the challenge carries only `resource_metadata` (RFC 6750 makes `scope` optional;
|
|
|
|
|
the MCP spec's server-side scope hint is a SHOULD, which is knowingly dropped here).
|
|
|
|
|
|
|
|
|
|
Purpose: clients request the advertised scope set and consent decides the grant.
|
|
|
|
|
Output: wrapper removed, smoke test pinned to the new header, README/docs and the
|
|
|
|
|
19.7 security review row updated.
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<execution_context>
|
|
|
|
|
@~/.claude/msd-core/workflows/execute-plan.md
|
|
|
|
|
@~/.claude/msd-core/templates/summary.md
|
|
|
|
|
</execution_context>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@.planning/STATE.md
|
|
|
|
|
@CLAUDE.md
|
|
|
|
|
|
|
|
|
|
Verified facts (do not re-investigate the live server):
|
|
|
|
|
- go-sdk is `github.com/modelcontextprotocol/go-sdk v1.8.0`. `auth.RequireBearerToken`
|
|
|
|
|
(`$(go list -m -f '{{.Dir}}' github.com/modelcontextprotocol/go-sdk)/auth/auth.go:97-125`)
|
|
|
|
|
writes `WWW-Authenticate: Bearer resource_metadata="<url>"` on 401/403 when
|
|
|
|
|
`ResourceMetadataURL` is set, and adds `scope=` ONLY when `opts.Scopes` is non-empty
|
|
|
|
|
(which would also enforce those scopes with 403 on every request). pier sets no
|
|
|
|
|
`Scopes`, so the SDK itself never emits a scope parameter.
|
|
|
|
|
- The wrapper type in `modules/pier/pier.go:288-310` exists only to append the scope;
|
|
|
|
|
its `Flush` and `Unwrap` methods only forward to the wrapped writer so the wrapper
|
|
|
|
|
does not hide the underlying `http.Flusher`. Removing the wrapper hands the SDK the
|
|
|
|
|
original writer, which keeps every capability it has. Nothing else in the repo
|
|
|
|
|
references the type (grep confirmed).
|
|
|
|
|
- `strings` stays imported in pier.go (used at lines ~166, ~220, ~222, ~332).
|
|
|
|
|
- `modules/pier/session.go:90` writes a bare 401 without `WWW-Authenticate` when no
|
|
|
|
|
identity is on the request (behind the bearer middleware, unreachable from outside);
|
|
|
|
|
it is out of scope and unchanged.
|
|
|
|
|
- `scripts/check-phase19.7.sh` has no grep on the challenge (only test file lists).
|
|
|
|
|
- `.planning/phases/19.7-generic-mcp-module/19.7-02-PLAN.md:252` mentions the old header;
|
|
|
|
|
it is a historical plan and is NOT edited.
|
|
|
|
|
- Another quick task (261106-kt2) is editing `modules/lagoon` concurrently: do not touch,
|
|
|
|
|
stage or format any lagoon file. Stage files explicitly by path.
|
|
|
|
|
- `summer` is not on PATH here; use `go run ./cmd/summer docs:build --check`.
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer">
|
|
|
|
|
<name>Task 1: Drop the scope parameter from pier's 401 challenge (code, smoke test, README, docs)</name>
|
|
|
|
|
<files>modules/pier/pier.go, modules/pier/tracer_smoke_test.go, modules/pier/README.md, docs/services/mcp.md</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- modules/pier/pier.go lines 248-311 (bearer() and the wrapper type)
|
|
|
|
|
- modules/pier/tracer_smoke_test.go lines 99-115 (bad-bearer assertion)
|
|
|
|
|
- modules/pier/README.md line 47
|
|
|
|
|
- docs/services/mcp.md line 141 (last sentences of the PRM paragraph)
|
|
|
|
|
</read_first>
|
|
|
|
|
<!-- planner-discipline-allow: challengeWriter -->
|
|
|
|
|
<action>
|
|
|
|
|
Per the user decision (option 1), the MCP endpoint's 401 challenge carries only resource_metadata.
|
|
|
|
|
|
|
|
|
|
1. modules/pier/pier.go: delete the challengeWriter type and its three methods (WriteHeader, Flush, Unwrap; lines ~288-310). In bearer(), return the middleware from auth.RequireBearerToken(verify, &auth.RequireBearerTokenOptions{ResourceMetadataURL: p.prmURL, AllowMissingExpiration: true})(next) directly instead of wrapping it in an http.HandlerFunc that substitutes the writer. Do NOT add a Scopes option: it would make the SDK answer 403 to every token lacking those scopes. Keep the verify closure unchanged. Rewrite the bearer() doc comment (lines ~248-251): a failure is a 401 whose WWW-Authenticate names only the protected resource metadata URL; it carries no scope parameter, so a client requests the scopes the metadata document advertises and the authorization server's consent decides the grant (tools are then filtered per granted scope). Neutral wording only; no consuming application or client product is named in code comments. Confirm the strings import is still used (it is, elsewhere in the file); run gofmt on the file.
|
|
|
|
|
|
|
|
|
|
2. modules/pier/tracer_smoke_test.go: in the bad-bearer block (~lines 99-114) keep the 401 and resource_metadata checks and replace the positive scope check with a negative one: the challenge must not contain the substring scope= at all. Make the expected header exact: the challenge equals Bearer resource_metadata="http://acme.test/.well-known/oauth-protected-resource/mcp". Update the comment above the block to say the challenge names only the metadata URL. Add a second request in the same block with no Authorization header (same initialize body and headers otherwise) asserting the same 401 and the same exact challenge, so both SDK failure paths (no bearer token, invalid token) are pinned.
|
|
|
|
|
|
|
|
|
|
3. modules/pier/README.md line 47: the 401 bullet states the header as Bearer resource_metadata="<app.url>/.well-known/oauth-protected-resource<mcp.path>" with no scope parameter, followed by one clause: there is no scope parameter, so a client requests the scopes_supported of the protected resource metadata. Keep the rest of the bullet (the Origin / 403 clause) as is.
|
|
|
|
|
|
|
|
|
|
4. docs/services/mcp.md line 141: replace the sentence that begins "A 401 carries" so the quoted header is Bearer resource_metadata="…" only, and add: it carries no scope parameter, so a client requests the scopes listed in the metadata document and consent decides what is granted. Keep the "which is how a client finds the authorization server" meaning and the following mcp.resource sentence.
|
|
|
|
|
|
|
|
|
|
Commit these four files together as one code commit, staged by explicit path (never git add -A, never a lagoon file), message: fix(pier): MCP 401 challenge names only resource_metadata. No co-author tag.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && FMT=$(gofmt -l modules/pier/) && test -z "$FMT" && go vet ./modules/pier/... && go test -short ./modules/pier/... && go test ./cmd/summer -run TestDocsTree && go run ./cmd/summer docs:build --check && test "$(grep -v '^\s*//' modules/pier/pier.go | grep -c challengeWriter)" = 0 && ! grep -n 'scope=\\"read\\"\|scope="read"' modules/pier/*.go modules/pier/README.md docs/services/mcp.md && grep -c 'resource_metadata' modules/pier/README.md docs/services/mcp.md</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>
|
|
|
|
|
bearer() wraps auth.RequireBearerToken with no writer substitution and no Scopes option; the wrapper type is gone; TestPierTracerSmoke asserts the exact header `Bearer resource_metadata="http://acme.test/.well-known/oauth-protected-resource/mcp"` for both a bad and a missing bearer; README and docs describe the header without a scope parameter; go vet, go test -short ./modules/pier/..., TestDocsTree and docs:build --check pass; one commit `fix(pier): ...` containing exactly the four files.
|
|
|
|
|
</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 2: Update security review row T-19.7-10 (planning doc, not in the code commit)</name>
|
|
|
|
|
<files>.planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md</files>
|
|
|
|
|
<read_first>
|
|
|
|
|
- .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md line 48 (row T-19.7-10)
|
|
|
|
|
</read_first>
|
|
|
|
|
<action>
|
|
|
|
|
Edit only row T-19.7-10 (line 48) with a scoped Edit. In the mitigation cell, replace the trailing clause about the RFC 9728 challenge so it reads: a failure is 401 with the RFC 9728 challenge naming only resource_metadata (no scope parameter since quick 261106-kt1, so clients request the PRM scopes_supported). Update the evidence cell's pier.go line range to the current bearer() lines after Task 1 (find them with grep -n 'func (p \*Pier) bearer' modules/pier/pier.go and the function's closing brace). Keep the test column (TestPierTracerSmoke bad bearer, now also missing bearer) and the status verified. Change no other row.
|
|
|
|
|
|
|
|
|
|
Do NOT commit this file in the code commit (planning docs and code are separate commits). Leave it modified and list it in the SUMMARY under files for the orchestrator's quick-task docs commit.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && test "$(grep 'T-19.7-10' .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md | grep -c '261106-kt1')" = 1 && ! grep 'T-19.7-10' .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md | grep -q 'scope="read"' && NS=$(git diff --numstat -- .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md) && test "$(printf '%s\n' "$NS" | awk '{print $1+$2}')" = 2</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>Row T-19.7-10 states the challenge names only resource_metadata, cites quick 261106-kt1 and the current bearer() line range; exactly one line changed in the file; the file is uncommitted and listed in the SUMMARY for the orchestrator's docs commit.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| MCP client -> pier `/mcp` | Untrusted bearer and request; every request re-verified by `auth.RequireBearerToken` with pier's resolver |
|
|
|
|
|
| MCP client -> OAuth authorization server | Client chooses which scopes to request; the server's allow-list and consent page decide the grant |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-261106-kt1-01 | Elevation of Privilege | 401 challenge without scope -> clients request the full PRM list (`write`, `ai`, `backend`, ...) | low | accept | The challenge is advisory; grants are unchanged server-side: wristband's authorize allow-list (T-19.7-09), granted = submitted ∩ requested with `backend` only with a link (T-19.7-33), and pier's per-request read/write ceiling and tool filter (`identity.go` `writes()`, T-19.7-13) still bound what a token reaches. |
|
|
|
|
|
| T-261106-kt1-02 | Spoofing | bearer() after wrapper removal | medium | mitigate | verify closure and RequireBearerToken options (ResourceMetadataURL, AllowMissingExpiration) unchanged; TestPierTracerSmoke pins 401 for a bad and a missing bearer. |
|
|
|
|
|
| T-261106-kt1-03 | Information Disclosure | WWW-Authenticate content | low | accept | Header names only the PRM URL, which is public; no scope or identity data added. |
|
|
|
|
|
| T-261106-kt1-04 | Denial of Service | streaming responses lose http.Flusher | low | mitigate | Wrapper removed rather than altered, so the SDK gets the original writer with its own Flusher; existing stream tests (TestEndpointGuards) keep running in `go test ./modules/pier/...`. |
|
|
|
|
|
| T-261106-kt1-SC | Tampering | npm/pip/cargo installs | low | accept | No package installs or dependency changes in this plan. |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- `go vet ./...` and `go test -short ./modules/pier/...` green.
|
|
|
|
|
- `go test ./cmd/summer -run TestDocsTree` and `go run ./cmd/summer docs:build --check` green.
|
|
|
|
|
- No `scope="read"` left in modules/pier, its README or docs/services/mcp.md; T-19.7-10 updated.
|
|
|
|
|
- `git show --stat HEAD` lists exactly the four Task 1 files; no lagoon file touched.
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
A 401 from the MCP endpoint carries `WWW-Authenticate: Bearer resource_metadata="…"` with no scope parameter for both bad and missing bearers, pinned by TestPierTracerSmoke; docs, README and the security review agree.
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/261106-kt1-pier-mcp-401-challenge-drops-the-fixed-r/261106-kt1-SUMMARY.md` when done. List `.planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md` as a planning-doc change for the orchestrator's docs commit.
|
|
|
|
|
</output>
|
Jakub Zych