Guard CLI bundle upload against oversized and out-of-app paths by isaacroldan · Pull Request #7542 · Shopify/cli

isaacroldan · 2026-05-13T13:17:08Z

WHY are these changes introduced?

Follow-up from the Slack discussion in #app-management around uploaded bundle size (thread). Today the CLI has no guard rails on the asset zip we upload to GCS: an asset path pointing outside the app folder (e.g. source = "../../" or an absolute home directory) silently copies whatever it resolves to, and the resulting bundle is then uploaded regardless of size. Josh confirmed a 2GB end-to-end upload, with the byproduct that a failed deploy still leaves a giant zip in .shopify/.

Server-side enforcement (#713307, capping ProcessBundle at 100MB) is the security boundary. This PR adds the CLI-side fail-fast for the accidental-misuse cases so developers don't OOM their own machine or wait through a multi-GB upload before being told no.

WHAT is this pull request doing?

Two guards in the bundle pipeline:

Asset paths must resolve inside the app directory. include_assets entries (configKey, static, pattern) now reject sources that resolve outside the app folder (where shopify.app.toml lives). Sibling-extension paths still work (the boundary is the app, not the extension). Implemented via a small assertPathWithinAppDir helper plus a threaded appDirectory param through the three entry handlers and include-assets-step.ts.
uploadToGCS aborts when the compressed bundle exceeds 100MB. Single chokepoint covers both deploy (services/deploy/upload.ts) and dev session (services/dev/processes/dev-session/dev-session.ts). The cap matches the new server-side check so the CLI and server agree on the limit. Error message points the developer at their asset config.

Out of scope on purpose: file-count limits, streaming the upload, and any opt-in GCS-side header — those were debated in the thread but defer cleanly.

How to test your changes?

Unit tests cover both guards:

packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts
packages/app/src/cli/services/build/steps/include-assets/*.test.ts (existing tests adjusted; behaviour preserved when paths stay inside the app dir)
packages/app/src/cli/services/bundle.test.ts (new uploadToGCS describe block — happy path + 101MB rejection)

Manual repro of the path bound: in any app, set an asset entry to source = "../../" or tools = "/Users/me/some/dir" and run shopify app build/deploy — you should get a clear Asset path '...' resolves outside the app directory error instead of a silent home-directory copy.

Manual repro of the size guard: temporarily lower MAX_BUNDLE_SIZE_MB to 1 in bundle.ts, then run deploy — the CLI should abort before the GCS PUT.

Post-release steps

None.

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows) — path check uses relativePath from cli-kit which normalises both POSIX and Windows separators
I've considered possible documentation changes — no public flag/config surface changed; the only new behaviour is a fail-fast error that didn't have docs before
I've considered analytics changes to measure impact — not adding metrics; if we want a counter for "rejected oversized bundle" we can add it in a follow-up
Added a changeset (patch bump for @shopify/app)

isaacroldan · 2026-05-13T13:17:23Z

Guard CLI bundle upload against oversized and out-of-app paths #7542 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

JoshuaWhite1

Tophatted:

- Dev/Deploy for valid assets path succeeds (note: bundle upload log)

- Escaping the App directory correctly errors (no bundle upload)

- Assets path that exceeds 100MB correctly errors (no bundle upload)

Side note:
"", ./ and ../ will still upload the entire app/extension contents. Out of the scope of this PR but related. PR for empty is in the works #7530 and I'll do a follow up PR restricting the other two cases tomorrow.

Copilot

Pull request overview

This PR adds client-side guardrails to the app bundle pipeline to prevent accidental inclusion of out-of-app files and to fail fast before attempting oversized bundle uploads, aligning behavior with the new server-side 100MB enforcement.

Changes:

Enforce that include_assets sources (static, configKey, pattern baseDir) resolve within the app directory via a shared assertPathWithinAppDir helper.
Abort uploadToGCS when the bundle file exceeds 100MB before reading/uploading it, and add unit tests for the size guard.
Thread appDirectory through include-assets step handlers and update related tests.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/app/src/cli/services/dev/extension/server/middlewares.test.ts	Updates middleware test setup to pass `appDirectory`.
packages/app/src/cli/services/bundle.ts	Adds bundle size constants and an early size check in `uploadToGCS`.
packages/app/src/cli/services/bundle.test.ts	Adds `uploadToGCS` tests for under-limit upload and over-limit rejection.
packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts	Adds app-directory boundary assertion for static source entries.
packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.test.ts	Updates tests to pass `appDirectory`.
packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts	Adds app-directory boundary assertion for configKey-resolved paths.
packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts	Updates tests to pass `appDirectory`.
packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts	Adds app-directory boundary assertion for pattern `sourceDir`.
packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts	Updates tests to pass new config values used for error reporting.
packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts	Introduces shared helper to reject out-of-app resolved paths.
packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts	Adds unit coverage for the new helper.
packages/app/src/cli/services/build/steps/include-assets-step.ts	Threads `appDirectory` through include-assets dispatch to entry handlers.
packages/app/src/cli/services/build/steps/include-assets-step.test.ts	Fixes test context to provide `options.app.directory`.
.changeset/guard-bundle-upload-size-and-paths.md	Patch changeset for `@shopify/app`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vividviolet

Looks great! Just some minor suggestions

- Check `..` as a path segment (not prefix) in assertPathWithinAppDir so a sibling named `..cache` isn't false-rejected. - Round up the displayed size in the bundle limit error via Math.ceil so a size just over the cap can't display as the cap. - Mock fileSize in the over-limit test to avoid allocating ~101MB in CI memory. - Move the app-directory boundary check before glob in copy-by-pattern so an out-of-app sourceDir fails fast; preserve missing-dir behavior via fileExists short-circuit.

The new fileExists short-circuit in copyByPattern broke the auto-mocked pattern tests at the step level (fileExists returns undefined → early return). Add a beforeEach default for the pattern-entries block, and switch the pattern-only manifest test from a flat false to an implementation that returns true for the sourceDir path.

Guard CLI bundle upload against oversized and out-of-app paths

926db8b

github-actions Bot added the Area: @shopify/app @shopify/app package issues label May 13, 2026

JoshuaWhite1 approved these changes May 13, 2026

View reviewed changes

isaacroldan marked this pull request as ready for review May 14, 2026 08:26

isaacroldan requested review from a team as code owners May 14, 2026 08:26

Copilot AI review requested due to automatic review settings May 14, 2026 08:26

Copilot started reviewing on behalf of isaacroldan May 14, 2026 08:27 View session

Copilot AI reviewed May 14, 2026

View reviewed changes

Comment thread packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts Outdated

Comment thread packages/app/src/cli/services/bundle.ts

Comment thread packages/app/src/cli/services/bundle.test.ts

Resolve symlinks in asset path guard so symlinked-out dirs are caught

5a9a740

vividviolet approved these changes May 14, 2026

View reviewed changes

Comment thread packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts Outdated

Comment thread packages/app/src/cli/services/bundle.ts

isaacroldan added 2 commits May 14, 2026 16:27

alfonso-noriega approved these changes May 15, 2026

View reviewed changes

isaacroldan added this pull request to the merge queue May 15, 2026

Merged via the queue into main with commit ced0ad4 May 15, 2026
26 of 28 checks passed

isaacroldan deleted the 05-13-cli_bundle_guard_size_and_path branch May 15, 2026 10:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guard CLI bundle upload against oversized and out-of-app paths#7542

Guard CLI bundle upload against oversized and out-of-app paths#7542
isaacroldan merged 4 commits into
mainfrom
05-13-cli_bundle_guard_size_and_path

isaacroldan commented May 13, 2026 •

edited

Loading

Uh oh!

isaacroldan commented May 13, 2026

Uh oh!

JoshuaWhite1 left a comment •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vividviolet left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

isaacroldan commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Checklist

Uh oh!

isaacroldan commented May 13, 2026

Uh oh!

JoshuaWhite1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vividviolet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

isaacroldan commented May 13, 2026 •

edited

Loading

JoshuaWhite1 left a comment •

edited

Loading