-
-
Notifications
You must be signed in to change notification settings - Fork 99
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.
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
Check permissions of queries via EXPLAIN EXECUTE <stmt>;
#563
base: master
Are you sure you want to change the base?
Changes from all commits
3275f25
61a9a2a
ccb52fe
caf500a
6cb9e04
900bd5f
5c4e7a8
25c235f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { sql } from '@pgtyped/runtime'; | ||
import { | ||
IInsertUserEmailQuery, | ||
IInsertUserEmailParams, | ||
IInsertUserEmailResult, | ||
} from './sample.types.js'; | ||
import { Client } from 'pg'; | ||
|
||
export async function insertUserEmail( | ||
address: string, | ||
receivesNotifications: boolean, | ||
client: Client, | ||
) { | ||
const insertUserEmail = sql<IInsertUserEmailQuery>` | ||
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]; | ||
} | ||
|
||
// 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; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -191,10 +192,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 +258,89 @@ 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<string[] | IParseError> { | ||
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]); | ||
} 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, | ||
}; | ||
Comment on lines
+311
to
+327
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not feel right, but it makes the error handled by |
||
} 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 +482,7 @@ async function getComments( | |
})); | ||
} | ||
|
||
const doTestRuns = true; | ||
export async function getTypes( | ||
queryData: InterpolatedQuery, | ||
queue: AsyncQueue, | ||
|
@@ -400,6 +492,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); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good practice to set this; but it also give people a way to
return null
rather thanraise exception
from theirSTABLE
functions if it'spgtyped
running.