From 63a1b1d05ebf929f1f846ab6c6f67b1e405668f7 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 16:47:57 -0700 Subject: [PATCH] fix: use crypto.randomUUID for slug suffix, use blockPreSet variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace Math.random().toString(36) with crypto.randomUUID() for uniformly-distributed 4-char hex suffix — eliminates weak entropy from floating-point base-36 conversion edge cases - Use already-captured blockPreSet variable instead of re-reading context.blockSessionSave for consistency and clarity - Update test to mock crypto.randomUUID instead of Math.random Addresses greptile review: weak entropy risk + redundant context read. --- src/hooks/bundled/session-memory/handler.test.ts | 8 ++++---- src/hooks/bundled/session-memory/handler.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 61aa3e2205d..e7c85d8f603 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -650,14 +650,14 @@ describe("session-memory hook", () => { content: createMockSessionContent([{ role: "user", content: "first session" }]), }); - // Pin Math.random AND timestamp to force deterministic fallback slug — + // Pin crypto.randomUUID AND timestamp to force deterministic fallback slug — // both handler calls produce the same HHMMSS prefix (fixed timestamp) - // and the same random suffix (pinned Math.random). LLM slug generation - // is disabled in the test environment (VITEST=true), so the collision + // and the same random suffix (pinned UUID). LLM slug generation is + // disabled in the test environment (VITEST=true), so the collision // is exercised entirely through the fallback path. Without pinning // the clock, a wall-clock second boundary between event1 and event2 // would produce different HHMMSS prefixes → no collision. - vi.spyOn(Math, "random").mockReturnValue(0.5); + vi.spyOn(crypto, "randomUUID").mockReturnValue("aaaa1111-2222-3333-4444-555566667777"); const fixedTimestamp = new Date("2024-01-15T12:34:56.000Z"); try { diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 2478770e26c..398786c8390 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -332,7 +332,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - const rand = Math.random().toString(36).slice(2, 6).padEnd(4, "0"); + const rand = crypto.randomUUID().replace(/-/g, "").slice(0, 4); slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } @@ -425,7 +425,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // Defer retraction/replacement to post-hook phase so that hooks // registered after this handler can set blockSessionSave or // sessionSaveContent and still have them honored. - const writtenEntry = context.blockSessionSave === true ? null : entry; + const writtenEntry = blockPreSet ? null : entry; // Post-hook callback — errors propagate to the framework's per-action // catch in triggerInternalHook, which provides consistent log formatting // and per-action isolation.