-
Notifications
You must be signed in to change notification settings - Fork 184
fix(sessions): redact connection-URI password in unsupported-URI errors #602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cffc429
5819163
ce16458
22508bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| /** | ||
| * Redacts the password from a connection URI so the URI can be safely included | ||
| * in error messages and logs. | ||
| * | ||
| * A database or session-service connection URI such as | ||
| * `postgres://user:password@host:5432/db` embeds the password in its userinfo | ||
| * component. Including such a URI verbatim in a thrown Error or log entry leaks | ||
| * the credential to wherever those are collected (log files, error-tracking | ||
| * services, stdout captured by an orchestrator), which is frequently a | ||
| * different trust boundary from whoever provisioned the connection string. | ||
| * | ||
| * This masks the password while keeping the rest of the URI intact for | ||
| * debugging, mirroring the semantics of Go's `net/url.URL.Redacted()`. | ||
| * | ||
| * If the input cannot be parsed as a URL, only its scheme prefix is returned so | ||
| * that a credential embedded in an otherwise-unparseable string is not leaked. | ||
| */ | ||
| export function redactUriPassword(uri: string): string { | ||
| try { | ||
| const url = new URL(uri); | ||
| if (url.password) { | ||
| url.password = '***'; | ||
| return url.toString(); | ||
| } | ||
| return uri; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a nit, though not a blocker either. A credential in the query string is returned verbatim, because return uri;I checked these against the real helper:
const SECRET_PARAMS = ['password', 'sslpassword', 'passwd', 'pwd'];
let touched = false;
for (const p of SECRET_PARAMS) {
if (url.searchParams.has(p)) {
url.searchParams.set(p, '***');
touched = true;
}
}
if (url.password) {
url.password = '***';
touched = true;
}
return touched ? url.toString() : uri;Caveat I'd want you to weigh rather than take on faith: this makes the function return |
||
| } catch { | ||
| const schemeEnd = uri.indexOf('://'); | ||
| return schemeEnd === -1 | ||
| ? '<redacted>' | ||
| : `${uri.slice(0, schemeEnd)}://<redacted>`; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, optional — the unparseable fallback is right, and I want to flag one consequence you may already have intended. : `${uri.slice(0, schemeEnd)}://<redacted>`;This throws away the host and database name for any URI that |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import {describe, expect, it} from 'vitest'; | ||
| import {getArtifactServiceFromUri} from '../../src/artifacts/registry.js'; | ||
| import {getConnectionOptionsFromUri} from '../../src/sessions/db/operations.js'; | ||
| import {getSessionServiceFromUri} from '../../src/sessions/registry.js'; | ||
| import {redactUriPassword} from '../../src/utils/redact_uri.js'; | ||
|
|
||
| describe('redactUriPassword', () => { | ||
| it('masks the password while keeping the rest of the URI', () => { | ||
| expect(redactUriPassword('postgres://user:pass@db.host:5432/mydb')).toBe( | ||
| 'postgres://user:***@db.host:5432/mydb', | ||
| ); | ||
| }); | ||
|
|
||
| it('masks the password for unsupported schemes too', () => { | ||
| expect(redactUriPassword('oracle://admin:hunter2@ora.host/xe')).toBe( | ||
| 'oracle://admin:***@ora.host/xe', | ||
| ); | ||
| }); | ||
|
|
||
| it('leaves a URI without a password unchanged', () => { | ||
| expect(redactUriPassword('postgres://user@db.host/mydb')).toBe( | ||
| 'postgres://user@db.host/mydb', | ||
| ); | ||
| }); | ||
|
|
||
| it('does not leak anything after the scheme for unparseable input', () => { | ||
| const out = redactUriPassword('not a url with :hunter2@ inside it'); | ||
| expect(out).not.toContain('hunter2'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('connection-URI errors do not leak the password', () => { | ||
| it('getConnectionOptionsFromUri redacts the password in its error', async () => { | ||
| await expect( | ||
| getConnectionOptionsFromUri('oracle://admin:hunter2@ora.host/xe'), | ||
| ).rejects.toThrow(/oracle:\/\/admin:\*\*\*@ora\.host\/xe/); | ||
| await expect( | ||
| getConnectionOptionsFromUri('oracle://admin:hunter2@ora.host/xe'), | ||
| ).rejects.not.toThrow(/hunter2/); | ||
| }); | ||
|
|
||
| it('getSessionServiceFromUri redacts the password in its error', () => { | ||
| expect(() => | ||
| getSessionServiceFromUri('oracle://admin:hunter2@ora.host/xe'), | ||
| ).toThrow(/oracle:\/\/admin:\*\*\*@ora\.host\/xe/); | ||
| expect(() => | ||
| getSessionServiceFromUri('oracle://admin:hunter2@ora.host/xe'), | ||
| ).not.toThrow(/hunter2/); | ||
| }); | ||
|
|
||
| it('getArtifactServiceFromUri redacts the password in its error', () => { | ||
| expect(() => | ||
| getArtifactServiceFromUri('s3://admin:hunter2@bucket/prefix'), | ||
| ).toThrow(/s3:\/\/admin:\*\*\*@bucket\/prefix/); | ||
| expect(() => | ||
| getArtifactServiceFromUri('s3://admin:hunter2@bucket/prefix'), | ||
| ).not.toThrow(/hunter2/); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.