Skip to content

Commit f124bf0

Browse files
committed
fix(supportBundle): cap log size and stop redacting defaults
Address PR #971 review findings: - CRF-12: tail-truncate log files over 50 MB in readLogFile to prevent OOM during rezip when a long-lived session has a multi-GB debug log - CRF-13: passthrough `defaultValue` in maybeRedact so package.json defaults (public, no user data) stay visible for triage
1 parent 162b611 commit f124bf0

4 files changed

Lines changed: 50 additions & 6 deletions

File tree

src/supportBundle/files.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ export interface CollectedFile {
1010
relativePath: string;
1111
}
1212

13-
// 3 days is enough context for recent issues; matching the 7-day
14-
// rotation would bloat the bundle.
13+
/** 3-day window; the 7-day rotation would bloat the bundle. */
1514
const LOG_MAX_AGE_MS = 3 * 24 * 60 * 60 * 1000;
1615
const MAX_LOG_SCAN_DEPTH = 6;
16+
/** Per-file cap to prevent OOM on multi-GB debug logs. */
17+
const MAX_LOG_FILE_BYTES = 50 * 1024 * 1024;
1718

1819
// Accept .log and VS Code's rotated .log.N form.
1920
export const isLogFile = (name: string): boolean => /\.log(\.\d+)?$/.test(name);
@@ -46,7 +47,22 @@ function isEnoent(error: unknown): boolean {
4647
);
4748
}
4849

49-
/** Read a file with an lstat guard that rejects symlinks. */
50+
/** Read the last `length` bytes of `filePath`. */
51+
async function readTail(filePath: string, length: number): Promise<Uint8Array> {
52+
const handle = await fs.open(filePath, "r");
53+
try {
54+
const { size } = await handle.stat();
55+
const offset = Math.max(0, size - length);
56+
const readLen = Math.min(length, size);
57+
const buf = Buffer.alloc(readLen);
58+
const { bytesRead } = await handle.read(buf, 0, readLen, offset);
59+
return buf.subarray(0, bytesRead);
60+
} finally {
61+
await handle.close();
62+
}
63+
}
64+
65+
/** lstat-guarded read; tail-truncates files over `MAX_LOG_FILE_BYTES`. */
5066
export async function readLogFile(
5167
filePath: string,
5268
logger: Logger,
@@ -56,6 +72,15 @@ export async function readLogFile(
5672
if (!stat.isFile()) {
5773
return undefined;
5874
}
75+
if (stat.size > MAX_LOG_FILE_BYTES) {
76+
logger.warn(
77+
`Truncating log file ${filePath} (${stat.size} bytes) to last ${MAX_LOG_FILE_BYTES} bytes`,
78+
);
79+
return {
80+
data: await readTail(filePath, MAX_LOG_FILE_BYTES),
81+
mtimeMs: stat.mtimeMs,
82+
};
83+
}
5984
return { data: await fs.readFile(filePath), mtimeMs: stat.mtimeMs };
6085
} catch (error) {
6186
logger.warn(`Could not read log file ${filePath}`, error);

src/supportBundle/settings.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,15 @@ function redactedSettingValue(value: SettingValue): string {
5757
: "<set>";
5858
}
5959

60+
/** inspect() metadata + public package.json default; not user-supplied. */
61+
const REDACTION_PASSTHROUGH = new Set(["key", "languageIds", "defaultValue"]);
62+
6063
function maybeRedact(
6164
key: string,
6265
name: string,
6366
value: SettingValue,
6467
): SettingValue {
65-
// `key` and `languageIds` are inspect() metadata, not the secret payload.
66-
if (name === "key" || name === "languageIds") {
68+
if (REDACTION_PASSTHROUGH.has(name)) {
6769
return value;
6870
}
6971
return REDACTED_SETTINGS.has(key) ? redactedSettingValue(value) : value;

test/unit/supportBundle/files.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isLogFile,
1111
normalizeZipPath,
1212
prefixFiles,
13+
readLogFile,
1314
} from "@/supportBundle/files";
1415

1516
import { createMockLogger } from "../../mocks/testHelpers";
@@ -88,6 +89,22 @@ describe("support bundle file helpers", () => {
8889
]);
8990
});
9091

92+
it("truncates oversized log files to the tail", async () => {
93+
const filePath = path.join(tmpDir, "huge.log");
94+
const head = Buffer.alloc(60 * 1024 * 1024, "H");
95+
const tail = Buffer.from("TAIL_MARKER\n");
96+
await fs.writeFile(filePath, Buffer.concat([head, tail]));
97+
98+
const result = await readLogFile(filePath, logger);
99+
100+
expect(result?.data.byteLength).toBe(50 * 1024 * 1024);
101+
expect(
102+
Buffer.from(result?.data ?? new Uint8Array())
103+
.subarray(-tail.byteLength)
104+
.toString(),
105+
).toBe("TAIL_MARKER\n");
106+
});
107+
91108
it.runIf(process.platform !== "win32")(
92109
"does not follow symlinks when reading files",
93110
async () => {

test/unit/supportBundle/settings.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ describe("collectSettingsFile", () => {
7979

8080
expect(raw).not.toContain("DO_NOT_LEAK_SECRET");
8181
expect(settings["coder.headerCommand"]).toEqual({
82-
defaultValue: "<empty>",
82+
defaultValue: "",
8383
effective: "<set>",
8484
globalValue: "<set>",
8585
key: "coder.headerCommand",

0 commit comments

Comments
 (0)