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
40 changes: 37 additions & 3 deletions packages/sql/src/modules/access/access-sql-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@ const PRIV_VERB: Record<string, string> = {
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',
Comment thread
huyplb marked this conversation as resolved.
};

/** "a", "a and b", "a, b and c" — one place, so every message reads the same. */
Expand All @@ -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}`,
};
}
166 changes: 163 additions & 3 deletions packages/sql/src/modules/access/access-sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
58 changes: 51 additions & 7 deletions packages/sql/src/providers/db2/db2.access-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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') {
Expand All @@ -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)}.<table> ${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)}.<table> ${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.
);
}

Expand Down
7 changes: 5 additions & 2 deletions packages/sql/src/providers/oracle/oracle.access-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))}.<table> ${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.
);
}

Expand Down
6 changes: 4 additions & 2 deletions packages/sql/src/providers/postgres/postgres.access-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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} <owning_role> ${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.
);
}
}
Expand Down
Loading