feat: validate exchange_token() inputs before the round-trip#114
Merged
Conversation
Follow-up to #113 (agent SSO). Applies the #112 discipline to the two token-exchange inputs that have a high-confidence, zero-false-positive local check: - `audience` is required; empty/whitespace comes back as invalid_target / invalid_request (400). Reuses `_require_nonempty`. - `subject_token`, when passed explicitly, must be a JWT. A `col_...` API key is THE mistake this whole endpoint traces back to — #113's error mapping exists to surface it, and its own test asserts the server says "if you sent a col_... API key". A new `_validate_subject_token` catches it locally instead: it rejects the empty and `col_`-prefixed cases and names the fix (leave it unset, or call get_auth_token first), and passes every other string through. Same spirit as `_validate_totp_code` rejecting a base32 *secret*. `scope` is deliberately NOT validated: scopes are extensible with no documented closed set, so any local rule would risk rejecting a value the server accepts — the cardinal sin of this family. Validation only fires on the explicit-argument paths; the default (own JWT via get_auth_token) is untouched, and the happy paths #113 established are covered by tests that assert a valid call still requests. Applied to sync + async (validators live in client.py, imported by async_client.py, as with the rest of the family). 13 new tests. Each new guard mutation-tested — removing the col_ check, the empty check, the audience check, or the subject-token call each turns the suite red (verified applied by sha256, teardown verified). Full unit suite 1094 passed; ruff format + check clean; no version bump.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #113. You asked whether the token-exchange functions should also validate their inputs — two of the three do, with the same discipline as #112 (reject only what the server definitively rejects; never false-reject).
No version bump.
What's validated
audience_require_nonemptyinvalid_target/invalid_request(400).subject_token(explicit only)_validate_subject_token: reject empty andcol_...prefixcol_...API key is the mistake this endpoint traces back to — #113's error mapping exists to surface it, and its own test asserts the server says "if you sent a col_... API key". Catching it locally names the fix before the round-trip. Does not parse JWT structure — an opaque non-col_token passes through, so it can't reject a token the server would accept.What's deliberately NOT validated
scope— scopes are extensible with no documented closed set. Any local rule risks rejecting a value the server accepts, which is the cardinal sin of this family. Left to the server.get_auth_token()— no inputs.Scope of effect
Validation fires only on the explicit-argument paths. The default path (own JWT via
get_auth_token) is untouched, and the ergonomic one-linerexchange_token(audience=...)is unaffected. Tests assert that valid calls — default subject token and an explicit JWT — still reach the transport._validate_subject_tokenmirrors_validate_totp_code's move (reject the one unambiguous wrong value, pass everything else) and lives inclient.py, imported byasync_client.py, as with the rest of the guard family. Applied to sync + async.Verification
col_check, the empty check, the audience check, or the subject-token call each turns the suite red — verified applied by sha256 before/after, teardown verified.ruff format --check+ruff checkclean; version untouched at 1.28.0.