Skip to content

Fix supautils_hook to use the correct role name in ALTER ROLE#185

Merged
steve-chavez merged 2 commits intomasterfrom
nickb/fix_supautils_hook_privileged_rolespec_handling
Apr 2, 2026
Merged

Fix supautils_hook to use the correct role name in ALTER ROLE#185
steve-chavez merged 2 commits intomasterfrom
nickb/fix_supautils_hook_privileged_rolespec_handling

Conversation

@pgnickb
Copy link
Copy Markdown
Contributor

@pgnickb pgnickb commented Apr 1, 2026

This Fixes a bug in the supautils_hook that causes a crash. See #108 for more details.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 1, 2026

Pull Request Test Coverage Report for Build 23877545812

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 88.737%

Totals Coverage Status
Change from base Build 23755934970: 0.08%
Covered Lines: 1103
Relevant Lines: 1243

💛 - Coveralls

@pgnickb pgnickb force-pushed the nickb/fix_supautils_hook_privileged_rolespec_handling branch from 7186a3f to 90fc1e3 Compare April 1, 2026 11:30
@pgnickb pgnickb marked this pull request as ready for review April 1, 2026 11:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a crash in supautils_hook when ALTER ROLE targets current_user / current_role while running as privileged_role, by resolving the role spec to an actual role name before checking privileged-role restrictions.

Changes:

  • Use get_rolespec_name(stmt->role) (via role_name) when calling is_role_privileged(...) for ALTER ROLE statements.
  • Add a regression test to ensure ALTER ROLE current_user / ALTER ROLE current_role does not crash under privileged_role.
  • Add an alternate expected output file (privileged_role_1.out) to accommodate differing server behavior for current_role.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/supautils.c Prevents NULL role-name dereference by using the resolved RoleSpec name for privileged-role checks.
test/sql/privileged_role.sql Adds regression coverage for ALTER ROLE current_user/current_role under privileged_role.
test/expected/privileged_role.out Updates expected output to include the new regression assertions.
test/expected/privileged_role_1.out Adds alternate expected output variant for environments where current_role yields a syntax error.

Comment thread test/expected/privileged_role.out.in
@steve-chavez steve-chavez force-pushed the nickb/fix_supautils_hook_privileged_rolespec_handling branch from 9263328 to d80a2bf Compare April 2, 2026 00:20
It has a different output on pg <= 13.
@steve-chavez steve-chavez force-pushed the nickb/fix_supautils_hook_privileged_rolespec_handling branch from d80a2bf to 1ecdbac Compare April 2, 2026 00:26
@steve-chavez steve-chavez merged commit d4dff38 into master Apr 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants