Skip to content

Check for dependency automation configuration files in App Doctor - #8556

Merged
jplhomer merged 4 commits into
mainfrom
joshlarson/app-doctor-dependency-auditing
Sep 17, 2026
Merged

jplhomer merged 4 commits into
mainfrom
joshlarson/app-doctor-dependency-auditing

Conversation

@jplhomer

@jplhomer jplhomer commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

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.json are 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?

pnpm shopify app doctor --path /path/to/app --skip-instructions
pnpm shopify app doctor --path /path/to/app --skip-instructions --json --blocking low

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type and added a changeset

@jplhomer
jplhomer added this pull request to stack #8557 September 15, 2026 17:26
@github-actions github-actions Bot added the Area: @shopify/app @shopify/app package issues label Sep 15, 2026
@jplhomer
jplhomer force-pushed the joshlarson/app-doctor-dependency-auditing branch from 2812527 to bd2843f Compare September 15, 2026 18:24
@jplhomer jplhomer changed the title Add dependency-auditing configuration check to App Doctor Add GitHub Actions dependency-auditing check to App Doctor Sep 15, 2026
@jplhomer
jplhomer force-pushed the joshlarson/app-doctor-dependency-auditing branch from bd2843f to 0ce1bc2 Compare September 15, 2026 19:02
@jplhomer jplhomer changed the title Add GitHub Actions dependency-auditing check to App Doctor Check for dependency automation configuration files in App Doctor Sep 15, 2026
@jplhomer

Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions

Copy link
Copy Markdown
Contributor

🫰✨ 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-20260915202920

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@jplhomer
jplhomer marked this pull request as ready for review September 16, 2026 14:34
@jplhomer
jplhomer requested a review from a team as a code owner September 16, 2026 14:34
Copilot AI lite review requested due to automatic review settings September 16, 2026 14:34
@jplhomer
jplhomer requested a review from jek September 16, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

  • filesystemError interpolates both its path argument and raw Node error details. The new callers pass absolute app-root or .git paths, 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.

Comment thread packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts Outdated
Comment thread packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts Outdated
Comment thread packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts Outdated
Comment thread packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts Outdated
Base automatically changed from joshlarson/remove-app-doctor-cve-audit to main September 16, 2026 17:49
jplhomer and others added 2 commits September 16, 2026 12:49
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
@jplhomer
jplhomer force-pushed the joshlarson/app-doctor-dependency-auditing branch from 068741d to 511403e Compare September 16, 2026 17:49
Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
@jplhomer
jplhomer requested a review from dmerand September 16, 2026 18:28

@dmerand dmerand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 jek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested locally. Good capability to add.

@nickwesselman

Copy link
Copy Markdown
Contributor

/snapit

@github-actions

Copy link
Copy Markdown
Contributor

🫰✨ 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-20260917000121

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

Co-authored-by: AI (Pi/GPT-6 Astra) <noreply@pi.dev>
@jplhomer
jplhomer requested a review from dmerand September 17, 2026 14:09
@dmerand

dmerand commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

/snapit

@dmerand dmerand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes! (I got tired of waiting for /snapit and just tested against the branch)

@jplhomer
jplhomer added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit 701d9b7 Sep 17, 2026
30 checks passed
@jplhomer
jplhomer deleted the joshlarson/app-doctor-dependency-auditing branch September 17, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: @shopify/app @shopify/app package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants