fix(access): warn when the generated grant is only a template - #350
Merged
Conversation
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 <schema>.<table> 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fb69433. Configure here.
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found testing Oracle by hand: the read-only preset at database scope produced
with no warning at all. Paste and run that and the account can log in and read nothing — the opposite of what "read only" implies, and not obvious from the SQL unless you spot that the second block is commented out.
Why nothing warned
missedPermissionWarningexists for exactly this. It was being silenced: Oracle and Db2 passed the table permissions as thecoversargument on a statement that is entirely a comment, so SELECT counted as granted. PostgreSQL's ownership note did the same for ALTER/DROP.A template cannot grant anything, so none of them claim coverage now.
The advice was also wrong
That 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, it now reads:
PostgreSQL still gets the ownership wording, since "pick tables" would be wrong there.
Verification
4 new tests, including one asserting the Oracle output has exactly one runnable line, and one confirming no warning appears when the grant really is expressible (Tables scope). A/B'd: restoring the false coverage fails the Oracle test.
Gates: 3292 tests, typecheck clean, 0 ESLint errors.
🤖 Generated with Claude Code
Note
Medium Risk
Changes permission SQL generation and warning behavior for Oracle, Db2, and PostgreSQL—areas where incorrect output could mislead operators about what was actually granted.
Overview
Fixes a dangerous gap where comment-only SQL templates were treated as covering table privileges, so presets like “read only” at database scope could show a runnable
CREATE SESSIONplus commentedSELECTlines with no caution. Oracle, Db2 (database scope), and PostgreSQL ownership hints no longer passcoversfor those templates, somissedPermissionWarningfires when the requested permission is not actually granted.missedPermissionWarningnow gives actionable remedies: pick tables when the engine has no schema-wide table grant (Oracle), choose a schema when it does (Db2/PostgreSQL at database scope), or ownership wording for non-table misses like PostgreSQL ALTER/DROP.Db2 schema scope now emits real
…IN ON SCHEMAgrants (SELECTIN,INSERTIN, etc.) instead of a commented per-table template, with human-readable labels for those keywords inPRIV_VERB. Tests cover Oracle/Db2 warnings, Db2 schema grants/revokes, and dedupedEXECUTEIN.Reviewed by Cursor Bugbot for commit ff57381. Bugbot is set up for automated code reviews on this repo. Configure here.