Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a917966d31 | ||
|
|
99abe88b6a | ||
|
|
16a20088a3 | ||
|
|
8ebfedf64c | ||
|
|
609caa4f3f |
@@ -1071,6 +1071,8 @@ Recent decisions affecting current work:
|
||||
| 261106-dv0 | shared plugins log boot details at Debug: paymentgateway operators registered, paymentgatewaysubscriptions registries collected through diary.For (todo 2026-10-28-plugins-log-boot-details-at-debug.md) | 2026-11-06 | 8a88b86, 1df6a26 | [261106-dv0-plugin-boot-logs-at-debug](./quick/261106-dv0-plugin-boot-logs-at-debug/) |
|
||||
| 261106-e5v | purge token rename leftovers: grzybyfunkcjonalne check-deploy refuses NUXT_SITE_* keys, journal/menu/faq READMEs name NUXT_CACHE_PURGE_TOKEN (todo 2026-10-20-rename-site-cache-purge-env-in-consumers.md) | 2026-11-06 | 4e2cf0c, f61c90a, 128a33d, a53511b | [261106-e5v-purge-token-rename-leftovers](./quick/261106-e5v-purge-token-rename-leftovers/) |
|
||||
| 261106-jhk | wristband MCP authorize drops OIDC identity scopes (openid, profile, email) so the ChatGPT connector can connect | 2026-11-06 | aba678b | [261106-jhk-wristband-mcp-authorize-ignores-oidc-ide](./quick/261106-jhk-wristband-mcp-authorize-ignores-oidc-ide/) |
|
||||
| 261106-kt1 | pier MCP 401 challenge names only resource_metadata, so clients request the advertised scopes (write included) | 2026-11-06 | 609caa4 | [261106-kt1-pier-mcp-401-challenge-drops-the-fixed-r](./quick/261106-kt1-pier-mcp-401-challenge-drops-the-fixed-r/) |
|
||||
| 261106-kt2 | deferred:purge, migrate* and queue:clear reuse the app's published database, so the in-server scheduler no longer fails with duplicate provider for *sql.DB | 2026-11-06 | 99abe88 | [261106-kt2-lagoon-deferred-purge-fails-in-the-sched](./quick/261106-kt2-lagoon-deferred-purge-fails-in-the-sched/) |
|
||||
|
||||
## Deferred Items
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@ A third fix found by the tests is functional, not a security gap: c29d228 `fix(p
|
||||
| T-19.7-07 | `payload` is accepted only for an action with a Prompt, only as a JSON string, trimmed, non-empty and capped in runes; every other record action still refuses a payload; a plain action runs with an empty Value. | F `cabana/actions.go:217`, `:256` | `TestRecordActionPromptSmoke`, `TestRecordActionPrompt`, `TestRecordActionPromptRun` (F) | verified |
|
||||
| T-19.7-08 | Contributed labels, statuses, messages and URLs render as text and attributes (no `v-html`); a URL opens with `noopener` and the visible link carries `rel="noopener noreferrer"`. | F `admin/src/components/account/AccountContributions.vue` | `admin/tests/account/AccountContributions.test.ts` ("Text, never HTML", URL attributes) (F) | corrected: fixed in 44835e4, 88559d5 (F); was verified (see Corrections) |
|
||||
| T-19.7-09 | wristband's authorize allow-list is exactly `Options.ScopesSupported`; the default server refuses `backend`; `openid` and unlisted scopes stay refused on the MCP flavour. | F `wristband/authorize.go:31`, `:231` | `TestMCPScopesSmoke`, `TestMCPScopes` (F) | verified |
|
||||
| T-19.7-10 | Every request re-verifies its bearer (`auth.RequireBearerToken` with pier's resolver); a session never replaces it; a failure is 401 with the RFC 9728 challenge and `scope="read"`. | F `pier/pier.go:235-260`; F `pier/identity.go:144` | `TestPierTracerSmoke` (bad bearer), `TestEndpointGuards` (stream ends after revocation), `TestMCPEndToEnd` "revoked token" (F, D) | verified |
|
||||
| T-19.7-10 | Every request re-verifies its bearer (`auth.RequireBearerToken` with pier's resolver); a session never replaces it; 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`). | F `pier/pier.go:256-274`; F `pier/identity.go:144` | `TestPierTracerSmoke` (bad bearer, missing bearer), `TestEndpointGuards` (stream ends after revocation), `TestMCPEndToEnd` "revoked token" (F, D) | verified |
|
||||
| T-19.7-11 | The session record stores only the binding (`oauth:<client>:<user>` or the personal token id), compared on every POST, GET and DELETE (403 `session_binding`); ids from `rand.Text()`; sliding idle TTL; per-binding cap. | F `pier/session.go:143`, `:211`, `:242`, `:139` | `TestPierSessionsSmoke`, `TestSessions` (Redis and memory) (F) | verified |
|
||||
| T-19.7-12 | The inner request is built fresh (`httptest.NewRequestWithContext`) with no Authorization or Cookie; identity travels only in the marker; client address, X-Forwarded-For, X-Real-IP and Accept-Language are copied. | F `pier/dispatch.go:159-180` | `TestPierTracerSmoke` (echo route `auth: false`), `TestDispatch` (outer Cookie and Authorization absent) (F) | verified |
|
||||
| T-19.7-13 | A backend subject exists only with `backend` in the grant AND a link the resolver confirms per request (5 s cache), or an admin `g15_` token behind `mcp.allow_personal_tokens`; the read/write ceiling applies on top; list and call use the same `allowed`. | F `pier/identity.go:152`, `:187-198`; F `pier/filter.go:16`, `:37` | `TestToolsList` (no link, no backend tool; union with a link), `TestPierTracerSmoke` "personal tokens behind the flag", `TestMCPEndToEnd` (no backend tool without the scope) (F, D) | verified |
|
||||
|
||||
@@ -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>
|
||||
@@ -0,0 +1,79 @@
|
||||
---
|
||||
phase: quick-261106-kt1
|
||||
plan: 01
|
||||
subsystem: pier
|
||||
tags: [mcp, oauth, rfc9728, www-authenticate]
|
||||
status: complete
|
||||
requires: []
|
||||
provides:
|
||||
- "pier MCP 401 challenge carrying only resource_metadata (no scope parameter)"
|
||||
affects:
|
||||
- modules/pier
|
||||
- docs/services/mcp.md
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "bearer() returns auth.RequireBearerToken(...)(next) directly; no ResponseWriter wrapper"
|
||||
key-files:
|
||||
created: []
|
||||
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
|
||||
decisions:
|
||||
- "MCP 401 challenge names only resource_metadata; the MCP spec's server-side scope hint (SHOULD) is knowingly dropped so clients request the PRM scopes_supported and consent decides the grant (user decision, option 1)"
|
||||
metrics:
|
||||
duration: ~10m
|
||||
completed: 2026-11-06
|
||||
estimate:
|
||||
tokens: 35000
|
||||
tasks: 2
|
||||
actuals:
|
||||
tokens: 3500
|
||||
tasks: 2
|
||||
commits: 1
|
||||
plan_head_before: ef47baf0a14379e37afe1b96fa2f6cf604e3337f
|
||||
plan_head_after: 609caa4f3f1f24f123696ab1d8fd8b6bfcef113a
|
||||
---
|
||||
|
||||
# Quick 261106-kt1: pier MCP 401 challenge drops the fixed read scope
|
||||
|
||||
pier's MCP endpoint now answers a bad or missing bearer with `401` and `WWW-Authenticate: Bearer resource_metadata="<app.url>/.well-known/oauth-protected-resource<mcp.path>"`. The header has no scope parameter. The response-writer wrapper that appended `scope="read"` is gone, so a client that follows the spec requests the PRM `scopes_supported` and can receive `write`.
|
||||
|
||||
## What changed
|
||||
|
||||
- **`modules/pier/pier.go`**: removed the `challengeWriter` type and its `WriteHeader`/`Flush`/`Unwrap` methods. `bearer()` now returns `auth.RequireBearerToken(verify, &auth.RequireBearerTokenOptions{ResourceMetadataURL: p.prmURL, AllowMissingExpiration: true})(next)` directly. The verify closure is unchanged and no `Scopes` option is set, so there is still no 403 insufficient-scope path. The SDK now receives the original writer, which keeps its own `http.Flusher`. The doc comment says why the challenge carries no scope. `strings` is still imported and used elsewhere in the file.
|
||||
- **`modules/pier/tracer_smoke_test.go`**: the bad-bearer check is now a two-case loop (bad bearer, missing bearer). Each case asserts 401, the exact challenge `Bearer resource_metadata="http://acme.test/.well-known/oauth-protected-resource/mcp"`, and no `scope=` substring.
|
||||
- **`modules/pier/README.md`** and **`docs/services/mcp.md`**: the header is quoted without a scope parameter, and each says the client therefore requests the scopes the metadata document advertises.
|
||||
- **`.planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md`**: one line changed, row T-19.7-10. It now says the challenge names only `resource_metadata` "since quick 261106-kt1", the evidence cites `pier/pier.go:256-274`, and the test column reads "bad bearer, missing bearer". **Left uncommitted for the orchestrator's docs commit.**
|
||||
|
||||
## Commits
|
||||
|
||||
| Task | Commit | Files |
|
||||
| ---- | ------ | ----- |
|
||||
| 1 | 609caa4 `fix(pier): MCP 401 challenge names only resource_metadata` | modules/pier/pier.go, modules/pier/tracer_smoke_test.go, modules/pier/README.md, docs/services/mcp.md |
|
||||
| 2 | (uncommitted planning doc) | .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md |
|
||||
|
||||
## Verification
|
||||
|
||||
- `gofmt -l modules/pier/` is empty. `go vet ./modules/pier/...` and `go vet ./...` pass.
|
||||
- `go test -short ./modules/pier/...` passes, and so does the full `go test ./modules/pier/...` (Docker-backed, 10.3s).
|
||||
- `go test ./cmd/summer -run TestDocsTree` passes. `go run ./cmd/summer docs:build --check` reports no problems.
|
||||
- No `challengeWriter` or `scope="read"` is left in the code, the pier README or docs/services/mcp.md. The only remaining mention is the historical 19.7-02-PLAN.md, which was intentionally not edited.
|
||||
- The Task 2 check passes: the row cites 261106-kt1, no `scope="read"` remains, and the numstat is 1+1.
|
||||
- Tracer gate: the `<verify>` chain was re-run end to end and passed before Task 2.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None. The plan was executed as written. The full Docker-backed pier test run and the repo-wide `go vet ./...` were extra checks beyond the plan.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None. No new surface was added. T-261106-kt1-02 (verify closure and options unchanged, 401 pinned for bad and missing bearer) and T-261106-kt1-04 (original writer kept, so Flusher is preserved; stream tests pass) are mitigated as planned.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- FOUND: 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
|
||||
- FOUND: commit 609caa4 (exactly the four Task 1 files; no lagoon/conga file touched)
|
||||
@@ -0,0 +1,222 @@
|
||||
---
|
||||
phase: quick-261106-kt2
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- modules/lagoon/commands.go
|
||||
- modules/lagoon/purge_test.go
|
||||
- modules/conga/schedule_test.go
|
||||
- modules/conga/commands.go
|
||||
- modules/conga/commands_test.go
|
||||
autonomous: true
|
||||
requirements:
|
||||
- QUICK-261106-kt2
|
||||
|
||||
estimate:
|
||||
tokens: 45000
|
||||
raw_tokens: 45000
|
||||
tasks: 2
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "When serve's in-process worker (or schedule:run) runs the framework entry summercms.lagoon[0]:deferred:purge, the command runs on the app's published database and succeeds; it no longer fails with `backpack: duplicate provider for *sql.DB`"
|
||||
- "`summer deferred:purge` from the CLI (no database published) still opens, publishes and closes its own pool and purges exactly as before (TestPurgeDeferredCommand unchanged and green)"
|
||||
- "A command run in-process never closes the shared pool it borrowed: after deferred:purge the published *sql.DB still answers PingContext and app.Lookup[*gorm.DB]() returns the same handle"
|
||||
- "migrate, migrate:rollback and migrate:status share lagoon's withDB helper and therefore also work in-process with a published database"
|
||||
- "queue:clear run in-process on an app with a published database (a plugin HasSchedule entry) clears the queue instead of failing with the duplicate-provider error"
|
||||
artifacts:
|
||||
- path: modules/lagoon/commands.go
|
||||
provides: "withDB reuses the published *gorm.DB when present; otherwise the unchanged open -> Publish -> Close CLI path"
|
||||
- path: modules/lagoon/purge_test.go
|
||||
provides: "TestPurgeDeferredCommandPublishedDB: deferred:purge through bonfire.Call on an app whose pool is already published, real purge counts, pool still open afterwards"
|
||||
- path: modules/conga/schedule_test.go
|
||||
provides: "TestScheduledDeferredPurgeInProcess: callScheduled and schedule:run --once run deferred:purge from lagoon.RuntimeCommands on an app with a published pool"
|
||||
- path: modules/conga/commands.go
|
||||
provides: "queue:clear uses withAppDB"
|
||||
- path: modules/conga/commands_test.go
|
||||
provides: "TestQueueClear also clears through bonfire.Call on the published app"
|
||||
key_links:
|
||||
- from: modules/conga/scheduler.go runScheduled / callScheduled
|
||||
to: modules/lagoon/commands.go deferred:purge Run -> withDB
|
||||
via: "bonfire.Catalog.Call in-process; withDB must take app.Lookup[*gorm.DB]() instead of lagoon.Publish a second time"
|
||||
- from: modules/lagoon/commands.go withDB
|
||||
to: modules/backpack/services.go Registry.Publish
|
||||
via: "the duplicate-provider error at services.go:36 is only reachable on the CLI-open branch, which now runs only when nothing is published"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Fix the nightly framework schedule entry `deferred:purge`, which fails every night
|
||||
in production since 2026-10-04 with
|
||||
`level=ERROR msg="schedule: command failed" component=conga command=deferred:purge error="backpack: duplicate provider for *sql.DB"`
|
||||
(framework v0.6.0; same code in v0.6.2). Root cause, verified by reading the code:
|
||||
lagoon's unexported `withDB` (`modules/lagoon/commands.go:486`) always calls
|
||||
`OpenFromApp` and then `lagoon.Publish`, which publishes `*sql.DB` on the app
|
||||
(`modules/lagoon/connection.go:169`). From the CLI nothing is published yet, so it
|
||||
works. Under serve the pool is already published (`modules/surf/serve.go:70-77`), and
|
||||
serve's in-process worker runs the entry through `runScheduled` -> `callScheduled` ->
|
||||
`bonfire.Catalog.Call` (`modules/conga/scheduler.go:148-182`), so the second Publish hits
|
||||
`backpack: duplicate provider for *sql.DB` (`modules/backpack/services.go:36`). The same
|
||||
happens under `schedule:run` (its `withDB` publishes first) and `schedule:run --once`
|
||||
(`withAppDB` publishes first). Plugin commands such as `chat:prune-pending` succeed because
|
||||
they take published services via `app.Lookup` and never publish a pool.
|
||||
|
||||
The fix follows the two guarded helpers that already exist: cabana's `withAdminDB`
|
||||
(`modules/cabana/commands.go:218`) and conga's `withAppDB` (`modules/conga/commands.go:340`):
|
||||
reuse the published handle when there is one, open/publish/close only for a CLI run.
|
||||
|
||||
Purpose: expired deferred bindings (abandoned uploads and records created under deferral)
|
||||
get purged again by the scheduler; the CLI command keeps working unchanged.
|
||||
Output: lagoon `withDB` fixed (deferred:purge plus the three migrate commands), conga
|
||||
`queue:clear` switched to `withAppDB`, Docker-backed regression tests on both the lagoon
|
||||
command and the conga scheduler path.
|
||||
</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 (planner read these; do not re-investigate):
|
||||
- Framework-registered `pact.ScheduledCommand` entries: only `lagoon.FrameworkSchedule`
|
||||
(`modules/lagoon/schedule.go:57`, `deferred:purge` daily at `database.deferred_bindings.purge_at`,
|
||||
default "03:00" in `app.timezone`, default UTC per `appLocation` in `modules/conga/schedule.go:91`).
|
||||
No other module returns a `ScheduledCommand`.
|
||||
- Sweep of command paths that open and publish a pool (`lagoon.OpenFromApp` + `lagoon.Publish`
|
||||
outside tests): lagoon `withDB` (migrate, migrate:rollback, migrate:status, deferred:purge:
|
||||
UNGUARDED, the bug); conga `withDB` (queue:work, schedule:run foreground, queue:clear:
|
||||
unguarded); cabana `withAdminDB` (guarded); surf serve (the pool owner, intentionally opens).
|
||||
`lagoon.Publish` is the only place that publishes `*sql.DB`.
|
||||
- `lagoon.OpenFromApp` also calls `loadEncryptionKeysFromApp` -> `PublishEncryptionKeys`, which is
|
||||
idempotent (`modules/lagoon/encrypted.go:130`). On the reuse branch the keys were already loaded
|
||||
by whoever opened the pool (serve); deferred:purge does not touch Encrypted columns.
|
||||
- `purgeBucket` (`modules/lagoon/commands.go:468`) already prefers the published `*blob.Bucket`.
|
||||
With no bucket and no `storage.uploads.bucket_url` it returns nil, and `PurgeDeferred` refuses only
|
||||
when an expired binding points at a file.
|
||||
- Under `bonfire.Call` `confirmProduction` never asks (`modules/lagoon/commands.go:32-50`).
|
||||
- Test helpers that exist: lagoon `newPurgeEnv`, `purgeConfig`, `purgePlugin`, `dedicatedDB`
|
||||
(`modules/lagoon/purge_test.go`, `postgres_test.go`; Docker, skip under -short via `lagoonDB`);
|
||||
conga `migratedDB`, `testApp` (publishes db + GORM like serve), `commandApp`, `testAppKey`
|
||||
(`modules/conga/postgres_test.go:113,154`, `listen_test.go:274,295`), `onceRun`, `mustTime`,
|
||||
`captureHandler` with `find(level, msg, key, value)` (`modules/conga/schedule_test.go:38-70,259`).
|
||||
- Docker tests: if the Docker socket is denied although the user is in the docker group, run
|
||||
them through `sudo -u golem` (CLAUDE.md).
|
||||
- Concurrency: quick task 261106-kt1 is editing `modules/pier` and `docs/services/mcp.md`; do not
|
||||
touch those. Stage files explicitly by path; leave `.planning/STATE.md`, `.planning/state.json`,
|
||||
`.planning/G15OFFICE.md` and `examples/hello/plugins.gen.go` (other sessions' edits) alone.
|
||||
- No README/docs change is expected: `withDB` and `withAppDB` are unexported, and the commands'
|
||||
flags, output, config keys and CLI behaviour are unchanged. A grep of `modules/lagoon/README.md`,
|
||||
`modules/conga/README.md`, `docs/plugins/scheduling.md` and `docs/console/` found no statement that
|
||||
these commands always open their own connection.
|
||||
|
||||
@modules/lagoon/commands.go
|
||||
@modules/conga/commands.go
|
||||
@modules/conga/scheduler.go
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer" tdd="true">
|
||||
<name>Task 1: Tracer - scheduler -> catalog -> deferred:purge runs on the app's published pool (test first, then fix lagoon withDB)</name>
|
||||
<files>modules/lagoon/purge_test.go, modules/conga/schedule_test.go, modules/lagoon/commands.go</files>
|
||||
<read_first>
|
||||
- modules/lagoon/commands.go (lines 304-366 deferred:purge Run, 464-496 purgeBucket and withDB)
|
||||
- modules/lagoon/purge_test.go (lines 49-73 newPurgeEnv, 386-400 purgeConfig, 444-503 purgePlugin and TestPurgeDeferredCommand)
|
||||
- modules/conga/scheduler.go (lines 145-200 runScheduled, callScheduled, scheduledCatalog)
|
||||
- modules/conga/schedule_test.go (lines 38-70 captureHandler, 259-310 onceRun and onceApp)
|
||||
- modules/conga/postgres_test.go (lines 111-176 migratedDB and testApp)
|
||||
- modules/cabana/commands.go (lines 218-234 withAdminDB, the pattern to mirror)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- lagoon TestPurgeDeferredCommandPublishedDB: an app whose pool was opened with OpenFromApp and published with Publish (as serve does) runs deferred:purge --days 2 through bonfire.Call and gets nil error and "purged 2 deferred bindings older than 2 days (1 files, 1 children, 0 skipped)"; afterwards the published *sql.DB still pings and app.Lookup[*gorm.DB]() is the same handle; migrate:status through bonfire.Call on the same app also returns nil.
|
||||
- conga TestScheduledDeferredPurgeInProcess: with testApp's published pool and a published bonfire.NewCatalog(lagoon.RuntimeCommands(app, nil)), callScheduled(ctx, app, log, "deferred:purge", nil, &buf) returns skipped false and nil error, buf contains "purged 0 deferred bindings older than 5 days", the capture handler has an Info "schedule: command finished" record with command=deferred:purge, the pool still pings; schedule:run --once at 2026-03-10 03:00:00 UTC returns nil and prints "Ran 1 due entries (0 failed)".
|
||||
- Before the fix both tests fail with an error containing "backpack: duplicate provider for *sql.DB" (the production error).
|
||||
- Existing TestPurgeDeferredCommand (CLI path through bonfire.NewRoot, nothing published) stays green unchanged.
|
||||
</behavior>
|
||||
<action>
|
||||
RED. In modules/lagoon/purge_test.go, directly after TestPurgeDeferredCommand, add TestPurgeDeferredCommandPublishedDB with a doc comment naming the bug (the in-process scheduler runs deferred:purge on an app whose pool serve already published). Build the environment the way TestPurgeDeferredCommand does: newPurgeEnv(t, "lagoon_purge_published"), one unattached file via e.file(t, ""), one created child via e.child, and two bindings backdated 3 days via e.bind (DeferredFileType for the file, "acme_purge_children" with DeferredEnvelope{Created: true} for the child). Create the app with backpack.New(purgeConfig(t, ...)) carrying database.dsn = e.dsn and app.key = base64 of 32 bytes of 7 (same as the existing test; app.key is required so that, before the fix, the failure is the duplicate-provider error and not the app.key error). Open the pool exactly as serve does: OpenFromApp(t.Context(), app), register t.Cleanup closing that sqlDB, then Publish(app, sqlDB, gdb); then attach.Publish(app, e.bucket). Run the command the way bonfire.Catalog.Call does for the scheduler: bonfire.Call(t.Context(), RuntimeCommands(app, plugins), "deferred:purge", []string{"--days", "2"}, &out) with plugins = a purgePlugin listing &purgeChild{}. Assert nil error, the output line above, that e.fileExists/e.blobExists/e.bindingIDs show the file, its blob and both bindings gone, that sqlDB.PingContext(t.Context()) still succeeds, and that app.Lookup[*gorm.DB]() returns the same gdb pointer. Then call bonfire.Call on the same app for "migrate:status" and assert nil error (the migrate commands share the helper).
|
||||
|
||||
In modules/conga/schedule_test.go add TestScheduledDeferredPurgeInProcess (Docker-backed through migratedDB, so it skips under -short). Use db, dsn := migratedDB(t) and app, _ := testApp(t, db, dsn, map with "app.key": testAppKey) — testApp publishes db and GORM on the app like serve. Publish slog.New over a fresh captureHandler on the app (as onceApp does), then publish bonfire.NewCatalog(lagoon.RuntimeCommands(app, nil)), the catalog the application binary publishes. Call callScheduled(t.Context(), app, loggerFromApp(app), "deferred:purge", nil, &buf) — the exact call runScheduled makes in serve's worker — and assert the behaviour above (skipped false, nil error, the "purged 0 deferred bindings older than 5 days" text in buf, the Info record via captureHandler.find, db.PingContext ok). Then run onceRun(t, app, nil, mustTime(t, time.UTC, "2026-03-10 03:00:00")) — the framework entry is due at the default 03:00 UTC — and assert nil error and "Ran 1 due entries (0 failed)" in the output. Add the lagoon import if schedule_test.go does not have it yet.
|
||||
|
||||
Run both new tests (commands in verify) BEFORE touching production code and confirm each fails with an error containing "backpack: duplicate provider for *sql.DB". Copy the failing lines into the SUMMARY as the RED evidence. Do not commit the failing tests on their own: CLAUDE.md requires go test ./... green at every commit, so tests and fix land in one commit.
|
||||
|
||||
GREEN. In modules/lagoon/commands.go change withDB: when app is non-nil and app.Lookup[*gorm.DB]() returns ok with a non-nil handle, call fn with that handle and return — no OpenFromApp, no Publish, and no Close, because the pool belongs to whoever published it (serve, schedule:run, queue:work, a test). Otherwise keep the current branch byte-for-byte: OpenFromApp, defer sqlDB.Close(), Publish, fn — so `summer deferred:purge` and the migrate commands from the CLI behave exactly as today. Mirror cabana's withAdminDB. Rewrite the doc comment of withDB in the repository's style: it runs fn on the app's published database when there is one (in-process runs: the scheduler in serve or schedule:run, bonfire.Catalog.Call), and otherwise opens and publishes a pool for the CLI run and closes it when fn returns. Do not change deferred:purge's Run body, purgeBucket, flags, output or Long text. This one change fixes deferred:purge and, through the shared helper, migrate, migrate:rollback and migrate:status.
|
||||
|
||||
Re-run: both new tests plus the existing TestPurgeDeferredCommand pass. Commit with message "fix(lagoon): run deferred:purge and the migrate commands on the app's published database (quick-261106-kt2)" and a body of two or three lines stating the production symptom and that the CLI path is unchanged. Stage exactly modules/lagoon/commands.go, modules/lagoon/purge_test.go and modules/conga/schedule_test.go by path. No Co-Authored-By trailer (user rule).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./modules/lagoon/... ./modules/conga/... && L=$(go test ./modules/lagoon -run 'TestPurgeDeferredCommand' -count=1 -v 2>&1) && echo "$L" | grep -q -- '--- PASS: TestPurgeDeferredCommand ' && echo "$L" | grep -q -- '--- PASS: TestPurgeDeferredCommandPublishedDB' && C=$(go test ./modules/conga -run 'TestScheduledDeferredPurgeInProcess' -count=1 -v 2>&1) && echo "$C" | grep -q -- '--- PASS: TestScheduledDeferredPurgeInProcess' && grep -n 'Lookup\[\*gorm.DB\]' modules/lagoon/commands.go</automated>
|
||||
</verify>
|
||||
<done>RED evidence (duplicate provider for *sql.DB from both new tests before the fix) is recorded; after the fix TestPurgeDeferredCommand, TestPurgeDeferredCommandPublishedDB and TestScheduledDeferredPurgeInProcess pass against testcontainers Postgres; lagoon withDB reuses a published *gorm.DB and never closes it; the CLI branch is unchanged; one commit with the three files.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Same fix for conga queue:clear, sweep record and full checks</name>
|
||||
<files>modules/conga/commands_test.go, modules/conga/commands.go</files>
|
||||
<read_first>
|
||||
- modules/conga/commands.go (lines 94-127 queue:clear, 338-345 withAppDB, 399-411 withDB)
|
||||
- modules/conga/commands_test.go (lines 20-85 TestQueueClear)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- TestQueueClear additionally clears through bonfire.Call on the published app: while the blocking job still holds the single default worker slot, two more pending ping jobs are enqueued, and queue:clear via bonfire.Call(ctx, RuntimeCommands(app, nil), "queue:clear", nil, &out) returns nil with "Cleared 2 jobs" in the output; the existing final assertions (block completes, zero ping rows left, zero pings ran) still hold.
|
||||
- Before the fix that new step fails with "backpack: duplicate provider for *sql.DB".
|
||||
- The existing CLI-style runClear (commandApp, nothing published) is unchanged and still passes.
|
||||
</behavior>
|
||||
<action>
|
||||
RED. In modules/conga/commands_test.go extend TestQueueClear: after the existing runClear assertions and BEFORE close(release) (the blocking job keeps the one default worker slot busy, so new pings stay pending), enqueue two more pingArgs jobs with m.Enqueue as the test already does (use a different Tag, for example "pending-in-process"), then run queue:clear through bonfire.Call(ctx, RuntimeCommands(app, nil), "queue:clear", nil, &out) on the test's own published `app` — the in-process path a plugin pact.HasSchedule entry naming queue:clear would take. Assert nil error and "Cleared 2 jobs" in the output. Leave the existing runClear (commandApp, the CLI path) and the final assertions as they are. Run it and confirm the new step fails with "backpack: duplicate provider for *sql.DB"; record it in the SUMMARY.
|
||||
|
||||
GREEN. In modules/conga/commands.go change only queue:clear's Run to call withAppDB instead of withDB. Leave queue:work and schedule:run (foreground) on withDB: both are long-running processes started from the CLI that own their pool, and running either as an in-process scheduled entry is not meaningful. Extend withAppDB's doc comment to say it serves schedule:run --once and queue:clear, which may run in-process on an app that already published its pool. Do not change flags, output, the confirmation question or the Long text.
|
||||
|
||||
Sweep record for the SUMMARY (no code change beyond this task): framework ScheduledCommand registrations are only lagoon.FrameworkSchedule's deferred:purge; remaining unguarded OpenFromApp + Publish paths are conga withDB for queue:work and schedule:run (CLI pool owners, intentionally) and surf serve (the pool owner); cabana withAdminDB was already guarded. Confirm with grep -rn 'lagoon.OpenFromApp' modules --include=*.go excluding _test.go.
|
||||
|
||||
Full checks (commands in verify): go vet ./..., go test -short ./... (CLAUDE.md keeps it green at every commit), then the Docker-backed go test ./modules/lagoon/... ./modules/conga/... -count=1. If the Docker socket is denied, rerun the Docker step through sudo -u golem per CLAUDE.md.
|
||||
|
||||
Commit with message "fix(conga): queue:clear reuses the app's published database (quick-261106-kt2)", staging exactly modules/conga/commands.go and modules/conga/commands_test.go by path. No Co-Authored-By trailer. Do not push or tag.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test -short ./... && Q=$(go test ./modules/conga -run 'TestQueueClear$' -count=1 -v 2>&1) && echo "$Q" | grep -q -- '--- PASS: TestQueueClear ' && grep -B1 'return clearQueue(ctx, app, queue, out)' modules/conga/commands.go | grep -q withAppDB && go test ./modules/lagoon/... ./modules/conga/... -count=1</automated>
|
||||
</verify>
|
||||
<done>queue:clear uses withAppDB; TestQueueClear passes both its CLI run and the new in-process run; go vet ./..., go test -short ./... and the Docker-backed lagoon and conga suites are green; sweep result recorded; one commit with the two conga files; nothing under modules/pier or docs/services/mcp.md touched.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| river_job rows -> scheduled command runner | A scheduled job row names an entry and command; runScheduled runs only an exact match of the compiled schedule table (T-11-09, unchanged) |
|
||||
| in-process command -> app's shared database pool | A command run through bonfire.Catalog.Call borrows the pool serve owns and must not close or replace it |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-261106-kt2-01 | Denial of Service | lagoon withDB / conga withAppDB reuse branch closing the borrowed pool would take serve's database away | high | mitigate | Reuse branch calls fn only, with no defer Close and no Publish; TestPurgeDeferredCommandPublishedDB and TestScheduledDeferredPurgeInProcess assert the published *sql.DB still pings and the same *gorm.DB stays published after the command |
|
||||
| T-261106-kt2-02 | Information Disclosure | deferred:purge failing nightly keeps abandoned pending uploads (files and records) past the configured retention (default 5 days) | medium | mitigate | The fix itself plus the scheduler-path regression test that runs the framework entry through callScheduled and schedule:run --once |
|
||||
| T-261106-kt2-03 | Elevation of Privilege | scheduler executing an arbitrary command from a forged river_job row | low | accept | Unchanged: runScheduled matches entry, command and args against the compiled table (T-11-09, covered by TestScheduledEntryMismatchSkipped); this plan does not touch scheduler.go |
|
||||
| T-261106-kt2-04 | Tampering | CLI branch regressions (migrate, deferred:purge from the terminal) | low | mitigate | CLI branch kept byte-for-byte; existing TestPurgeDeferredCommand (bonfire.NewRoot, nothing published) and TestQueueClear's runClear (commandApp) keep covering it |
|
||||
| T-261106-kt2-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 ./...` green.
|
||||
- Docker: `go test ./modules/lagoon/... ./modules/conga/... -count=1` green, including TestPurgeDeferredCommand, TestPurgeDeferredCommandPublishedDB, TestScheduledDeferredPurgeInProcess and TestQueueClear.
|
||||
- SUMMARY holds the RED output (duplicate provider for *sql.DB) for the three new test steps and the sweep result.
|
||||
- `git show --stat HEAD~1 HEAD` lists exactly the three Task 1 files and the two Task 2 files; nothing under modules/pier, docs/ or .planning/ in the code commits.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
The framework schedule entry deferred:purge succeeds when serve's worker, schedule:run or schedule:run --once runs it on an app whose database is already published, without closing that pool; `summer deferred:purge` and the migrate commands from the CLI behave as before; queue:clear works in-process too; all pinned by Docker-backed tests that failed with the production error before the fix.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/261106-kt2-lagoon-deferred-purge-fails-in-the-sched/261106-kt2-SUMMARY.md` when done, with the RED evidence, the sweep result, the two commit hashes, and a note that no README/docs change was needed (unexported helpers, command behaviour from the CLI unchanged). Note for the user: the fix reaches production only with the next framework patch tag and an app bump; do not tag or push here.
|
||||
</output>
|
||||
@@ -0,0 +1,106 @@
|
||||
---
|
||||
phase: quick-261106-kt2
|
||||
plan: 01
|
||||
subsystem: lagoon, conga
|
||||
tags: [scheduler, deferred-bindings, database-pool, bugfix]
|
||||
status: complete
|
||||
requires: []
|
||||
provides:
|
||||
- "lagoon withDB reuses the published *gorm.DB (deferred:purge, migrate, migrate:rollback, migrate:status work in-process)"
|
||||
- "conga queue:clear runs through withAppDB"
|
||||
affects: [conga scheduler, serve in-process worker, schedule:run]
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns: ["reuse the published handle, open/publish/close only for a CLI run (same as cabana withAdminDB, conga withAppDB)"]
|
||||
key-files:
|
||||
created: []
|
||||
modified:
|
||||
- modules/lagoon/commands.go
|
||||
- modules/lagoon/purge_test.go
|
||||
- modules/conga/schedule_test.go
|
||||
- modules/conga/commands.go
|
||||
- modules/conga/commands_test.go
|
||||
decisions:
|
||||
- "lagoon withDB borrows the published *gorm.DB without Publish or Close; the pool belongs to whoever published it"
|
||||
- "queue:work and foreground schedule:run stay on conga withDB: long-running CLI processes that own their pool"
|
||||
metrics:
|
||||
duration: ~15min
|
||||
completed: 2026-11-06
|
||||
actuals:
|
||||
tokens: 2070
|
||||
tasks: 2
|
||||
commits: 3
|
||||
plan_head_before: 609caa4f3f1f24f123696ab1d8fd8b6bfcef113a
|
||||
plan_head_after: 99abe88b6a33275817ae3f90d33b2f8a50717a2f
|
||||
---
|
||||
|
||||
# Quick 261106-kt2: deferred:purge fails in the scheduler, Summary
|
||||
|
||||
The nightly framework entry `deferred:purge` now runs on the app's published database pool under serve's in-process worker, `schedule:run` and `schedule:run --once`. It no longer fails with `backpack: duplicate provider for *sql.DB`. `queue:clear` got the same fix.
|
||||
|
||||
## What changed
|
||||
|
||||
- `modules/lagoon/commands.go` `withDB`: when the app already has a published `*gorm.DB`, `fn` runs on that handle. There is no `OpenFromApp`, no `Publish` and no `Close`. Otherwise the CLI branch (open, defer Close, Publish, fn) is unchanged. This fixes `deferred:purge`, `migrate`, `migrate:rollback` and `migrate:status`. It mirrors cabana's `withAdminDB`.
|
||||
- `modules/conga/commands.go`: `queue:clear` calls `withAppDB` instead of `withDB`, and the `withAppDB` doc comment now names schedule:run --once and queue:clear.
|
||||
|
||||
## Commits
|
||||
|
||||
| Task | Commit | Message |
|
||||
|------|--------|---------|
|
||||
| 1 | 16a2008 | fix(lagoon): run deferred:purge and the migrate commands on the app's published database (quick-261106-kt2) |
|
||||
| 2 | 99abe88 | fix(conga): queue:clear reuses the app's published database (quick-261106-kt2) |
|
||||
|
||||
`actuals.commits` reads 3 because the ledger range `609caa4..HEAD` also holds `8ebfedf`, a docs commit from the concurrent quick task 261106-kt1. This task made 2 commits.
|
||||
|
||||
## RED evidence (before the fix)
|
||||
|
||||
```
|
||||
=== RUN TestPurgeDeferredCommandPublishedDB
|
||||
purge_test.go:532: deferred:purge: backpack: duplicate provider for *sql.DB
|
||||
--- FAIL: TestPurgeDeferredCommandPublishedDB (0.24s)
|
||||
|
||||
schedule_test.go:275: callScheduled: skipped=false err=conga: scheduled command deferred:purge: backpack: duplicate provider for *sql.DB
|
||||
--- FAIL: TestScheduledDeferredPurgeInProcess (0.28s)
|
||||
|
||||
commands_test.go:86: in-process queue:clear: backpack: duplicate provider for *sql.DB
|
||||
--- FAIL: TestQueueClear (10.38s)
|
||||
```
|
||||
|
||||
TestPurgeDeferredCommand (the CLI path) passed both before and after the fix.
|
||||
|
||||
## GREEN / verification
|
||||
|
||||
- `go vet ./...`: ok
|
||||
- `go test -short ./...`: ok
|
||||
- Docker: `go test ./modules/lagoon/... ./modules/conga/... -count=1`: all ok (lagoon 22.2s, lagoon/attach 40.8s, lagoon/attach/webp, conga 31.5s). Includes TestPurgeDeferredCommand, TestPurgeDeferredCommandPublishedDB, TestScheduledDeferredPurgeInProcess and TestQueueClear.
|
||||
- The new tests check that the published `*sql.DB` still answers PingContext after the command and that `app.Lookup[*gorm.DB]()` returns the same handle (T-261106-kt2-01).
|
||||
|
||||
## Sweep result
|
||||
|
||||
`grep -rn 'OpenFromApp(' modules --include=*.go` (non-test):
|
||||
- `modules/lagoon/commands.go` withDB: now guarded (this task)
|
||||
- `modules/cabana/commands.go` withAdminDB: already guarded
|
||||
- `modules/conga/commands.go` withDB: used only by queue:work and foreground schedule:run. Both are long-running CLI processes that own their pool, so this is intentional. queue:clear moved to withAppDB.
|
||||
- `modules/surf/serve.go`: serve owns the pool, so it opens one on purpose.
|
||||
|
||||
The only framework `ScheduledCommand` registration is `lagoon.FrameworkSchedule` (`deferred:purge`).
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
**1. [Rule 3 - Blocking] TestQueueClear's app needed `app.key`**
|
||||
- **Found during:** Task 2 RED
|
||||
- **Issue:** testApp in TestQueueClear had no `app.key`, so the in-process queue:clear first failed with `lagoon: app.key is empty or invalid` rather than the production error.
|
||||
- **Fix:** added `"app.key": testAppKey` to that test's testApp config. The RED run then showed `backpack: duplicate provider for *sql.DB`. The CLI-style runClear and the final assertions are unchanged.
|
||||
- **Files modified:** modules/conga/commands_test.go
|
||||
- **Commit:** 99abe88
|
||||
|
||||
## Notes
|
||||
|
||||
- No README or docs change was needed: `withDB` and `withAppDB` are unexported, and the commands' flags, output, config keys and CLI behaviour are unchanged.
|
||||
- The fix reaches production only with the next framework patch tag and an app bump. Nothing was tagged or pushed.
|
||||
- No files under modules/pier or docs/services/mcp.md were touched.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- FOUND: modules/lagoon/commands.go, modules/lagoon/purge_test.go, modules/conga/schedule_test.go, modules/conga/commands.go, modules/conga/commands_test.go
|
||||
- FOUND: 16a2008, 99abe88
|
||||
@@ -138,7 +138,7 @@ Sessions follow the streamable HTTP transport: `initialize` opens one, `Mcp-Sess
|
||||
|
||||
## Protected resource metadata
|
||||
|
||||
pier serves the OAuth protected resource metadata (RFC 9728) at `/.well-known/oauth-protected-resource` and at `/.well-known/oauth-protected-resource<mcp.path>`: the resource, the authorization server (the verifier's resource origin, else `app.url`), the supported scopes and the bearer method. When the published verifier implements `pact.MCPAdvertisedScopes` and `pact.MCPAdvertisedScopes.MCPScopesSupported` is non-empty, that slice is copied; otherwise the document keeps `pier.PRMScopes` (`read`, `write`, `ai`, `offline_access`, `backend`). pier does not read `pact.MemberTokenScopeCatalog`. The list the OAuth server plugin publishes is the MCP advertised set: the fixed names `read`, `write`, `ai`, `offline_access` and `backend` are always on it. Declared member scopes follow the plugin's `golem15.oauthserver.mcp_scopes` filter, which is that plugin's key and not a framework `mcp.*` key: unset means every declared member scope, a set filter means only those declared names, and an empty filter means the five fixed names only. An empty `wristband.Options.ScopesSupported` on the MCP flavour resolves to those five names; a non-empty list is used as given. OpenID Connect discovery stays `openid`, `profile` and `email`. A route reads who called a tool with `pact.MCPCallFrom`. A 401 carries `WWW-Authenticate: Bearer resource_metadata="…", scope="read"`, which is how a client finds the authorization server. `mcp.resource` must equal the resource the OAuth server issues tokens for; a difference is logged as a warning at boot.
|
||||
pier serves the OAuth protected resource metadata (RFC 9728) at `/.well-known/oauth-protected-resource` and at `/.well-known/oauth-protected-resource<mcp.path>`: the resource, the authorization server (the verifier's resource origin, else `app.url`), the supported scopes and the bearer method. When the published verifier implements `pact.MCPAdvertisedScopes` and `pact.MCPAdvertisedScopes.MCPScopesSupported` is non-empty, that slice is copied; otherwise the document keeps `pier.PRMScopes` (`read`, `write`, `ai`, `offline_access`, `backend`). pier does not read `pact.MemberTokenScopeCatalog`. The list the OAuth server plugin publishes is the MCP advertised set: the fixed names `read`, `write`, `ai`, `offline_access` and `backend` are always on it. Declared member scopes follow the plugin's `golem15.oauthserver.mcp_scopes` filter, which is that plugin's key and not a framework `mcp.*` key: unset means every declared member scope, a set filter means only those declared names, and an empty filter means the five fixed names only. An empty `wristband.Options.ScopesSupported` on the MCP flavour resolves to those five names; a non-empty list is used as given. OpenID Connect discovery stays `openid`, `profile` and `email`. A route reads who called a tool with `pact.MCPCallFrom`. A 401 carries `WWW-Authenticate: Bearer resource_metadata="…"`, which is how a client finds the authorization server. It carries no scope parameter, so a client requests the scopes listed in the metadata document and consent decides what is granted. `mcp.resource` must equal the resource the OAuth server issues tokens for; a difference is logged as a warning at boot.
|
||||
|
||||
## Security notes
|
||||
|
||||
|
||||
@@ -120,7 +120,7 @@ asks first; --force skips the question. Off a terminal it never asks.`,
|
||||
if err := confirmProduction(app, in, out, fmt.Sprintf("Delete every pending job of queue %q in production?", queue)); err != nil {
|
||||
return err
|
||||
}
|
||||
return withDB(ctx, app, func() error {
|
||||
return withAppDB(ctx, app, func() error {
|
||||
return clearQueue(ctx, app, queue, out)
|
||||
})
|
||||
},
|
||||
@@ -336,7 +336,9 @@ func dueAt(c pact.Cadence, t time.Time) bool {
|
||||
}
|
||||
|
||||
// withAppDB runs fn with the app's database: the published one when there is
|
||||
// one, otherwise one opened for the command the way withDB does.
|
||||
// one, otherwise one opened for the command the way withDB does. It serves
|
||||
// schedule:run --once and queue:clear, which may run in-process on an app
|
||||
// that already published its pool.
|
||||
func withAppDB(ctx context.Context, app *backpack.App, fn func() error) error {
|
||||
if gdb, ok := app.Lookup[*gorm.DB](); ok && gdb != nil {
|
||||
return fn()
|
||||
|
||||
@@ -19,7 +19,7 @@ import (
|
||||
// none; a running job and other queues are left alone.
|
||||
func TestQueueClear(t *testing.T) {
|
||||
db, dsn := migratedDB(t)
|
||||
app, gdb := testApp(t, db, dsn, map[string]any{"queue.queues.default": 1})
|
||||
app, gdb := testApp(t, db, dsn, map[string]any{"queue.queues.default": 1, "app.key": testAppKey})
|
||||
m, err := From(app)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
@@ -73,6 +73,21 @@ func TestQueueClear(t *testing.T) {
|
||||
if !strings.Contains(out, `Clearing queue "default"`) || !strings.Contains(out, "Cleared 2 jobs") {
|
||||
t.Fatalf("output = %q", out)
|
||||
}
|
||||
// In process, on the app that already published its pool (a plugin
|
||||
// HasSchedule entry naming queue:clear): the block still holds the one
|
||||
// worker slot, so these two stay pending.
|
||||
for range 2 {
|
||||
if err := m.Enqueue(ctx, nil, pingArgs{Tag: "pending-in-process"}, EnqueueOpts{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
var inProcess bytes.Buffer
|
||||
if err := bonfire.Call(ctx, RuntimeCommands(app, nil), "queue:clear", nil, &inProcess); err != nil {
|
||||
t.Fatalf("in-process queue:clear: %v\n%s", err, inProcess.String())
|
||||
}
|
||||
if !strings.Contains(inProcess.String(), "Cleared 2 jobs") {
|
||||
t.Fatalf("in-process output = %q", inProcess.String())
|
||||
}
|
||||
close(release)
|
||||
deadline := time.Now().Add(10 * time.Second)
|
||||
for countRows(t, gdb, `SELECT count(*) FROM river_job WHERE kind = ? AND state = 'completed'`, "conga_test_block") != 1 {
|
||||
|
||||
@@ -254,6 +254,44 @@ func TestScheduleRunsCommand(t *testing.T) {
|
||||
logs.waitFor(t, slog.LevelWarn, "schedule: command not registered; skipping", "command", "acme:missing")
|
||||
}
|
||||
|
||||
// TestScheduledDeferredPurgeInProcess runs the framework entry deferred:purge
|
||||
// the way serve's worker does (callScheduled through the published catalog)
|
||||
// and through schedule:run --once, both on an app whose pool is already
|
||||
// published. Both used to fail with "backpack: duplicate provider for
|
||||
// *sql.DB".
|
||||
func TestScheduledDeferredPurgeInProcess(t *testing.T) {
|
||||
db, dsn := migratedDB(t)
|
||||
app, _ := testApp(t, db, dsn, map[string]any{"app.key": testAppKey})
|
||||
logs := &captureHandler{}
|
||||
if err := app.Publish(slog.New(logs)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := app.Publish(bonfire.NewCatalog(lagoon.RuntimeCommands(app, nil))); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var buf bytes.Buffer
|
||||
skipped, err := callScheduled(t.Context(), app, loggerFromApp(app), "deferred:purge", nil, &buf)
|
||||
if skipped || err != nil {
|
||||
t.Fatalf("callScheduled: skipped=%v err=%v\n%s", skipped, err, buf.String())
|
||||
}
|
||||
if !strings.Contains(buf.String(), "purged 0 deferred bindings older than 5 days") {
|
||||
t.Fatalf("output = %q", buf.String())
|
||||
}
|
||||
if !logs.find(slog.LevelInfo, "schedule: command finished", "command", "deferred:purge") {
|
||||
t.Fatal("no Info 'schedule: command finished' record for deferred:purge")
|
||||
}
|
||||
if err := db.PingContext(t.Context()); err != nil {
|
||||
t.Fatalf("published pool closed by the command: %v", err)
|
||||
}
|
||||
out, err := onceRun(t, app, nil, mustTime(t, time.UTC, "2026-03-10 03:00:00"))
|
||||
if err != nil {
|
||||
t.Fatalf("schedule:run --once: %v\n%s", err, out)
|
||||
}
|
||||
if !strings.Contains(out, "Ran 1 due entries (0 failed)") {
|
||||
t.Fatalf("schedule:run --once output = %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
// onceRun runs `schedule:run --once` for plugins at the given app time and
|
||||
// returns its output.
|
||||
func onceRun(t *testing.T, app *backpack.App, plugins []party.Plugin, now time.Time) (string, error) {
|
||||
|
||||
@@ -483,7 +483,16 @@ func purgeBucket(ctx context.Context, app *backpack.App) (*blob.Bucket, func(),
|
||||
return b, func() { _ = b.Close() }, nil
|
||||
}
|
||||
|
||||
// withDB runs fn on the app's published database when there is one: an
|
||||
// in-process run (the scheduler in serve or schedule:run, through
|
||||
// bonfire.Catalog.Call) borrows that pool and leaves it open. Otherwise it
|
||||
// opens and publishes a pool for the CLI run and closes it when fn returns.
|
||||
func withDB(ctx context.Context, app *backpack.App, fn func(*gorm.DB) error) error {
|
||||
if app != nil {
|
||||
if gdb, ok := app.Lookup[*gorm.DB](); ok && gdb != nil {
|
||||
return fn(gdb)
|
||||
}
|
||||
}
|
||||
sqlDB, gdb, err := OpenFromApp(ctx, app)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -501,3 +501,50 @@ func TestPurgeDeferredCommand(t *testing.T) {
|
||||
t.Fatal("the command left the file, its blob or a binding")
|
||||
}
|
||||
}
|
||||
|
||||
// TestPurgeDeferredCommandPublishedDB runs deferred:purge the way the
|
||||
// in-process scheduler does: through bonfire.Call on an app whose pool serve
|
||||
// already opened and published. The command used to publish a second pool
|
||||
// and fail with "backpack: duplicate provider for *sql.DB"; it now runs on
|
||||
// the published handle and leaves that pool open.
|
||||
func TestPurgeDeferredCommandPublishedDB(t *testing.T) {
|
||||
e := newPurgeEnv(t, "lagoon_purge_published")
|
||||
f := e.file(t, "")
|
||||
created := e.child(t, "created")
|
||||
e.bind(t, "cover", DeferredFileType, fmt.Sprint(f.ID), nil, 3*day)
|
||||
e.bind(t, "children", "acme_purge_children", fmt.Sprint(created), &DeferredEnvelope{Created: true}, 3*day)
|
||||
|
||||
app := backpack.New(purgeConfig(t, map[string]any{"database.dsn": e.dsn, "app.key": base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{7}, 32))}))
|
||||
sqlDB, gdb, err := OpenFromApp(t.Context(), app)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = sqlDB.Close() })
|
||||
if err := Publish(app, sqlDB, gdb); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := attach.Publish(app, e.bucket); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
plugins := []party.Plugin{purgePlugin{models: []any{&purgeChild{}}}}
|
||||
var out bytes.Buffer
|
||||
if err := bonfire.Call(t.Context(), RuntimeCommands(app, plugins), "deferred:purge", []string{"--days", "2"}, &out); err != nil {
|
||||
t.Fatalf("deferred:purge: %v\n%s", err, out.String())
|
||||
}
|
||||
if !strings.Contains(out.String(), "purged 2 deferred bindings older than 2 days (1 files, 1 children, 0 skipped)") {
|
||||
t.Fatalf("output = %q", out.String())
|
||||
}
|
||||
if e.fileExists(t, f.ID) || e.blobExists(t, f) || len(e.bindingIDs(t)) != 0 {
|
||||
t.Fatal("the command left the file, its blob or a binding")
|
||||
}
|
||||
if err := sqlDB.PingContext(t.Context()); err != nil {
|
||||
t.Fatalf("published pool closed by the command: %v", err)
|
||||
}
|
||||
if got, ok := app.Lookup[*gorm.DB](); !ok || got != gdb {
|
||||
t.Fatalf("published *gorm.DB replaced: %p, want %p", got, gdb)
|
||||
}
|
||||
out.Reset()
|
||||
if err := bonfire.Call(t.Context(), RuntimeCommands(app, plugins), "migrate:status", nil, &out); err != nil {
|
||||
t.Fatalf("migrate:status: %v\n%s", err, out.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,7 +44,7 @@ Use cases:
|
||||
- Widening on return (A-WR-04): when a binding comes back with a different identity (more or fewer scopes, a backend subject added or dropped), its stateful session or stateless server is re-filtered on that request, so newly allowed tools and prompts appear at once (with `list_changed`) instead of waiting for an event. Token ids play no part in this, so an access-token rotation alone re-filters nothing. A stateless server is shared by every live token of its binding, so a return with less reach only adds there and never removes: the receiving middleware hides, per request, what that request's identity may not use, and a call of a wider token is never cut off by a narrower one. A guard provider error during this re-filtering is not a refusal: the cached server stays as it was and the next request retries (logged as `pier: widening deferred`); a fan-out recompute that meets one hides the failing items and leaves the server to be re-filtered by the binding's next request.
|
||||
- Protected resource metadata (RFC 9728): `GET /.well-known/oauth-protected-resource` and `GET /.well-known/oauth-protected-resource<mcp.path>` answer the resource, the authorization server (the published verifier's resource origin, else `app.url`), the supported scopes and the bearer method. When the published `pact.MCPTokenVerifier` also implements `pact.MCPAdvertisedScopes` and `pact.MCPAdvertisedScopes.MCPScopesSupported` is non-empty, that slice is copied; otherwise the document keeps `pier.PRMScopes`. pier does not read `pact.MemberTokenScopeCatalog` and does not widen the read/write ceiling. A `mcp.resource` that differs from the verifier's resource is a boot warning. The list the OAuth server plugin publishes is the MCP advertised set: the fixed names `read`, `write`, `ai`, `offline_access` and `backend` are always on it. Declared member scopes follow the plugin's `golem15.oauthserver.mcp_scopes` filter, which is that plugin's key and not a framework `mcp.*` key: unset means every declared member scope, a set filter means only those declared names, and an empty filter means the five fixed names only. An empty `wristband.Options.ScopesSupported` on the MCP flavour resolves to those five names; a non-empty list is used as given. OpenID Connect discovery stays `openid`, `profile` and `email`. A route reads who called a tool with `pact.MCPCallFrom`.
|
||||
- Prompts (D-28): prefixed and filtered like tools, required arguments checked before `Render` runs; a prompt never dispatches a route. A `Render` error is logged server-side and answered with the fixed JSON-RPC error `pier: the prompt could not be rendered`, so no database or provider error reaches the agent. A prompt declared with `Writes` (it directs tools that are not read-only) sits under the read/write ceiling and is hidden from an identity without `write`.
|
||||
- A 401 carries `WWW-Authenticate: Bearer resource_metadata="<app.url>/.well-known/oauth-protected-resource<mcp.path>", scope="read"`; a browser request whose `Origin` is not in `mcp.allowed_origins` is refused with 403 before any body is read.
|
||||
- A 401 carries `WWW-Authenticate: Bearer resource_metadata="<app.url>/.well-known/oauth-protected-resource<mcp.path>"`; there is no scope parameter, so a client requests the `scopes_supported` of the protected resource metadata; a browser request whose `Origin` is not in `mcp.allowed_origins` is refused with 403 before any body is read.
|
||||
- One audit line per tool call (`pier: tool call`: tool, guard, user id, admin id, client id, token kind, session prefix, status, duration) and one per session step (`pier: session`: `open`, `rehydrate`, `end`, `binding_refused`, `revoked`, `stream_closed`, `expired`), never arguments, bodies or tokens. A tool call served on the stateful path without a session logs the session `ephemeral`, one on the stateless path `stateless`.
|
||||
|
||||
## Usage
|
||||
|
||||
@@ -247,8 +247,12 @@ func (p *Pier) bodyLimit(next http.Handler) http.Handler {
|
||||
|
||||
// bearer verifies the bearer of every request afresh and stores the
|
||||
// resolved Identity on the request for the session router and the SDK
|
||||
// handlers; a failure is a 401 carrying WWW-Authenticate with the
|
||||
// protected resource metadata URL and the read scope.
|
||||
// handlers. 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. No Scopes option is set: the SDK would answer 403 to
|
||||
// every token lacking them.
|
||||
func (p *Pier) bearer(next http.Handler) http.Handler {
|
||||
verify := func(ctx context.Context, token string, r *http.Request) (*auth.TokenInfo, error) {
|
||||
ctx = towel.WithClientIP(ctx, p.clientIP(r))
|
||||
@@ -263,13 +267,10 @@ func (p *Pier) bearer(next http.Handler) http.Handler {
|
||||
Extra: map[string]any{extraIdentity: id, extraCall: callInfoFrom(r)},
|
||||
}, nil
|
||||
}
|
||||
h := auth.RequireBearerToken(verify, &auth.RequireBearerTokenOptions{
|
||||
return auth.RequireBearerToken(verify, &auth.RequireBearerTokenOptions{
|
||||
ResourceMetadataURL: p.prmURL,
|
||||
AllowMissingExpiration: true,
|
||||
})(next)
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
h.ServeHTTP(&challengeWriter{ResponseWriter: w}, r)
|
||||
})
|
||||
}
|
||||
|
||||
// clientIP is the request's client address: the host's answer when it
|
||||
@@ -285,30 +286,6 @@ func (p *Pier) clientIP(r *http.Request) string {
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
// challengeWriter completes the 401 challenge with scope="read", which the
|
||||
// SDK middleware would otherwise enforce on every request.
|
||||
type challengeWriter struct {
|
||||
http.ResponseWriter
|
||||
}
|
||||
|
||||
func (c *challengeWriter) WriteHeader(status int) {
|
||||
if status == http.StatusUnauthorized {
|
||||
h := c.Header()
|
||||
if v := h.Get("WWW-Authenticate"); v != "" && !strings.Contains(v, "scope=") {
|
||||
h.Set("WWW-Authenticate", v+`, scope="read"`)
|
||||
}
|
||||
}
|
||||
c.ResponseWriter.WriteHeader(status)
|
||||
}
|
||||
|
||||
func (c *challengeWriter) Flush() {
|
||||
if f, ok := c.ResponseWriter.(http.Flusher); ok {
|
||||
f.Flush()
|
||||
}
|
||||
}
|
||||
|
||||
func (c *challengeWriter) Unwrap() http.ResponseWriter { return c.ResponseWriter }
|
||||
|
||||
// identityFromRequest reads the Identity the bearer middleware stored.
|
||||
func identityFromRequest(r *http.Request) (Identity, bool) {
|
||||
ti := auth.TokenInfoFromContext(r.Context())
|
||||
|
||||
@@ -96,21 +96,31 @@ func TestPierTracerSmoke(t *testing.T) {
|
||||
t.Fatalf("text content = %+v", res.Content[0])
|
||||
}
|
||||
|
||||
// A bad bearer is a 401 carrying the protected resource metadata challenge.
|
||||
// A bad or missing bearer is a 401 whose challenge names only the
|
||||
// protected resource metadata URL: no scope parameter, so a client
|
||||
// requests the scopes the metadata document advertises.
|
||||
init := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"x","version":"1"}}}`
|
||||
req, _ := http.NewRequestWithContext(ctx, http.MethodPost, srv.URL+"/mcp", bytes.NewBufferString(init))
|
||||
req.Header.Set("Authorization", "Bearer oas_at_bad")
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
_ = resp.Body.Close()
|
||||
challenge := resp.Header.Get("WWW-Authenticate")
|
||||
if resp.StatusCode != http.StatusUnauthorized || !strings.Contains(challenge, `resource_metadata="http://acme.test/.well-known/oauth-protected-resource/mcp"`) || !strings.Contains(challenge, `scope="read"`) {
|
||||
t.Fatalf("bad bearer = %d %q", resp.StatusCode, challenge)
|
||||
const wantChallenge = `Bearer resource_metadata="http://acme.test/.well-known/oauth-protected-resource/mcp"`
|
||||
for _, tc := range []struct{ name, authorization string }{
|
||||
{"bad bearer", "Bearer oas_at_bad"},
|
||||
{"missing bearer", ""},
|
||||
} {
|
||||
req, _ := http.NewRequestWithContext(ctx, http.MethodPost, srv.URL+"/mcp", bytes.NewBufferString(init))
|
||||
if tc.authorization != "" {
|
||||
req.Header.Set("Authorization", tc.authorization)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
_ = resp.Body.Close()
|
||||
challenge := resp.Header.Get("WWW-Authenticate")
|
||||
if resp.StatusCode != http.StatusUnauthorized || challenge != wantChallenge || strings.Contains(challenge, "scope=") {
|
||||
t.Fatalf("%s = %d %q, want 401 %q", tc.name, resp.StatusCode, challenge, wantChallenge)
|
||||
}
|
||||
}
|
||||
|
||||
// The same tool declared twice fails Mount naming the exposed name.
|
||||
|
||||
Reference in New Issue
Block a user