refactor(proof/apikey): readability + maintainability sweep#67
Merged
Conversation
"Plaintext" was misleading in a crypto context, and inside the package already collided with the parsed inner `secret` component used by formatToken/parseToken/hashSecret. Rename the field, the VerifyAPIToken parameter, and the parseToken parameter to Token/token. The field holds the full opaque token (ak_<id>_<secret>), so Token reads naturally and mirrors access/jwt.IssuedToken.JWT (PR #61) which named the field by what the value is. Tests outside proof/apikey/ that reference Plaintext on this struct will fail to compile after this commit and are fixed in the cascade commit that follows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pick up the field-rename cascade that the apikey commit started. All three consumer-side names follow the source of truth: - exchange.APITokenRequest.Plaintext -> Token (deferred since PR #63) - management.IssuedAPIToken.Plaintext -> Token (deferred since PR #65) - exchange.APITokenVerifier.VerifyAPIToken parameter -> token - testkit/internal/authflow.Runtime.ExchangeAPIToken parameter -> token - testkit/internal/httpui.authRuntime.ExchangeAPIToken parameter -> token - management testTokenSecret constant -> testToken (the value is the full ak_<id>_<secret> token, not just the inner secret) Comments and test fixtures aligned with the new vocabulary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add godocs to private items the audit flagged as undocumented: the options struct, defaultOptions, formatToken, parseToken, hashSecret, and unauthenticated. parseToken's godoc names the attack class its input-shape check defends against (separator-smuggling in the secret). Annotate the two security-critical branches in VerifyAPIToken: - The ErrTokenNotFound -> unauthenticated mapping is a deliberate existence-leak defense. - The subtle.ConstantTimeCompare branch defends against timing side-channel attacks on the stored secret hash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
godoclint flagged the unauthenticated() godoc for naming errors.Is without the bracket syntax that links to the stdlib symbol. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 28, 2026
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.
Summary
Seventh slice of the multi-session readability sweep (after
access/#61,authz/#62,exchange/#63,http/#64,management/#65,onboarding/#66). First of three PRs coveringproof/. Same 10-criteria bar.Headline change: discharge the deferred
Plaintextrename queued since PR #61.Rename
Plaintext→Tokenproof/apikey.IssuedToken.Plaintext→Token. The field holds the full opaque token (ak_<id>_<secret>), not just the secret material. The package internally distinguishes the parsed innersecretcomponent (parseToken,hashSecret,SecretHash) from the full assembled token — so the previously-proposedSecretrename would have collided.Tokenreads naturally asissued.Tokenand mirrorsaccess/jwt.IssuedToken.JWT(PR #61), which named the field by what the value is.Cascade lands atomically in this PR:
exchange.APITokenRequest.Plaintext→Token(deferred since PR refactor(exchange): readability + maintainability sweep #63)management.IssuedAPIToken.Plaintext→Token(deferred since PR refactor(management): readability + maintainability sweep #65)exchange.APITokenVerifier.VerifyAPITokenparameter →tokentestkit/internal/authflow.Runtime.ExchangeAPITokenparameter →tokentestkit/internal/httpui.authRuntime.ExchangeAPITokenparameter →tokenmanagement.testTokenSecretconstant →testToken(value is the full token, not just the inner secret)Other apikey changes
optionsstruct,defaultOptions(),formatToken,parseToken,hashSecret,unauthenticated.parseToken's godoc names the separator-smuggling attack class its input-shape check defends against.VerifyAPIToken:ErrTokenNotFound→unauthenticatedmapping (existence-leak defense).subtle.ConstantTimeComparebranch (timing side-channel defense).TokenStoreandTokenMetadataListerstay hand-rolled (package-local; convention since PR refactor(access): readability + maintainability sweep + introduce mockery #61).Out of scope
proof/slices land as separate follow-up PRs:proof/oidcandproof/passkey+proof/passkey/session.plaintextparameter in the OIDC paths (testkit/.../runtime.go:298,httpui/server.go:37) is unrelated to this rename and stays for the OIDC pass.Commits
refactor(proof/apikey): rename IssuedToken.Plaintext to Tokenrefactor(exchange,management,testkit): cascade Plaintext to Token renamerefactor(proof/apikey): godocs + inline security commentschore(proof/apikey): link errors.Is in godoc per godoclintTest plan
moon run root:check --summary minimal— format, lint, build, unit, Testcontainers integration all green.Plaintextreferences in API-token paths (only OIDC-pathplaintextparameters survive, intentionally).🤖 Generated with Claude Code