fix(sessions): decode Cloud SQL Unix-socket URIs in getConnectionOptionsFromUri

[AnupamKumar-1]

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

• Closes: getConnectionOptionsFromUri fails for Cloud SQL Unix-socket connection URIs (percent-encoded host) #799

Problem:
getConnectionOptionsFromUri() passes Postgres URIs to MikroORM as clientUrl. Cloud SQL Unix-socket URIs aren't handled: they either cause Invalid URL, or the socket path gets treated as a hostname and sent to DNS instead of being used as a Unix socket. The ?host=/cloudsql/... form is also not handled.

Solution:
Added parsePostgresSocketUri() to detect the supported Unix-socket forms and return the socket path as an explicit host, along with the other connection fields. Normal TCP Postgres URIs are unaffected.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

core/test/sessions/db/operations_test.ts — added coverage for:

• Unix-socket host with unescaped colons in the instance name (throws under
  new URL(), resolved via manual parsing to an explicit host)
• The ?host=/cloudsql/... query-param form (same, explicit host)
• Percent-encoded colons (%3A) in the instance name — already resolve
  correctly today via MikroORM's own clientUrl handling, left untouched
• schema query param preservation for socket URIs
• Parity: non-schema query params (e.g. sslmode) were already dropped by
  MikroORM before this change and still are — not a regression

Manual Verification (URI resolution, not a live Cloud SQL connection):
No live Cloud SQL instance was available, so this verifies getConnectionOptionsFromUri() resolves the correct options rather than testing an actual socket connection.

import('./dist/esm/sessions/db/operations.js').then(async (m) => {
  const uri = 'postgresql://USER:PASS@%2Fcloudsql%2Fmy-project:us-central1:my-instance/mydb';
  const options = await m.getConnectionOptionsFromUri(uri);
  console.log(options.host, options.user, options.password, options.dbName, options.clientUrl);
});

/cloudsql/my-project:us-central1:my-instance USER PASS mydb undefined

Confirmed for the unescaped-colon form shown above: resolves to an explicit host with no Invalid URL and no clientUrl set. The ?host= form behaves the same way (see unit tests). Percent-encoded colons and a plain TCP URI both resolve via the existing clientUrl path, unaffected by this change.

Checklist

• I have read the CONTRIBUTING.md document.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• I have manually tested my changes end-to-end.
• Any dependent changes have been merged and published in downstream modules.

Additional context

Root cause: new URL(clientUrl) throws on unescaped colons and on the ?host=/cloudsql/... query-param convention. Percent-encoded colons (%3A) already resolve correctly today via decodeURIComponent(url.hostname), so only the two throwing forms are handled here.

[kalenkevich]

Thanks for tracking this down — the ?host= and unescaped-colon forms genuinely throw today, so the fix is worth having. Two things before it lands: a port regression, and the stated root cause.

Root cause is inaccurate. MikroORM already decodes the percent-encoded host (decodeURIComponent(url.hostname) in Connection.getConnectionOptions()), so the %3A-escaped form resolves to /cloudsql/... correctly today. Only the unescaped-colon and ?host= forms throw. "Confirmed for all three URI forms" overstates what this changes.

Resolved through a real PostgreSqlDriver:

URI	before	after
…@%2Fcloudsql%2Fp%3Ar%3Ai/mydb	host=/cloudsql/p:r:i port=0 ✅	host=/cloudsql/p:r:i port=5432
…@%2Fcloudsql%2Fp:r:i/mydb	Invalid URL ❌	host=/cloudsql/p:r:i port=5432
…@/mydb?host=/cloudsql/p:r:i	Invalid URL ❌	host=/cloudsql/p:r:i port=5432
…@%2Fvar%2Frun%2Fpostgresql:5433/mydb	host=/var/run/postgresql port=5433 ✅	host=/var/run/postgresql:5433 port=5432 ❌

That last row is the blocking one — details inline.

[kalenkevich]

Regression: this drops the port. postgresql://user@%2Fvar%2Frun%2Fpostgresql:5433/mydb resolves to host=/var/run/postgresql port=5433 today, and to host=/var/run/postgresql:5433 port=5432 after this change. pg builds the socket as host + '/.s.PGSQL.' + port.

[AnupamKumar-1]

Fixed — this URI parses fine with new URL(), so it's no longer intercepted. Verified with the real PostgreSqlDriver: port: 5433 (was 5432 before the fix).

[kalenkevich]

Suggest returning {clientUrl: uri, host: socket.host, ...} when new URL(uri) succeeds — MikroORM lets an explicit host win over clientUrl (Connection.getConnectionOptions()), so port/user/password/dbName/schema keep their current handling. Only fall back to the fully-manual parse for URIs that actually throw.

[AnupamKumar-1]

Went with full reconstruction instead, since MikroORM’s getConnectionOptions() still calls new URL(clientUrl) internally. Passing a clientUrl that fails parsing would therefore fail regardless of an explicit host override. Manual reconstruction avoids relying on clientUrl entirely for the cases where it cannot be parsed.

Confirmed empirically: passing {clientUrl: 'postgresql://user:pass@%2Fcloudsql%2Fp:r:i/mydb', host: '/cloudsql/p:r:i'} still throws Invalid URL — the explicit host never gets a chance to override it.

[kalenkevich]

The postgres:// / postgresql:// prefix check already ran at line 37; fold this block into that branch.

[AnupamKumar-1]

Done — restructured to a single-pass if/return chain, one branch per URI scheme, no duplicate check.

[kalenkevich]

Needs a doc comment saying why new URL() can't express this.

[AnupamKumar-1]

Added, on both parsePostgresSocketUri and the new buildPostgresOptions.

[kalenkevich]

An unescaped @ in the password (e.g. the Cloud SQL IAM user@project.iam form) fails to match here and then throws Invalid URL downstream. WHATWG URL handles it.

[AnupamKumar-1]

Fixed — userinfo is now captured greedily up to the last @ (mirrors the WHATWG algorithm), then split on the first :. Added a test for exactly this case.

[kalenkevich]

socket isn't a libpq or pg parameter and nothing else in the repo reads it. Drop it.

[AnupamKumar-1]

Removed.

[kalenkevich]

postgresql://u:p@%2Ftmp/ yields dbName: '', which suppresses the driver default. || undefined.

[AnupamKumar-1]

Fixed — normalizes to undefined via || undefined.

[kalenkevich]

Tautological — for TCP the function returns only {clientUrl}, so these assertions can't fail. The parity claim is about MikroORM, which this doesn't exercise.

[AnupamKumar-1]

Replaced with a test that instantiates the real PostgreSqlDriver and asserts on driver.getConnection().getConnectionOptions() directly, demonstrating that MikroORM itself drops sslmode/connect_timeout rather than just that our function doesn’t add them.

[kalenkevich]

Casts unnecessary; host, user and schema are typed on Options. Only password needs one.

[AnupamKumar-1]

Fixed — removed casts on host, user, schema; kept only password.

[kalenkevich]

Missing coverage for a socket URI with an explicit port, and for a plain TCP URI still falling through to clientUrl.

[AnupamKumar-1]

Both added, along with tests for the @-in-password and empty-dbName cases.

[AnupamKumar-1]

Thanks for tracking this down — the ?host= and unescaped-colon forms genuinely throw today, so the fix is worth having. Two things before it lands: a port regression, and the stated root cause.

Root cause is inaccurate. MikroORM already decodes the percent-encoded host (decodeURIComponent(url.hostname) in Connection.getConnectionOptions()), so the %3A-escaped form resolves to /cloudsql/... correctly today. Only the unescaped-colon and ?host= forms throw. "Confirmed for all three URI forms" overstates what this changes.

Resolved through a real PostgreSqlDriver:

URI before after
…@%2Fcloudsql%2Fp%3Ar%3Ai/mydb host=/cloudsql/p:r:i port=0 ✅ host=/cloudsql/p:r:i port=5432
…@%2Fcloudsql%2Fp:r:i/mydb Invalid URL ❌ host=/cloudsql/p:r:i port=5432
…@/mydb?host=/cloudsql/p:r:i Invalid URL ❌ host=/cloudsql/p:r:i port=5432
…@%2Fvar%2Frun%2Fpostgresql:5433/mydb host=/var/run/postgresql port=5433 ✅ host=/var/run/postgresql:5433 port=5432 ❌
That last row is the blocking one — details inline.

Confirmed and fixed — thanks for the detailed table, that made it easy to verify.

Root cause corrected: the fix now tries new URL() first; only URIs it genuinely can’t parse (unescaped colons, ?host=) fall back to manual parsing. The escaped-colon case goes through the existing clientUrl path untouched, exactly as it did before — so it’s not “confirmed for all three forms,” it’s the two that actually throw.

Verified against the real PostgreSqlDriver with your four cases — host and port match the expected behavior in your table, including the previously-blocking explicit-port row:

explicit port => host: /var/run/postgresql | port: 5433

[AnupamKumar-1]

@kalenkevich Fixed the CI type-check issue. Parameters<> was incorrect for the Configuration class, so I switched it to ConstructorParameters<>.

23/23 tests pass, the full core suite (3356 tests) is green, and lint/Prettier are clean. The remaining 4 @google/adk-devtools errors are pre-existing on main and unrelated to this PR:

• tests/integration/adk_web/webui_test.ts:7
• tests/integration/build_setup/ts_commonjs/devtools_check.ts:6
• tests/integration/build_setup/ts_commonjs_native_addon/verify_devtools.ts:6
• tests/integration/build_setup/ts_esm/devtools_check.ts:6

[kalenkevich]

found one more issue

[kalenkevich]

This branch ignores parsedUrl.port. postgresql://user:pass@localhost:5433/mydb?host=/var/run/postgresql resolves to port=5432 here, versus 5433 on main — same port drop as the one just fixed, in the new branch. The test should resolve the host query param even when new URL() otherwise succeeds covers this path but doesn't assert port.

Suggested change

	if (queryHost?.startsWith('/')) {
	const schema = parsedUrl.searchParams.get('schema');
	return {
	entities: ENTITIES,
	driver,
	host: queryHost,
	user: parsedUrl.username
	? decodeURIComponent(parsedUrl.username)
	: undefined,
	password: parsedUrl.password
	? decodeURIComponent(parsedUrl.password)
	: undefined,
	dbName: decodeURIComponent(parsedUrl.pathname.slice(1)) || undefined,
	...(schema ? {schema} : {}),
	if (queryHost?.startsWith('/')) {
	const schema = parsedUrl.searchParams.get('schema');
	return {
	entities: ENTITIES,
	driver,
	host: queryHost,
	user: parsedUrl.username
	? decodeURIComponent(parsedUrl.username)
	: undefined,
	password: parsedUrl.password
	? decodeURIComponent(parsedUrl.password)
	: undefined,
	dbName: decodeURIComponent(parsedUrl.pathname.slice(1)) || undefined,
	...(parsedUrl.port ? {port: Number(parsedUrl.port)} : {}),
	...(schema ? {schema} : {}),

[AnupamKumar-1]

Fixed — the ?host= branch read username/password/pathname/searchParams off parsedUrl but never port, so it silently dropped to pg's default 5432. Added port: Number(parsedUrl.port) alongside the existing schema handling, and updated the test you flagged to assert on it.

Verified with the real driver: port: 5433 now resolves correctly. 23/23 targeted tests pass, and the core build, ESLint, and Prettier checks are green.

[ScottMansfield]

The fix works for the case that was reported. Two things I'd change, and the scope is narrower than the bug.

I ran every URI form through the built getConnectionOptionsFromUri rather than reading the diff.

unescaped colons (the issue's repro)   host="/cloudsql/proj:us-central1:inst" user="u" password="p" dbName="db"
?host= param                           host="/cloudsql/proj:r:i" user="u" password="p" dbName="db"
fully %-encoded colons                 clientUrl="…"   (unchanged, correct)
plain TCP                              clientUrl="…"   (unchanged, correct)

The userinfo handling is genuinely careful and I tried hard to break it: a password containing @, an IAM user in svc@proj.iam form, no password, and percent-encoded credentials all parse correctly. Splitting on the last @ is the right call and the comment explains why.

You were right to leave the percent-encoded form alone — and that means the issue's root cause is wrong

#799 says the percent-encoded host "is never decoded". It is: MikroORM's Connection.getConnectionOptions() does decodeURIComponent(url.hostname) (@mikro-orm/core/connections/Connection.js:62), and I confirmed postgresql://u:p@%2Fcloudsql%2Fproj%3Ar%3Ainst/db resolves correctly today with no change.

What actually breaks is the reporter's repro URI, which percent-encodes the slashes but leaves the colons raw — that throws Invalid URL. So the issue's title and diagnosis point at the wrong thing while its reproduction is right, and your fix does close it. Worth saying so on #799 explicitly, so nobody later "fixes" the path that already works.

---

1. Scope: the same bug is live on mysql:// and mariadb://

Both still forward the raw URI (operations.ts:163-185), and Cloud SQL for MySQL uses the identical /cloudsql/PROJECT:REGION:INSTANCE socket convention. Measured:

mysql   socket, unescaped colons    clientUrl="mysql://u:p@%2Fcloudsql%2Fproj:r:inst/db"
mysql   socket, %-encoded           clientUrl="mysql://u:p@%2Fcloudsql%2Fproj%3Ar%3Ainst/db"
mysql   ?host= form                 clientUrl="mysql://u:p@/db?host=/cloudsql/proj:r:i"
mariadb socket, unescaped colons    clientUrl="mariadb://u:p@%2Fcloudsql%2Fproj:r:inst/db"

A Cloud Run deployment on Cloud SQL for MySQL hits exactly what #799 describes, and this PR doesn't help it.

But generalising is not mechanical, which is worth knowing before anyone asks you to just widen the prefix check. The drivers disagree on how a socket is expressed:

• pg treats a host beginning with / as a domain socket (pg/lib/connection-parameters.js:104-105) — which is why your host: mapping is correct here.
• mysql2 and mariadb use a separate socketPath option (mysql2/lib/connection_config.js:52, mariadb/lib/config/connection-options.js:104); a /-prefixed host means nothing to them.

So the parsing generalises cleanly and the option mapping does not. If you want to set this up for a follow-up without doing the work now, the cheap move is to split parsePostgresSocketUri into a scheme-agnostic parseSocketUri(uri) returning {host, user, password, dbName, schema, port}, and leave the driver-specific host vs socketPath decision at the call site. That's a rename and a signature change today, and it means the MySQL fix is later just a second call site. Happy for the actual MySQL support to be its own PR.

2. Security: ?host= now silently overrides a real TCP authority

This is the one I'd want changed before merge.

postgresql://u:secret@real-db.example.com:5432/db?host=/tmp/evil
  ->  host="/tmp/evil" port=5432 user="u" password="secret" dbName="db"

The hostname is dropped and the credentials are sent to a local Unix socket instead. Before this PR ?host= was inert — MikroORM read url.hostname and ignored it — so this is new capability introduced here.

I want to be fair about it: libpq behaves the same way, so as a compatibility decision it is defensible, and the URI is operator configuration rather than attacker input, which keeps the severity down. What makes it worth addressing anyway is that /tmp is world-writable on a normal Linux host, so any local user can plant a listening socket and collect the database password the next time the agent connects — and nothing in the current code makes that trade visible to whoever writes the URI.

Details and suggestion inline.

3. A parsing bug: the userinfo group isn't bounded to the authority

One character class. Details inline, with a before/after.

Smaller notes

• No validation of the socket path at all: ?host=/etc and ?host=/../../tmp/x are accepted verbatim. I don't think you should restrict it to /cloudsql/ — that would break a perfectly good local /var/run/postgresql socket — but the feature is described throughout as Cloud SQL while accepting any absolute path, and that gap is worth a sentence in the JSDoc.
• Credential redaction survives, which I checked because this PR adds parsing paths that see the password: oracle://user:SUPERSECRET@host/db still throws Unsupported database URI: oracle://user:***@host/db. #602's protection is intact and none of the new code logs the URI.
• 23 tests pass, ts:check / lint / format:check clean locally.
• CI on this PR shows only check-changes and cla/google — the fork workflow gate again, so the matrix has not run on 52ebdab. A maintainer will need to approve it.

_{Automated review · CloudCode session ses_e5fd9d69397ffe38frdSpP16Ue}

[ScottMansfield]

?host= silently wins over a real TCP authority, and the credentials follow it.

Measured against the built module at this head:

postgresql://u:secret@real-db.example.com:5432/db?host=/tmp/evil
  ->  host="/tmp/evil"  port=5432  user="u"  password="secret"  dbName="db"

real-db.example.com is discarded with no error and no warning, the port is carried over from the authority that was just ignored, and the password is handed to whatever is listening on /tmp/evil. The manual path at :129 does the same thing for an authority new URL() can't parse.

This is new. Before this PR, ?host= reached MikroORM inside clientUrl and was ignored entirely — Connection.getConnectionOptions() reads url.hostname. So the ability for a query parameter to redirect where credentials are sent is introduced here.

Being even-handed about severity: libpq resolves host the same way, so matching it is a reasonable compatibility choice, and --session_service_uri is operator config rather than attacker input. What keeps it on my list anyway is that /tmp is world-writable on a stock Linux host — any local user can bind a socket there and harvest the database password on the next connect — and nothing about the current code signals that a ?host= in a URI overrides the host you can plainly see in it.

Three options, in the order I'd pick them:

1. Warn when both are present. Keeps libpq compatibility, makes the override visible:

   if (queryHost?.startsWith('/') && parsedUrl.hostname) {
     logger.warn(
       `Connection URI names host '${parsedUrl.hostname}' but the ?host= parameter ` +
       `overrides it with the Unix socket '${queryHost}'; connecting to the socket.`,
     );
   }

   redactUriPassword is already imported if you'd rather log the URI.

2. Only honour ?host= when the authority has no hostname, which is the shape every real Cloud SQL URI uses (postgresql://u:p@/db?host=/cloudsql/...). Stricter, and it makes the combination an error rather than a silent preference.

3. Leave it and document the precedence in the JSDoc on buildPostgresOptions.

I'd take (1). Whichever you choose, the precedence should be a decision the code states rather than a consequence of which branch is checked first.

One more thing on this branch specifically: port is forwarded (:122) even though the resolved host is a Unix socket, where a TCP port is meaningless. pg ignores it, so nothing breaks — but it means the emitted options carry a port for a socket connection, which is confusing to read in a debug dump. Dropping it when queryHost wins would be tidier.

[ScottMansfield]

The userinfo group isn't bounded to the authority, so a socket URI with an @ later in it misparses.

(.*)@ is greedy across the entire URI, including the path and query. The doc comment says this mirrors the WHATWG algorithm — WHATWG does split userinfo on the last @, but only within the authority, which ends at the first /, ? or #. Unbounded, an @ anywhere downstream captures everything before it:

postgresql://%2Fcloudsql%2Fa:b:c/db?opt=x@y
  current  userinfo="%2Fcloudsql%2Fa:b:c/db?opt=x"   authority="y"
  bounded  userinfo=(none)                            authority="%2Fcloudsql%2Fa:b:c"

With the current expression authority is "y", which doesn't start with /, so parsePostgresSocketUri returns null and the URI falls through to clientUrl — where new URL() has already failed, so it fails again at connect time with the original confusing error. A valid socket URI that happens to carry an @ in a query parameter is silently not fixed.

Bounding the class to the authority fixes it and is strictly better — I checked the two cases the current expression exists to handle and both are unchanged:

postgresql://u:p@ss@%2Fcloudsql%2Fa:b:c/db
  current  userinfo="u:p@ss"           authority="%2Fcloudsql%2Fa:b:c"
  bounded  userinfo="u:p@ss"           authority="%2Fcloudsql%2Fa:b:c"

postgresql://svc@proj.iam:p@%2Fcloudsql%2Fa:b:c/db
  current  userinfo="svc@proj.iam:p"   authority="%2Fcloudsql%2Fa:b:c"
  bounded  userinfo="svc@proj.iam:p"   authority="%2Fcloudsql%2Fa:b:c"

Greedy matching within the bounded class still gives you last-@ semantics, so the IAM-user case keeps working.

const match =
  /^postgres(?:ql)?:\/\/(?:([^/?#]*)@)?([^/?#]*)(\/[^?#]*)?(?:\?([^#]*))?$/.exec(uri);

I also added # to the path and query classes: a fragment is meaningless in a connection URI, but excluding it keeps the groups matching what WHATWG would produce rather than swallowing a # into the database name. Worth a test with the opt=x@y case above, since it currently fails silently rather than loudly.

[ScottMansfield]

These two have the identical bug and Cloud SQL for MySQL uses the identical /cloudsql/PROJECT:REGION:INSTANCE socket path. Verified at this head — all the forms your Postgres path now handles still come out as a raw clientUrl here:

mysql   unescaped colons   clientUrl="mysql://u:p@%2Fcloudsql%2Fproj:r:inst/db"
mysql   %-encoded          clientUrl="mysql://u:p@%2Fcloudsql%2Fproj%3Ar%3Ainst/db"
mysql   ?host= form        clientUrl="mysql://u:p@/db?host=/cloudsql/proj:r:i"
mariadb unescaped colons   clientUrl="mariadb://u:p@%2Fcloudsql%2Fproj:r:inst/db"

Not asking you to implement MySQL support in this PR — it is more than a prefix change, because the drivers disagree about how a socket is named:

driver	socket option
pg	host beginning with / (connection-parameters.js:104-105)
mysql2	socketPath (connection_config.js:52)
mariadb	socketPath (connection-options.js:104)

So your host: mapping is right for Postgres and would be inert for the other two. Note also that the %3A-encoded form works for Postgres only because MikroORM decodes url.hostname into host; for MySQL that decoded value lands in host as well, which is still the wrong option, so MySQL is broken on all the socket forms rather than just the two that throw.

What would make the follow-up cheap is separating the two concerns now: rename parsePostgresSocketUri to something scheme-agnostic returning {host, user, password, dbName, schema, port}, and keep the host vs socketPath decision at the call site in getConnectionOptionsFromUri. That is a rename plus a signature tweak today and turns the MySQL fix into a second call site later. Entirely reasonable to say no and file an issue instead — I'd just rather the shape didn't harden around one driver first.

[ScottMansfield]

Minor: nothing constrains the resolved socket path. All of these are accepted verbatim:

?host=/etc                 ->  host="/etc"
?host=/../../tmp/x         ->  host="/../../tmp/x"
authority %2Ftmp%2Fanything ->  host="/tmp/anything:x:y"

I am not suggesting an allowlist on /cloudsql/ — that would break a local /var/run/postgresql socket, which is a perfectly ordinary way to run this. And a non-normalised /../../ is harmless here since the kernel resolves it at connect time.

The reason it is worth a line: everything about this feature — the PR title, the function name, the JSDoc — says Cloud SQL, while the behaviour is "any absolute path becomes a Unix socket". That is the more useful behaviour and I would keep it, but the JSDoc should say so, otherwise the first person to read this code will assume a constraint that isn't there. Something like "any authority or ?host= value beginning with / is treated as a Unix socket path, not only /cloudsql/…".

[AnupamKumar-1]

@ScottMansfield Thanks for the thorough review. Addressed both blocking items (Security, parsing bug) — details below rather than inline. Posted a clarification on #799 about the root cause. Proposing to keep the mysql/mariadb generalization as a follow-up rather than folding it into this PR. Once this PR lands, I can open a separate issue and follow-up PR for that work.

• Added a warning when ?host= overrides a real TCP hostname, per your option 1.

• Parsing bug: Replaced the backtracking regex with an explicit bounded split — userinfo is now scoped to the authority region, so an @ in the path/query no longer gets swallowed into it.

• JSDoc: Added a line clarifying any absolute path (not just /cloudsql/...) is treated as a Unix socket.

• I'm deliberately keeping the port here. Dropping it would bring back the regression @kalenkevich pointed out earlier, where the port was lost and pg fell back to the default port (5432) instead of using the port from the URI.