Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
114 changes: 114 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,120 @@ describe('createWorkspaceProvidersStatusProvider', () => {
expect(result.initialized).toBe(false);
});

it('keeps a port and the text after it when the message has no credentials', async () => {
// A `:` in the authority used to be read as the start of a password, so any
// later `@` — an email address, an npm scope — was taken as the end of the
// userinfo and everything between them was deleted.
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Cannot reach https://api.example:8443/v1 — contact admin@example.com';
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(
'Cannot reach https://api.example:8443/v1 — contact admin@example.com',
);
});

it('strips a password containing an @ instead of splitting on the first one', async () => {
// `indexOf('@')` found the one inside the password, so the cut landed too
// early and the rest of the password was emitted.
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Failed loading provider https://user:p@ssw0rd-tail@broken.example/v1';
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(
'Failed loading provider https://broken.example/v1',
);
expect(JSON.stringify(result)).not.toContain('ssw0rd-tail');
});

it('strips credentials from every URL in a message and leaves the rest intact', async () => {
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Moving from https://user:sec ret@a.example/v1 to https://user:other@b.example:8443/v2; see admin@example.com';
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(
'Moving from https://a.example/v1 to https://b.example:8443/v2; see admin@example.com',
);
});

it('keeps a pathless URL and the email after it intact', async () => {
// A pathless URL has no `/`, `?` or `#` to bound its authority, so handing
// the trailing prose to the sanitizer let the `@` in an email address read
// as the end of a userinfo: the host was rewritten and the text deleted.
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Cannot reach https://api.example.com — contact admin@example.com';
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(
'Cannot reach https://api.example.com — contact admin@example.com',
);
});

it('keeps a pathless URL with a port and the email after it intact', async () => {
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Cannot reach https://api.example.com:8443 — contact admin@example.com';
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(
'Cannot reach https://api.example.com:8443 — contact admin@example.com',
);
});

it('strips a spaced password from a pathless URL followed by prose', async () => {
// The counterpart to the two above: bounding the URL at the first space
// must not stop a password that legally contains one from being found.
coreMock.throwModelsConfigError = true;
coreMock.modelsConfigErrorMessage =
'Failed loading provider https://user:sec ret@broken.example — retry later';
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(
'Failed loading provider https://broken.example — retry later',
);
expect(JSON.stringify(result)).not.toContain('sec ret');
});

async function writeUserSettings(settings: Record<string, unknown>) {
await fs.writeFile(
path.join(qwenHome, 'settings.json'),
Expand Down
74 changes: 42 additions & 32 deletions packages/cli/src/serve/workspace-providers-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,25 @@ 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;

/**
* A `user:password@` prefix, where the password may contain spaces. The
* negative lookahead rejects `host:8443 ` so a port is not read as the start of
* a password — that is the only way a colon in the authority can be followed by
* whitespace and still not be credentials.
*/
const CREDENTIAL_PREFIX_PATTERN = /^[^\s/?#'"`<>]+:(?!\d+(?:\s|$))[^/?#]*@/;

/**
* Strips credentials from every URL in a free-text warning or error message.
*
* Each URL is handed to `sanitizeProviderBaseUrl`, which scopes credential
* detection to the authority. Splitting the message into spans first rather than
* matching URLs with one global regex is what lets a password containing a space
* or a quote — legal in userinfo, but excluded by any `[^\s'"]`-style URL
* pattern — still be found.
*/
function sanitizeProviderWarning(warning: string): string {
let result = '';
let index = 0;
Expand All @@ -260,7 +276,9 @@ 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 urlEnd = findUrlEnd(segment, next.marker.length);
result +=
sanitizeProviderBaseUrl(segment.slice(0, urlEnd)) + segment.slice(urlEnd);

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

/**
* Where the URL ends inside a span that may continue into prose.
*
* A span runs to the next URL or end of line, so it can carry trailing text.
* Handing that text to `sanitizeProviderBaseUrl` would let an `@` in it — an
* email address, say — be read as the end of a userinfo, since a pathless URL
* has no delimiter to bound the authority. So the URL ends at the first space,
* except that a recognised `user:password@` prefix is consumed first: a
* password may legally contain spaces, and finding those is the whole reason
* the message is split into spans rather than matched with a URL regex.
*/
function findUrlEnd(segment: string, markerLength: number): number {
const credentials = CREDENTIAL_PREFIX_PATTERN.exec(
segment.slice(markerLength),
);
const from = credentials
? markerLength + credentials[0].length
: markerLength;
const space = segment.slice(from).search(/\s/);
return space === -1 ? segment.length : from + space;
}

function findNextUrlStart(
value: string,
from: number,
Expand All @@ -295,36 +335,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