fix(service-datasource): a declared ssl reaches the mysql client on both branches, in the spelling mysql2 accepts (#8874)

[os-project-manager]

Fixes#8874

A mysql datasource that declared TLS and wrote a config.url negotiated no TLS at all. buildMysqlConnection resolved the option and then returned before anything could use it — declared, resolved, dropped, with no diagnostic — while the discrete-fields branch of the same arm carried it. Whether a connection was encrypted therefore depended on which branch of one arm the datasource happened to take.

The fork the card poses ("wire it, or refuse loudly") is taken as ruled at triage: honour the declared channel. There is nothing to refuse. MysqlConfigSchema declares the key honoured with no branch caveat, its sslmode / tls / usessl aliases all rewrite to it, and where that schema means "not honoured, put it in the url" it says so in as many words — the charset guidance does exactly that, and ssl does not. The "TLS is a connection-string concern here" line the card half-remembers is the mongo arm's guidance, not this one's. And mechanically the branch can honour it: mysql2 reads a uri and the ssl option as separate channels, exactly as pg does.

The card's cost estimate was wrong in a way that mattered, twice

Half one — the no-secret sub-case is not one line. With a secret bound, #8696 already made the branch return { uri, password } and adding ssl there really is one line. With no secret bound the branch returned if (!spec.secret) return url; — a bare string, which has no key an ssl option can live in. That is the return-shape change the card flagged as deserving its own decision, and it is where most of this diff's care went.

Half two — the branch the card calls "honouring it" was throwing. Found while measuring half one. Measured on mysql2 3.23.1, lib/connection_config.js, no connection opened:

{host,port,database,user, ssl:true} -> TypeError: SSL profile must be an object, instead it's a boolean
{host,port,database,user, ssl:{}} -> ssl {rejectUnauthorized:true}
{uri:'mysql://app@db.internal:3306/app'} -> ssl false (the dropped DSN case)
{uri:'mysql://app@db.internal:3306/app', ssl:{}} -> ssl {rejectUnauthorized:true}

true is exactly what resolveSslOption answers for the two commonest declarations: ssl: { enabled: true } with no certificate material, and the config.ssl shorthand, whose schema is z.boolean() and therefore has no other authorable value. So the discrete-fields branch has been handing mysql2 a value that makes every acquireRawConnection throw. pg takes a boolean; mysql2 takes an object or a bundled profile name.

What was fixed here, and why the second half is not scope creep

Both halves, in one arm, because they are one defect wearing two spellings — and because emitting ssl: true onto the DSN branch to "honour" the declaration would have shipped the throw to a second branch. Delivering this card's own acceptance criterion (a declared sslreaches the client) requires the translation; applying it to only one branch would have planted the exact per-branch asymmetry this card exists to remove.

The translation is forced, not chosen: resolveSslOption's own comment already states the equivalence — "ssl: {} would read as 'TLS with default options' … which is what enabled: true with nothing else means anyway" — so mysqlSslOption re-expands the collapsed form for the one client that cannot read it. rejectUnauthorized: true on the result is mysql2's own default for an object (this.ssl.rejectUnauthorized = this.ssl.rejectUnauthorized !== false), not a verification policy invented here. Certificate objects, false, and a stored profile name ('Amazon RDS') all pass through untouched.

No new dependency. pg-connection-string, new in this package from #9090, is a postgres DSN parser and is deliberately not reached for — mysql2 parses its own uri, so no second dialect of mysql://… enters this repo.

The blast radius is exactly the broken population

The DSN branch returns an object instead of the bare string only when a declared ssl actually resolved (mysqlSsl !== undefined) — or when a secret is bound, unchanged from #8696. A datasource that declared neither still gets the byte-identical string knex has always parsed for it. That scoping is asserted, not merely intended: leaves a DSN with NOTHING declared exactly as it was and leaves a DSN with only a secret bound as the #8696 shape are two of the six cases that stay green under ablation.

Where the switch does happen the DSN moves from knex's own connection-string parser to mysql2's, so every non-TLS key it contributed has to arrive unchanged. Compared key-by-key on host / port / user / password / database / charset / timezone / connectTimeout / flags / socketPath / multipleStatements, across the bare-username, embedded-password, no-userinfo, portless, percent-encoded-username and query-parameter (?charset=, ?connectTimeout=, ?timezone=) forms: identical in every case. Pinned as a test, not measured once.

So the only datasources whose behaviour moves are the ones that were already broken — connecting in cleartext against their own metadata, or unable to connect at all.

A correction to an existing comment rides along, because this change depends on it: the #8696 paragraph describes mysql2's merge as filling in "keys the caller did not supply". It is actually if (options[key]) continue; — falsy, not absent. Harmless for a bound secret (always truthy), load-bearing for ssl, so it is now stated precisely.

The stale comment the card asked about

The in-code comment declaring the omission deliberate and "filed separately" is gone, replaced by the section documenting how it was closed.

The pin asserts at the client-resolution layer

Inherited from #9042 and #8873 rather than re-learned: every assertion reads what mysql2 resolved — ConnectionConfig, from the mysql2 module knex resolved for itself (knex.client.driver), fed the exact connectionSettings knex will hand it. Nothing asserts on the connection object the factory built, except the two places where the return shape is itself the claim (a string cannot carry an ssl key), and their comments say so.

Here that constraint is sharper than it was on the sibling cards: the boolean half is invisible at the config layer by construction — { ssl: true } is a perfectly good-looking object, and only the client's own parse says otherwise. new ConnectionConfig(settings) 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 and the tests at their fixed state: 8 failed / 6 passed, and specifically which. Measured exactly that set, case for case:

× carries a declared TLS block onto the DSN branch with no secret bound
AssertionError: expected false to deeply equal { rejectUnauthorized: true }
× carries TLS and a bound secret together on the DSN branch
AssertionError: expected false to deeply equal { rejectUnauthorized: false }
× carries certificate material onto the DSN branch verbatim
AssertionError: expected false to deeply equal { …(2) }
× carries the `config.ssl` on/off shorthand onto the DSN branch
AssertionError: expected false to deeply equal { rejectUnauthorized: true }
× returns an object rather than the bare DSN string once TLS is declared
AssertionError: expected 'string' to be 'object'
× honours a declared `enabled: false` on the DSN branch
AssertionError: expected 'string' to be 'object'
× resolves TLS on the discrete-fields branch instead of throwing on a boolean
TypeError: SSL profile must be an object, instead it's a boolean
× resolves the `config.ssl` shorthand on the discrete-fields branch too
TypeError: SSL profile must be an object, instead it's a boolean

The last two are the sharper direction and the reason this file pins a branch the card describes as working: they go red by a different route from every other failure — the old branch did carry the option, as a value the client refuses. The six that stayed green are the ones that must not move: the two blast-radius bounds, the parser-equivalence sweep (it compares two parses, not the fix), and the three discrete-branch cases whose declaration was never the broken spelling.

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). Nothing is built for it: these tests import the factory source through the .js-to-.ts test alias, so the ablated code is the code that ran.

Verification

All at 89c2e7ea, the final commit, tree clean.

pnpm --filter '@objectstack/service-datasource^...' build success (fresh-worktree closure)
pnpm --filter @objectstack/service-datasource test 21 files, 468 tests passed (14 new)
pnpm --filter @objectstack/service-datasource typecheck clean

Gates — everything node scripts/pm/dispatch-gates.mjs derived for the actual changed paths (seven path-matched, five convention-triggered by adding a test file), plus check:nul-bytes. All green:

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

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, green above), so nothing in this diff can move that ratchet. Its structural half is green above. CI runs it either way.

Out of scope, filed not fixed

• MysqlConfigSchema.ssl says "passed to mysql2 verbatim" — the arm now translates the boolean, so the doc line is stale #9125 — MysqlConfigSchema.ssl's doc line still says "passed to mysql2 verbatim", which this change makes stale for the boolean. It lands in packages/spec, a different verification surface (authorable-surface anchor, JSON-Schema projection), so it is a separate card rather than a rider. Nothing in this diff touches packages/spec.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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