fix: run custom scripts on non privileged extensions

[soedirgo]

What kind of change does this PR introduce?

Fixes #146

What is the current behavior?

Extension custom scripts are only run for privileged_extensions

What is the new behavior?

Support extension custom scripts for all extensions

Additional context

More info on the linked issue

[steve-chavez]

@soedirgo If this is the case, then please separate custom scripts to its own test suite, separate from privileged extensions. To avoid further confusion. Also to ensure this behavior covered, we need more tests:

• tests that custom scripts are ran for privileged extensions (these exist, but should be on their own test files).
• tests that custom scripts are ran without privileged extensions (new ones)

[steve-chavez]

I find this recursive custom scripts behavior mysterious. When can this happen? Can we cover it by tests so it can be fully understood?

I can help by adding the tests on another PR too, if it requires some other extension it's fine (I've added tests requiring plpgsql_check and pgmq before). Just let me know how can I reproduce.

[soedirgo]

The pathological behavior we're trying to avoid is an infinite loop caused by a create extension triggering a custom script that then runs a create extension, which in turn triggers a custom script, etc.

Can't really put the infinite loop as a test case, but preventing nested custom scripts can be done with 2 extensions: have custom scripts for both where one does a create extension for the other, and have the other one do a side effect (e.g. create a table with some sentinel name) and we assert not to happen.

[steve-chavez]

Ditto. I find these already_switched_to_superuser flags mysterious. When can this happen? Is it tested? If not, please mention how can a test be added to cover this behavior.

[soedirgo]

The mechanism we use to switch to and from superuser (switch_to_superuser(), switch_to_original_role()) can't be done in a nested manner, unless we use a stack. (Nor does it need to be, for our use case this is sufficient)

The reason is if we do switch_to_superuser() twice, the first one will make the current user id the superuser oid, and the second one will set the prev_role_oid (utils.c) to also be the superuser oid (because that's now the current user id). Then when switch_to_original_role is called, it sets the current user id to the prev_role_oid, which is now the superuser oid. Now the user gets access to a superuser.

To avoid this we just prevent switch_to_superuser() to be called twice without switch_to_original_role() in between. That's what the flag is for; don't run switch_to_superuser() if we already are one. This is tested here by asserting the original role is restored when running a statement that triggers switch_to_superuser() at least twice during its execution.

[coveralls]

Pull Request Test Coverage Report for Build 15216870438

Details

• 123 of 125 (98.4%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.07%) to 87.287%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/extension_custom_scripts.c	86	87	98.85%
src/extensions_parameter_overrides.c	20	21	95.24%

Totals
Change from base Build 15089561468:	0.07%
Covered Lines:	975
Relevant Lines:	1117

---

💛 - Coveralls

[soedirgo]

Addressed the comments and added the tests, the one for running_custom_scripts can be done in a separate PR