mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(pg-meta): escape unencrypted FDW server options via format() %L (#47014)
Closes #47012 ## What kind of change does this PR introduce? Bug fix. ## What is the current behavior? Unencrypted FDW server option values are pre-escaped with `literal(value).replace(/'/g, "''")` and embedded inside the outer `create server` `E'...'` string built by `format()`. That nests the value inside two `E'...'` literals, so backslashes are decoded twice. A value like `domain\user` aborts wrapper creation with `invalid Unicode escape`, and `p@ss\w0rd` is silently stored as `p@ssw0rd`. ## What is the new behavior? Unencrypted option values are passed as `format()` `%L` arguments, the same way the encrypted options already supply their secret id, so Postgres escapes each value exactly once. Before and after, on postgres:16: | Value | Before | After | | --- | --- | --- | | `domain\user` | aborts (invalid Unicode escape) | stored `domain\user` | | `p@ss\w0rd` | stored `p@ssw0rd` | stored `p@ss\w0rd` | | `a\b` | stored `a`+backspace | stored `a\b` | ## Additional context Added a unit test in `packages/pg-meta/test/sql/studio/fdw.test.ts` asserting unencrypted options use a `%L` placeholder with the value as a `format()` argument, and that the old double-escaped form is gone. The encrypted-option path is unchanged. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of special characters in foreign data wrapper server option values. * **Tests** * Added test coverage for foreign data wrapper configuration, including edge cases with special characters. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
2207d7f665
commit
99db104cf5
2 files changed
+70
-12
No files matched your search
@@ -156,15 +156,14 @@ export function getCreateFDWSql({
|
||||
const encryptedOptionsSqlArray = encryptedOptions
|
||||
.filter((option) => formState[option.name])
|
||||
.map((option) => safeSql`${ident(option.name)} ''%s''`)
|
||||
const unencryptedOptionsSqlArray = unencryptedOptions
|
||||
.filter((option) => formState[option.name])
|
||||
.map((option) => {
|
||||
// literal() returns 'value' with single quotes. Escape those quotes for
|
||||
// the surrounding E'...' string context ('' represents one ')
|
||||
const escapedValue = literal(formState[option.name]).replace(/'/g, "''") as SafeSqlFragment
|
||||
|
||||
return safeSql`${ident(option.name)} ${escapedValue}`
|
||||
})
|
||||
// Unencrypted option values are passed as format() %L arguments (below) so
|
||||
// Postgres escapes them once for the inner create-server literal. Pre-escaping
|
||||
// here and embedding in the outer E'...' string would decode backslashes twice,
|
||||
// which aborts creation (invalid Unicode escape) or silently corrupts the value.
|
||||
const unencryptedOptionsFilter = unencryptedOptions.filter((option) => formState[option.name])
|
||||
const unencryptedOptionsSqlArray = unencryptedOptionsFilter.map(
|
||||
(option) => safeSql`${ident(option.name)} %L`
|
||||
)
|
||||
const optionsSqlArray = joinSqlFragments(
|
||||
[...encryptedOptionsSqlArray, ...unencryptedOptionsSqlArray],
|
||||
','
|
||||
@@ -225,9 +224,12 @@ export function getCreateFDWSql({
|
||||
execute format(
|
||||
E'create server ${ident(formState.server_name)} foreign data wrapper ${ident(formState.wrapper_name)} options (${optionsSqlArray});',
|
||||
${joinSqlFragments(
|
||||
encryptedOptions
|
||||
.filter((option) => formState[option.name])
|
||||
.map((option) => ident(`v_${option.name}`)),
|
||||
[
|
||||
...encryptedOptions
|
||||
.filter((option) => formState[option.name])
|
||||
.map((option) => ident(`v_${option.name}`)),
|
||||
...unencryptedOptionsFilter.map((option) => literal(formState[option.name])),
|
||||
],
|
||||
','
|
||||
)}
|
||||
);
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { expect, test } from 'vitest'
|
||||
|
||||
import { getCreateFDWSql } from '../../../src'
|
||||
import { literal } from '../../../src/pg-format'
|
||||
|
||||
const baseArgs = {
|
||||
mode: 'skip' as const,
|
||||
tables: [],
|
||||
sourceSchema: '',
|
||||
targetSchema: '',
|
||||
}
|
||||
|
||||
// A value with both a backslash and a single quote. literal() escapes it to a
|
||||
// single E'...' literal; the old code then re-escaped the quotes and embedded
|
||||
// it in the outer E'...' string, decoding the backslash twice.
|
||||
const trickyValue = "ab\\cd'ef"
|
||||
|
||||
test('unencrypted server option values are passed as format() %L arguments', () => {
|
||||
const sql = getCreateFDWSql({
|
||||
...baseArgs,
|
||||
wrapperMeta: {
|
||||
handlerName: 'wasm_fdw_handler',
|
||||
validatorName: 'wasm_fdw_validator',
|
||||
server: { options: [{ name: 'api_key', encrypted: false }] },
|
||||
},
|
||||
formState: { wrapper_name: 'my_wrapper', server_name: 'my_server', api_key: trickyValue },
|
||||
})
|
||||
|
||||
// The option is emitted as a %L placeholder so format() escapes the value once.
|
||||
expect(sql).toContain('api_key %L')
|
||||
|
||||
// The raw value reaches format() as a single-level literal() argument.
|
||||
expect(sql).toContain(literal(trickyValue))
|
||||
|
||||
// Regression: the value must NOT be embedded as a double-escaped literal in the
|
||||
// outer E'...' string (literal(value).replace(/'/g, "''")), which corrupted
|
||||
// backslashes or aborted creation with "invalid Unicode escape".
|
||||
const doubleEscaped = literal(trickyValue).replace(/'/g, "''")
|
||||
expect(sql).not.toContain(doubleEscaped)
|
||||
})
|
||||
|
||||
test('encrypted server options still resolve their value through Vault unchanged', () => {
|
||||
const sql = getCreateFDWSql({
|
||||
...baseArgs,
|
||||
wrapperMeta: {
|
||||
handlerName: 'wasm_fdw_handler',
|
||||
validatorName: 'wasm_fdw_validator',
|
||||
server: { options: [{ name: 'api_secret', encrypted: true }] },
|
||||
},
|
||||
formState: { wrapper_name: 'my_wrapper', server_name: 'my_server', api_secret: 'shh' },
|
||||
})
|
||||
|
||||
// Encrypted options keep the ''%s'' placeholder filled by the vault secret id.
|
||||
expect(sql).toContain("api_secret ''%s''")
|
||||
expect(sql).toContain('vault.create_secret')
|
||||
})
|
||||
Reference in new issue
Block a user