Bump COMMITTED_SECRET to version 4

Lockfiles are now skipped by file name in every scan directory, so the
check's evidence boundary changed. Stored check records and findings
carry the new version, and the agent-check Markdown and embedded.ts keep
the shared check ID consistent.

Co-authored-by: Qlaw <noreply@qlaw.quick.shopify.io>

Author: jplhomer

packages/app/src/cli/services/app-security-engine/checks/COMMITTED_SECRET.md

@@ -1,6 +1,6 @@
 ---
 id: COMMITTED_SECRET
-version: 3
+version: 4
 severity: high
 precedence: union
 ---

packages/app/src/cli/services/app-security-engine/checks/embedded.ts

@@ -7,7 +7,7 @@ export const EMBEDDED_CHECK_SOURCES: ReadonlyArray<string> = [
   "---\nid: ACTIVE_UPLOADS_AND_PRIVILEGED_PREVIEWS\nversion: 1\nseverity: high\n---\n\n# Active Uploads And Privileged Previews\n\nFind cases where merchant-, customer-, webhook-, or external-service-supplied\nfiles become active content in a privileged origin. Trace uploads, imports,\npreviews, and generated assets from ingestion through storage and final render.\n\nThe risk is not the upload alone. The risk is an untrusted-upload-to-active-render\npath: SVG, HTML, XML, PDF, blob/data URL, or another active format is accepted and\nlater rendered in a storefront, embedded admin, customer-account, theme-editor,\nor operator/admin context where it can execute or leak protected data.\n\n## What to look for\n\n1. **Find upload and import entry points.** Search for file uploads, import jobs,\n   webhook attachments, remote fetches, document parsers, blob/data URL handling,\n   and generated preview endpoints.\n\n2. **Trace file metadata and validation.** Check size limits, extension checks,\n   declared MIME type, magic-byte/file-signature verification, filename handling,\n   generated storage names, antivirus/sanitization, and any image/PDF re-encoding.\n\n3. **Inspect storage and serving boundaries.** Determine whether the object is\n   stored on a non-executable origin, served with explicit `Content-Type` and\n   `Content-Disposition`, and prevented from inheriting privileged cookies or\n   browser authority.\n\n4. **Follow every final renderer.** Check storefront/theme renderers, embedded\n   admin previews, customer-account views, email/PDF previews, admin/operator\n   tools, iframe/srcdoc/blob/data URL renderers, and any browser code that inserts\n   the uploaded content into the DOM.\n\n5. **Check sandboxing and isolation.** Verify iframes, preview origins, CSP,\n   download headers, SVG sanitization, PDF handling, and re-encoding before\n   deciding the content is safe.\n\n## What to report\n\nReport a finding only for a complete untrusted-upload-to-active-render path where\nthe uploaded or imported object is actually rendered or served into a privileged\nexecutable context. Show:\n- who controls the uploaded/imported content;\n- which validation or isolation boundary is missing;\n- where the content becomes active or executable;\n- which privileged origin or user is affected; and\n- file/line evidence for both the ingest path and the renderer/serving path.\n\nExample:\n\n```json\n{\n  \"file\": \"app/controllers/previews_controller.rb\",\n  \"line\": 28,\n  \"message\": \"Uploaded SVG is rendered inline in the admin preview without sanitization or origin isolation\",\n  \"evidence\": [\n    { \"file\": \"app/controllers/uploads_controller.rb\", \"line\": 14, \"quote\": \"params[:file]\" },\n    { \"file\": \"app/controllers/previews_controller.rb\", \"line\": 28, \"quote\": \"render inline: blob.download\" }\n  ],\n  \"confidence\": \"high\",\n  \"reasoning\": \"The merchant-controlled SVG is stored without re-encoding and later rendered inline in the embedded admin origin, so script-capable SVG content can execute with merchant authority.\"\n}\n```\n\nDo not report:\n- files that are forced to download and never rendered in an active origin;\n- images/PDFs that are re-encoded or sanitized before serving;\n- isolated preview origins with no privileged cookies, storage, or message bridge;\n- missing deployment details where you cannot establish executable rendering or unsafe serving.\n- permissive content types, inline disposition, or storage/header hygiene issues\n  without a concrete privileged renderer or execution surface.\n",
   "---\nid: APP_PROXY_LIQUID_INJECTION\nversion: 2\nseverity: high\nprecedence: prefer-agent\n---\n\n# App Proxy Liquid Injection\n\nTrace verified app-proxy request values into active response bodies, including Liquid and HTML response types. Report only a request-controlled value that reaches an active response; static templates and inert JSON are not findings.\n",
   "---\nid: APP_PROXY_UNVERIFIED_SIGNATURE\nversion: 2\nseverity: high\n---\n\nFind app proxy endpoints that read proxy parameters without verifying\nthe Shopify signature, allowing an attacker to impersonate Shopify and\nsend fake proxy requests.\n\nApp proxies let an app serve content directly on the merchant's store\nvia a URL like `https://shop.example.com/apps/my-app/proxy`. Shopify\nsigns every proxy request with an HMAC using the app's shared secret.\nIf the app doesn't verify this signature, anyone can send requests to\nthe proxy endpoint with forged parameters — including `shop`,\n`logged_in_customer_id`, and `path_prefix`.\n\n## What to look for\n\n1. **Find app proxy route handlers.** These are endpoints configured as\n   app proxies in `shopify.app.toml` under `[app_proxy]` or in the app's\n   routing config. They typically read parameters like:\n   - `shop` or `shop_id`\n   - `logged_in_customer_id`\n   - `path_prefix`\n   - `signature`\n   - `timestamp`\n\n2. **Check for signature verification.** The handler must verify the\n   HMAC signature before trusting any proxy parameter. Look for:\n   - **Remix:** `authenticate.public.appProxy(request)` — the official\n     verification function\n   - **Rails:** `verified_request?` or manual HMAC verification using\n     `ShopifyApp` utilities\n   - **Express:** Manual HMAC verification using the app secret\n   - **PHP:** `ShopifyUtils::verifyProxyRequest()` or equivalent\n\n3. **If no verification is present, check whether the handler:**\n   - Reads `shop` from the query string and uses it to scope data\n   - Reads `logged_in_customer_id` and uses it for authorisation\n   - Returns any shop-specific data\n\n   If any of these are true and there's no signature check, it's a real\n   finding.\n\n4. **Check for the HMAC pattern even if the function name isn't obvious.**\n   Some apps implement custom verification:\n   - `crypto.createHmac('sha256', API_SECRET)`\n   - `OpenSSL::HMAC.digest`\n   - `hash_hmac('sha256', ...)`\n   - Comparison with `timingSafeEqual` or `secure_compare`\n\n5. **Separate app-local findings from protocol hardening signals.** Missing\n   verification is a finding when the handler trusts signed parameters without\n   any verification boundary. Weak comparison, unusual canonicalization, or\n   delimiterless concatenation is not automatically an app finding: keep it\n   unresolved unless you can show a usable victim-signed request path or another\n   concrete exploit condition in this app.\n\n## What to report\n\nFor each proxy handler that reads shop/customer parameters without\nsignature verification, or where you can demonstrate a usable victim-signed\nrequest path through a weak verification implementation:\n\n```json\n{\n  \"file\": \"app/routes/proxy.ts\",\n  \"line\": 15,\n  \"message\": \"App proxy handler reads shop parameter without signature verification\",\n  \"snippet\": \"const shop = url.searchParams.get('shop')\",\n  \"evidence\": [\n    {\n      \"file\": \"app/routes/proxy.ts\",\n      \"line\": 15,\n      \"quote\": \"const shop = url.searchParams.get('shop')\"\n    },\n    {\n      \"file\": \"app/routes/proxy.ts\",\n      \"line\": 1,\n      \"quote\": \"no authenticate.public.appProxy or HMAC verification found\"\n    }\n  ],\n  \"confidence\": \"high\",\n  \"reasoning\": \"The handler reads the shop parameter from the query string and uses it to query shop data, but no signature verification is present. An attacker can send requests with any shop parameter.\"\n}\n```\n\nDo not report:\n\n- Handlers that call `authenticate.public.appProxy(request)` (Remix)\n- Handlers with manual HMAC verification\n- Handlers that return only static content (no shop-specific data)\n- Protocol-only canonicalization concerns with no demonstrated app-local exploit path\n- Test handlers\n",
-  "---\nid: COMMITTED_SECRET\nversion: 3\nseverity: high\nprecedence: union\n---\n\n# Committed Secret\n\nInspect files skipped by deterministic secret scanning for committed credentials. Never quote or reproduce a secret; cite only the file and redacted credential kind, and recommend rotation.\n\nDo not report placeholders, public client identifiers (`SHOPIFY_API_KEY`, Stripe `pk_`), or files git confirms are untracked and ignored. Template env files (`.env.example`, `.sample`, `.template`, `.dist`) are findings only when they contain a known credential format.\n",
+  "---\nid: COMMITTED_SECRET\nversion: 4\nseverity: high\nprecedence: union\n---\n\n# Committed Secret\n\nInspect files skipped by deterministic secret scanning for committed credentials. Never quote or reproduce a secret; cite only the file and redacted credential kind, and recommend rotation.\n\nDo not report placeholders, public client identifiers (`SHOPIFY_API_KEY`, Stripe `pk_`), or files git confirms are untracked and ignored. Template env files (`.env.example`, `.sample`, `.template`, `.dist`) are findings only when they contain a known credential format.\n",
   "---\nid: CREDENTIAL_BROWSER_LEAKAGE\nversion: 1\nseverity: high\nprecedence: prefer-agent\n---\n\n# Credential Browser Leakage\n\nTrace credentials, access tokens, session tokens, and client secrets into loader/HTTP responses, browser globals, DOM values, client bundles, or external requests. Do not report server-only use or safe boolean/redacted/hash-derived values.\n",
   "---\nid: CREDENTIAL_LOG_LEAKAGE\nversion: 1\nseverity: high\nprecedence: prefer-agent\n---\n\n# Credential Log Leakage\n\nTrace credentials, access tokens, session tokens, and client secrets through aliases and helpers to console, logger, telemetry, or error-reporting sinks. Do not report boolean presence checks, deliberate redaction, or one-way hashes.\n",
   "---\nid: CROSS_SITE_SCRIPTING\nversion: 1\nseverity: high\n---\n\n# Cross-Site Scripting\n\nFind reflected, stored, and client-side paths where lower-trust data becomes\nexecutable browser content across a trust boundary. Cover app-owned HTML pages,\nserver templates, embedded app views, customer-facing pages, and operator UIs,\nnot only theme extensions. Prove the source, rendering context, victim, and\nreachable execution path; an HTML-looking string or raw-rendering API alone is\nnot a finding.\n\n## What to look for\n\n1. **Map producers and consumers.** Identify the actual web frameworks, template\n   engines, versions, escaping defaults, and rendering helpers. Trace URL/query\n   and form/JSON input, stored customer/merchant content, product/metafield data,\n   imports, webhook fields, and third-party API responses to their renderers.\n   For client-side flows, include location fragments, storage, DOM attributes,\n   and `postMessage` data after examining sender/origin validation. A database\n   read, authenticated route, or Shopify API response does not by itself make\n   the contained user-authored data trusted.\n\n2. **Inspect server-rendered escape hatches.** Follow response builders, layouts,\n   partials, and component wrappers into final HTML, including:\n   - Express/React Router HTML responses assembled with strings, and custom\n     server-side rendering or hydration/bootstrap data.\n   - Rails `raw`, `html_safe`, and HTML/inline rendering; EJS `<%- ... %>`,\n     unescaped Handlebars output, Django/Jinja `safe` or disabled autoescaping,\n     and Blade/Twig raw output.\n   - Markdown/rich-text renderers with raw HTML or unsafe link handling.\n   Read the real helper and framework behavior. A bypass API is only a lead;\n   constant HTML and correctly escaped dynamic text are not findings.\n\n3. **Follow browser-side rendering and execution.** Inspect DOM HTML writes,\n   React `dangerouslySetInnerHTML`, Vue `v-html`, Svelte `{@html}`, Lit\n   `unsafeHTML`, Angular trust-bypass APIs, and wrappers around these sinks.\n   Also inspect dynamic script URLs, event handlers, `srcdoc`, and strings\n   passed to browser `eval`, `Function`, or timers. Follow stored content from\n   its original write to later preview, support, or admin views; do not stop\n   at a safe first renderer. Coordinate these paths with the existing checks\n   listed below instead of creating duplicate findings.\n\n4. **Evaluate the exact output context.** Determine whether input lands in HTML\n   text, a quoted/unquoted attribute, a URL, JavaScript data/code, CSS, or a nested\n   context. HTML text escaping is not sufficient for an event handler or a\n   script/URL context. URL encoding a parameter is not scheme validation for an\n   entire `href` or `src`. For JSON embedded inside an HTML `<script>` element,\n   including non-executable JSON data blocks, verify that serialization prevents\n   an HTML `</script>` breakout; `JSON.stringify` alone does not do this. Do not\n   invent the same breakout for JSON served as an inert `application/json`\n   response. Trace any later consumer that reparses it as HTML or code.\n\n5. **Verify defenses at the final sink.** Follow escaping, HTML sanitization,\n   URL allowlists, framework autoescaping, Trusted Types policies, and any\n   decoding or mutations after sanitization. Check actual configuration and\n   context, not just a sanitizer's name. A custom sanitizer is not automatically\n   vulnerable, and a library call is not automatically safe in every context.\n   To report a bypass, explain the concrete construct that survives the defense\n   and can execute in that renderer. Account for enforced CSP and sandboxing;\n   do not assume a bypass. Missing CSP alone is not XSS, and HttpOnly cookies\n   do not prevent script from acting with the victim's browser authority.\n\n6. **Establish a victim and authority boundary.** Identify who can supply the\n   input, how another principal encounters it, the document's actual origin,\n   and the actions or data script could reach there. An embedded app iframe\n   does not inherit the Shopify Admin parent's origin or authority. Content\n   executing in an isolated or sandboxed origin must not be described as\n   executing in the parent without evidence. Intentional author-controlled\n   HTML, self-XSS requiring the victim to paste code, and a merchant editing\n   their own permitted storefront code are not automatically privilege\n   escalation. Show a lower-trust writer reaching a more privileged reader,\n   another user/tenant, or a surface where executable content is not authorized.\n\n## Coordinate with existing checks\n\nFollow the complete path even when it crosses surfaces, but report the same\nsource-to-sink vulnerability only once, under the most specific owning check:\n\n- `UNSAFE_INNERHTML`: DOM HTML writes and browser code-evaluation sinks.\n- `THEME_EXTENSION_XSS` / `LIQUID_UNSAFE_RENDER`: theme-extension Liquid output.\n- `TEXT_SETTING_HTML_SMUGGLING`: merchant text settings becoming active content.\n- `APP_PROXY_LIQUID_INJECTION`: values from verified app-proxy requests reaching\n  active responses.\n- `SCRIPT_TAG_URL_INJECTION`: Shopify ScriptTag source URLs.\n- `ACTIVE_UPLOADS_AND_PRIVILEGED_PREVIEWS`: uploaded/imported active files and\n  their privileged previews.\n\nDelegate only when the specialized check covers the complete source-to-sink\npath, not merely the response surface, and use that check's own provenance.\nUse `CROSS_SITE_SCRIPTING` for remaining paths, such as reflected server HTML,\nstored customer content in an operator template, unsafe hydration data, or\nactive URL/attribute output outside those specialized paths. Stored buyer\nreviews rendered in app-proxy HTML/Liquid responses remain here when the\ncontent comes from a separate submission endpoint rather than the verified\nproxy request. Do not suppress a distinct vulnerable sink just because the\nsame input is used elsewhere.\n\n## What to report\n\nUse the finding and execution schemas in agent checks, including this check's\nID and version. Each finding must include:\n\n- The controlling principal, entry point, victim interaction, and required access.\n- File/line evidence for input or persistence, transformations, render call,\n  and final sink/template, including any ineffective defense.\n- The exact parser context and a minimal inert marker/example showing how data\n  becomes executable content. Explain the execution mechanism; do not assume\n  a `<script>` inserted via `innerHTML` executes like a parser-inserted script.\n- The affected origin and concrete authority exposed, without assuming access\n  to parent frames, HttpOnly cookies, or unrelated tenants.\n- A context-appropriate fix: preserve autoescaping, render as text, serialize\n  safely for HTML embedding, validate active URLs, or sanitize permitted HTML.\n\nDo not probe live stores, exfiltrate data, or persist active payloads in real\nmerchant content. Code-level evidence can establish the path. Do not report\nordinary autoescaped interpolation, text-node writes in non-executable elements,\nconstant markup, or adequately sanitized content. HTML injection without an\nexecutable path, server-side template execution, and unsafe redirects are not\nby themselves proof of XSS. Email/PDF output is not browser script execution\nwithout evidence of an affected active renderer.\n\nIf a missing producer, renderer, sanitizer implementation, or execution context\nprevents a conclusion, record an unresolved check with a reason code and\nmessage. Do not turn incomplete tracing into either a finding or a pass.\n\n## Optional reference\n\n[OWASP Cross Site Scripting Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html)\n\nWhen available, consult this reference for defensive techniques and remediation\nexamples. It supplements the evidence and reporting requirements above; a\ndeviation from its recommendations is not by itself a finding. If the reference\nis unavailable, continue using this prompt and repository evidence.\n",

packages/app/src/cli/services/app-security-engine/scanners/index.ts

@@ -145,7 +145,7 @@ const DETERMINISTIC_CHECK_DEFINITIONS: ReadonlyArray<DeterministicCheckDefinitio
   configRule(insecureWebhookUrl, 2),
   {
     id: 'COMMITTED_SECRET',
-    version: 3,
+    version: 4,
     lifecycle: 'active',
     analysisMode: 'regex',
     target: 'secrets',

packages/app/src/cli/services/app-security-engine/tests/deterministic-rules.test.ts

@@ -50,7 +50,7 @@ describe('deterministic rules product contract', () => {
     expect(DETERMINISTIC_CHECKS.get('REQUEST_CONTROLLED_ADMIN_CONTEXT')?.version).toBe(3)
     expect(DETERMINISTIC_CHECKS.get('APP_PROXY_LIQUID_INJECTION')?.version).toBe(2)
     expect(DETERMINISTIC_CHECKS.get('INSECURE_WEBHOOK_URL')?.version).toBe(2)
-    expect(DETERMINISTIC_CHECKS.get('COMMITTED_SECRET')?.version).toBe(3)
+    expect(DETERMINISTIC_CHECKS.get('COMMITTED_SECRET')?.version).toBe(4)
     expect(DETERMINISTIC_CHECKS.get('UNAUTHENTICATED_ENDPOINT')?.version).toBe(2)
   })
 

packages/app/src/cli/services/app-security-engine/tests/scan-directories.test.ts

@@ -412,7 +412,7 @@ describe('lockfiles in a monorepo', () => {
     const result = await scanAll({appDirectory: join(monorepo, 'apps', 'foo'), scanDirectories: [monorepo]})
 
     expect(result.scan.files_skipped_count).toBe(0)
-    expect(secretCheck(result)).toMatchObject({status: 'executed'})
+    expect(secretCheck(result)).toMatchObject({status: 'executed', version: 4})
   })
 
   test('skips the lockfile of an include directory outside the app directory', async () => {
@@ -442,6 +442,7 @@ describe('lockfiles in a monorepo', () => {
 
     const issue = result.issues.find((candidate) => candidate.id === 'COMMITTED_SECRET')
     expect(issue?.location.file).toBe('../../packages/server/config.json')
+    expect(issue?.rule_version).toBe(4)
     expect(secretCheck(result)).toMatchObject({status: 'executed'})
   })
 })