fix(security): a bound credentialsRef reaches the postgres SERVER on the DSN branch, not just the knex config (#8873)

[os-project-manager]

Fixes#8873

A postgres datasource whose config.url is a DSN and whose credential is bound through external.credentialsRef opened its connection with no password at all. Severity as ratified at triage: a broken binding, not a disclosure — the operator sees a bound credential and a datasource reporting connected, while the handshake carried nothing.

The measurement the card asked for

The card named two candidate remedies and deliberately declined to choose, asking for the measurement first. Here it is, driven against pg 8.22.0's own ConnectionParameters and knex 3.3.0 (no connection opened anywhere).

Why the arm looked correct

// pg 8.22.0, lib/connection-parameters.jsif(config.connectionString){config=Object.assign({},config,parse(config.connectionString))}

{ connectionString, password } loses the credential twice over, by two independent mechanisms, either sufficient alone:

1. parse() emits a password key for every url — measured "" for a credential-free DSN, not absent — and Object.assign copies it over the injected value. val('password', …) then reads "", falls through to PGPASSWORD and the defaults, and resolves null.
2. knex hides the key first: setHiddenProperty makes password a non-enumerable own property of connectionSettings, and Object.assign copies only enumerable ones — so it never reaches the merge at all. (This one is not in the card; it is what makes the real knex→pg path lose the secret even before the parse.)

Measured before the fix:

postgresql://app@db.internal:5432/app + bound secret -> password null
postgresql://app:embedded-legacy@db.internal/app + bound secret -> password 'embedded-legacy'

Since #8082 refuses a user:password@ userinfo at the publish door, the credential-free DSN is the only authorable URL shape — so this is the shape the connection form produces.

The DSN shapes the card asked to be measured, before and after

stored config.url	before	after
postgresql://app@db.internal:5432/app (credential-free — the only #8082-authorable shape)	null	bound secret
postgresql://app:embedded-legacy@db.internal:5432/app (stored pre-#8082 row)	'embedded-legacy'	bound secret
postgresql://db.internal:5432/app (names no user)	null	bound secret
postgresql://app@db.internal:5432/app?password=… (stored pre-#8337 row)	'from-query-param'	bound secret
any of the above with no secret bound	unchanged	unchanged, byte for byte

Candidate B — re-serialise the userinfo — was measured and rejected

Two counts, both measured:

• It does not even fix the defect.pg-connection-string honours a ?password= query parameter over userinfo (the reason turso: ?authToken= in an authored config.url / config.syncUrl query string is credential material the #8082 userinfo refusal does not cover #8337 refuses that spelling at authoring). A stored pre-turso: ?authToken= in an authored config.url / config.syncUrl query string is credential material the #8082 userinfo refusal does not cover #8337 row with the secret spliced into the userinfo still resolves to from-query-param — the bound credential still loses.

• It would materialise the cleartext credential into a string nothing hides. Measured on knex 3.3.0, JSON.stringify(client.connectionSettings):

  discrete password : {"host":"db.internal","port":5432,"database":"app","user":"app"} secret absent
  spliced userinfo : {"connectionString":"postgresql://app:s3cr3t…@db.internal:5432/app"} secret present
  

  That is also the shape [Decision] URL-embedded credentials (user:password@host in driver config.url) remain a live cleartext door after #7990 — refuse at publish, or accept as residual risk? #8082 refuses to let anyone author and redactUrlPassword exists to scrub — synthesising it at connect time pushes the platform's hardest-to-redact credential spelling back into circulation.

The fix

On the DSN branch, and only when a secret is bound, connectionString is dropped: the arm hands pgpg's own parse of the url (pg-connection-string — the client's parser, so no second dialect of postgresql://… lives in this repo to drift out of agreement) with the credential applied after it, where nothing re-parses over it.

⛔ Not reached by symmetry with either sibling, which is the card's central point: mysql2 lets the explicit key win ({ uri, password }, #8875), mongodb rides in options.auth beside an untouched url (#9042), pg lets the DSN win. Three clients, three shapes, each measured against its own client. The two stale ⛔ comments the sibling fixes left at the mysql and mongo arms — which described this arm as a live defect — are updated to say how it was fixed.

Equivalence, verified key-by-key. Dropping connectionString means pg no longer reads the url itself, so every non-credential key it contributed has to arrive by the new route unchanged. Compared across the sslmode, unix-socket, ?options=, ?application_name=, credential-free, embedded-password and no-userinfo forms, on user / database / port / host / ssl / application_name / statement_timeout / options / connect_timeout / client_encoding: identical in every case. That sweep is pinned in the test file, not just done once here.

Two deliberate decisions worth review:

• A url naming no user still gets the credential, unlike the mongodb arm's deliberate no-op there. The asymmetry is mechanical, not a second policy: MongoClient cannot carry a password without a username and {username:''} would turn a working anonymous connection into a guaranteed failure, whereas pg sends a password only when the server asks for one — so injecting cannot break a datasource that connects today, and refusing would silently drop a credential the operator bound. Making that contradictory pair loud belongs at the authoring door, Nothing refuses the contradictory pair "external.credentialsRef bound + a config.url naming no user" — the binding is a silent no-op at connect #9041, which this card lands before.
• A url pg's own parser rejects is now refused when the driver is built, rather than as a bare Invalid URL on first query. Measured: parse and new ConnectionParameters both throw ERR_INVALID_URL on a multi-host DSN, so such a datasource has never been able to connect — the failure moves earlier and gets named, it is not invented. Both create() call sites already turn a throw into a located datasource failure rather than a boot crash. The url is deliberately not echoed in the message: it may itself embed a credential, which is why pg redacts it in its own error.

The pin asserts at the client-resolution layer

Inherited from #9042's review and the single most important constraint on this card: every credential assertion reads what the pg client resolved — via the pg module knex itself will use (knex.client.driver.Client) fed the exact connectionSettings knex will hand it. Nothing asserts on the connection object the factory built, because that assertion passed throughout this defect's entire life. The one config-layer assertion in the file is about the mechanism instead (connectionString is absent once a secret is bound), and its comment says so.

new Client(config) resolves connectionParameters in its constructor and does no I/O, which is what makes the seam assertable without a server.

Reverse verification — predicted in writing, then measured

Predicted before running, with the pre-fix branch restored over the tests at their fixed state: 8 failed / 3 passed, and specifically which. Measured exactly that set, case for case:

× resolves the bound secret as the handshake password instead of nothing
AssertionError: expected null to be 's3cr3t-from-sys_secret'
× lets the bound secret win over a legacy password embedded in a stored DSN
AssertionError: expected 'embedded-legacy' to be 's3cr3t-from-sys_secret'
× wins over a `?password=` query parameter
AssertionError: expected 'from-query-param' to be 's3cr3t-from-sys_secret'
× carries the credential on a DSN that names no user
AssertionError: expected null to be 's3cr3t-from-sys_secret'
× keeps the datasource's own `ssl` block and pg extras reaching the client
AssertionError: expected null to be 's3cr3t-from-sys_secret'
× changes NOTHING a bound secret does not touch (equivalence sweep)
AssertionError: expected null to be 's3cr3t-from-sys_secret'
× hands `pg` no `connectionString` to re-parse once a secret is bound
AssertionError: expected 'postgresql://app@db.internal:5432/app' to be undefined
× refuses a DSN pg cannot parse, without echoing the url
promise resolved instead of rejecting (red by a DIFFERENT route)
✓ leaves a DSN with nothing bound exactly as it was
✓ still reads the bound secret on the discrete-fields branch (control)
✓ keeps preferring the bound secret over an inline `config.password` (control)

The two stored-row failures are the sharper direction: the credential is not merely missing, it is someone else's. The three green are the three that must not move — the defect is branch-local, and an arm-wide regression would mean the file measures something else. The ablation was restored from the commit and proven byte-identical (git hash-object on the restored file equals git rev-parse of the same path at HEAD).

New dependency

pg-connection-string (^2.14.0, MIT, zero runtime dependencies) joins @objectstack/service-datasource's dependencies. It is pg's own parser, and using it rather than modelling it is the whole safety argument: the fields handed to pg are the fields pg would have derived, by construction. It also carries the parts a partial parse would silently drop — that package copies every query parameter into the config, which is how ?sslmode=, ?application_name=, ?options= and ?connect_timeout= reach the client at all. Confirmed externalized rather than bundled in both build outputs. Three sibling services already carry third-party dependencies (ioredis, croner, @noble/ciphers), so this is not a new kind of thing for packages/services.

Verification

All at e36872f42, the final commit, with the tree clean.

pnpm --filter @objectstack/service-datasource typecheck clean
pnpm --filter @objectstack/service-datasource test 20 files, 454 tests passed (11 new)
pnpm --filter @objectstack/service-datasource build success, dependency externalized

Gates — the two the dispatch named, plus everything node scripts/pm/dispatch-gates.mjs added for the actual changed paths (five changeset-driven families the path list could not see, and five convention-triggered families that adding a test file moves). All green:

check:test-source-alias · check:type-source-resolution · check:changeset-gate-self-tests
check:objectui-changeset · check:query-options-erasure · check:engine-double-contract
check:where-matcher · check:type-check-coverage · check:nul-bytes
check-adr-0087-registration · check-changeset-no-major · check-empty-changeset
check-type-check-coverage --self-test

Not run locally: check:type-check-debt --re-measure. It needs the whole workspace closure built and re-runs tsc per ledger entry — service-datasource carries no entry (it declares a real typecheck script and passes it), so nothing in this diff can move that ratchet. Its structural half is green above. CI runs it either way.

Scope note: #8874 is not addressed here — the mysql arm of this same file stays queued and unassigned, per the dispatch. Nothing in this diff touches buildMysqlConnection beyond correcting the stale ⛔ comment that called this arm a live defect.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

No hand-written docs reference the 1 changed package(s). ✅