Repository navigation
Conversation
Add a shared detector that finds the origins a theme app extension loads from or sends requests to, and compares them with `[extensions.capabilities.network_access] allowed_sources`. - The theme build step prints a non-blocking warning for each undeclared origin, or one info line when the extension doesn't declare allowed_sources. - A new deterministic app security check, THEME_EXTENSION_UNDECLARED_EGRESS (version 1, medium), reports the same origins. Navigation targets (links, form actions, redirects) are classified but not reported.
jplhomer
added this pull request to stack #9005
October 26, 2026 00:33
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Prototype, stacked on #9003. The base branch is #9003's branch, so this diff shows only the new check. Draft for discussion. It warns and reports, and blocks nothing.
WHY are these changes introduced?
#9003 lets theme app extensions declare the origins they contact in
[extensions.capabilities.network_access] allowed_sources. Nothing tells developers when their extension contacts an origin they didn't declare, or that they haven't declared anything at all. Undeclared origins may be reported now and blocked on storefronts later, so it's worth surfacing them while that's cheap to fix.WHAT is this pull request doing?
One shared detector:
packages/app/src/cli/utilities/extensions/theme-egress.ts. It finds the origins an extension's files contact, classifies each sink by kind, and matches origins with #9003'snormalizeAllowedSource. Two surfaces use it:allowed_sourcesis declared, it prints one warning per undeclared origin withfile:line, then one hint. A single line summarizes URLs built at runtime. When the table isn't declared, it prints one info line with the number of external origins, so existing extensions aren't flooded with warnings. The step is wrapped intry/catchand logs failures only at debug level, so a detector bug can't fail a build or deploy.THEME_EXTENSION_UNDECLARED_EGRESS(deterministic, version 1, medium, theme app extensions only). With the table declared, there's one finding per undeclared origin at the line where it's used, plus a fix. Without the table, there's one finding per extension on itsshopify.extension.toml, listing the origins. A Liquid parser failure marks the checkunresolved, the same asLIQUID_UNSAFE_RENDER. It also has achecks/THEME_EXTENSION_UNDECLARED_EGRESS.mdagent prompt (precedence: union), which covers what the deterministic rule can't resolve, a regeneratedembedded.ts, and catalog and registry entries.Sink kinds. Only
egressis checked and reported.navigationsinks are collected and returned but reported nowhere yet. They'd fit a later check against app-level navigation hosts.<script src>,<link href>(stylesheet, preconnect, prefetch, preload, modulepreload, dns-prefetch, icon, manifest),<iframe src>,<img src>,<source src>, thescript_tagandstylesheet_tagfilters, and inline<script>and<style>bodies. JS assets:fetch(),XMLHttpRequest#open,new WebSocket(),new EventSource(),navigator.sendBeacon(), dynamicimport(),new URL()(literal absolute URLs only). CSS assets:url(),@import.<a href>,<form action>,location.href =,location.assign(),location.replace(),window.open()Each finding is
{file, line, column, kind: 'egress' | 'navigation', status: 'undeclared' | 'dynamic', sink, origin?, insecure?}.Matching uses the same rules as #9003's validation. An origin is the scheme, host and port. Hosts compare case-insensitively, and the default
:443is dropped. A leading*.matches one or more subdomain labels but not the bare domain, the same as a CSP host source.wss:matches likehttps:, and the warning suggests thehttps://entry.http:andws:are always reported, becauseallowed_sourcesis HTTPS only.Ignored: relative URLs, non-network schemes (
data:,blob:,mailto:and others), Liquid URL filters (asset_url,shopify_asset_url,image_url,file_urland the like), the shop's own URLs (routes.*,shop.*,request.*in Liquid;Shopify.routesandlocationin JS), and Shopify-owned hosts:cdn.shopify.com,shopify.comand*.myshopify.com.Dynamic URLs (template interpolation, concatenation, variables, Liquid output) get
status: 'dynamic'. They're never reported as undeclared. A literal origin followed by a dynamic path (https://api.example.com/${id}) still counts as that origin.Known blind spots:
UNSAFE_INNERHTMLalso uses regexes. Regex literals and strings that look like code can confuse it.srcset,<video>/<audio>/<embed>/<object>,styleattributes,Worker/importScripts,@font-facevia JS, and requests made inside bundled third-party libraries through computed URLs..cssinside theme extension directories. A small change todiscover.tsadds CSS for theme extensions only, so the language inventory and other checks don't change. Like the rest of the engine, it recognizes theme extensions in the flat TOML format (type = "theme"at the top level).Phase 2. Once storefronts enforce
allowed_sources, blocking deploys belongs in the theme specification'spreDeployValidation, using the same detector. There's a comment there. This PR adds no blocking errors.Sample output
shopify app buildon a fixture app with a theme extension that declaresallowed_sources = ["https://api.example.com"]. TheRemoteAssetwarnings come from the existing theme check.The same extension without the table:
In both cases the build succeeds. In the first case,
shopify app security checkreports the same four origins asTHEME_EXTENSION_UNDECLARED_EGRESSfindings with file, line and column. The<a href>in the block isn't reported.How to test your changes?
[capabilities.network_access] allowed_sources = ["https://api.example.com"]and a block containing<script src="https://other.example.com/x.js"></script>.shopify app build. You should see one warning forhttps://other.example.com, and the build still succeeds.shopify app security checkand look forTHEME_EXTENSION_UNDECLARED_EGRESS.Tests (
packages/app):src/cli/utilities/extensions/theme-egress.test.ts(detector, 44 tests): each sink kind, line and column, declared vs. undeclared, wildcard edge cases (*.cdn.example.commatchesa.cdn.example.comanda.b.cdn.example.combut notcdn.example.comorevilcdn.example.com),:443and other ports,http/ws,wss, Shopify-owned hosts, relative URLs, Liquid URL filters, dynamic URLs, comments, parser failures, and navigation classified separately.src/cli/services/build/steps/build-theme-step.test.ts(5 tests): warnings when declared, one info line when not declared, nothing when everything is declared, navigation not reported, and no failure when the detector throws.src/cli/services/app-security-engine/tests/theme-egress.test.ts(5 tests, scanning real temporary apps): findings when declared, one finding when not declared, no findings when declared,unresolvedon a parser failure, andnot_applicablewithout a theme extension. The registry, catalog, checks and deterministic-rules tests are updated for the new ID.vitest runonapp-security-engine,utilities/extensions,services/build,models/extensions/specifications,extension-instance,app/loader,dev/extension/payloadandservices/context: 77 files and 1376 tests. 1375 pass. The other one, inui_extension.test.ts, timed out under load and passes when rerun alone (60/60).pnpm tsc --noEmit, eslint andprettier --checkon the changed files,embed-checks.mjs --checkandpnpm knip: all clean.Open questions
cdn.shopify.com,shopify.comand*.myshopify.com.Post-release steps
Add the
theme-extension-undeclared-egresspage under the app security checks docs.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add