Skip to content

Commit d13d7f5

Browse files
authored
Merge pull request #8502 from francisbeaudoin/fix-static-asset-script-src
Allow static theme asset script URLs in app doctor
2 parents 338fb70 + 034eedd commit d13d7f5

3 files changed

Lines changed: 23 additions & 1 deletion

File tree

‎.changeset/quiet-assets-repair.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/app': patch
3+
---
4+
5+
Avoid flagging static theme asset script tags in app doctor.

‎packages/app/src/cli/services/app-doctor-engine/rules/liquid-rules.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ function liquidExecutableContextVisitor(file: SourceFile) {
6666
const context = liquidOutputContext(ancestors)
6767
if (context !== 'javascript' && context !== 'executable_attribute') return undefined
6868
if (context === 'javascript' && filterNames(node).includes('json')) return undefined
69+
if (isStaticScriptAssetUrl(node, ancestors)) return undefined
6970
return [
7071
makeLiquidIssue(
7172
'UNSAFE_INNERHTML',
@@ -118,6 +119,14 @@ function attributeName(attribute: AttributeNode): string {
118119
return attribute.name.map((part) => ('value' in part && typeof part.value === 'string' ? part.value : '')).join('')
119120
}
120121

122+
function isStaticScriptAssetUrl(node: LiquidVariableOutput, ancestors: LiquidHtmlNode[]): boolean {
123+
const script = ancestors.find((ancestor) => ancestor.type === 'HtmlRawNode' && ancestor.name === 'script')
124+
const attribute = ancestors.find((ancestor): ancestor is AttributeNode => ancestor.type.startsWith('Attr'))
125+
if (!script || !attribute || attributeName(attribute).toLowerCase() !== 'src') return false
126+
127+
return /^(['"])(?:\\.|(?!\1)[^\\\n])+\1\s*\|\s*asset_url\s*$/.test(outputSource(node).trim())
128+
}
129+
121130
function outputSource(node: LiquidVariableOutput): string {
122131
return typeof node.markup === 'string' ? node.markup : node.markup.rawSource
123132
}

‎packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1084,8 +1084,16 @@ describe('Liquid public AST analysis', () => {
10841084
])
10851085
expect(ordinary.issues).toEqual([])
10861086

1087+
const staticAsset = scanLiquidSecurity([
1088+
source('<script src="{{ \'chat.js\' | asset_url }}" defer></script>', 'extensions/theme/blocks/chat.liquid'),
1089+
])
1090+
expect(staticAsset.issues).toEqual([])
1091+
10871092
const script = scanLiquidSecurity([
1088-
source('<script src="{{ block.settings.script | escape }}"></script>', 'extensions/theme/blocks/script.liquid'),
1093+
source(
1094+
'<script src="{{ block.settings.script | asset_url }}"></script>',
1095+
'extensions/theme/blocks/script.liquid',
1096+
),
10891097
])
10901098
expect(script.issues.map((finding) => finding.id)).toEqual(['LIQUID_UNSAFE_RENDER', 'UNSAFE_INNERHTML'])
10911099
})

0 commit comments

Comments
 (0)