fix: reduce public content credential fixture false positives - #1929
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe public-content scanner now uses provider-specific credential evidence, refined assignment parsing, and broader benign-value handling. Comment auditing carries review-comment paths into scanning and diagnostics, with expanded tests for detection and fixture behavior. ChangesPublic content auditing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EventFile
participant CommentAudit
participant PublicContent
participant Diagnostics
EventFile->>CommentAudit: read event body and review-comment path
CommentAudit->>PublicContent: ScanCommentAtPath(kind, path, body)
PublicContent->>Diagnostics: return diagnostics with the resolved path
CommentAudit->>Diagnostics: print diagnostics and exit with report.ExitCode
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/qualitygate/publiccontent/scan.go`:
- Around line 62-64: Update sourceCodeTypedCredentialRHS and both its
evidence-extraction and benign-expression classification call sites to accept
and use the current match offset location[0], rather than searching from the
start of the line. Propagate that offset through all affected paths, including
the additional occurrences noted in the review. Add an adjacent regression test
with two identical typed assignments on one line where only the second RHS is
provider-shaped, and verify the second assignment is evaluated correctly.
🪄 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: 31834fb3-2579-4c41-95ba-f650b46bc221
📒 Files selected for processing (10)
internal/qualitygate/cmd/comment-audit/main.gointernal/qualitygate/cmd/comment-audit/main_test.gointernal/qualitygate/publiccontent/collect_test.gointernal/qualitygate/publiccontent/comment_audit.gointernal/qualitygate/publiccontent/comment_audit_test.gointernal/qualitygate/publiccontent/credential.gointernal/qualitygate/publiccontent/rules.gointernal/qualitygate/publiccontent/scan.gointernal/qualitygate/publiccontent/scan_test.gointernal/qualitygate/rules/run_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2ede076dde0e04336618291110b89ecc84730d51🧩 Skill updatenpx skills add larksuite/cli#fix/public-content-credential-fixtures -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1929 +/- ##
==========================================
- Coverage 75.02% 74.97% -0.06%
==========================================
Files 894 895 +1
Lines 94294 94379 +85
==========================================
+ Hits 70746 70761 +15
- Misses 18137 18200 +63
- Partials 5411 5418 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
HanShaoshuai-k
force-pushed
the
fix/public-content-credential-fixtures
branch
from
July 17, 2026 06:51
8465147 to
0ee2e94
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/qualitygate/publiccontent/scan_test.go`:
- Around line 417-418: Replace aggregate count-only assertions with individual
classification assertions in internal/qualitygate/publiccontent/scan_test.go at
lines 417-418, 394-395, 597-598, 1150-1163, 1183-1184, and 1656-1657: verify
each benign-key, provider, readable, placeholder, expression, and source-literal
fixture directly as allowed or rejected, then retain combined deduplication
checks where applicable.
🪄 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: c8e13390-295e-4117-9338-57fe18486bd3
📒 Files selected for processing (10)
internal/qualitygate/cmd/comment-audit/main.gointernal/qualitygate/cmd/comment-audit/main_test.gointernal/qualitygate/publiccontent/collect_test.gointernal/qualitygate/publiccontent/comment_audit.gointernal/qualitygate/publiccontent/comment_audit_test.gointernal/qualitygate/publiccontent/credential.gointernal/qualitygate/publiccontent/rules.gointernal/qualitygate/publiccontent/scan.gointernal/qualitygate/publiccontent/scan_test.gointernal/qualitygate/rules/run_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/qualitygate/publiccontent/comment_audit_test.go
- internal/qualitygate/cmd/comment-audit/main.go
- internal/qualitygate/publiccontent/credential.go
- internal/qualitygate/cmd/comment-audit/main_test.go
- internal/qualitygate/publiccontent/rules.go
- internal/qualitygate/publiccontent/scan.go
- internal/qualitygate/publiccontent/collect_test.go
HanShaoshuai-k
force-pushed
the
fix/public-content-credential-fixtures
branch
from
July 17, 2026 07:03
0ee2e94 to
037667f
Compare
HanShaoshuai-k
force-pushed
the
fix/public-content-credential-fixtures
branch
2 times, most recently
from
July 20, 2026 03:16
f6a0c8d to
ee7d781
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/qualitygate/cmd/comment-audit/main_test.go (1)
47-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract working directory setup into a helper.
This
os.Getwd,os.Chdir, andt.Cleanupboilerplate is repeated across multiple tests in this file (here, lines 22-31, and lines 74-83) to safely supply a relative path. Extracting this into a small test helper (or usingt.Chdir(dir)if this project uses Go 1.24+) would reduce duplication.♻️ Proposed helper extraction
// Add this helper to the bottom of the file func chdirTemp(t *testing.T) string { dir := t.TempDir() origDir, err := os.Getwd() if err != nil { t.Fatal(err) } if err := os.Chdir(dir); err != nil { t.Fatal(err) } t.Cleanup(func() { _ = os.Chdir(origDir) }) return dir }Then replace the boilerplate in the tests:
- dir := t.TempDir() + dir := chdirTemp(t) if err := writeTestFile(filepath.Join(dir, "event.json"), `{"comment":{"body":"test suggestion","path":"cmd/agent/list_test.go"}}`); err != nil { t.Fatal(err) } - origDir, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - _ = os.Chdir(origDir) - })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/qualitygate/cmd/comment-audit/main_test.go` around lines 47 - 56, Extract the repeated working-directory setup into a shared test helper, such as chdirTemp, in main_test.go. Have it create and return a temporary directory, change into it, and register cleanup restoring the original directory; replace the duplicated os.Getwd, os.Chdir, and t.Cleanup blocks in the affected tests while preserving their existing directory usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/qualitygate/publiccontent/rules.go`:
- Around line 385-388: Update credentialShapedValue to apply strings.TrimSpace
to value before removing boundary quotes and angle brackets with strings.Trim,
ensuring padded credential strings are normalized before
credentialShapedIdentifier is called.
---
Nitpick comments:
In `@internal/qualitygate/cmd/comment-audit/main_test.go`:
- Around line 47-56: Extract the repeated working-directory setup into a shared
test helper, such as chdirTemp, in main_test.go. Have it create and return a
temporary directory, change into it, and register cleanup restoring the original
directory; replace the duplicated os.Getwd, os.Chdir, and t.Cleanup blocks in
the affected tests while preserving their existing directory usage.
🪄 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: b0e07eb4-ca3a-41bd-aba5-bbce1af6a789
📒 Files selected for processing (10)
internal/qualitygate/cmd/comment-audit/main.gointernal/qualitygate/cmd/comment-audit/main_test.gointernal/qualitygate/publiccontent/collect_test.gointernal/qualitygate/publiccontent/comment_audit.gointernal/qualitygate/publiccontent/comment_audit_test.gointernal/qualitygate/publiccontent/credential.gointernal/qualitygate/publiccontent/rules.gointernal/qualitygate/publiccontent/scan.gointernal/qualitygate/publiccontent/scan_test.gointernal/qualitygate/rules/run_test.go
🚧 Files skipped from review as they are similar to previous changes (8)
- internal/qualitygate/publiccontent/comment_audit.go
- internal/qualitygate/rules/run_test.go
- internal/qualitygate/publiccontent/comment_audit_test.go
- internal/qualitygate/cmd/comment-audit/main.go
- internal/qualitygate/publiccontent/credential.go
- internal/qualitygate/publiccontent/collect_test.go
- internal/qualitygate/publiccontent/scan.go
- internal/qualitygate/publiccontent/scan_test.go
HanShaoshuai-k
force-pushed
the
fix/public-content-credential-fixtures
branch
from
July 20, 2026 03:28
ee7d781 to
2ede076
Compare
liangshuo-1
approved these changes
Jul 20, 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
2 participants
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
Reduce false positives in public-content credential detection while preserving detection of real credentials.
Changes
Test Plan
Summary by CodeRabbit