Skip to content

Fix Dev Server to allow serving static assets from a shared folder outside of the extension directory#7386

Open
vividviolet wants to merge 1 commit into04-22-address_png_encoding_bug_path_changesfrom
04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory
Open

Fix Dev Server to allow serving static assets from a shared folder outside of the extension directory#7386
vividviolet wants to merge 1 commit into04-22-address_png_encoding_bug_path_changesfrom
04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory

Conversation

@vividviolet
Copy link
Copy Markdown
Member

@vividviolet vividviolet commented Apr 23, 2026

WHY are these changes introduced?

When two extension points within the same extension declare assets whose source basenames collide (e.g. both reference a tools.json, or both compile to main.js), the dev server previously served whichever file happened to win the filesystem race. The URL shape (/assets/<basename>) gave no way to distinguish which extension point a request belonged to, so one target's asset would silently shadow another's.

Additionally, the middleware served files directly from the extension's source directory, which allowed path traversal to files outside the extension's bundle and made it impossible to serve include_assets-copied files that had been renamed by uniqueBasename during the build step.

WHAT is this pull request doing?

Opaque, target-scoped asset URLs

Asset URLs are now emitted as /assets/<target>/<assetKey>[.ext] instead of /assets/<basename>. This makes every URL unique per extension point, even when two targets share a source file.

Asset resolver

A new AssetResolver (Map<string, string>) is introduced per extension. During payload generation, each emitted URL subpath is registered in the resolver alongside the output-relative filesystem path the middleware should read. The resolver is cleared and rebuilt on every payload regeneration so stale entries from removed targets or renamed assets don't persist.

The resolver is stored in ExtensionsPayloadStore (keyed by devUUID) and exposed via getAssetResolver(devUUID). It is cleaned up when an extension is deleted.

Middleware rewrite

getExtensionAssetMiddleware now:

  1. Looks up the request's assetPath in the extension's resolver to find the actual output-relative filename (falling back to the raw path for direct compiled-artefact fetches).
  2. Resolves the candidate path against the extension's outputDir (the bundle directory that include_assets populates).
  3. Rejects any path — whether from the URL or from a resolver value — that escapes outputDir via traversal, returning 404 rather than 403 to avoid leaking directory structure.

Static directory assets

A new staticAssetsMapper handles directory-valued config entries (e.g. assets = "./assets"). It emits a directory-prefix URL with a trailing slash and registers one resolver entry per copied file, including nested subdirectories.

resolveOutputDir extracted

The logic for deriving an output directory from outputPath (file with extension → dirname; no extension → path itself) is extracted into a shared resolveOutputDir helper used by both the build step and the middleware.

How to test your changes?

  1. Create a UI extension with two extension points that both declare a tools asset pointing at different source files.
  2. Run shopify app dev and observe that each extension point's payload contains a distinct URL (/<target-a>/tools.json vs /<target-b>/tools.json).
  3. Fetch each URL from the dev server and confirm the correct file content is returned for each target.
  4. Verify that requesting a file path not present in the resolver (e.g. a raw ../secret.txt) returns 404.
  5. Create an extension point with assets = "./assets" containing nested files; confirm each file is individually servable via its resolver-mapped URL.

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

Copy link
Copy Markdown
Member Author

vividviolet commented Apr 23, 2026

@vividviolet vividviolet changed the base branch from main to graphite-base/7386 April 24, 2026 03:23
@vividviolet vividviolet force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from 8cac9b8 to 5fa082f Compare April 24, 2026 03:23
@vividviolet vividviolet changed the base branch from graphite-base/7386 to 04-21-add_assets_config_key_support_for_ui_extension_points April 24, 2026 03:24
@vividviolet vividviolet marked this pull request as ready for review April 24, 2026 03:27
@vividviolet vividviolet requested a review from a team as a code owner April 24, 2026 03:28
@vividviolet vividviolet changed the base branch from 04-21-add_assets_config_key_support_for_ui_extension_points to graphite-base/7386 April 24, 2026 21:53
@vividviolet vividviolet force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from 5fa082f to 26c602f Compare April 24, 2026 21:53
@vividviolet vividviolet changed the base branch from graphite-base/7386 to 04-22-address_png_encoding_bug_path_changes April 24, 2026 21:53
@elanalynn elanalynn force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from 26c602f to 5deb0e5 Compare April 24, 2026 23:15
@elanalynn elanalynn force-pushed the 04-22-address_png_encoding_bug_path_changes branch from b6d7170 to 10a3496 Compare April 24, 2026 23:15
@vividviolet vividviolet force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from 5deb0e5 to fa4818d Compare April 27, 2026 13:14
@vividviolet vividviolet force-pushed the 04-22-address_png_encoding_bug_path_changes branch from 10a3496 to 0504b81 Compare April 27, 2026 13:14
@vividviolet vividviolet force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from fa4818d to c81189a Compare April 27, 2026 13:36
@github-actions
Copy link
Copy Markdown
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/cli-launcher.d.ts
@@ -1,8 +1,6 @@
-import type { LazyCommandLoader } from './custom-oclif-loader.js';
 interface Options {
     moduleURL: string;
     argv?: string[];
-    lazyCommandLoader?: LazyCommandLoader;
 }
 /**
  * Launches the CLI.
packages/cli-kit/dist/public/node/cli.d.ts
@@ -1,4 +1,3 @@
-import type { LazyCommandLoader } from './custom-oclif-loader.js';
 /**
  * IMPORTANT NOTE: Imports in this module are dynamic to ensure that "setupEnvironmentVariables" can dynamically
  * set the DEBUG environment variable before the 'debug' package sets up its configuration when modules
@@ -8,8 +7,6 @@ interface RunCLIOptions {
     /** The value of import.meta.url of the CLI executable module */
     moduleURL: string;
     development: boolean;
-    /** Optional lazy command loader for on-demand command loading */
-    lazyCommandLoader?: LazyCommandLoader;
 }
 /**
  * A function that abstracts away setting up the environment and running
@@ -20,7 +17,6 @@ export declare function runCLI(options: RunCLIOptions & {
     runInCreateMode?: boolean;
 }, launchCLI?: (options: {
     moduleURL: string;
-    lazyCommandLoader?: LazyCommandLoader;
 }) => Promise<void>, argv?: string[], env?: NodeJS.ProcessEnv, versions?: NodeJS.ProcessVersions): Promise<void>;
 /**
  * A function for create-x CLIs that automatically runs the "init" command.
@@ -42,5 +38,5 @@ export declare const jsonFlag: {
 /**
  * Clear the CLI cache, used to store some API responses and handle notifications status
  */
-export declare function clearCache(): Promise<void>;
+export declare function clearCache(): void;
 export {};
\ No newline at end of file

@elanalynn elanalynn force-pushed the 04-22-address_png_encoding_bug_path_changes branch from 0504b81 to a81656c Compare April 27, 2026 19:36
@elanalynn elanalynn force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from c81189a to 55dad85 Compare April 27, 2026 19:36
@elanalynn elanalynn force-pushed the 04-22-address_png_encoding_bug_path_changes branch from a81656c to 6e8015e Compare April 27, 2026 20:37
@elanalynn elanalynn force-pushed the 04-23-fix_dev_server_to_allow_serving_static_assets_from_a_shared_folder_outside_of_the_extension_directory branch from 55dad85 to 2cf0f84 Compare April 27, 2026 20:37
@elanalynn elanalynn requested a review from isaacroldan April 27, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant