Ayuriel/openclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: verify own write before retraction to prevent concurrent save clobber

Browse Source 
The retraction path now reads current file content and compares it to
writtenEntry before restoring preExistingContent or unlinking. If the
content differs (another session wrote to the same file concurrently),
retraction is skipped with a warning — preventing stale preExistingContent
from overwriting a newer concurrent save.

Also removes the now-redundant ENOENT catch in the unlink path since
the pre-read already handles missing files.
This commit is contained in:
zeroaltitude 2026-03-09 23:21:03 -07:00
parent d9cce17a75
commit 2ae019c330
No known key found for this signature in database
GPG Key ID: 77592FB1C703882E
1 changed files with 33 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/hooks/bundled/session-memory/handler.ts
View File

@ -458,6 +458,37 @@ const saveSessionToMemory: HookHandler = async (event) => {
"session-memory handler runs.",
);
}
// Verify we're reverting our own write before touching the file.
// A concurrent session (e.g. /new, /reset) may have written to the
// same filename between our inline write and this post-hook drain.
// If the current content doesn't match what we wrote, skip retraction
// to avoid clobbering the other session's data.
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 — nothing to retract.
log.debug("Session save retraction skipped — file already removed");
return;
}
throw err;
}
if (currentContent !== writtenEntry) {
// File content differs from what we wrote — another session has
// written to this file since our inline write. Do not clobber.
log.warn(
"Session save retraction skipped — file was modified by another " +
"session since our inline write (concurrent save detected)",
);
return;
}
if (preExistingContent !== null) {
// Slug collision: another entry already existed at this filename
// before our inline write. Restore the original content rather
@ -473,24 +504,8 @@ const saveSessionToMemory: HookHandler = async (event) => {
});
log.debug("Session save retracted by post-hook — pre-existing file restored");
} else {
try {
await fs.unlink(memoryFilePath);
log.debug("Session save retracted by post-hook (blockSessionSave)");
} catch (err) {
// ENOENT can occur if the file was externally deleted between
// the inline write and the post-hook drain — not a concern.
// Re-throw non-ENOENT errors (e.g. EACCES, EROFS) so
// triggerInternalHook logs them. Note: errors are caught
// per-action and do NOT propagate to the session caller;
// the file may remain on disk under adversarial FS conditions.
if (
!(err instanceof Error) ||
!("code" in err) ||
(err as NodeJS.ErrnoException).code !== "ENOENT"
) {
throw err;
}
}
await fs.unlink(memoryFilePath);
log.debug("Session save retracted by post-hook (blockSessionSave)");
}
return;
}