Skip to content

fix: revoke cloudsqlsuperuser permission#25

Merged
FabrizioCafolla merged 3 commits intomainfrom
fix/revoke-cloudsqlsuperuser
Mar 18, 2026
Merged

fix: revoke cloudsqlsuperuser permission#25
FabrizioCafolla merged 3 commits intomainfrom
fix/revoke-cloudsqlsuperuser

Conversation

@FabrizioCafolla
Copy link
Copy Markdown
Member

@FabrizioCafolla FabrizioCafolla commented Mar 18, 2026

PR Type

Bug fix


Description

  • Fix MySQL 8.0/8.4 REVOKE cloudsqlsuperuser causing Access Denied (1045) error

  • Add SHOW GRANTS pre-check before attempting role revocation

  • Skip revoke if role is absent, avoiding ROLE_ADMIN privilege requirement

  • Update CHANGELOG.md with version 0.5.2 release notes


Diagram Walkthrough

flowchart LR
  A["MySQL 8.0/8.4 provisioning"] -- "SHOW GRANTS FOR user" --> B["Check grants output"]
  B -- "cloudsqlsuperuser found" --> C["REVOKE cloudsqlsuperuser FROM user"]
  B -- "cloudsqlsuperuser not found" --> D["Skip REVOKE, log and continue"]
  C -- "success" --> E["Grant ALL PRIVILEGES on database"]
  C -- "failure" --> F["Log ERROR and exit 1"]
  D --> E
Loading

File Walkthrough

Relevant files
Bug fix
execute_sql.sh
Add SHOW GRANTS pre-check before revoking cloudsqlsuperuser role

scripts/execute_sql.sh

  • Replace blind REVOKE cloudsqlsuperuser with a SHOW GRANTS pre-check to
    detect role presence
  • Only attempt revoke if cloudsqlsuperuser is found in grants output
  • Log and skip revoke gracefully if role is absent, avoiding Access
    Denied (1045) errors
  • Exit with error if SHOW GRANTS itself fails
+14/-5   
Documentation
CHANGELOG.md
Document version 0.5.2 bug fix in changelog                           

CHANGELOG.md

  • Add new 0.5.2 release entry documenting the MySQL 8.0/8.4 provisioning
    fix
  • Describe the root cause and resolution of the Access Denied error on
    REVOKE cloudsqlsuperuser
+8/-0     

@sparkfabrik-ai-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

SHOW GRANTS Failure Handling

The SHOW GRANTS FOR user command may fail not only due to connectivity issues but also if the user does not exist yet or if the admin lacks sufficient privileges to inspect grants for other users. In such cases, the script will exit with an error (exit 1), which may be too strict. Consider whether a non-fatal fallback (e.g., skipping the revoke) would be more appropriate in certain failure scenarios.

if ! GRANTS_OUTPUT=$(mysql_exec --execute="SHOW GRANTS FOR ${USER_IDENTIFIER};" 2>&1); then
    log "ERROR: Failed to retrieve grants for ${USER_IDENTIFIER}:\n${GRANTS_OUTPUT}" >&2
    exit 1
fi
Grep Pattern Reliability

The grep -qi "cloudsqlsuperuser" check on the SHOW GRANTS output could produce false positives if the string cloudsqlsuperuser appears in a comment, error message, or other context within the output. Consider using a more precise pattern (e.g., matching the exact grant role syntax) to avoid unintended matches.

if printf '%s' "${GRANTS_OUTPUT}" | grep -qi "cloudsqlsuperuser"; then

Copy link
Copy Markdown
Contributor

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

This PR updates the MySQL grant/revoke helper script to avoid failing MySQL 8.0/8.4 provisioning when REVOKE cloudsqlsuperuser would otherwise error, and documents the fix in the changelog.

Changes:

  • Add a SHOW GRANTS pre-check in execute_sql.sh to only attempt REVOKE cloudsqlsuperuser when the role is actually present.
  • Update CHANGELOG.md with a 0.5.2 release entry describing the MySQL 8.0/8.4 fix.

Reviewed changes

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

File Description
scripts/execute_sql.sh Adds a grants pre-check to conditionally revoke cloudsqlsuperuser for MySQL 8.0/8.4.
CHANGELOG.md Documents the fix as release 0.5.2.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@sparkfabrik-ai-bot
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use precise pattern to detect role grant

The SHOW GRANTS output may contain the string "cloudsqlsuperuser" in contexts other
than an actual role grant (e.g., in a comment or a different privilege description).
Use a more precise grep pattern to match only the role grant line, such as anchoring
to GRANT 'cloudsqlsuperuser' or similar MySQL output format, to avoid false
positives.

scripts/execute_sql.sh [60]

-if printf '%s' "${GRANTS_OUTPUT}" | grep -qi "cloudsqlsuperuser"; then
+if printf '%s' "${GRANTS_OUTPUT}" | grep -qi "GRANT 'cloudsqlsuperuser'"; then
Suggestion importance[1-10]: 6

__

Why: The suggestion is valid - using a more specific grep pattern like GRANT 'cloudsqlsuperuser' would reduce false positives compared to just matching cloudsqlsuperuser anywhere in the output. This is a reasonable defensive improvement, though the risk of false positives in practice is relatively low.

Low
General
Conditionally apply SET DEFAULT ROLE command

The SET DEFAULT ROLE NONE command may fail or have no effect if the
cloudsqlsuperuser role was never assigned and was skipped. More importantly, if the
role was not present, setting the default role to NONE could still cause an error if
the user has no roles at all. Consider conditionally including SET DEFAULT ROLE NONE
only when the role was actually revoked, or ensure it is safe to run unconditionally
in all cases.

scripts/execute_sql.sh [71]

-SQL_COMMANDS="SET DEFAULT ROLE NONE TO ${USER_IDENTIFIER}; GRANT ALL PRIVILEGES ON ${DATABASE_IDENTIFIER} TO ${USER_IDENTIFIER};"
+if printf '%s' "${GRANTS_OUTPUT}" | grep -qi "GRANT 'cloudsqlsuperuser'"; then
+    SQL_COMMANDS="SET DEFAULT ROLE NONE TO ${USER_IDENTIFIER}; GRANT ALL PRIVILEGES ON ${DATABASE_IDENTIFIER} TO ${USER_IDENTIFIER};"
+else
+    SQL_COMMANDS="GRANT ALL PRIVILEGES ON ${DATABASE_IDENTIFIER} TO ${USER_IDENTIFIER};"
+fi
Suggestion importance[1-10]: 5

__

Why: The suggestion raises a valid concern about SET DEFAULT ROLE NONE potentially failing when no roles exist. However, in MySQL 8.0+, SET DEFAULT ROLE NONE is generally safe to run even when no roles are assigned, so the actual risk may be lower than suggested. The improved code also reuses the GRANTS_OUTPUT grep check which could be refactored more cleanly.

Low

@FabrizioCafolla
Copy link
Copy Markdown
Member Author

/improve

@FabrizioCafolla FabrizioCafolla requested a review from Copilot March 18, 2026 14:04
@sparkfabrik-ai-bot
Copy link
Copy Markdown

Generating PR code suggestions

Work in progress ...

Copy link
Copy Markdown
Contributor

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

This PR fixes MySQL 8.0/8.4 provisioning failures caused by attempting to REVOKE cloudsqlsuperuser when the admin user lacks ROLE_ADMIN, by adding a SHOW GRANTS pre-check and documenting the change in the changelog.

Changes:

  • Add SHOW GRANTS FOR user pre-check before attempting REVOKE cloudsqlsuperuser on MySQL 8.0/8.4.
  • Skip role revocation when the role is not present; keep provisioning flowing to DB-scoped grants.
  • Add 0.5.2 release notes describing the fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
scripts/execute_sql.sh Adds a grants pre-check to decide whether to revoke cloudsqlsuperuser and adjusts SQL statements accordingly.
CHANGELOG.md Documents the 0.5.2 bug fix and links the comparison to the previous version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@FabrizioCafolla
Copy link
Copy Markdown
Member Author

/describe

Copy link
Copy Markdown
Contributor

@Stevesibilia Stevesibilia left a comment

Choose a reason for hiding this comment

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

LGTM!

@FabrizioCafolla FabrizioCafolla merged commit 5992b9c into main Mar 18, 2026
1 check passed
@FabrizioCafolla FabrizioCafolla deleted the fix/revoke-cloudsqlsuperuser branch March 18, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants