Skip to content

fix(core): redact URL userinfo credentials in shared redactors #4597

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.

Sign up for GitHub

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

Open
Rangsh wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(core): redact URL userinfo credentials in shared redactors #4597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee48f70
fix(core): redact URL userinfo credentials in shared redactors
Rangsh Sep 3, 2026
eb94687
style(core): format URL userinfo redaction tests
Rangsh Sep 3, 2026
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
109 changes: 109 additions & 0 deletions packages/core/src/__tests__/display-redaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import assert from 'node:assert/strict';
import { describe, test } from 'node:test';
import {
redactReversibleStreamingSuffix,
redactSecrets,
redactStableStreamingSuffix,
} from '../display-redaction.js';

const USERINFO_CASES: Array<[string, string]> = [
[
'https://myuser:glpat-AbCdEf12345XyZ@gitlab.com/team/repo.git',
'https://<redacted>@gitlab.com/team/repo.git',
],
[
'https://alice:hunter2@internal.example.com/repo.git',
'https://<redacted>@internal.example.com/repo.git',
],
[
'https://alice:ATBBxyz123abc456@bitbucket.org/team/repo.git',
'https://<redacted>@bitbucket.org/team/repo.git',
],
[
'fatal: unable to access https://deploy:s3cretP@ss@git.corp.example/x.git/: 403',
'fatal: unable to access https://<redacted>@git.corp.example/x.git/: 403',
],
['https://user@host.example/team/repo.git', 'https://<redacted>@host.example/team/repo.git'],
[
'origin https://alice:hunter2@internal.example.com/repo.git (fetch)',
'origin https://<redacted>@internal.example.com/repo.git (fetch)',
],
[
'see https://alice:hunter2@internal.example.com/repo.git.',
'see https://<redacted>@internal.example.com/repo.git.',
],
[
'clone (https://alice:hunter2@internal.example.com/repo.git)',
'clone (https://<redacted>@internal.example.com/repo.git)',
],
];

describe('display redactSecrets', () => {
test('masks URL userinfo credentials without swallowing host or path', () => {
for (const [input, expected] of USERINFO_CASES) {
assert.equal(redactSecrets(input), expected);
}
assert.equal(
redactSecrets('https://api.example.com/v1?token=abc123'),
'https://api.example.com/v1?token=<redacted>',
);
assert.equal(
redactSecrets('https://ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789@github.com/o/r.git'),
'https://<redacted>@github.com/o/r.git',
);
assert.equal(
redactSecrets('https://alice:hunter2@api.example.com/v1?token=abc123'),
'https://<redacted>@api.example.com/v1?token=<redacted>',
);
});
});

describe('display streaming suffix redactors', () => {
test('keeps a stable userinfo suffix compacted until the authority ends', () => {
const suffix = redactStableStreamingSuffix(
'fatal: unable to access https://deploy:s3cretP@ss@',
);
assert.ok(suffix);
assert.equal(suffix.text, 'fatal: unable to access https://<redacted>@');
assert.equal(suffix.settledPrefixText, 'fatal: unable to access ');
assert.equal(suffix.compactedSuffix, 'https://deploy:s3cretP@ss@');
assert.equal(suffix.terminator.test('/'), true);
assert.equal(suffix.terminator.test('?'), true);
assert.equal(redactSecrets(suffix.settledPrefixText + suffix.compactedSuffix), suffix.text);
});

test('does not treat a completed userinfo URL as a streaming suffix', () => {
for (const [input] of USERINFO_CASES) {
const suffix = redactStableStreamingSuffix(input);
assert.equal(suffix, undefined, input);
assert.equal(redactReversibleStreamingSuffix(input), undefined, input);
}
});

test('still shortens a reversible provider token that reaches end-of-input', () => {
const token = `ghp_${'A'.repeat(200)}`;
const reversible = redactReversibleStreamingSuffix(token);
assert.ok(reversible);
assert.equal(redactSecrets(reversible.compactedInput), redactSecrets(token));
assert.equal(reversible.compactedToken.length < token.length, true);
});
});
49 changes: 49 additions & 0 deletions packages/core/src/__tests__/redaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,55 @@ describe('redactSecrets', () => {
assert.equal(text.includes('secret-value'), false);
});

test('masks URL userinfo credentials without swallowing host or path', () => {
const cases: Array<[string, string]> = [
[
'https://myuser:glpat-AbCdEf12345XyZ@gitlab.com/team/repo.git',
'https://[redacted]@gitlab.com/team/repo.git',
],
[
'https://alice:hunter2@internal.example.com/repo.git',
'https://[redacted]@internal.example.com/repo.git',
],
[
'https://alice:ATBBxyz123abc456@bitbucket.org/team/repo.git',
'https://[redacted]@bitbucket.org/team/repo.git',
],
[
'fatal: unable to access https://deploy:s3cretP@ss@git.corp.example/x.git/: 403',
'fatal: unable to access https://[redacted]@git.corp.example/x.git/: 403',
],
['https://user@host.example/team/repo.git', 'https://[redacted]@host.example/team/repo.git'],
[
'origin https://alice:hunter2@internal.example.com/repo.git (fetch)',
'origin https://[redacted]@internal.example.com/repo.git (fetch)',
],
[
'see https://alice:hunter2@internal.example.com/repo.git.',
'see https://[redacted]@internal.example.com/repo.git.',
],
[
'clone (https://alice:hunter2@internal.example.com/repo.git)',
'clone (https://[redacted]@internal.example.com/repo.git)',
],
];
for (const [input, expected] of cases) {
assert.equal(redactSecrets(input), expected);
}
assert.equal(
redactSecrets('https://api.example.com/v1?token=abc123'),
'https://api.example.com/v1?token=[redacted]',
);
assert.equal(
redactSecrets('https://ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789@github.com/o/r.git'),
'https://[redacted]@github.com/o/r.git',
);
assert.equal(
redactSecrets('https://alice:hunter2@api.example.com/v1?token=abc123'),
'https://[redacted]@api.example.com/v1?token=[redacted]',
);
});

test('masks quoted sensitive object keys in serialized JSON', () => {
const text = redactSecrets(
JSON.stringify({
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/display-redaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ const PATTERNS: Pattern[] = [
streamingTerminator: /[\s"'<>]/,
streamingValueGroup: 3,
},
// URL userinfo: https://user:pass@host / https://token@host
// Structural — any authority that contains `@` is credential-bearing, so
// this does not depend on a provider prefix list. Runs before the query
// rule so only the userinfo is replaced and host/path survive.
{
label: 'url userinfo',
regex: /(https?:\/\/)([^/?#]*@)/gi,

@Astro-Han Astro-Han Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 (same defect as redaction.ts): this pattern already declares streamingTerminator: /[/?#\s"'<>]/ on the next line, but the regex class is only [^/?#]. The two disagree, and the regex is the loose one, so a benign https://host plus a later @ on the same or a following line over-redacts the text in between. Bringing the regex in line with the terminator fixes both the over-redaction and the inconsistency: /(https?:\/\/)([^\s"'<>/?#]*@)/gi.

replacement: (m) => `${m[1]}<redacted>@`,
streamingTerminator: /[/?#\s"'<>]/,
streamingValueGroup: 2,
},
// URL query secrets: ?key=[redacted] ?token=[redacted] ?api_key=[redacted] &access_token=[redacted]
// (runs before the api-key-header rule so the URL form isn't mangled.)
{
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/redaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export function redactSecrets(value: string): string {

function redactTextSecrets(value: string): string {
let next = value;
next = redactUrlUserinfoSecrets(next);
next = redactUrlQuerySecrets(next);
next = next.replace(QUOTED_SECRET_KEY_VALUE_PATTERN, (match, prefix: string, key: string) =>
isSensitiveKey(key) ? `${prefix}[redacted]` : match,
Expand Down Expand Up @@ -168,6 +169,12 @@ function redactJsonValue(value: unknown): { value: unknown; changed: boolean } {
return { value: next, changed };
}

function redactUrlUserinfoSecrets(value: string): string {
// Authority runs through the first `/`, `?`, or `#`. If it contains `@`,
// everything from the host-start through the last `@` is userinfo.
return value.replace(/(https?:\/\/)[^/?#]*@/gi, '$1[redacted]@');

@Astro-Han Astro-Han Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [^/?#]* excludes only /, ? and #, so it also matches spaces, quotes and newlines. Any bare https://host followed later by an @, with no /, ? or # in between, swallows everything between them.

I ran this through the real redactSecrets chain (built dist/redaction.js with this same patch applied):

in:   see https://example.com and mail bob@corp.com
out:  see https://[redacted]@corp.com

in:   Fetching https://registry.example.com\nContact: support@example.com for help
out:  Fetching https://[redacted]@example.com for help

Because it crosses newlines, it also trips the whole-screen suppression test in pty-screen-collector.ts:497. With this completely benign npm output:

$ npm install
npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org
npm ERR! 404 '@acme-internal' is not in this registry.

the per-line pass at :472 finds all four lines clean, but redactSecrets(complete) !== complete on the joined text is true, so screenText becomes REDACTED_MARKER and scrollbackText is dropped. The whole screen plus scrollback disappears for both the user and the model, with no indication why.

Smallest fix: exclude whitespace from the userinfo class here, /(https?:\/\/)[^\s/?#]*@/gi, and make the display pattern's regex match the terminator set it already declares. Please also add negative assertions for the two cases above, since every test in this PR is a positive case.

}

function redactUrlQuerySecrets(value: string): string {
return value.replace(/([?&])([^=\s&?#]+)=([^&\s#]*)/g, (match, sep: string, key: string) => {
if (!isSensitiveKey(key)) return match;
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/__tests__/streaming-display-redaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ describe('streaming display redaction', () => {
`Authorization:${' '.repeat(2_048)}Bearer arbitrary-secret-value tail`,
'Authorization:\n\nBearer newline-secret-value tail',
'x-api-key\n:\nnewline-api-key-value tail',
'https://alice:hunter2@internal.example.com/repo.git tail',
];
for (const input of cases) {
for (const sizes of [[1], [3], [7], [20], [64], [1, 31, 2, 127, 5]]) {
Expand Down