From 3275f25f7b2ce56b5d00f7df692ad96a7b31b389 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 13:20:25 +0000 Subject: [PATCH 1/8] Add check by running EXPLAIN EXECUTE on prepared statement --- packages/query/src/actions.ts | 83 ++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/query/src/actions.ts b/packages/query/src/actions.ts index caa9ec4a..496d7ecc 100644 --- a/packages/query/src/actions.ts +++ b/packages/query/src/actions.ts @@ -191,10 +191,17 @@ type TypeData = } | IParseError; +// Copied from https://github.com/brianc/node-postgres/blob/860cccd53105f7bc32fed8b1de69805f0ecd12eb/lib/client.js#L285-L302 +// Ported from PostgreSQL 9.2.4 source code in src/interfaces/libpq/fe-exec.c +// Replaced with regexp because it's 11x faster by Benjie. +// Added `\0` escape because PostgreSQL strings cannot contain `\0` but JS strings can. +export function escapeSqlIdentifier(str: string): string { + return `"${str.replace(/["\0]/g, '""')}"`; +} + /** * Returns the raw query type data as returned by the Describe message * @param query query string, can only contain proper Postgres numeric placeholders - * @param query name, should be unique per query body * @param queue */ export async function getTypeData( @@ -250,6 +257,72 @@ export async function getTypeData( return { params, fields }; } +/** + * Checks that `EXPLAIN EXECUTE` of the prepared statement works, otherwise returns the error. + * @param query query string, can only contain proper Postgres numeric placeholders + * @param typeData type data, the result from getTypeData for this same query + * @param queue + */ +export async function explainQuery( + query: string, + typeData: TypeData, + queue: AsyncQueue, +): Promise { + if ('errorCode' in typeData) return typeData; + const uniqueName = crypto.createHash('md5').update(query).digest('hex'); + // Prepare query + await queue.send(messages.parse, { + name: uniqueName, + query, + dataTypes: [], + }); + await queue.send(messages.flush, {}); + const parseResult = await queue.reply( + messages.errorResponse, + messages.parseComplete, + ); + try { + if ('fields' in parseResult) { + // Error case + const { fields: errorFields } = parseResult; + return { + errorCode: errorFields.R, + hint: errorFields.H, + message: errorFields.M, + position: errorFields.P, + }; + } + + // Explain query (will throw error on permissions failure). Leverages the + // fact that nullability is not checked in types at this stage, so `null` is + // a valid value for all types (yes, even domain types with a `non null` + // constraint). + const length = typeData.params.length; + const params = Array.from({ length }, () => 'null'); + const explain = await runQuery( + `explain execute ${escapeSqlIdentifier(uniqueName)}${ + params.length === 0 ? '' : ` (${params.join(', ')})` + };`, + queue, + ); + + return explain.map((e) => e[0]); + } finally { + // Release prepared statement + await queue.send(messages.close, { + target: PreparedObjectType.Statement, + targetName: uniqueName, + }); + + // Flush all messages + await queue.send(messages.flush, {}); + + // Recover server state from any errors + await queue.send(messages.sync, {}); + await queue.reply(messages.closeComplete); + } +} + enum TypeCategory { ARRAY = 'A', BOOLEAN = 'B', @@ -391,6 +464,7 @@ async function getComments( })); } +const doTestRuns = true; export async function getTypes( queryData: InterpolatedQuery, queue: AsyncQueue, @@ -400,6 +474,13 @@ export async function getTypes( return typeData; } + if (doTestRuns) { + const testRunResult = await explainQuery(queryData.query, typeData, queue); + if (testRunResult && 'errorCode' in testRunResult) { + return testRunResult; + } + } + const { params, fields } = typeData; const paramTypeOIDs = params.map((p) => p.oid); From 61a9a2a0b174bcbe9de7be821012fc27a0f65f4f Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 13:55:50 +0000 Subject: [PATCH 2/8] Initial positive test --- packages/example/config.json | 2 +- packages/example/sql/schema.sql | 25 +++++++++++++++++++ packages/example/src/user_emails/sample.ts | 19 ++++++++++++++ .../example/src/user_emails/sample.types.ts | 21 ++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 packages/example/src/user_emails/sample.ts create mode 100644 packages/example/src/user_emails/sample.types.ts diff --git a/packages/example/config.json b/packages/example/config.json index 6dc053dd..308cfff6 100644 --- a/packages/example/config.json +++ b/packages/example/config.json @@ -25,5 +25,5 @@ "category": { "return": "./src/customTypes.js#Category" } }, "srcDir": "./src/", - "dbUrl": "postgres://postgres:password@localhost/postgres" + "dbUrl": "postgres://pgtyped_test:password@localhost/postgres" } diff --git a/packages/example/sql/schema.sql b/packages/example/sql/schema.sql index ddfa9ceb..e23f76a1 100644 --- a/packages/example/sql/schema.sql +++ b/packages/example/sql/schema.sql @@ -1,3 +1,11 @@ +-- Our test user; generally has all permissions; but some are revoked later. +CREATE USER pgtyped_test WITH LOGIN PASSWORD 'password'; +GRANT CONNECT ON DATABASE postgres TO pgtyped_test; +GRANT USAGE ON SCHEMA public TO pgtyped_test; +ALTER DEFAULT PRIVILEGES GRANT ALL ON TABLES TO pgtyped_test; +ALTER DEFAULT PRIVILEGES GRANT ALL ON SEQUENCES TO pgtyped_test; +ALTER DEFAULT PRIVILEGES GRANT ALL ON FUNCTIONS TO pgtyped_test; + CREATE TABLE users ( id SERIAL PRIMARY KEY, email TEXT NOT NULL, @@ -101,3 +109,20 @@ CREATE TABLE book_country ( INSERT INTO book_country (country) VALUES ('CZ'), ('DE'); + +-- This table has different insert/update permissions +CREATE TABLE user_emails ( + id int PRIMARY KEY GENERATED ALWAYS AS IDENTITY, + address text NOT NULL UNIQUE, + receives_notifications boolean NOT NULL DEFAULT true, + created_at timestamptz NOT NULL DEFAULT NOW(), + updated_at timestamptz NOT NULL DEFAULT NOW() +); + +REVOKE ALL ON user_emails FROM pgtyped_test; +GRANT + SELECT, + INSERT (address, receives_notifications), + UPDATE (receives_notifications), + DELETE +ON user_emails TO pgtyped_test; diff --git a/packages/example/src/user_emails/sample.ts b/packages/example/src/user_emails/sample.ts new file mode 100644 index 00000000..722235a3 --- /dev/null +++ b/packages/example/src/user_emails/sample.ts @@ -0,0 +1,19 @@ +import { sql } from '@pgtyped/runtime'; +import { IInsertUserEmailQuery } from './sample.types.js'; +import { Client } from 'pg'; + +export async function insertUserEmail( + address: string, + receivesNotifications: boolean, + client: Client, +) { + const insertUserEmail = sql` + INSERT INTO user_emails (address, receives_notifications) + VALUES $userEmail(address, receives_notifications) + RETURNING id;`; + const result = await insertUserEmail.run( + { userEmail: { address, receives_notifications: receivesNotifications } }, + client, + ); + return result[0]; +} diff --git a/packages/example/src/user_emails/sample.types.ts b/packages/example/src/user_emails/sample.types.ts new file mode 100644 index 00000000..e319bb48 --- /dev/null +++ b/packages/example/src/user_emails/sample.types.ts @@ -0,0 +1,21 @@ +/** Types generated for queries found in "src/user_emails/sample.ts" */ + +/** 'InsertUserEmail' parameters type */ +export interface IInsertUserEmailParams { + userEmail: { + address: string | null | void, + receives_notifications: boolean | null | void + }; +} + +/** 'InsertUserEmail' return type */ +export interface IInsertUserEmailResult { + id: number; +} + +/** 'InsertUserEmail' query type */ +export interface IInsertUserEmailQuery { + params: IInsertUserEmailParams; + result: IInsertUserEmailResult; +} + From ccb52fe9da3d76ea247920174b37888170c5ca9a Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 14:08:02 +0000 Subject: [PATCH 3/8] Add negative assertions --- packages/example/src/user_emails/sample.ts | 22 ++++++++++++++++++- .../example/src/user_emails/sample.types.ts | 8 +++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/example/src/user_emails/sample.ts b/packages/example/src/user_emails/sample.ts index 722235a3..5ced5637 100644 --- a/packages/example/src/user_emails/sample.ts +++ b/packages/example/src/user_emails/sample.ts @@ -1,5 +1,11 @@ import { sql } from '@pgtyped/runtime'; -import { IInsertUserEmailQuery } from './sample.types.js'; +import { + IInsertUserEmailQuery, + IInsertUserEmailParams, + IInsertUserEmailResult, + IForbiddenInsertUserEmailWithIdParams, + IForbiddenInsertUserEmailWithIdResult, +} from './sample.types.js'; import { Client } from 'pg'; export async function insertUserEmail( @@ -17,3 +23,17 @@ export async function insertUserEmail( ); return result[0]; } + +const forbiddenInsertUserEmailWithId = sql` + INSERT INTO user_emails (id, address, receives_notifications) + VALUES $userEmail(id, address, receives_notifications) + RETURNING id;`; + +const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdParams; +const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdResult; + +// And just to check the above assertions are valid, we would expect these to error: +// @ts-expect-error +const assertRegularParamsIsNever: never = null as unknown as IInsertUserEmailParams; +// @ts-expect-error +const assertRegularResultIsNever: never = null as unknown as IInsertUserEmailResult; diff --git a/packages/example/src/user_emails/sample.types.ts b/packages/example/src/user_emails/sample.types.ts index e319bb48..9972e3ee 100644 --- a/packages/example/src/user_emails/sample.types.ts +++ b/packages/example/src/user_emails/sample.types.ts @@ -19,3 +19,11 @@ export interface IInsertUserEmailQuery { result: IInsertUserEmailResult; } +/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its result is assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithIdResult = never; + +/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its parameters are assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithIdParams = never; + From caf500a641ce33dd620c0ad5ac41d67ff2598564 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 14:34:47 +0000 Subject: [PATCH 4/8] Remove negative assertions from this test --- packages/example/docker-compose.yml | 2 +- packages/example/src/user_emails/sample.ts | 18 ------------------ .../example/src/user_emails/sample.types.ts | 8 -------- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/packages/example/docker-compose.yml b/packages/example/docker-compose.yml index a910a3b2..a7053d6d 100644 --- a/packages/example/docker-compose.yml +++ b/packages/example/docker-compose.yml @@ -20,7 +20,7 @@ services: NODE_IMAGE: "node:${NODE_VERSION:-18}-alpine" environment: PGHOST: db - PGUSER: postgres + PGUSER: pgtyped_test PGDATABASE: postgres PGPASSWORD: password CI: $CI diff --git a/packages/example/src/user_emails/sample.ts b/packages/example/src/user_emails/sample.ts index 5ced5637..1dbdf687 100644 --- a/packages/example/src/user_emails/sample.ts +++ b/packages/example/src/user_emails/sample.ts @@ -1,10 +1,6 @@ import { sql } from '@pgtyped/runtime'; import { IInsertUserEmailQuery, - IInsertUserEmailParams, - IInsertUserEmailResult, - IForbiddenInsertUserEmailWithIdParams, - IForbiddenInsertUserEmailWithIdResult, } from './sample.types.js'; import { Client } from 'pg'; @@ -23,17 +19,3 @@ export async function insertUserEmail( ); return result[0]; } - -const forbiddenInsertUserEmailWithId = sql` - INSERT INTO user_emails (id, address, receives_notifications) - VALUES $userEmail(id, address, receives_notifications) - RETURNING id;`; - -const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdParams; -const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdResult; - -// And just to check the above assertions are valid, we would expect these to error: -// @ts-expect-error -const assertRegularParamsIsNever: never = null as unknown as IInsertUserEmailParams; -// @ts-expect-error -const assertRegularResultIsNever: never = null as unknown as IInsertUserEmailResult; diff --git a/packages/example/src/user_emails/sample.types.ts b/packages/example/src/user_emails/sample.types.ts index 9972e3ee..e319bb48 100644 --- a/packages/example/src/user_emails/sample.types.ts +++ b/packages/example/src/user_emails/sample.types.ts @@ -19,11 +19,3 @@ export interface IInsertUserEmailQuery { result: IInsertUserEmailResult; } -/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its result is assigned type 'never'. - * */ -export type IForbiddenInsertUserEmailWithIdResult = never; - -/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its parameters are assigned type 'never'. - * */ -export type IForbiddenInsertUserEmailWithIdParams = never; - From 6cb9e046623f7704e261de2b64ccf189baca7ef2 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 14:35:17 +0000 Subject: [PATCH 5/8] Negative assertions (failing) --- .../example/src/user_emails/assert_fail.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 packages/example/src/user_emails/assert_fail.ts diff --git a/packages/example/src/user_emails/assert_fail.ts b/packages/example/src/user_emails/assert_fail.ts new file mode 100644 index 00000000..bedcee67 --- /dev/null +++ b/packages/example/src/user_emails/assert_fail.ts @@ -0,0 +1,25 @@ +import { sql } from '@pgtyped/runtime'; +import { + IForbiddenInsertUserEmailWithIdParams, + IForbiddenInsertUserEmailWithIdResult, +} from './assert_fail.types.js'; +import { + IInsertUserEmailParams, + IInsertUserEmailResult, +} from './sample.types.js'; +import { Client } from 'pg'; + +// This is forbidden since `INSERT` to `created_at` is not permitted +const forbiddenInsertUserEmailWithId = sql` + INSERT INTO user_emails (address, receives_notifications, created_at) + VALUES $userEmail(address, receives_notifications, created_at) + RETURNING id;`; + +const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdParams; +const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdResult; + +// And just to check the above assertions are valid, we would expect these to error: +// @ts-expect-error +const assertRegularParamsIsNever: never = null as unknown as IInsertUserEmailParams; +// @ts-expect-error +const assertRegularResultIsNever: never = null as unknown as IInsertUserEmailResult; From 900bd5f31054bba6edbf3fa8330f91b53ccd0168 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 14:59:15 +0000 Subject: [PATCH 6/8] Separate out two types of failure --- .../example/src/user_emails/assert_fail.ts | 25 ------------------- .../src/user_emails/assert_fail_generated.ts | 15 +++++++++++ .../assert_fail_generated.types.ts | 10 ++++++++ .../src/user_emails/assert_fail_rbac.ts | 14 +++++++++++ packages/example/src/user_emails/sample.ts | 9 +++++++ 5 files changed, 48 insertions(+), 25 deletions(-) delete mode 100644 packages/example/src/user_emails/assert_fail.ts create mode 100644 packages/example/src/user_emails/assert_fail_generated.ts create mode 100644 packages/example/src/user_emails/assert_fail_generated.types.ts create mode 100644 packages/example/src/user_emails/assert_fail_rbac.ts diff --git a/packages/example/src/user_emails/assert_fail.ts b/packages/example/src/user_emails/assert_fail.ts deleted file mode 100644 index bedcee67..00000000 --- a/packages/example/src/user_emails/assert_fail.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { sql } from '@pgtyped/runtime'; -import { - IForbiddenInsertUserEmailWithIdParams, - IForbiddenInsertUserEmailWithIdResult, -} from './assert_fail.types.js'; -import { - IInsertUserEmailParams, - IInsertUserEmailResult, -} from './sample.types.js'; -import { Client } from 'pg'; - -// This is forbidden since `INSERT` to `created_at` is not permitted -const forbiddenInsertUserEmailWithId = sql` - INSERT INTO user_emails (address, receives_notifications, created_at) - VALUES $userEmail(address, receives_notifications, created_at) - RETURNING id;`; - -const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdParams; -const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdResult; - -// And just to check the above assertions are valid, we would expect these to error: -// @ts-expect-error -const assertRegularParamsIsNever: never = null as unknown as IInsertUserEmailParams; -// @ts-expect-error -const assertRegularResultIsNever: never = null as unknown as IInsertUserEmailResult; diff --git a/packages/example/src/user_emails/assert_fail_generated.ts b/packages/example/src/user_emails/assert_fail_generated.ts new file mode 100644 index 00000000..558d833c --- /dev/null +++ b/packages/example/src/user_emails/assert_fail_generated.ts @@ -0,0 +1,15 @@ +import { sql } from '@pgtyped/runtime'; +import { + IForbiddenInsertUserEmailWithIdParams, + IForbiddenInsertUserEmailWithIdResult, +} from './assert_fail_generated.types.js'; +import { Client } from 'pg'; + +// This is invalid since `id` is a generated column +const forbiddenInsertUserEmailWithId = sql` + INSERT INTO user_emails (id, address, receives_notifications) + VALUES $userEmail(id, address, receives_notifications) + RETURNING id;`; + +const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdParams; +const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithIdResult; diff --git a/packages/example/src/user_emails/assert_fail_generated.types.ts b/packages/example/src/user_emails/assert_fail_generated.types.ts new file mode 100644 index 00000000..8ffc586b --- /dev/null +++ b/packages/example/src/user_emails/assert_fail_generated.types.ts @@ -0,0 +1,10 @@ +/** Types generated for queries found in "src/user_emails/assert_fail_generated.ts" */ + +/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its result is assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithIdResult = never; + +/** Query 'ForbiddenInsertUserEmailWithId' is invalid, so its parameters are assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithIdParams = never; + diff --git a/packages/example/src/user_emails/assert_fail_rbac.ts b/packages/example/src/user_emails/assert_fail_rbac.ts new file mode 100644 index 00000000..763227da --- /dev/null +++ b/packages/example/src/user_emails/assert_fail_rbac.ts @@ -0,0 +1,14 @@ +import { sql } from '@pgtyped/runtime'; +import { + IForbiddenInsertUserEmailWithCreatedAtParams, + IForbiddenInsertUserEmailWithCreatedAtResult, +} from './assert_fail_rbac.types.js'; + +// This is forbidden since `INSERT` to `created_at` is not granted +const forbiddenInsertUserEmailWithCreatedAt = sql` + INSERT INTO user_emails (address, receives_notifications, created_at) + VALUES $userEmail(address, receives_notifications, created_at) + RETURNING id;`; + +const assertForbiddenParamsIsNever: never = null as unknown as IForbiddenInsertUserEmailWithCreatedAtParams; +const assertForbiddenResultIsNever: never = null as unknown as IForbiddenInsertUserEmailWithCreatedAtResult; diff --git a/packages/example/src/user_emails/sample.ts b/packages/example/src/user_emails/sample.ts index 1dbdf687..419a7a4e 100644 --- a/packages/example/src/user_emails/sample.ts +++ b/packages/example/src/user_emails/sample.ts @@ -1,6 +1,8 @@ import { sql } from '@pgtyped/runtime'; import { IInsertUserEmailQuery, + IInsertUserEmailParams, + IInsertUserEmailResult, } from './sample.types.js'; import { Client } from 'pg'; @@ -19,3 +21,10 @@ export async function insertUserEmail( ); return result[0]; } + +// Just to check the assertions in the `./assert_fail_*` files are valid, we +// would expect these to error: +// @ts-expect-error +const assertRegularParamsIsNever: never = null as unknown as IInsertUserEmailParams; +// @ts-expect-error +const assertRegularResultIsNever: never = null as unknown as IInsertUserEmailResult; From 5c4e7a840a48618ac583cb569de3ece00b818ddf Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 15:10:02 +0000 Subject: [PATCH 7/8] Turn errors into IParseError --- .../src/user_emails/assert_fail_rbac.types.ts | 10 ++++++++++ packages/query/src/actions.ts | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 packages/example/src/user_emails/assert_fail_rbac.types.ts diff --git a/packages/example/src/user_emails/assert_fail_rbac.types.ts b/packages/example/src/user_emails/assert_fail_rbac.types.ts new file mode 100644 index 00000000..750da491 --- /dev/null +++ b/packages/example/src/user_emails/assert_fail_rbac.types.ts @@ -0,0 +1,10 @@ +/** Types generated for queries found in "src/user_emails/assert_fail_rbac.ts" */ + +/** Query 'ForbiddenInsertUserEmailWithCreatedAt' is invalid, so its result is assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithCreatedAtResult = never; + +/** Query 'ForbiddenInsertUserEmailWithCreatedAt' is invalid, so its parameters are assigned type 'never'. + * */ +export type IForbiddenInsertUserEmailWithCreatedAtParams = never; + diff --git a/packages/query/src/actions.ts b/packages/query/src/actions.ts index 496d7ecc..dc91a289 100644 --- a/packages/query/src/actions.ts +++ b/packages/query/src/actions.ts @@ -307,6 +307,23 @@ export async function explainQuery( ); return explain.map((e) => e[0]); + } catch (e: any) { + // This is most likely from the `runQuery` statement + /* Example error: + * ``` + * { + * type: 'ServerError', + * bufferOffset: 100, + * severity: 'ERROR', + * message: 'permission denied for table user_emails' + * } + * ``` + */ + console.error('Error occurred whilst testing query:', e); + return { + errorCode: '_ERROR', + message: e.message, + }; } finally { // Release prepared statement await queue.send(messages.close, { From 25c235f14d45783547e0e0857bdf5b0bd39c2982 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Tue, 9 Jan 2024 17:18:32 +0000 Subject: [PATCH 8/8] Set application_name on connect --- packages/query/src/actions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/query/src/actions.ts b/packages/query/src/actions.ts index dc91a289..640f8cf3 100644 --- a/packages/query/src/actions.ts +++ b/packages/query/src/actions.ts @@ -43,6 +43,7 @@ export async function startup( user: options.user, database: options.dbName, client_encoding: "'utf-8'", + application_name: 'pgtyped', }; await queue.send(messages.startupMessage, { params: startupParams }); const result = await queue.reply(