fix: harden error handling, fix TOCTOU races, and improve test accuracy

Core library fixes:
- session-manager.js: wrap all statSync calls in try-catch to prevent
  TOCTOU crashes when files are deleted between readdir and stat
- session-manager.js: use birthtime||ctime fallback for Linux compat
- session-manager.js: remove redundant existsSync before readFile
- utils.js: fix findFiles TOCTOU race on statSync inside readdir loop

Hook improvements:
- Add 1MB stdin buffer limits to all PostToolUse hooks to prevent
  unbounded memory growth from large payloads
- suggest-compact.js: use fd-based atomic read+write for counter file
  to reduce race window between concurrent invocations
- session-end.js: log when transcript file is missing, check
  replaceInFile return value for failed timestamp updates
- start-observer.sh: log claude CLI failures instead of silently
  swallowing them, check observations file exists before analysis

Test fixes:
- Fix blocking hook tests to send matching input (dev server command)
  and expect correct exit code 2 instead of 1
This commit is contained in:
Affaan Mustafa
2026-02-12 13:40:14 -08:00
parent f375171b13
commit 36864ea11a
11 changed files with 113 additions and 44 deletions

View File

@@ -26,10 +26,13 @@ const EXCLUDED_PATTERNS = [
/__mocks__\//,
];
const MAX_STDIN = 1024 * 1024; // 1MB limit
let data = '';
process.stdin.on('data', chunk => {
data += chunk;
if (data.length < MAX_STDIN) {
data += chunk;
}
});
process.stdin.on('end', () => {

View File

@@ -11,10 +11,13 @@
const fs = require('fs');
const MAX_STDIN = 1024 * 1024; // 1MB limit
let data = '';
process.stdin.on('data', chunk => {
data += chunk;
if (data.length < MAX_STDIN) {
data += chunk;
}
});
process.stdin.on('end', () => {

View File

@@ -11,10 +11,13 @@
const { execFileSync } = require('child_process');
const fs = require('fs');
const MAX_STDIN = 1024 * 1024; // 1MB limit
let data = '';
process.stdin.on('data', chunk => {
data += chunk;
if (data.length < MAX_STDIN) {
data += chunk;
}
});
process.stdin.on('end', () => {

View File

@@ -13,10 +13,13 @@ const { execFileSync } = require('child_process');
const fs = require('fs');
const path = require('path');
const MAX_STDIN = 1024 * 1024; // 1MB limit
let data = '';
process.stdin.on('data', chunk => {
data += chunk;
if (data.length < MAX_STDIN) {
data += chunk;
}
});
process.stdin.on('end', () => {

View File

@@ -99,17 +99,24 @@ async function main() {
const transcriptPath = process.env.CLAUDE_TRANSCRIPT_PATH;
let summary = null;
if (transcriptPath && fs.existsSync(transcriptPath)) {
summary = extractSessionSummary(transcriptPath);
if (transcriptPath) {
if (fs.existsSync(transcriptPath)) {
summary = extractSessionSummary(transcriptPath);
} else {
log(`[SessionEnd] Transcript not found: ${transcriptPath}`);
}
}
if (fs.existsSync(sessionFile)) {
// Update existing session file
replaceInFile(
const updated = replaceInFile(
sessionFile,
/\*\*Last Updated:\*\*.*/,
`**Last Updated:** ${currentTime}`
);
if (!updated) {
log(`[SessionEnd] Failed to update timestamp in ${sessionFile}`);
}
// If we have a new summary and the file still has the blank template, replace it
if (summary) {

View File

@@ -13,10 +13,10 @@
* - Compact after completing a milestone, before starting next
*/
const fs = require('fs');
const path = require('path');
const {
getTempDir,
readFile,
writeFile,
log
} = require('../lib/utils');
@@ -32,16 +32,28 @@ async function main() {
let count = 1;
// Read existing count or start at 1
const existing = readFile(counterFile);
if (existing) {
const parsed = parseInt(existing.trim(), 10);
// Guard against NaN from corrupted counter file
count = Number.isFinite(parsed) ? parsed + 1 : 1;
// Use fd-based read+write to reduce (but not eliminate) race window
// between concurrent hook invocations
try {
const fd = fs.openSync(counterFile, 'a+');
try {
const buf = Buffer.alloc(64);
const bytesRead = fs.readSync(fd, buf, 0, 64, 0);
if (bytesRead > 0) {
const parsed = parseInt(buf.toString('utf8', 0, bytesRead).trim(), 10);
count = Number.isFinite(parsed) ? parsed + 1 : 1;
}
// Truncate and write new value
fs.ftruncateSync(fd, 0);
fs.writeSync(fd, String(count), 0);
} finally {
fs.closeSync(fd);
}
} catch {
// Fallback: just use writeFile if fd operations fail
writeFile(counterFile, String(count));
}
// Save updated count
writeFile(counterFile, String(count));
// Suggest compact after threshold tool calls
if (count === threshold) {
log(`[StrategicCompact] ${threshold} tool calls reached - consider /compact if transitioning phases`);

View File

@@ -58,10 +58,6 @@ function getSessionPath(filename) {
* @returns {string|null} Session content or null if not found
*/
function getSessionContent(sessionPath) {
if (!fs.existsSync(sessionPath)) {
return null;
}
return readFile(sessionPath);
}
@@ -217,8 +213,14 @@ function getAllSessions(options = {}) {
const sessionPath = path.join(sessionsDir, filename);
// Get file stats
const stats = fs.statSync(sessionPath);
// Get file stats (wrapped in try-catch to handle TOCTOU race where
// file is deleted between readdirSync and statSync)
let stats;
try {
stats = fs.statSync(sessionPath);
} catch {
continue; // File was deleted between readdir and stat
}
sessions.push({
...metadata,
@@ -226,7 +228,7 @@ function getAllSessions(options = {}) {
hasContent: stats.size > 0,
size: stats.size,
modifiedTime: stats.mtime,
createdTime: stats.birthtime
createdTime: stats.birthtime || stats.ctime
});
}
@@ -278,14 +280,19 @@ function getSessionById(sessionId, includeContent = false) {
}
const sessionPath = path.join(sessionsDir, filename);
const stats = fs.statSync(sessionPath);
let stats;
try {
stats = fs.statSync(sessionPath);
} catch {
return null; // File was deleted between readdir and stat
}
const session = {
...metadata,
sessionPath,
size: stats.size,
modifiedTime: stats.mtime,
createdTime: stats.birthtime
createdTime: stats.birthtime || stats.ctime
};
if (includeContent) {
@@ -319,11 +326,12 @@ function getSessionTitle(sessionPath) {
* @returns {string} Formatted size (e.g., "1.2 KB")
*/
function getSessionSize(sessionPath) {
if (!fs.existsSync(sessionPath)) {
let stats;
try {
stats = fs.statSync(sessionPath);
} catch {
return '0 B';
}
const stats = fs.statSync(sessionPath);
const size = stats.size;
if (size < 1024) return `${size} B`;
@@ -387,7 +395,11 @@ function deleteSession(sessionPath) {
* @returns {boolean} True if session exists
*/
function sessionExists(sessionPath) {
return fs.existsSync(sessionPath) && fs.statSync(sessionPath).isFile();
try {
return fs.statSync(sessionPath).isFile();
} catch {
return false;
}
}
module.exports = {

View File

@@ -168,14 +168,19 @@ function findFiles(dir, pattern, options = {}) {
const fullPath = path.join(currentDir, entry.name);
if (entry.isFile() && regex.test(entry.name)) {
let stats;
try {
stats = fs.statSync(fullPath);
} catch {
continue; // File deleted between readdir and stat
}
if (maxAge !== null) {
const stats = fs.statSync(fullPath);
const ageInDays = (Date.now() - stats.mtimeMs) / (1000 * 60 * 60 * 24);
if (ageInDays <= maxAge) {
results.push({ path: fullPath, mtime: stats.mtimeMs });
}
} else {
const stats = fs.statSync(fullPath);
results.push({ path: fullPath, mtime: stats.mtimeMs });
}
} else if (entry.isDirectory() && recursive) {