diff --git a/packages/sql/src/modules/access/access-sql-helpers.ts b/packages/sql/src/modules/access/access-sql-helpers.ts index 0c0d2935..8a87a346 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. */ @@ -268,10 +278,34 @@ 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 caps = accessCapabilities(dialect); + const perObjectAvailable = + caps.tableScope && (request.scope.type === 'database' || request.scope.type === 'schema'); + const tableLevel = missed.every( + (p) => p === 'read' || p === 'insert' || p === 'update' || p === 'delete' + ); + 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', - 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..28560d81 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', () => { @@ -460,3 +461,162 @@ 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 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: 'database', database: 'FOXDB' }, + }, + '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); + }); +}); + +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); + + 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); + }); +}); diff --git a/packages/sql/src/providers/db2/db2.access-sql.ts b/packages/sql/src/providers/db2/db2.access-sql.ts index 31d3f9fc..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,13 +141,14 @@ 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.', - highestRisk(permissions), - tablePerms + `-- 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. ); } 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. ); } }