Conversation
Gitleaks rules only matched AKIA-prefixed access key IDs but missed the paired secret access key values. Add a new rule matching 40-char base64 strings preceded by aws_secret_access_key/aws_secret_key/secret_access_key keywords, with entropy threshold 4.0 and the AWS example key allowlisted.
Replace the static Style int enum and Replacement() method with a Replacer interface that accepts per-finding context (value + Finding). This enables context-aware replacements needed for hash style and tokenization. Eliminates the thread-unsafe customReplacement global. Also fix env format processor to detect against the full line so keyword-based rules (e.g., aws_secret_access_key) see the key name for pre-filtering, while still only redacting the value portion.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/output/terminal.go (1)
69-75:⚠️ Potential issue | 🟠 MajorKeep adjacent findings as separate replacement spans.
As in
Redact, Line 72 merges adjacent exclusive-end ranges. That drops the second replacement for back-to-back findings.🐛 Proposed fix
- if s.start <= last.end { + if s.start < last.end {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/output/terminal.go` around lines 69 - 75, The merging loop in terminal.go (involving merged []coloredSpan, the loop over spans[1:], and last := &merged[len(merged)-1]) treats adjacent exclusive-end ranges as overlapping by using the condition s.start <= last.end; change that comparison to strictly overlapping (s.start < last.end) so adjacent (exclusive-end) replacement spans are kept separate like in Redact, and retain the existing extension logic (if s.end > last.end { last.end = s.end }) for true overlaps.internal/redact/redactor.go (1)
37-43:⚠️ Potential issue | 🟠 MajorDo not merge adjacent findings as overlaps.
Line 40 treats
[start:end]spans that merely touch as overlapping. BecauseEndis an exclusive slice bound, adjacent findings should each get their own replacement; otherwise context-aware replacements drop the later finding.🐛 Proposed fix
- if s.start <= last.end { + if s.start < last.end {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/redact/redactor.go` around lines 37 - 43, The merge logic in the spans coalescing loop (in internal/redact/redactor.go where merged := []replacementSpan{spans[0]} and the for _, s := range spans[1:] loop runs) treats touching spans as overlapping; change the overlap check from using <= to < (i.e., only merge when s.start < last.end) so adjacent/exclusive-end spans are not merged and each replacementSpan (s) remains separate.
🧹 Nitpick comments (2)
internal/detect/patterns/gitleaks.toml (1)
233-236: Allowlist regex is fine; consider anchoring or adding common example variants.The entry matches the canonical AWS docs example key exactly (via substring match on
value, perisAlloweddefaultOR/match-target). That's sufficient for the current goal. Two small, optional improvements:
- Anchor with
^…$(or use\b) if you want to avoid accidental partial matches when the example appears as a prefix/suffix of something unusual.- AWS SDK docs occasionally use
bPxRfiCYEXAMPLEKEYwith different casings or theAKIAIOSFODNN7EXAMPLEaccess-key counterpart; those already won't match this rule's regex so no action needed — just flagging for awareness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/detect/patterns/gitleaks.toml` around lines 233 - 236, The allowlist entry under [[rules.allowlists]] currently matches the exact AWS example secret via the regex in the regexes array; tighten it by anchoring the pattern (e.g., add ^ and $ or \b) or add common example variants (different casing or the AKIA... access-key form) so it doesn't accidentally match substrings of larger values—update the regex in the regexes list for the rule to use anchored patterns or add additional regex entries to cover common example variants.internal/detect/detector_test.go (1)
28-81: LGTM — good table-driven coverage.The test exercises the rule across env, quoted, and YAML styles, and includes both allowlist and too-short negatives. Entropy of the first case (with
zzzzzzzzpadding) computes to ~4.57, so it safely clears the 4.0 threshold — nice choice of fixture.One optional hardening: consider also asserting
f.Valuefor the positive cases (not justf.RuleID) to catch regressions insecretGrouphandling where the wrong capture might be surfaced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/detect/detector_test.go` around lines 28 - 81, Enhance TestDetector_AWSSecretAccessKey to also assert the captured secret string (f.Value) for positive detections: add an expected field (e.g., wantValue string) to the test case struct for the positive cases, populate it with the exact secret substring you expect from the input, and inside the loop where you check for f.RuleID == "aws-secret-access-key" also compare f.Value to tt.wantValue (use t.Errorf when mismatched); keep the existing found boolean logic and only perform the f.Value assertion for cases where tt.want is true so negative cases remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/detect/patterns/gitleaks.toml`:
- Line 225: The alternation (?:=|:|=>) in the regex inside
internal/detect/patterns/gitleaks.toml makes the => branch unreachable; change
the alternation to put the longest token first (e.g., (?:=>|=|:)) so fat-arrow
Ruby hashes can match, update the regex assignment (the regex variable on that
line) accordingly, and add a unit test in detector_test.go that includes a
sample like `secret_access_key => "..."` to ensure the => form is detected.
In `@internal/output/terminal.go`:
- Around line 20-24: The TTY path in Terminal regenerates redactions from
original (via colorize/Replace) which can leak multiline secrets and break
stateful replacers; instead, when findings are present always use the
already-safe redacted string passed into Terminal and do not call
replacer.Replace again. Update Terminal(original, redacted, findings, replacer)
to return redacted for the TTY branch and remove or short‑circuit any second
call to replacer.Replace inside colorize (or wherever Replace is invoked a
second time) so the code uses the precomputed redacted buffer rather than
recomputing replacements from original.
In `@internal/redact/redactor.go`:
- Line 59: The file internal/redact/redactor.go is not gofmt-formatted; run
gofmt (or gofmt -w) on that file (or run gofmt -w ./internal/redact or goimports
-w if you prefer) to reformat the source so it passes the lint check, then
re-run the pipeline; no code changes beyond formatting are required in the
redactor.go package/types.
---
Outside diff comments:
In `@internal/output/terminal.go`:
- Around line 69-75: The merging loop in terminal.go (involving merged
[]coloredSpan, the loop over spans[1:], and last := &merged[len(merged)-1])
treats adjacent exclusive-end ranges as overlapping by using the condition
s.start <= last.end; change that comparison to strictly overlapping (s.start <
last.end) so adjacent (exclusive-end) replacement spans are kept separate like
in Redact, and retain the existing extension logic (if s.end > last.end {
last.end = s.end }) for true overlaps.
In `@internal/redact/redactor.go`:
- Around line 37-43: The merge logic in the spans coalescing loop (in
internal/redact/redactor.go where merged := []replacementSpan{spans[0]} and the
for _, s := range spans[1:] loop runs) treats touching spans as overlapping;
change the overlap check from using <= to < (i.e., only merge when s.start <
last.end) so adjacent/exclusive-end spans are not merged and each
replacementSpan (s) remains separate.
---
Nitpick comments:
In `@internal/detect/detector_test.go`:
- Around line 28-81: Enhance TestDetector_AWSSecretAccessKey to also assert the
captured secret string (f.Value) for positive detections: add an expected field
(e.g., wantValue string) to the test case struct for the positive cases,
populate it with the exact secret substring you expect from the input, and
inside the loop where you check for f.RuleID == "aws-secret-access-key" also
compare f.Value to tt.wantValue (use t.Errorf when mismatched); keep the
existing found boolean logic and only perform the f.Value assertion for cases
where tt.want is true so negative cases remain unchanged.
In `@internal/detect/patterns/gitleaks.toml`:
- Around line 233-236: The allowlist entry under [[rules.allowlists]] currently
matches the exact AWS example secret via the regex in the regexes array; tighten
it by anchoring the pattern (e.g., add ^ and $ or \b) or add common example
variants (different casing or the AKIA... access-key form) so it doesn't
accidentally match substrings of larger values—update the regex in the regexes
list for the rule to use anchored patterns or add additional regex entries to
cover common example variants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4475adda-0e3e-4a23-a5ae-ffd790c600c8
📒 Files selected for processing (13)
internal/cmd/root.gointernal/detect/detector_test.gointernal/detect/patterns/gitleaks.tomlinternal/format/env.gointernal/format/format_test.gointernal/format/json.gointernal/format/plain.gointernal/format/process.gointernal/format/yaml.gointernal/output/terminal.gointernal/redact/redactor.gointernal/redact/redactor_test.gointernal/redact/styles.go
| [[rules]] | ||
| id = "aws-secret-access-key" | ||
| description = "Detected an AWS Secret Access Key, risking unauthorized access to AWS services and data breaches." | ||
| regex = '''(?i)(?:aws_secret_access_key|aws_secret_key|secret_access_key)[\s'"]{0,3}(?:=|:|=>)[\s'"]{0,5}([A-Za-z0-9/+=]{40})(?:[\x60'"\s;]|\\[nr]|$)''' |
There was a problem hiding this comment.
Alternation ordering makes the => branch dead code.
In (?:=|:|=>), regex engines try alternatives left-to-right, so for input like secret_access_key => "…", the engine matches = first, then [\s'"]{0,5} cannot consume the > (it's not whitespace/quote), and the following capture class [A-Za-z0-9/+=]{40} also rejects >. The match fails even though the => alternative was presumably added to support Ruby-hash/fat-arrow style. Put the longest alternative first so it has a chance to match.
🐛 Proposed fix
-regex = '''(?i)(?:aws_secret_access_key|aws_secret_key|secret_access_key)[\s'"]{0,3}(?:=|:|=>)[\s'"]{0,5}([A-Za-z0-9/+=]{40})(?:[\x60'"\s;]|\\[nr]|$)'''
+regex = '''(?i)(?:aws_secret_access_key|aws_secret_key|secret_access_key)[\s'"]{0,3}(?:=>|=|:)[\s'"]{0,5}([A-Za-z0-9/+=]{40})(?:[\x60'"\s;]|\\[nr]|$)'''Consider adding a test case with => in detector_test.go once fixed, to lock this in.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| regex = '''(?i)(?:aws_secret_access_key|aws_secret_key|secret_access_key)[\s'"]{0,3}(?:=|:|=>)[\s'"]{0,5}([A-Za-z0-9/+=]{40})(?:[\x60'"\s;]|\\[nr]|$)''' | |
| regex = '''(?i)(?:aws_secret_access_key|aws_secret_key|secret_access_key)[\s'"]{0,3}(?:=>|=|:)[\s'"]{0,5}([A-Za-z0-9/+=]{40})(?:[\x60'"\s;]|\\[nr]|$)''' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/detect/patterns/gitleaks.toml` at line 225, The alternation
(?:=|:|=>) in the regex inside internal/detect/patterns/gitleaks.toml makes the
=> branch unreachable; change the alternation to put the longest token first
(e.g., (?:=>|=|:)) so fat-arrow Ruby hashes can match, update the regex
assignment (the regex variable on that line) accordingly, and add a unit test in
detector_test.go that includes a sample like `secret_access_key => "..."` to
ensure the => form is detected.
Comment on lines
+20
to
+24
| func Terminal(original, redacted string, findings []detect.Finding, replacer redact.Replacer) string { | ||
| if !isTTY() || len(findings) == 0 { | ||
| return redacted | ||
| } | ||
| return colorize(original, findings, style) | ||
| return colorize(original, findings, replacer) |
There was a problem hiding this comment.
Do not regenerate redactions from original in the TTY path.
Line 24 discards the already-safe redacted buffer, and Line 59 calls Replace a second time. This can leak multiline secrets in terminal output because multiline findings are normalized to the first line, and it can produce different tokens for stateful replacers.
🛡️ Proposed safe guard
func Terminal(original, redacted string, findings []detect.Finding, replacer redact.Replacer) string {
if !isTTY() || len(findings) == 0 {
return redacted
}
+ if _, ok := redact.StaticReplacement(replacer); !ok ||
+ strings.Count(original, "\n") != strings.Count(redacted, "\n") {
+ return redacted
+ }
return colorize(original, findings, replacer)
}Also applies to: 57-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/output/terminal.go` around lines 20 - 24, The TTY path in Terminal
regenerates redactions from original (via colorize/Replace) which can leak
multiline secrets and break stateful replacers; instead, when findings are
present always use the already-safe redacted string passed into Terminal and do
not call replacer.Replace again. Update Terminal(original, redacted, findings,
replacer) to return redacted for the TTY branch and remove or short‑circuit any
second call to replacer.Replace inside colorize (or wherever Replace is invoked
a second time) so the code uses the precomputed redacted buffer rather than
recomputing replacements from original.
| } | ||
|
|
There was a problem hiding this comment.
Run gofmt to clear the failing lint check.
The pipeline reports this file is not gofmt-formatted.
🧰 Tools
🪛 GitHub Actions: PR
[error] 59-59: golangci-lint: File is not properly formatted (gofmt). Run gofmt on this file.
🪛 GitHub Check: lint
[failure] 59-59:
File is not properly formatted (gofmt)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/redact/redactor.go` at line 59, The file internal/redact/redactor.go
is not gofmt-formatted; run gofmt (or gofmt -w) on that file (or run gofmt -w
./internal/redact or goimports -w if you prefer) to reformat the source so it
passes the lint check, then re-run the pipeline; no code changes beyond
formatting are required in the redactor.go package/types.
- Move internal/detect/, internal/redact/, internal/format/ to top-level packages - Create public API entry point: wick.Redact(input, ...Option) (string, Report, error) - Add functional options pattern: WithReplacer(), WithCustomPatterns() - Create Report and Finding types for structured results - Update all import paths across internal packages - Add 6 tests for public API (basic, options, custom patterns, JSON, concurrency) - All 33 tests pass, go vet clean, GoDoc renders correctly This establishes the public library foundation for v0.2 (R6).
Adds --style hash which replaces each detected value with a deterministic, one-way bracketed tag: [TYPE:abcdef12]. The tag uses the uppercased rule ID and a truncated SHA-256 of the original value, enabling log correlation across multiple redacted outputs without exposing the underlying data.
Adds --tokenize and --rehydrate modes to the CLI, plus Dehydrate() and Rehydrate() to the public Go API. --tokenize replaces each unique detected value with a stable token of the form [RULEID-N], writes an AES-256-GCM encrypted token map to disk (default: .wick-tokens.enc), and prints the encryption key to stderr. The same value always maps to the same token within a single run. --rehydrate reads the token map and restores the original values. Round-trip invariant: rehydrate(dehydrate(input)) == input. New public API: GenerateKey, DecodeKey, Dehydrate, Rehydrate, SaveTokenMap, LoadTokenMap, TokenMap, TokenEntry.
Allowlist entries in .wick.yaml suppress detection for known-safe values, supporting exact string matches (case-insensitive) and regular expressions. Blocklist entries always redact matching values even if no built-in rule would catch them. Both are configurable via .wick.yaml, the public Go API (WithAllowlist, WithBlocklist), and are applied consistently across all input formats and CLI modes including --tokenize.
--report prints a line-by-line report to stderr showing line number, column, category, rule ID, and a truncated value preview for each finding. Output goes to stderr so it does not interfere with the redacted stdout. Composes correctly with --summary, --format json, and all other flags.
…r-pattern replacements - AddRulesFile loads extra Gitleaks-compatible TOML rules at runtime - DisableRules filters both secret and PII rules by ID - PerRule replacer wraps any Replacer with per-ruleID replacement overrides - Custom patterns support a Replacement field for per-pattern redaction strings - Wired through public API (WithRulesFile, WithDisabledRules options) and CLI (rules_file, disable_rules in .wick.yaml)
Semgrep findings requiring manual fixesThese findings have no automatic fix and require manual remediation.
|
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
aws-secret-access-keyrule to gitleaks.toml matching 40-char base64 values preceded byaws_secret_access_key/aws_secret_key/secret_access_keykeywords (entropy 4.0, AWS example key allowlisted)Styleint enum withReplacerinterface accepting per-finding context (value+Finding), enabling context-aware replacements for upcoming hash style and tokenizationTest plan
make test— all 27 tests pass with-racegolangci-lint— no issues--style stars,--style custom=XXX, JSON, YAML, env formatswJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY) properly allowlistedSummary by CodeRabbit
New Features
Refactor