fix: update GitHub token redaction regex for new JWT format - #736
Conversation
GitHub's April 2026 token format change introduces underscores in ghs_ installation tokens (ghs_APPID_JWT, ~520 chars). The existing regex [a-zA-Z0-9] fails to match these, causing tokens to leak unredacted in logs. Add underscore to the character class for all GitHub token prefix patterns (ghp_, gho_, ghu_, ghs_, ghr_). Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
Site previewPreview: https://64c5df6f-site.fullsend-ai.workers.dev Commit: |
Review: #736Head SHA: 23ffc72 SummaryThe change correctly broadens GitHub token redaction regex patterns to handle the new JWT-style installation token format ( FindingsInfo
FooterOutcome: approve Previous runReview: #736Head SHA: 4c0f372 SummaryThis is a well-scoped security fix that adds underscore ( FindingsNo findings. FooterOutcome: approve |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Nice catch on the token format change. Two notes below — neither blocks merge, but worth considering before the broader rollout hits.
Sources consulted:
| {"github_pat", `ghp_[a-zA-Z0-9_]{36,}`}, | ||
| {"github_oauth", `gho_[a-zA-Z0-9_]{36,}`}, | ||
| {"github_user_token", `ghu_[a-zA-Z0-9_]{36,}`}, | ||
| {"github_server_token", `ghs_[a-zA-Z0-9_]{36,}`}, |
There was a problem hiding this comment.
[critical — deferred] The GitHub blog post describes the new format as ghs_APPID_JWT and says tokens should be treated as opaque strings (~520 chars). Standard JWTs contain dots (.) as segment separators and base64url encoding uses hyphens (-). If the actual token includes these characters, this regex [a-zA-Z0-9_] will stop matching at the first dot, leaking the remaining JWT segments unredacted.
The test case at scanner_test.go constructs a token by repeating a base64 string without dots, so it wouldn't catch this.
Worth verifying against a real token from the rollout (which started April 27). If dots are present, the character class should be [a-zA-Z0-9_.\-] and the test should include a dot-separated JWT structure.
| {"github_user_token", `ghu_[a-zA-Z0-9]{36,}`}, | ||
| {"github_server_token", `ghs_[a-zA-Z0-9]{36,}`}, | ||
| {"github_refresh_token", `ghr_[a-zA-Z0-9]{36,}`}, | ||
| {"github_pat", `ghp_[a-zA-Z0-9_]{36,}`}, |
There was a problem hiding this comment.
[moderate — informational] Per the GitHub announcement, only ghs_ (installation server-to-server) tokens are changing to the JWT format right now. The other types (ghp_, gho_, ghu_, ghr_) are noted as being "addressed separately in future updates." Adding _ to all five is harmless and arguably good future-proofing — just noting for accuracy.
The new JWT-based ghs_ tokens use base64url segments separated by dots (header.payload.signature). The previous character class [a-zA-Z0-9_] stops matching at the first dot, leaking the remaining JWT segments unredacted. Update ghs_ pattern to [a-zA-Z0-9_.\-] and fix the test to use a realistic dot-separated JWT structure. Signed-off-by: Wayne Sun <gsun@redhat.com>
|
fullsend review is working on this — view logs |
3 participants
Summary
ghp_,gho_,ghu_,ghs_,ghr_) to include underscores in the character classghs_APPID_JWTtokens (~520 chars) containing underscores between segments[a-zA-Z0-9]character class would fail to match the full token, leaving new-format installation tokens unredacted in logsghs_tokens with underscoresTest plan
go test ./internal/security/... -run TestSecretRedactor— all pass including new JWT format testgo vet ./internal/security/...— cleangithub_pattest still passes (no regression)