From 99db104cf55750f90cb1d87424e8b336cf22e1ee Mon Sep 17 00:00:00 2001 From: oniani1 Date: Mon, 22 Jun 2026 11:45:56 +0400 Subject: [PATCH] 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. ## 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. --- .../pg-meta/src/sql/studio/database/fdw.ts | 26 +++++---- packages/pg-meta/test/sql/studio/fdw.test.ts | 56 +++++++++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 packages/pg-meta/test/sql/studio/fdw.test.ts diff --git a/packages/pg-meta/src/sql/studio/database/fdw.ts b/packages/pg-meta/src/sql/studio/database/fdw.ts index 3c31d26899d..3678f475c8a 100644 --- a/packages/pg-meta/src/sql/studio/database/fdw.ts +++ b/packages/pg-meta/src/sql/studio/database/fdw.ts @@ -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])), + ], ',' )} ); diff --git a/packages/pg-meta/test/sql/studio/fdw.test.ts b/packages/pg-meta/test/sql/studio/fdw.test.ts new file mode 100644 index 00000000000..bf8613b4c4a --- /dev/null +++ b/packages/pg-meta/test/sql/studio/fdw.test.ts @@ -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') +})