Skip to content
42 changes: 42 additions & 0 deletions packages/cli/src/serve/workspace-providers-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,48 @@ describe('createWorkspaceProvidersStatusProvider', () => {
expect(result.initialized).toBe(false);
});

it.each([
[
'keeps a port and later email unchanged',
'Cannot reach https://api.example:8443/v1 — contact admin@example.com',
'Cannot reach https://api.example:8443/v1 — contact admin@example.com',
],
[
'strips a password containing an at sign',
'Failed loading provider https://user:p@ssw0rd-tail@broken.example/v1',
'Failed loading provider https://broken.example/v1',
],
[
'keeps an uncredentialed URL with a port byte-identical',
'Cannot reach https://api.example:8443/v1',
'Cannot reach https://api.example:8443/v1',
],
[
'keeps a pathless URL and later email unchanged',
'Cannot reach https://api.example — contact admin@example.com',
'Cannot reach https://api.example — contact admin@example.com',
],
[
'strips credentials from a pathless URL before trailing prose',
'Cannot reach https://user:pass@host.io please contact admin@corp.io',
'Cannot reach https://host.io please contact admin@corp.io',
],
])('%s', async (_name, message, expected) => {
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage = message;
const provider = createWorkspaceProvidersStatusProvider({ env: {} });
await writeUserSettings({
security: { auth: { selectedType: 'openai' } },
modelProviders: {
openai: [{ id: 'model-a', name: 'Model A' }],
},
});

const result = await provider(workspace, true);

expect(result.errors?.[0]?.error).toBe(expected);
});

async function writeUserSettings(settings: Record<string, unknown>) {
await fs.writeFile(
path.join(qwenHome, 'settings.json'),
Expand Down
76 changes: 44 additions & 32 deletions packages/cli/src/serve/workspace-providers-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ function resolveApprovalMode(settings: Settings): ApprovalMode {
return ApprovalMode.AUTO;
}

const URL_LIKE_PATTERN = /\b[A-Za-z][A-Za-z\d+.-]*:\/\/[^\s'"`<>]+/g;
const URL_START_PATTERN = /\b[A-Za-z][A-Za-z\d+.-]*:\/\//g;

function sanitizeProviderWarning(warning: string): string {
Expand All @@ -260,7 +259,8 @@ function sanitizeProviderWarning(warning: string): string {

const segmentEnd = findUrlSegmentEnd(warning, next.index, next.marker);
const segment = warning.slice(next.index, segmentEnd);
result += sanitizeProviderWarningSegment(segment, next.marker.length);
const { urlToken, trailing } = splitProviderWarningUrlToken(segment);
result += sanitizeProviderBaseUrl(urlToken) + trailing;

index = segmentEnd;
next = findNextUrlStart(warning, index);
Expand All @@ -269,6 +269,48 @@ function sanitizeProviderWarning(warning: string): string {
return result + warning.slice(index);
}

/**
* Split a URL-leading warning segment into the URL token and trailing prose.
*
* Whitespace usually ends the URL, but a space inside userinfo
* (`https://user:sec ret@host/v1`) must stay in the token so credential
* stripping can see the terminating `@`. Multi-word gaps before `@`
* (e.g. ` — contact admin@example.com`) stay in trailing prose.
*/
function splitProviderWarningUrlToken(segment: string): {
urlToken: string;
trailing: string;
} {
const urlBreak = segment.search(/[\s'"`<>]/);
if (urlBreak === -1) {
return { urlToken: segment, trailing: '' };
}

let end = urlBreak;
const scheme = segment.match(/^[A-Za-z][A-Za-z\d+.-]*:\/\//);
if (scheme && !segment.slice(scheme[0].length, urlBreak).includes('@')) {
const rest = segment.slice(urlBreak);
const userinfoAt = rest.indexOf('@');
if (userinfoAt !== -1) {
const gap = rest.slice(0, userinfoAt);
if (/^\s*\S+$/.test(gap)) {
const afterAt = rest.slice(userinfoAt + 1);
const hostPathBreak = afterAt.search(/[\s'"`<>]/);
end =
urlBreak +
userinfoAt +
1 +
(hostPathBreak === -1 ? afterAt.length : hostPathBreak);
}
}
}

return {
urlToken: segment.slice(0, end),
trailing: segment.slice(end),
};
}

function findNextUrlStart(
value: string,
from: number,
Expand All @@ -295,36 +337,6 @@ function findUrlSegmentEnd(
return Math.min(lineEnd, nextUrl?.index ?? value.length);
}

function sanitizeProviderWarningSegment(
segment: string,
markerLength: number,
): string {
const at = segment.indexOf('@', markerLength);
if (
at !== -1 &&
hasCredentialPrefix(segment, markerLength, at) &&
segment[at + 1] !== undefined &&
/[A-Za-z0-9.[\]-]/.test(segment[at + 1])
) {
return `${segment.slice(0, markerLength)}${segment.slice(at + 1)}`;
}

return segment.replace(URL_LIKE_PATTERN, (url) =>
sanitizeProviderBaseUrl(url),
);
}

function hasCredentialPrefix(
segment: string,
markerLength: number,
at: number,
): boolean {
const colon = segment.indexOf(':', markerLength);
if (colon === -1 || colon > at) return false;
const username = segment.slice(markerLength, colon);
return !/[/?#\s'"`<>]/.test(username);
}

function buildCurrent(
authType: AuthType | undefined,
modelId: string | undefined,
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/utils/acpModelUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,28 @@ describe('acpModelUtils', () => {
['https://user:p?x@api.example/v1', 'https://api.example/v1'],
['https://user:p#x@api.example/v1', 'https://api.example/v1'],
['https://user:secret@api.example', 'https://api.example'],
[
'https://api.example — contact admin@example.com',
'https://api.example — contact admin@example.com',
],
[
'https://user:pass@host.io please contact admin@corp.io',
'https://host.io please contact admin@corp.io',
],
[
'https://host bad:x/v1 — contact admin@corp.io',
'https://host bad:x/v1 — contact admin@corp.io',
],
['https://user:pa ss@host.io', 'https://host.io'],
['https://user:12 34@host.io', 'https://host.io'],
['https://user name:pass@host.io', 'https://host.io'],
['https://user name@host.io', 'https://host.io'],
['https://us er@host', 'https://host'],
[
'https://user:pa ss@host.io please contact admin@corp.io',
'https://host.io please contact admin@corp.io',
],
['https://user:p w@host.io/a@b', 'https://host.io/a@b'],
])('sanitizes provider base URL credentials for %s', (input, expected) => {
expect(sanitizeProviderBaseUrl(input)).toBe(expected);
});
Expand Down
46 changes: 33 additions & 13 deletions packages/cli/src/utils/acpModelUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,33 +179,45 @@ export function sanitizeProviderBaseUrl(baseUrl: string): string {

try {
const parsed = new URL(baseUrl);
if (parsed.username || parsed.password) {
return authorityAtIndex >= authorityStart
? stripAt(authorityAtIndex)
: baseUrl;
if (!(parsed.username || parsed.password)) {
return baseUrl;
}
if (authorityAtIndex >= authorityStart) {
return stripAt(authorityAtIndex);
}
// WHATWG accepts spaces in userinfo (encoded as %20), but our authority
// span stops at raw whitespace — so `@` can sit past authorityEnd for a
// password that contains a space. Strip at the first `@` after authorityEnd
// when the gap still looks like userinfo, not trailing prose.
const userinfoAt = baseUrl.indexOf('@', authorityEnd);
if (userinfoAt !== -1) {
const gap = baseUrl.slice(authorityEnd, userinfoAt);
if (parsed.password || /^\s*\S+$/.test(gap)) {
return stripAt(userinfoAt);
}
}
return baseUrl;
} catch {
if (authorityAtIndex >= authorityStart) {
return stripAt(authorityAtIndex);
}

const fallbackAt = findUnescapedUserInfoFallbackAt(
baseUrl,
authorityStart,
authorityEnd,
);
return fallbackAt === -1 ? baseUrl : stripAt(fallbackAt);
}

const fallbackAt = findUnescapedUserInfoFallbackAt(
baseUrl,
authorityStart,
authorityEnd,
);
return fallbackAt === -1 ? baseUrl : stripAt(fallbackAt);
}

function findUnescapedUserInfoFallbackAt(
baseUrl: string,
authorityStart: number,
authorityEnd: number,
): number {
const at = baseUrl.lastIndexOf('@');
if (at < authorityStart || authorityEnd >= at) {
const at = baseUrl.indexOf('@', authorityEnd);
if (at === -1 || at < authorityStart) {
return -1;
}

Expand All @@ -226,6 +238,14 @@ function findAuthorityEnd(baseUrl: string, authorityStart: number): number {
if (slash !== -1) end = Math.min(end, slash);
if (query !== -1) end = Math.min(end, query);
if (hash !== -1) end = Math.min(end, hash);
// Raw whitespace is never valid in a URL authority. Treat it as a terminator
// so a pathless URL followed by prose (e.g. a contact email) does not pull
// the trailing text into the authority span.
for (let i = authorityStart; i < end; i++) {
if (/\s/.test(baseUrl.charAt(i))) {
return i;
}
}
return end;
}

Expand Down
Loading