Skip to content

Conversation

@YuryHrytsuk
Copy link
Contributor

@YuryHrytsuk YuryHrytsuk commented Jun 27, 2025

What do these changes do?

  1. create readonly role to reuse for users that need to access database
    • can be reused for readonly user and for metabase's user
  2. fix security issue with public schema in Postgres 14
  3. make readonly user rely on readonly role

Actions:

  • init.sql script shall be executed in all deployments.
  • Create role and user scripts shall be executed on demand (if necessary)

Related issue/s

How to test

  1. Create a user. Grant it readonly role. Select data from tables
  2. Create a new user. Connect to db. Create a table should show an error permission denied

Dev-ops

  • Execute init script in all deployments (as superuser for every database [simcore, metabase, ...])
  • If applicable, create read only role and user

NOTE: for templating env use repo config

1. create readonly role to reuse for users that need to access database
    * can be reused for readonly user and for metabase's user
2. fix security issue with public schema in Postgres 14
3. make readonly user rely on readonly role
@YuryHrytsuk YuryHrytsuk added this to the Engage milestone Jun 27, 2025
@YuryHrytsuk YuryHrytsuk self-assigned this Jun 27, 2025
@YuryHrytsuk YuryHrytsuk added t:enhancement Improvement or request on an existing feature security Pull requests that address a security vulnerability 🤖-automerge marks PR as ready to be merged for Mergify 🤖-do-not-merge (optional) blocks Mergify from merging the PR and removed 🤖-automerge marks PR as ready to be merged for Mergify labels Jun 27, 2025
Copy link
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

Adds and refactors SQL scripts to enforce a read-only role for Postgres databases and lock down the public schema in Postgres 14.

  • Introduce init.sql to revoke CREATE on the public schema for all new databases.
  • Create and remove templates for a reusable ${POSTGRES_DB}_readonly role, and refactor the read-only user scripts to grant/revoke this role.
  • Update .gitignore to track template files and the new init.sql.

Reviewed Changes

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

Show a summary per file
File Description
services/postgres/scripts/init.sql New script to revoke CREATE on public schema per DB
services/postgres/scripts/create-readonly-role.sql.template New template for creating the ${POSTGRES_DB}_readonly role
services/postgres/scripts/create-readonly-user.sql.template Refactored user script to grant the readonly role
services/postgres/scripts/remove-readonly-role.sql.template New template for revoking privileges and dropping the readonly role
services/postgres/scripts/remove-readonly-user.sql.template Updated script to revoke the readonly role before dropping the user
services/postgres/scripts/.gitignore Kept template files and init.sql tracked

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

our colleague copilot has some valid points

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

thx

@YuryHrytsuk YuryHrytsuk added 🤖-automerge marks PR as ready to be merged for Mergify and removed 🤖-do-not-merge (optional) blocks Mergify from merging the PR labels Jul 1, 2025
@YuryHrytsuk
Copy link
Contributor Author

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 1, 2025

queue

🛑 The pull request has been removed from the queue default

The following conditions don't match anymore:

  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • any of: [🛡 GitHub branch protection]
        • check-neutral = system-tests
        • check-skipped = system-tests
        • check-success = system-tests

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 1, 2025

@mergify
Copy link
Contributor

mergify bot commented Jul 1, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@YuryHrytsuk YuryHrytsuk merged commit 9c0f57f into ITISFoundation:master Jul 1, 2025
110 of 113 checks passed
@YuryHrytsuk YuryHrytsuk deleted the update-postgres-configuration branch July 1, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify security Pull requests that address a security vulnerability t:enhancement Improvement or request on an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants