secretsscan: tighten Docker PAT, add new vendor patterns, cap quantifiers#2697
Conversation
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
2 findings (1 LIKELY · 1 CONFIRMED) in newly-added code.
| // openrouter-api-key. OpenRouter (LLM router) keys carry | ||
| // the documented `sk-or-v1-` prefix followed by a 64-char | ||
| // lowercase-hex body. | ||
| expression: `sk-or-v1-[a-f0-9]{64}`, |
There was a problem hiding this comment.
[MEDIUM] OpenRouter key body pattern [a-f0-9]{64} is too narrow — likely causes false negatives
The comment says "64-char lowercase-hex body," but OpenRouter API keys in practice use a base64url-encoded body, not pure hex. Gitleaks' upstream rule for OpenRouter uses [a-zA-Z0-9]{64} (full alphanumeric) rather than [a-f0-9]{64}.
Any real key whose body contains g–z or A–Z would silently bypass redaction with the current pattern.
The test only repeats "a" 64 times — all within the [a-f] set — so it cannot detect this mismatch.
Suggested fix:
expression: `sk-or-v1-[a-zA-Z0-9]{64}`,| {"tailscale_auth_key", "tskey-auth-" + strings.Repeat("h", 12) + "-" + strings.Repeat("i", 90)}, | ||
| {"tailscale_api_token", "tskey-api-" + strings.Repeat("j", 12) + "-" + strings.Repeat("k", 90)}, | ||
| } | ||
| const suffix = " and the rest of the line" |
There was a problem hiding this comment.
[MEDIUM] Quantifier-cap test uses a space-prefixed suffix — the upper bound is never actually validated
TestRedactDoesNotSwallowAdjacentTextAfterPrefixedTokens is supposed to prove that an explicit upper bound (e.g. {20,80}) prevents the regex from swallowing adjacent alphanumeric text. But the suffix is " and the rest of the line" — it starts with a space.
Because every body character class in the tested rules is alphanumeric ([A-Za-z0-9], [A-Za-z0-9_-], etc.), a space already terminates the match even with an unbounded quantifier ({N,}). The test would pass identically if every {N,M} upper bound were replaced with an open-ended {N,}, so it gives no confidence that the caps are real.
To actually exercise the cap, the suffix must consist of characters that the body class would consume — e.g.:
const suffix = strings.Repeat("Z", 200) // alphanumeric, consumed by an unbounded ruleThen the assert that suffix survives would only pass when the quantifier ceiling genuinely blocks it.
3 participants
Expands the secret-redaction catalogue and fixes a class of bugs in the rules added along the way.
Docker PAT/OAT
dckr_pat_rule: dropped(?i)and the hyphen from the body class — Docker's spec is strictly alphanumeric.dckr_oat_).New vendor-prefixed patterns
Confirmed against gitleaks default rules and vendor docs:
ops_eyJsk-or-v1-squ_/sqp_/sqa_pckey_sb_secret_tskey-auth-/tskey-api-vcp_/vck_/vci_rzp_test_/rzp_live_AQEaccess-(sandbox|development|production)-phx_rnd_hcaik_/hcaic_akab-aio_Bug fix: capped greedy quantifiers
The first pass of new rules used unbounded
{N,}quantifiers. With Go's RE2 those match greedily and silently absorb adjacent alphanumeric text into the redaction span —vcp_aaa…aaaTRAILINGTEXTwould collapse to a single[REDACTED], dropping the trailing context. Same class of bug the existingTestRedactDoesNotSwallowAdjacentTextAfterSlackRotatingTokenwas added to prevent.Every new rule now has an explicit upper bound (e.g.
{20,80}for Vercel,{250,1000}for 1Password — RE2's per-quantifier ceiling), and there is a regression test (TestRedactDoesNotSwallowAdjacentTextAfterPrefixedTokens) that pushes each body past its cap and asserts the trailing text survives.