Skip to content

Commit 5ff8c5d

Browse files
avalletecharmandercharmander
authored
chore: sync upstream/master + bump @supabase/pg to 8.21.1 (#34)
* test: Ensure failure to throw at all doesn’t pass (brianc#3671) * Assorted test fixes and cleanup (brianc#3672) * cleanup: Remove duplicate test * cleanup: Remove nonsense test * cleanup: Simplify promise rejection test * test: Fix and tighten assertion that would always pass because of the `SELECTR` typo. * cleanup: Add missing `await`s when using `assert.rejects` in tests; remove unneeded function wrappers * ci: Node 26 followup (brianc#3670) * Revert unneeded pg-native→libpq dependency range adjustment This reverts part of commit 1025d12. * dev: Upgrade libpq/nan in lockfile for Node 26 compatibility * fix: apply SASLprep (RFC 4013) to passwords before SCRAM-SHA-256 PBKDF2 (brianc#3669) * fix: apply SASLprep (RFC 4013) to passwords before SCRAM-SHA-256 PBKDF2 `pg`'s SCRAM-SHA-256 client passes the raw password into PBKDF2 with no normalization, while PostgreSQL's server (and libpq) apply SASLprep (B.1 mapping -> NFKC -> prohibition + bidi check) when computing the stored verifier. Passwords whose NFKC form differs from themselves (e.g. containing U+00A8 dieresis, U+2011 non-breaking hyphen, U+00BC vulgar one quarter, NBSP, soft hyphen) authenticate with psql/libpq but fail against pg with `28P01`. Wire `@mongodb-js/saslprep` (the maintained fork used by mongodb's official Node driver) into `continueSession` before `crypto.deriveKey`, with a try/catch fallback to the raw password on prohibited / bidi violations to match `libpq`'s `pg_saslprep` behavior. Also adds: - Unit tests covering the soft-hyphen B.1 mapping equivalence, the Roman-numeral-IX NFKC asymmetry, the prohibited-char fallback, and a deterministic snapshot for the original bug-report password. - A gated integration test block (SCRAM_TEST_PGUSER_UNICODE / SCRAM_TEST_PGPASSWORD_UNICODE) covering raw + NFKC-equivalent + wrong password. - A `scram_unicode_test` role (password `U&'IX-\2168'`) provisioned in CI plus matching env vars so the new integration tests run on every Node version. - A Cloudflare Workers regression guard that exercises `sasl.continueSession` to ensure `@mongodb-js/saslprep` resolves cleanly under workerd. - A `pg@8.21.0` CHANGELOG entry. * fix: inline SASLprep, drop @mongodb-js/saslprep dependency Per review feedback on brianc#3669: ship the SASLprep step as a small in-tree function instead of pulling a runtime dep with an unpinned transitive. The function performs only the three byte-changing steps from RFC 4013 (Table C.1.2 -> SPACE, Table B.1 -> empty, NFKC) and skips the prohibition (RFC 4013 section 2.3) and bidi (RFC 3454 section 6) checks, since libpq is forgiving on those paths and Postgres's own SASLprep is similarly lenient. Removes the try/catch fallback (no code path throws). The deterministic snapshot tests stay byte-for-byte valid because none of them touch U+200B, the only edge case where the inline impl diverges from `@mongodb-js/saslprep`. RFC 3454 places U+200B in Table B.1 (mapped to nothing); the dep maps it to SPACE. PostgreSQL's saslprep.c follows the RFC, so the inline impl matches libpq more closely on that codepoint. The B.1 unit-test rename ("passes ASCII control characters through normalization unchanged") keeps the same snapshot bytes since BEL is unchanged by all three steps. Co-authored-by: charmander <charmander@noreply.github.com> * Revert unrelated no-op changes to yarn.lock now that the associated dependency isn’t being added. * cleanup: Allow Prettier to format some lines * cleanup: Remove changelog entry for unreleased pg version normally added as part of the release process * refactor: Simplify comments in sasl.js and remove unused test cases Updated comments in sasl.js to clarify the password normalization process and removed redundant test cases from vitest-cf.test.ts, streamlining the codebase. * Remove redundant NFKC-only SASLprep test Confirmed in pull request comments that the “macOS/iOS” thing was an AI inventing an unneeded justification, and NFKC is already covered by another test. * fix: SASLprep zero-width space the same way PostgreSQL does As mentioned in the test comment, RFC 3454 defines appendix B for mapping tables and appendix C for prohibition tables. RFC 4013 SASLprep is probably misusing that list of non-ASCII spaces, and says nothing about the overlap. (At least it’s obsoleted.) * cleanup: Simplify regex character classes with ranges --------- Co-authored-by: charmander <charmander@noreply.github.com> Co-authored-by: Charmander <~@charmander.me> --------- Co-authored-by: Charmander <~@charmander.me> Co-authored-by: charmander <charmander@noreply.github.com>
1 parent 6868812 commit 5ff8c5d

10 files changed

Lines changed: 271 additions & 151 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ dist
1010
/.eslintcache
1111
.vscode/
1212
manually-test-on-heroku.js
13+
*.tsbuildinfo

packages/pg-native/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
},
3535
"homepage": "https://github.com/supabase/node-postgres/tree/master/packages/pg-native",
3636
"dependencies": {
37-
"libpq": "^1.11.0",
37+
"libpq": "^1.8.15",
3838
"pg-types": "2.2.0"
3939
},
4040
"devDependencies": {

packages/pg/lib/crypto/sasl.js

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,34 @@
22
const crypto = require('./utils')
33
const { signatureAlgorithmHashFromCertificate } = require('./cert-signatures')
44

5+
// SASLprep (RFC 4013) — minimal in-tree implementation.
6+
//
7+
// Per RFC 5802 §2.2, the SCRAM-SHA-256 client must normalize the password via
8+
// SASLprep before feeding it into PBKDF2. PostgreSQL's server applies the same
9+
// SASLprep when computing the stored verifier, and libpq does the same client
10+
// side, so passwords whose NFKC form differs from the raw form
11+
// would otherwise authenticate against psql/libpq but fail against pg with `28P01`.
12+
//
13+
// We deliberately implement only the three steps that change the byte content:
14+
// 1. RFC 3454 Table C.1.2 (non-ASCII space) → U+0020 SPACE.
15+
// 2. RFC 3454 Table B.1 (commonly mapped to nothing) → empty.
16+
// 3. NFKC normalization.
17+
// We skip the prohibition (RFC 4013 §2.3) and bidi (RFC 3454 §6) checks.
18+
// libpq is forgiving on those paths and Postgres's own SASLprep matches that
19+
// leniency for legacy roles, so omitting the rejection logic keeps existing
20+
// roles working without adding complexity.
21+
function saslprep(password) {
22+
// RFC 3454 Table C.1.2 — non-ASCII space characters, mapped to U+0020.
23+
const nonAsciiSpace = /[\u00A0\u1680\u2000-\u200B\u202F\u205F\u3000]/g
24+
// RFC 3454 Table B.1 — "commonly mapped to nothing". The set intentionally
25+
// contains zero-width joiners and variation selectors — the very characters
26+
// ESLint's no-misleading-character-class warns about — because they combine
27+
// with their neighbors and the RFC strips them for that reason.
28+
// eslint-disable-next-line no-misleading-character-class
29+
const mappedToNothing = /[\u00AD\u034F\u1806\u180B\u180C\u180D\u200C\u200D\u2060\uFE00-\uFE0F\uFEFF]/g
30+
return password.replace(nonAsciiSpace, ' ').replace(mappedToNothing, '').normalize('NFKC')
31+
}
32+
533
function startSession(mechanisms, stream) {
634
const candidates = ['SCRAM-SHA-256']
735
if (stream) candidates.unshift('SCRAM-SHA-256-PLUS') // higher-priority, so placed first
@@ -70,7 +98,7 @@ async function continueSession(session, password, serverData, stream) {
7098
const authMessage = clientFirstMessageBare + ',' + serverFirstMessage + ',' + clientFinalMessageWithoutProof
7199

72100
const saltBytes = Buffer.from(sv.salt, 'base64')
73-
const saltedPassword = await crypto.deriveKey(password, saltBytes, sv.iteration)
101+
const saltedPassword = await crypto.deriveKey(saslprep(password), saltBytes, sv.iteration)
74102
const clientKey = await crypto.hmacSha256(saltedPassword, 'Client Key')
75103
const storedKey = await crypto.sha256(clientKey)
76104
const clientSignature = await crypto.hmacSha256(storedKey, authMessage)

packages/pg/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@supabase/pg",
3-
"version": "8.21.0",
3+
"version": "8.21.1",
44
"description": "PostgreSQL client - pure javascript & libpq with the same API",
55
"keywords": [
66
"database",

packages/pg/test/integration/client/promise-api-tests.js

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@ suite.test('valid connection completes promise', () => {
1313
})
1414
})
1515

16-
suite.test('valid connection completes promise', () => {
17-
const client = new pg.Client()
18-
return client.connect().then(() => {
19-
return client.end().then(() => {})
20-
})
21-
})
22-
2316
suite.test('valid connection returns the client in a promise', () => {
2417
const client = new pg.Client()
2518
return client.connect().then((clientInside) => {
@@ -28,25 +21,7 @@ suite.test('valid connection returns the client in a promise', () => {
2821
})
2922
})
3023

31-
suite.test('invalid connection rejects promise', (done) => {
24+
suite.test('invalid connection rejects promise', async () => {
3225
const client = new pg.Client({ host: 'alksdjflaskdfj', port: 1234 })
33-
return client.connect().catch((e) => {
34-
assert(e instanceof Error)
35-
done()
36-
})
37-
})
38-
39-
suite.test('connected client does not reject promise after connection', (done) => {
40-
const client = new pg.Client()
41-
return client.connect().then(() => {
42-
setTimeout(() => {
43-
client.on('error', (e) => {
44-
assert(e instanceof Error)
45-
client.end()
46-
done()
47-
})
48-
// manually kill the connection
49-
client.emit('error', new Error('something bad happened...but not really'))
50-
}, 50)
51-
})
26+
await assert.rejects(client.connect(), Error)
5227
})

packages/pg/test/integration/client/sasl-scram-tests.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,82 @@ suite.test('sasl/scram fails when password is empty', async () => {
108108
)
109109
assert.ok(usingSasl, 'Should be using SASL for authentication')
110110
})
111+
112+
/**
113+
* SASLprep regression coverage. RFC 5802 / RFC 4013 require the SCRAM client
114+
* to normalize the password (B.1 mapping → NFKC → prohibition + bidi check)
115+
* before feeding it into PBKDF2. PostgreSQL's server applies the same
116+
* SASLprep when computing the verifier, so any password whose NFKC form
117+
* differs from the raw form would otherwise authenticate against psql/libpq
118+
* but fail against pg with `28P01`.
119+
*
120+
* To exercise these tests, provision a role whose password contains an
121+
* NFKC-asymmetric character. For example, in psql:
122+
*
123+
* SET password_encryption = 'scram-sha-256';
124+
* CREATE ROLE scram_unicode_test LOGIN PASSWORD U&'IX-\2168';
125+
*
126+
* `\2168` is ROMAN NUMERAL IX; the server SASLprep-normalizes this to
127+
* `IX-IX` when computing the verifier. Then export:
128+
*
129+
* SCRAM_TEST_PGUSER_UNICODE=scram_unicode_test
130+
* SCRAM_TEST_PGPASSWORD_UNICODE='IX-\u2168' (i.e. the raw form)
131+
*
132+
* If either env var is unset the suite is skipped, matching the convention
133+
* of the ASCII SCRAM block above.
134+
*/
135+
const unicodeConfig = {
136+
user: process.env.SCRAM_TEST_PGUSER_UNICODE,
137+
password: process.env.SCRAM_TEST_PGPASSWORD_UNICODE,
138+
host: process.env.SCRAM_TEST_PGHOST,
139+
port: process.env.SCRAM_TEST_PGPORT,
140+
database: process.env.SCRAM_TEST_PGDATABASE,
141+
}
142+
143+
if (!unicodeConfig.user || !unicodeConfig.password) {
144+
suite.test('skipping SCRAM unicode tests (missing env)', () => {})
145+
} else {
146+
suite.test('sasl/scram authenticates a password requiring SASLprep (raw form)', async () => {
147+
const client = new pg.Client(unicodeConfig)
148+
let usingSasl = false
149+
client.connection.once('authenticationSASL', () => {
150+
usingSasl = true
151+
})
152+
await client.connect()
153+
assert.ok(usingSasl, 'Should be using SASL for authentication')
154+
await client.end()
155+
})
156+
157+
suite.test('sasl/scram authenticates the NFKC-equivalent ASCII form of the same password', async () => {
158+
// The unicode password contains a codepoint that NFKC-decomposes to ASCII
159+
// (e.g. U+2168 → "IX"). The server stored the verifier from the
160+
// SASLprep'd ASCII form, so feeding the client the ASCII form directly
161+
// must also authenticate. This proves that the prep step is symmetric:
162+
// any NFKC-equivalent representation reaches the same PBKDF2 input.
163+
const client = new pg.Client({
164+
...unicodeConfig,
165+
password: unicodeConfig.password.normalize('NFKC'),
166+
})
167+
await client.connect()
168+
await client.end()
169+
})
170+
171+
suite.test('sasl/scram fails when unicode password is wrong', async () => {
172+
const client = new pg.Client({
173+
...unicodeConfig,
174+
password: unicodeConfig.password + 'append-something-to-make-it-bad',
175+
})
176+
let usingSasl = false
177+
client.connection.once('authenticationSASL', () => {
178+
usingSasl = true
179+
})
180+
await assert.rejects(
181+
() => client.connect(),
182+
{
183+
code: '28P01',
184+
},
185+
'Error code should be for a password error'
186+
)
187+
assert.ok(usingSasl, 'Should be using SASL for authentication')
188+
})
189+
}

packages/pg/test/integration/gh-issues/3174-tests.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ const testErrorBuffer = (bufferName, errorBuffer) => {
104104
if (!cli.native) {
105105
assert(errorHit)
106106
// further queries on the client should fail since its in an invalid state
107-
await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject')
107+
await assert.rejects(client.query('SELECT NOW()'), {
108+
message: 'Client has encountered a connection error and is not queryable',
109+
})
108110
}
109111

110112
await closeServer()
@@ -129,7 +131,9 @@ const testErrorBuffer = (bufferName, errorBuffer) => {
129131
if (!cli.native) {
130132
assert(errorHit)
131133
// further queries on the client should fail since its in an invalid state
132-
await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject')
134+
await assert.rejects(client.query('SELECT NOW()'), {
135+
message: 'Client has encountered a connection error and is not queryable',
136+
})
133137
}
134138

135139
await client.end()

0 commit comments

Comments
 (0)