diff --git a/packages/cli/src/serve/workspace-providers-status.test.ts b/packages/cli/src/serve/workspace-providers-status.test.ts index 1658ca5deed..36c71af9742 100644 --- a/packages/cli/src/serve/workspace-providers-status.test.ts +++ b/packages/cli/src/serve/workspace-providers-status.test.ts @@ -641,6 +641,58 @@ 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', + ], + [ + 'strips multi-word password userinfo from warning URLs', + 'Failed loading provider https://user:pa ss word@host.io/v1', + 'Failed loading provider https://host.io/v1', + ], + [ + 'strips password with embedded at-sign before host', + 'Failed loading provider https://user:p ss@real@host.io/v1', + 'Failed loading provider https://host.io/v1', + ], + ])('%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) { await fs.writeFile( path.join(qwenHome, 'settings.json'), diff --git a/packages/cli/src/serve/workspace-providers-status.ts b/packages/cli/src/serve/workspace-providers-status.ts index 56425c42c91..252f24e95bf 100644 --- a/packages/cli/src/serve/workspace-providers-status.ts +++ b/packages/cli/src/serve/workspace-providers-status.ts @@ -260,7 +260,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 { @@ -273,7 +272,7 @@ 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); + result += sanitizeProviderBaseUrl(segment); index = segmentEnd; next = findNextUrlStart(warning, index); @@ -308,36 +307,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, diff --git a/packages/cli/src/utils/acpModelUtils.test.ts b/packages/cli/src/utils/acpModelUtils.test.ts index 9291ec67238..d496d3821a4 100644 --- a/packages/cli/src/utils/acpModelUtils.test.ts +++ b/packages/cli/src/utils/acpModelUtils.test.ts @@ -249,6 +249,32 @@ 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'], + ['https://user:p ss@real@host.io', 'https://host.io'], + ['https://user @host.io', 'https://host.io'], + ['https://foo bar baz@corp.io', 'https://corp.io'], + ['https://user:pa ss word@host.io/v1', 'https://host.io/v1'], ])('sanitizes provider base URL credentials for %s', (input, expected) => { expect(sanitizeProviderBaseUrl(input)).toBe(expected); }); diff --git a/packages/cli/src/utils/acpModelUtils.ts b/packages/cli/src/utils/acpModelUtils.ts index de14c6f0aad..a2901df0b65 100644 --- a/packages/cli/src/utils/acpModelUtils.ts +++ b/packages/cli/src/utils/acpModelUtils.ts @@ -171,32 +171,108 @@ export function sanitizeProviderBaseUrl(baseUrl: string): string { const stripAt = (at: number) => `${baseUrl.slice(0, authorityStart)}${baseUrl.slice(at + 1)}`; const authorityEnd = findAuthorityEnd(baseUrl, authorityStart); - const authorityAt = baseUrl - .slice(authorityStart, authorityEnd) - .lastIndexOf('@'); + const authoritySlice = baseUrl.slice(authorityStart, authorityEnd); + const authorityAt = authoritySlice.lastIndexOf('@'); const authorityAtIndex = authorityAt === -1 ? -1 : authorityStart + authorityAt; 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); + } + if (shouldExtendUserInfoSearch(authoritySlice, parsed)) { + const userInfoAt = findExtendedUserInfoAt(baseUrl, authorityEnd); + if (userInfoAt !== -1) { + 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 shouldExtendUserInfoSearch( + authoritySlice: string, + parsed: URL, +): boolean { + if (parsed.password) { + return true; + } + if (authoritySlice.includes(':')) { + return true; + } + // A dotted token without ':' is a complete host; trailing prose may follow. + return !(authoritySlice.includes('.') && !authoritySlice.includes(':')); +} + +function findExtendedUserInfoAt(baseUrl: string, authorityEnd: number): number { + const terminator = baseUrl.charAt(authorityEnd); + if (terminator === '/' || terminator === '?' || terminator === '#') { + return findLastAtAfter(baseUrl, authorityEnd); + } + return findHostAtBeforePathOrProse(baseUrl, authorityEnd); +} + +function findHostAtBeforePathOrProse( + baseUrl: string, + authorityEnd: number, +): number { + const pathStart = findPathDelimiterStart(baseUrl, authorityEnd); + let proseEnd = baseUrl.length; + for (let i = authorityEnd + 1; i < baseUrl.length; i++) { + if ( + /\s/.test(baseUrl.charAt(i)) && + baseUrl.indexOf('@', authorityEnd) < i + ) { + proseEnd = i; + break; + } + } + const searchEnd = Math.min(pathStart, proseEnd); + return findLastAtBefore(baseUrl, authorityEnd, searchEnd); +} + +function findPathDelimiterStart( + baseUrl: string, + authorityStart: number, +): number { + const slash = baseUrl.indexOf('/', authorityStart); + const query = baseUrl.indexOf('?', authorityStart); + const hash = baseUrl.indexOf('#', authorityStart); + let end = baseUrl.length; + if (slash !== -1) end = Math.min(end, slash); + if (query !== -1) end = Math.min(end, query); + if (hash !== -1) end = Math.min(end, hash); + return end; +} + +function findLastAtBefore(baseUrl: string, start: number, end: number): number { + const at = baseUrl.slice(start, end).lastIndexOf('@'); + return at === -1 ? -1 : start + at; +} + +function findLastAtAfter(baseUrl: string, from: number): number { + let last = -1; + for (let i = from; i < baseUrl.length; i++) { + if (baseUrl.charAt(i) === '@') { + last = i; + } } + return last; } function findUnescapedUserInfoFallbackAt( @@ -204,8 +280,8 @@ function findUnescapedUserInfoFallbackAt( authorityStart: number, authorityEnd: number, ): number { - const at = baseUrl.lastIndexOf('@'); - if (at < authorityStart || authorityEnd >= at) { + const at = findLastAtAfter(baseUrl, authorityEnd); + if (at === -1) { return -1; } @@ -226,6 +302,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; }