Skip to content

fix(k8sjwt): skip unusable keys in JWKS discovery#499

Merged
Zoe Zhao (zoez7) merged 1 commit into
agent-substrate:mainfrom
aramase:aramase/i/k8sjwt_skip_unusable
Jul 23, 2026
Merged

fix(k8sjwt): skip unusable keys in JWKS discovery#499
Zoe Zhao (zoez7) merged 1 commit into
agent-substrate:mainfrom
aramase:aramase/i/k8sjwt_skip_unusable

Conversation

@aramase

@aramase Anish Ramasekar (aramase) commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

discoverKeysForIssuer now parses each JWK via parseJWK and skips keys the verifier can't use (unsupported kty/curve, malformed params, or no kid), failing only when no usable key remains — with distinct errors for empty JWKS vs no usable keys ... (N skipped). Matches the go-oidc/jose convention: skip at discovery, fail only when a token's kid matches no usable key. Selection stays by kid and signatures are still verified, so skipping cannot enable forgery.

Behavior note

A key with no kid is now skipped instead of failing the whole set. Safe: tokens must carry a kid and match by exact kid, so a no-kid key was never selectable anyway.

  • Tests pass
  • Appropriate changes to documentation are included in the PR — N/A

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI first pass experiment ...]

Comment thread cmd/ateapi/internal/k8sjwt/k8sjwt.go Outdated
}

if skipped > 0 {
slog.InfoContext(ctx, "Skipped unusable JWKs from issuer",

@BenTheElder Benjamin Elder (BenTheElder) Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 nit 🟢 – This Info summary fires on every Verify (discovery is per-request until key caching lands) — the same per-request noise the PR avoided by putting per-key skips at Debug. Consider Debug here too, or rate-limiting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was on the fence on this one. Moved it to Debug for now, will bump it back to Info once discovery is cached (the TODO above).

A key the verifier can't parse no longer fails discovery for the whole
issuer: skip it, erroring only when no usable keys remain. Selection stays
by kid + signature, so skipping is safe.
@zoez7
Zoe Zhao (zoez7) merged commit 73b8240 into agent-substrate:main Jul 23, 2026
11 checks passed
@aramase
Anish Ramasekar (aramase) deleted the aramase/i/k8sjwt_skip_unusable branch July 23, 2026 18:12
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.

3 participants