Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/utils/__tests__/dsn-obfuscate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
57 changes: 57 additions & 0 deletions src/utils/__tests__/safe-url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
16 changes: 14 additions & 2 deletions src/utils/safe-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading