Skip to content

fix(store): check model-level headers at the models.yml door (RIG-5287) - #2227

Merged
mattwilkinsonn merged 1 commit into
compass-comms/rig-3134-models-header-shapesfrom
compass-comms/rig-5287-model-level-headers
Oct 12, 2026
Merged

mattwilkinsonn merged 1 commit into
compass-comms/rig-3134-models-header-shapesfrom
compass-comms/rig-5287-model-level-headers

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

This PR is part of a stack containing 2 PRs:

  1. main
  2. fix(store): refuse non-string models.yml header values at the door (RIG-3134) #2224
  3. "fix(store): check model-level headers at the models.yml door (RIG-5287)" (this PR)

What

The models.yml credential door (rejectCredentialModels, go/internal/store/agent_config.go) checked header values only at providers.<name>.headers. The installed SDK (@oh-my-pi/pi-coding-agent@18.0.11) reads headers from two more sites and sends them the same way, through createLiveConfigHeaders → resolveConfigValue:

  • providers.<name>.models[].headers (mergeCustomModelHeaders, custom-models.ts)
  • providers.<name>.modelOverrides.<id>.headers

On main, the door accepted a literal secret at either site.

  • The header rule moves into rejectCredentialHeaders, and all three sites call it. Errors name the path, for example providers.x.models[1].headers.X-Key or providers.x.modelOverrides.m.headers.Authorization.
  • A non-mapping providers or modelOverrides node is refused. The SDK schema coerces a list at a record-typed node into an index-keyed record. Probes:
    • providers: [{apiKey: sk-…}] → {"0": {apiKey: …}}, accepted.
    • A modelOverrides list → {"0": {headers: …}}, accepted.
    • The door used to skip both, so the list form of providers also bypassed the apiKey check. That gap predates this PR, and the same rule closes it here.
  • The design record gets a dated amendment ahead of the freeze line naming the extra sites. CP-4's scope is otherwise unchanged.

Stacked on #2224 (RIG-3134), which has the same function and the same header rule.

Tests

  • Red first: 5 model-level cases failed on the parent and pass now. The header-shape cases moved to TestValidateConfigBundleRejectsModelsHeaderShapes (funlen), sharing assertBundleRejects.
  • Mutations go red: skipping the models[] loop, skipping the modelOverrides loop, dropping the providers non-mapping refusal, and dropping the modelOverrides non-mapping refusal.
  • An accept case covers env and !command references at model level.
  • go test ./internal/store/ passes, golangci-lint reports 0 issues, rumdl is clean, and the design-ledger-gate passes.

Out of scope

compat.extraBody and credentials embedded in URLs are not CP-4 surfaces today. That scope decision is RIG-5292, assigned to Matt.

Spec-impact: none (design-record amendment only)
Ledger-impact: none

Closes RIG-5287

Co-authored-by: Matt Wilkinson matt@rigel.build

rejectCredentialModels checked providers.<name>.headers only. The SDK also
reads providers.<name>.models[].headers and
providers.<name>.modelOverrides.<id>.headers, merges them onto the
provider's headers, and resolves each value through resolveConfigValue. A
literal secret at either site passed the door.

The header rule moves into rejectCredentialHeaders, and all three sites
call it, with the offending path named (models[i] or modelOverrides.<id>).
The design record gains a dated amendment naming the extra sites.

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@linear-code

linear-code Bot commented Oct 12, 2026

Copy link
Copy Markdown

RIG-5287

@rigel-mintaka

Copy link
Copy Markdown
Contributor Author

Review at head bf27324c: high 0, medium 0, low 0, nit 0

  • Scope: the whole diff, all lenses, 2 rounds. Each SDK claim was checked by probing getModelsConfigSchema and reading the registry merge paths.
  • Round 1: 2 high. (1) A modelOverrides list is coerced into a record, so the door skipped it. (2) A providers list is coerced the same way, bypassing the whole door; this predates the PR. Both are fixed by refusing non-mapping nodes, which I verified myself. Also 4 low: the sibling-in-item shield case, a doc pointer, amendment citations, and the extraBody/URL scope (filed for Matt as RIG-5292). All folded.
  • Round 2: 3 low, all comment wording. Folded.
  • Checks: go test ./internal/store/ passes, golangci-lint reports 0 issues, rumdl is clean, design-ledger-gate passes. Red first; 4 mutations go red.

@github-actions

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-comms-rig-5287-model.compass-eng-docs.pages.dev

Deployed from compass-comms/rig-5287-model-level-headers at bf27324.

Changed pages:

@rigel-mintaka
rigel-mintaka marked this pull request as ready for review October 12, 2026 03:49
@mattwilkinsonn
mattwilkinsonn added this pull request to stack #2241 October 12, 2026 19:18
@mattwilkinsonn
mattwilkinsonn merged commit b86a40e into main Oct 12, 2026
21 checks passed
@mattwilkinsonn
mattwilkinsonn deleted the compass-comms/rig-5287-model-level-headers branch October 12, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants