diff --git a/.changeset/turso-unsafe-identifier-envelope.md b/.changeset/turso-unsafe-identifier-envelope.md new file mode 100644 index 0000000000..a2f47f7eda --- /dev/null +++ b/.changeset/turso-unsafe-identifier-envelope.md @@ -0,0 +1,27 @@ +--- +'@objectstack/driver-turso': patch +'@objectstack/spec': patch +--- + +drivers(turso): an unsafe identifier on the remote transport answers `400 INVALID_REQUEST`, not an opaque 500 + +`RemoteTransport.assertSafeIdentifier` is the one gate for every position where +an identifier is inlined into SQL — the `object`, `field` and `groupBy` +field/output-key positions of `aggregate`, the table and column names in +`syncSchema` / `syncSchemasBatch`, and the index name and columns in unique-index +sync — plus the free function of the same name in the remote canonical backfill. +All of them threw a bare `Error` with no `code` and no `status`, so `mapDataError` +reached none of its classifying branches and served a sanitised **500**: a caller +whose own identifier was refused was told the server had faulted, and an SDK +reading a 5xx retries a request that can never succeed. + +Every one of those refusals now carries the ADR-0112 envelope `code: +'INVALID_REQUEST'`, `status: 400`, built by one constructor so the positions +cannot answer three ways. `@objectstack/spec` gains only the error-code ledger's +provenance row registering this driver as an emitter of a code seven packages +already register — the registered-code union is byte-identical (248 codes before +and after) and no new code is minted. + +**Nothing about which identifiers are refused changed.** `SAFE_IDENTIFIER` and +every refusal message are untouched; exactly the inputs refused before are +refused after, with byte-identical prose. Only `code` and `status` are new. diff --git a/packages/drivers/driver-turso/src/remote-canonical-backfill.ts b/packages/drivers/driver-turso/src/remote-canonical-backfill.ts index 10f8f0a8ef..5d5ce5ea9b 100644 --- a/packages/drivers/driver-turso/src/remote-canonical-backfill.ts +++ b/packages/drivers/driver-turso/src/remote-canonical-backfill.ts @@ -93,6 +93,12 @@ * twin's contract calls out, kept across the transport boundary. */ +// [#14287] The unsafe-identifier refusal's ADR-0112 envelope, from the one +// place that decides it. Imported rather than re-spelled so this module and +// `RemoteTransport` cannot answer one condition two ways; `turso-driver.ts` +// already loads both modules together, so the edge costs nothing at runtime. +import { unsafeIdentifierError } from './remote-transport.js'; + /** The `@libsql/client` surface this module uses — nothing more. */ export interface RemoteBackfillClient { execute(stmt: { sql: string; args?: unknown[] } | string): Promise<{ @@ -218,9 +224,34 @@ export const REMOTE_BACKFILL_EPOCH_MS_MAX = 4_102_444_800_000; */ const SAFE_IDENTIFIER = /^[a-zA-Z_][a-zA-Z0-9_]*$/; -function assertSafeIdentifier(name: string): void { +/** + * [#14287] The second producer of the transport's unsafe-identifier refusal, + * carrying the identical ADR-0112 envelope — `INVALID_REQUEST` / 400 — through + * the one constructor that decides it ({@link unsafeIdentifierError}). One + * condition, one wire answer, whichever of the two helpers refused it. + * + * ## Measured: on THIS module's paths the envelope is defence in depth, not a + * ## wire answer — and saying so is the point + * + * Both callers flatten the throw by design: {@link probeRemoteCanonicalColumns} + * turns it into `{ error: message }` and {@link backfillRemoteCanonicalColumn} + * into `report.error`, because ADR-0053 D-B3 forbids a migration from taking a + * boot down. So `code` and `status` reach no response envelope from here today + * — only the MESSAGE survives, into the report a caller logs. The envelope is + * still worth carrying, for two reasons and no third: the two producers of one + * condition must not drift onto two spellings (the card's own "decide the code + * once" argument), and a future caller that propagates instead of reporting + * inherits the right answer rather than re-deriving one. + * + * ⛔ It is therefore NOT the wire-reachability evidence for this package's + * ledger provenance row — `RemoteTransport`'s DDL and `aggregate` positions + * are, and they answer over HTTP. Exported so the envelope has a real pin: no + * exported path can observe it, and an unobservable assertion is the phantom + * check `AGENTS.md` names rather than a test. + */ +export function assertSafeIdentifier(name: string): void { if (!SAFE_IDENTIFIER.test(name)) { - throw new Error(`remote canonical backfill: unsafe identifier rejected: "${name}"`); + throw unsafeIdentifierError(`remote canonical backfill: unsafe identifier rejected: "${name}"`); } } diff --git a/packages/drivers/driver-turso/src/remote-transport-aggregation-alias-quoting.test.ts b/packages/drivers/driver-turso/src/remote-transport-aggregation-alias-quoting.test.ts index 9db8cce7d6..d4a75cdaa5 100644 --- a/packages/drivers/driver-turso/src/remote-transport-aggregation-alias-quoting.test.ts +++ b/packages/drivers/driver-turso/src/remote-transport-aggregation-alias-quoting.test.ts @@ -216,6 +216,21 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat }); describe('regression controls — the positions this card did NOT touch', () => { + /** + * [#14287] Every control below asserts the ADR-0112 envelope beside the + * sentence, because "still refuses" was the half that was true all along + * and the half that was WRONG was invisible to it: these positions threw a + * bare `Error`, which `mapDataError` served as an opaque 500. #14287 gave + * the one producer `code: 'INVALID_REQUEST'` / `status: 400`. Asserting + * `message` alone here would let a regression to the bare `Error` land + * green through the very controls written to hold this shape. + */ + const envelopeOf = (err: unknown) => ({ + code: (err as { code?: unknown }).code, + status: (err as { status?: unknown }).status, + }); + const ENVELOPE = { code: 'INVALID_REQUEST', status: 400 }; + it('the `field` position still refuses an unsafe identifier, and sends nothing', async () => { // `SAFE_IDENTIFIER` is doing real work here: `field` becomes a column // REFERENCE, which is grammar. Escaping is the answer for a NAME only. @@ -233,6 +248,8 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat // itself safe is what makes this case reach the `field` check at all. expect(err.message).toContain('unsafe identifier rejected'); expect(err.message).toContain('amount"; DROP TABLE showcase_delivery; --'); + // [#14287] …and it is a 400 the caller can act on, not an opaque 500. + expect(envelopeOf(err)).toEqual(ENVELOPE); expect(seen).toEqual([]); }); @@ -248,6 +265,7 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat (e) => e as Error, ); expect(err.message).toContain('unsafe identifier rejected'); + expect(envelopeOf(err)).toEqual(ENVELOPE); expect(seen).toEqual([]); }); @@ -277,6 +295,10 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat ); expect(err.message).toContain('unsafe identifier rejected'); expect(err.message).toContain('showcase_delivery.region'); + // [#14287] The GATING is what this control holds; the ENVELOPE is what + // that card added to it. Both are pinned, so the open gating card cannot + // be mistaken for having landed. + expect(envelopeOf(err)).toEqual(ENVELOPE); expect(seen).toEqual([]); }); diff --git a/packages/drivers/driver-turso/src/remote-transport-unsafe-identifier-envelope.test.ts b/packages/drivers/driver-turso/src/remote-transport-unsafe-identifier-envelope.test.ts new file mode 100644 index 0000000000..94fe9e38ad --- /dev/null +++ b/packages/drivers/driver-turso/src/remote-transport-unsafe-identifier-envelope.test.ts @@ -0,0 +1,430 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14287] Every unsafe-identifier refusal on the Turso REMOTE transport + * carries the ADR-0112 envelope — `code: 'INVALID_REQUEST'`, `status: 400`. + * + * ## The defect + * + * `RemoteTransport.assertSafeIdentifier` is the ONE gate for every position + * where an identifier is INLINED into SQL (SQLite cannot bind one): `object`, + * `field` and the `groupBy` field / output key in `aggregate`, the table and + * column names in `syncSchema` / `syncSchemasBatch` / `buildCreateTableSQL`, + * and the index name and columns in `syncUniqueIndexes`. It threw a bare + * `Error` — no `code`, no `status` — so `mapDataError` + * (`packages/rest/src/error-response.ts`) reached none of its classifying + * branches, fell through to its sanitised terminal and served a **500**. A + * caller whose own identifier was refused was told the SERVER had faulted, and + * an SDK reading a 5xx retries a request that can never succeed. Same class as + * #11455 / #8931, one position over from #14113's alias half. + * + * ## What is asserted, and why not `toThrow()` + * + * A bare `expect(...).toThrow()` is blind in both directions here — it passed + * for the whole life of the defect, on the very positions this card is about. + * The refusal's wire identity is `code` + `status`, so those are what every + * case below asserts, beside the offending text (#6144). + * + * ## The `mapDataError` reading — the benefit survives the boundary + * + * Measured by READING `packages/rest/src/error-response.ts` at `d62f990a9` + * rather than assumed — and deliberately not by importing it: `@objectstack/rest` + * is not a dependency of this package, and a test reaching outside its own + * package is the `check:cross-package-test-inputs` shape. + * + * - `mapDataError` → `classifyDataError`. Nine `error?.code === '…'` branches + * run first; `'INVALID_REQUEST'` matches none of them, and none of them + * reads the MESSAGE, so nothing intercepts this refusal ahead of the + * declared-status passthrough. + * - `const declaredStatus = declaredHttpStatus(error)` reads `error.status` + * (or `statusCode`) and keeps it when `400 <= s < 600`. Our 400 qualifies. + * - `declaredServerFaultAnswer` is the 5xx arm — not taken at 400. + * - The 4xx arm returns `{ status: 400, body: { error: , + * ...thrownCodeFields(error, 400) } }`, and `thrownCodeFields` emits the + * closed ADR-0112 member verbatim when the code IS in the union. It is: + * `INVALID_REQUEST` is in `ERROR_CODE_LEDGER`, and this package now carries + * the provenance row naming itself as an emitter. + * + * So the two fields asserted here are exactly the two that door reads, and the + * 500 becomes a 400 carrying `INVALID_REQUEST`. + * + * ## The accept set is UNTOUCHED — pinned, not claimed in prose + * + * `SAFE_IDENTIFIER` and every refusal message are byte-identical to before. + * The last two `describe`s pin both halves: the same inputs are refused, and + * safe identifiers still compile and run. That is what makes this an envelope + * change rather than a gating change — the `groupBy` alias GATING question is + * a separate card and stays open. + * + * ## Reverse verification — direction predicted BEFORE it was run + * + * Restore the bare `throw new Error(...)` in both `assertSafeIdentifier` + * helpers (the pre-#14287 line): + * + * - every `code` / `status` assertion in this file goes RED on a COMPARISON + * (`undefined` vs `'INVALID_REQUEST'` / `400`), never by failing to throw. + * - the accept-set `describe` stays GREEN in BOTH of its directions, because + * neither the predicate nor the prose moved. That asymmetry is the claim: a + * change that flipped which inputs are refused would take it red too. + * + * MEASURED, and the second prediction was too coarse — recorded rather than + * quietly rewritten (19 failed / 13 passed across this file and the #14113 + * regression file): + * + * - the first prediction held exactly. All 19 failures are `toEqual` + * comparisons reading `{ code: undefined, status: undefined }`; not one is a + * "expected the transport to refuse …" — every input the gate refuses today + * is still refused with the ablated helper. + * - the accept-set describe went half red, because its REFUSED half asserts + * the envelope in the same `toEqual` as the message. Its ACCEPTED half — + * the direction that would catch a TIGHTENED gate — stayed green, and the + * red half's diff shows the `message` line UNMARKED between `-` and `+`: + * only `code` and `status` moved. `still refuses the empty string` is the + * worked example, in the PR body. + * + * That unmarked message line is the sharper form of what the prediction was + * reaching for: prose and predicate are provably untouched, so this is an + * envelope change and not a gating one. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { TursoDriver } from './turso-driver.js'; +import { + makeLibsqlSqliteStub, + asLibsqlClient, + type LibsqlSqliteStub, +} from './libsql-sqlite-stub.testkit.js'; +import { + UNSAFE_IDENTIFIER_CODE, + UNSAFE_IDENTIFIER_STATUS, + unsafeIdentifierError, +} from './remote-transport.js'; +import { + assertSafeIdentifier as assertSafeBackfillIdentifier, + backfillRemoteCanonicalColumn, + type RemoteBackfillClient, +} from './remote-canonical-backfill.js'; + +/** The two fields the REST door reads, plus the sentence. */ +const envelopeOf = (err: unknown) => ({ + code: (err as { code?: unknown }).code, + status: (err as { status?: unknown }).status, + message: (err as { message?: unknown }).message, +}); + +/** What every refusal in this file must carry. */ +const ENVELOPE = { code: 'INVALID_REQUEST', status: 400 }; + +/** + * Columns chosen so the accept-set half can EXECUTE: an underscore-leading + * name, a name with digits and a SCREAMING_SNAKE one are all inside + * `SAFE_IDENTIFIER`, and a group-by over a column that does not exist would + * fail for the wrong reason. + */ +const OBJECT_DEF = { + name: 'envelope_probe', + fields: { + id: { type: 'string' }, + region: { type: 'string' }, + amount: { type: 'number' }, + _region: { type: 'string' }, + region2: { type: 'string' }, + AMT_TOTAL: { type: 'number' }, + }, +}; + +/** One payload, so the cases differ only in WHERE it was placed. */ +const UNSAFE = 'amount"; DROP TABLE envelope_probe; --'; + +describe('[#14287] RemoteTransport — an unsafe identifier is a 400 INVALID_REQUEST, not an opaque 500', () => { + let stub: LibsqlSqliteStub; + let driver: TursoDriver; + + beforeAll(async () => { + stub = makeLibsqlSqliteStub(); + driver = new TursoDriver({ + url: 'libsql://envelope-probe.turso.io', + client: asLibsqlClient(stub), + }); + await driver.connect(); + // The mode this suite is about — the one with its own hand-written SQL. + expect(driver.transportMode).toBe('remote'); + await driver.syncSchema(OBJECT_DEF.name, OBJECT_DEF); + }); + + afterAll(async () => { + await driver.disconnect(); + stub.close(); + }); + + /** A driver on the same database that records every statement it sends. */ + const capturing = async () => { + const seen: string[] = []; + const spy = { + ...stub, + execute: async (stmt: unknown) => { + seen.push((stmt as { sql: string }).sql); + return stub.execute(stmt); + }, + }; + const t = new TursoDriver({ + url: 'libsql://envelope-probe.turso.io', + client: asLibsqlClient(spy), + }); + await t.connect(); + return { t, seen }; + }; + + /** Drive `fn` and hand back the thrown value — loudly, if it resolves. */ + const refusalFrom = async (what: string, fn: () => Promise): Promise => + fn().then( + () => { + throw new Error(`expected the transport to refuse the ${what}`); + }, + (e) => e, + ); + + describe('the QUERY positions — reached by an ordinary aggregate over HTTP', () => { + it('`object` refuses with the envelope, and sends nothing', async () => { + const { t, seen } = await capturing(); + const err = await refusalFrom('object', () => + t.aggregate(UNSAFE, { + object: UNSAFE, + aggregations: [{ function: 'count', alias: 'n' }], + } as never), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + // The OFFENDING TEXT, not merely the sentence (#6144). + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + // A refused identifier must not cost a round trip. + expect(seen).toEqual([]); + }); + + it('the aggregation `field` refuses with the envelope, and sends nothing', async () => { + const { t, seen } = await capturing(); + const err = await refusalFrom('aggregation field', () => + t.aggregate(OBJECT_DEF.name, { + object: OBJECT_DEF.name, + // The alias is safe on purpose, so the refusal can only be the field. + aggregations: [{ function: 'sum', field: UNSAFE, alias: 'n' }], + } as never), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + expect(seen).toEqual([]); + }); + + it('the `groupBy` FIELD refuses with the envelope, and sends nothing', async () => { + const { t, seen } = await capturing(); + const err = await refusalFrom('groupBy field', () => + t.aggregate(OBJECT_DEF.name, { + object: OBJECT_DEF.name, + groupBy: [UNSAFE], + aggregations: [{ function: 'count', alias: 'n' }], + } as never), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + expect(seen).toEqual([]); + }); + + it('the `groupBy` OUT KEY refuses with the envelope — gating unchanged, envelope added', async () => { + // ⚠️ Deliberate scope line. Whether this position should be ESCAPED + // rather than refused (as `driver-sql` escapes it post-#13714, and as + // #14113 changed the aggregation alias to do one position over) is a + // separate, still-open card: it MOVES the accept set. This case pins that + // the refusal itself is unchanged and only its envelope is new — so the + // day that card lands, this is the test that has to be rewritten + // deliberately rather than found red by surprise. + const { t, seen } = await capturing(); + const err = await refusalFrom('groupBy alias', () => + t.aggregate(OBJECT_DEF.name, { + object: OBJECT_DEF.name, + groupBy: [{ field: 'region', alias: 'envelope_probe.region' }], + aggregations: [{ function: 'count', alias: 'n' }], + } as never), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: 'RemoteTransport: unsafe identifier rejected: "envelope_probe.region"', + }); + expect(seen).toEqual([]); + }); + }); + + describe('the DDL positions — reached by publishing an object on a live server', () => { + it('`syncSchema` refuses an unsafe TABLE name with the envelope', async () => { + const { t } = await capturing(); + const err = await refusalFrom('table name', () => + t.syncSchema(UNSAFE, { name: UNSAFE, fields: { id: { type: 'string' } } }), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + }); + + it('`syncSchema` refuses an unsafe COLUMN name with the envelope — the CREATE leg', async () => { + const { t } = await capturing(); + const err = await refusalFrom('created column name', () => + t.syncSchema('envelope_create', { + name: 'envelope_create', + fields: { id: { type: 'string' }, [UNSAFE]: { type: 'string' } }, + }), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + }); + + it('`syncSchema` refuses an unsafe COLUMN name with the envelope — the ALTER leg', async () => { + // A second sync against a table that already exists takes the + // `ALTER TABLE … ADD COLUMN` limb, a different call site from the CREATE + // one above — the same gate reached by a different route. + const { t } = await capturing(); + const err = await refusalFrom('added column name', () => + t.syncSchema(OBJECT_DEF.name, { + ...OBJECT_DEF, + fields: { ...OBJECT_DEF.fields, [UNSAFE]: { type: 'string' } }, + }), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + }); + + it('`syncSchemasBatch` refuses an unsafe object name with the envelope', async () => { + const { t } = await capturing(); + const err = await refusalFrom('batched object name', () => + t.syncSchemasBatch([ + { object: OBJECT_DEF.name, schema: OBJECT_DEF }, + { object: UNSAFE, schema: { name: UNSAFE, fields: { id: { type: 'string' } } } }, + ]), + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${UNSAFE}"`, + }); + }); + }); + + describe('the BACKFILL producer — same envelope, and what it is worth there', () => { + it('the free `assertSafeIdentifier` throws the identical envelope', () => { + // Pinned DIRECTLY because nothing else can see it: both of that module's + // callers flatten the throw into a report by design (ADR-0053 D-B3), so + // an indirect assertion would pin the message and silently pin NOTHING + // about `code`/`status` — the phantom-check shape. + let thrown: unknown = new Error('expected the backfill helper to refuse'); + try { + assertSafeBackfillIdentifier(UNSAFE); + } catch (e) { + thrown = e; + } + expect(envelopeOf(thrown)).toEqual({ + ...ENVELOPE, + message: `remote canonical backfill: unsafe identifier rejected: "${UNSAFE}"`, + }); + }); + + it('a safe identifier still passes it untouched', () => { + expect(() => assertSafeBackfillIdentifier('created_at')).not.toThrow(); + }); + + it('the backfill still REPORTS rather than throws, and sends no statement', async () => { + // The observable half on that path, and the reason the envelope above is + // defence in depth rather than a wire answer: `report.error` is a STRING, + // so `code`/`status` are dropped here by design. A migration may not take + // a boot down. + const neverCalled: RemoteBackfillClient = { + execute: async () => { + throw new Error('a refused identifier must not reach the database'); + }, + batch: async () => { + throw new Error('a refused identifier must not reach the database'); + }, + }; + const report = await backfillRemoteCanonicalColumn( + neverCalled, + { table: UNSAFE, field: 'at', kind: 'datetime' }, + (_kind, columnSql) => columnSql, + ); + expect(report.error).toBe( + `remote canonical backfill: unsafe identifier rejected: "${UNSAFE}"`, + ); + expect(report.canonical).toBe(false); + expect(report).not.toHaveProperty('code'); + expect(report).not.toHaveProperty('status'); + }); + }); + + describe('the accept set is UNTOUCHED — this card added an envelope, not a gate', () => { + // Driven through the `groupBy` FIELD position, which has no `|| '*'` + // default in front of it, so an empty string reaches the gate rather than + // being read as "count everything". + const groupByField = async (name: string) => { + const { t, seen } = await capturing(); + const run = t.aggregate(OBJECT_DEF.name, { + object: OBJECT_DEF.name, + groupBy: [name], + aggregations: [{ function: 'count', alias: 'n' }], + } as never); + return { run, seen }; + }; + + it.each([ + ['a quote that would close the identifier', 'amount"'], + ['a dot — the qualified-reference spelling', 'cube.measure'], + ['a leading digit', '1region'], + ['a space', 'my column'], + ['a hyphen', 'my-column'], + ['the empty string', ''], + ['a stringified object, the #6212 shape', '[object Object]'], + ])('still refuses %s, now with the envelope', async (_why, name) => { + const { run } = await groupByField(name); + const err = await run.then( + () => { + throw new Error(`expected ${JSON.stringify(name)} to be refused`); + }, + (e) => e, + ); + expect(envelopeOf(err)).toEqual({ + ...ENVELOPE, + message: `RemoteTransport: unsafe identifier rejected: "${name}"`, + }); + }); + + it.each([ + ['a plain snake_case column', 'region'], + ['a leading underscore', '_region'], + ['digits after the first character', 'region2'], + ['SCREAMING_SNAKE', 'AMT_TOTAL'], + ])('still ACCEPTS %s — the gate did not tighten', async (_why, name) => { + const { run, seen } = await groupByField(name); + await run; + // The proof that nothing became stricter is a statement reaching the + // database and running, not merely the absence of a throw. + expect(seen).toEqual([ + `SELECT "${name}", count(*) AS "n" FROM "envelope_probe" GROUP BY "${name}"`, + ]); + }); + }); + + describe('the envelope constants are the wire vocabulary, not a local spelling', () => { + it('names the ledger-registered code and the 4xx status once, for both producers', () => { + // The ledger row under `@objectstack/driver-turso` registers exactly this + // string; a rename on either side has to move both, which is the pairing + // `check:error-code-provenance` reads. + expect(UNSAFE_IDENTIFIER_CODE).toBe('INVALID_REQUEST'); + expect(UNSAFE_IDENTIFIER_STATUS).toBe(400); + expect(envelopeOf(unsafeIdentifierError('x'))).toEqual({ ...ENVELOPE, message: 'x' }); + }); + }); +}); diff --git a/packages/drivers/driver-turso/src/remote-transport.ts b/packages/drivers/driver-turso/src/remote-transport.ts index 03cafbad40..2b8d1bc117 100644 --- a/packages/drivers/driver-turso/src/remote-transport.ts +++ b/packages/drivers/driver-turso/src/remote-transport.ts @@ -70,6 +70,73 @@ const BUILTIN_COLUMNS = new Set(['id', 'created_at', 'updated_at']); */ const SAFE_IDENTIFIER = /^[a-zA-Z_][a-zA-Z0-9_]*$/; +/** + * [#14287] The ADR-0112 envelope every unsafe-identifier refusal carries — + * decided ONCE, here, so the `object`, `field`, `outKey`, DDL, index-sync and + * backfill positions cannot answer a caller three different ways. + * + * ## What it replaces + * + * {@link RemoteTransport.assertSafeIdentifier} threw a bare `Error`: `code` + * and `status` were `undefined`, so `mapDataError` fell through to its + * terminal branch and served a sanitised **500** — the caller was told the + * server had faulted when in fact their own identifier was refused, and an + * SDK reading that status retries a request that can never succeed. It is the + * same un-enveloped-500 class {@link invalidFilterError} closed for the filter + * refusals one door over, at the last positions on this transport still + * carrying it. + * + * ## Why `INVALID_REQUEST` / 400 and not a new code + * + * Triage ruling, 2026-09-02: `INVALID_REQUEST` is ALREADY a member of the + * declared ADR-0112 vocabulary (`error-code-ledger.zod.ts`, registered by + * seven packages) for a request that is well-formed but not acceptable, which + * is exactly this condition — the identifier arrived intact and is refused on + * its SHAPE. An "unsafe identifier" code would be a permanent spelling the + * vocabulary does not need, so none is registered; only this package's + * provenance row is. + * + * ⛔ Not `INVALID_QUERY` / `INVALID_FILTER`: those name a query this transport + * could not COMPILE, and the DDL and index-sync positions reach this refusal + * with no query in sight. + * + * ## What it deliberately does NOT change + * + * The accept set — {@link SAFE_IDENTIFIER} is untouched, and so is every + * message. Exactly the identifiers refused before this change are refused + * after it, with byte-identical prose; `code` and `status` are the whole diff. + * The `groupBy` alias GATING question (whether that position should be escaped + * rather than refused, as `driver-sql` escapes it) is a separate card and + * stays open — this envelopes what is refused today, it does not move which + * inputs those are. + * + * `*_CODE` / `*_STATUS` is the shape `driver-memory`'s `UNIQUE_VIOLATION_CODE` + * established for a driver-side wire code, and one of the three the error-code + * provenance gate recognises — a registered code stamped through an unnamed + * constant is invisible to it. + */ +export const UNSAFE_IDENTIFIER_CODE = 'INVALID_REQUEST'; +/** @see {@link UNSAFE_IDENTIFIER_CODE} */ +export const UNSAFE_IDENTIFIER_STATUS = 400; + +/** + * [#14287] Build the refusal above. The ONE constructor both producers use — + * {@link RemoteTransport.assertSafeIdentifier} and the free + * `assertSafeIdentifier` in `remote-canonical-backfill.ts` — so the two cannot + * drift onto two spellings of one condition, which is the argument the card + * made for deciding the code at the helper rather than per position. + * + * The message stays the producer's own, verbatim: each names itself and quotes + * the offending text, and #6144's rule (quote the OFFENDING TEXT, not just the + * sentence) is what makes the refusal actionable at all. + */ +export function unsafeIdentifierError(message: string): Error { + const err = new Error(message) as Error & { code?: string; status?: number }; + err.code = UNSAFE_IDENTIFIER_CODE; + err.status = UNSAFE_IDENTIFIER_STATUS; + return err; +} + /** * Every filter operator `buildWhereSQL` compiles — the vocabulary this * transport CLAIMS to speak, and (since #1004) the exact set it accepts. @@ -1841,10 +1908,18 @@ export class RemoteTransport { /** * Validate that a string is a safe SQL identifier. * Prevents injection in DDL where parameterized queries are unsupported. + * + * [#14287] The ONE producer for every position that still gates an + * identifier — `object`, `field` and the `groupBy` `outKey` in + * {@link RemoteTransport.aggregate}, the table and column names in + * `syncSchema` / `syncSchemasBatch` / `buildCreateTableSQL`, and the index + * name and columns in `syncUniqueIndexes` — so the envelope is decided once + * for all of them. See {@link unsafeIdentifierError} for which envelope and + * why. The predicate and the message are unchanged. */ private assertSafeIdentifier(name: string): void { if (!SAFE_IDENTIFIER.test(name)) { - throw new Error(`RemoteTransport: unsafe identifier rejected: "${name}"`); + throw unsafeIdentifierError(`RemoteTransport: unsafe identifier rejected: "${name}"`); } } diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 614fb8b4c0..d72e9be969 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -773,6 +773,44 @@ export const ERROR_CODE_LEDGER = { // Producer: `packages/drivers/driver-sql/src/dialect-emission-refusal.ts`. 'SQL_DIALECT_EMISSION_UNSUPPORTED', ], + '@objectstack/driver-turso': [ + // [#14287] Provenance for the Turso REMOTE transport's unsafe-identifier + // refusal. Identifiers are INLINED into that transport's SQL (SQLite + // cannot bind one), so `object`, `field`, the `groupBy` output key, DDL + // table/column names and index names are all held to `SAFE_IDENTIFIER` — + // and every one of those positions threw a BARE `Error`, which + // `mapDataError` served as a sanitised 500. The caller was told the server + // faulted when their own identifier was refused. Stamped in ONE place for + // all of them — `unsafeIdentifierError` + // (`packages/drivers/driver-turso/src/remote-transport.ts`), `code: + // 'INVALID_REQUEST'` / `status: 400` via the exported + // `UNSAFE_IDENTIFIER_CODE` / `UNSAFE_IDENTIFIER_STATUS`, and read by the + // free `assertSafeIdentifier` in `remote-canonical-backfill.ts` too. + // + // Provenance ONLY — the eighth EMITTER of a code seven packages already + // register, so the union, its casing and every other package's rows are + // byte-unchanged. Per this file's header, a code emitted by several + // packages is listed once per emitting package. + // + // `INVALID_REQUEST` rather than a new "unsafe identifier" code by triage + // ruling (2026-09-02, #14287): the identifier arrived well-formed and is + // refused on its SHAPE, which is what this code already names for seven + // other emitters — and a code minted for one driver's guard would be a + // permanent vocabulary entry. ⛔ Not `INVALID_QUERY`: the DDL and + // index-sync positions reach the refusal with no query in sight. + // + // Wire-reachable by the test the "Retiring a code" section applies + // (#8035), on two independent paths on a server already serving HTTP: + // publishing an object calls `engine.syncObjectSchema` → + // `TursoDriver.syncSchema` → `RemoteTransport.syncSchema`'s table/column + // gate (the same argument `SQL_DIALECT_EMISSION_UNSUPPORTED` above makes), + // and an aggregate query reaches the `object` / `field` gate in + // `RemoteTransport.aggregate`. `resolveThrownHttpError` then puts the + // `code`/`status` on the envelope. The backfill module's twin producer is + // NOT the evidence — its callers flatten the throw into a report by + // design (ADR-0053 D-B3), which its own docblock records. + 'INVALID_REQUEST', + ], '@objectstack/spec': [ 'CONNECTOR_UPSTREAM_UNAVAILABLE', 'EXTERNAL_SCHEMA_MISMATCH',