Uh oh!
There was an error while loading. Please reload this page.
fix(service-datasource): inject a bound secret on the mongodb DSN branch - #9042
Merged
Merged
Conversation
…nch (#8696) buildMongoUrl's DSN branch returned the authored config.url verbatim and never applied spec.secret, so a mongo datasource that bound external.credentialsRef connected with whatever the URL carried — i.e. no credential at all, since #8082 refuses a user:password@ userinfo at the publish door. The credential now rides beside an unmodified url as MongoClient's `auth` option: no URL rewrite, so the multi-host and +srv forms are unharmed and no second dialect of mongodb:// enters this repo. The userinfo username `auth` also requires is read through @objectstack/spec/data's urlUserinfoUsername (#8876) and percent-decoded at the call site. A url naming no user is left alone rather than given a fabricated empty username. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 1 changed package(s). ✅ |
qq9340100
marked this pull request as ready for review
August 16, 2026 07:31
qq9340100
enabled auto-merge
August 16, 2026 07:32
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#8696
The card's last arm. PR #8875 closed the mysql half with
Part of; this closes mongodb, so the card closes with it.The defect
buildMongoUrl's DSN branch returned the authoredconfig.urlverbatim and appliedspec.secretnowhere. A mongo datasource that boundexternal.credentialsReftherefore connected with whatever the URL itself carried — which, since #8082 refuses auser:password@userinfo at the publish door, is no credential at all. Re-measured onorigin/main@792524c22before any edit (the card's:542was stale; the branch was at:613), mongodb 7.5.0:The connect path is fail-closed on a ref it cannot resolve, so "the datasource connected" reads as "the bound credential was used". It was not — declared, resolved, injected into the factory, then dropped at the last call site with no diagnostic. The arm's behaviour was decided by whether the operator happened to author a URL: the composed branch five lines below has honoured the secret since #4410.
The fix:
options.authbeside an unmodified urlMongoDBDriveralready spreadsconfig.optionsintoMongoClientOptions, soauthis a live channel. Measured on mongodb 7.5.0 — theMongoClientconstructor resolves credentials eagerly, so all of this is assertable with no server:So the authored URL is handed over byte for byte — no rewrite, no re-encoding of the secret, no second dialect of
mongodb://…in this repo. The multi-host and+srvforms ride through unharmed, which a rewrite could not have promised.The userinfo username that
authalso requires is read through the platform's own DSN grammar —urlUserinfoUsernamefrom@objectstack/spec/data(#8876) — and percent-decoded at the call site, since the accessor returns the raw component by contract and the client decodes the same component from the URL itself (a%2Fbauthenticates asa/b; handing the raw value through would authenticate as a different user). Hand-rolling that parse here is the shape #8082's ruling rejects by name, andnew URL()cannot even parse the multi-host form this schema documents (ERR_INVALID_URL, measured).⛔ Not reached by symmetry with mysql. The clients merge a DSN against explicit keys in opposite directions —
pgmergesparse(connectionString)over the explicit config, which is why the postgres arm looks correct and is broken one layer lower (#8873). Each arm's precedence is measured against its own client; this one needed a different mechanism from #8875's{ uri, password }to reach the same precedence rule.Two decisions, made deliberately
A URL that names no user gets nothing.
authis not constructible from a password alone (MongoParseError: credentials must be an object with 'username' and 'password' properties), and inventing an empty username is measurably worse than silence:mongodb://db.internal:27017/appcarries no credentials at all today, and would carry{username:''}— a guaranteed handshake failure — if the arm injected regardless. Injection happens only where the URL already declares authenticated intent. That also matches what the composed branch has always done with the same input, so no new per-branch asymmetry is planted. Making that contradictory pair loud belongs at the authoring door, where both halves are visible at once — filed as #9041, not guessed at here.An embedded userinfo password is left in place. The bound secret wins over it anyway (measured above), so removing it would be a URL rewrite bought for nothing. Same precedence the mysql arm states, reached by a different mechanism.
The pin
Extends
__tests__/bound-secret-dsn-branches.test.ts— the mysql half's own file — rather than opening a parallel one. Every mongo assertion readsMongoClient's resolvedcredentials, never the URL string this factory built: a test assertingbuildMongoUrl's return value would have passed throughout this defect's life, and the postgres arm passes the equivalent config-layer assertion while still being broken below it.Reverse verification, direction predicted in writing first (recorded in the file header before the run): with the pre-fix arm restored and the tests at their fixed state, the six injecting cases go RED on the password and the five remaining cases stay GREEN — the passthrough-preservation control, the two no-injection cases where both versions agree, and the two composed-branch controls, because the defect is branch-local. Measured exactly that: 6 failed / 5 passed.
The fix was committed before the ablation; restored with
git checkout HEAD -- ...,git status --porcelainclean afterwards (noMM).Verification
Union re-run after the final commit, at HEAD
7cd52556c, workspace closure built first. Gate output redirected to files and each exit code read — never piped.pnpm --filter @objectstack/service-datasource test→Test Files 19 passed (19) / Tests 443 passed (443);typecheckclean.node scripts/pm/dispatch-gates.mjsover the three changed paths), all green: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, plus the convention-triggeredcheck:query-options-erasure,check:type-check-coverage,check:type-check-debt --re-measure(33 ledger entries re-measured, none above its recorded number),check:engine-double-contract,check:where-matcher.check:slot-lookup(unnamed forpackages/**source changes and known to redden CI anyway),check:nul-bytes(any edit),check:error-code-casing— no error code is minted here, this arm's behaviour carries no new code, so it is a confirmation rather than a requirement.Blast radius is exactly the broken class: a datasource that binds no secret reaches the client byte-for-byte as before, and the
optionspassthrough keeps arriving verbatim — the injectedauthis merged into it, not assigned over it, and a resolvedcredentialsRefoutranks anauthblock written there by hand.Filed in passing, unassigned, not fixed here
config.options.auth.passwordis a fourth spelling of an inline credential — authorable, persisted cleartext, unredacted, and read by the client #9040 —config.options.auth.passwordis a fourth spelling of an inline credential: authorable (parses clean), persisted cleartext insys_metadata, unredacted on the admin read path (redactedKeys: [], measured), and read by the client. One syntax over from [security]sys_metadata.metadatais a general cleartext sink: any authored artefact whose schema permits an inline credential lands it there (datasourceconfig.password, connectorauthentication) #7990 / [Decision] URL-embedded credentials (user:password@hostin driverconfig.url) remain a live cleartext door after #7990 — refuse at publish, or accept as residual risk? #8082 / turso:?authToken=in an authoredconfig.url/config.syncUrlquery string is credential material the #8082 userinfo refusal does not cover #8337, and it lands inpackages/spec.external.credentialsRefbound + aconfig.urlnaming no user" — the binding is a silent no-op at connect #9041 — nothing refuses the contradictory pair "credentialsRefbound + a URL naming no user"; the authoring-door half of this card's remaining silence.Generated by Claude Code