Skip to content

fix(ci): reduce public content false positives - #1582

Merged
HanShaoshuai-k merged 1 commit into
mainfrom
fix/public-content-placeholder-fps
Jun 25, 2026
Merged

fix(ci): reduce public content false positives#1582
HanShaoshuai-k merged 1 commit into
mainfrom
fix/public-content-placeholder-fps

Conversation

@HanShaoshuai-k

@HanShaoshuai-k HanShaoshuai-k commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reduce false positives in the public-content quality gate for documented placeholders, idempotency tokens, and benign source-code expressions. This keeps real credential-shaped values rejected while allowing common public examples used by skills and tests.

Changes

  • Allow explicit placeholder/idempotency patterns such as ellipsis placeholders, numeric and UUID client_token values, and token-like resource placeholders.
  • Skip benign source-code expressions such as regexp validators, field references, and local argument parsing variables.
  • Avoid treating lark-cli schema ... dotted identifiers as JWT-like tokens.
  • Add positive and negative regression coverage for the newly allowed patterns, including credential-shaped values that must still be rejected.

Test Plan

  • go test ./internal/qualitygate/publiccontent -count=1
  • go test ./internal/qualitygate/... -count=1
  • QUALITY_GATE_CHANGED_FROM=upstream/main make quality-gate

Related Issues

Summary by CodeRabbit

  • Bug Fixes

    • Improved public content scanning to better distinguish real credentials from benign placeholders, code references, and common test fixture values.
    • Expanded support for additional placeholder formats, including more user/openid-style identifiers and minute-based fixture tokens.
    • Reduced false positives for JWT-like tokens and schema-style dotted identifiers.
  • Tests

    • Added broader coverage for allowed and blocked token/placeholder patterns to verify the updated scanning behavior.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Public Content Screenshot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The public content scanner now recognizes more benign placeholder and fixture token patterns, adds new filters for certain code-like credential expressions and schema-like JWT matches, and extends tests to cover the new acceptance and detection cases.

Changes

Public content scanner updates

Layer / File(s) Summary
Placeholder allowlist expansion
internal/qualitygate/publiccontent/rules.go
Additional placeholder literals and identifiers are accepted, and minute-based resource-token fixture values are recognized.
Scanner filters and token classifiers
internal/qualitygate/publiccontent/scan.go
The scanner skips benign credential expressions and schema dotted JWT-like matches, and adds generalized token-like placeholder classification.
Scanner coverage updates
internal/qualitygate/publiccontent/scan_test.go
Tests add benign and credential-shaped cases for fixture secrets, validators, code expressions, schema identifiers, client tokens, placeholder examples, and minute tokens.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

Hoppity hop, the scanner sings,
🐰 no false alarms on gentle things.
Minute tokens, placeholders too,
Now hop through checks that know what's true.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: reducing public-content false positives in CI.
Description check ✅ Passed The description follows the template and includes summary, changes, test plan, and related issues.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-placeholder-fps

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.

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

@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 394-396: The schema dotted-identifier exemption in
isSchemaDottedIdentifier is too broad because it only checks for “schema ” and
an underscore, which can incorrectly suppress real secrets. Tighten the logic in
isSchemaDottedIdentifier so it only skips matches that are truly dotted schema
identifiers, and make scanText continue flagging JWT-like tokens on
schema-related lines when they are not identifiers. Add a regression test around
isSchemaDottedIdentifier/scanText that includes a real JWT on a schema line to
confirm it is still detected.
🪄 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: 972979e4-b0a8-462c-8483-842d999578aa

📥 Commits

Reviewing files that changed from the base of the PR and between 7346de3 and 9cacaa8.

📒 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
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.74312% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.74%. Comparing base (7346de3) to head (9cacaa8).

Files with missing lines Patch % Lines
internal/qualitygate/publiccontent/scan.go 90.81% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1582      +/-   ##
==========================================
+ Coverage   74.72%   74.74%   +0.02%     
==========================================
  Files         799      799              
  Lines       80274    80380     +106     
==========================================
+ Hits        59983    60084     +101     
- Misses      15846    15849       +3     
- Partials     4445     4447       +2     

☔ 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

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9cacaa83e6aceeb025cb9902e84ac52705f2aaeb

🧩 Skill update

npx skills add larksuite/cli#fix/public-content-placeholder-fps -y -g

@HanShaoshuai-k
HanShaoshuai-k merged commit d69761e into main Jun 25, 2026
39 of 40 checks passed
@HanShaoshuai-k
HanShaoshuai-k deleted the fix/public-content-placeholder-fps branch June 25, 2026 12:23
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.

3 participants