Proposal: governance endpoints should reject unknown JSON fields (money-safety)

[yjy19999]

Type: discussion / proposal — not a decided change
Area: transports/bifrost-http/handlers/governance.go

Summary

Every governance write endpoint decodes its request body with a plain json.Unmarshal,
so Go's encoding/json silently discards any field the request struct does not declare.
For most APIs that is a harmless tolerance. For the governance API it is a money-safety
problem: the fields being discarded are the ones that cap spend.

I'd like to discuss making these endpoints reject unknown fields instead. I am not
proposing this as a done deal — it is a breaking change, and the trade-off deserves
argument before anyone writes the patch.

The concrete scenario

A user follows the documentation to create a virtual key with a $5 spend cap:

curl -X POST http://localhost:8080/api/governance/virtual-keys \
  -H "Content-Type: application/json" \
  -d '{
    "name": "contractor-key",
    "budget": { "max_limit": 5.00, "reset_duration": "1d" }
  }'

The API returns HTTP 200 and a fully-formed virtual key. The user reasonably
concludes the cap is in place.

It is not. CreateVirtualKeyRequest binds only the plural array:

Budgets []CreateBudgetRequest `json:"budgets,omitempty"`

— governance.go:243 (and :288 for update; :232 / :276 for the same field inside
provider_configs). There is no singular Budget field on the virtual-key request at all.
The "budget" object is parsed, matched against nothing, and dropped.

The key is then created with zero budgets, which is not a $0 cap — it is no cap.
Nothing downstream catches it either: budget validation is gated behind

if len(req.Budgets) > 0 {

— governance.go:1725 — so an empty budget list skips validation entirely rather than
failing. The request succeeds, the key works, and spend runs unbounded until someone
notices the bill.

This is not hypothetical. Our own documentation shipped exactly this payload, and a user
followed it, got a 200, and had no spend cap. (The docs are being corrected separately;
this issue is about why the API let it happen quietly.)

Why the failure is invisible from the client side

There is no signal available to the caller at any point:

• the status code is 200, not 4xx;
• the response body contains no warning field;
• the created key looks normal in the response and in the UI;
• the only way to detect it is to re-read the key and notice budgets is empty — i.e.
  to already suspect the bug you are trying to find.

A typo in a field name ("budgets" → "budgest"), a copy-paste from an older API
version, or an SDK built against a different release all fail the same silent way.

Proposed change

Decode governance request bodies strictly, so an unknown field is a 400 with a message
naming the offending field:

decoder := json.NewDecoder(bytes.NewReader(ctx.PostBody()))
decoder.DisallowUnknownFields()
if err := decoder.Decode(&req); err != nil { /* 400 */ }

There is already precedent for this in the same file. updateComplexityAnalyzerConfig
does exactly this at governance.go:1501-1503 — and it is the only use of
DisallowUnknownFields anywhere in the Go tree. So the strict pattern is established and
accepted in this codebase; it is simply not applied to the endpoints where the cost of
being wrong is measured in dollars.

An equivalent outcome via a different mechanism would be just as good — for example
validating the raw body against the OpenAPI schema, or a decode-time "unknown field"
warning collected and returned in the response. The specific mechanism matters less than
turning a silent discard into something the caller can see.

Suggested scope

If we do this, the endpoints where a dropped field costs money are the ones that matter:

Endpoint	Request struct	Reference
POST /api/governance/virtual-keys	CreateVirtualKeyRequest	governance.go:224
PUT /api/governance/virtual-keys/{vk_id}	UpdateVirtualKeyRequest	governance.go:267
POST/PUT /api/governance/teams	Create/UpdateTeamRequest	governance.go:1301, :1310
POST/PUT /api/governance/customers	Create/UpdateCustomerRequest	governance.go:1322, :1331
POST/PUT /api/governance/model-configs	Create/UpdateModelConfigRequest	governance.go:1342, :1354
PUT /api/governance/providers/{provider_name}	UpdateProviderGovernanceRequest	governance.go:1364
PUT .../budgets/{budget_id}/override	BudgetOverrideRequest	governance.go:347

This is a breaking change — the honest cost

Calls that are wrong today but accepted would start failing. That is the entire point of
the change, and it is also its main risk:

• Existing automation breaks on upgrade. Any script, Terraform provider, or internal
  tool currently sending a stray field gets a 400 where it used to get a 200. Some of
  those callers are already silently broken and would be fixed by the noise — but some
  send harmless extras (a "comment" field, a re-POSTed response body containing id /
  created_at / current_usage) and would break for no safety benefit.
• Round-tripping a GET into a PUT stops working. This is a common and reasonable
  client pattern, and response structs carry many fields the request structs do not.
  Strict decoding forbids it outright. This may be the strongest argument against a blanket
  DisallowUnknownFields.
• config.json compatibility fields blur the line. Some names are legitimately
  accepted in a config file but not over HTTP — calendar_aligned on an individual budget
  is honored by applyV1Compat (transports/bifrost-http/lib/config.go:797) but is not a
  field on CreateBudgetRequest (governance.go:330-337). Users will not expect the two
  surfaces to diverge, and a strict API would make that divergence loud.
• Deprecated aliases must keep working. budget (singular) is still bound on
  customers (governance.go:1325, :1334) and provider governance (:1365). Strict
  decoding must not break those, and there is already a mutual-exclusion check that
  returns 400 when both budget and budgets are sent (governance.go:3221, :3341).

Options worth weighing

1. Strict decode everywhere on governance writes. Loudest and simplest; largest blast
   radius.
2. Strict decode behind a config flag, default off for one minor release, then flip.
   Lets operators find broken callers on their own schedule.
3. Warn, don't fail — accept the request but return the ignored field names in the
   response body and log them. No breakage, much weaker guarantee, and easy for a client
   to ignore.
4. Targeted allowlist — reject unknown fields only when they collide with a known
   money field under a different shape (e.g. budget where only budgets is bound).
   Narrow and low-breakage, but bespoke and needs maintenance as fields evolve.

My own lean is (2) or (4): both make the $5-cap scenario loud without breaking every
caller that ever sent an extra key. But I'd rather hear which of these the maintainers
consider acceptable before anyone builds it.

Question for maintainers

Is turning silently-ignored governance fields into a hard error something this project
wants at all, and if so, which of the four options above fits the compatibility promises
Bifrost makes across minor versions?

---

Filed alongside a documentation-only fix that corrects the drifted examples. That fix
stops the docs from teaching the broken payload, but it does not stop a hand-written or
SDK-generated request from failing the same silent way.

[coderabbitai]

🔗 Related PRs

#4931 - cost for prompt guardrails [merged]
#6004 - governance: extend reset_budget_usage to teams, customers, model limits and provider governance [merged]
#6088 - harness fixes [merged]
#6089 - openai harness fixes and mcp test fixes [closed]
#6110 - core updates [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!