Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/app-security-unresolved-calibration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/app': patch
'@shopify/cli': patch
---

Stop app security agent checks from leaving the React Router app template's metafield, authorization and frame-ancestors checks unresolved
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ For each check:
5. Keep the check `id` and `version` exactly as they appear in {{AGENT_CHECKS_PATH}}.
6. Include concise evidence citations. Never include a detected secret value or unnecessary personal data.

A check with no verified issue must not produce a fabricated finding. If you cannot establish exploitability or affected authority, record the check as `unresolved` with a reason instead.
A check with no verified issue must not produce a fabricated finding. Choose the status from what you were able to investigate, not from whether you can prove a negative:

- `executed`: you traced the paths the check directs you to and found no concrete issue. This includes code that follows the safe pattern the check or its `docs_url` describes. A candidate is ruled out when the code establishes the boundary the check asks about, even if you can't rule out hypothetical policies or requirements that the repository doesn't define.
- `unresolved`: you couldn't complete the investigation, for example because required code was unreadable, out of scope, or outside the repository and you couldn't inspect it, or you found a specific candidate whose boundary you could neither establish nor show to be missing. Name the candidate's file and line, or the missing input, in the reason.
Comment thread
jplhomer marked this conversation as resolved.

### 4. Write one findings document

Expand Down Expand Up @@ -88,7 +91,7 @@ Write a single JSON document that covers every check you ran:
- `status` is one of:
- `executed`: you investigated the check, whether or not it produced findings.
- `not_applicable`: the capability the check covers is absent. It can't have findings.
- `unresolved`: you couldn't finish the check or prove the issue. An unresolved check didn't pass; never describe it as passing.
- `unresolved`: you couldn't complete the investigation, or you named a specific candidate whose boundary is still unclear. An unresolved check didn't pass; never describe it as passing.
- `not_applicable` and `unresolved` require a `reason` with a short `code` and a `message`.
- Each finding needs `file`, `line` (1 or greater), `message`, and at least one `evidence` item with `file`, `line`, and `quote`.
- Optional finding fields: `snippet`, `confidence` (`high`, `medium`, or `low`), `reasoning`, and `suppression` (`{"justification": "..."}`).
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
---
id: METAFIELD_OFFLINE_TOKEN
version: 1
version: 2
severity: high
---

# Metafield Offline Token

Trace metafield writes and determine whether they use an offline Admin API context where an authenticated online context is required. Confirm token provenance and compensating authorization before reporting.
Trace metafield and metaobject writes and determine their token provenance. Report a write that runs in an offline Admin API context that no verified request for the same shop authorizes: for example `unauthenticated.admin(shop)` or a stored offline session reached from an unauthenticated route, an unverified webhook or app proxy request, or a job whose shop or values come from request-controlled input.

An offline session returned by `authenticate.admin(request)` (the React Router SDK default when `useOnlineTokens` isn't set) is a verified, shop-bound context: Shopify verified the session token and the staff member's access to the app. Don't report a write only because its token is offline or because an online token would add user attribution or expiry. Report it when:

- the app defines its own per-user permission, for example an app-level role or allowlist check, and this write bypasses it. Name that permission.
- request-controlled values reach an app-reserved (`$app`) metafield or metaobject that the app treats as trusted state and merchants can't edit themselves, for example a definition with `access.admin = "merchant_read"` or no merchant access, or billing, entitlement or feature-flag data. Writing to the `$app` namespace isn't safe by itself.

Confirm token provenance and compensating authorization before reporting.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
id: MISSING_AUTHORIZATION_CHECK
version: 2
version: 3
severity: high
---

Expand Down Expand Up @@ -57,6 +57,16 @@ actions.
changes, direct URLs that survive permission downgrades, and backend actions
that remain callable after a feature is hidden or disabled in the UI.

## Shopify embedded apps (React Router)

`authenticate.admin(request)` verifies the Shopify session token, so the caller is a staff member of that shop whom Shopify has granted access to this app. With offline tokens (the SDK default), Admin API calls use the app's granted scopes rather than the staff member's own permissions. That is Shopify's standard embedded app model, not a finding by itself. Report a handler only when it skips a concrete authorization boundary inside the app:

- The app defines its own roles, owners, or allowlists (for example a check on `sessionToken.sub`, `onlineAccessInfo.associated_user`, `account_owner`, or an app database role), and a handler that performs the same or a more privileged operation doesn't apply it. Compare each route's loader and action, and sibling routes.
- A request-supplied identifier selects an app-owned record that belongs to another user of the app.
- The app uses online tokens to respect staff permissions on some paths, but a privileged path falls back to an offline session.

If none of these applies, record the check as executed with no finding.

## What to report

For each action that exposes a concrete unauthorized operation beyond authentication:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
---
id: STATIC_FRAME_ANCESTORS
version: 1
version: 2
severity: high
precedence: prefer-agent
---

# Static Frame Ancestors

Inspect embedded-app Content-Security-Policy construction. Report wildcard or static cross-shop frame-ancestors policies; accept a policy derived safely for the authenticated shop plus Shopify Admin.
Inspect embedded-app Content-Security-Policy construction, including headers that app code sets after or instead of the SDK's. Report wildcard or static cross-shop frame-ancestors policies, such as `*`, `https:`, `*.myshopify.com`, or a fixed list of shop domains, whether the value is written literally or built from variables. Accept a policy derived safely for the shop plus Shopify Admin.
Comment thread
jplhomer marked this conversation as resolved.

The React Router SDK's `addDocumentResponseHeaders` (from `shopifyApp()`, usually called in `entry.server`) is the accepted pattern: it sets `frame-ancestors` to the sanitized request shop plus Shopify Admin origins, including Shopify-operated development admin origins. Don't report it unless app code overrides or extends its header.
Loading
Loading