From aa34a6d03552e0416fe9e2237cf59be39f02c585 Mon Sep 17 00:00:00 2001 From: Jaynel Patiarba Date: Tue, 11 Aug 2026 23:57:44 +0800 Subject: [PATCH] fix(cli): prevent SQL injection into platform Supabase via package.json name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit provisionAnalytics() in migrate.ts read `name` from the migrated repo's package.json and interpolated it, unescaped, into a single-quoted SQL literal sent to the decocms Supabase Management API `/database/query` endpoint (raw SQL, no bound parameters) against the central `public.sites` table that tracks every site. A crafted name such as "name": "x'; UPDATE public.sites SET metadata=...::jsonb WHERE name='victim'; --" broke out of the literal and ran arbitrary SQL cross-tenant on the platform DB (runs whenever an operator migrates the repo with SUPABASE_ACCESS_TOKEN set). Fix (new migrate/sql-safety.ts): - isValidNpmPackageName() — reject any name that is not a valid npm package name before it can reach the query; provisioning is skipped for invalid names. The grammar forbids quotes, `;`, spaces, newlines. - escapeSqlLiteral() — SQL-standard quote doubling, applied to both the executed query and the printAnalyticsSQL manual-run output as defense-in-depth. Adds migrate/sql-safety.test.ts covering valid names, injection payloads, non-string input, and quote-escaping. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/blocks-cli/scripts/migrate.ts | 16 +++++- .../scripts/migrate/sql-safety.test.ts | 52 +++++++++++++++++++ .../blocks-cli/scripts/migrate/sql-safety.ts | 36 +++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 packages/blocks-cli/scripts/migrate/sql-safety.test.ts create mode 100644 packages/blocks-cli/scripts/migrate/sql-safety.ts diff --git a/packages/blocks-cli/scripts/migrate.ts b/packages/blocks-cli/scripts/migrate.ts index da483103..881b1c23 100755 --- a/packages/blocks-cli/scripts/migrate.ts +++ b/packages/blocks-cli/scripts/migrate.ts @@ -29,6 +29,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { banner, green, red, stat, yellow } from "./migrate/colors"; import { loadConfig, validateConfig } from "./migrate/config"; +import { escapeSqlLiteral, isValidNpmPackageName } from "./migrate/sql-safety"; import { analyze } from "./migrate/phase-analyze"; import { cleanup } from "./migrate/phase-cleanup"; import { cleanupAudit } from "./migrate/phase-cleanup-audit"; @@ -296,6 +297,17 @@ async function provisionAnalytics(sourceDir: string): Promise { return; } + // `siteName` comes from the untrusted repo's package.json and is spliced into + // a raw SQL statement against the central platform DB. Reject anything that is + // not a valid npm package name before it can reach the query (blocks SQL + // injection via a crafted `name`). + if (!isValidNpmPackageName(siteName)) { + console.log( + ` ${yellow("⚠")} Invalid package name — skipping analytics provision: ${JSON.stringify(siteName)}`, + ); + return; + } + const token = process.env.SUPABASE_ACCESS_TOKEN; if (!token) { console.log(` ${yellow("⚠")} SUPABASE_ACCESS_TOKEN not set — skipping analytics provision`); @@ -303,7 +315,7 @@ async function provisionAnalytics(sourceDir: string): Promise { return; } - const sql = `UPDATE public.sites SET metadata = metadata || '{"analytics": "onedollarstats"}'::jsonb WHERE name = '${siteName}'`; + const sql = `UPDATE public.sites SET metadata = metadata || '{"analytics": "onedollarstats"}'::jsonb WHERE name = '${escapeSqlLiteral(siteName)}'`; try { const res = await fetch( @@ -333,7 +345,7 @@ async function provisionAnalytics(sourceDir: string): Promise { function printAnalyticsSQL(siteName: string): void { console.log(` Run manually on decocms Supabase (${DECOCMS_SUPABASE_REF}):`); console.log( - ` UPDATE public.sites SET metadata = metadata || '{"analytics": "onedollarstats"}'::jsonb WHERE name = '${siteName}';`, + ` UPDATE public.sites SET metadata = metadata || '{"analytics": "onedollarstats"}'::jsonb WHERE name = '${escapeSqlLiteral(siteName)}';`, ); } diff --git a/packages/blocks-cli/scripts/migrate/sql-safety.test.ts b/packages/blocks-cli/scripts/migrate/sql-safety.test.ts new file mode 100644 index 00000000..fe9672e6 --- /dev/null +++ b/packages/blocks-cli/scripts/migrate/sql-safety.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { escapeSqlLiteral, isValidNpmPackageName } from "./sql-safety"; + +describe("isValidNpmPackageName", () => { + it("accepts real package names", () => { + for (const name of ["my-site", "@org/site", "site_1.2.3", "a", "@deco/casaevideo", "x-y.z~q"]) { + expect(isValidNpmPackageName(name)).toBe(true); + } + }); + + it("rejects SQL-injection payloads and other illegal names", () => { + for (const name of [ + "x'; UPDATE public.sites SET metadata='{}'::jsonb; --", // the injection + "x' OR '1'='1", + "a b", // space + "UPPER", // uppercase not allowed + "name\nwith-newline", + "", // empty + "'", + "a".repeat(215), // too long + ]) { + expect(isValidNpmPackageName(name)).toBe(false); + } + }); + + it("rejects non-string input", () => { + for (const v of [null, undefined, 42, {}, ["x"]]) { + expect(isValidNpmPackageName(v)).toBe(false); + } + }); +}); + +describe("escapeSqlLiteral", () => { + it("doubles single quotes so a literal cannot be broken out of", () => { + expect(escapeSqlLiteral("x'; DROP TABLE sites; --")).toBe("x''; DROP TABLE sites; --"); + expect(escapeSqlLiteral("o'brien")).toBe("o''brien"); + }); + + it("leaves quote-free values unchanged", () => { + expect(escapeSqlLiteral("my-site")).toBe("my-site"); + }); + + it("neutralizes a breakout when embedded in a single-quoted literal", () => { + const evil = "x'; UPDATE public.sites SET metadata='{}'::jsonb WHERE name='victim'; --"; + const sql = `WHERE name = '${escapeSqlLiteral(evil)}'`; + // The escaped payload contains no lone `'` that could terminate the literal: + // every original quote is now doubled, so the whole thing stays one string. + expect(sql).toBe( + "WHERE name = 'x''; UPDATE public.sites SET metadata=''{}''::jsonb WHERE name=''victim''; --'", + ); + }); +}); diff --git a/packages/blocks-cli/scripts/migrate/sql-safety.ts b/packages/blocks-cli/scripts/migrate/sql-safety.ts new file mode 100644 index 00000000..5f1f0b40 --- /dev/null +++ b/packages/blocks-cli/scripts/migrate/sql-safety.ts @@ -0,0 +1,36 @@ +/** + * Helpers for safely embedding untrusted values into SQL sent to the decocms + * Supabase Management API. + * + * The `/database/query` REST endpoint runs raw SQL — it has no bound-parameter + * facility — so any value spliced into a query string must be both validated + * and quote-escaped by us. The migrator reads `package.json` `name` from the + * (untrusted) repo being migrated and used to interpolate it straight into a + * single-quoted SQL literal, which allowed quote-breakout SQL injection against + * the central `public.sites` table. + */ + +// npm package-name grammar: optional `@scope/`, then the name. Lowercase only, +// limited to [a-z0-9-._~]. Crucially forbids `'`, `;`, spaces, backslashes and +// newlines — i.e. everything needed for SQL breakout. +const NPM_PACKAGE_NAME = /^(?:@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/; + +/** True iff `name` is a syntactically valid npm package name. */ +export function isValidNpmPackageName(name: unknown): name is string { + return ( + typeof name === "string" && + name.length > 0 && + name.length <= 214 && + NPM_PACKAGE_NAME.test(name) + ); +} + +/** + * Escape a value for inclusion inside a single-quoted SQL string literal by + * doubling embedded single quotes (SQL-standard escaping). Defense-in-depth on + * top of {@link isValidNpmPackageName} — also covers the manual `printAnalyticsSQL` + * output path. + */ +export function escapeSqlLiteral(value: string): string { + return value.replace(/'/g, "''"); +}