Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPLT-1049] Prevent modification of other schemas during provisioning #114

Conversation

morgsmccauley
Copy link
Collaborator

@morgsmccauley morgsmccauley commented Jun 28, 2023

Currently, it's possible to execute any arbitrary SQL during schema provisioning. We set the search_path to an schema specific to the indexer, but there's nothing preventing the user from executing SQL against other schemas, i.e. CREATE TABLE other_schema.table ..., or DROP SCHEMA other_schema. To prevent this, we now create a Postgres Role which has access limited to the current schema only.

As we only allow creating new schemas, and not updating existing schemas, we shouldn't need to migrate any existing schemas to have a corresponding role.

Additionally, I've:

  • Merged schema creation and migration to a single step, reducing the back and forth to Hasura
  • Renamed roleName -> hasuraRoleName to avoid confusion with the PG role

@morgsmccauley morgsmccauley force-pushed the DPLT-1049-prevent-modification-of-other-schemas-during-provisioning branch from bad27cb to a42aada Compare June 28, 2023 23:11
SET SCHEMA '${schemaName}';

-- indexer provided migration
${migration};
Copy link
Collaborator

@gabehamilton gabehamilton Jun 29, 2023

Choose a reason for hiding this comment

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

Can this be attacked by calling

SET ROLE someotheruser;
SET SCHEMA otherschema

at the start of a migration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just checked, yes it can.. let me try and figure out how to prevent that

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could disallow SET in schema. There may be other clever syntaxes to do the same thing but it'd be a start. Keeping people from setting config sounds like a good idea as well.

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.

2 participants