Converge privileges between a fresh install and an updated one

[jnasbyupgrade]

Add _cat_tools.privilege__normalize() and call it as the last statement of both the install script and the update accumulator, so a fresh CREATE EXTENSION and an ALTER EXTENSION UPDATE produce identical pg_init_privs, and therefore identical pg_dump output. Both paths now execute the same explicit GRANT/REVOKE statements, so the convergence holds by construction rather than by remembering to mirror one script in the other.

Resolves defect D1 of #93.

What diverged

A fresh install granted USAGE on the cat_tools types implicitly, via the ALTER DEFAULT PRIVILEGES IN SCHEMA cat_tools GRANT USAGE ON TYPES near the top of the install script. An ADP-granted privilege is never snapshotted into pg_init_privs. The update path got the same privilege from an explicit GRANT USAGE ON TYPE, which runs with creating_extension set and therefore is snapshotted. Identical runtime privileges, different dump.

Numbers

CREATE EXTENSION cat_tools vs. CREATE EXTENSION cat_tools VERSION '0.2.2' + ALTER EXTENSION cat_tools UPDATE, same cluster, same template:

	PG17 before	PG17 after	PG12 before	PG12 after
pg_init_privs rows, fresh	319	336	246	263
pg_init_privs rows, updated	324	336	251	263
pg_dump diff lines	44	8	44	8

The 8 remaining pg_dump diff lines are entirely the two \restrict/\unrestrict nonce hunks, which differ on every invocation. Per-classoid breakdown after, identical on both sides (PG17): pg_class 169, pg_namespace 5, pg_proc 148, pg_type 14.

The count rises rather than just equalising because normalization states privileges that were previously implicit: all 14 grantable types get a recorded row instead of 5, the new function adds one, and two _cat_tools relations that carried no explicit ACL (catalog_metadata, pg_depend_identity_v) now carry an owner-only one. No privilege changes hands — effective permissions are unchanged, and the pgTAP permission suite passes untouched.

bin/structural_diff compare reports IDENTICAL on both majors, before and after; it does not look at pg_init_privs, which is the gap #95 closes.

Design

ALTER DEFAULT PRIVILEGES stays. It documents what default privileges are in effect for our schemas, and removing it would break restore for already-updated databases, whose dumps carry no explicit GRANT. What changes is that nothing relies on it any more.

• Object list comes from the catalog — extension membership via pg_depend deptype = 'e' — so it stays correct as objects are added. The extension's own schema is not a member (CREATE EXTENSION creates it from schema = in the control file), so it is unioned in explicitly.
• REVOKE before GRANT is what makes this normalization rather than a top-up. An ALTER DEFAULT PRIVILEGES configured outside cat_tools reaches every object a fresh install creates and nothing an update script touches; without the revoke, whatever it added would be a permanent divergence.
• Scope is the two grantees cat_tools itself issues privileges to, PUBLIC and the usage role. Grants an administrator made to other roles are left alone — an update must not silently revoke them.
• PUBLIC keeps the USAGE PostgreSQL gives it on every type. Schema USAGE is what actually gates our types (this is what the permission test's type checks assert), so revoking would tighten the documented model rather than normalize it. USAGE is also the only type privilege, so nothing an external ADP does to PUBLIC on types can add anything.
• Routines get only the REVOKE ... FROM PUBLIC. Which routines the usage role may execute is a per-routine decision made at the creation site, where __cat_tools.create_function already issues REVOKE-then-GRANT.
• Tables are handled now, not left for later: _cat_tools.catalog_metadata is a real table with seeded data, and the relation branch covers r/p/v/m/f. Sequences are excluded, with a comment, because their privilege set is unlike a table's and cat_tools has none.
• Column-level privileges (pg_attribute.attacl) are deliberately not handled. cat_tools grants nothing at column granularity, and ALTER DEFAULT PRIVILEGES has no column granularity either, so the drift this exists to prevent cannot reach them. Since a table-level REVOKE never subsumes column-level privileges, the function's comment records that granting at column granularity in future means extending it at the same time.

Constraints from #88

All three honoured:

• Does not require superuser. Verified directly, not just reasoned: with superuser = false added to the control file, a genuine non-superuser login role created the extension, normalization ran, and the resulting database has the same 336 pg_init_privs rows as the superuser fresh install.
• Tolerates the usage role being absent rather than erroring. Verified against the unmodified function by renaming the role away inside a transaction: it logged role cat_tools__usage does not exist; normalizing PUBLIC privileges only at DEBUG, completed, applied the PUBLIC revokes, and left existing grants untouched.
• The grantee is named in one place, the c_usage_role constant, so whatever Install script requires CREATEROLE, blocking non-superuser install even with superuser = false #88 settles on is a single-point change. Nothing else in the function names a role.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: e528463a-9353-48d7-83c3-9a62bf795c8e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude finished @jnasbyupgrade's task in 7m 58s —— View job

---

Code review in progress

Task: Run /code-review on this PR and post findings as inline comments (per repo custom instructions).

• Gather context (PR description, changed files, CLAUDE.md conventions)
• Review _cat_tools.privilege__normalize() and its call sites in sql/cat_tools.sql.in / sql/cat_tools--0.3.0--stable.sql.in
• Cross-check against __cat_tools.create_function, upgrade chain, HISTORY.asc, test/build/expected/build.out
• Verify findings and post inline PR comments
• Post final summary

Review is running now, will update shortly.

[jnasbyupgrade]

This is a shitload of code to duplicate. Eventually it'd be good for us to make something like this an actual cat_tools feature, but in the meantime lets at least have an internal view.

[jnasbyupgrade]

Extracted the object enumeration into _cat_tools.privilege_target_v (1497030): one row per object cat_tools issues privileges on, plus what it intends that object to have (object_kind, object_identity, revoke_public, revoke_role, grant_privilege). privilege__normalize() is now just a loop over it, so the per-kind policy is a set-returning thing you can inspect with a SELECT rather than something buried in plpgsql.

Both files still have to define both the view and the function — an install sitting at 0.3.0 has neither — but what is duplicated is now the declarative list, and the function around it is ~30 lines instead of ~110. The two copies remain byte-identical. Making this a real cat_tools feature is left to separate work.

[jnasbyupgrade]

26 lines of comment here is overkill. Again, IMPORTANT details about individual steps/things belong as comments with the relevant code. A header comment like this should only hit high-level things, and only if needed.

[jnasbyupgrade]

Header is down to 9 lines: what this achieves, plus the one non-obvious reason it is needed (implicit ALTER DEFAULT PRIVILEGES grants never reach pg_init_privs, explicit ones do). Everything else moved to the code it explains — the REVOKE-before-GRANT rationale sits on the REVOKE in the loop, the column-ACL exclusion sits on the TABLE branch it would apply to, and the grantee scope sits on the view that defines it. Same trim applied to the copy in sql/cat_tools.sql.in.

[github-actions]

Claude finished @jnasbyupgrade's task in 6m 2s —— View job

---

Code review in progress

Task: Run /code-review on this PR and post findings as inline comments.

• Gather context (PR description, changed files, CLAUDE.md conventions)
• Kick off code-review skill (finder agents scanning line-by-line, removed-behavior, cross-file, reuse, simplification, efficiency, altitude, conventions)
• Dedupe and verify findings
• Post findings as inline PR comments
• Post final summary

Review is running now, will update shortly.

[jnasbyupgrade]

@claude it looks like your runs are dying...

[claude]

Claude finished @jnasbyupgrade's task in 2m 28s —— View job

---

Investigation complete

• Gather context (PR description, prior run links)
• Inspect the two prior Claude workflow runs that died mid-review
• Identify root cause
• Report findings

Both prior runs (34648962488, 34652912518) didn't crash — the Claude Code Action's own SDK call reports "is_error": false / "subtype": "success" in both. They just never got anywhere:

Run	num_turns	permission_denials_count	Final comment content
34648962488	11	20	stuck at "Review is running now..."
34652912518	6	23	stuck at "Review is running now..."

Root cause: .github/workflows/claude-code-review.yml runs the code-review@claude-code-plugins plugin via claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'. That flag is additive on top of the action's own baseline tool set for agent mode, but the resulting allow‑list (confirmed from the SDK options dump in both logs) is only:

Glob, Grep, LS, Read,
mcp__github_comment__update_claude_comment,
mcp__github_ci__get_ci_status, mcp__github_ci__get_workflow_run_details, mcp__github_ci__download_job_log,
mcp__github_inline_comment__create_inline_comment,
Bash(git add:*), Bash(git commit:*), Bash(<git-push.sh>:*), Bash(git rm:*)

There's no Task/subagent tool in that list. The /code-review command's own workflow (as its progress checklist in this same PR shows: "Kick off code-review skill (finder agents scanning line-by-line, removed-behavior, cross-file, reuse, simplification, efficiency, altitude, conventions)") spawns multiple parallel finder subagents to do the actual review — that requires the Task tool. Every attempt to spawn one is denied, which matches the 20–23 permission_denials_count piling up over only 6–11 turns. Eventually the run just stops (SDK returns cleanly) without the plugin ever finishing a pass or posting real findings — the tracked comment is left on its last real update ("Review is running now, will update shortly"), and only the automatic "update comment with job link" step (which isn't a Claude tool call) touches it after that.

Likely fix: add Task to the --allowedTools list in claude_args for the claude-review job, e.g. --allowedTools mcp__github_inline_comment__create_inline_comment,Task. I can't make that change myself — edits to .github/workflows/* are outside what I'm permitted to modify from here — so this needs a human edit + merge to master (per this repo's own CLAUDE.md, a pull_request_target workflow file change can only be verified on the next PR after merging, not the PR that introduces it).

Note this is unrelated to the actual PR content (_cat_tools.privilege__normalize()); it's purely a CI plumbing issue in the review-bot workflow.