fix: close three gaps in the plugin gateway boundary - #7
Conversation
Restrict /api/plugins/invoke to diagnostic actions. Invoke is a raw runtime channel gated only by plugin:admin: it takes an arbitrary action, consults no per-method scopes, and binds no operator target. That was tolerable when it was the only way to reach a staged artifact, but /api/plugins/call is now the real gateway and is the only path that enforces the manifest's declared scopes, binds an operator target to a single invocation, and (for host-risk work) requires a plan and an approval. Left open, invoke would let a plugin:admin operator call a plugin's declared methods while bypassing their scopes — and once any plugin ships an execute action it would bypass the plan/approval/one-time-capability binding entirely and apply host changes unreviewed. The action list is now closed to describe and health; everything else is refused before the artifact runs. Derive the plugin-asset CSP only from the configured public URL. The policy fell back to an origin built from the request's Host header when the public URL was unset, so the policy confining plugin code could be named by the request it was confining. Asset serving now fails closed without a public URL, and the request-derived fallback is gone. Redact the operator target from guard failures. An operator target may carry its secret in the URL path, and url.Parse errors echo the URL they failed on — text that reaches both the audit record and the API response, while the audit record must never contain it. The reason is now surfaced without the value. A substring scrub is not enough on its own: *url.Error renders the URL quoted and escaped, so the inner reason is unwrapped and both raw and escaped forms are scrubbed. Tests: go vet clean; internal/plugin and internal/server green under -race.
|
[ack] — zeus review, Olympus TASK-0001 item 1 (2026-07-26) All three fixes verified in diff and at the integration tip: closed diagnostic action list (403+audit before artifact run), CSP fail-closed without LATTICE_PUBLIC_URL (503; consistent with the WebAuthn RPID rule), operator-target redaction via url.Error unwrap + raw/escaped scrub. Composition with #12 verified: executePluginOperation's only-path property depends on this PR's closed list. Evidence: this commit is already contained in Deploy note: plugin assets now 503 without LATTICE_PUBLIC_URL — already true in prod since the passkey deploy; matters for fresh environments. Disposition: landed via the integration cut → close-with-landing-commit per Olympus rules/01 §8.5 (do not merge to main directly). Verdict letter: lattice-olympus |
|
Closed as landed — zeus, operator-approved sweep (2026-07-26, rules/01 §8.5): commit |
1 participant
Three decision-independent findings from the 2026-07-14 plugin-boundary architecture review (
lattice_plan/ARCH-REVIEW-2026-07-14-plugin-boundary.md). None depend on how the core-vs-runtime ownership question is settled.1.
/api/plugins/invokeis a raw runtime channel (Medium)handlePluginInvoketakes an arbitrary{id, action, payload}and callspluginRuntime.Invokedirectly, gated only byplugin:admin. It consults no per-method scopes and binds no operator target — unlike/api/plugins/call, which is now the real gateway and the only path that enforces the manifest's declared scopes, binds an operator target to a single invocation, and (for host-risk work) requires a plan and an approval.Today the blast radius is bounded: a
plugin:adminoperator can invoke Sub-Store'scallmethods bypassing their declared scopes, andInvoke(notInvokeConstrained) binds no operator target so those fail closed. The real hazard is forward-looking — once any plugin ships anexecuteaction for the §9.3 host-risk apply protocol, this endpoint would sidestep the plan/approval/one-time-capability binding entirely and letplugin:adminapply host changes unreviewed.The action list is now closed to
describeandhealth. Everything else —call,plan,execute,migrate, anything — is refused with 403 and audited before the artifact runs. Nothing in the dashboard calls this endpoint (grepconfirms zero callers); it is an operator diagnostic, so restricting it costs nothing.2. Plugin-asset CSP was partly derived from the
Hostheader (Low)pluginAssetCSPfell back to an origin built fromr.HostwhenpublicURLwas unset, injecting it intoscript-src/style-src. That makes the policy which confines plugin code nameable by the request it is confining. No cross-user impact (a request's Host only shapes its own response, and the frame is opaque-origin), but it is fragile and drifts from the spec's fixed'self'.Asset serving now fails closed (503) when
publicURLis unset, and the request-derived fallback is deleted. Also adds the explicitframe-src 'none'the spec's §7.2 block lists (previously only implied bydefault-src 'none').Note the existing test asserted the fallback worked — that assertion encoded the defect, so it is replaced with one asserting no request-derived host can reach the CSP.
3. Operator-target URL could leak into the audit record (Low)
Spec §9.1: "the audit record never includes the secret URL path." But
extractOperatorTargetswrappedoutbound.GuardOperatorURL's error with%w, andurl.Parse's error echoes the URL it failed on — text that lands in both the audit reason and the API response.The guard's reason is now surfaced without the value. A plain substring scrub is not sufficient:
*url.Errorrenders asparse "<url>": <reason>with the URL quoted and Go-escaped, so the raw target never matches. The inner reason is unwrapped (it never carries the URL) and raw + escaped forms are scrubbed as belt-and-braces. The added test fails against the previous code.Test plan
go build ./...cleango vet ./internal/...cleango test ./internal/plugin/ ./internal/server/ -race— both greenTestPluginInvokeRefusesNonDiagnosticActions(403 + artifact never runs for call/plan/execute/migrate; describe/health still 200)TestPluginAssetCSPIgnoresRequestOriginTestOperatorTargetErrorRedactsSecret(fails against previous code)Follow-up (not in this PR): the ownership decision — whether the three façade plugins' in-core-backed services become an explicit signed
backingdeclaration. Tracked in the review doc.https://claude.ai/code/session_01FzYExyy6G7Cb7QpXoUEY78