Skip to content

Commit ed23303

Browse files
ihor-sokoliukclaudecodex
committed
fix(search): avoid mutating read-only error.message; harden redaction fallback (BUG-007)
Addresses a second Copilot review on PR #136: - Build a redacted Error instead of assigning to error.message. The timeout path's AbortController abort rejects fetch with a DOMException whose `message` is getter-only, so the in-place mutation threw a secondary TypeError that masked the real error. The new Error preserves code/cause for createNetworkError; a DOMException regression test guards it. - Widen the URL()-parse-failure fallback to strip userinfo through the last `@` in the authority ([^/]*@) so multi-@ authorities cannot leak later segments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
1 parent f76c514 commit ed23303

4 files changed

Lines changed: 40 additions & 6 deletions

File tree

__tests__/unit/search.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,28 @@ async function runTests() {
969969
envManager.restore();
970970
}, results);
971971

972+
await testFunction('read-only error message (aborted request) does not throw a secondary TypeError', async () => {
973+
clearSearxngInstanceStateForTests();
974+
envManager.set('SEARXNG_URL', 'https://user:pass@abort.example.com');
975+
976+
const mockServer = createMockServer();
977+
fetchMocker.mock(async () => {
978+
throw new DOMException('This operation was aborted', 'AbortError');
979+
});
980+
981+
try {
982+
await performWebSearch(mockServer as any, 'abort');
983+
assert.fail('Expected a handled network error');
984+
} catch (error: any) {
985+
assert.ok(!/only a getter|set property message/.test(error.message), error.message);
986+
assert.match(error.message, /^🌐 Network Error:/);
987+
assert.ok(!error.message.includes('user:pass@'), error.message);
988+
}
989+
990+
fetchMocker.restore();
991+
envManager.restore();
992+
}, results);
993+
972994
await testFunction('SEARXNG_TIMEOUT_MS env override is respected (50 ms fires before 500 ms mock)', async () => {
973995
envManager.set('SEARXNG_URL', 'https://test-searx.example.com');
974996
envManager.set('SEARXNG_TIMEOUT_MS', '50');

__tests__/unit/searxng-instances.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ async function runTests() {
9898
assert.ok(!redacted.includes('pass'), redacted);
9999
}, results);
100100

101+
await testFunction('redactSearxngInstanceUrl strips multi-at userinfo from unparsable URL strings', () => {
102+
const redacted = redactSearxngInstanceUrl('https://a:b@c@ho st.example.com');
103+
104+
assert.equal(redacted, 'https://ho st.example.com');
105+
assert.ok(redacted.includes('ho st.example.com'), redacted);
106+
assert.ok(!redacted.includes('a:b'), redacted);
107+
assert.ok(!redacted.includes('@c'), redacted);
108+
}, results);
109+
101110
await testFunction('redactSearxngInstanceUrl leaves non-URL strings unchanged after parse failure', () => {
102111
assert.equal(redactSearxngInstanceUrl('not a url'), 'not a url');
103112
}, results);

src/search.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,20 @@ async function fetchWithSearchTimeout(
150150
signal: controller.signal,
151151
});
152152
} catch (error: any) {
153-
if (typeof error.message === "string") {
154-
error.message = error.message.replaceAll(rawUrl, redactedUrl);
155-
}
156-
logMessage(mcpServer, "error", `Network error during search request: ${error.message}`, { query, url: redactedUrl });
153+
const safeMessage = typeof error?.message === "string"
154+
? error.message.replaceAll(rawUrl, redactedUrl)
155+
: error?.message;
156+
const safeError = new Error(safeMessage);
157+
(safeError as any).code = error?.code;
158+
(safeError as any).cause = error?.cause;
159+
logMessage(mcpServer, "error", `Network error during search request: ${safeMessage}`, { query, url: redactedUrl });
157160
const context: ErrorContext = {
158161
url: redactedUrl,
159162
searxngUrl: redactSearxngInstanceUrl(searxngUrl),
160163
proxyAgent: !!(requestOptions as any).dispatcher,
161164
username: process.env.AUTH_USERNAME,
162165
};
163-
throw createNetworkError(error, context);
166+
throw createNetworkError(safeError, context);
164167
} finally {
165168
clearTimeout(timeoutId);
166169
}

src/searxng-instances.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export function redactSearxngInstanceUrl(raw: string): string {
6565
url.password = "";
6666
return url.toString();
6767
} catch {
68-
return raw.replace(/^([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)[^/@]*@/, "$1");
68+
return raw.replace(/^([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)[^/]*@/, "$1");
6969
}
7070
}
7171

0 commit comments

Comments
 (0)