From f515352b51381c2ef7f37c9988bcd94726e841e2 Mon Sep 17 00:00:00 2001 From: huyplb Date: Wed, 2 Sep 2026 23:47:12 -0600 Subject: [PATCH 1/3] fix(access): warn when the generated grant is only a template Reported from a live Oracle test: the read-only preset at database scope produced GRANT CREATE SESSION TO "FOXUSER"; -- Oracle grants object privileges per object. Repeat for each table: -- GRANT SELECT ON . TO "FOXUSER"; with no warning at all. Pasting that gives the account login and no read access, which is the opposite of what "read only" implies and is not visible from the SQL unless you notice the second block is commented out. The warning built for exactly this case was being silenced. Oracle and Db2 passed the table permissions as the `covers` argument on a statement that is entirely a comment, so `missedPermissionWarning` counted SELECT as granted. PostgreSQL's ownership note did the same for ALTER and DROP. A template cannot grant anything, so none of them claim coverage now. The advice was wrong for this case too. "Handle those through object ownership or an engine-specific privilege" is right for the PostgreSQL ownership case and useless for Oracle, where the answer is to narrow the scope. Where the engine has per-object grants and the miss is a table privilege, the warning now says to switch the scope to Tables and pick the objects. Co-Authored-By: Claude Opus 5 --- .../src/modules/access/access-sql-helpers.ts | 24 +++++- .../sql/src/modules/access/access-sql.test.ts | 73 +++++++++++++++++++ .../sql/src/providers/db2/db2.access-sql.ts | 5 +- .../src/providers/oracle/oracle.access-sql.ts | 7 +- .../providers/postgres/postgres.access-sql.ts | 6 +- 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/packages/sql/src/modules/access/access-sql-helpers.ts b/packages/sql/src/modules/access/access-sql-helpers.ts index 0c0d2935..09c30a42 100644 --- a/packages/sql/src/modules/access/access-sql-helpers.ts +++ b/packages/sql/src/modules/access/access-sql-helpers.ts @@ -268,10 +268,28 @@ export function missedPermissionWarning( const missed = request.permissions.filter((p) => !covered.has(p)); if (missed.length === 0) return null; const labels = missed.map((p) => describePermission(p).label.toLowerCase()); + const verb = request.action === 'grant' ? 'grants' : 'revokes'; + + // Advice the reader can act on beats a general statement of the limitation. + // Where the engine has per-object grants and the miss is a table privilege, + // narrowing the scope is the whole answer — and it is the case people + // actually hit, because "read only" on a database scope looks like it should + // work and produces a runnable CREATE SESSION plus a commented template. + const perObjectAvailable = + accessCapabilities(dialect).tableScope && + (request.scope.type === 'database' || request.scope.type === 'schema'); + const tableLevel = missed.every( + (p) => p === 'read' || p === 'insert' || p === 'update' || p === 'delete' + ); + const remedy = + perObjectAvailable && tableLevel + ? `Switch the scope to Tables and pick the objects to ${ + request.action === 'grant' ? 'grant' : 'revoke' + } on — ${dialect} has no schema-wide table grant.` + : `Handle ${missed.length === 1 ? 'that one' : 'those'} through object ownership or an engine-specific privilege.`; + return { level: 'caution', - message: `${dialect} cannot express ${listWords(labels)} at this scope — nothing below ${ - request.action === 'grant' ? 'grants' : 'revokes' - } it. Handle ${missed.length === 1 ? 'that one' : 'those'} through object ownership or an engine-specific privilege.`, + message: `${dialect} cannot express ${listWords(labels)} at this scope — nothing below ${verb} it. ${remedy}`, }; } diff --git a/packages/sql/src/modules/access/access-sql.test.ts b/packages/sql/src/modules/access/access-sql.test.ts index ad4900ce..f9886fb8 100644 --- a/packages/sql/src/modules/access/access-sql.test.ts +++ b/packages/sql/src/modules/access/access-sql.test.ts @@ -460,3 +460,76 @@ describe('access-sql registry', () => { expect(sqlOf(ok(req, 'yugabytedb'))).toBe(sqlOf(ok(req, 'postgres'))); }); }); + +describe('a commented-out template does not count as granting anything', () => { + // Reported from a live Oracle test: "read only" at database scope produced a + // runnable GRANT CREATE SESSION and a commented "repeat for each table" + // block, with no warning. Pasting it gave the account login and no read + // access at all. The template was passing itself off as covering SELECT, + // which silenced the warning built for exactly this case. + it('warns that Oracle read is not granted at database scope', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['connect', 'read'], + scope: { type: 'database', database: 'FREEPDB1' }, + }, + 'oracle' + ); + const sql = sqlOf(r); + expect(sql).toMatch(/GRANT CREATE SESSION/); + // The only runnable line is the session grant; the rest is a comment. + const runnable = sql + .split('\n') + .filter((l) => l.trim() && !l.trim().startsWith('--')); + expect(runnable).toHaveLength(1); + + const warned = r.warnings.map((w) => w.message).join(' '); + expect(warned).toMatch(/cannot express read data/i); + expect(warned).toMatch(/switch the scope to tables/i); + }); + + it('warns the same way on Db2', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['read'], + scope: { type: 'schema', schema: 'DEMO_A' }, + }, + 'db2' + ); + expect(r.warnings.map((w) => w.message).join(' ')).toMatch(/cannot express read data/i); + }); + + it('warns that PostgreSQL alter and drop are ownership, not grants', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['alter-object', 'drop-object'], + scope: { type: 'schema', schema: 'reporting' }, + }, + 'postgres' + ); + const warned = r.warnings.map((w) => w.message).join(' '); + expect(warned).toMatch(/cannot express/i); + // Not a table privilege, so the "pick tables" advice would be wrong here. + expect(warned).toMatch(/ownership/i); + }); + + it('still reports nothing when the grant really is expressible', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['read'], + scope: { type: 'tables', schema: 'DEMO_A', tables: ['ORDERS'] }, + }, + 'oracle' + ); + expect(sqlOf(r)).toMatch(/GRANT SELECT ON "DEMO_A"\."ORDERS"/); + expect(r.warnings.map((w) => w.message).join(' ')).not.toMatch(/cannot express/i); + }); +}); diff --git a/packages/sql/src/providers/db2/db2.access-sql.ts b/packages/sql/src/providers/db2/db2.access-sql.ts index 31d3f9fc..d7df00a7 100644 --- a/packages/sql/src/providers/db2/db2.access-sql.ts +++ b/packages/sql/src/providers/db2/db2.access-sql.ts @@ -103,8 +103,9 @@ function emitDb2(ctx: EmitCtx): void { add( `-- Db2 grants table privileges per object. Repeat for each table:\n-- ${verb} ${privs.join(', ')} ON TABLE ${qualifier(ident, schema)}.
${dir} ${grantee};`, 'Db2 has no schema-wide table grant. Select individual tables to generate runnable statements.', - highestRisk(permissions), - tablePerms + highestRisk(permissions) + // No `covers` — see the note in oracle.access-sql.ts. A commented-out + // template cannot grant anything, so it must not silence the warning. ); } diff --git a/packages/sql/src/providers/oracle/oracle.access-sql.ts b/packages/sql/src/providers/oracle/oracle.access-sql.ts index 7cf34aef..b00b395c 100644 --- a/packages/sql/src/providers/oracle/oracle.access-sql.ts +++ b/packages/sql/src/providers/oracle/oracle.access-sql.ts @@ -92,8 +92,11 @@ function emitOracle(ctx: EmitCtx): void { add( `-- Oracle grants object privileges per object. Repeat for each table:\n-- ${verb} ${privs.join(', ')} ON ${qualifier(ident, scopeSchema(scope))}.
${dir} ${grantee};`, 'Oracle has no schema-wide table grant; a schema is a user. Select individual tables to generate runnable statements.', - highestRisk(permissions), - tablePerms + highestRisk(permissions) + // No `covers`: this is a template, not a statement. Claiming it covered + // read/insert/update/delete told `missedPermissionWarning` the job was + // done, so the preview carried no warning at all — and a reader who + // pasted it granted CREATE SESSION and nothing else. ); } diff --git a/packages/sql/src/providers/postgres/postgres.access-sql.ts b/packages/sql/src/providers/postgres/postgres.access-sql.ts index 43b22a89..9efff4ae 100644 --- a/packages/sql/src/providers/postgres/postgres.access-sql.ts +++ b/packages/sql/src/providers/postgres/postgres.access-sql.ts @@ -207,8 +207,10 @@ function emitPostgres(ctx: EmitCtx): void { // the other way round. `-- PostgreSQL has no ALTER or DROP privilege: only an object's owner (or a\n-- member of its owning role) may alter or drop it. Consider:\n-- ${verb} ${dir} ${ident(request.principal.name)};`, 'PostgreSQL controls altering and dropping through ownership, not grants. Add the principal to the owning role instead.', - 'critical', - ownerPerms + 'critical' + // No `covers` — the statement is a comment. Ownership is the answer, and + // the reader needs that as a warning, not only as prose under a line + // that does nothing when run. ); } } From fb69433cfbd2f5d9ce0106f2cb95593760456d0b Mon Sep 17 00:00:00 2001 From: huyplb Date: Wed, 2 Sep 2026 23:57:38 -0600 Subject: [PATCH 2/3] feat(access): Db2 schema-wide grants, which it turns out it has MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing privilege grouping across the dialects turned up a wrong claim in this repo. `db2.access-sql.ts` said "Db2 has no schema-wide table grant — SELECTIN-style schema privileges exist only for a few verbs" and emitted a commented template instead of SQL. Verified against the Db2 12.1 container: SELECTIN, INSERTIN, UPDATEIN, DELETEIN, EXECUTEIN, ALTERIN, CREATEIN and DROPIN all grant on a schema and all record as Y in SYSCAT.SCHEMAAUTH. The generated statement was then executed as-is and recorded exactly the four privileges asked for. So schema scope now emits a real grant. Unlike PostgreSQL's ALL TABLES, Db2's form keeps covering objects created later, which the explanation says. The block sits above the empty-table-privileges return, because EXECUTEIN, ALTERIN and DROPIN have nothing to do with table privileges — with it below, an execute-only grant at schema scope emitted nothing at all. A test covers that case. Database scope keeps the template: there is no schema to name there. Co-Authored-By: Claude Opus 5 --- .../src/modules/access/access-sql-helpers.ts | 10 +++ .../sql/src/modules/access/access-sql.test.ts | 75 +++++++++++++++++-- .../sql/src/providers/db2/db2.access-sql.ts | 53 +++++++++++-- 3 files changed, 128 insertions(+), 10 deletions(-) diff --git a/packages/sql/src/modules/access/access-sql-helpers.ts b/packages/sql/src/modules/access/access-sql-helpers.ts index 09c30a42..316a09fe 100644 --- a/packages/sql/src/modules/access/access-sql-helpers.ts +++ b/packages/sql/src/modules/access/access-sql-helpers.ts @@ -247,6 +247,16 @@ const PRIV_VERB: Record = { CREATE: 'Create objects', ALTER: 'Alter objects', DROP: 'Drop objects', + // Db2's schema-wide forms, so the explanation reads in the same words as + // every other engine's rather than echoing the keyword back. + SELECTIN: 'Read', + INSERTIN: 'Insert', + UPDATEIN: 'Update', + DELETEIN: 'Delete', + EXECUTEIN: 'Run routines', + ALTERIN: 'Alter objects', + CREATEIN: 'Create objects', + DROPIN: 'Drop objects', }; /** "a", "a and b", "a, b and c" — one place, so every message reads the same. */ diff --git a/packages/sql/src/modules/access/access-sql.test.ts b/packages/sql/src/modules/access/access-sql.test.ts index f9886fb8..6f455697 100644 --- a/packages/sql/src/modules/access/access-sql.test.ts +++ b/packages/sql/src/modules/access/access-sql.test.ts @@ -195,13 +195,14 @@ describe('Db2 and Oracle', () => { expect(sqlOf(r)).toMatch(/GRANT SELECT ON TABLE "REPORTING"\."SALES" TO USER "REPORT_USER";/); }); - it('Db2 refuses to invent a schema-wide table grant', () => { + it('Db2 grants schema-wide with its …IN privileges', () => { + // This used to assert a commented template. Db2 11.1+ has real schema + // grants; verified against 12.1, they record in SYSCAT.SCHEMAAUTH. const r = ok( { principal: user, action: 'grant', permissions: ['read'], scope: { type: 'schema', schema: 'REPORTING' } }, 'db2' ); - expect(sqlOf(r)).toMatch(/^--/m); - expect(r.statements.some((s) => /no schema-wide table grant/i.test(s.explanation))).toBe(true); + expect(sqlOf(r)).toMatch(/GRANT SELECTIN ON SCHEMA "REPORTING" TO USER/); }); it('Oracle calls connecting CREATE SESSION', () => { @@ -490,13 +491,15 @@ describe('a commented-out template does not count as granting anything', () => { expect(warned).toMatch(/switch the scope to tables/i); }); - it('warns the same way on Db2', () => { + it('warns the same way on Db2 at database scope', () => { + // Schema scope now emits a real SELECTIN grant, so the template — and the + // warning — only remain where there is no schema to name. const r = ok( { principal: user, action: 'grant', permissions: ['read'], - scope: { type: 'schema', schema: 'DEMO_A' }, + scope: { type: 'database', database: 'FOXDB' }, }, 'db2' ); @@ -533,3 +536,65 @@ describe('a commented-out template does not count as granting anything', () => { expect(r.warnings.map((w) => w.message).join(' ')).not.toMatch(/cannot express/i); }); }); + +describe('Db2 schema-wide grants', () => { + // The emitter used to say Db2 had none and emit a commented template. + // Verified against Db2 12.1: these grant and record in SYSCAT.SCHEMAAUTH. + it('grants the …IN privileges instead of a template', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['read', 'insert', 'update', 'delete'], + scope: { type: 'schema', schema: 'DEMO_A' }, + }, + 'db2' + ); + expect(sqlOf(r)).toBe('GRANT SELECTIN, INSERTIN, UPDATEIN, DELETEIN ON SCHEMA "DEMO_A" TO USER "report_user";'); + // Nothing is missed, so no caution about an unexpressible privilege. + expect(r.warnings.map((w) => w.message).join(' ')).not.toMatch(/cannot express/i); + }); + + it('maps execute to EXECUTEIN once, not twice', () => { + // Both routine permissions map to the same keyword. + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['execute-function', 'execute-procedure'], + scope: { type: 'schema', schema: 'DEMO_A' }, + }, + 'db2' + ); + expect(sqlOf(r)).toMatch(/GRANT EXECUTEIN ON SCHEMA/); + expect(sqlOf(r)).not.toMatch(/EXECUTEIN, EXECUTEIN/); + }); + + it('revokes with the same keywords', () => { + const r = ok( + { + principal: user, + action: 'revoke', + permissions: ['read'], + scope: { type: 'schema', schema: 'DEMO_A' }, + }, + 'db2' + ); + expect(sqlOf(r)).toMatch(/REVOKE SELECTIN ON SCHEMA "DEMO_A" FROM USER/); + }); + + it('still explains itself at database scope, where there is no schema to name', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['read'], + scope: { type: 'database', database: 'FOXDB' }, + }, + 'db2' + ); + const runnable = sqlOf(r).split('\n').filter((l) => l.trim() && !l.trim().startsWith('--')); + expect(runnable).toHaveLength(0); + expect(r.warnings.map((w) => w.message).join(' ')).toMatch(/cannot express read data/i); + }); +}); diff --git a/packages/sql/src/providers/db2/db2.access-sql.ts b/packages/sql/src/providers/db2/db2.access-sql.ts index d7df00a7..ade6885e 100644 --- a/packages/sql/src/providers/db2/db2.access-sql.ts +++ b/packages/sql/src/providers/db2/db2.access-sql.ts @@ -6,7 +6,24 @@ * Db2 GRANT/REVOKE. Table privileges are per object; schema-wide table grants * are a comment, not a guess. */ -import { highestRisk } from '../../modules/access/intent.js'; +import { highestRisk, type AccessPermission } from '../../modules/access/intent.js'; + +/** + * Db2's schema-wide privileges, in the order they read best in a statement. + * + * `…IN ON SCHEMA` covers every object of the matching kind in the schema, and + * unlike PostgreSQL's ALL TABLES it keeps covering objects created later. + */ +const SCHEMA_IN_PRIVILEGE: readonly (readonly [AccessPermission, string])[] = [ + ['read', 'SELECTIN'], + ['insert', 'INSERTIN'], + ['update', 'UPDATEIN'], + ['delete', 'DELETEIN'], + ['execute-procedure', 'EXECUTEIN'], + ['execute-function', 'EXECUTEIN'], + ['alter-object', 'ALTERIN'], + ['drop-object', 'DROPIN'], +]; import { describePrivs, executePermissions, @@ -85,6 +102,32 @@ function emitDb2(ctx: EmitCtx): void { for (const pv of extra.privs) if (!privs.includes(pv)) privs.push(pv); tablePerms.push(...extra.covers); } + // Db2 *does* have schema-wide grants: the `…IN ON SCHEMA` privileges, which + // apply to every object of the right kind in the schema, present and future. + // This file previously said they "exist only for a few verbs" and emitted a + // commented template instead. Verified against Db2 12.1: SELECTIN, INSERTIN, + // UPDATEIN, DELETEIN, EXECUTEIN, ALTERIN, CREATEIN and DROPIN all grant and + // all record in SYSCAT.SCHEMAAUTH. + if (scope.type === 'schema' && schema) { + const inPrivs: string[] = []; + const covers: AccessPermission[] = []; + for (const [permission, priv] of SCHEMA_IN_PRIVILEGE) { + if (permissions.includes(permission)) { + if (!inPrivs.includes(priv)) inPrivs.push(priv); + covers.push(permission); + } + } + if (inPrivs.length > 0) { + add( + `${verb} ${inPrivs.join(', ')} ON SCHEMA ${ident(schema)} ${dir} ${grantee}${option};`, + `${describePrivs(inPrivs)} on every object in ${schema}, including objects added later. Db2 11.1 and later.`, + highestRisk(permissions), + covers + ); + return; + } + } + if (privs.length === 0) return; if (scope.type === 'tables') { @@ -98,11 +141,11 @@ function emitDb2(ctx: EmitCtx): void { } return; } - // Db2 has no "all tables in schema" grant — SELECTIN-style schema privileges - // exist only for a few verbs, so name the limitation instead of guessing. + // Database scope has no schema to name, so the per-object template is still + // the honest answer there. add( - `-- Db2 grants table privileges per object. Repeat for each table:\n-- ${verb} ${privs.join(', ')} ON TABLE ${qualifier(ident, schema)}.
${dir} ${grantee};`, - 'Db2 has no schema-wide table grant. Select individual tables to generate runnable statements.', + `-- Db2 grants these per object or per schema. Repeat for each table:\n-- ${verb} ${privs.join(', ')} ON TABLE ${qualifier(ident, schema)}.
${dir} ${grantee};`, + 'Choose a schema to use Db2’s schema-wide grants, or select individual tables.', highestRisk(permissions) // No `covers` — see the note in oracle.access-sql.ts. A commented-out // template cannot grant anything, so it must not silence the warning. From ff573818540e51a5be212589e3a80a85a08bba1e Mon Sep 17 00:00:00 2001 From: huyplb Date: Thu, 3 Sep 2026 00:10:16 -0600 Subject: [PATCH 3/3] fix(access): stop telling Db2 readers it has no schema-wide grant The remedy text was written when the only engines that missed a table privilege at a wide scope were ones with no schema-wide grant at all. The previous commit gave Db2 real SELECTIN-style schema grants, which made the advice contradict both the SQL and the template's own explanation: the warning said "db2 has no schema-wide table grant" while the emitter one scope over produced exactly that grant. The two situations are different and the message now tells them apart. An engine with schema-wide grants missing a privilege at *database* scope needs a schema named, so it says to choose one. Oracle genuinely has none, so there it still points at Tables. Co-Authored-By: Claude Opus 5 --- .../src/modules/access/access-sql-helpers.ts | 22 ++++++++++------- .../sql/src/modules/access/access-sql.test.ts | 24 ++++++++++++++++++- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/sql/src/modules/access/access-sql-helpers.ts b/packages/sql/src/modules/access/access-sql-helpers.ts index 316a09fe..8a87a346 100644 --- a/packages/sql/src/modules/access/access-sql-helpers.ts +++ b/packages/sql/src/modules/access/access-sql-helpers.ts @@ -285,18 +285,24 @@ export function missedPermissionWarning( // narrowing the scope is the whole answer — and it is the case people // actually hit, because "read only" on a database scope looks like it should // work and produces a runnable CREATE SESSION plus a commented template. + const caps = accessCapabilities(dialect); const perObjectAvailable = - accessCapabilities(dialect).tableScope && - (request.scope.type === 'database' || request.scope.type === 'schema'); + caps.tableScope && (request.scope.type === 'database' || request.scope.type === 'schema'); const tableLevel = missed.every( (p) => p === 'read' || p === 'insert' || p === 'update' || p === 'delete' ); - const remedy = - perObjectAvailable && tableLevel - ? `Switch the scope to Tables and pick the objects to ${ - request.action === 'grant' ? 'grant' : 'revoke' - } on — ${dialect} has no schema-wide table grant.` - : `Handle ${missed.length === 1 ? 'that one' : 'those'} through object ownership or an engine-specific privilege.`; + const act = request.action === 'grant' ? 'grant' : 'revoke'; + + // Two different situations, and telling them apart matters. Db2 and + // PostgreSQL do have schema-wide grants, so a miss at *database* scope means + // "name a schema", not "this engine cannot do it" — saying the latter + // contradicted the very statement the emitter had just produced. Oracle + // genuinely has none, and there the only way through is per object. + const remedy = !(perObjectAvailable && tableLevel) + ? `Handle ${missed.length === 1 ? 'that one' : 'those'} through object ownership or an engine-specific privilege.` + : caps.schemaScope && request.scope.type === 'database' + ? `Choose a schema — ${dialect} ${act}s these per schema — or switch the scope to Tables and pick the objects.` + : `Switch the scope to Tables and pick the objects to ${act} on — ${dialect} has no schema-wide table grant.`; return { level: 'caution', diff --git a/packages/sql/src/modules/access/access-sql.test.ts b/packages/sql/src/modules/access/access-sql.test.ts index 6f455697..28560d81 100644 --- a/packages/sql/src/modules/access/access-sql.test.ts +++ b/packages/sql/src/modules/access/access-sql.test.ts @@ -595,6 +595,28 @@ describe('Db2 schema-wide grants', () => { ); const runnable = sqlOf(r).split('\n').filter((l) => l.trim() && !l.trim().startsWith('--')); expect(runnable).toHaveLength(0); - expect(r.warnings.map((w) => w.message).join(' ')).toMatch(/cannot express read data/i); + + const warned = r.warnings.map((w) => w.message).join(' '); + expect(warned).toMatch(/cannot express read data/i); + // The advice has to match the engine. Db2 does have schema-wide grants, so + // telling the reader it does not would contradict the statement the + // emitter produces one scope over. + expect(warned).toMatch(/choose a schema/i); + expect(warned).not.toMatch(/no schema-wide table grant/i); + }); + + it('tells Oracle readers the opposite, because Oracle really has none', () => { + const r = ok( + { + principal: user, + action: 'grant', + permissions: ['read'], + scope: { type: 'database', database: 'FREEPDB1' }, + }, + 'oracle' + ); + const warned = r.warnings.map((w) => w.message).join(' '); + expect(warned).toMatch(/no schema-wide table grant/i); + expect(warned).not.toMatch(/choose a schema/i); }); });