Fix role leakage between connections

[nsavvide]

Fixes #1341.

The bug

In transaction pooling, a client that pins its backend and runs SET ROLE leaks its current_user to the next client handed that backend:

direct: source=('app','scm_session_role') peer=('app','app')             pid_same=False
pgdog:  source=('app','scm_session_role') peer=('app','scm_session_role') pid_same=True

The next client executes queries as a role it never asked for and may not be entitled to.

Root cause, see scoping

The fix is adding one statement to the DIRTY cleanup list:

Query::new("RESET SESSION AUTHORIZATION")

On your (@levkk) RESET ROLE suggestion: I implemented and tested that first. RESET SESSION AUTHORIZATION makes all tests green, because per the docs SET SESSION AUTHORIZATION also resets role — so RESET_ROLE alongside it is dead weight and I dropped it. Happy to switch to RESET ROLE instead if you'd rather keep the change narrower in intent, but then the SET SESSION AUTHORIZATION variant documented in the issue stays broken: it leaks session_user as well, and session_authorization is in -out path never resets it either.

[nsavvide]

@levkk feel free to review this one, thanks :))!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[levkk]

that pins its backend and runs SET ROLE

This is an edge case I think. The real bug is if a client runs SET ROLE in any scenario, and I suspect, in that case, the role will leak to the next client as well. We need to detect SET ROLE in the parser and place it in the Parameters along with others (or have it handled separately) and make sure we check it on each connection checkout from the pool.

[nsavvide]

that pins its backend and runs SET ROLE

This is an edge case I think. The real bug is if a client runs SET ROLE in any scenario, and I suspect, in that case, the role will leak to the next client as well. We need to detect SET ROLE in the parser and place it in the Parameters along with others (or have it handled separately) and make sure we check it on each connection checkout from the pool.

You're right, I reproduced it. SET ROLE issued after a backend is attached leaks with no pin involved:

BEGIN;
SELECT 1;          -- backend is attached from here
SET ROLE pgdog1;   -- plain SET, survives COMMIT in Postgres
COMMIT;

Next client on that backend gets current_user = pgdog1 fails with this PR's fix applied, so check-in cleanup can't reach it. Worth noting what is already working: role is not in UNTRACKED_PARAMS, the parser already produces Command::Set { name: "role" }, and link_client already emits RESET "role" at check-out. A SET ROLE outside a transaction is therefore already handled on main; I have a passing test for it. The hole is specifically that a SET issued once a server is attached is never written to client_params (populated only at check-out, server.rs:731), so the next check-out has nothing to reset.

That's the same hole #1299 is fixing for parameters generally.

So: how would you like this split?

1. Land this PR for the dirty-check-in case, cover the attached-server case in Track a client's parameter changes on the server connection #1299.
2. Close this, fold role into Track a client's parameter changes on the server connection #1299.
3. I extend Track a client's parameter changes on the server connection #1299 to cover role and session_authorization.