The inline grammar discards MethodForVerb's known-verb signal, so an unrecognised verb silently mints a POST #297

Closed
opened 2026-08-16 05:00:53 +00:00 by coilyco-ops · 1 comment
Member

Filed by Angie (ENG, claude seat), splitting the live half out of #281. That issue asked whether reopen was missing from the verb table; it is not, and it closed. This is the footgun it identified along the way, which is real.

The signal exists and nothing reads it

MethodForVerb already distinguishes a known verb from a guess, and says so in its own doc comment:

// MethodForVerb returns the HTTP method a verb maps to by convention; ok is
// false only for the bare-unknown-verb POST default.
func MethodForVerb(verb string) (string, bool)

It has exactly one production caller, and that caller throws the second return away:

// http/opcore/inline.go:130
method, _ := MethodForVerb(verb)

So can transfer repo, can restore repo, or can merge pr parses cleanly, mints a tool, and emits POST against an API that almost certainly wants something else. Nothing at parse time reports that a method was inferred rather than known.

Why it is worth closing

This is the one place the grammar guesses. Everywhere else it refuses to infer authority: an unwritten grant is an unminted tool, a can body rejects an unknown node fail-closed, and a fetch overlay must state its output. A verb it has never seen getting a mutating method by default is the odd one out, and Darren put it well on #281:

Guessing is the odd one out in a design that otherwise refuses to infer authority.

The failure is silent in the dangerous direction. A wrong GET fails loudly at the API. A wrong POST against a real endpoint may not.

The constraint that makes this a decision rather than a repair

pin relies on the fallthrough and is documented as doing so. coilyco-bridge/deploy/services/forgejo-mcp/forgejo.mcp.kdl names it explicitly: "unknown verbs such as pin -> POST". Failing closed today breaks that grant, and any other guardfile leaning on the same default.

So this needs an audit before a switch, which is why it is filed rather than fixed.

Options

  1. Fail closed. An unrecognised verb becomes a parse error naming it. Every guardfile relying on the fallthrough must first declare its verb, so this needs the audit.
  2. Add the known-good verbs to the table so the fallthrough gets rarer, and pin -> POST becomes explicit rather than accidental. Smaller, and it does not close the hole for a genuinely novel verb.
  3. Warn rather than refuse. Consume the ok and report an inferred method at parse time. Keeps every existing guardfile working and makes the next instance visible instead of silent.

Option 3 is the cheapest thing that removes the silence, and it composes with 1 later.

Acceptance

  • An unrecognised verb is either refused or reported at parse time, rather than resolving to POST with no signal.
  • pin keeps working, or is declared explicitly in every guardfile that uses it, in the same change.
  • Which guardfiles rely on the fallthrough is written down, since that audit is the actual blocker.
  • #281 - where this surfaced. Its reopen premise was false; this half was not.
  • coilyco-bridge/deploy carries a header comment describing the verb table that omits reopen, archive, unarchive, delete, and remove. Worth correcting there so the next reader is not misled the same way.
**Filed by Angie (ENG, `claude` seat)**, splitting the live half out of #281. That issue asked whether `reopen` was missing from the verb table; it is not, and it closed. This is the footgun it identified along the way, which is real. ## The signal exists and nothing reads it `MethodForVerb` already distinguishes a known verb from a guess, and says so in its own doc comment: ```go // MethodForVerb returns the HTTP method a verb maps to by convention; ok is // false only for the bare-unknown-verb POST default. func MethodForVerb(verb string) (string, bool) ``` It has exactly one production caller, and that caller throws the second return away: ```go // http/opcore/inline.go:130 method, _ := MethodForVerb(verb) ``` So `can transfer repo`, `can restore repo`, or `can merge pr` parses cleanly, mints a tool, and emits **POST** against an API that almost certainly wants something else. Nothing at parse time reports that a method was inferred rather than known. ## Why it is worth closing This is the one place the grammar guesses. Everywhere else it refuses to infer authority: an unwritten grant is an unminted tool, a `can` body rejects an unknown node fail-closed, and a fetch overlay must state its `output`. A verb it has never seen getting a mutating method by default is the odd one out, and Darren put it well on #281: > Guessing is the odd one out in a design that otherwise refuses to infer authority. The failure is silent in the dangerous direction. A wrong GET fails loudly at the API. A wrong POST against a real endpoint may not. ## The constraint that makes this a decision rather than a repair **`pin` relies on the fallthrough and is documented as doing so.** `coilyco-bridge/deploy/services/forgejo-mcp/forgejo.mcp.kdl` names it explicitly: *"unknown verbs such as pin -> POST"*. Failing closed today breaks that grant, and any other guardfile leaning on the same default. So this needs an audit before a switch, which is why it is filed rather than fixed. ## Options 1. **Fail closed.** An unrecognised verb becomes a parse error naming it. Every guardfile relying on the fallthrough must first declare its verb, so this needs the audit. 2. **Add the known-good verbs to the table** so the fallthrough gets rarer, and `pin -> POST` becomes explicit rather than accidental. Smaller, and it does not close the hole for a genuinely novel verb. 3. **Warn rather than refuse.** Consume the `ok` and report an inferred method at parse time. Keeps every existing guardfile working and makes the next instance visible instead of silent. Option 3 is the cheapest thing that removes the silence, and it composes with 1 later. ## Acceptance * An unrecognised verb is either refused or reported at parse time, rather than resolving to POST with no signal. * `pin` keeps working, or is declared explicitly in every guardfile that uses it, in the same change. * Which guardfiles rely on the fallthrough is written down, since that audit is the actual blocker. ## Related * #281 - where this surfaced. Its `reopen` premise was false; this half was not. * `coilyco-bridge/deploy` carries a header comment describing the verb table that omits `reopen`, `archive`, `unarchive`, `delete`, and `remove`. Worth correcting there so the next reader is not misled the same way.
Author
Member

Landed on main as 3893db5. make test and pre-commit run --all-files green.

The audit first, since it was the stated blocker

Scanned every *.mcp.kdl under deploy/services for can <verb> <resource> grants whose verb misses the convention table (accounting for the list- and create-on- prefixes and search). Two verbs, three grants:

  • comment - services/sirens-echo/forgejo-mcp.mcp.kdl:73, services/forgejo-mcp/forgejo.mcp.kdl:262
  • pin - services/forgejo-mcp/forgejo.mcp.kdl:290

That is the whole dependent set. It is small enough that options 2 and 3 stop being alternatives and compose, which is what shipped.

What shipped

  • Descriptor.MethodInferred records the guess instead of discarding it. This is the actual defect in the title fixed: MethodForVerb always returned ok, and inline.go:130 threw it away.
  • ParseInlineWithWarnings returns one note per inferred grant, naming the verb and the method it was handed. ParseInline keeps its signature, so every out-of-tree caller compiles unchanged.
  • New method "PUT" node in a can body states the method outright and suppresses the inference. A stated DELETE also marks the leaf Destructive whatever the verb is called, because the confirmation gate should key off the effect rather than the spelling. Fails closed on a non-method and on a duplicate.
  • comment and pin are now in the convention table, so zero guardfiles rely on the fallthrough. pin -> POST is a decision rather than an accident, which is what the deploy comment was documenting.

What I did not do, and why

Did not fail closed (option 1), even though the audit shows the in-fleet dependent set is now empty. The audit covers deploy, which is where MCP guardfiles live, but out-of-tree guardfiles are not all visible from this checkout. Refusing would break any that lean on the fallthrough, with nothing gained over reporting. Option 3 removes the silence today and composes with the switch later, exactly as the issue framed it.

Acceptance

  • An unrecognised verb is reported at parse time rather than resolving to POST with no signal. TestParseInlineUnknownVerbIsMarkedAndWarned.
  • pin keeps working and is declared explicitly, in the same change.
  • The fallthrough audit is written down, above.

Follow-up outside this repo

coilyco-bridge/deploy carries a header comment describing the verb table. With comment and pin now stated upstream, the note in services/forgejo-mcp/forgejo.mcp.kdl reading "unknown verbs such as pin -> POST" is stale and describes the old behaviour. Not touching that repo from here. Worth a one-line correction next time someone is in it, alongside the omissions the issue already flagged (reopen, archive, unarchive, delete, remove).

Angie, engineer seat

Landed on `main` as `3893db5`. `make test` and `pre-commit run --all-files` green. ## The audit first, since it was the stated blocker Scanned every `*.mcp.kdl` under `deploy/services` for `can <verb> <resource>` grants whose verb misses the convention table (accounting for the `list-` and `create-on-` prefixes and `search`). **Two verbs, three grants:** * `comment` - `services/sirens-echo/forgejo-mcp.mcp.kdl:73`, `services/forgejo-mcp/forgejo.mcp.kdl:262` * `pin` - `services/forgejo-mcp/forgejo.mcp.kdl:290` That is the whole dependent set. It is small enough that options 2 and 3 stop being alternatives and compose, which is what shipped. ## What shipped * `Descriptor.MethodInferred` records the guess instead of discarding it. This is the actual defect in the title fixed: `MethodForVerb` always returned `ok`, and `inline.go:130` threw it away. * `ParseInlineWithWarnings` returns one note per inferred grant, naming the verb and the method it was handed. **`ParseInline` keeps its signature**, so every out-of-tree caller compiles unchanged. * New `method "PUT"` node in a `can` body states the method outright and suppresses the inference. A stated `DELETE` also marks the leaf `Destructive` whatever the verb is called, because the confirmation gate should key off the effect rather than the spelling. Fails closed on a non-method and on a duplicate. * `comment` and `pin` are now in the convention table, so **zero guardfiles rely on the fallthrough**. `pin -> POST` is a decision rather than an accident, which is what the deploy comment was documenting. ## What I did not do, and why **Did not fail closed** (option 1), even though the audit shows the in-fleet dependent set is now empty. The audit covers `deploy`, which is where MCP guardfiles live, but out-of-tree guardfiles are not all visible from this checkout. Refusing would break any that lean on the fallthrough, with nothing gained over reporting. Option 3 removes the silence today and composes with the switch later, exactly as the issue framed it. ## Acceptance * An unrecognised verb is reported at parse time rather than resolving to POST with no signal. `TestParseInlineUnknownVerbIsMarkedAndWarned`. * `pin` keeps working **and** is declared explicitly, in the same change. * The fallthrough audit is written down, above. ## Follow-up outside this repo `coilyco-bridge/deploy` carries a header comment describing the verb table. With `comment` and `pin` now stated upstream, the note in `services/forgejo-mcp/forgejo.mcp.kdl` reading *"unknown verbs such as pin -> POST"* is stale and describes the old behaviour. Not touching that repo from here. Worth a one-line correction next time someone is in it, alongside the omissions the issue already flagged (`reopen`, `archive`, `unarchive`, `delete`, `remove`). <!-- ward-agent-signature --> Angie, engineer seat
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/umbra#297
No description provided.