Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 5 additions & 4 deletions lib/collectors/codebase.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

const fs = require('fs');
const path = require('path');
const { readFileWithLimit } = require('../utils/fs-safe');

const DEFAULT_OPTIONS = {
depth: 'thorough',
Expand Down Expand Up @@ -194,10 +195,10 @@ function scanFileSymbols(basePath, topLevelDirs) {
if (entry.name.includes('.test.') || entry.name.includes('.spec.')) continue;

try {
const stat = fs.statSync(fullPath);
if (stat.size > MAX_FILE_SIZE) continue;

const content = fs.readFileSync(fullPath, 'utf8');
// Open once and read through the fd (size check + read on the
// same inode) to avoid a stat/read TOCTOU. Oversized or
// unreadable files throw and are skipped by the catch below.
const content = readFileWithLimit(fullPath, MAX_FILE_SIZE);
const symbols = extractSymbols(content);

if (symbols.functions.length || symbols.classes.length || symbols.exports.length) {
Expand Down
13 changes: 8 additions & 5 deletions lib/enhance/auto-suppression.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
const fs = require('fs');
const path = require('path');
const { execFileSync } = require('child_process');
const { readFileWithLimit } = require('../utils/fs-safe');
const { writeJsonAtomic } = require('../utils/atomic-write');

// Try to import cross-platform helpers
let getSuppressionPath;
Expand Down Expand Up @@ -488,19 +490,20 @@ function saveAutoSuppressions(suppressionPath, projectId, findings) {
*/
function clearAutoSuppressions(suppressionPath, projectId) {
try {
if (!fs.existsSync(suppressionPath)) return;

const data = JSON.parse(fs.readFileSync(suppressionPath, 'utf8'));
// Read via fd (no existsSync pre-check) and write atomically, so neither
// the read nor the write races against a swap of the path. A missing file
// throws ENOENT and is swallowed below.
const data = JSON.parse(readFileWithLimit(suppressionPath));

if (data.projects?.[projectId]?.auto_learned) {
data.projects[projectId].auto_learned = {
patterns: {},
stats: { totalSuppressed: 0, lastAnalysis: new Date().toISOString() }
};
fs.writeFileSync(suppressionPath, JSON.stringify(data, null, 2));
writeJsonAtomic(suppressionPath, data);
}
} catch {
// Ignore errors
// Missing file or write error: ignore.
}
}

Expand Down
6 changes: 4 additions & 2 deletions lib/enhance/cross-file-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

const fs = require('fs');
const path = require('path');
const { readFileWithLimit } = require('../utils/fs-safe');
const { parseMarkdownFrontmatter } = require('./agent-analyzer');
const { crossFilePatterns, loadKnownTools } = require('./cross-file-patterns');

Expand Down Expand Up @@ -474,8 +475,9 @@ function loadAllSkills(rootDir, options = {}) {
if (!isPathWithinRoot(skillPath, rootDir)) continue;

try {
fs.accessSync(skillPath);
const content = fs.readFileSync(skillPath, 'utf8');
// Read through a single fd (no separate access check) to avoid an
// access/read TOCTOU; a missing/unreadable file throws and is skipped.
const content = readFileWithLimit(skillPath);
Comment thread
avifenesh marked this conversation as resolved.
const { frontmatter, body } = parseMarkdownFrontmatter(content);

skills.push({
Expand Down
8 changes: 6 additions & 2 deletions lib/enhance/docs-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

const fs = require('fs');
const path = require('path');
const { writeFileAtomic } = require('../utils/atomic-write');
const { getPatternsForMode, estimateTokens } = require('./docs-patterns');

function analyzeDoc(docPath, options = {}) {
Expand Down Expand Up @@ -252,6 +253,9 @@ function applyDocsFixes(issues, options = {}) {
}

let content = fs.readFileSync(filePath, 'utf8');
// Snapshot the on-disk content now so the backup does not re-read
// filePath right before the write (which would be a read/write TOCTOU).
const originalContent = content;
Comment thread
avifenesh marked this conversation as resolved.
const appliedToFile = [];

for (const issue of fileIssues) {
Expand Down Expand Up @@ -283,9 +287,9 @@ function applyDocsFixes(issues, options = {}) {
// Write changes
if (!dryRun && appliedToFile.length > 0) {
if (backup) {
fs.writeFileSync(`${filePath}.backup`, fs.readFileSync(filePath, 'utf8'), 'utf8');
writeFileAtomic(`${filePath}.backup`, originalContent);
}
fs.writeFileSync(filePath, content, 'utf8');
writeFileAtomic(filePath, content);
}

results.applied.push(...appliedToFile);
Expand Down
11 changes: 6 additions & 5 deletions lib/enhance/fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

const fs = require('fs');
const path = require('path');
const { writeFileAtomic } = require('../utils/atomic-write');

/**
* Reject symlinks before read/write operations.
Expand Down Expand Up @@ -195,12 +196,12 @@ function applyFixes(issues, options = {}) {
} else {
newContent = JSON.stringify(modified, null, 2);
}
// Re-check immediately before write. Narrows the TOCTOU window
// between the initial lstat and this writeFileSync (an attacker
// who swaps the regular file for a symlink between calls will
// be caught here).
// Refuse a symlink swap (explicit, early), then write atomically via
// a temp file + rename. The rename replaces the path entry itself and
// never follows a symlink, so the write cannot be redirected through a
// swapped link - closing the lstat/write TOCTOU window.
assertNotSymlink(filePath);
fs.writeFileSync(filePath, newContent, 'utf8');
writeFileAtomic(filePath, newContent);
}

results.applied.push(...appliedToFile);
Expand Down
8 changes: 6 additions & 2 deletions lib/enhance/prompt-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

const fs = require('fs');
const path = require('path');
const { writeFileAtomic } = require('../utils/atomic-write');
Comment thread
avifenesh marked this conversation as resolved.
const { promptPatterns, estimateTokens, extractCodeBlocks } = require('./prompt-patterns');
const reporter = require('./reporter');

Expand Down Expand Up @@ -344,6 +345,9 @@ function applyFixes(results, options = {}) {
}

let content = fs.readFileSync(filePath, 'utf8');
// Snapshot the on-disk content now so the backup does not re-read
// filePath right before the write (which would be a read/write TOCTOU).
const originalContent = content;
Comment thread
avifenesh marked this conversation as resolved.
const appliedToFile = [];

for (const issue of fileIssues) {
Expand All @@ -369,9 +373,9 @@ function applyFixes(results, options = {}) {
// Write changes
if (!dryRun && appliedToFile.length > 0) {
if (backup) {
fs.writeFileSync(`${filePath}.backup`, fs.readFileSync(filePath, 'utf8'), 'utf8');
writeFileAtomic(`${filePath}.backup`, originalContent);
}
fs.writeFileSync(filePath, content, 'utf8');
writeFileAtomic(filePath, content);
}

fixResults.applied.push(...appliedToFile);
Expand Down
25 changes: 12 additions & 13 deletions lib/enhance/suppression.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const fs = require('fs');
const path = require('path');
const { readFileWithLimit } = require('../utils/fs-safe');

/**
* Default suppression config
Expand Down Expand Up @@ -34,20 +35,18 @@ function loadConfig(projectRoot) {
];

for (const configPath of configPaths) {
if (fs.existsSync(configPath)) {
try {
// Check file size before reading to prevent DoS
const stats = fs.statSync(configPath);
if (stats.size > MAX_CONFIG_SIZE) {
console.error(`[WARN] Config file too large (${stats.size} bytes), using defaults`);
continue;
}
const content = fs.readFileSync(configPath, 'utf8');
const userConfig = JSON.parse(content);
return mergeConfig(DEFAULT_CONFIG, userConfig);
} catch (err) {
// Invalid config, use defaults
try {
// Open once and read through the fd (size check + read on the same
// inode) to avoid an exists/stat/read TOCTOU. A missing file throws
// ENOENT and falls through to the next candidate / defaults.
const content = readFileWithLimit(configPath, MAX_CONFIG_SIZE);
const userConfig = JSON.parse(content);
return mergeConfig(DEFAULT_CONFIG, userConfig);
} catch (err) {
if (err && err.code === 'EFBIG') {
console.error('[WARN] Config file too large, using defaults');
}
// Missing or invalid config: try the next candidate / defaults.
}
}

Expand Down
23 changes: 15 additions & 8 deletions lib/patterns/slop-analyzers.js
Original file line number Diff line number Diff line change
Expand Up @@ -858,15 +858,22 @@ function countEntryPointExports(repoPath, options = {}) {
for (const entry of ENTRY_POINTS) {
const fullPath = path.join(repoPath, entry);
try {
const stat = fs.statSync(fullPath);
if (stat.isFile()) {
const content = fs.readFileSync(fullPath, 'utf8');
const lang = detectLanguage(entry);
const exports = countExportsInContent(content, lang);
if (exports > 0) {
foundEntries.push(entry);
count += exports;
// Open once and check/read through the fd so the is-file check and the
// read apply to the same inode (no stat/read TOCTOU). Uses the injected
// fs so mocked tests keep working.
const fd = fs.openSync(fullPath, 'r');
try {
if (fs.fstatSync(fd).isFile()) {
const content = fs.readFileSync(fd, 'utf8');
const lang = detectLanguage(entry);
const exports = countExportsInContent(content, lang);
if (exports > 0) {
foundEntries.push(entry);
count += exports;
}
}
} finally {
fs.closeSync(fd);
}
} catch {
// File doesn't exist, try next
Expand Down
42 changes: 42 additions & 0 deletions lib/utils/fs-safe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const fs = require('fs');

/**
* Read a file with an optional size cap, free of check-then-use (TOCTOU)
* races.
*
* The path is opened once and every subsequent operation (the regular-file
* check, the size check, the read) acts on that file descriptor. Because the
* descriptor is bound to a single inode, swapping the path for another file or
* a symlink between calls cannot redirect the read - which a separate
* `fs.statSync(path)` / `fs.existsSync(path)` followed by `fs.readFileSync(path)`
* cannot guarantee.
*
* @param {string} filePath
* @param {number} [maxSize] - byte ceiling; larger files throw `EFBIG`
* @param {BufferEncoding} [encoding='utf8']
* @returns {string|Buffer} file contents (string when an encoding is given)
* @throws if the path is missing, not a regular file, too large, or unreadable
*/
function readFileWithLimit(filePath, maxSize, encoding = 'utf8') {
const fd = fs.openSync(filePath, 'r');
try {
const stat = fs.fstatSync(fd);
if (!stat.isFile()) {
const err = new Error(`Not a regular file: ${filePath}`);
err.code = 'ENOTFILE';
throw err;
}
if (typeof maxSize === 'number' && stat.size > maxSize) {
const err = new Error(`File too large: ${stat.size} > ${maxSize} bytes`);
err.code = 'EFBIG';
throw err;
}
return fs.readFileSync(fd, encoding);
} finally {
fs.closeSync(fd);
}
}

module.exports = { readFileWithLimit };
Loading