fix(security): sanitize SQL interpolation in Cloudflare Analytics Engine queries#1702
Merged
riderx merged 3 commits intoFeb 28, 2026
Merged
Conversation
…ine queries Add escapeSqlString() utility to prevent SQL injection via user-controlled parameters (deviceIds, search, actions, version_name, cursor) interpolated into Cloudflare Analytics Engine SQL queries in readDevicesCF and readStatsCF. Addresses GHSA-f83x-p28r-pf74.
Contributor
📝 WalkthroughWalkthroughAdds a private helper Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/cloudflare.ts (1)
670-696: Consider escapingparams.app_idtoo for full consistency.Nice coverage on the changed filters. For defense-in-depth, I’d align the base app filters as well (Line 558 and Line 721) so all interpolated string inputs follow the same policy.
Suggested patch
- const conditions: string[] = [`index1 = '${params.app_id}'`] + const conditions: string[] = [`index1 = '${escapeSqlString(params.app_id)}'`]-WHERE - app_id = '${params.app_id}' ${deviceFilter} ${actionsFilter} ${searchFilter} ${startFilter} ${endFilter} +WHERE + app_id = '${escapeSqlString(params.app_id)}' ${deviceFilter} ${actionsFilter} ${searchFilter} ${startFilter} ${endFilter}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 670 - 696, The app_id value is interpolated into SQL strings without using escapeSqlString; update all places that reference params.app_id (the base app filters earlier in the file and the later app filter near where searchFilter/deviceFilter/actionsFilter are built) to pass through escapeSqlString(params.app_id) before embedding; use the same escapeSqlString helper and keep the surrounding single-quote interpolation pattern so variables like deviceFilter, actionsFilter and searchFilter remain consistent and safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@supabase/functions/_backend/utils/cloudflare.ts`:
- Around line 670-696: The app_id value is interpolated into SQL strings without
using escapeSqlString; update all places that reference params.app_id (the base
app filters earlier in the file and the later app filter near where
searchFilter/deviceFilter/actionsFilter are built) to pass through
escapeSqlString(params.app_id) before embedding; use the same escapeSqlString
helper and keep the surrounding single-quote interpolation pattern so variables
like deviceFilter, actionsFilter and searchFilter remain consistent and safe.
…ency Wrap all app_id and params.app_id values with escapeSqlString() where they are interpolated into Analytics Engine SQL query strings. This addresses CodeRabbit review feedback on PR Cap-go#1702 requesting defense-in-depth consistency with the existing escaping applied to deviceIds, search, etc.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
supabase/functions/_backend/utils/cloudflare.ts (4)
1452-1452:⚠️ Potential issue | 🟠 MajorMissing escaping:
org_idnot sanitized in Analytics Engine query.This user-controlled parameter is interpolated directly, unlike
app_idwhich is escaped in other admin functions.🛡️ Proposed fix
- const orgFilter = org_id ? `AND blob2 = '${org_id}'` : '' + const orgFilter = org_id ? `AND blob2 = '${escapeSqlString(org_id)}'` : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` at line 1452, The org_id is interpolated unsafely into the SQL fragment via orgFilter (const orgFilter = org_id ? `AND blob2 = '${org_id}'` : ''), so change this to use a safe, parameterized query or at minimum escape/sanitize org_id before interpolation; update the code that builds the query to pass org_id as a bound parameter (or replace single quotes in org_id with escaped quotes) and reference blob2 and orgFilter accordingly so user-controlled org_id cannot inject SQL.
1634-1634:⚠️ Potential issue | 🟠 MajorMissing escaping:
org_idnot sanitized in Analytics Engine query.Same issue as in
getAdminPlatformOverview— user-controlled parameter should be escaped.🛡️ Proposed fix
- const orgFilter = org_id ? `AND blob2 = '${org_id}'` : '' + const orgFilter = org_id ? `AND blob2 = '${escapeSqlString(org_id)}'` : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` at line 1634, The orgFilter currently interpolates raw org_id into the SQL string (const orgFilter = org_id ? `AND blob2 = '${org_id}'` : ''), which allows injection; update the code in the Analytics Engine query to stop direct string interpolation of org_id and instead pass org_id as a parameter or use the same sanitization used in getAdminPlatformOverview (e.g., a parameterized query or an escape helper) so that org_id is safely bound/escaped before constructing orgFilter or removed from string concatenation entirely; locate usages of orgFilter and the org_id binding in this module to apply the parameterized binding consistently.
485-486:⚠️ Potential issue | 🟡 MinorInconsistent escaping:
channelNameshould useescapeSqlString.The inline escaping only handles single quotes, missing backslash escaping. For consistency and completeness with the rest of this security fix, use the new helper.
🛡️ Proposed fix
- const safeChannel = channelName ? channelName.replace(/'/g, `''`) : '' + const safeChannel = channelName ? escapeSqlString(channelName) : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 485 - 486, The code currently builds safeChannel by manually replacing single quotes (safeChannel = channelName ? channelName.replace(/'/g, `''`) : ''), which is inconsistent with the rest of the codebase; update it to use the escapeSqlString helper instead (use escapeSqlString(channelName) when channelName is truthy) and then use that escaped value to build channelFilter (`channelFilter = escaped ? \`AND default_channel = '${escaped}'\` : ''`), leaving the channelName and channelFilter symbols in place so the change is localized and consistent with other SQL escaping calls.
1008-1024:⚠️ Potential issue | 🟠 MajorD1 SQL injection:
appIdis not parameterized.While outside the immediate scope of this Analytics Engine fix, this D1 query interpolates
appIddirectly without escaping or parameterization. D1 supports bound parameters (as used elsewhere in this file, e.g.,createIfNotExistStoreInfo).🛡️ Proposed fix using parameterized query
export async function getStoreAppByIdCF(c: Context, appId: string): Promise<StoreApp> { if (!c.env.DB_STOREAPPS) return Promise.resolve({} as StoreApp) - const query = `SELECT * FROM store_apps WHERE app_id = '${appId}' LIMIT 1` + const query = `SELECT * FROM store_apps WHERE app_id = ? LIMIT 1` cloudlog({ requestId: c.get('requestId'), message: 'getStoreAppByIdCF query', query }) try { const readD1 = getD1ReadStoreAppSession(c) .prepare(query) + .bind(appId) .first() const res = await readD1 return res🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 1008 - 1024, The getStoreAppByIdCF function currently interpolates appId into the SQL string causing a D1 SQL injection risk; change the query to use a parameter placeholder (e.g. "SELECT * FROM store_apps WHERE app_id = ? LIMIT 1") and call getD1ReadStoreAppSession(c).prepare(query).bind(appId).first() instead of directly interpolating appId into the template, keeping the existing try/catch and return behavior (function: getStoreAppByIdCF; methods: getD1ReadStoreAppSession, prepare, bind, first).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@supabase/functions/_backend/utils/cloudflare.ts`:
- Line 1452: The org_id is interpolated unsafely into the SQL fragment via
orgFilter (const orgFilter = org_id ? `AND blob2 = '${org_id}'` : ''), so change
this to use a safe, parameterized query or at minimum escape/sanitize org_id
before interpolation; update the code that builds the query to pass org_id as a
bound parameter (or replace single quotes in org_id with escaped quotes) and
reference blob2 and orgFilter accordingly so user-controlled org_id cannot
inject SQL.
- Line 1634: The orgFilter currently interpolates raw org_id into the SQL string
(const orgFilter = org_id ? `AND blob2 = '${org_id}'` : ''), which allows
injection; update the code in the Analytics Engine query to stop direct string
interpolation of org_id and instead pass org_id as a parameter or use the same
sanitization used in getAdminPlatformOverview (e.g., a parameterized query or an
escape helper) so that org_id is safely bound/escaped before constructing
orgFilter or removed from string concatenation entirely; locate usages of
orgFilter and the org_id binding in this module to apply the parameterized
binding consistently.
- Around line 485-486: The code currently builds safeChannel by manually
replacing single quotes (safeChannel = channelName ? channelName.replace(/'/g,
`''`) : ''), which is inconsistent with the rest of the codebase; update it to
use the escapeSqlString helper instead (use escapeSqlString(channelName) when
channelName is truthy) and then use that escaped value to build channelFilter
(`channelFilter = escaped ? \`AND default_channel = '${escaped}'\` : ''`),
leaving the channelName and channelFilter symbols in place so the change is
localized and consistent with other SQL escaping calls.
- Around line 1008-1024: The getStoreAppByIdCF function currently interpolates
appId into the SQL string causing a D1 SQL injection risk; change the query to
use a parameter placeholder (e.g. "SELECT * FROM store_apps WHERE app_id = ?
LIMIT 1") and call
getD1ReadStoreAppSession(c).prepare(query).bind(appId).first() instead of
directly interpolating appId into the template, keeping the existing try/catch
and return behavior (function: getStoreAppByIdCF; methods:
getD1ReadStoreAppSession, prepare, bind, first).
Author
|
Hi @riderx 👋 This PR fixes multiple SQL injection vulnerabilities in The fix introduces an Could you take a look when you get a chance? These are security-critical fixes that protect against SQL injection attacks on the Cloudflare D1 database layer. Thanks! |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
supabase/functions/_backend/utils/cloudflare.ts (4)
1452-1452:⚠️ Potential issue | 🟠 MajorMissing escape:
org_idnot sanitized.The
org_idparameter is interpolated without escaping, inconsistent with the PR's goal of sanitizing all user-controlled inputs.🐛 Proposed fix
- const orgFilter = org_id ? `AND blob2 = '${org_id}'` : '' + const orgFilter = org_id ? `AND blob2 = '${escapeSqlString(org_id)}'` : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` at line 1452, The org_id is directly interpolated into orgFilter causing an injection risk; instead, stop building SQL via string concatenation and either use a parameterized/bound query for blob2 or sanitize/escape org_id before concatenation. Locate the orgFilter assignment in cloudflare.ts (variable orgFilter, referenced blob2 and org_id) and change it to use a query parameter placeholder with the org_id bound by the DB client (preferred), or if binding is not available, validate org_id against an allowed pattern and escape single quotes before inserting.
485-486:⚠️ Potential issue | 🟡 MinorInconsistent escaping:
channelNamebypassesescapeSqlString.This inline escape only handles single quotes but not backslashes, making it inconsistent with the new
escapeSqlStringfunction and potentially vulnerable.🐛 Proposed fix
- const safeChannel = channelName ? channelName.replace(/'/g, `''`) : '' + const safeChannel = channelName ? escapeSqlString(channelName) : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 485 - 486, The code builds safeChannel and channelFilter using channelName.replace(...) which only escapes single quotes; replace that logic to call the existing escapeSqlString(channelName) helper (handling null/undefined) so escaping is consistent and backslashes are handled—i.e., compute safeChannel = channelName ? escapeSqlString(channelName) : '' and then build channelFilter = safeChannel ? `AND default_channel = '${safeChannel}'` : '' (use the escape function where safeChannel is referenced).
1008-1025:⚠️ Potential issue | 🟠 MajorD1 SQL injection:
appIdnot parameterized.Unlike other D1 queries in this file that use
.bind()for parameters, this function directly interpolatesappIdinto the SQL string. IfappIdoriginates from user input, this is vulnerable to SQL injection.🐛 Proposed fix using prepared statement
export async function getStoreAppByIdCF(c: Context, appId: string): Promise<StoreApp> { if (!c.env.DB_STOREAPPS) return Promise.resolve({} as StoreApp) - const query = `SELECT * FROM store_apps WHERE app_id = '${appId}' LIMIT 1` + const query = `SELECT * FROM store_apps WHERE app_id = ? LIMIT 1` cloudlog({ requestId: c.get('requestId'), message: 'getStoreAppByIdCF query', query }) try { const readD1 = getD1ReadStoreAppSession(c) .prepare(query) + .bind(appId) .first() const res = await readD1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 1008 - 1025, The SQL in getStoreAppByIdCF interpolates appId directly causing SQL injection risk; change the query to use a parameter placeholder (e.g. "WHERE app_id = ? LIMIT 1"), then call getD1ReadStoreAppSession(c).prepare(query).bind(appId).first() so the appId is bound instead of concatenated; keep existing error logging (cloudlog/cloudlogErr) and return behavior but replace the string-interpolated query and ensure any cloudlog that prints the query does not leak raw unbound values.
1634-1634:⚠️ Potential issue | 🟠 MajorMissing escape:
org_idnot sanitized.Same issue as in
getAdminPlatformOverview—theorg_idparameter needs escaping.🐛 Proposed fix
- const orgFilter = org_id ? `AND blob2 = '${org_id}'` : '' + const orgFilter = org_id ? `AND blob2 = '${escapeSqlString(org_id)}'` : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` at line 1634, The org_id value is interpolated into SQL without escaping, allowing injection; update the code that builds orgFilter (the const orgFilter = org_id ? `AND blob2 = '${org_id}'` : ''`) to use a safe parameterized query or at minimum escape single quotes (e.g., replace all ' with ''), and prefer switching the calling function (the code path that uses orgFilter, same pattern as getAdminPlatformOverview) to pass org_id as a bound parameter to the query rather than string interpolation so blob2 comparison is safe.
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/cloudflare.ts (1)
13-16: Good addition for SQL injection defense.The escape function correctly handles single quotes and backslashes for ClickHouse SQL literals. For additional hardening, consider also stripping null bytes (
\x00) which could cause string truncation in some SQL implementations:🛡️ Optional: strip null bytes for defense-in-depth
function escapeSqlString(value: string): string { - return value.replace(/'/g, '\'\'').replace(/\\/g, '\\\\') + return value.replace(/\x00/g, '').replace(/'/g, '\'\'').replace(/\\/g, '\\\\') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/cloudflare.ts` around lines 13 - 16, The escapeSqlString function currently escapes single quotes and backslashes but should also strip null bytes to avoid potential string truncation; update escapeSqlString to remove any '\x00' characters (e.g., apply a .replace(/\x00/g, '') or equivalent) in addition to the existing replaces so the function consistently sanitizes null bytes, single quotes, and backslashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@supabase/functions/_backend/utils/cloudflare.ts`:
- Line 1452: The org_id is directly interpolated into orgFilter causing an
injection risk; instead, stop building SQL via string concatenation and either
use a parameterized/bound query for blob2 or sanitize/escape org_id before
concatenation. Locate the orgFilter assignment in cloudflare.ts (variable
orgFilter, referenced blob2 and org_id) and change it to use a query parameter
placeholder with the org_id bound by the DB client (preferred), or if binding is
not available, validate org_id against an allowed pattern and escape single
quotes before inserting.
- Around line 485-486: The code builds safeChannel and channelFilter using
channelName.replace(...) which only escapes single quotes; replace that logic to
call the existing escapeSqlString(channelName) helper (handling null/undefined)
so escaping is consistent and backslashes are handled—i.e., compute safeChannel
= channelName ? escapeSqlString(channelName) : '' and then build channelFilter =
safeChannel ? `AND default_channel = '${safeChannel}'` : '' (use the escape
function where safeChannel is referenced).
- Around line 1008-1025: The SQL in getStoreAppByIdCF interpolates appId
directly causing SQL injection risk; change the query to use a parameter
placeholder (e.g. "WHERE app_id = ? LIMIT 1"), then call
getD1ReadStoreAppSession(c).prepare(query).bind(appId).first() so the appId is
bound instead of concatenated; keep existing error logging
(cloudlog/cloudlogErr) and return behavior but replace the string-interpolated
query and ensure any cloudlog that prints the query does not leak raw unbound
values.
- Line 1634: The org_id value is interpolated into SQL without escaping,
allowing injection; update the code that builds orgFilter (the const orgFilter =
org_id ? `AND blob2 = '${org_id}'` : ''`) to use a safe parameterized query or
at minimum escape single quotes (e.g., replace all ' with ''), and prefer
switching the calling function (the code path that uses orgFilter, same pattern
as getAdminPlatformOverview) to pass org_id as a bound parameter to the query
rather than string interpolation so blob2 comparison is safe.
---
Nitpick comments:
In `@supabase/functions/_backend/utils/cloudflare.ts`:
- Around line 13-16: The escapeSqlString function currently escapes single
quotes and backslashes but should also strip null bytes to avoid potential
string truncation; update escapeSqlString to remove any '\x00' characters (e.g.,
apply a .replace(/\x00/g, '') or equivalent) in addition to the existing
replaces so the function consistently sanitizes null bytes, single quotes, and
backslashes.
|
Author
|
Hi @riderx 👋 The lint issue has been fixed in the latest commit. Could you please approve the workflow run so CI can complete? The fix adds proper SQL escaping for user inputs in Cloudflare Analytics queries to prevent injection attacks. Happy to make any changes if needed. Thanks! |
Author
|
Hi @riderx 👋 Thank you for merging this security fix! 🎉 This PR addresses SQL injection vulnerabilities in the Cloudflare Analytics queries, which could have allowed attackers to manipulate database queries through malicious Since this is a security vulnerability fix, would you consider awarding a bounty for this contribution? Thanks again! |
Author
|
Hi @riderx, thanks so much! 🙏 Just a small note - looks like the tip command had a typo (
Thanks again for the recognition! |
Author
|
Hi @riderx, just a friendly reminder - the tip command had a small typo (ampersand instead of dollar sign). Could you please re-run it? Thanks! |
Member
|
/tip @artylobos $150 thanks for the fix it was not a big vulnerability as the user can only inject statistics data but good finding |
|
🎉🎈 @artylobos has been awarded $150 by Capgo! 🎈🎊 |
This was referenced Mar 1, 2026
riderx
added a commit
that referenced
this pull request
Mar 3, 2026
* fix(security): restrict apikey oracle rpc access * fix: webapp url * fix: fix * chore(release): 12.116.9 * fix: envs * Revert "Merge pull request #1707 from Cap-go/fix_webapp_url" This reverts commit ff20d1a. * fix: typo * chore(release): 12.116.10 * fix(security): restrict apikey oracle rpc access * fix: return 503 instead of 400 for service_unavailable build errors Builder availability errors (not configured, call failed, error response, missing upload URL) are transient server-side failures, not client errors. Returning 503 allows the CLI retry logic to automatically retry these requests instead of treating them as terminal 400 errors. * chore(release): 12.116.11 * fix: update PWD script * fix: env vars * fix: modal responsive * feat: forward buildOptions + buildCredentials to builder (pass-through) * fix: correct vue/html-indent in DemoOnboardingModal * fix: use snake_case (build_options, build_credentials) in public API, map to camelCase for builder * fix(security): sanitize SQL interpolation in Cloudflare Analytics Engine queries (#1702) * chore(release): 12.116.12 * Add unit tests for builder payload shape contract Extract buildBuilderPayload() from the inline fetch body so the snake_case → camelCase mapping and exact key set can be tested. 6 vitest cases verify: camelCase output, no legacy credentials field, correct metadata keys, and pass-through of contents. * Reject deprecated `credentials` field with clear upgrade error Old CLI clients sending the flat `credentials` field would have it silently dropped, causing confusing builder failures. Now the proxy explicitly rejects non-empty `credentials` with a migration message pointing to `build_credentials`. * fix(security): clean up role_bindings on member removal (#1722) * chore(release): 12.116.13 * fix(security): use parameterized query in getStoreAppByIdCF to prevent SQL injection The appId parameter was directly interpolated into the D1 SQL query string, creating a SQL injection vulnerability. Switched to bound parameter via .bind(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): prevent privilege escalation in role_bindings endpoint Add priority_rank check so callers cannot assign or update roles with higher privileges than their own. Without this, any user with org.update_user_roles could escalate to org_super_admin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): enforce is_assignable in role_bindings INSERT RLS policy Direct PostgREST inserts could bypass the endpoint's is_assignable check and assign non-assignable roles (e.g. platform_super_admin). The RLS INSERT policy now requires the target role to have is_assignable = true. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): cascade all role bindings on member removal delete_org_member_role previously only deleted the org-level binding, leaving orphaned app/channel bindings. A removed member could retain app-level access. Now deletes all bindings for the user in the org. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): add trigger to prevent deleting last super_admin binding Direct PostgREST DELETEs on role_bindings could bypass the last super_admin guards in delete_org_member_role. A BEFORE DELETE trigger now rejects deletion of the last org_super_admin binding in any org. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): support hashed API keys in rbac_check_permission_direct The RBAC path in rbac_check_permission_direct looked up API keys with WHERE key = p_apikey, which silently failed for hashed keys. Switched to find_apikey_by_value() which handles both plain-text and hashed keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: reword comment to pass typos CI check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove unused desc import from role_bindings.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): add FOR UPDATE lock to prevent write-skew on last super_admin delete Two concurrent DELETE transactions could both pass the count check and both delete their rows, leaving zero super_admins. A SELECT ... FOR UPDATE on the super_admin binding set now serializes concurrent deletes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: prevent API key privilege escalation and fix organization member deletion test - Add validation to prevent limited API keys from creating unlimited keys - Fix organization-api test to work with sync_org_user_to_role_binding trigger - Change test user_right from 'invite_read' to 'read' (trigger-compatible) - Verify trigger-created role_bindings instead of manually inserting them * fix: allow CASCADE deletions in prevent_last_super_admin_binding_delete and fix RBAC test compatibility - Add org existence check in trigger to allow CASCADE deletions when org is being deleted - Add service_role bypass for administrative operations and tests - Update tests to work with RBAC security constraints: - 34_test_rbac_rls.sql: Remove DELETE operation that violated super_admin protection - 35_test_is_admin_rbac.sql: Use service_role for test setup INSERT - All SQL database tests now pass (860 tests) - Backend tests remain passing (68 tests) * fix(security): make getCallerMaxPriorityRank auth-type-aware and remove API key data leak * chore(release): 12.116.14 * fix(security): correct API key RBAC principal mapping and remove service_role bypass * fix(security): correct RBAC migration comments and add privilege check on delete - Update migration comments to accurately reflect that service_role is NOT exempt from the last super_admin protection trigger - Replace FOR UPDATE scan with pg_advisory_xact_lock to avoid cross-transaction deadlocks - Add privilege-rank check in delete handler to prevent deleting higher-ranked role bindings - Aligns with established advisory lock patterns in codebase Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix: add self-2fa-required message for 2FA enforcement in multiple languages * chore(release): 12.116.15 * fix(frontend): validate 2fa before enabling org enforcement (#1729) * chore(release): 12.116.16 * fix(deps): update vue monorepo to v3.5.29 (#1731) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(release): 12.116.17 * chore: remove unused cloudflare function getStoreAppByIdCF * chore(release): 12.116.18 * chore: stop editing immutable base migration * fix(frontend): disable auto demo onboarding modal (#1733) * chore(release): 12.116.19 * fix(auth): block sensitive account actions for unverified users (#1690) * fix(auth): block account deletion for unverified users * fix(auth): refresh session fields for email verification gate * fix(auth): make delete_user insert idempotent * fix(auth): explain blocked delete/settings when email unverified * fix(auth): block delete action when email is unverified * fix(auth): localize resend email block and make delete_user idempotent * Restrict invite_user_to_org RPC to authenticated callers (#1710) * fix(db): restrict invite_user_to_org public rpc * fix(db): use caller identity in invite 2FA check * fix(security): restrict webhook select to admin users (#1705) * Secure record_build_time RPC for authorized callers (#1711) * fix(db): secure record_build_time rpc writes * fix(db): preserve service-role record_build_time path * fix(security): restrict apikey oracle rpc access * chore: stop editing immutable base migration * fix(security): restrict apikey oracle rpc access * chore(release): 12.116.20 * fix(security): restrict apikey oracle rpc access * chore: stop editing immutable base migration * fix(security): restrict apikey oracle rpc access * chore: stop editing immutable base migration * fix(security): restrict apikey oracle rpc access --------- Co-authored-by: WcaleNieWolny <isupermichael007@gmail.com> Co-authored-by: WcaleNieWolny <50914789+WcaleNieWolny@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: LOLO <131777939+artylobos@users.noreply.github.com> Co-authored-by: Jordan Lorho <jordan.lorho@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes SQL injection vulnerabilities in Cloudflare Analytics Engine queries (GHSA-f83x-p28r-pf74).
User-controlled string parameters were interpolated directly into SQL query strings without sanitization in
readDevicesCFandreadStatsCF, allowing an attacker to break out of string literals and inject arbitrary SQL.Changes
escapeSqlString()utility function that escapes single quotes and backslashes for safe SQL string interpolationescapeSqlString()to all user-controlled values interpolated into Analytics Engine SQL queriesAffected Functions
readDevicesCFparams.deviceIds— single value and IN-list interpolationparams.search— search filter interpolation (both with and without deviceIds)params.version_name— version filter interpolationparams.cursor— cursor-based pagination (timestamp and device_id components)readStatsCFparams.deviceIds— single value and IN-list interpolation (the IN-list was critically missing quotes entirely)params.actions— single value and IN-list interpolationparams.search— search filter interpolation (both with and without deviceIds)Critical Fix
The
readStatsCFIN-list fordeviceIdswas particularly dangerous — values were joined without any quoting at all, making injection trivial./claim #1667
Summary by CodeRabbit