Skip to content

Commit c32b589

Browse files
msamoilenkomsamoilenko
committed
fix: bound error log payloads and isolate test homes
Mask raw sk- keys, truncate every logged string, cap entries at 32k chars, and retain the last 50 entries. Run each package test suite with a temporary HOME so tests never write to the real ~/.deepcode.
1 parent 6163168 commit c32b589

5 files changed

Lines changed: 162 additions & 72 deletions

File tree

‎packages/cli/src/tests/run-tests.mjs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,22 @@
22
import { globSync } from "glob";
33
import { spawnSync } from "child_process";
44
import { fileURLToPath } from "url";
5+
import * as fs from "fs";
6+
import * as os from "os";
57
import * as path from "path";
68

79
const __dirname = path.dirname(fileURLToPath(import.meta.url));
810
const testFiles = globSync("*.test.ts", { cwd: __dirname });
9-
10-
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", ...testFiles], {
11-
stdio: "inherit",
12-
cwd: __dirname,
13-
});
14-
15-
process.exit(result.status ?? 1);
11+
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "deepcode-cli-tests-home-"));
12+
let status = 1;
13+
try {
14+
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", ...testFiles], {
15+
stdio: "inherit",
16+
cwd: __dirname,
17+
env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome },
18+
});
19+
status = result.status ?? 1;
20+
} finally {
21+
fs.rmSync(tempHome, { recursive: true, force: true });
22+
}
23+
process.exit(status);

‎packages/core/src/common/error-logger.ts‎

Lines changed: 48 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,59 +17,44 @@ function ensureLogDir(): void {
1717
* or response bodies.
1818
*/
1919
function maskSensitive(text: string): string {
20-
return (
21-
text
22-
// Mask Bearer tokens in Authorization headers
23-
.replace(/(Authorization:\s*Bearer\s+)[^\s\r\n]+/gi, "$1***MASKED***")
24-
// Mask "apiKey" or "api_key" values in JSON-like strings
25-
.replace(/((?:api[Kk]ey|api_key|secret)\s*[:=]\s*"?)[^",}\s]+/gi, "$1***MASKED***")
26-
);
20+
return text
21+
.replace(/(Authorization:\s*(?:Bearer\s+)?)[^\s\r\n]+/gi, "$1***MASKED***")
22+
.replace(/\bsk-[A-Za-z0-9_-]{8,}\b/g, "sk-***MASKED***")
23+
.replace(/((?:api[Kk]ey|api_key|secret|token|authorization)\s*[:=]\s*"?)[^",}\s]+/gi, "$1***MASKED***");
2724
}
2825

29-
const CONTENT_TRUNCATE_PREVIEW = 100;
26+
const MAX_STRING_LENGTH = 500;
27+
const MAX_LOG_LINE_CHARS = 32_000;
28+
const MAX_ENTRIES = 50;
3029

31-
/**
32-
* Truncate a content string for logging: keep a short prefix and append the
33-
* total length so the payload structure is preserved while content bloat is
34-
* avoided.
35-
*/
36-
function truncateContent(value: string): string {
37-
if (value.length <= CONTENT_TRUNCATE_PREVIEW) {
38-
return value;
30+
function truncateString(value: string): string {
31+
const masked = maskSensitive(value);
32+
if (masked.length <= MAX_STRING_LENGTH) {
33+
return masked;
3934
}
40-
return `${value.slice(0, CONTENT_TRUNCATE_PREVIEW)}...(total ${value.length} chars)`;
35+
return `${masked.slice(0, MAX_STRING_LENGTH)}...(total ${masked.length} chars)`;
4136
}
4237

4338
/**
44-
* Deep-clone a request payload, only truncating `content` fields whose value
45-
* is a string. Every other field is kept exactly as-is so the logged request
46-
* mirrors the original API payload (no fields added or removed).
39+
* Deep-clone a payload, masking and truncating every string value. This keeps
40+
* the structure for debugging without logging full prompts, tool arguments, or
41+
* response bodies.
4742
*/
48-
function sanitizeRequestPayload(request: Record<string, unknown>): Record<string, unknown> {
49-
function walk(value: unknown): unknown {
50-
if (!value || typeof value !== "object") {
51-
return value;
52-
}
53-
54-
if (Array.isArray(value)) {
55-
return value.map(walk);
56-
}
57-
58-
const record = value as Record<string, unknown>;
59-
const result: Record<string, unknown> = {};
60-
61-
for (const [key, val] of Object.entries(record)) {
62-
if (key === "content" && typeof val === "string") {
63-
result[key] = truncateContent(val);
64-
} else {
65-
result[key] = walk(val);
66-
}
67-
}
68-
69-
return result;
43+
function sanitizePayload(value: unknown): unknown {
44+
if (typeof value === "string") {
45+
return truncateString(value);
7046
}
71-
72-
return walk(request) as Record<string, unknown>;
47+
if (!value || typeof value !== "object") {
48+
return value;
49+
}
50+
if (Array.isArray(value)) {
51+
return value.map(sanitizePayload);
52+
}
53+
const result: Record<string, unknown> = {};
54+
for (const [key, val] of Object.entries(value)) {
55+
result[key] = sanitizePayload(val);
56+
}
57+
return result;
7358
}
7459

7560
export type ApiErrorLogEntry = {
@@ -99,18 +84,30 @@ export function logApiError(entry: ApiErrorLogEntry): void {
9984
model: entry.model,
10085
baseURL: entry.baseURL,
10186
error: sanitizeError(entry.error),
102-
request: sanitizeRequestPayload(entry.request),
87+
request: sanitizePayload(entry.request),
10388
};
10489

10590
if (entry.response !== undefined) {
106-
logLine.response = typeof entry.response === "string" ? maskSensitive(entry.response) : entry.response;
91+
logLine.response = sanitizePayload(entry.response);
10792
}
10893

109-
const newLine = JSON.stringify(logLine) + "\n";
110-
fs.appendFileSync(ERROR_LOG_PATH, newLine, "utf8");
94+
let newLine = JSON.stringify(logLine);
95+
if (newLine.length > MAX_LOG_LINE_CHARS) {
96+
newLine = JSON.stringify({
97+
timestamp: entry.timestamp,
98+
location: entry.location,
99+
requestId: entry.requestId,
100+
sessionId: entry.sessionId,
101+
model: entry.model,
102+
baseURL: entry.baseURL,
103+
error: sanitizeError(entry.error),
104+
truncated: true,
105+
note: `entry exceeded ${MAX_LOG_LINE_CHARS} chars`,
106+
});
107+
}
108+
fs.appendFileSync(ERROR_LOG_PATH, `${newLine}\n`, "utf8");
111109

112110
// Keep only the last N entries
113-
const MAX_ENTRIES = 20;
114111
const raw = fs.readFileSync(ERROR_LOG_PATH, "utf8");
115112
const lines = raw.split("\n").filter((line) => line.trim().length > 0);
116113
if (lines.length > MAX_ENTRIES) {
@@ -124,8 +121,8 @@ export function logApiError(entry: ApiErrorLogEntry): void {
124121
function sanitizeError(error: LlmErrorDetails): LlmErrorDetails {
125122
return {
126123
...error,
127-
message: maskSensitive(error.message),
128-
stack: error.stack ? maskSensitive(error.stack) : undefined,
124+
message: truncateString(error.message),
125+
stack: error.stack ? truncateString(error.stack) : undefined,
129126
causes: error.causes?.map(sanitizeError),
130127
};
131128
}

‎packages/core/src/tests/error-logger.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { test } from "node:test";
2+
import assert from "node:assert/strict";
3+
import * as fs from "fs";
4+
import * as os from "os";
5+
import * as path from "path";
6+
7+
test("logApiError masks secrets, truncates payloads, and enforces retention", async () => {
8+
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "deepcode-error-logger-"));
9+
const previousHome = process.env.HOME;
10+
process.env.HOME = tempHome;
11+
try {
12+
const { logApiError } = await import("../common/error-logger.js");
13+
const logPath = path.join(tempHome, ".deepcode", "logs", "error.log");
14+
15+
for (let index = 0; index < 55; index += 1) {
16+
logApiError({
17+
timestamp: new Date().toISOString(),
18+
location: "test",
19+
requestId: `req-${index}`,
20+
sessionId: "session-1",
21+
error: {
22+
name: "Error",
23+
message: "Bearer sk-1234567890abcdef api_key=secret-value",
24+
} as never,
25+
request: {
26+
model: "test",
27+
messages: [{ role: "user", content: `hello ${"x".repeat(10_000)}` }],
28+
tools: [{ type: "function", function: { name: "x", description: "y".repeat(10_000) } }],
29+
},
30+
response: { body: "z".repeat(50_000) },
31+
});
32+
}
33+
34+
const retained = fs.readFileSync(logPath, "utf8");
35+
const retainedLines = retained.split("\n").filter((line) => line.trim().length > 0);
36+
assert.equal(retainedLines.length, 50);
37+
assert.ok(retained.length < 2_000_000, `log grew to ${retained.length} bytes`);
38+
assert.ok(!retained.includes("sk-1234567890abcdef"), "raw secret key leaked into the log");
39+
assert.ok(!retained.includes("secret-value"), "raw api_key leaked into the log");
40+
assert.ok(retained.includes("***MASKED***"), "sensitive values were not masked");
41+
assert.ok(retained.includes("...(total "), "long strings were not truncated");
42+
for (const line of retainedLines) {
43+
assert.ok(line.length <= 32_000, `log entry is too long: ${line.length}`);
44+
}
45+
46+
const oversizedRequest: Record<string, string> = {};
47+
for (let index = 0; index < 100; index += 1) {
48+
oversizedRequest[`field${index}`] = "x".repeat(500);
49+
}
50+
logApiError({
51+
timestamp: new Date().toISOString(),
52+
location: "test",
53+
requestId: "req-oversized",
54+
error: { name: "Error", message: "oversized" } as never,
55+
request: oversizedRequest,
56+
});
57+
58+
const finalLines = fs
59+
.readFileSync(logPath, "utf8")
60+
.split("\n")
61+
.filter((line) => line.trim().length > 0);
62+
const last = JSON.parse(finalLines.at(-1) ?? "{}") as { truncated?: boolean };
63+
assert.equal(last.truncated, true);
64+
assert.ok((finalLines.at(-1) ?? "").length <= 32_000);
65+
} finally {
66+
process.env.HOME = previousHome;
67+
fs.rmSync(tempHome, { recursive: true, force: true });
68+
}
69+
});

‎packages/core/src/tests/run-tests.mjs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,22 @@
22
import { globSync } from "glob";
33
import { spawnSync } from "child_process";
44
import { fileURLToPath } from "url";
5+
import * as fs from "fs";
6+
import * as os from "os";
57
import * as path from "path";
68

79
const __dirname = path.dirname(fileURLToPath(import.meta.url));
810
const testFiles = globSync("*.test.ts", { cwd: __dirname });
9-
10-
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", "--test-concurrency=1", ...testFiles], {
11-
stdio: "inherit",
12-
cwd: __dirname,
13-
});
14-
15-
process.exit(result.status ?? 1);
11+
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "deepcode-core-tests-home-"));
12+
let status = 1;
13+
try {
14+
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", "--test-concurrency=1", ...testFiles], {
15+
stdio: "inherit",
16+
cwd: __dirname,
17+
env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome },
18+
});
19+
status = result.status ?? 1;
20+
} finally {
21+
fs.rmSync(tempHome, { recursive: true, force: true });
22+
}
23+
process.exit(status);

‎packages/vscode-ide-companion/src/tests/run-tests.mjs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,22 @@
22
import { globSync } from "glob";
33
import { spawnSync } from "child_process";
44
import { fileURLToPath } from "url";
5+
import * as fs from "fs";
6+
import * as os from "os";
57
import * as path from "path";
68

79
const __dirname = path.dirname(fileURLToPath(import.meta.url));
810
const testFiles = globSync("*.test.ts", { cwd: __dirname });
9-
10-
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", ...testFiles], {
11-
stdio: "inherit",
12-
cwd: __dirname,
13-
});
14-
15-
process.exit(result.status ?? 1);
11+
const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "deepcode-vscode-tests-home-"));
12+
let status = 1;
13+
try {
14+
const result = spawnSync(process.execPath, ["--import", "tsx", "--test", ...testFiles], {
15+
stdio: "inherit",
16+
cwd: __dirname,
17+
env: { ...process.env, HOME: tempHome, USERPROFILE: tempHome },
18+
});
19+
status = result.status ?? 1;
20+
} finally {
21+
fs.rmSync(tempHome, { recursive: true, force: true });
22+
}
23+
process.exit(status);

0 commit comments

Comments
 (0)