Skip to content

Commit 41758a1

Browse files
committed
fix: remove edge case chance of SET ROLE leaking
This changes removes the edge case chance that `SET ROLE` is never reset either by a rollback or `SET ROLE none`, by switching to `SET LOCAL ROLE` which will last only until the end of the current transaction. Whilst the scenario where the role leaks outside of the transaction without being reset correctly is extremely unlikely, and I am unable to create test scenario that replicates it, I think it is wise to future proof the code and make further guarantees that the leak cannot occur. See: - https://www.postgresql.org/docs/14/sql-set-role.html - https://www.postgresql.org/docs/14/sql-set.html
1 parent 460f669 commit 41758a1

File tree

6 files changed

+8
-10
lines changed

6 files changed

+8
-10
lines changed

.github/workflows/npm-pack-check.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ jobs:
2222

2323
# Install dependencies so that prepublish scripts work as expected
2424
- name: Install deps
25-
run: npm i
25+
run: npm ci
2626

2727
- name: Build package
2828
run: npm run build

.github/workflows/unit-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ jobs:
1010
- uses: actions/setup-node@v1
1111
with:
1212
node-version: 20
13-
- run: npm i
13+
- run: npm ci
1414
- run: npm test
1515
- run: npm run test:types

Dockerfile.sut

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# This container is used to run e2e tests against the built API container
2-
FROM node:latest
2+
FROM node:22
33

44
COPY package.json /app/package.json
55
COPY package-lock.json /app/package-lock.json
66

77
WORKDIR /app
88

9-
RUN npm i
9+
RUN npm ci
1010

1111
COPY . .
1212

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,4 @@
4848
"@prisma/client": "^5.11.0",
4949
"prisma": "^5.11.0"
5050
}
51-
}
51+
}

src/index.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ export const createClient = (
272272
prisma
273273
.$transaction(
274274
async (tx) => {
275-
// Switch to the user role, We can't use a prepared statement here, due to limitations in PG not allowing prepared statements to be used in SET ROLE
276-
await tx.$queryRawUnsafe(`SET ROLE ${batch.pgRole}`);
275+
// Switch to the user role, We can't use a prepared statement here, due to limitations in PG not allowing prepared statements to be used in SET LOCAL ROLE
276+
await tx.$queryRawUnsafe(`SET LOCAL ROLE ${batch.pgRole}`);
277277
// Now set all the context variables using `set_config` so that they can be used in RLS
278278
for (const [key, value] of toPairs(batch.context)) {
279279
await tx.$queryRaw`SELECT set_config(${key}, ${value.toString()}, true);`;
@@ -299,8 +299,6 @@ export const createClient = (
299299
}),
300300
),
301301
);
302-
// Switch role back to admin user
303-
await tx.$queryRawUnsafe("SET ROLE none");
304302

305303
return results;
306304
},

test/integration/middleware.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe("middlewares", () => {
3838
});
3939

4040
expect(post.id).toBeDefined();
41-
expect(middlewareSpy).toHaveBeenCalledTimes(3);
41+
expect(middlewareSpy).toHaveBeenCalledTimes(2);
4242
expect(middlewareSpy.mock.calls[0][0].model).toBe("Post");
4343
expect(middlewareSpy.mock.calls[1][0].model).toBeUndefined();
4444
expect(middlewareSpy.mock.calls[1][0].action).toBe("queryRaw");

0 commit comments

Comments
 (0)