Skip to content

fix: reduce public content token false positives - #1718

Merged
HanShaoshuai-k merged 1 commit into
mainfrom
fix/public-content-resource-token-placeholders
Jul 2, 2026
Merged

fix: reduce public content token false positives#1718
HanShaoshuai-k merged 1 commit into
mainfrom
fix/public-content-resource-token-placeholders

Conversation

@HanShaoshuai-k

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

Copy link
Copy Markdown
Collaborator

Summary

Reduce false positives in public-content scanning by treating weak token-like fields, fixture credential URLs, and private IP fixtures with more precise evidence checks.

Changes

  • Require stronger value evidence before public_content_generic_credential rejects weak token-like fields such as token, image_token, and resource-style *_token fields.
  • Keep strong auth token fields such as access_token, api_token, service_token, bot_token, and secret_token rejected.
  • Allow obvious credential URL fixtures in source/test files while preserving rejects for credential-shaped URL passwords.
  • Suppress private IPv4 warnings in source/test fixtures while keeping warnings for docs/config-style public content.
  • Add regression coverage for weak token fields, strong token fields, credential URL fixtures, and private IPv4 fixtures.

Test Plan

  • /opt/homebrew/bin/go test ./internal/qualitygate/publiccontent ./internal/qualitygate/rules ./internal/qualitygate/cmd/quality-gate -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

Related Issues

  • None

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection for credential-like placeholders, reducing incorrect secret matches across different file contexts (including docs and fixtures).
    • Reduced false positives for credential URL findings and private IPv4 warnings by tightening fixture/source allowlisting.
    • Enhanced token heuristics with stronger vs weak token key/value classification to better avoid over-flagging.
  • Tests
    • Expanded scanner coverage for fixture allowlists (credential URL and private IPv4) and updated expected finding counts.
    • Added new assertions for token-heuristic scenarios and replaced one test to verify non-fixture resource token values aren’t flagged.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 2, 2026
@coderabbitai

coderabbitai Bot commented Jul 2, 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: 9b7e0f9b-6e73-4785-8a2b-7088e4043291

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4f0ed and 8f79c5e.

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

📝 Walkthrough

Walkthrough

This PR extends public-content credential scanning with entropy-based weak token classification and file-aware placeholder handling, while broadening fixture/testdata allowances for credential URL and private IPv4 warnings. Tests were added and updated to cover the new behavior.

Changes

Credential/token detection refinement

Layer / File(s) Summary
Weak/strong token key and entropy heuristics
internal/qualitygate/publiccontent/scan.go
Adds entropy-based helpers and expands token-key classification used to judge whether token-like values look credential-like.
Credential assignment and placeholder gating
internal/qualitygate/publiccontent/scan.go
Rejects weak-token assignments without credential-like values and excludes auth credential token keys from one placeholder path.
File-context-aware URL and IPv4 placeholder handling
internal/qualitygate/publiccontent/scan.go
Passes file context into credential URL placeholder checks and broadens private IPv4 suppression to source and fixture/testdata paths.
Test coverage for new heuristics
internal/qualitygate/publiccontent/scan_test.go
Adds and updates scanner tests for fixture allowances, weak/strong token cases, placeholder URL handling, and private IPv4 suppression.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • larksuite/cli#1582: Both PRs modify internal/qualitygate/publiccontent token and placeholder classification logic in the same scanner path.
  • larksuite/cli#1700: Both PRs refine public-content scanner heuristics and corresponding regression tests in the same module.

Suggested labels: size/L

Suggested reviewers: liangshuo-1, Roy-oss1

🚥 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 token false positives.
Description check ✅ Passed The description follows the required template and covers summary, changes, test plan, and related issues.
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-resource-token-placeholders

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 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.26214% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.50%. Comparing base (d0cde9a) to head (8f79c5e).

Files with missing lines Patch % Lines
internal/qualitygate/publiccontent/scan.go 91.26% 8 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1718      +/-   ##
==========================================
- Coverage   74.51%   74.50%   -0.01%     
==========================================
  Files         852      852              
  Lines       87655    87755     +100     
==========================================
+ Hits        65312    65379      +67     
- Misses      17315    17347      +32     
- Partials     5028     5029       +1     

☔ 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 2, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@8f79c5eed32cb575526b0990df994a3d80cc093c

🧩 Skill update

npx skills add larksuite/cli#fix/public-content-resource-token-placeholders -y -g

@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: 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 996-1000: The sourceOrTestFixtureFile helper only recognizes
"/fixtures/" and "/testdata/" paths, so root-level fixtures can slip through and
trigger false positives. Update sourceOrTestFixtureFile in scan.go to also treat
files that start with fixtures/ or testdata/ as fixtures, alongside the existing
sourceCodeFile checks and path containment logic. Use the existing
sourceOrTestFixtureFile function and filepath.ToSlash handling so all fixture
locations are covered consistently.
🪄 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: 0a45caaa-f5eb-4d15-be23-d1ba158919dd

📥 Commits

Reviewing files that changed from the base of the PR and between d0cde9a and e5f180b.

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

Comment thread internal/qualitygate/publiccontent/scan.go
@HanShaoshuai-k
HanShaoshuai-k force-pushed the fix/public-content-resource-token-placeholders branch from 2a4f0ed to 8f79c5e Compare July 2, 2026 08:11
@HanShaoshuai-k
HanShaoshuai-k merged commit 440867f into main Jul 2, 2026
46 of 48 checks passed
@HanShaoshuai-k
HanShaoshuai-k deleted the fix/public-content-resource-token-placeholders branch July 2, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants