diff --git a/src/utils/__tests__/dsn-obfuscate.test.ts b/src/utils/__tests__/dsn-obfuscate.test.ts index 11956ce9..bbf8ec04 100644 --- a/src/utils/__tests__/dsn-obfuscate.test.ts +++ b/src/utils/__tests__/dsn-obfuscate.test.ts @@ -67,6 +67,17 @@ describe('DSN Obfuscation Utilities', () => { const result = obfuscateDSNPassword(dsn); expect(result).toBe('postgres://user:****@localhost:5432?sslmode=require'); }); + + it('should obfuscate the whole password when it contains an @', () => { + // Splitting the authority on the first '@' masks only the part before it + // and leaves the rest in the string, so the tail of the password reaches + // whatever reads this line — DBHub prints it per source at startup. + const dsn = 'postgres://user:pa@ss@localhost:5432/db'; + const result = obfuscateDSNPassword(dsn); + + expect(result).toBe('postgres://user:*****@localhost:5432/db'); + expect(result).not.toContain('ss@'); + }); }); describe('obfuscateSSHConfig', () => { diff --git a/src/utils/__tests__/safe-url.test.ts b/src/utils/__tests__/safe-url.test.ts index 05e805c2..403341e7 100644 --- a/src/utils/__tests__/safe-url.test.ts +++ b/src/utils/__tests__/safe-url.test.ts @@ -104,4 +104,61 @@ describe('SafeURL', () => { it('should throw an error for URLs without a protocol', () => { expect(() => new SafeURL('localhost:5432/dbname')).toThrow('Invalid URL format: missing protocol'); }); + + describe("'@' inside the password", () => { + it('splits on the last @ of the authority, not the first', () => { + // Splitting on the first '@' yields password 'pa' and hostname + // 'ss@localhost', so the failure surfaces as a DNS lookup error rather + // than as a bad password — a confusing way to learn it was truncated. + const url = new SafeURL('sqlserver://user:pa@ss@localhost/dbname'); + + expect(url.username).toBe('user'); + expect(url.password).toBe('pa@ss'); + expect(url.hostname).toBe('localhost'); + expect(url.pathname).toBe('/dbname'); + }); + + it('handles a password containing several @ characters', () => { + const url = new SafeURL('sqlserver://user:a@b@c@host/db'); + + expect(url.username).toBe('user'); + expect(url.password).toBe('a@b@c'); + expect(url.hostname).toBe('host'); + }); + + it('does not treat an @ in the path as the separator', () => { + // The path is still attached when the authority is split, so an unbounded + // lastIndexOf would pick the '@' in the database name instead. + const url = new SafeURL('sqlserver://user:pass@localhost/we@ird'); + + expect(url.username).toBe('user'); + expect(url.password).toBe('pass'); + expect(url.hostname).toBe('localhost'); + expect(url.pathname).toBe('/we@ird'); + }); + + it('keeps working when only the path contains an @', () => { + const url = new SafeURL('sqlserver://localhost/we@ird'); + + expect(url.username).toBe(''); + expect(url.password).toBe(''); + expect(url.hostname).toBe('localhost'); + expect(url.pathname).toBe('/we@ird'); + }); + + it('still accepts a percent-encoded @', () => { + const url = new SafeURL('sqlserver://user:pa%40ss@localhost/dbname'); + + expect(url.password).toBe('pa@ss'); + expect(url.hostname).toBe('localhost'); + }); + + it('keeps the port when the password carries an @', () => { + const url = new SafeURL('sqlserver://user:pa@ss@localhost:1433/dbname'); + + expect(url.password).toBe('pa@ss'); + expect(url.hostname).toBe('localhost'); + expect(url.port).toBe('1433'); + }); + }); }); \ No newline at end of file diff --git a/src/utils/safe-url.ts b/src/utils/safe-url.ts index 5b3b491f..ba84ebb2 100644 --- a/src/utils/safe-url.ts +++ b/src/utils/safe-url.ts @@ -82,8 +82,20 @@ export class SafeURL implements ISafeURL { }); } - // Extract authentication - const atIndex: number = urlString.indexOf('@'); + // Extract authentication. + // + // The separator is the LAST '@' of the authority, not the first: '@' is a + // legal password character, and passwords carrying one are common. Taking + // the first '@' would cut the password short and fold its tail into the + // hostname, which surfaces as a DNS failure rather than as a bad password. + // + // The search is bounded by the start of the path, because the path is + // still attached at this point and may legitimately contain '@' (a + // database named "a@b", say) — an unbounded lastIndexOf would then treat + // that one as the separator. + const pathStart: number = urlString.indexOf('/'); + const authorityEnd: number = pathStart === -1 ? urlString.length : pathStart; + const atIndex: number = urlString.lastIndexOf('@', authorityEnd - 1); if (atIndex !== -1) { const auth: string = urlString.substring(0, atIndex); urlString = urlString.substring(atIndex + 1);