Repository navigation
Exempt inert schema-typed Liquid settings from LIQUID_UNSAFE_RENDER - #8935
Conversation
|
/snapit |
1 similar comment
|
/snapit |
|
🫰✨ Thanks @jplhomer! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261020135348Caution After installing, validate the version by running |
…teral filter args In an app block, section.settings refers to the host section, so it is no longer checked against the block schema. Filter arguments are rendered too, so a filter argument that references a setting or variable (for example default: block.settings.title) now disqualifies the exemption.
|
/snapit |
|
🫰✨ Thanks @jplhomer! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261020141829Caution After installing, validate the version by running |
There was a problem hiding this comment.
🟡 Changes recommended
The exemption guards can suppress valid findings for transformed values, rebound block variables, and omitted URL contexts.
3 open issues
What changed in this PR
Reduces false positives in LIQUID_UNSAFE_RENDER by using app-block schemas to identify inert settings.
Changes:
- Adds schema-based exemptions for selected setting types in HTML contexts.
- Adds regression tests for exemptions and fallback behavior.
- Updates the check to version 2 and refreshes embedded guidance.
| File | Description |
|---|---|
packages/app/src/cli/services/app-security-engine/tests/deterministic-rules.test.ts |
Adds exemption tests and version assertion. |
packages/app/src/cli/services/app-security-engine/scanners/index.ts |
Bumps the deterministic check version. |
packages/app/src/cli/services/app-security-engine/rules/liquid-rules.ts |
Implements schema parsing and exemption guards. |
packages/app/src/cli/services/app-security-engine/checks/LIQUID_UNSAFE_RENDER.md |
Documents version 2 behavior. |
packages/app/src/cli/services/app-security-engine/checks/embedded.ts |
Refreshes embedded check guidance. |
Review effort: Balanced
…filters The v2 exemption could still suppress findings for output that is not inert: - Attribute contexts used a URL denylist that missed object data, srcset, and imagesrcset. Exempt attribute values are now an allowlist (class, id, title, alt, role, width, height, lang, dir, aria-*), and only in quoted values with a fully static attribute name. - Liquid directly in an attribute list, in an attribute or tag name, in an unquoted value, or inside raw-text elements such as style is no longer exempt. - Literal filter arguments did not prove inert output (url_decode, t, script_tag). Only argument-less case and whitespace filters, escape, and default with an inert literal fallback are allowed. - A file that rebinds block (assign, capture, for, tablerow, increment, decrement) loses the exemption, since block.settings may no longer refer to the schema-owning block. Co-authored-by: Qlaw <noreply@qlaw.quick.shopify.io>
Add cases for duplicate schema blocks, duplicate setting ids, empty select options, and the inert radio exemption. Each fails when its guard is removed. Co-authored-by: Qlaw <noreply@qlaw.quick.shopify.io>
…e-render-schema-types # Conflicts: # packages/app/src/cli/services/app-security-engine/tests/deterministic-rules.test.ts
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/24098
LIQUID_UNSAFE_RENDERflagged everyblock.settings.*/section.settings.*output the same way, includingselect/checkboxsettings whose values are fixed by the extension schema (e.g. a layout select withleft|rightoptions, or a boolean toggle used as a class). Those closed, developer-defined value domains cannot carry merchant-injected markup in HTML text or ordinary attribute contexts.WHAT is this pull request doing?
liquid-rules.ts: extract the{% schema %}JSON from the same file, map settingid-> type, and thread it intoliquidUnsafeRenderVisitor. Exempts only exactblock.settings.<id>/section.settings.<id>(no property access) inhtml_text/ non-URLhtml_attributecontexts forcheckbox,range, andselect/radiowhere everyoptions[].valuepasses a conservative inert charset (no<,>,&, quotes, backtick, whitespace, or=). Missing, unparseable, or duplicated schema, unsafe option values, and all other types fall through to previous behaviour (fail-closed).liquidExecutableContextVisitor(UNSAFE_INNERHTML), JavaScript/executable contexts, URL attributes, metafields.LIQUID_UNSAFE_RENDERv1 -> v2 inscanners/index.tsandchecks/LIQUID_UNSAFE_RENDER.md, regenerateschecks/embedded.ts, adds the missing version-pin assertion, and adds the targeted test matrix (inert select clean in text/attribute, bad option flagged, checkbox/range clean, text/richtext flagged, script/onclick flagged, missing/unparseable schema flagged, escaped stays clean).Follow-up (out of scope): update the shopify.dev app-security check page for
LIQUID_UNSAFE_RENDERinshop/worldto describe the typed exemption.How to manually test your changes?
In a theme app extension block, add a
{% schema %}with aselectsetting (inert options), acheckbox, and atextsetting; render each unescaped in element text and in aclassattribute, then run the app-security scan. The select/checkbox outputs should no longer raiseLIQUID_UNSAFE_RENDER, while the text output still should.