diff --git a/packages/drivers/src/index.ts b/packages/drivers/src/index.ts index 8102d6e27..4854785c6 100644 --- a/packages/drivers/src/index.ts +++ b/packages/drivers/src/index.ts @@ -2,7 +2,7 @@ export type { Connector, ConnectorResult, SchemaColumn, ConnectionConfig } from "./types" // Re-export config normalization -export { normalizeConfig } from "./normalize" +export { normalizeConfig, sanitizeConnectionString } from "./normalize" // Re-export driver connect functions export { connect as connectPostgres } from "./postgres" diff --git a/packages/drivers/src/normalize.ts b/packages/drivers/src/normalize.ts index 4c43acd8b..0f936feb1 100644 --- a/packages/drivers/src/normalize.ts +++ b/packages/drivers/src/normalize.ts @@ -111,6 +111,95 @@ const DRIVER_ALIASES: Record = { // duckdb and sqlite have simple configs — no aliases needed } +// --------------------------------------------------------------------------- +// Connection string password encoding +// --------------------------------------------------------------------------- + +/** + * URI-style connection strings (postgres://, mongodb://, etc.) embed + * credentials in the userinfo section: `scheme://user:password@host/db`. + * If the password contains special characters (@, #, :, /, ?, etc.) and + * they are NOT percent-encoded, drivers will mis-parse the URI and fail + * with cryptic auth errors. + * + * This function detects an unencoded password in the userinfo portion and + * re-encodes it so the connection string is valid. Already-encoded + * passwords (containing %XX sequences) are left untouched. + */ +export function sanitizeConnectionString(connectionString: string): string { + // Only touch scheme://... URIs. + const schemeMatch = connectionString.match(/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//) + if (!schemeMatch) return connectionString + + const scheme = schemeMatch[0] + const afterScheme = connectionString.slice(scheme.length) + + // Find the userinfo/host separator. A password can legitimately contain + // '@', '#', '/', '?', or ':' characters, so the URI spec is ambiguous + // when those are unencoded. We use the LAST '@' as the separator because + // that correctly handles the common case of passwords with special + // characters (the stated purpose of this function — see issue #589). + // + // The known trade-off: if the query string or path also contains an + // unencoded '@' (e.g. `?email=alice@example.com`), the rightmost '@' + // is NOT the userinfo separator. We detect this ambiguous case below + // with a guard and bail to avoid corrupting the URI. + const lastAt = afterScheme.lastIndexOf("@") + if (lastAt < 0) return connectionString // No userinfo — nothing to fix + + const beforeAt = afterScheme.slice(0, lastAt) + const afterAt = afterScheme.slice(lastAt + 1) + + // Ambiguity guard: if the content AFTER the '@' has no path/query/ + // fragment delimiter ('/', '?', or '#') but the content BEFORE the + // '@' does, then the '@' is almost certainly inside a path, query, + // or fragment — not the userinfo separator. Bail and leave the URI + // untouched so the caller can pre-encode explicitly. + // + // Examples that trigger the guard (correctly left alone): + // postgresql://u:p@host/db?email=alice@example.com ('@' in query) + // postgresql://u:p@host:5432/db@archive ('@' in path) + // postgresql://u:p@host/db#at@frag ('@' in fragment) + // + // Examples that pass the guard (correctly encoded): + // postgresql://u:p@ss@host/db (last '@' has '/' after) + // postgresql://u:f@ke#PLACEHOLDER@host/db (last '@' has '/' after) + const afterAtHasDelim = /[/?#]/.test(afterAt) + const beforeAtHasDelim = /[/?#]/.test(beforeAt) + if (!afterAtHasDelim && beforeAtHasDelim) { + return connectionString + } + + // Idempotent re-encoding: decode any existing percent-escapes and + // re-encode. Already-encoded values round-trip unchanged; raw values + // with special characters get encoded. Malformed percent sequences + // fall back to encoding the raw input. + const encodeIdempotent = (v: string): string => { + if (v.length === 0) return v + try { + return encodeURIComponent(decodeURIComponent(v)) + } catch { + return encodeURIComponent(v) + } + } + + // Split userinfo on the FIRST ':' only (password may contain ':'). + const colonIdx = beforeAt.indexOf(":") + let encodedUserinfo: string + if (colonIdx < 0) { + // Username-only userinfo: still encode if it contains special chars + // (e.g. email addresses used as usernames). + encodedUserinfo = encodeIdempotent(beforeAt) + } else { + const user = beforeAt.slice(0, colonIdx) + const password = beforeAt.slice(colonIdx + 1) + encodedUserinfo = `${encodeIdempotent(user)}:${encodeIdempotent(password)}` + } + + const rebuilt = `${scheme}${encodedUserinfo}@${afterAt}` + return rebuilt === connectionString ? connectionString : rebuilt +} + // --------------------------------------------------------------------------- // Core logic // --------------------------------------------------------------------------- @@ -178,6 +267,15 @@ export function normalizeConfig(config: ConnectionConfig): ConnectionConfig { const aliases = DRIVER_ALIASES[type] let result = aliases ? applyAliases(config, aliases) : { ...config } + // Sanitize connection_string: if the password contains special characters + // (@, #, :, /, etc.) that are not percent-encoded, URI-based drivers will + // mis-parse the string and fail with cryptic auth errors. This is the + // single integration point — every caller of normalizeConfig() gets the + // fix automatically. + if (typeof result.connection_string === "string") { + result.connection_string = sanitizeConnectionString(result.connection_string) + } + // Type-specific post-processing // Note: MySQL SSL fields (ssl_ca, ssl_cert, ssl_key) are NOT constructed // into an ssl object here. They stay as top-level fields so the credential diff --git a/packages/opencode/test/altimate/driver-normalize.test.ts b/packages/opencode/test/altimate/driver-normalize.test.ts index 4ca59ef9e..95f348289 100644 --- a/packages/opencode/test/altimate/driver-normalize.test.ts +++ b/packages/opencode/test/altimate/driver-normalize.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { normalizeConfig } from "@altimateai/drivers" +import { normalizeConfig, sanitizeConnectionString } from "@altimateai/drivers" import { isSensitiveField } from "../../src/altimate/native/connections/credential-store" // --------------------------------------------------------------------------- @@ -947,3 +947,212 @@ describe("normalizeConfig — ClickHouse", () => { expect(result.ssl_key).toBeUndefined() }) }) + +// --------------------------------------------------------------------------- +// sanitizeConnectionString — special character encoding +// --------------------------------------------------------------------------- + +describe("sanitizeConnectionString", () => { + test("encodes @ in password", () => { + const input = "postgresql://testuser:t@st@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:t%40st@localhost:5432/testdb") + }) + + test("encodes # in password", () => { + const input = "postgresql://testuser:test#val@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:test%23val@localhost:5432/testdb") + }) + + test("encodes : in password", () => { + const input = "postgresql://testuser:test:val@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:test%3Aval@localhost:5432/testdb") + }) + + test("encodes multiple special characters", () => { + const input = "postgresql://testuser:t@st#v:al@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:t%40st%23v%3Aal@localhost:5432/testdb") + }) + + test("encodes / in password", () => { + const input = "postgresql://testuser:test/val@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:test%2Fval@localhost:5432/testdb") + }) + + test("encodes ? in password", () => { + const input = "postgresql://testuser:test?val@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:test%3Fval@localhost:5432/testdb") + }) + + test("handles malformed percent sequence in username gracefully", () => { + const input = "postgresql://bad%ZZuser:t@st@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + // Should not throw — falls back to encoding the raw username + expect(result).toContain("@localhost:5432/testdb") + }) + + test("leaves already-encoded passwords untouched", () => { + const input = "postgresql://testuser:t%40st%23val@localhost:5432/testdb" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("leaves passwords without special characters untouched", () => { + const input = "postgresql://testuser:simpletestval@localhost:5432/testdb" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("leaves non-URI strings untouched", () => { + const input = "host=localhost dbname=mydb" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("handles mongodb scheme", () => { + const input = "mongodb://testuser:t@st@localhost:27017/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("mongodb://testuser:t%40st@localhost:27017/testdb") + }) + + test("handles mongodb+srv scheme", () => { + const input = "mongodb+srv://testuser:t@st@cluster.example.com/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("mongodb+srv://testuser:t%40st@cluster.example.com/testdb") + }) + + test("leaves URIs without password untouched", () => { + const input = "postgresql://testuser@localhost:5432/testdb" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("preserves @ in query string (does not misinterpret as userinfo)", () => { + const input = + "postgresql://testuser:simpleval@localhost:5432/testdb?contact=alice@example.com" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("bails on ambiguous URIs where both password and query contain @", () => { + // When both the password and the query string contain unencoded '@', + // there's no way to deterministically pick the userinfo separator. + // We return the URI untouched and expect the caller to pre-encode. + const input = + "postgresql://testuser:p@ss@localhost:5432/testdb?contact=alice@example.com" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("encodes @ in username-only userinfo (no password)", () => { + // Email-as-username with no password: the '@' in the email must be + // encoded so the driver doesn't treat the domain as the host. + const input = "postgresql://alice@example.com@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe( + "postgresql://alice%40example.com@localhost:5432/testdb", + ) + }) + + test("encodes @ in partially-encoded password (not short-circuited by %XX)", () => { + // Password contains an encoded space (%20) AND a raw '@'. Previous + // logic short-circuited on seeing %XX and left '@' unencoded, + // producing a broken URI. + const input = "postgresql://testuser:p%20ss@word@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe( + "postgresql://testuser:p%20ss%40word@localhost:5432/testdb", + ) + }) + + test("encodes # in partially-encoded password", () => { + const input = "postgresql://testuser:pa%40ss#word@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + // %40 is preserved; raw '#' gets encoded to %23 + expect(result).toBe( + "postgresql://testuser:pa%40ss%23word@localhost:5432/testdb", + ) + }) + + test("handles malformed percent sequence in password gracefully", () => { + // '%ZZ' is not a valid percent-escape. Falls back to encoding raw. + const input = "postgresql://testuser:bad%ZZpass@localhost:5432/testdb" + const result = sanitizeConnectionString(input) + // Raw-encoded password contains %25 (encoded '%') and ZZ literal + expect(result).toBe( + "postgresql://testuser:bad%25ZZpass@localhost:5432/testdb", + ) + }) + + test("preserves @ in path after authority", () => { + // A path segment with '@' is unusual but valid and must not be + // treated as userinfo. + const input = "postgresql://testuser:simpleval@localhost:5432/db@archive" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("preserves @ in fragment", () => { + const input = "postgresql://testuser:simpleval@localhost:5432/testdb#at@frag" + expect(sanitizeConnectionString(input)).toBe(input) + }) + + test("handles scheme-only URI with no path", () => { + const input = "postgresql://testuser:p@ss@localhost:5432" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:p%40ss@localhost:5432") + }) + + test("handles URI with no port", () => { + const input = "postgresql://testuser:p@ss@localhost/testdb" + const result = sanitizeConnectionString(input) + expect(result).toBe("postgresql://testuser:p%40ss@localhost/testdb") + }) +}) + +// --------------------------------------------------------------------------- +// normalizeConfig — connection_string sanitization integration +// --------------------------------------------------------------------------- + +describe("normalizeConfig — connection_string sanitization", () => { + test("sanitizes connection_string with special chars in password", () => { + const result = normalizeConfig({ + type: "postgres", + connection_string: "postgresql://testuser:f@ke#PLACEHOLDER@localhost:5432/testdb", + }) + expect(result.connection_string).toBe( + "postgresql://testuser:f%40ke%23PLACEHOLDER@localhost:5432/testdb", + ) + }) + + test("sanitizes connectionString alias with special chars", () => { + const result = normalizeConfig({ + type: "postgres", + connectionString: "postgresql://testuser:t@st@localhost:5432/testdb", + }) + // alias resolved to connection_string, then sanitized + expect(result.connection_string).toBe( + "postgresql://testuser:t%40st@localhost:5432/testdb", + ) + expect(result.connectionString).toBeUndefined() + }) + + test("does not alter connection_string without special chars", () => { + const result = normalizeConfig({ + type: "redshift", + connection_string: "postgresql://testuser:testval@localhost:5439/testdb", + }) + expect(result.connection_string).toBe( + "postgresql://testuser:testval@localhost:5439/testdb", + ) + }) + + test("does not alter config without connection_string", () => { + const result = normalizeConfig({ + type: "postgres", + host: "localhost", + password: "f@ke#PLACEHOLDER", + }) + // Individual fields are NOT URI-encoded — drivers handle them natively + expect(result.password).toBe("f@ke#PLACEHOLDER") + expect(result.connection_string).toBeUndefined() + }) +})