Report what a JWT claims, not what it is - #126
Merged
Merged
Conversation
The scanner decoded a token's exp claim and then printed conclusions it had not earned: "LIVE until 2036" and, for a token with no exp, "this token does not stop working". Neither is knowable from decoding. We never ask an issuer anything, so a token's own statement about its lifetime proves nothing about whether it currently works or has already been revoked. A revoked key with a 2036 exp claim prints as LIVE; that is the exact direction a security tool must not be wrong in. Raised by codexmb in the post-merge review of #123 as a non-blocking follow-up. Wording is now scoped to the claim in both the stageable and history scanners, and in the EXPIRED branch too, which had the same defect in the other direction: past exp is still only a claim, and says nothing about revocation. The tests asserted the old strings, so they move with it, and they now also assert the old wording CANNOT come back. Reintroducing either phrase on a copy fails the suite, which is what makes this a guard rather than a restatement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
To use Codex here, create an environment for this repo. |
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
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.
Post-merge follow-up to #123, raised by @codexmb as non-blocking.
The defect
The scanner decoded a JWT
expclaim and printed a conclusion it had not earned:Decoding proves neither. We never ask an issuer anything, so the token's own statement about its lifetime says nothing about whether it currently works or has already been revoked. A revoked key carrying a 2036 exp claim printed as LIVE — that is the one direction a security tool must not be wrong in, because it turns "I could not check" into "I checked and it is fine".
The fix
Wording is scoped to the claim, in both scanners:
The
EXPIREDbranch is included although codexmb only named the other two. It had the same defect pointing the other way: a pastexpis still only a claim, and a token being past its stated expiry tells you nothing about whether it was revoked before that.Also corrected a comment in
secret-patterns.mjsthat described what a token "IS".Why the tests changed, and the negative control
The tests asserted the old strings, so they had to move. But swapping expected strings would leave a suite that merely restates the implementation. They now also assert the overclaiming phrases cannot come back:
Verified by reintroducing the old wording on a copy in a temp dir: the suite fails, exit 1. With the fix in place the full suite passes, exit 0, read directly rather than through a pipe.
Reviewer artifact:
review-123-security.md. This does not reopen the disclosure or false-clean cases, which codexmb confirmed resolved at6e80478.🤖 Generated with Claude Code