fix: add TOCTOU ownership check to post-hook content overwrite path

The sessionSaveContent replacement path now verifies the file still
contains our writtenEntry before overwriting — same guard as the
late-block retraction path. Prevents concurrent runs with the same
filename from clobbering each other's valid saves.
This commit is contained in:
zeroaltitude 2026-03-09 23:56:33 -07:00
parent b6583a379d
commit 28f44b1736
No known key found for this signature in database
GPG Key ID: 77592FB1C703882E

View File

@ -525,6 +525,34 @@ const saveSessionToMemory: HookHandler = async (event) => {
// is null because blockPreSet was true) OR if the content changed.
(writtenEntry === null || postContent !== writtenEntry)
) {
// Verify ownership before overwriting — if another concurrent run wrote
// to the same file since our inline write, do not clobber their content.
// Same TOCTOU guard as the late-block retraction path.
if (writtenEntry !== null) {
let currentContent: string | null = null;
try {
currentContent = await fs.readFile(memoryFilePath, "utf-8");
} catch (err: unknown) {
if (
err instanceof Error &&
"code" in err &&
(err as NodeJS.ErrnoException).code === "ENOENT"
) {
// File was externally deleted — safe to recreate with new content.
currentContent = null;
} else {
throw err;
}
}
if (currentContent !== null && currentContent !== writtenEntry) {
log.warn(
"Session save content replacement skipped — file was modified by another " +
"session since our inline write (concurrent save detected)",
);
return;
}
}
// Ensure memoryDir exists — the inline write may have been
// skipped (e.g. blockSessionSave was true initially) so mkdir
// might never have run.