Skip to content

[Security] Contain theme file operations within the theme directory - #8600

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
security-maintenance-35547738595
Draft

github-actions[bot] wants to merge 1 commit into
mainfrom
security-maintenance-35547738595

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Theme file keys used by theme pull, theme dev, and reconciliation originate from remote Admin API responses, which are outside the CLI's trust boundary. Those keys were joined onto the theme root without verifying the result stayed inside it, so a key containing parent-directory segments could resolve to a location outside the theme directory. This is a path traversal issue in the theme filesystem layer, affecting local read, write, and delete operations.

WHAT is this pull request doing?

Adds a resolveThemeFilePath helper in packages/theme/src/cli/utilities/theme-fs.ts that resolves a theme file key against the theme root and rejects it with an AbortError when the result falls outside that root. writeThemeFile, readThemeFile, and removeThemeFile now go through it, so the containment check happens at the boundary rather than at each call site.

The check uses the existing isSubpath helper from @shopify/cli-kit/node/path, matching how the extension asset middleware already guards its served paths. Legitimate keys resolve exactly as before, so normal theme operations are unchanged.

A regression test covers the rejected case and confirms no file is written outside the theme root.

How to manually test your changes?

shopify theme pull --store <store> --theme <theme-id>
shopify theme dev --store <store>

Both should sync theme files as before, with files written only inside the theme directory.

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 (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Theme file keys originate from remote API responses, but were joined onto
the theme root without verifying the result stayed inside it. Resolve each
key and reject any that escapes the theme directory before reading,
writing, or removing the file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants