Skip to content

fix: reduce public content credential false positives - #1700

Merged
liangshuo-1 merged 1 commit into
mainfrom
fix/public-content-python-type-annotation
Jul 1, 2026
Merged

fix: reduce public content credential false positives#1700
liangshuo-1 merged 1 commit into
mainfrom
fix/public-content-python-type-annotation

Conversation

@HanShaoshuai-k

@HanShaoshuai-k HanShaoshuai-k commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reduce false positives in the public-content generic credential detector by classifying source-code credential candidates with value and context evidence. This fixes Python token-kind typed state fields while preserving detection for real credential-shaped source literals, typed assignments, numeric secrets, and partially masked credentials.

Changes

  • Update internal/qualitygate/publiccontent/scan.go to treat source-code credential candidates as contextual findings, including typed RHS parsing for Python and TypeScript declarations.
  • Update internal/qualitygate/publiccontent/rules.go to narrow placeholder handling for masked values and printf placeholders.
  • Add synthetic regression coverage in internal/qualitygate/publiccontent/scan_test.go for false-positive controls, real leak controls, typed assignments, masked credentials, and format-string boundaries.

Test Plan

  • make unit-test not run; scoped qualitygate verification was used for this detector-only change.
  • /opt/homebrew/bin/go test ./internal/qualitygate/publiccontent -count=1
  • /opt/homebrew/bin/go test ./internal/qualitygate/... -count=1
  • /opt/homebrew/bin/go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=upstream/main ./internal/qualitygate/publiccontent/...
  • PATH=/opt/homebrew/bin:$PATH make quality-gate QUALITY_GATE_CHANGED_FROM=upstream/main
  • Manual verification: read-only stock scan stayed at 7 non-test candidates after the final rule changes.

Related Issues

N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved public-content credential detection to better distinguish real secrets from placeholder, masked, and dry-run values.
    • Reduced false positives for source-code credential-shaped literals, type annotations, format-string placeholders, and scope-like token assignments.
    • Enhanced handling of partially masked credential strings to ensure mixed real/masked values are flagged when appropriate.
  • Tests

    • Expanded credential-detection test coverage for additional placeholder and edge-case scenarios across multiple languages and token-like patterns.

@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 75b281e9-2033-43df-bd4e-d040199665cc

📥 Commits

Reviewing files that changed from the base of the PR and between 2cca6c9 and 0e095ef.

📒 Files selected for processing (3)
  • internal/qualitygate/publiccontent/rules.go
  • internal/qualitygate/publiccontent/scan.go
  • internal/qualitygate/publiccontent/scan_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • internal/qualitygate/publiccontent/rules.go
  • internal/qualitygate/publiccontent/scan_test.go
  • internal/qualitygate/publiccontent/scan.go

📝 Walkthrough

Walkthrough

This PR expands placeholder detection in rules.go, rewrites public-content credential filtering in scan.go, and adds tests covering new non-secret, placeholder, and masked-value cases.

Changes

Public content credential scanner refinements

Layer / File(s) Summary
Placeholder detection helper additions
internal/qualitygate/publiccontent/rules.go
isPlaceholderValue gains printf, HTML-entity angle, and star-masked placeholder checks; namedPlaceholderValue accepts additional literal placeholder markers.
Token and permission-scope credential filtering
internal/qualitygate/publiccontent/scan.go
Credential-assignment filtering passes file/line/match context; tokenLikePlaceholderValue takes key and value; new helpers classify masked token fixtures, auth token keys, and permission-scope identifier assignments.
Benign code credential expression rewrite
internal/qualitygate/publiccontent/scan.go
isBenignCodeCredentialExpression is rewritten with typed RHS parsing and new non-secret literal classification helpers; codeReferenceExpression acceptance is simplified to codeIdentifier.
Test coverage for edge cases
internal/qualitygate/publiccontent/scan_test.go
New tests cover type annotations, non-secret literals, public placeholders, dry-run/printf exemptions, and detection of masked, typed-secret, and source-code-shaped credentials.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • larksuite/cli#1582: Both PRs change the same public-content placeholder and benign-code credential detection paths.
  • larksuite/cli#1633: Both PRs adjust placeholder recognition and related credential false-positive handling in the same area.

Suggested reviewers: Roy-oss1, liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: reducing public-content credential false positives.
Description check ✅ Passed The description follows the template with Summary, Changes, Test Plan, and Related Issues, and it includes concrete verification steps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/public-content-python-type-annotation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.29126% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.51%. Comparing base (3bda9e1) to head (0e095ef).

Files with missing lines Patch % Lines
internal/qualitygate/publiccontent/scan.go 89.13% 11 Missing and 9 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1700      +/-   ##
==========================================
+ Coverage   74.48%   74.51%   +0.03%     
==========================================
  Files         850      850              
  Lines       86703    86895     +192     
==========================================
+ Hits        64583    64753     +170     
- Misses      17175    17188      +13     
- Partials     4945     4954       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0e095ef9488603bd3dd61a2c56c483bb8db5a13c

🧩 Skill update

npx skills add larksuite/cli#fix/public-content-python-type-annotation -y -g

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

PR Quality Summary

CI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun.

Failed checks

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 298-314: The maskedTokenFixturePlaceholderValue exemption is too
broad and hides partially masked real tokens, so tighten it to only recognize
explicit fixture markers or stronger context instead of any alnum-plus-asterisk
value with six stars. Update maskedTokenFixturePlaceholderValue in scan.go to
stop exempting bare generic token keys like token=ab********cd, and keep the
authCredentialTokenKey() path unchanged for real auth credentials. Add a
regression test covering a bare token key to ensure partially masked secrets
still produce public_content_generic_credential findings.
- Around line 401-403: The plus-concatenation shortcut in the scan logic is too
broad and hides literal-only secrets. Update the `strings.Contains(match, "+")`
handling in the relevant `scan.go` check so it only treats concatenations as
safe when they involve identifiers or references, not string literals. Use the
existing match/parsing logic in this path to distinguish `"real-" +
"secret-value"` from variable-based expressions, and add a regression test
covering concatenated string literals to ensure they still get flagged.
🪄 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: 2fe30a45-6b04-4fca-ac69-d209155873a2

📥 Commits

Reviewing files that changed from the base of the PR and between e753b15 and 35de911.

📒 Files selected for processing (3)
  • internal/qualitygate/publiccontent/rules.go
  • internal/qualitygate/publiccontent/scan.go
  • internal/qualitygate/publiccontent/scan_test.go

Comment thread internal/qualitygate/publiccontent/scan.go
Comment thread internal/qualitygate/publiccontent/scan.go
@HanShaoshuai-k
HanShaoshuai-k force-pushed the fix/public-content-python-type-annotation branch from 35de911 to 2cca6c9 Compare July 1, 2026 08:07
@HanShaoshuai-k
HanShaoshuai-k force-pushed the fix/public-content-python-type-annotation branch from 2cca6c9 to 0e095ef Compare July 1, 2026 09:21
@liangshuo-1
liangshuo-1 merged commit e8bfbab into main Jul 1, 2026
31 of 35 checks passed
@liangshuo-1
liangshuo-1 deleted the fix/public-content-python-type-annotation branch July 1, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants