Give serve-upstream allowlists a validation surface, the way lint gave guardfiles one #46
Labels
No labels
autonomy
async-consult
autonomy
epic
autonomy
headless
autonomy
live-collab
coherence-core
priority
P0
priority
P1
priority
P2
priority
P3
priority
P4
qa-fixture
role/ai
role/creator
role/design
role/director
role/engineer
role/exec
role/human
role/ops
role/qa
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
coilyco-flight-deck/mcp-beaver#46
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
ward-mcp#45 gave
.mcp.kdlguardfiles an offline validation surface, andcoilyco-bridge/deploynow invokes it instead of parsing guardfiles itself.serve-upstreamallowlists have no equivalent, so the same gap remains for upstream-mode services.Concretely:
deploy/services/signoz-mcp/values.yamldeclares a read-only allowlist of 26 upstream SigNoz tools. Deploy carried atest-signoz-mcpverb that read that values file withyqand failed if any entry matched^signoz_(create|update|delete|import)_. That is a consumer repo asserting against its own configuration, which the workspace rule forbids, so it was removed alongside the discord-mcp harness. The boundary itself is still sound -serve-upstreamexposes exactly the named tools and deny-by-absence does the real work - but nothing mechanically catches a mutation tool being added to that list in review.Why this is not just ward-mcp#45 again
linttakes a spec path and builds a server offline. An upstream allowlist has no spec file, and validating it properly means knowing the upstream's tool surface, which needs a live MCP connection. So this is not a trivial second case of the same command.Possible shapes, roughly in increasing cost
ward-mcp lint-upstream --tool <name>...that validates only allowlist well-formedness (no duplicates, no empty names) and reports the list back. Cheap, and it moves the assertion into the owning loader, but it cannot classify a tool as read-only without upstream knowledge.--read-only, and have ward-mcp reject an allowlist entry whose name matches a mutation-verb pattern. This encodes a naming heuristic in the loader rather than in each consumer, which is the right home for it, but it is still a heuristic over names.tools/listannotations and fails when an allowlisted tool is notreadOnlyHint. Most correct, needs a live upstream, so it belongs in a rollout or smoke path, not in offline CI.I have no strong preference between these and have not thought hard about which is right. Filing so the gap is tracked rather than silently absorbed.
Context
lintsurface this parallels.coilyco-bridge/deploy-services/signoz-mcp/README.mdnow points here for the missing enforcement, andscripts/lint-mcp-specs.shis the consumer-side pattern to match.Shipped in
30304d2onmainasward-mcp lint-upstream.All three shapes landed, because they turned out to be complementary rather than alternatives, and the incremental cost of the second and third was small once the first existed.
Shape 1, offline well-formedness. The checks already existed inside
newProxyBackendbut were unreachable without a live upstream. Extracted tomcpserver.ValidateAllowlist, which the serving path now calls too, so the lint check cannot drift from whatserve-upstreamaccepts. That extraction also fixed a real bug:uniqueStringssilently dropped an empty entry, so--tool ""reported itself as a duplicate. It now reports the entry index.Shape 2,
--read-only heuristic. Offline mutation-verb screen over tool names, in the owning loader. It segments names on_,-,.,/and camelCase humps rather than substring-matching, which is what keepsget_field_valuesclear of thesetverb andlist_compute_nodesclear ofput.Shape 3,
--read-only strict. Requires--upstream, connects, and fails any allowlisted tool the upstream does not annotatereadOnlyHint: true. Cheaper than expected:NewProxyalready connects and snapshots, so this is annotation inspection on the result. An unannotated tool counts as mutable, since the MCP default for the hint is false and silence is not a promise. Passing--upstreamalso inherits the proxy's existence check, so a typo'd tool name fails whether or not--read-onlyis set.The error from the heuristic names the offending tools and points at
--read-only strictas the escape when the heuristic is wrong. That is the false-positive answer for now; there is no per-tool exemption flag.Verified against the real motivating case. Ran the 26-tool SigNoz allowlist out of
deploy/services/signoz-mcp/values.yamlthrough--read-only heuristic: exits 0, prints all 26 sorted. Then confirmed it catches every verb the retiredtest-signoz-mcpregex covered -signoz_create_dashboard,signoz_update_alert,signoz_delete_view,signoz_import_dashboardall rejected. It is a strict superset of what deploy removed.15 new tests.
ward exec build,vet,testgreen, plushelm-lint-chartandhelm-template-upstream.Not done here, needs a deploy-side issue. deploy still has no invocation. Adopting this means a verb that reads its own
values.yamland passes the tools toward-mcp lint-upstream, which is the allowed shape since the assertion lives in the owning loader. Worth checking whether the SigNoz upstream actually setsreadOnlyHintbefore wiringstrictinto a rollout path - if it does not annotate,strictwill reject all 26 and only the heuristic is usable there.services/signoz-mcp/README.mdpoints at this issue and should be repointed once that lands.