Give serve-upstream allowlists a validation surface, the way lint gave guardfiles one #46

Closed
opened 2026-08-08 15:35:29 +00:00 by coilyco-ops · 1 comment
Member

Problem

ward-mcp#45 gave .mcp.kdl guardfiles an offline validation surface, and coilyco-bridge/deploy now invokes it instead of parsing guardfiles itself. serve-upstream allowlists have no equivalent, so the same gap remains for upstream-mode services.

Concretely: deploy/services/signoz-mcp/values.yaml declares a read-only allowlist of 26 upstream SigNoz tools. Deploy carried a test-signoz-mcp verb that read that values file with yq and 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-upstream exposes 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

lint takes 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

  • A pure offline check: 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.
  • A declared-intent flag: let the caller assert a shape, e.g. --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.
  • A connected check that reads the upstream's advertised tools/list annotations and fails when an allowlisted tool is not readOnlyHint. 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

  • ward-mcp#45 - the guardfile lint surface this parallels.
  • coilyco-bridge/deploy - services/signoz-mcp/README.md now points here for the missing enforcement, and scripts/lint-mcp-specs.sh is the consumer-side pattern to match.
## Problem ward-mcp#45 gave `.mcp.kdl` guardfiles an offline validation surface, and `coilyco-bridge/deploy` now invokes it instead of parsing guardfiles itself. `serve-upstream` allowlists have no equivalent, so the same gap remains for upstream-mode services. Concretely: `deploy/services/signoz-mcp/values.yaml` declares a read-only allowlist of 26 upstream SigNoz tools. Deploy carried a `test-signoz-mcp` verb that read that values file with `yq` and 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-upstream` exposes 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 `lint` takes 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 * A pure offline check: `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. * A declared-intent flag: let the caller assert a shape, e.g. `--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. * A connected check that reads the upstream's advertised `tools/list` annotations and fails when an allowlisted tool is not `readOnlyHint`. 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 * ward-mcp#45 - the guardfile `lint` surface this parallels. * `coilyco-bridge/deploy` - `services/signoz-mcp/README.md` now points here for the missing enforcement, and `scripts/lint-mcp-specs.sh` is the consumer-side pattern to match.
Author
Member

Shipped in 30304d2 on main as ward-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 newProxyBackend but were unreachable without a live upstream. Extracted to mcpserver.ValidateAllowlist, which the serving path now calls too, so the lint check cannot drift from what serve-upstream accepts. That extraction also fixed a real bug: uniqueStrings silently 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 keeps get_field_values clear of the set verb and list_compute_nodes clear of put.

Shape 3, --read-only strict. Requires --upstream, connects, and fails any allowlisted tool the upstream does not annotate readOnlyHint: true. Cheaper than expected: NewProxy already 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 --upstream also inherits the proxy's existence check, so a typo'd tool name fails whether or not --read-only is set.

The error from the heuristic names the offending tools and points at --read-only strict as 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.yaml through --read-only heuristic: exits 0, prints all 26 sorted. Then confirmed it catches every verb the retired test-signoz-mcp regex covered - signoz_create_dashboard, signoz_update_alert, signoz_delete_view, signoz_import_dashboard all rejected. It is a strict superset of what deploy removed.

15 new tests. ward exec build, vet, test green, plus helm-lint-chart and helm-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.yaml and passes the tools to ward-mcp lint-upstream, which is the allowed shape since the assertion lives in the owning loader. Worth checking whether the SigNoz upstream actually sets readOnlyHint before wiring strict into a rollout path - if it does not annotate, strict will reject all 26 and only the heuristic is usable there. services/signoz-mcp/README.md points at this issue and should be repointed once that lands.

Shipped in `30304d2` on `main` as `ward-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 `newProxyBackend` but were unreachable without a live upstream. Extracted to `mcpserver.ValidateAllowlist`, which the serving path now calls too, so the lint check cannot drift from what `serve-upstream` accepts. That extraction also fixed a real bug: `uniqueStrings` silently 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 keeps `get_field_values` clear of the `set` verb and `list_compute_nodes` clear of `put`. **Shape 3, `--read-only strict`.** Requires `--upstream`, connects, and fails any allowlisted tool the upstream does not annotate `readOnlyHint: true`. Cheaper than expected: `NewProxy` already 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 `--upstream` also inherits the proxy's existence check, so a typo'd tool name fails whether or not `--read-only` is set. The error from the heuristic names the offending tools and points at `--read-only strict` as 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.yaml` through `--read-only heuristic`: exits 0, prints all 26 sorted. Then confirmed it catches every verb the retired `test-signoz-mcp` regex covered - `signoz_create_dashboard`, `signoz_update_alert`, `signoz_delete_view`, `signoz_import_dashboard` all rejected. It is a strict superset of what deploy removed. 15 new tests. `ward exec build`, `vet`, `test` green, plus `helm-lint-chart` and `helm-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.yaml` and passes the tools to `ward-mcp lint-upstream`, which is the allowed shape since the assertion lives in the owning loader. Worth checking whether the SigNoz upstream actually sets `readOnlyHint` before wiring `strict` into a rollout path - if it does not annotate, `strict` will reject all 26 and only the heuristic is usable there. `services/signoz-mcp/README.md` points at this issue and should be repointed once that lands.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/mcp-beaver#46
No description provided.