Check for dependency automation configuration files in App Doctor - #8556
Conversation
2812527 to
bd2843f
Compare
bd2843f to
0ce1bc2
Compare
|
/snapit |
|
🫰✨ Thanks @jplhomer! Your snapshot has been published to npm. Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260915202920Caution After installing, validate the version by running |
There was a problem hiding this comment.
🟡 Changes recommended
Three unresolved moderate findings in scanners/discover.ts require safer input provenance and non-sensitive diagnostics.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an App Doctor check for missing local Dependabot or Renovate configuration.
Changes:
- Registers and scores the new dependency-automation rule.
- Adds bounded discovery, hashing, tracing, and submission integration.
- Adds coverage tests and release metadata.
File summaries
| File | Summary |
|---|---|
packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts |
Updates scan context fixtures. |
packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts |
Updates the deterministic rule contract. |
packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts |
Tests integration behavior. |
packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts |
Tests rule behavior. |
packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts |
Tests discovery safety boundaries. |
packages/app/src/cli/services/app-doctor-engine/scanners/types.ts |
Defines dependency-automation input types. |
packages/app/src/cli/services/app-doctor-engine/scanners/index.ts |
Integrates execution, hashing, and scoring. |
packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts |
Implements bounded discovery. Three moderate findings remain: rejected entries bypass skipped-input tracking (2 votes); diagnostics expose absolute repository paths (2 votes); filesystem errors expose paths and raw details (1 vote). |
packages/app/src/cli/services/app-doctor-engine/rules/types.ts |
Extends scan context typing. |
packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts |
Defines configuration paths and finding behavior. |
packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts |
Registers rule metadata and scoring. |
.changeset/add-app-doctor-dependency-automation.md |
Documents the user-facing change. |
Review details
Suppressed comments (1)
packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts:660
filesystemErrorinterpolates both its path argument and raw Node error details. The new callers pass absolute app-root or.gitpaths, and this text becomes an unresolved execution reason; JSON/trace redaction does not remove arbitrary filesystem paths. Return a relative or generic diagnostic instead of exposing these values.
return `Could not inspect ${path}: ${detail}`
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
068741d to
511403e
Compare
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
dmerand
left a comment
There was a problem hiding this comment.
Sorry for the review churn. I found a couple more things on manual testing that are worth consideration. The overall approach looks OK, though I still kind of wonder if there are FS primitives we could be sharing with all of the FS reads happening in the app doctor space.
jek
left a comment
There was a problem hiding this comment.
Tested locally. Good capability to add.
|
/snapit |
|
🫰✨ Thanks @nickwesselman! Your snapshot has been published to npm. Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260917000121Caution After installing, validate the version by running |
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
|
/snapit |
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/23910
Follow-up to #8555. After removing package-manager CVE audits, App Doctor should flag missing dependency-automation configuration without trying to interpret CI workflows or shell commands.
WHAT is this pull request doing?
Adds
MISSING_DEPENDENCY_SECURITY_AUTOMATION, a low-severity check worth -5 points. The presence of a standard Dependabot or Renovate configuration file satisfies the check; contents are not parsed or validated. This is an adoption signal, not proof that automation is enabled, covers the app, or fixes vulnerabilities.Keeps bounded reads, path/symlink containment, input hashing, and normal scoring, blocking, trace, and submission handling. Unsafe or unreadable inputs and nested repository boundaries remain unresolved. Hosted integrations, centrally managed configuration, CI workflows, and Renovate configuration embedded in
package.jsonare not detected.No new dependencies. Replaces the previous workflow analyzer and supersedes the GitLab/CircleCI extensions in #8558 and #8559.
How to manually test your changes?
Checklist