|
1 | 1 | --- |
2 | 2 | id: SCOPE_OVER_REQUEST |
3 | | -version: 2 |
| 3 | +version: 1 |
4 | 4 | severity: high |
5 | 5 | --- |
6 | 6 |
|
7 | | -Review whether the app's requested and effective OAuth access is necessary |
8 | | -for its supported behavior. Distinguish least-privilege recommendations |
9 | | -from demonstrated access beyond the intended authorization boundary. |
| 7 | +Find cases where an app requests OAuth scopes it does not use, or uses |
| 8 | +scopes in ways that exceed what the merchant authorised. |
10 | 9 |
|
11 | | -A scope declaration is not proof of a live grant, data misuse, or a |
12 | | -privacy violation. Failure to find a matching API call is not proof that |
13 | | -a scope is unused. Report a finding only when repository evidence shows |
14 | | -a concrete excess-authority path and its security impact; do not turn a |
15 | | -configuration cleanup opportunity into a high-severity finding. |
| 10 | +When a merchant installs an app, they grant a set of access scopes (e.g. |
| 11 | +`read_orders`, `write_products`). The app should only access data covered |
| 12 | +by those scopes. Two risks: |
16 | 13 |
|
17 | | -## What to look for |
18 | | - |
19 | | -1. **Find the requested scopes and configuration.** Use the app config |
20 | | - selected by the review workflow. Inspect `[access_scopes].scopes`, |
21 | | - `required_scopes`, and `optional_scopes`, plus OAuth authorization URLs, |
22 | | - environment configuration, and dynamic optional-scope requests when |
23 | | - present. Do not combine declarations from unrelated apps or environments. |
| 14 | +1. **Over-requested scopes:** the app declares scopes in its config that it |
| 15 | + never references in code. This is a privacy violation — the merchant |
| 16 | + granted access to data the app doesn't need. |
24 | 17 |
|
25 | | -2. **Trace scope usage broadly.** Search for operations that exercise |
26 | | - Shopify resources across the entire reviewed boundary, not only the |
27 | | - entry directory: |
28 | | - - REST and GraphQL API calls (`admin.rest`, `admin.graphql`, |
29 | | - `client.get/post`, REST resource classes) |
30 | | - - Generated or dynamically built GraphQL (template strings, query |
31 | | - builders, `.graphql` files, codegen output) |
32 | | - - Wrappers and SDK helpers that hide the underlying operation |
33 | | - - Shared packages, monorepo workspaces, and background jobs reachable |
34 | | - from the app |
35 | | - - Flag-gated paths and optional features that may not run on every |
36 | | - install but still exercise a scope when enabled |
| 18 | +2. **Under-verified usage:** the app calls an API endpoint that requires a |
| 19 | + scope, but doesn't check that the scope was granted before making the |
| 20 | + call. This can fail at runtime or, worse, access data the merchant |
| 21 | + didn't authorise if the scope was added by a different code path. |
37 | 22 |
|
38 | | -3. **Distinguish declared, requested, and granted scopes.** An optional |
39 | | - declaration allows a later request; it does not establish that the |
40 | | - merchant granted it. Deployment, installation, approval, and revocation |
41 | | - can leave the current grant different from local configuration. Use |
42 | | - available session, grant-query, and consent-handling evidence. State |
43 | | - when the effective grant cannot be established rather than inventing it. |
| 23 | +## What to look for |
44 | 24 |
|
45 | | -4. **Match exact operations to their scope requirements.** Use the |
46 | | - applicable API version's field or mutation requirements, not keyword |
47 | | - matches or a guessed resource-to-scope table. Account for implied |
48 | | - access such as a write scope also granting read access, and for |
49 | | - wrappers or generated queries that do not name the resource literally. |
| 25 | +1. **Find the declared scopes.** Look in `shopify.app.toml` under |
| 26 | + `[access_scopes]` → `scopes`, or in the app's OAuth redirect URL, or |
| 27 | + in environment variables like `SCOPES`. |
50 | 28 |
|
51 | | -5. **Establish necessity before calling a scope excessive.** Compare the |
52 | | - requested authority with supported features and reachable operations. |
53 | | - A complete reviewed corpus with no use may justify cleanup guidance, |
54 | | - but it does not by itself demonstrate a trust-boundary violation. |
55 | | - Planned work is not proof a scope is necessary today, nor is the |
56 | | - declaration alone a security finding. Identify the concrete data or |
57 | | - operation exposed through the excessive authority before reporting it. |
| 29 | +2. **Find where scopes are used.** Search for API calls that reference |
| 30 | + Shopify resources: `admin.rest.get`, `admin.graphql`, REST resource |
| 31 | + classes, GraphQL queries on `orders`, `products`, `customers`, etc. |
58 | 32 |
|
59 | | -6. **Inspect scope-dependent behavior without inventing platform bypasses.** |
60 | | - Follow optional-scope requests, consent handling, grant checks, and |
61 | | - denied or revoked access through the affected feature. Shopify enforces |
62 | | - API scopes server-side. A missing local declaration or preflight check, |
63 | | - or an expected access-denied API response, does not establish |
64 | | - unauthorized access. Report only a demonstrated excess-authority path, |
65 | | - not an assumption that an API call succeeds without authorization. |
| 33 | +3. **Match scopes to usage.** Each scope should map to at least one API |
| 34 | + call: |
| 35 | + - `read_orders` → queries on orders |
| 36 | + - `write_products` → mutations on products |
| 37 | + - `read_customers` → queries on customers |
| 38 | + - etc. |
66 | 39 |
|
67 | | -7. **Keep API scopes and object authorization separate.** OAuth access |
68 | | - scopes govern which resource classes an app may touch. |
69 | | - Staff-permission or customer object-level authorization is a |
70 | | - different boundary. If the real bug is missing object-level |
71 | | - authorization or tenant isolation, refer to the owning check |
72 | | - (`MISSING_AUTHORIZATION_CHECK`, `MISSING_TENANT_ISOLATION`) instead |
73 | | - of duplicating it here. |
| 40 | +4. **Flag scopes with no matching usage.** If `read_analytics` is declared |
| 41 | + but no code references analytics, that's an over-requested scope. |
74 | 42 |
|
75 | | -8. **Respect review boundaries.** If the review pack scopes you to a |
76 | | - subdirectory or a subset of the app, absence of a matching call |
77 | | - inside that boundary is not proof of absence across the whole app. |
78 | | - State the boundary you actually reviewed and mark scope-match |
79 | | - questions unresolved when the unreviewed remainder could contain the |
80 | | - usage. |
| 43 | +5. **Flag API calls with no matching scope.** If code queries customers |
| 44 | + but `read_customers` isn't declared, that's an under-verified usage. |
81 | 45 |
|
82 | 46 | ## What to report |
83 | 47 |
|
84 | | -Use the generated review pack's current finding and execution schemas; |
85 | | -do not invent extra fields or a standalone JSON envelope. |
86 | | - |
87 | | -A finding must identify: |
88 | | - |
89 | | -- The selected configuration and the relevant scope declaration or request. |
90 | | -- The principal, intended authorization boundary, and requested authority. |
91 | | -- The reachable operation and concrete data or capability exposed through |
92 | | - the excess authority, with file/line evidence for the complete path. |
93 | | -- Which effective-grant facts are established and which are unavailable. |
94 | | -- The reviewed directories, packages, helpers, and feature paths. Account |
95 | | - for any unreviewed code that could change the conclusion. |
96 | | - |
97 | | -Do not report a finding solely because: |
98 | | - |
99 | | -- A scope is declared ahead of planned work or for an optional feature. |
100 | | -- No matching keyword or API call appears in one directory. |
101 | | -- The app is config-only and no source corpus is available. |
102 | | -- A scope is missing from local TOML, or an API call fails with access denied. |
103 | | -- A write scope is used for reads or implies another required scope. |
104 | | -- A declaration could theoretically increase the impact of a future leak, |
105 | | - without a demonstrated excess-authority path in the reviewed code. |
106 | | - |
107 | | -Keep supported least-privilege recommendations separate from findings. |
108 | | -Do not label planned scopes or documented intent as automatically safe: |
109 | | -comments and documentation are evidence to corroborate, not authorization. |
110 | | -If incomplete source, unknown feature reachability, or unavailable grant |
111 | | -evidence prevents a conclusion, record the check as unresolved with the |
112 | | -review pack's structured reason and guidance rather than asserting either |
113 | | -a vulnerability or a pass. |
| 48 | +```json |
| 49 | +{ |
| 50 | + "file": "shopify.app.toml", |
| 51 | + "line": 10, |
| 52 | + "message": "Scope 'read_analytics' is declared but never referenced in app code", |
| 53 | + "evidence": [ |
| 54 | + { |
| 55 | + "file": "shopify.app.toml", |
| 56 | + "line": 10, |
| 57 | + "quote": "scopes = \"read_orders,read_analytics\"" |
| 58 | + } |
| 59 | + ], |
| 60 | + "confidence": "medium", |
| 61 | + "reasoning": "Searched all source files for 'analytics' and found no API calls referencing analytics endpoints or resources." |
| 62 | +} |
| 63 | +``` |
| 64 | + |
| 65 | +For under-verified usage, report the code location, not the TOML: |
| 66 | + |
| 67 | +```json |
| 68 | +{ |
| 69 | + "file": "app/services/customer_export.rb", |
| 70 | + "line": 15, |
| 71 | + "message": "Queries customers but 'read_customers' is not in declared scopes", |
| 72 | + "evidence": [ |
| 73 | + { |
| 74 | + "file": "app/services/customer_export.rb", |
| 75 | + "line": 15, |
| 76 | + "quote": "Customer.all" |
| 77 | + }, |
| 78 | + { |
| 79 | + "file": "shopify.app.toml", |
| 80 | + "line": 10, |
| 81 | + "quote": "scopes = \"read_orders\"" |
| 82 | + } |
| 83 | + ], |
| 84 | + "confidence": "high" |
| 85 | +} |
| 86 | +``` |
| 87 | + |
| 88 | +Note: if the app has zero source files (config-only app), do not report |
| 89 | +over-requested scopes — you cannot verify usage from an empty corpus. |
0 commit comments