Skip to content

Commit dfa2924

Browse files
committed
fix: close two file-system race windows flagged by CodeQL
Both are js/file-system-race (TOCTOU) findings: - server/src/translation.ts ensureTraConfig: replace the existsSync-then-write check with an atomic exclusive create (writeFileSync flag "wx"), treating EEXIST as the intended no-op. This makes the "never clobbers an existing project config" guarantee actually atomic instead of racy. - binary/src/cli.ts loadJsonToBinary: open the JSON file once and fstat + read through the same descriptor, so the size check and the read operate on one inode. A statSync->readFileSync pair on the path could otherwise be swapped between check and read to bypass the snapshot-input size cap. Behavior is unchanged on the normal paths (same error messages and exit codes); binary CLI, binary unit, and server translation suites pass.
1 parent 7b2bf1c commit dfa2924

2 files changed

Lines changed: 21 additions & 17 deletions

File tree

binary/src/cli.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -198,34 +198,32 @@ Examples:
198198
* Validation and semantic round-trip checks happen inside the shared snapshot loader.
199199
*/
200200
function loadJsonToBinary(jsonPath: string, parseOptions: ParseOptions): void {
201-
let stat: fs.Stats;
201+
// Open once, then fstat and read through the SAME descriptor so the size check and the read
202+
// operate on one inode. A statSync->readFileSync pair on the path leaves a TOCTOU window
203+
// (CodeQL js/file-system-race) where a file swapped after the check could bypass the input cap.
204+
let fd: number;
202205
try {
203-
stat = fs.statSync(jsonPath);
206+
fd = fs.openSync(jsonPath, "r");
204207
} catch (error) {
205208
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
206209
console.error(`Not found: ${jsonPath}`);
207210
process.exit(1);
208211
}
209212
throw error;
210213
}
211-
if (stat.size > MAX_JSON_SNAPSHOT_INPUT_BYTES) {
212-
console.error(
213-
`File too large: ${stat.size} bytes (snapshot input cap is ${MAX_JSON_SNAPSHOT_INPUT_BYTES}); refusing to read ${jsonPath}`,
214-
);
215-
process.exit(1);
216-
}
217214

218215
let jsonText: string;
219216
try {
220-
// Read with try/catch instead of existsSync->readFileSync to avoid
221-
// the TOCTOU window CodeQL js/file-system-race flags.
222-
jsonText = fs.readFileSync(jsonPath, "utf-8");
223-
} catch (error) {
224-
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
225-
console.error(`Not found: ${jsonPath}`);
217+
const size = fs.fstatSync(fd).size;
218+
if (size > MAX_JSON_SNAPSHOT_INPUT_BYTES) {
219+
console.error(
220+
`File too large: ${size} bytes (snapshot input cap is ${MAX_JSON_SNAPSHOT_INPUT_BYTES}); refusing to read ${jsonPath}`,
221+
);
226222
process.exit(1);
227223
}
228-
throw error;
224+
jsonText = fs.readFileSync(fd, "utf-8");
225+
} finally {
226+
fs.closeSync(fd);
229227
}
230228

231229
const loaded = loadBinaryJsonSnapshot(jsonText, parseOptions);

server/src/translation.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,11 +465,17 @@ export class Translation {
465465
private ensureTraConfig(relDir: string): void {
466466
if (!this.workspaceRoot) return;
467467
const configPath = path.join(this.workspaceRoot, ".bgforge.yml");
468-
if (fs.existsSync(configPath)) return;
469468
try {
470-
fs.writeFileSync(configPath, yaml.stringify({ mls: { translation: { directory: relDir } } }));
469+
// `flag: "wx"` creates the file only if it does not already exist and fails with EEXIST
470+
// otherwise, so an existing project config is never clobbered - atomically, with no
471+
// existsSync->writeFileSync TOCTOU window.
472+
fs.writeFileSync(configPath, yaml.stringify({ mls: { translation: { directory: relDir } } }), {
473+
flag: "wx",
474+
});
471475
conlog(`Translation: created .bgforge.yml (translation.directory: ${relDir})`);
472476
} catch (error) {
477+
// File already present: the config exists, nothing to create - not a failure.
478+
if ((error as NodeJS.ErrnoException).code === "EEXIST") return;
473479
conlog(`Translation: could not create .bgforge.yml: ${errorMessage(error)}`, "warn");
474480
}
475481
}

0 commit comments

Comments
 (0)