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.

Sign up for GitHub

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

Jump to bottom

Add SECURITY DEFINER to internal functions #191

Merged
merged 6 commits into from
Oct 25, 2023
Merged

Add SECURITY DEFINER to internal functions #191

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
28b9758
Add `SECURITY DEFINER` to `raw_migration` func
andrew-farries Oct 18, 2023
390f48a
Add `SECURITY DEFINER` to `latest_version` func
andrew-farries Oct 18, 2023
aed4d73
Set search_path, namespace function calls
exekias Oct 24, 2023
4d44429
add pg_temp to the search path
exekias Oct 24, 2023
bd7505f
count too
exekias Oct 24, 2023
b7830a3
namespace random and md5
exekias Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions pkg/state/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ CREATE OR REPLACE FUNCTION %[1]s.is_active_migration_period(schemaname NAME) RET

-- Get the latest version name (this is the one with child migrations)
CREATE OR REPLACE FUNCTION %[1]s.latest_version(schemaname NAME) RETURNS text
SECURITY DEFINER
Copy link

Choose a reason for hiding this comment

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

This probably needs a search path and probably should also get some explicit schemas for all called functions, otherwise I can imagine that this can be used to gain the rights of the user used for migrations.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for raising this! I'm adding SET search_path and reviewing calls

SET search_path = %[1]s, pg_catalog, pg_temp
AS $$
SELECT p.name FROM %[1]s.migrations p
WHERE NOT EXISTS (
Expand Down Expand Up @@ -153,27 +155,29 @@ END;
$$;

CREATE OR REPLACE FUNCTION %[1]s.raw_migration() RETURNS event_trigger
LANGUAGE plpgsql AS $$
LANGUAGE plpgsql
SECURITY DEFINER
SET search_path = %[1]s, pg_catalog, pg_temp AS $$
DECLARE
schemaname TEXT;
BEGIN
-- Ignore migrations done by pgroll
IF (current_setting('pgroll.internal', 'TRUE') <> 'TRUE') THEN
IF (pg_catalog.current_setting('pgroll.internal', 'TRUE') <> 'TRUE') THEN
RETURN;
END IF;

IF tg_event = 'sql_drop' THEN
-- Guess the schema from drop commands
SELECT schema_name INTO schemaname FROM pg_event_trigger_dropped_objects() WHERE schema_name IS NOT NULL;
SELECT schema_name INTO schemaname FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE schema_name IS NOT NULL;

ELSIF tg_event = 'ddl_command_end' THEN
-- Guess the schema from ddl commands, ignore migrations that touch several schemas
IF (SELECT COUNT(DISTINCT schema_name) FROM pg_event_trigger_ddl_commands() WHERE schema_name IS NOT NULL) > 1 THEN
IF (SELECT pg_catalog.count(DISTINCT schema_name) FROM pg_catalog.pg_event_trigger_ddl_commands() WHERE schema_name IS NOT NULL) > 1 THEN
RAISE NOTICE 'pgroll: ignoring migration that changes several schemas';
RETURN;
END IF;

SELECT schema_name INTO schemaname FROM pg_event_trigger_ddl_commands() WHERE schema_name IS NOT NULL;
SELECT schema_name INTO schemaname FROM pg_catalog.pg_event_trigger_ddl_commands() WHERE schema_name IS NOT NULL;
END IF;

IF schemaname IS NULL THEN
Expand All @@ -191,8 +195,8 @@ BEGIN
INSERT INTO %[1]s.migrations (schema, name, migration, resulting_schema, done, parent)
VALUES (
schemaname,
format('sql_%%s', substr(md5(random()::text), 0, 15)),
json_build_object('sql', json_build_object('up', current_query())),
pg_catalog.format('sql_%%s',pg_catalog.substr(pg_catalog.md5(pg_catalog.random()::text), 0, 15)),
pg_catalog.json_build_object('sql', pg_catalog.json_build_object('up', pg_catalog.current_query())),
%[1]s.read_schema(schemaname),
true,
%[1]s.latest_version(schemaname)
Expand Down
Loading