Conversation
Coverage report
Test suite run success3783 tests passing in 1455 suites. Report generated by 🧪jest coverage report action from 66deccf |
e165a9d to
5b7340f
Compare
cd9fc6e to
4dd73b7
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4a80cd2 to
ae0d089
Compare
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
ae0d089 to
510502e
Compare
510502e to
289a2c6
Compare
289a2c6 to
510502e
Compare
510502e to
f9f6e10
Compare
|
/snapit |
|
🫰✨ Thanks @elanalynn! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260222203314Caution After installing, validate the version by running |
ff3f4e0 to
635fedb
Compare
635fedb to
d296cb0
Compare
| copyStaticAssets: async (config, directory, outputPath) => { | ||
| if (!config.static_root) return | ||
| const sourceDir = joinPath(directory, config.static_root) | ||
| const outputDir = dirname(outputPath) |
There was a problem hiding this comment.
Validate static_root to prevent path traversal/arbitrary file copying
static_root is taken directly from config and joined into sourceDir (joinPath(directory, config.static_root)). If misconfigured or attacker-controlled, values like ../.., absolute paths, or symlinks can escape the app directory and copy arbitrary filesystem contents into the deploy bundle. This is risky even if developer-controlled (CI agents often have adjacent secrets). Recursive copy may amplify impact, especially if symlinks are followed.
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - No issues 📋 History✅ 1 findings → ✅ 1 findings → ✅ No issues |
d296cb0 to
635fedb
Compare
|
|
||
| return copyDirectoryContents(sourceDir, outputDir).catch((error) => { | ||
| throw new Error(`Failed to copy static assets from ${sourceDir} to ${outputDir}: ${error.message}`) | ||
| }) |
There was a problem hiding this comment.
Catch handler assumes error.message exists and can mask root cause
The catch handler assumes error has a .message field. If the underlying promise rejects with a non-Error (string/object), this code can throw a secondary error (e.g., reading message), masking the original failure.
Evidence:
.catch((error) => {
throw new Error(`...: ${error.message}`)
})Impact:
- Failures become harder to debug; incorrect/noisy diagnostics.
635fedb to
66deccf
Compare

WHY are these changes introduced?
Related to Hosted app project.
Related to https://github.com/shop/world/pull/382743
Closes https://github.com/shop/issues-admin-extensibility/issues/2186
WHAT is this pull request doing?
This PR introduces the asset pipeline flow for hosted static apps, enabling static asset copying during the build process.
app_config_hosted_app_home.ts): Adds a configuration extension specification for hosted apps with astatic_rootfieldstatic_appbuild mode: Extends the build config modes to includestatic_app, which triggers static asset copyingHow it works
When a hosted app defines a static_root in its configuration:
hosted_appspecification is loaded with build modestatic_appcopyStaticAssets()is invokedstatic_rootdirectory are copied to the output directoryHow to test your changes?
static_rootpointing to a static assets directoryshopify app deployand verify assets are copied to the output directory -.shopify/deploy-bundleMeasuring impact
How do we know this change was effective? Please choose one:
Checklist