From 810bf64be37729c5c6acdd81cd7e9b4e6b59ce37 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 07:16:05 +0000 Subject: [PATCH] fix(cli): the startup banner reads a DSN-declared datasource, and stops printing credentials (#3793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OS_DATABASE_URL="postgres://u:p@127.0.0.1:59437/nope" os serve` bannered `Driver: SqlDriver(pg) → (unknown)` — the driver identified, the address gone, at exactly the moment (an unreachable database) when the address is the one thing worth reading. `describeRegisteredDriver` knew three of the four shapes a driver's `config.connection` arrives in — a DSN string, `{ filename }`, and discrete `{ host, port, database }` — but not `{ connectionString }`, which is what `defaultDatasourceDriverFactory` builds for a pg datasource declared with `config.url` / `config.connectionString`. `driver.config` keeps the shape its author passed (pinned by a driver-sql test), so #3791's knex-side rewrite neither caused nor worsened this. Two more `(unknown)`s had the same cause: the reader returned as soon as a driver had a `config` at all, so only a SqlDriver could ever succeed. MongoDBDriver keeps its DSN in a top-level `config.url` and *does* have a `config`, so the "mongo/turso expose the URL on the instance" arm below was unreachable; InMemoryDriver's `config` is `{}`, which is truthy, so the `(in-memory)` arm was unreachable too. Both are now read by shape. A latent label bug goes with them: SqliteWasmDriver passes knex a dialect *class* as `client`, and `SqlDriver(${cfg.client})` would have pasted the class source into the banner — only a string `client` is interpolated now. Both halves are one renderer, `utils/connection-display.ts`: - `describeDriverConnection(config)` knows every `connection` shape plus the `{ uri }` / `{ url }` spellings, applies the same precedence to a top-level address, and returns `undefined` — not a guess — for a function-valued `connection` the host builds per pool checkout. - `redactConnectionUrl(value)` drops credentials rather than masking them. The old `//user:****@` mask left two holes: a password with no username (`postgres://:secret@host/db`) did not match its regex and printed verbatim, and a Turso DSN carries its secret in the query string, which was never touched. Userinfo and query are both dropped; non-URL values (`:memory:`, a sqlite path) pass through; the function is idempotent. Three byte-identical copies of the old mask (serve / start / dev) and a fourth variant in `schema-migrate` collapse into it. The `schema-migrate` copy had the same DSN blindness with higher stakes — it feeds the `Apply N change(s) to …?` confirm, where a `{ connectionString }` config rendered as a bare `pg`, asking an operator to approve a destructive migration against an unnamed database. Verified end to end on `examples/app-todo` with a preset-wired `SqlDriver({ connection: { connectionString } })`: `→ (unknown)` before, `→ postgres://127.0.0.1:59437/nope` after, with `admin:hunter2` gone. Display only — no configuration or API surface moves. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E1DiRxoPdcTSGkzywm5zN4 --- .changeset/banner-dsn-connection-display.md | 69 +++++++++ packages/cli/src/commands/dev.ts | 11 +- .../src/commands/serve-driver-banner.test.ts | 142 ++++++++++++++++++ packages/cli/src/commands/serve.ts | 72 ++++----- packages/cli/src/commands/start.ts | 11 +- .../cli/src/utils/connection-display.test.ts | 126 ++++++++++++++++ packages/cli/src/utils/connection-display.ts | 100 ++++++++++++ packages/cli/src/utils/schema-migrate.ts | 28 ++-- 8 files changed, 485 insertions(+), 74 deletions(-) create mode 100644 .changeset/banner-dsn-connection-display.md create mode 100644 packages/cli/src/commands/serve-driver-banner.test.ts create mode 100644 packages/cli/src/utils/connection-display.test.ts create mode 100644 packages/cli/src/utils/connection-display.ts diff --git a/.changeset/banner-dsn-connection-display.md b/.changeset/banner-dsn-connection-display.md new file mode 100644 index 0000000000..45d06df7e6 --- /dev/null +++ b/.changeset/banner-dsn-connection-display.md @@ -0,0 +1,69 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): the startup banner reads a DSN-declared datasource, and stops printing credentials (#3793) + +`OS_DATABASE_URL="postgres://u:p@127.0.0.1:59437/nope" os serve` printed: + +``` + Driver: SqlDriver(pg) → (unknown) +``` + +The driver was identified; the address was not. The banner exists so a +developer sees at a glance which database they are on, and it went blank +exactly when the database is unreachable and the address is the one thing worth +reading. + +`describeRegisteredDriver` knew three of the four shapes a driver's +`config.connection` arrives in — a DSN string, `{ filename }` (sqlite), and +discrete `{ host, port, database }` — but not `{ connectionString }`, which is +what `defaultDatasourceDriverFactory` builds for a pg datasource declared with +`config.url` / `config.connectionString`. So *any* DSN-declared datasource read +`(unknown)`, while the same datasource spelled out in discrete fields read +fine. (`driver.config` keeps the shape its author passed — pinned by a +driver-sql test — so #3791's knex-side `{ connectionString, connectionTimeoutMillis }` +rewrite neither caused nor worsened this.) + +Chasing that turned up two more `(unknown)`s with the same cause — the reader +returned as soon as a driver had a `config` at all, so it could only ever +succeed for a `SqlDriver`: + +- **MongoDB** keeps its DSN in a top-level `config.url`, and `MongoDBDriver` + *does* have a `config`, so the "mongo/turso expose the URL on the instance" + arm below it was unreachable — every mongo boot banner read `(unknown)`. +- **In-memory** `config` is `{}` when none was passed, which is truthy, so the + `(in-memory)` arm was unreachable too — a preset-wired memory driver + bannered as `com.objectstack.driver.memory → (unknown)`. + +Both are now read by shape rather than by which arm matched first. A related +latent one is fixed alongside: `SqliteWasmDriver` passes knex a dialect *class* +as `client`, and the label interpolated it — `SqlDriver(${cfg.client})` would +have pasted the class source into the banner. The label only interpolates a +string `client` now, and falls back to the driver's constructor name. + +Both halves of the original bug are now one renderer, +`utils/connection-display.ts`: + +- **`describeDriverConnection(config)`** knows all four `connection` shapes, + plus the `{ uri }` / `{ url }` mongo-family spellings, applies the same field + precedence to a top-level address, and returns `undefined` — not a guess — + for a function-valued `connection` the host builds per pool checkout. +- **`redactConnectionUrl(value)`** drops credentials rather than masking them. + The previous `//user:****@` mask left two holes: a password supplied with no + username (`postgres://:secret@host/db`) did not match its regex and printed + verbatim, and a Turso DSN carries its secret in the query string + (`libsql://….turso.io?authToken=…`), which was never touched. Userinfo *and* + query are now dropped; non-URL values (`:memory:`, a sqlite path, + `(in-memory)`) pass through untouched, and the function is idempotent. + +Three byte-identical copies of the old mask (`serve` / `start` / `dev`) and a +fourth variant in `schema-migrate` collapse into that one helper. The +`schema-migrate` copy had the same DSN blindness with higher stakes: it feeds +the `Apply N change(s) to …?` confirm, where a `{ connectionString }` config +rendered as a bare `pg` — an operator was asked to approve a destructive +migration against an unnamed database. + +Banner/prompt output changes accordingly — `postgres://admin:hunter2@db:5432/app` +was shown as `postgres://admin:****@db:5432/app` and is now +`postgres://db:5432/app`. Display only; no configuration or API surface moves. diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index f690750cb9..d8e87573b0 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -8,6 +8,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { printHeader, printKV, printStep, printError } from '../utils/format.js'; +import { redactConnectionUrl } from '../utils/connection-display.js'; import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types'; /** @@ -252,7 +253,7 @@ export default class Dev extends Command { }; printKV('Environment ID', environmentId, '🎯'); printKV('Artifact', isUrl ? artifactPath : path.relative(process.cwd(), artifactPath), '📦'); - if (effectiveDb) printKV('Database', redactDbUrl(effectiveDb), '🗄️'); + if (effectiveDb) printKV('Database', redactConnectionUrl(effectiveDb), '🗄️'); const port = flags.port ?? readEnvWithDeprecation('OS_PORT', 'PORT', { silent: true }); const binPath = process.argv[1]; @@ -500,11 +501,3 @@ export default class Dev extends Command { }); } } - -function redactDbUrl(url: string): string { - try { - return url.replace(/(\/\/[^/@:]+):[^/@]+@/, '$1:****@'); - } catch { - return url; - } -} diff --git a/packages/cli/src/commands/serve-driver-banner.test.ts b/packages/cli/src/commands/serve-driver-banner.test.ts new file mode 100644 index 0000000000..a5bb48621a --- /dev/null +++ b/packages/cli/src/commands/serve-driver-banner.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// framework#3793 — the startup banner's `Driver:` row. +// +// The banner exists so a developer sees at a glance which database they are +// talking to. When the driver was wired by an app preset or by +// `EnvironmentKernelFactory` rather than by serve's own `OS_DATABASE_URL` +// fallback, that row comes from probing the registered driver — and for a +// datasource declared with a DSN it read `SqlDriver(pg) → (unknown)`: the +// right driver, no address, at exactly the moment (an unreachable database) +// when the address is what you need. + +import { describe, it, expect } from 'vitest'; +import { describeRegisteredDriver } from './serve.js'; + +/** A kernel whose `getService` throws for anything not registered, like the real one. */ +function fakeKernel(services: Record) { + return { + getService(name: string) { + if (!(name in services)) throw new Error(`Service '${name}' not found`); + return services[name]; + }, + }; +} + +describe('describeRegisteredDriver', () => { + it('reads a DSN-declared pg datasource — the #3793 (unknown)', () => { + // Exactly what `defaultDatasourceDriverFactory` hands `new SqlDriver(...)` + // for a datasource whose config is `{ url }` / `{ connectionString }`. + const kernel = fakeKernel({ + 'driver.com.objectstack.driver.sql': { + config: { + client: 'pg', + connection: { connectionString: 'postgres://u:p@127.0.0.1:59437/nope' }, + pool: { min: 0, max: 5 }, + }, + }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ + label: 'SqlDriver(pg)', + url: 'postgres://127.0.0.1:59437/nope', + }); + }); + + it('keeps reading the shapes it already knew', () => { + expect(describeRegisteredDriver(fakeKernel({ + 'driver.sql': { config: { client: 'pg', connection: 'postgres://u:p@host:5432/app' } }, + }))).toEqual({ label: 'SqlDriver(pg)', url: 'postgres://host:5432/app' }); + + expect(describeRegisteredDriver(fakeKernel({ + 'driver.sql': { config: { client: 'better-sqlite3', connection: { filename: './data.db' } } }, + }))).toEqual({ label: 'SqlDriver(better-sqlite3)', url: './data.db' }); + + expect(describeRegisteredDriver(fakeKernel({ + 'driver.sql': { config: { client: 'pg', connection: { host: 'db.example.com', port: 5432, database: 'app' } } }, + }))).toEqual({ label: 'SqlDriver(pg)', url: 'db.example.com:5432/app' }); + }); + + it('never puts credentials in the banner', () => { + for (const connection of [ + 'postgres://admin:hunter2@db.example.com:5432/app', + { connectionString: 'postgres://admin:hunter2@db.example.com:5432/app' }, + { host: 'db.example.com', port: 5432, database: 'app', user: 'admin', password: 'hunter2' }, + ]) { + const probe = describeRegisteredDriver(fakeKernel({ + 'driver.sql': { config: { client: 'pg', connection } }, + })); + expect(probe?.url).not.toContain('hunter2'); + } + }); + + it('reads the top-level url a MongoDBDriver keeps on its config', () => { + // Every driver has a `config` property (MongoDBDriver's is `{ url, … }`, + // InMemoryDriver's is `{}`), so the old "has a config → it's a SqlDriver" + // early return meant *every* mongo boot banner read `(unknown)`. + const kernel = fakeKernel({ + 'driver.com.objectstack.driver.mongodb': { + constructor: { name: 'MongoDBDriver' }, + name: 'com.objectstack.driver.mongodb', + config: { url: 'mongodb://admin:hunter2@cluster0.mongodb.net/app', database: 'app' }, + }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ + label: 'MongoDBDriver', + url: 'mongodb://cluster0.mongodb.net/app', + }); + }); + + it('redacts the URL a driver exposes on the instance instead', () => { + const kernel = fakeKernel({ + 'driver.com.objectstack.driver.turso': { + constructor: { name: 'TursoDriver' }, + url: 'libsql://app-org.turso.io?authToken=secret-jwt', + }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ + label: 'TursoDriver', + url: 'libsql://app-org.turso.io', + }); + }); + + it('still says (unknown) when the config genuinely carries no address', () => { + const kernel = fakeKernel({ + // knex lets the host build a connection per pool checkout; there is + // nothing to read until it is called. + 'driver.sql': { config: { client: 'pg', connection: () => ({ host: 'h' }) } }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ label: 'SqlDriver(pg)', url: '(unknown)' }); + }); + + it('labels an in-memory driver "(in-memory)", not "(unknown)"', () => { + const kernel = fakeKernel({ + // InMemoryDriver always has a `config` — `{}` when none was passed. + 'driver.memory': { + constructor: { name: 'InMemoryDriver' }, + name: 'com.objectstack.driver.memory', + config: {}, + }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ label: 'InMemoryDriver', url: '(in-memory)' }); + }); + + it('does not paste a Client class into the label (SqliteWasmDriver)', () => { + // SqliteWasmDriver passes knex a dialect *class* as `client`; interpolating + // it would put the class source in the banner. + class Client_WasmSqlite {} + const kernel = fakeKernel({ + 'driver.sql': { + constructor: { name: 'SqliteWasmDriver' }, + config: { client: Client_WasmSqlite, connection: { filename: './app.wasm.db' } }, + }, + }); + expect(describeRegisteredDriver(kernel)).toEqual({ + label: 'SqliteWasmDriver', + url: './app.wasm.db', + }); + }); + + it('returns null when no known driver is registered', () => { + expect(describeRegisteredDriver(fakeKernel({}))).toBeNull(); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index d13f28e3c3..32b5f74b2e 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -14,6 +14,7 @@ import { PLATFORM_CAPABILITY_TOKENS } from '@objectstack/spec/kernel'; import { missingProviderMessage } from '../utils/capability-preflight.js'; import { resolveObjectStackHome } from '@objectstack/runtime'; import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js'; +import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js'; import { printHeader, printKV, @@ -544,18 +545,13 @@ export default class Serve extends Command { }; const trackPlugin = (name: string) => { loadedPlugins.push(shortPluginName(name)); }; - // Track resolved storage driver + redacted URL for the startup banner. + // Track resolved storage driver + connection target for the startup banner. + // The value lands here raw when it came from this command's own + // OS_DATABASE_URL fallback and already-redacted when it came from probing a + // registered driver, so it is redacted at print time — `redactConnectionUrl` + // is idempotent, so the second pass over an already-clean URL is a no-op. let resolvedDriverLabel: string | undefined; let resolvedDatabaseUrl: string | undefined; - const redactDbUrl = (url: string | undefined): string | undefined => { - if (!url) return undefined; - try { - // Redact passwords inside connection URLs: protocol://user:****@host/db - return url.replace(/(\/\/[^/@:]+):[^/@]+@/, '$1:****@'); - } catch { - return url; - } - }; // Save original console/stdout methods — we'll suppress noise during boot const originalConsoleLog = console.log; @@ -2370,7 +2366,7 @@ export default class Serve extends Command { // never accept a request — shutdown immediately so the deploy // pipeline can move on. if (process.env.OS_MIGRATE_AND_EXIT === '1') { - console.log(chalk.green(`✓ Migration complete (${loadedPlugins.length} plugins started against ${redactDbUrl(resolvedDatabaseUrl) || 'configured DB'})`)); + console.log(chalk.green(`✓ Migration complete (${loadedPlugins.length} plugins started against ${resolvedDatabaseUrl ? redactConnectionUrl(resolvedDatabaseUrl) : 'configured DB'})`)); try { await kernel.shutdown(); } catch (err: any) { @@ -2443,7 +2439,7 @@ export default class Serve extends Command { uiEnabled: enableUI, consolePath: loadedPlugins.includes('ConsoleUI') ? CONSOLE_PATH : undefined, driverLabel: resolvedDriverLabel, - databaseUrl: redactDbUrl(resolvedDatabaseUrl), + databaseUrl: resolvedDatabaseUrl ? redactConnectionUrl(resolvedDatabaseUrl) : undefined, multiTenant: resolveMultiOrgEnabled(), seededAdmin, automation: automationSummary, @@ -2508,8 +2504,13 @@ export default class Serve extends Command { * (e.g. when the example app's preset or `EnvironmentKernelFactory` wired * it). Returns `null` when nothing matches; the caller treats that as * "no driver info available" and skips the line. + * + * Reading the address out of a driver is `describeDriverConnection`'s job + * (#3793) — it is the one place that knows every shape a driver config + * arrives in, so a DSN-declared datasource no longer falls through to + * `(unknown)`, and no shape prints credentials. */ -function describeRegisteredDriver(kernel: any): { label: string; url: string } | null { +export function describeRegisteredDriver(kernel: any): { label: string; url: string } | null { const candidates = [ 'driver.com.objectstack.driver.sql', 'driver.com.objectstack.driver.mongodb', @@ -2522,33 +2523,26 @@ function describeRegisteredDriver(kernel: any): { label: string; url: string } | try { driver = kernel?.getService?.(name); } catch { /* not registered */ } if (!driver) continue; - // SqlDriver: `{ client, connection: string | { filename, host, ... } }` const cfg = driver.config; - if (cfg) { - const client = cfg.client; - const conn = cfg.connection; - let url = ''; - if (typeof conn === 'string') { - url = conn; - } else if (conn && typeof conn === 'object') { - url = conn.filename - ?? (conn.host ? `${conn.host}${conn.port ? `:${conn.port}` : ''}${conn.database ? `/${conn.database}` : ''}` : ''); - } - const label = client ? `SqlDriver(${client})` : (driver.name ?? 'SqlDriver'); - return { label, url: url || '(unknown)' }; - } - - // MongoDB / Turso drivers expose the URL on the instance itself. - if (driver.url) { - const label = driver.constructor?.name ?? driver.name ?? 'Driver'; - return { label, url: String(driver.url) }; - } - - // InMemoryDriver — no URL. - return { - label: driver.constructor?.name ?? driver.name ?? 'Driver', - url: '(in-memory)', - }; + // `client` is a plain string for the stock knex dialects, but a Client + // *class* for SqliteWasmDriver — interpolating that would paste the whole + // class source into the banner. + const label = typeof cfg?.client === 'string' + ? `SqlDriver(${cfg.client})` + : (driver.constructor?.name ?? driver.name ?? 'Driver'); + + // Read the address by shape, not by which branch matches first. Every + // driver here has a `config` property — SqlDriver's is knex-shaped, + // MongoDBDriver's holds a top-level `url`, InMemoryDriver's is `{}` — so + // returning early on "has a config" meant the mongo and in-memory arms + // below never ran and every non-SqlDriver boot banner read `(unknown)`. + // `driver.url` still covers a driver that keeps its DSN on the instance. + const url = describeDriverConnection(cfg) + ?? (typeof driver.url === 'string' && driver.url ? redactConnectionUrl(driver.url) : undefined); + + // A memory driver has no address to show — say so, rather than + // `(unknown)`, which reads as "we looked for one and failed". + return { label, url: url ?? (name.endsWith('.memory') ? '(in-memory)' : '(unknown)') }; } return null; } diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index fac1384487..7d959590e4 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -9,6 +9,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { printHeader, printKV, printStep, printError } from '../utils/format.js'; +import { redactConnectionUrl } from '../utils/connection-display.js'; import { readEnvWithDeprecation } from '@objectstack/types'; /** @@ -209,7 +210,7 @@ export default class Start extends Command { } else { printKV('Artifact', 'none (empty kernel — install apps via the Console marketplace)', '📦'); } - printKV('Database', redactDbUrl(databaseUrl), '🗄️'); + printKV('Database', redactConnectionUrl(databaseUrl), '🗄️'); printKV('Environment', environmentId, '🎯'); // Resolve the port the child `serve` will actually bind, matching its // flag default (`--port` > $OS_PORT/$PORT > 3000). Using `flags.port` @@ -325,14 +326,6 @@ function resolveArtifactSource(flagValue: string | undefined, homeDir: string): return undefined; } -function redactDbUrl(url: string): string { - try { - return url.replace(/(\/\/[^/@:]+):[^/@]+@/, '$1:****@'); - } catch { - return url; - } -} - /** * Read the persisted AUTH_SECRET from `/auth-secret`, or generate * one on first run and persist it so subsequent restarts keep existing diff --git a/packages/cli/src/utils/connection-display.test.ts b/packages/cli/src/utils/connection-display.test.ts new file mode 100644 index 0000000000..0014848d8e --- /dev/null +++ b/packages/cli/src/utils/connection-display.test.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// framework#3793 — the startup banner rendered a DSN-declared datasource as +// `→ (unknown)`: `describeRegisteredDriver` knew a `connection` that was a +// string, a `{ filename }` or a `{ host, port, database }`, but not the +// `{ connectionString }` that `defaultDatasourceDriverFactory` builds for a +// pg datasource declared with `config.url` / `config.connectionString`. The +// address went missing exactly when the database is unreachable and the +// address is the one thing worth reading. +// +// These pin both halves of the fix: every shape resolves to a target, and no +// shape leaks the credentials the DSN carries. + +import { describe, it, expect } from 'vitest'; +import { redactConnectionUrl, describeDriverConnection } from './connection-display.js'; + +describe('redactConnectionUrl', () => { + it('drops the userinfo segment from a DSN', () => { + expect(redactConnectionUrl('postgres://admin:hunter2@db.example.com:5432/app')) + .toBe('postgres://db.example.com:5432/app'); + }); + + it('drops a password supplied without a username', () => { + // The old `(\/\/[^/@:]+):[^/@]+@` mask required a username to match, so + // this shape printed the password verbatim. + expect(redactConnectionUrl('postgres://:hunter2@db.example.com/app')) + .toBe('postgres://db.example.com/app'); + }); + + it('drops the query string, where tokens hide', () => { + expect(redactConnectionUrl('libsql://app-org.turso.io?authToken=secret-jwt')) + .toBe('libsql://app-org.turso.io'); + }); + + it('handles a mongodb+srv DSN', () => { + expect(redactConnectionUrl('mongodb+srv://u:p@cluster0.mongodb.net/app?retryWrites=true')) + .toBe('mongodb+srv://cluster0.mongodb.net/app'); + }); + + it('leaves a credential-free DSN alone, and is idempotent', () => { + const clean = 'postgres://db.example.com:5432/app'; + expect(redactConnectionUrl(clean)).toBe(clean); + expect(redactConnectionUrl(redactConnectionUrl('postgres://u:p@db.example.com:5432/app'))) + .toBe(clean); + }); + + it('passes non-URL values through untouched', () => { + expect(redactConnectionUrl(':memory:')).toBe(':memory:'); + expect(redactConnectionUrl('./data/objectstack.db')).toBe('./data/objectstack.db'); + expect(redactConnectionUrl('(in-memory)')).toBe('(in-memory)'); + expect(redactConnectionUrl('sqlite:/var/lib/app.db')).toBe('sqlite:/var/lib/app.db'); + }); + + it('still strips userinfo from something the URL parser rejects', () => { + expect(redactConnectionUrl('weird scheme://u:p@host/db')).toBe('weird scheme://host/db'); + }); +}); + +describe('describeDriverConnection', () => { + it('reads a DSN string', () => { + expect(describeDriverConnection({ client: 'pg', connection: 'postgres://u:p@host:5432/app' })) + .toBe('postgres://host:5432/app'); + }); + + it('reads `{ connectionString }` — the shape that used to render (unknown)', () => { + expect(describeDriverConnection({ + client: 'pg', + connection: { connectionString: 'postgres://u:p@127.0.0.1:59437/nope', password: 'hunter2' }, + })).toBe('postgres://127.0.0.1:59437/nope'); + }); + + it('reads the `{ uri }` / `{ url }` spellings of the same thing', () => { + expect(describeDriverConnection({ connection: { uri: 'mongodb://u:p@host:27017/app' } })) + .toBe('mongodb://host:27017/app'); + expect(describeDriverConnection({ connection: { url: 'mongodb://u:p@host:27017/app' } })) + .toBe('mongodb://host:27017/app'); + }); + + it('reads a sqlite `{ filename }`', () => { + expect(describeDriverConnection({ client: 'better-sqlite3', connection: { filename: ':memory:' } })) + .toBe(':memory:'); + expect(describeDriverConnection({ client: 'better-sqlite3', connection: { filename: './data.db' } })) + .toBe('./data.db'); + }); + + it('reads discrete `{ host, port, database }` and never their password', () => { + expect(describeDriverConnection({ + client: 'pg', + connection: { host: 'db.example.com', port: 5432, database: 'app', user: 'admin', password: 'hunter2' }, + })).toBe('db.example.com:5432/app'); + }); + + it('omits the port and database when they are absent', () => { + expect(describeDriverConnection({ client: 'pg', connection: { host: 'db.example.com' } })) + .toBe('db.example.com'); + }); + + it('prefers the DSN when a config carries both shapes', () => { + expect(describeDriverConnection({ + client: 'pg', + connection: { connectionString: 'postgres://u:p@dsn-host/dsn-db', host: 'discrete-host' }, + })).toBe('postgres://dsn-host/dsn-db'); + }); + + it('falls back to a top-level address — MongoDBDriver keeps `config.url`', () => { + expect(describeDriverConnection({ url: 'mongodb://u:p@cluster0.mongodb.net/app', database: 'app' })) + .toBe('mongodb://cluster0.mongodb.net/app'); + }); + + it('prefers `connection` over a top-level address', () => { + expect(describeDriverConnection({ + connection: { connectionString: 'postgres://u:p@nested-host/nested-db' }, + url: 'postgres://u:p@top-level-host/top-level-db', + })).toBe('postgres://nested-host/nested-db'); + }); + + it('returns undefined when the config carries no address', () => { + expect(describeDriverConnection(undefined)).toBeUndefined(); + expect(describeDriverConnection({})).toBeUndefined(); + expect(describeDriverConnection({ client: 'pg', connection: '' })).toBeUndefined(); + expect(describeDriverConnection({ client: 'pg', connection: {} })).toBeUndefined(); + // knex lets a host hand back a fresh connection per pool checkout — there + // is no address to read until it is called, so don't invent one. + expect(describeDriverConnection({ client: 'pg', connection: () => ({ host: 'h' }) })).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/utils/connection-display.ts b/packages/cli/src/utils/connection-display.ts new file mode 100644 index 0000000000..523d43726e --- /dev/null +++ b/packages/cli/src/utils/connection-display.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * One renderer for "which database am I actually talking to?" — the string the + * startup banner (`os serve` / `os start` / `os dev`) and the migrate/resync + * confirms put in front of a developer. + * + * Two jobs, both previously done ad hoc and both wrong in their own way + * (#3793): + * + * 1. **Know every connection shape.** A driver's `config.connection` reaches + * us as a DSN string, as `{ connectionString }` / `{ uri }` / `{ url }`, as + * `{ filename }` (sqlite), or as discrete `{ host, port, database }` — and + * a non-SqlDriver keeps its address somewhere else again. The banner knew + * three of those, so a datasource declared with a DSN — exactly what + * `defaultDatasourceDriverFactory` turns into `{ connectionString }` — + * rendered as `→ (unknown)`. The address went missing precisely when the + * database is unreachable and the address is the one thing worth reading. + * + * 2. **Never print credentials.** A DSN carries `user:password@`, and a Turso + * DSN carries its `?authToken=…` in the query string. The banner is read on + * shared terminals, in CI logs and in screenshots, so credentials are + * *dropped* rather than masked — `admin:****@` tells a developer nothing + * that the driver label and the host don't already say. + */ + +/** + * Strip credentials from a connection string. + * + * Values that aren't URLs — a sqlite path, `:memory:`, the `(in-memory)` + * marker — pass through untouched, as does anything the URL parser rejects + * (minus its userinfo segment). Idempotent, so a call site that can't tell + * whether its input was already redacted may apply it defensively. + */ +export function redactConnectionUrl(value: string): string { + if (!value.includes('://')) return value; + try { + const url = new URL(value); + url.username = ''; + url.password = ''; + // Query params are where tokens hide (`libsql://….turso.io?authToken=…`), + // and none of them help answer "which database". + url.search = ''; + return url.toString(); + } catch { + // Not parseable (a driver-specific dialect, a malformed URL). Drop the + // userinfo segment by hand rather than printing the whole thing. + return value.replace(/:\/\/[^@/]*@/, '://'); + } +} + +/** Read a connection target out of one bag of connection-ish fields. */ +function readTarget(fields: Record | undefined): string | undefined { + if (!fields) return undefined; + + // DSN-shaped. `connectionString` is what `defaultDatasourceDriverFactory` + // builds for a pg datasource declared with `config.url`/`config.connectionString`; + // `uri`/`url` are the mongo-family spellings of the same thing. + for (const key of ['connectionString', 'uri', 'url'] as const) { + const dsn = fields[key]; + if (typeof dsn === 'string' && dsn) return redactConnectionUrl(dsn); + } + + if (typeof fields.filename === 'string' && fields.filename) return fields.filename; + + if (fields.host) { + const port = fields.port === undefined || fields.port === null || fields.port === '' + ? '' + : `:${fields.port}`; + const database = fields.database ? `/${fields.database}` : ''; + return `${fields.host}${port}${database}`; + } + + return undefined; +} + +/** + * Render a driver's `config` as a credential-free connection target, or + * `undefined` when the config carries no address at all — an in-memory driver, + * or a function-valued `connection` the host builds fresh per pool checkout. + * + * Drivers don't agree on where the address lives, and a banner has to read all + * of them: `SqlDriver` carries knex's `{ client, connection }`, while + * `MongoDBDriver` keeps a top-level `config.url`. The same field precedence is + * therefore applied to `config.connection` first and to `config` itself second. + * + * A DSN keeps its scheme (`postgres://db.example.com:5432/app`); discrete + * fields have none to keep (`db.example.com:5432/app`). Callers that print + * this without a driver label alongside should say which engine themselves. + */ +export function describeDriverConnection(config: unknown): string | undefined { + if (!config || typeof config !== 'object') return undefined; + const cfg = config as Record; + + const conn = cfg.connection; + if (typeof conn === 'string' && conn) return redactConnectionUrl(conn); + + return readTarget(conn && typeof conn === 'object' ? conn as Record : undefined) + ?? readTarget(cfg); +} diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index e3d884369b..850697e6b6 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -14,6 +14,7 @@ */ import chalk from 'chalk'; import type { ManagedDriftEntry, DriftCategory } from '@objectstack/driver-sql'; +import { describeDriverConnection } from './connection-display.js'; export interface SqlDriverLike { detectManagedDrift(): Promise; @@ -52,26 +53,19 @@ function findSqlDriver(kernel: any): SqlDriverLike | null { return null; } +/** + * Name the database the migrate/resync commands are about to write to. + * + * Shares the startup banner's renderer (#3793): the same + * `{ connectionString }` shape that made the banner print `(unknown)` used to + * fall through here to a bare `pg` — and this string is what the + * `Apply N change(s) to …?` confirm shows, so it has to name the real target. + * Falls back to the client name only when the config carries no address at all. + */ function describeDb(driver: SqlDriverLike | null): string { const cfg: any = driver?.config; if (!cfg) return 'unknown'; - const conn = cfg.connection; - if (typeof conn === 'string') return redactUrl(conn); - if (conn && typeof conn === 'object') { - if (conn.filename) return `sqlite:${conn.filename}`; - if (conn.host) return `${cfg.client}://${conn.host}${conn.database ? '/' + conn.database : ''}`; - } - return String(cfg.client ?? 'unknown'); -} - -function redactUrl(url: string): string { - try { - const u = new URL(url); - if (u.password) u.password = '***'; - return u.toString(); - } catch { - return url.replace(/:\/\/[^@]*@/, '://***@'); - } + return describeDriverConnection(cfg) ?? String(cfg.client ?? 'unknown'); } /** Boot the schema stack. Caller MUST call `shutdown()` when done. */