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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
35 changes: 35 additions & 0 deletions .changeset/turso-groupby-alias-escaped.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
---
"@objectstack/driver-turso": patch
---

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)

`RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly two
positions. #14113 moved the aggregation alias to escaping and deliberately left
the groupBy alias (`GroupByNodeSchema.alias`, reaching the driver as
`g.alias ?? g.field`) on `assertSafeIdentifier`, because that position carried a
landed pin asserting the refusal. So a groupBy alias that was not a bare
`[A-Za-z_][A-Za-z0-9_]*` — `'Region Name'`, `'deal.stage_bucket'` — was refused
on this face while the in-memory, MongoDB and (post-#13714) SQL faces all
project it verbatim: one query, two answers, decided by a connection string.

The groupBy select site now emits `"<field>" AS <aliasIdentifierSql(outKey)>`,
the same quote-doubling escape the aggregation alias beside it already uses, so
both output-name positions of the method agree with `driver-sql`. The `field`
position keeps `assertSafeIdentifier` — a column REFERENCE is grammar and a
qualified one is legitimate, so it must be validated; an output NAME is one
name by definition and is quoted and escaped. `outKey === field` still emits the
alias-less `"<field>"`, byte-identical to before.

The #6401 pin that asserted the refusal is rewritten in place, on the same
input, to assert what the transport now emits — the recorded, non-silent
reversal the card asked for rather than a rider on someone else's change. The
escaped alias is pinned against a real SQLite-backed libsql stub as well as on
the captured statement, because only executing it tells "escaped" apart from
"broke out".

No accept set moves at the contract: `GroupByNodeSchema.alias` already declares
this key and the spec already admits these names. What moves is this driver's
accept set, toward the contract the other three faces already implement —
declared = enforced, restored. The refusal envelope for the positions that stay
gated (#14287, `INVALID_REQUEST` / 400) is untouched.
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,37 +269,44 @@ describe('[#14113] RemoteTransport — the aggregation alias is escaped, not gat
expect(seen).toEqual([]);
});

it('the groupBy alias position is UNCHANGED — still refused, and that is a separate card', async () => {
// ⚠️ Deliberate scope line, pinned so it cannot drift silently. The
// groupBy `alias` is the same class of thing (an output NAME) and
// `driver-sql` escapes it post-#13714 (`aliasIdentifierSql` at its
// groupBy select site), so this face diverges there too — but that
// position carries a LANDED pin (#6401, `remote-transport-groupby-node`)
// asserting the refusal, so reversing it is a judgement this card was not
// dispatched to make. Filed separately rather than patched inline; this
// control records the state it was left in.
it('[#14235] the groupBy alias position is now ESCAPED TOO — the scope line this card left is closed', async () => {
// ⚠️ This case REPLACES the deliberate scope line #14113 left here, which
// asserted this same input is REFUSED and said of itself: "Filed
// separately rather than patched inline; this control records the state
// it was left in." #14235 is the card it was waiting for. It reached the
// same reference-versus-name line by the same reasoning — an output NAME
// is quoted and escaped, a column REFERENCE is validated — so BOTH
// output-name positions of `aggregate` now route through
// `aliasIdentifierSql`, and this face agrees with `driver-sql` (which has
// routed both since #13714) on the whole method rather than one loop.
//
// It is also NOT on the reproducing path: `ObjectQLStrategy` resolves a
// dimension to a bare column name, so no analytics query sends a dotted
// groupBy alias.
// What is pinned here is what the two positions do TOGETHER in one
// statement: a dotted dimension alias beside a dotted measure alias, both
// quoted, neither refused. That is the assertion #14113's control could
// not make while it was holding the scope line.
const { t, seen } = await capturing();
const err = await t
.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never)
.then(
() => { throw new Error('expected the groupBy alias to still be refused') },
(e) => e as Error,
);
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([]);
const rows = await t.aggregate(DELIVERY_OBJECT.name, {
object: DELIVERY_OBJECT.name,
groupBy: [{ field: 'region', alias: 'showcase_delivery.region' }],
aggregations: [{ function: 'count', alias: 'showcase_delivery.count' }],
} as never);
expect(seen).toEqual([
'SELECT "region" AS "showcase_delivery.region", count(*) AS "showcase_delivery.count" ' +
'FROM "showcase_delivery" GROUP BY "region"',
]);
// ⛔ Neither dot may become a qualified reference: both stay INSIDE their
// own quotes.
expect(seen[0]).not.toContain('"showcase_delivery"."region"');
expect(seen[0]).not.toContain('"showcase_delivery"."count"');
// Executed, not merely emitted — the values come back under the caller's
// own keys, and GROUP BY still keys on the FIELD.
const byRegion = Object.fromEntries(
(rows as Array<Record<string, unknown>>).map((r) => [
r['showcase_delivery.region'],
r['showcase_delivery.count'],
]),
);
expect(byRegion).toEqual({ west: 2, east: 1 });
});

it('the default alias is byte-identical to what it was before', async () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -63,7 +63,79 @@
* reversed — the condition it named ("only one face would read it") is what
* stopped being true.
*
* # Reverse verification — direction predicted BEFORE it was run, per case
* # `alias` is ESCAPED, not gated — as of #14235
*
* `RemoteTransport.aggregate` emits a caller-supplied output NAME in exactly
* two positions. #13714 routed BOTH of `driver-sql`'s through
* `SqlDriver.aliasIdentifierSql`; #14113 moved this transport's AGGREGATION
* alias to the same escaping and deliberately left the groupBy alias alone,
* because the groupBy position carried the landed #6401 pin below asserting the
* refusal. #14235 is the card that reverses that pin on the record: an
* output-column key is a NAME — quoted and escaped — and a column REFERENCE is
* grammar, so `field` keeps `assertSafeIdentifier` and `outKey` no longer has
* it. `GroupByNodeSchema.alias` is the same class of key as
* `AggregationNodeSchema.alias`, and the in-memory face projects
* `g.alias ?? g.field` verbatim, so this face was the last one refusing names
* the contract permits — with a bare `Error` before #14287, an opaque 500 after
* `mapDataError`, and a 400 `INVALID_REQUEST` since.
*
* ⚠️ The `[#6401] refuses an unsafe identifier in \`alias\`` case named in the
* #6212 record below **no longer exists** — it is replaced in place by
* `[#14235] ESCAPES an alias that would close the quoting`, on the same input.
* The record is left as it was measured rather than rewritten to match today's
* cases: it is the #6212 ablation, not this one.
*
* ## Reverse verification (#14235) — direction predicted BEFORE it was run
*
* Restore the two pre-#14235 lines at the groupBy select site —
* `this.assertSafeIdentifier(outKey)` above the push, and
* `` `"${field}" AS "${outKey}"` `` as the aliased emission:
*
* - the two capture cases (`ESCAPES an alias that would close the quoting`,
* `a dotted or spaced alias round-trips`) go RED by THROWING inside the call
* — `unsafe identifier rejected: "bucket"; DROP TABLE deal; --"` /
* `"Region Name"` — not on a comparison.
* - both executing cases go RED the same way, inside `driver.aggregate`.
* - the `field`-position control (`still refuses an unsafe identifier inside a
* structured entry`) stays GREEN — untouched by this change, and that is
* exactly what it is here to hold.
* - `projects \`alias\` as the column name` and `an alias equal to the field
* name emits no self-rename` stay GREEN: `bucket` and `stage` pass
* `SAFE_IDENTIFIER` either way, so they pin byte-identical emission across
* the change.
* - every date-bucket, parity and string-form case stays GREEN.
*
* MEASURED, case for case as predicted — **4 failed / 14 passed of 18**:
*
* ```
* ESCAPES an alias that would close Error: RemoteTransport: unsafe identifier
* the quoting rejected: "bucket"; DROP TABLE deal; --"
* a dotted or spaced alias …rejected: "Region Name", then
* round-trips …rejected: "deal.stage_bucket"
* a dotted alias comes back under …rejected: "deal.stage_bucket"
* the result key (executing)
* an alias that tries to close the …rejected: "bucket"; DROP TABLE deal; --"
* quoting (executing)
* ── green ──
* still refuses an unsafe identifier inside a structured entry (the CONTROL)
* projects `alias` as the column name · no self-rename for alias === field
* the string-form control · all five date-bucket refusals · both parity cases
* ```
*
* Not one failure came through a comparison: the restored gate throws before
* any SQL is built, which is why the guard could not simply be deleted from the
* `field` position and why the control above is the case that holds it.
*
* ⚠️ No `dist/` leg applies to this ablation. The suite imports the mutated
* unit as `./remote-transport.js` — a RELATIVE, in-package specifier that
* vitest resolves to `src/remote-transport.ts`, not through the package's
* `exports` — and `vitest.config.ts` declares no alias (it sets
* `disableConsoleIntercept` and nothing else). The mutation was proved on disk
* by grep counts on both the injected and the removed text and by
* `git hash-object` against the HEAD blob, and the restore by the same hash
* matching again plus an empty `git diff HEAD`.
*
* # Reverse verification (#6212) — direction predicted BEFORE it was run, per case
*
* Restore `const groupBy: string[] = Array.isArray(query?.groupBy) ? … : []`
* (with a cast, since the narrowed signature no longer permits it):
Expand DownExpand Up@@ -115,9 +187,11 @@
* rather than merely that something was thrown.
*/

import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest';
import { SqlDriver } from '@objectstack/driver-sql';
import { RemoteTransport } from './remote-transport.js';
import { TursoDriver } from './turso-driver.js';
import { makeLibsqlSqliteStub, asLibsqlClient, type LibsqlSqliteStub } from './libsql-sqlite-stub.testkit.js';

interface WireBearingError extends Error {
code?: string;
Expand DownExpand Up@@ -218,25 +292,59 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(calls[0].sql).toBe('SELECT "stage", count("stage") AS "n" FROM "deal" GROUP BY "stage"');
});

it('[#6401] refuses an unsafe identifier in `alias`, not only in `field`', async () => {
// The alias is caller-supplied text that now reaches the statement as a
// quoted identifier, so it needs the gate `field` already has. The
// assertion names the OFFENDING TEXT, not just the sentence (#6144): a
// `field` that is itself safe is what makes this case reach the alias
// check at all.
it('[#14235] ESCAPES an alias that would close the quoting — one inert name, one statement', async () => {
// ⚠️ This case REPLACES the #6401 pin that asserted the same input is
// REFUSED (`unsafe identifier rejected`, naming the offending text). That
// pin was the deliberate call when the alias was newly read here, and
// reversing it is a recorded, non-silent reversal rather than a rider:
// #13714 routed BOTH of driver-sql's output-name positions through
// `aliasIdentifierSql`, #14113 moved this transport's aggregation alias
// to escaping, and #14235 brings the second output-name position of the
// same method to the same line. An output-column key is a NAME: it is
// quoted and escaped, never gated.
//
// The old assertion is REPLACED, not dropped — the exact input it named
// is the input here, and what is pinned now is the statement it produces.
const { t, calls } = transportWithCapturingClient();
const err = await t
.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
})
.then(
() => { throw new Error('expected the transport to refuse an unsafe alias'); },
(e) => e as Error,
const rows = await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias: 'bucket"; DROP TABLE deal; --' }],
aggregations: [{ function: 'count', alias: 'n' }],
});
expect(rows).toEqual([]);
// The whole payload is ONE column name, the quote doubled — the standard
// escape inside a quoted SQL identifier. It is data, never grammar.
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
'SELECT "stage" AS "bucket""; DROP TABLE deal; --", count(*) AS "n" FROM "deal" GROUP BY "stage"',
);
// ⛔ ONE statement, not two: a payload that had broken out of its quoting
// would appear as a second one here. The executing block at the foot of
// this file proves the same thing against a real database, which is the
// only instrument that tells "escaped" apart from "broke out".
expect(calls[0].sql.match(/SELECT/g)).toHaveLength(1);
// And the grouping key is still the FIELD, exactly as for a bare alias.
expect(calls[0].sql.endsWith('GROUP BY "stage"')).toBe(true);
});

it('[#14235] a dotted or spaced alias round-trips as one quoted output column', async () => {
// The reachable population the card measured: a caller writing
// `groupBy: [{ field, alias }]` through the Query Protocol directly.
// Both spellings work on the in-memory, MongoDB and SQL faces and were an
// opaque 500 on this one — no `code`, no `status`, out of `mapDataError`.
for (const alias of ['Region Name', 'deal.stage_bucket']) {
const { t, calls } = transportWithCapturingClient();
await t.aggregate('deal', {
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', field: 'stage', alias: 'n' }],
});
expect(calls).toHaveLength(1);
expect(calls[0].sql).toBe(
`SELECT "stage" AS "${alias}", count("stage") AS "n" FROM "deal" GROUP BY "stage"`,
);
expect(err.message).toContain('unsafe identifier rejected');
expect(err.message).toContain('bucket"; DROP TABLE deal; --');
expect(calls).toEqual([]);
// ⛔ The dot stays INSIDE the quotes. The failure this rules out is a
// face that reads an output NAME as a qualified REFERENCE.
expect(calls[0].sql).not.toContain('"deal"."stage_bucket"');
}
});

it('still refuses an unsafe identifier inside a structured entry', async () => {
Expand DownExpand Up@@ -394,4 +502,70 @@ describe('[#6212] RemoteTransport compiles the GroupByNode union', () => {
expect(err.message).toContain("dialect 'better-sqlite3'");
});
});

// ── [#14235] Executed, not merely emitted ─────────────────────────────────

/**
* ⚠️ Only EXECUTING the statement tells "escaped" apart from "broke out" —
* #14113's reasoning one position over, and the reason its pin is backed by a
* real database rather than a captured string. A capture assertion alone
* passes on an alias that terminates the quoting, because the text still
* *looks* like a select list. libsql IS SQLite, so the stub runs exactly what
* this transport emits: an alias that escaped its quoting is a syntax error
* (or a second statement better-sqlite3 refuses to prepare), and reading the
* value back under the literal alias is the proof that it did not.
*/
describe('[#14235] the escaped groupBy alias is inert against a real database', () => {
const DEAL = {
name: 'deal',
fields: { id: { type: 'string' }, stage: { type: 'string' }, amount: { type: 'number' } },
};
let driver: TursoDriver;
let stub: LibsqlSqliteStub;

beforeAll(async () => {
stub = makeLibsqlSqliteStub();
driver = new TursoDriver({ url: 'libsql://groupby-alias.turso.io', client: asLibsqlClient(stub) });
await driver.connect();
// The mode this block is about — the one with its own hand-written SQL.
expect(driver.transportMode).toBe('remote');
await driver.syncSchema(DEAL.name, DEAL);
for (const row of [
{ id: '1', stage: 'won', amount: 10 },
{ id: '2', stage: 'won', amount: 20 },
{ id: '3', stage: 'lost', amount: 30 },
]) {
await driver.create(DEAL.name, { ...row });
}
});

afterAll(async () => {
await driver.disconnect();
stub.close();
});

it('a dotted alias comes back under the result key the caller asked for, on rows', async () => {
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias: 'deal.stage_bucket' }],
aggregations: [{ function: 'sum', field: 'amount', alias: 'deal.total' }],
} as never)) as Array<Record<string, unknown>>;
const byBucket = Object.fromEntries(rows.map((r) => [r['deal.stage_bucket'], r['deal.total']]));
// A dot is inert inside a quoted identifier — the whole claim of the card.
expect(byBucket).toEqual({ won: 30, lost: 30 });
});

it('an alias that tries to close the quoting and append a statement leaves the table standing', async () => {
const alias = 'bucket"; DROP TABLE deal; --';
const rows = (await driver.aggregate(DEAL.name, {
object: DEAL.name,
groupBy: [{ field: 'stage', alias }],
aggregations: [{ function: 'count', alias: 'n' }],
} as never)) as Array<Record<string, unknown>>;
// The payload came back as a COLUMN NAME — it was data, never grammar.
expect(rows.map((r) => r[alias]).sort()).toEqual(['lost', 'won']);
// And the table it named is still there, with every row.
expect(stub.raw.prepare('select count(*) as c from deal').all()).toEqual([{ c: 3 }]);
});
});
});
Loading
Loading