Skip to content

Commit 2ad91c3

Browse files
os-zhuangclaude
andauthored
feat(spec): refuse a postgres config.url that pg itself cannot parse at publish (#9091) (#9158)
PostgresConfigSchema.url documented a grammar it never enforced: the shared credentialFreeUrl/placeholderFree checks are string scans by design (their refusal to parse is load-bearing for mongo multi-host/+srv, #8696), so a URL pg throws on (libpq multi-host DSN, bad port) published green and failed at connect with a redacted Invalid URL. A per-driver superRefine now asks pg's own grammar (pg-connection-string parse, new spec dependency), refusing what parse throws on, scheme-less values parse only resolves against its placeholder base, and the fs-reading ?sslcert/?sslkey/?sslrootcert params (publish must not read the validating host's filesystem; certificates live in the datasource-level ssl block). ADR-0087 semantic entry + registry regen + docs regen; accept-side pins for every measured shape pg genuinely opens and for mongo's untouched multi-host form. Claude-Session: https://claude.ai/code/session_01225pUjnCKWqxcc1PeqKFUq Co-authored-by: Claude <noreply@anthropic.com>
1 parent d9813a9 commit 2ad91c3

8 files changed

Lines changed: 499 additions & 5 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): refuse a postgres `config.url` that `pg` itself cannot parse at publish (#9091)
6+
7+
**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep
8+
launch-window convention ships it as `minor`, like the sibling refusals #8337,
9+
#9040 and #9041; the migration prescription is registered under protocol major
10+
18, where `os migrate meta` users will look).
11+
12+
`PostgresConfigSchema.url`'s own describe text documents the postgres URL
13+
grammar (`postgresql://[user@][host][:port][/dbname][?params]`) and, until now,
14+
enforced none of it: the value was only string-scanned for credentials
15+
(#8082/#8337) and placeholders (#8336). That leniency is deliberate at the
16+
SHARED helper — its refusal to parse is load-bearing for mongo's
17+
multi-host/`+srv` forms (#8696) — but for postgres it amounted to no check at
18+
all. Measured on `pg@8.22.0`: both `pg-connection-string`'s `parse` and `pg`'s
19+
`ConnectionParameters` throw `TypeError [ERR_INVALID_URL]` on
20+
`postgresql://app@h1:5432,h2:5433/app` (node-postgres does not implement
21+
libpq's multi-host DSN), yet the schema accepted that exact value — the
22+
operator discovered the datasource could never connect only at connect time,
23+
via a bare `Invalid URL` whose `input` field `pg` redacts.
24+
25+
The schema now asks `pg`'s own grammar at publish — a per-driver `superRefine`
26+
on the postgres `url` runs `parse` from `pg-connection-string` (the parser `pg`
27+
itself uses; now a dependency of `@objectstack/spec`) — and refuses, at the
28+
value's path:
29+
30+
- anything `parse` throws on (multi-host DSNs, non-numeric ports, malformed
31+
percent-escapes), with the parser's own message quoted;
32+
- a scheme-less non-URL, which `parse` only "accepts" by resolving it against
33+
its placeholder base (`postgres://base`) — pg would connect to the literal
34+
host `base` with the authored text as the database name;
35+
- the fs-reading query parameters `?sslcert=` / `?sslkey=` / `?sslrootcert=`,
36+
which make `parse` itself call `fs.readFileSync` — a publish verdict must
37+
not depend on the validating host's filesystem, and certificate material
38+
already has its declared home in the datasource-level `ssl` block (the same
39+
prescription the config-level `ca`/`cert`/`key` keys carry).
40+
41+
Every measured shape `pg` genuinely opens stays accepted byte-identically:
42+
single-host URLs (credential-free ones included), the empty-host libpq forms
43+
(`postgresql:///db`, `postgresql://user@/db`), unix-socket spellings (a
44+
leading-`/` path, `socket:`, a percent-encoded socket host), IPv6 hosts, and
45+
non-credential/non-fs query parameters. Mongo, mysql and turso URLs are
46+
untouched — the shared helpers keep refusing to parse, per-driver by design.
47+
48+
## FROM → TO
49+
50+
```yaml
51+
# before — parsed green; `pg` then threw a redacted `Invalid URL` at connect
52+
driver: postgres
53+
config:
54+
url: postgresql://app@h1:5432,h2:5433/app
55+
56+
# after — point the URL at a single host (or a proxy/pooler in front of the
57+
# cluster); `pg` does not implement libpq's multi-host DSN, so no spelling of
58+
# it can connect
59+
driver: postgres
60+
config:
61+
url: postgresql://app@h1:5432/app
62+
```
63+
64+
There is deliberately no automatic rewrite: a URL `pg` cannot parse does not
65+
carry enough structure to say which single host the author meant (a multi-host
66+
DSN names several on purpose), so the choice of target is the author's.
67+
Runtime-environment DSNs (`OS_DATABASE_URL` and friends) never pass through
68+
this publish door and are unaffected by construction.
69+
70+
<!-- adr-0087: registered datasource-config-postgres-url-unparseable-refused -->

content/docs/references/data/driver-postgres.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ PostgreSQL connection configuration
4040

4141
| Property | Type | Required | Description |
4242
| :--- | :--- | :--- | :--- |
43-
| **url** | `string` | optional | Connection URI (supersedes the discrete fields; must not embed a password — bind the secret instead) |
43+
| **url** | `string` | optional | Connection URI (supersedes the discrete fields; must be a URL `pg` can parse; must not embed a password — bind the secret instead) |
4444
| **host** | `string` | optional (default: `"localhost"`) | Host address |
4545
| **port** | `integer` | optional (default: `5432`) | Port number |
4646
| **database** | `string` | optional | Database name |

packages/spec/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@
246246
"vitest": "^4.1.10"
247247
},
248248
"dependencies": {
249+
"pg-connection-string": "^2.14.0",
249250
"zod": "^4.4.3"
250251
},
251252
"peerDependencies": {

packages/spec/src/data/driver/postgres.test.ts

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { describe, it, expect } from 'vitest';
2+
import { DatasourceSchema } from '../datasource.zod';
3+
import { MongoConfigSchema } from './mongo.zod';
24
import { PostgresConfigSchema } from './postgres.zod';
35

46
describe('PostgresConfigSchema', () => {
@@ -192,3 +194,159 @@ describe('PostgresConfigSchema', () => {
192194
.toThrow();
193195
});
194196
});
197+
198+
/**
199+
* #9091 — a `url` that `pg` itself cannot parse is refused at publish.
200+
*
201+
* The describe text always documented the postgres URL grammar; until #9091
202+
* the value was only string-scanned (credentials #8082/#8337, placeholders
203+
* #8336) because the SHARED helper's refusal to parse is load-bearing for
204+
* mongo's multi-host/`+srv` forms (#8696). The parse question is asked
205+
* per-driver, of `pg`'s own parser (`pg-connection-string`).
206+
*
207+
* Envelope note (the standing minimum for rejection pins): the zod issue's
208+
* `code` and its (re-pathed) location are the whole envelope at this layer —
209+
* `status` does not exist here; the publish door wraps every schema refusal
210+
* uniformly (metadata-protocol's `422 INVALID_METADATA`, whose `issues[]`
211+
* carry these zod codes verbatim).
212+
*/
213+
describe('PostgresConfigSchema.url pg-grammar enforcement (#9091)', () => {
214+
it("refuses libpq's multi-host DSN — the form `pg` measurably cannot open", () => {
215+
// Measured on pg@8.22.0 / pg-connection-string@2.14.0: both `parse` and
216+
// `ConnectionParameters` throw `TypeError [ERR_INVALID_URL]` on this exact
217+
// value. It parsed green here until #9091.
218+
const result = PostgresConfigSchema.safeParse({
219+
url: 'postgresql://app@h1:5432,h2:5433/app',
220+
});
221+
222+
expect(result.success).toBe(false);
223+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'url');
224+
expect(issue, 'refusal must land at `url`').toBeDefined();
225+
expect(issue!.code).toBe('custom');
226+
expect(issue!.message).toContain('not a connection URL `pg` can open');
227+
// The message names the common cause and its working replacements.
228+
expect(issue!.message).toContain('multi-host');
229+
// The runtime-DSN carve-out, stated rather than implied (family convention).
230+
expect(issue!.message).toContain('OS_DATABASE_URL');
231+
});
232+
233+
it('re-paths the refusal at `config.url` through the datasource door', () => {
234+
const result = DatasourceSchema.safeParse({
235+
name: 'warehouse',
236+
driver: 'postgres',
237+
config: { url: 'postgresql://app@h1:5432,h2:5433/app' },
238+
});
239+
240+
expect(result.success).toBe(false);
241+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'config.url');
242+
expect(issue, 'refusal must be re-pathed at `config.url`').toBeDefined();
243+
expect(issue!.code).toBe('custom');
244+
expect(issue!.message).toContain('not a connection URL `pg` can open');
245+
});
246+
247+
it('refuses a non-numeric port — `pg` throws ERR_INVALID_URL on it', () => {
248+
const result = PostgresConfigSchema.safeParse({
249+
url: 'postgresql://db.example.com:notaport/app',
250+
});
251+
252+
expect(result.success).toBe(false);
253+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'url');
254+
expect(issue).toBeDefined();
255+
expect(issue!.code).toBe('custom');
256+
expect(issue!.message).toContain('not a connection URL `pg` can open');
257+
});
258+
259+
it('refuses a scheme-less non-URL — `pg` would resolve it against a placeholder host', () => {
260+
// `pg-connection-string` parses these via `new URL(str, 'postgres://base')`,
261+
// so they do NOT throw: pg would connect to the literal host `base` with
262+
// the authored text as the database name. Structurally unusable, refused.
263+
for (const url of ['not a url at all', 'host=localhost dbname=app']) {
264+
const result = PostgresConfigSchema.safeParse({ url });
265+
266+
expect(result.success, url).toBe(false);
267+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'url');
268+
expect(issue, `refusal for ${url} must land at \`url\``).toBeDefined();
269+
expect(issue!.code).toBe('custom');
270+
expect(issue!.message).toContain('no scheme');
271+
expect(issue!.message).toContain('`base`');
272+
}
273+
});
274+
275+
it('refuses the fs-reading query parameters, pointing at the datasource-level `ssl` block', () => {
276+
// `?sslcert=`/`?sslkey=`/`?sslrootcert=` make `parse` itself call
277+
// `fs.readFileSync` — a publish verdict must not depend on the validating
278+
// host's filesystem, and certificate material already has its declared
279+
// home (the same prescription the config-level `ca`/`cert`/`key` keys
280+
// carry).
281+
const result = PostgresConfigSchema.safeParse({
282+
url: 'postgresql://db.example.com/app?sslcert=/etc/ssl/client.pem',
283+
});
284+
285+
expect(result.success).toBe(false);
286+
const issue = result.error!.issues.find((i) => i.path.join('.') === 'url');
287+
expect(issue).toBeDefined();
288+
expect(issue!.code).toBe('custom');
289+
expect(issue!.message).toContain('?sslcert=');
290+
expect(issue!.message).toContain('datasource-level `ssl` block');
291+
});
292+
293+
it('mirrors `pg` exactly on the fs-param boundary: exact-case, non-empty value', () => {
294+
// Measured: `?SSLCERT=` is copied into the parsed config and read by
295+
// nothing (no fs touch), and an empty `?sslcert=` is falsy at the
296+
// parser's guard (no fs touch) — refusing either would narrow past what
297+
// `pg` does. Both stay accepted.
298+
for (const url of [
299+
'postgresql://db.example.com/app?SSLCERT=/etc/ssl/client.pem',
300+
'postgresql://db.example.com/app?sslcert=',
301+
]) {
302+
const result = PostgresConfigSchema.safeParse({ url });
303+
expect(result.success, JSON.stringify(result.error?.issues)).toBe(true);
304+
}
305+
});
306+
307+
it('reports the parse refusal ALONGSIDE the credential refusal on a value violating both', () => {
308+
// Composition pin: independent superRefines judge one value, each
309+
// reporting its own finding (#8082 userinfo + #9091 grammar here).
310+
const result = PostgresConfigSchema.safeParse({
311+
url: 'postgresql://user:pass@h1:5432,h2:5433/app',
312+
});
313+
314+
expect(result.success).toBe(false);
315+
const messages = result.error!.issues
316+
.filter((i) => i.path.join('.') === 'url')
317+
.map((i) => i.message);
318+
expect(messages.some((m) => m.includes('embeds a password'))).toBe(true);
319+
expect(messages.some((m) => m.includes('not a connection URL `pg` can open'))).toBe(true);
320+
});
321+
322+
it('accepts every measured shape `pg` genuinely opens', () => {
323+
for (const url of [
324+
// The documented single-host forms, credential-free ones included.
325+
'postgresql://db.example.com/app',
326+
'postgresql://user@db.example.com:5432/production',
327+
'postgres://host/db',
328+
// Empty-host libpq forms (default socket/localhost).
329+
'postgresql:///dbname',
330+
'postgresql://user@/mydb',
331+
// Unix-socket spellings: leading-`/` path, `socket:`, encoded host.
332+
'/var/run/postgresql',
333+
'socket:/var/run/postgresql?db=app',
334+
'postgresql://%2Fvar%2Frun%2Fpostgresql/mydb',
335+
// IPv6 host and non-credential, non-fs query parameters.
336+
'postgresql://user@[2001:db8::1]:5432/db',
337+
'postgresql://db.example.com/app?application_name=objectstack',
338+
]) {
339+
const result = PostgresConfigSchema.safeParse({ url, database: 'app' });
340+
expect(result.success, `${url}: ${JSON.stringify(result.error?.issues)}`).toBe(true);
341+
}
342+
});
343+
344+
it("leaves mongo's multi-host form untouched — the shared helper's leniency it must keep (#8696)", () => {
345+
// The #9091 parse check is per-driver BY DESIGN: for mongo the multi-host
346+
// DSN is a real, working, documented shape. Pin that it still parses.
347+
const result = MongoConfigSchema.safeParse({
348+
url: 'mongodb://app@h1:27017,h2:27017/app',
349+
});
350+
expect(result.success, JSON.stringify(result.error?.issues)).toBe(true);
351+
});
352+
});

0 commit comments

Comments
 (0)