diff --git a/docs/config/toml.mdx b/docs/config/toml.mdx index 24c77153..00901de7 100644 --- a/docs/config/toml.mdx +++ b/docs/config/toml.mdx @@ -212,6 +212,12 @@ Sources define database connections. Each source represents a database that DBHu ``` + + + **Mixing `dsn` with individual fields.** A `dsn` already encodes the connection identity (type, host, port, database, user, password). Fields that can also live in the DSN query string — `sslmode`, `sslrootcert`, `instanceName`, `authentication`, `domain` — may be set alongside a `dsn` and are applied when the DSN omits them. + + If such a field is set to a value that **contradicts** the DSN (e.g. `dsn = "...?sslmode=disable"` together with `sslmode = "require"`, or a `host`/`user`/`database` that differs from the DSN), DBHub rejects the configuration at startup rather than silently ignoring the field. Set each value in only one place, or make the two values match. + ### connection_timeout diff --git a/src/config/__tests__/toml-loader.test.ts b/src/config/__tests__/toml-loader.test.ts index 84863e69..8f57664f 100644 --- a/src/config/__tests__/toml-loader.test.ts +++ b/src/config/__tests__/toml-loader.test.ts @@ -116,29 +116,57 @@ dsn = "sqlite:///path/to/database.db" expect(result?.sources[0].user).toBeUndefined(); }); - it('should not override explicit connection params with DSN values', () => { + it('should reject identity fields that conflict with the DSN', () => { + // A DSN already encodes the connection identity; setting a field to a + // different value is silently ignored at connection time, so it must error. const tomlContent = ` [[sources]] id = "explicit_override" dsn = "postgres://dsn_user:pass@dsn_host:5432/dsn_db" type = "postgres" host = "explicit_host" -port = 9999 -database = "explicit_db" -user = "explicit_user" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting host"); + }); + + it('should accept a host field that differs only in case from the DSN', () => { + const tomlContent = ` +[[sources]] +id = "case_host" +dsn = "postgres://user:pass@DB.EXAMPLE.COM:5432/db" +host = "db.example.com" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + const result = loadTomlConfig(); + + expect(result?.sources[0].id).toBe('case_host'); + }); + + it('should accept identity fields that match the DSN', () => { + const tomlContent = ` +[[sources]] +id = "redundant" +dsn = "postgres://dsn_user:pass@dsn_host:5432/dsn_db" +type = "postgres" +host = "dsn_host" +port = 5432 +database = "dsn_db" +user = "dsn_user" `; fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); const result = loadTomlConfig(); - // Explicit values should be preserved, not overwritten by DSN expect(result?.sources[0]).toMatchObject({ - id: 'explicit_override', + id: 'redundant', type: 'postgres', - host: 'explicit_host', - port: 9999, - database: 'explicit_db', - user: 'explicit_user', + host: 'dsn_host', + port: 5432, + database: 'dsn_db', + user: 'dsn_user', }); }); @@ -483,6 +511,145 @@ sslmode = "invalid" expect(() => loadTomlConfig()).toThrow("invalid sslmode 'invalid'"); }); + it('should throw error when DSN sslmode conflicts with sslmode field', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:pass@localhost:5432/db?sslmode=disable" +sslmode = "require" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting sslmode"); + }); + + it('should accept matching DSN sslmode and sslmode field', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:pass@localhost:5432/db?sslmode=require" +sslmode = "require" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + const result = loadTomlConfig(); + + expect(result).toBeTruthy(); + expect(result?.sources[0].sslmode).toBe('require'); + }); + + it('should populate sslmode field from DSN query parameter', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:pass@localhost:5432/db?sslmode=require" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + const result = loadTomlConfig(); + + expect(result?.sources[0].sslmode).toBe('require'); + }); + + it('should treat an empty DSN sslmode (?sslmode=) as present and conflicting', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:pass@localhost:5432/db?sslmode=" +sslmode = "require" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting sslmode"); + }); + + it('should throw error when DSN user conflicts with user field', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://dsn_user:pass@localhost:5432/db" +user = "other_user" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting user"); + }); + + it('should throw error when DSN database conflicts with database field', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:pass@localhost:5432/dsn_db" +database = "other_db" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting database"); + }); + + it('should throw error when password field conflicts with DSN password', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user:dsn_pass@localhost:5432/db" +password = "other_pass" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("password' field that conflicts"); + // The error must not echo either password value + expect(() => loadTomlConfig()).not.toThrow(/dsn_pass|other_pass/); + }); + + it('should report a clear error when password field is set but DSN has no password', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "postgres://user@localhost:5432/db" +password = "field_pass" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("the DSN has no password"); + expect(() => loadTomlConfig()).not.toThrow(/field_pass/); + }); + + it('should throw error when type = "sqlite" conflicts with a non-SQLite DSN', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +type = "sqlite" +dsn = "postgres://user:pass@localhost:5432/db" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting type"); + }); + + it('should throw error when type = "postgres" conflicts with a SQLite DSN', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +type = "postgres" +dsn = "sqlite:///path/to/db.sqlite" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting type"); + }); + + it('should throw error when DSN instanceName conflicts with instanceName field', () => { + const tomlContent = ` +[[sources]] +id = "test_db" +dsn = "sqlserver://sa:pass@localhost:1433/db?instanceName=ENV1" +instanceName = "ENV2" +`; + fs.writeFileSync(path.join(tempDir, 'dbhub.toml'), tomlContent); + + expect(() => loadTomlConfig()).toThrow("conflicting instanceName"); + }); + it('should throw error when sslmode is specified for SQLite', () => { const tomlContent = ` [[sources]] @@ -1134,6 +1301,143 @@ dsn = "mysql://user:pass@localhost:3306/testdb" expect(dsn).toBe('postgres://user:pass@localhost:5432/db'); }); + it('should merge sslmode field into a DSN that lacks it', () => { + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db', + sslmode: 'require', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('postgres://user:pass@localhost:5432/db?sslmode=require'); + }); + + it('should append sslmode with & when DSN already has query params', () => { + const source: SourceConfig = { + id: 'test', + type: 'sqlserver', + dsn: 'sqlserver://user:pass@localhost:1433/db?instanceName=ENV1', + sslmode: 'require', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('sqlserver://user:pass@localhost:1433/db?instanceName=ENV1&sslmode=require'); + }); + + it('should not duplicate sslmode when DSN already specifies it', () => { + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db?sslmode=require', + sslmode: 'require', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('postgres://user:pass@localhost:5432/db?sslmode=require'); + }); + + it('should merge instanceName field into a SQL Server DSN that lacks it', () => { + const source: SourceConfig = { + id: 'test', + type: 'sqlserver', + dsn: 'sqlserver://sa:pass@localhost:1433/db', + instanceName: 'ENV1', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('sqlserver://sa:pass@localhost:1433/db?instanceName=ENV1'); + }); + + it('should merge authentication and domain fields into a SQL Server DSN', () => { + const source: SourceConfig = { + id: 'test', + type: 'sqlserver', + dsn: 'sqlserver://user:pass@localhost:1433/db', + authentication: 'ntlm', + domain: 'CORP', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('sqlserver://user:pass@localhost:1433/db?authentication=ntlm&domain=CORP'); + }); + + it('should merge sslrootcert field into a postgres DSN for verify-ca', () => { + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db', + sslmode: 'verify-ca', + sslrootcert: '/etc/ssl/ca bundle.pem', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe( + 'postgres://user:pass@localhost:5432/db?sslmode=verify-ca&sslrootcert=' + + encodeURIComponent('/etc/ssl/ca bundle.pem') + ); + }); + + it('should not merge sslrootcert when sslmode is not a verify mode', () => { + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db', + sslmode: 'require', + sslrootcert: '/etc/ssl/ca.pem', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('postgres://user:pass@localhost:5432/db?sslmode=require'); + }); + + it('should not append a duplicate when the DSN has an empty-valued param', () => { + // SafeURL drops `?sslmode=`, but the raw presence check must still see it + // so we never produce an ambiguous `?sslmode=&sslmode=require`. + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db?sslmode=', + sslmode: 'require', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('postgres://user:pass@localhost:5432/db?sslmode='); + }); + + it('should not produce "?&" when the DSN ends with a bare "?"', () => { + const source: SourceConfig = { + id: 'test', + type: 'postgres', + dsn: 'postgres://user:pass@localhost:5432/db?', + sslmode: 'require', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('postgres://user:pass@localhost:5432/db?sslmode=require'); + }); + + it('should not add sslmode to a SQLite DSN', () => { + const source: SourceConfig = { + id: 'test', + type: 'sqlite', + dsn: 'sqlite:///path/to/db.sqlite', + }; + + const dsn = buildDSNFromSource(source); + + expect(dsn).toBe('sqlite:///path/to/db.sqlite'); + }); + it('should build PostgreSQL DSN from individual params', () => { const source: SourceConfig = { id: 'test', diff --git a/src/config/toml-loader.ts b/src/config/toml-loader.ts index 07ccb8b3..a5c4ff13 100644 --- a/src/config/toml-loader.ts +++ b/src/config/toml-loader.ts @@ -223,6 +223,150 @@ function validateToolsConfig( } } +/** + * Read a query parameter's value from a DSN's raw query string, distinguishing + * "absent" (returns null) from "present but empty" (returns ""). + * + * SafeURL drops params whose value is empty (e.g. `?sslmode=`), which would make + * an empty-but-present param look absent — causing conflict checks to be skipped + * and the merge step to append a duplicate param. Scanning the raw query string + * here avoids that. + */ +function getRawDSNQueryParam(dsn: string, key: string): string | null { + const queryStart = dsn.indexOf("?"); + if (queryStart === -1) { + return null; + } + for (const pair of dsn.substring(queryStart + 1).split("&")) { + if (pair === "") { + continue; + } + const eq = pair.indexOf("="); + const rawKey = eq === -1 ? pair : pair.substring(0, eq); + if (rawKey === key) { + const rawValue = eq === -1 ? "" : pair.substring(eq + 1); + try { + return decodeURIComponent(rawValue); + } catch { + return rawValue; + } + } + } + return null; +} + +/** + * Reject standalone fields that contradict a DSN. + * + * A DSN already encodes the connection identity (type/host/port/database/user/ + * password) and may carry query parameters (sslmode/sslrootcert plus the SQL + * Server instanceName/authentication/domain). When the user also sets one of + * those as a standalone field with a different value, the DSN wins at connection + * time (buildDSNFromSource returns the DSN), so the field would be silently + * ignored — we fail fast here instead. + * + * A field left unset never trips this check: processSourceConfigs() copies a + * subset of these (type/host/port/database/user + sslmode/sslrootcert) from the + * DSN into the source when the field is unset, so they end up matching. + */ +function validateDSNFieldConflicts(source: SourceConfig, configPath: string): void { + const conflict = (field: string, fieldValue: string, dsnValue: string): never => { + throw new Error( + `Configuration file ${configPath}: source '${source.id}' has conflicting ${field}: ` + + `the DSN specifies '${dsnValue}' but the ${field} field is '${fieldValue}'. ` + + `Set ${field} in only one place, or make the two values match.` + ); + }; + + const info = parseConnectionInfoFromDSN(source.dsn!); + + // Reject a type field that disagrees with the DSN protocol. Checked before the + // SQLite short-circuit below, and keyed off the DSN's parsed type rather than + // source.type, so a `type = "sqlite"` paired with a non-SQLite DSN (or the + // reverse) is still caught instead of silently skipped. + if (source.type && info?.type && source.type !== info.type) { + conflict("type", source.type, info.type); + } + + // A SQLite DSN only carries a database path — no host/credentials or + // query-string fields to cross-check. + if (info?.type === "sqlite") { + return; + } + + let url: SafeURL; + try { + url = new SafeURL(source.dsn!); + } catch { + // DSN parse failures are surfaced by the connector; skip the conflict check + return; + } + + // Identity fields embedded in the DSN + if (info) { + // Hostnames are case-insensitive, so compare them normalized + if (source.host && info.host && source.host.toLowerCase() !== info.host.toLowerCase()) { + conflict("host", source.host, info.host); + } + if (source.port !== undefined && info.port !== undefined && source.port !== info.port) { + conflict("port", String(source.port), String(info.port)); + } + if (source.database && info.database && source.database !== info.database) { + conflict("database", source.database, info.database); + } + if (source.user && info.user && source.user !== info.user) { + conflict("user", source.user, info.user); + } + } + + // Password is never introspected into a field (omitted from API responses), + // so compare against the DSN directly. Never echo the values. + if (source.password && source.password !== url.password) { + if (!url.password) { + throw new Error( + `Configuration file ${configPath}: source '${source.id}' has a 'password' field but the DSN has no password. ` + + `The field is ignored at connection time — add the password to the DSN, or use individual connection parameters instead of a DSN.` + ); + } + throw new Error( + `Configuration file ${configPath}: source '${source.id}' has a 'password' field that conflicts ` + + `with the password in the DSN. Set the password in only one place.` + ); + } + + // Dual-home query-string fields. Use the raw query string (not SafeURL, which + // drops empty-valued params) so a present-but-empty param like `?sslmode=` is + // still treated as present and a conflicting field is rejected. + const dsnSslmode = getRawDSNQueryParam(source.dsn!, "sslmode"); + if (source.sslmode && dsnSslmode !== null && dsnSslmode !== source.sslmode) { + conflict("sslmode", source.sslmode, dsnSslmode); + } + + const dsnSslrootcert = getRawDSNQueryParam(source.dsn!, "sslrootcert"); + if ( + source.sslrootcert && + dsnSslrootcert !== null && + expandHomeDir(source.sslrootcert) !== expandHomeDir(dsnSslrootcert) + ) { + conflict("sslrootcert", expandHomeDir(source.sslrootcert), expandHomeDir(dsnSslrootcert)); + } + + const dsnInstanceName = getRawDSNQueryParam(source.dsn!, "instanceName"); + if (source.instanceName && dsnInstanceName !== null && dsnInstanceName !== source.instanceName) { + conflict("instanceName", source.instanceName, dsnInstanceName); + } + + const dsnAuthentication = getRawDSNQueryParam(source.dsn!, "authentication"); + if (source.authentication && dsnAuthentication !== null && dsnAuthentication !== source.authentication) { + conflict("authentication", source.authentication, dsnAuthentication); + } + + const dsnDomain = getRawDSNQueryParam(source.dsn!, "domain"); + if (source.domain && dsnDomain !== null && dsnDomain !== source.domain) { + conflict("domain", source.domain, dsnDomain); + } +} + /** * Validate a single source configuration */ @@ -352,6 +496,15 @@ function validateSourceConfig(source: SourceConfig, configPath: string): void { } } + // Reject fields that contradict the DSN. A DSN already encodes the connection + // identity (type/host/port/database/user/password) and may carry query params + // (sslmode/sslrootcert/instanceName/authentication/domain). When the same value + // is also set as a standalone field with a different value, the field is + // silently ignored at connection time, so we fail fast instead. + if (source.dsn) { + validateDSNFieldConflicts(source, configPath); + } + // Validate sslrootcert if provided if (source.sslrootcert !== undefined) { if (source.sslmode !== "verify-ca" && source.sslmode !== "verify-full") { @@ -595,14 +748,94 @@ function expandHomeDir(filePath: string): string { return filePath; } +/** + * Merge "dual-home" fields (those that can be expressed either as a TOML field + * or as a DSN query parameter) into an existing DSN. Only appends a param when + * the DSN does not already specify it; conflicting values are rejected earlier + * by validateSourceConfig, so any param already present is guaranteed to match. + * + * This mirrors the query parameters built by the connection-params path of + * buildDSNFromSource so both paths produce equivalent DSNs. + */ +function mergeSourceFieldsIntoDSN(dsn: string, source: SourceConfig): string { + // SQLite DSNs never carry these parameters + if (source.type === "sqlite") { + return dsn; + } + + try { + // Parse only to validate the DSN; if it can't be parsed, leave it untouched + // and let the connector surface the error. + new SafeURL(dsn); + } catch { + return dsn; + } + + // Use raw key presence (not SafeURL, which drops empty-valued params) so we + // never append a duplicate of a param the DSN already specifies, even as + // `?sslmode=`. Such empty-but-present params are rejected by validation when a + // conflicting field is set. + const hasParam = (key: string): boolean => getRawDSNQueryParam(dsn, key) !== null; + + const additions: string[] = []; + + // SQL Server query parameters + if (source.type === "sqlserver") { + if (source.instanceName && !hasParam("instanceName")) { + additions.push(`instanceName=${encodeURIComponent(source.instanceName)}`); + } + if (source.authentication && !hasParam("authentication")) { + additions.push(`authentication=${encodeURIComponent(source.authentication)}`); + } + if (source.domain && !hasParam("domain")) { + additions.push(`domain=${encodeURIComponent(source.domain)}`); + } + } + + if (source.sslmode && !hasParam("sslmode")) { + additions.push(`sslmode=${source.sslmode}`); + } + + if ( + source.sslrootcert && + source.type === "postgres" && + (source.sslmode === "verify-ca" || source.sslmode === "verify-full") && + !hasParam("sslrootcert") + ) { + const expandedCertPath = expandHomeDir(source.sslrootcert); + additions.push(`sslrootcert=${encodeURIComponent(expandedCertPath)}`); + } + + if (additions.length === 0) { + return dsn; + } + + // Pick the right join character: "?" when no query string exists yet, nothing + // when the DSN already ends with "?" or "&" (an empty/trailing query string), + // otherwise "&". + let separator: string; + if (!dsn.includes("?")) { + separator = "?"; + } else if (dsn.endsWith("?") || dsn.endsWith("&")) { + separator = ""; + } else { + separator = "&"; + } + return `${dsn}${separator}${additions.join("&")}`; +} + /** * Build DSN from source connection parameters * Similar to buildDSNFromEnvParams in env.ts but for TOML sources */ export function buildDSNFromSource(source: SourceConfig): string { - // If DSN is already provided, use it + // If DSN is already provided, use it — but merge in dual-home fields + // (sslmode/sslrootcert/instanceName/authentication/domain) so they actually + // affect the connection. Conflicts between the DSN query string and these + // fields are rejected at validation time, so any param already present in the + // DSN is guaranteed to match. if (source.dsn) { - return source.dsn; + return mergeSourceFieldsIntoDSN(source.dsn, source); } // Validate required fields