fix(security): namespace Notion thread cache per caller + validate client thread ids (IDOR)

Security-review follow-up to #7900. The notion-web thread-session cache was keyed only
by Notion spaceId (space-, not user-scoped) and accepted arbitrary client-supplied thread
ids, so two users of the same space could pin/read each other's thread. Now: (1) the cache
key includes hashNotionCallerCookie(cookie) so each caller gets an isolated namespace, and
(2) readClientThreadId rejects any value that is not a well-formed Notion UUID.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

changelog.d/fixes/notion-thread-idor-hardening.md

@@ -0,0 +1 @@
+- Harden the Notion web provider thread-session cache against cross-tenant access: the cache is now namespaced per caller (hash of the caller's cookie) instead of only by Notion space id, and client-supplied thread ids are validated as real Notion UUIDs before use — closing an IDOR where two users of the same Notion space could pin/read each other's thread (#7900 security review).

open-sse/executors/notion-web.ts

@@ -43,6 +43,7 @@ import {
   notionThreadSessionLookup,
   notionThreadSessionStore,
   readClientThreadId,
+  hashNotionCallerCookie,
   resolveNotionThreadBinding,
   type NotionMessage,
 } from "../services/notionThreadSessions.ts";
@@ -553,8 +554,13 @@ export class NotionWebExecutor extends BaseExecutor {
         | Record<string, string>
         | undefined);
     const clientThreadId = readClientThreadId(requestBody, inboundHeaders ?? undefined);
-    // Namespace thread cache by custom agent so default AI and agents never share threads.
-    const threadSpaceKey = agent.workflowId ? `${spaceId}|wf:${agent.workflowId}` : spaceId;
+    // Namespace the thread cache PER CALLER (hash of the caller's cookie) AND by custom
+    // agent, so (a) two users of the same Notion space never share a cached thread
+    // (cross-tenant IDOR, #7900 review) and (b) default AI and agents never share threads.
+    const callerScope = hashNotionCallerCookie(cookie);
+    const threadSpaceKey = agent.workflowId
+      ? `caller:${callerScope}|${spaceId}|wf:${agent.workflowId}`
+      : `caller:${callerScope}|${spaceId}`;
     const binding = resolveNotionThreadBinding(threadSpaceKey, messages, clientThreadId);
     let { threadId, createThread, rootKey } = binding;
 

open-sse/services/notionThreadSessions.ts

@@ -400,6 +400,16 @@ export function notionThreadSessionStore(
 
 /** Client-supplied thread continuity pin: body (`notion_thread_id`/`thread_id`) or
  * the `X-Notion-Thread-Id` header (case-insensitive). */
+/**
+ * A Notion page/thread id is a UUID (32 hex chars, dashed or undashed). Reject
+ * anything else so a client cannot pin/poison the session cache with an arbitrary
+ * string (defense against cross-tenant thread-id injection — see #7900 review).
+ */
+export function isValidNotionThreadId(id: string): boolean {
+  const t = id.trim().replace(/-/g, "");
+  return /^[0-9a-f]{32}$/i.test(t);
+}
+
 export function readClientThreadId(
   body: NotionThreadRequestBody,
   headers?: Record<string, string>
@@ -408,12 +418,31 @@ export function readClientThreadId(
     (typeof body.notion_thread_id === "string" && body.notion_thread_id.trim()) ||
     (typeof body.thread_id === "string" && body.thread_id.trim()) ||
     "";
-  if (fromBody) return fromBody;
+  if (fromBody) return isValidNotionThreadId(fromBody) ? fromBody : "";
   if (!headers) return "";
   for (const [k, v] of Object.entries(headers)) {
     if (k.toLowerCase() === "x-notion-thread-id" && typeof v === "string" && v.trim()) {
-      return v.trim();
+      const h = v.trim();
+      return isValidNotionThreadId(h) ? h : "";
     }
   }
   return "";
 }
+
+/**
+ * Short FNV-1a hash of a caller's Notion cookie, used to namespace the thread-session
+ * cache PER CALLER. Without this, two users of the SAME Notion space share one cache
+ * key (spaceId is space-, not user-scoped), so one user's thread could be served to
+ * another (cross-tenant IDOR, #7900 review). The raw cookie is never stored — only this
+ * non-reversible digest.
+ */
+export function hashNotionCallerCookie(cookie: string): string {
+  const raw = (cookie || "").trim();
+  if (!raw) return "anon";
+  let hash = 0x811c9dc5;
+  for (let i = 0; i < raw.length; i++) {
+    hash ^= raw.charCodeAt(i);
+    hash = Math.imul(hash, 0x01000193) >>> 0;
+  }
+  return hash.toString(16).padStart(8, "0");
+}

tests/unit/notion-thread-idor-7900.test.ts

@@ -0,0 +1,47 @@
+// Regression guard for the cross-tenant IDOR flagged in the #7900 security review:
+// the Notion thread-session cache was keyed only by spaceId (space-, not user-scoped)
+// and accepted an arbitrary client-supplied thread id, so one user of a shared Notion
+// space could pin/read another user's thread. Fix: validate the id is a real Notion
+// UUID, and namespace the cache per caller (hash of the caller's cookie).
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import {
+  isValidNotionThreadId,
+  readClientThreadId,
+  hashNotionCallerCookie,
+} from "../../open-sse/services/notionThreadSessions.ts";
+
+test("isValidNotionThreadId accepts real Notion UUIDs (dashed + undashed), rejects junk", () => {
+  assert.equal(isValidNotionThreadId("11111111-2222-3333-4444-555555555555"), true);
+  assert.equal(isValidNotionThreadId("11111111222233334444555555555555"), true);
+  assert.equal(isValidNotionThreadId("../../etc/passwd"), false);
+  assert.equal(isValidNotionThreadId("not-a-uuid"), false);
+  assert.equal(isValidNotionThreadId(""), false);
+  assert.equal(isValidNotionThreadId("11111111-2222-3333-4444-55555555555"), false); // too short
+});
+
+test("readClientThreadId rejects a non-UUID client-supplied thread id (body + header)", () => {
+  // A malformed id must NOT be accepted into the session cache.
+  assert.equal(readClientThreadId({ notion_thread_id: "attacker-pinned-value" }), "");
+  assert.equal(readClientThreadId({ thread_id: "'; DROP TABLE" }), "");
+  assert.equal(
+    readClientThreadId({}, { "x-notion-thread-id": "../../secret" }),
+    ""
+  );
+  // A well-formed id is still accepted.
+  const good = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee";
+  assert.equal(readClientThreadId({ notion_thread_id: good }), good);
+  assert.equal(readClientThreadId({}, { "x-notion-thread-id": good }), good);
+});
+
+test("hashNotionCallerCookie namespaces per caller — different cookies never collide", () => {
+  const a = hashNotionCallerCookie("token_v2=USER_A_SESSION; space=X");
+  const b = hashNotionCallerCookie("token_v2=USER_B_SESSION; space=X");
+  assert.notEqual(a, b, "two users of the same space must get different cache namespaces");
+  // Stable for the same caller.
+  assert.equal(a, hashNotionCallerCookie("token_v2=USER_A_SESSION; space=X"));
+  // Never stores the raw cookie.
+  assert.ok(!a.includes("USER_A_SESSION"));
+  assert.equal(hashNotionCallerCookie(""), "anon");
+});