Skip to content

fix(notion-web): reuse threadId across OpenAI multi-turn (no new chat each request) - #7900

Merged
diegosouzapw merged 8 commits into
cheaper-inference:release/v3.8.49from
artickc:fix/notion-web-thread-continuity
Jul 22, 2026
Merged

diegosouzapw merged 8 commits into
cheaper-inference:release/v3.8.49from
artickc:fix/notion-web-thread-continuity

Conversation

@artickc

@artickc artickc commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Hardens Notion AI Web multi-turn continuity and reduces temporarily-unavailable / new-chat spam.

Root causes fixed

  1. New chat after errors — threadId was only stored after a successful parse. Failed temporarily-unavailable replies dropped the binding; the next attempt minted createThread:true again.
  2. Agentic/UREW hash mismatch — VibeProxy rewrites the last user turn; SkillsManager replays original text. Session keys now normalize task text + sticky root key on first user message.
  3. Oversized follow-ups — re-sending the full tool-loop transcript every turn overloaded Notion. Follow-ups now use isPartialTranscript:true and only steps after the last assistant.
  4. In-band errors — HTTP 200 NDJSON {type:error, subType:temporarily-unavailable} is detected, returned as 503, and retried once with the same threadId (createThread:false).
  5. Process restart — sticky map is persisted under DATA_DIR/notion-web-thread-sessions.json.
  6. Custom agents — workflowId / context_page_id from credentials; browser-aligned payload (threadParentPointer.table=workflow, etc.).
  7. clientHeaders — X-Notion-Thread-Id read from ExecuteInput.clientHeaders (not nonexistent headers).

Test plan

  • node --import tsx --test tests/unit/executor-notion-web.test.ts → 42/42
  • Unit: sticky root survives failed first request (no second createThread)
  • Unit: UREW-rewritten turn1 matches original turn2 history
  • Unit: temporarily-unavailable parsed as retryable
  • Unit: multi-turn createThread true then false with same threadId
  • Live: SkillsManager multi-turn + tool loop keeps one Notion chat
  • Live: force a transient error → retry stays on same thread

Base

release/v3.8.49

… each request)

Root cause: every execute() minted a random threadId with createThread:true,
so each OpenAI messages[] turn became a brand-new Notion AI chat. That broke
multi-turn agent flows (tool result follow-ups looked like cold starts).

- History-keyed in-memory session cache (spaceId + conversation prefix hash)
- First user turn: createThread true + new UUID
- Follow-up with prior turns: createThread false + same threadId
- Optional client continuity: body.notion_thread_id / X-Notion-Thread-Id
- Echo thread id on chat.completion (notion_thread_id + response header)
- Also accept OpenAI content-parts arrays for message content
- Unit tests: 34/34 (session lookup/store + createThread false on turn 2)
@artickc
artickc requested a review from diegosouzapw as a code owner July 20, 2026 19:39

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces thread session continuity for Notion AI chats, mapping multi-turn OpenAI-style messages to a single Notion chat via an in-memory cache and client-supplied thread IDs, and adds corresponding unit tests. The reviewer identified a critical issue where input.headers is accessed instead of input.clientHeaders, which would result in a TypeScript compilation error or runtime failure when reading client-supplied headers.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread open-sse/executors/notion-web.ts Outdated
// Thread continuity: prefer explicit client id, else history-keyed session cache.
// First user turn → createThread:true + new UUID. Follow-ups with prior turns in
// messages[] → createThread:false + same threadId (one Notion AI chat).
const clientThreadId = readClientThreadId(requestBody, input.headers as Record<string, string> | undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The ExecuteInput interface defined in open-sse/executors/base.ts does not have a headers property. Instead, it defines clientHeaders for the original client request headers. Accessing input.headers will result in a TypeScript compilation error or evaluate to undefined at runtime, which prevents the client-supplied X-Notion-Thread-Id header from being read correctly.

Suggested change
const clientThreadId = readClientThreadId(requestBody, input.headers as Record<string, string> | undefined);
const clientThreadId = readClientThreadId(requestBody, input.clientHeaders as Record<string, string> | undefined);

Artur and others added 7 commits July 20, 2026 23:18
ExecuteInput exposes client request headers as clientHeaders, not headers.
input.headers was always undefined so client-supplied thread pins were ignored.
- Bind conversation root (first user) to a threadId *before* upstream call so
  temporarily-unavailable / empty replies never mint a new Notion chat on retry
- Persist sticky map under DATA_DIR so multi-turn survives process restarts
- Follow-ups use createThread:false, isPartialTranscript:true, and only the
  steps after the last assistant (full re-transcript was overloading Notion)
- Detect in-band Notion error objects (subType temporarily-unavailable) and
  retry once with the same threadId
- Keep custom-agent workflowId support and clientHeaders thread pin
Merge release/v3.8.49 into fix/notion-web-thread-continuity: the sticky
thread-session cache (root binding, disk persistence, client pin via
notion_thread_id/thread_id/X-Notion-Thread-Id) collided with cheaper-inference#7896's
identical extractNotionMessageText/buildNotionMessageStep dedupe already
landed on release — kept the PR's exported extractNotionMessageText plus
release's content-parts docstring, and release's buildNotionMessageStep
comment. Removed the dead @deprecated buildNotionCreateThreadRequestBody
alias (zero call sites, confirmed by grep) flagged in review.

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>
…uilder into services

The merged notion-web.ts (1490 lines) and its test file (1000 lines) tripped
the file-size gate (cap 800 for new/uncapped files). Extract three
self-contained pieces into open-sse/services/, no behavior change:

- notionThreadSessions.ts: sticky thread-session cache, disk persistence,
  conversation hashing, client thread-id pin (body/header)
- notionStreamParser.ts: NDJSON runInferenceTranscript response parsing +
  in-band upstream error detection
- notionTranscriptBuilder.ts: config/context/message-step transcript building

Split the corresponding "Notion thread session continuity" describe block
into tests/unit/executor-notion-web-thread-sessions.test.ts. All symbols
previously reachable via the notion-web.ts namespace import stay reachable
(re-exported) so existing test destructuring is unaffected. 44/44 tests pass.

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit dc41a73 into cheaper-inference:release/v3.8.49 Jul 22, 2026
3 of 5 checks passed
diegosouzapw added a commit that referenced this pull request Jul 22, 2026
…ient 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>
diegosouzapw added a commit that referenced this pull request Jul 24, 2026
…ated usage-analytics file

Root cause A (#8159): sendNotionInferenceRequest() in
open-sse/executors/notion-web.ts was migrated from fetch() to
tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native
tls-client-node binary) to get past Notion's Cloudflare TLS
fingerprinting. #8159 updated the mock in the sibling
tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired
through __setTlsFetchOverrideForTesting) but never touched
tests/unit/executor-notion-web-thread-sessions.test.ts (split out
earlier by #7900) — its 3 execute()-driven tests still mocked
globalThis.fetch, which tlsFetchNotion() never calls once the native
TLS client loads successfully. Confirmed live: all 3 tests hit real
https://app.notion.com with a fake cookie and got a real 401
(~8.1-8.5s each here; on a network with blocked/slow egress this
would instead hang up to the client's ~190s timeout+grace per test —
a CI-hang risk).

Fix: replicate installNotionTlsMock verbatim from the sibling file
into executor-notion-web-thread-sessions.test.ts so the 3 tests mock
the TLS override point instead of global fetch. Suite is now fully
hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s.

Root cause B (#7700): tests/unit/usage-analytics-route-extra.test.ts
was created as a byte-for-byte duplicate of 10 of the 22 tests in
tests/unit/usage-analytics-route.test.ts. #7300 later fixed a fixture
bug in the retention-window boundary test ("does not double-count raw
and aggregated rows") in the main file only — reading
getUserDatabaseSettings().retention.usageHistory live instead of a
hardcoded 30-day cutoff (default retention is 365 days) — leaving the
duplicate copy on the stale hardcoded value, which now fails (1 !== 2).

Fix: delete the duplicate file. All 10 of its test names exist
verbatim in the main file (verified with comm -12) and that file
passes 22/22:
  - does not double-count raw and aggregated rows
  - does not persist guessed API key attribution
  - does not throw Unknown named parameter on short range (needsAggregated=false)
  - does not throw Unknown named parameter with apiKey filter on long range
  - groups renamed API key usage by stable ID
  - includes activityMap for heatmap
  - includes cost by API key
  - omits global aggregates when filtering by API key
  - returns 500 on database errors
  - returns weeklyPattern for the costs dashboard
No coverage loss — same production code, same assertions, one fewer
redundant file.

Validation:
  - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail
    (401 !== 200, real network hit), 31.0s
  - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail
    (1 !== 2), 18.7s
  - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s,
    hermetic (no network)
  - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37
    pass, byte-identical diff
  - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass,
    byte-identical diff
  - npx eslint on the changed file: clean
  - npm run typecheck:core: clean (exit 0)

Refs #8159
Refs #7300
Refs #7700
diegosouzapw added a commit that referenced this pull request Jul 24, 2026
…alytics suite (#8392)

* test: hermetic TLS mock for notion thread-session suite + drop duplicated usage-analytics file

Root cause A (#8159): sendNotionInferenceRequest() in
open-sse/executors/notion-web.ts was migrated from fetch() to
tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native
tls-client-node binary) to get past Notion's Cloudflare TLS
fingerprinting. #8159 updated the mock in the sibling
tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired
through __setTlsFetchOverrideForTesting) but never touched
tests/unit/executor-notion-web-thread-sessions.test.ts (split out
earlier by #7900) — its 3 execute()-driven tests still mocked
globalThis.fetch, which tlsFetchNotion() never calls once the native
TLS client loads successfully. Confirmed live: all 3 tests hit real
https://app.notion.com with a fake cookie and got a real 401
(~8.1-8.5s each here; on a network with blocked/slow egress this
would instead hang up to the client's ~190s timeout+grace per test —
a CI-hang risk).

Fix: replicate installNotionTlsMock verbatim from the sibling file
into executor-notion-web-thread-sessions.test.ts so the 3 tests mock
the TLS override point instead of global fetch. Suite is now fully
hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s.

Root cause B (#7700): tests/unit/usage-analytics-route-extra.test.ts
was created as a byte-for-byte duplicate of 10 of the 22 tests in
tests/unit/usage-analytics-route.test.ts. #7300 later fixed a fixture
bug in the retention-window boundary test ("does not double-count raw
and aggregated rows") in the main file only — reading
getUserDatabaseSettings().retention.usageHistory live instead of a
hardcoded 30-day cutoff (default retention is 365 days) — leaving the
duplicate copy on the stale hardcoded value, which now fails (1 !== 2).

Fix: delete the duplicate file. All 10 of its test names exist
verbatim in the main file (verified with comm -12) and that file
passes 22/22:
  - does not double-count raw and aggregated rows
  - does not persist guessed API key attribution
  - does not throw Unknown named parameter on short range (needsAggregated=false)
  - does not throw Unknown named parameter with apiKey filter on long range
  - groups renamed API key usage by stable ID
  - includes activityMap for heatmap
  - includes cost by API key
  - omits global aggregates when filtering by API key
  - returns 500 on database errors
  - returns weeklyPattern for the costs dashboard
No coverage loss — same production code, same assertions, one fewer
redundant file.

Validation:
  - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail
    (401 !== 200, real network hit), 31.0s
  - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail
    (1 !== 2), 18.7s
  - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s,
    hermetic (no network)
  - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37
    pass, byte-identical diff
  - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass,
    byte-identical diff
  - npx eslint on the changed file: clean
  - npm run typecheck:core: clean (exit 0)

Refs #8159
Refs #7300
Refs #7700

* chore(quality): register usage-analytics-route-extra deletion in test-masking allowlist

check:test-masking hard-flags any deleted test file without a
_deletedWithReplacement entry. The deletion is legitimate (100% duplicate
suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts)
-- same registration pattern as the video-dashscope entry.

Refs #7700
Refs #7300
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
… each request) (cheaper-inference#7900)

* fix(notion-web): reuse threadId across OpenAI multi-turn (no new chat each request)

Root cause: every execute() minted a random threadId with createThread:true,
so each OpenAI messages[] turn became a brand-new Notion AI chat. That broke
multi-turn agent flows (tool result follow-ups looked like cold starts).

- History-keyed in-memory session cache (spaceId + conversation prefix hash)
- First user turn: createThread true + new UUID
- Follow-up with prior turns: createThread false + same threadId
- Optional client continuity: body.notion_thread_id / X-Notion-Thread-Id
- Echo thread id on chat.completion (notion_thread_id + response header)
- Also accept OpenAI content-parts arrays for message content
- Unit tests: 34/34 (session lookup/store + createThread false on turn 2)

* fix(notion-web): read X-Notion-Thread-Id from clientHeaders

ExecuteInput exposes client request headers as clientHeaders, not headers.
input.headers was always undefined so client-supplied thread pins were ignored.

* fix(notion-web): prefer clientHeaders with defensive headers fallback

* fix(notion-web): sticky threads on errors + partial follow-ups

- Bind conversation root (first user) to a threadId *before* upstream call so
  temporarily-unavailable / empty replies never mint a new Notion chat on retry
- Persist sticky map under DATA_DIR so multi-turn survives process restarts
- Follow-ups use createThread:false, isPartialTranscript:true, and only the
  steps after the last assistant (full re-transcript was overloading Notion)
- Detect in-band Notion error objects (subType temporarily-unavailable) and
  retry once with the same threadId
- Keep custom-agent workflowId support and clientHeaders thread pin

* refactor(notion-web): split thread-session/stream-parser/transcript-builder into services

The merged notion-web.ts (1490 lines) and its test file (1000 lines) tripped
the file-size gate (cap 800 for new/uncapped files). Extract three
self-contained pieces into open-sse/services/, no behavior change:

- notionThreadSessions.ts: sticky thread-session cache, disk persistence,
  conversation hashing, client thread-id pin (body/header)
- notionStreamParser.ts: NDJSON runInferenceTranscript response parsing +
  in-band upstream error detection
- notionTranscriptBuilder.ts: config/context/message-step transcript building

Split the corresponding "Notion thread session continuity" describe block
into tests/unit/executor-notion-web-thread-sessions.test.ts. All symbols
previously reachable via the notion-web.ts namespace import stay reachable
(re-exported) so existing test destructuring is unaffected. 44/44 tests pass.

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: Artur <artur@local>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…ient thread ids (IDOR)

Security-review follow-up to cheaper-inference#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>
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…alytics suite (cheaper-inference#8392)

* test: hermetic TLS mock for notion thread-session suite + drop duplicated usage-analytics file

Root cause A (cheaper-inference#8159): sendNotionInferenceRequest() in
open-sse/executors/notion-web.ts was migrated from fetch() to
tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native
tls-client-node binary) to get past Notion's Cloudflare TLS
fingerprinting. cheaper-inference#8159 updated the mock in the sibling
tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired
through __setTlsFetchOverrideForTesting) but never touched
tests/unit/executor-notion-web-thread-sessions.test.ts (split out
earlier by cheaper-inference#7900) — its 3 execute()-driven tests still mocked
globalThis.fetch, which tlsFetchNotion() never calls once the native
TLS client loads successfully. Confirmed live: all 3 tests hit real
https://app.notion.com with a fake cookie and got a real 401
(~8.1-8.5s each here; on a network with blocked/slow egress this
would instead hang up to the client's ~190s timeout+grace per test —
a CI-hang risk).

Fix: replicate installNotionTlsMock verbatim from the sibling file
into executor-notion-web-thread-sessions.test.ts so the 3 tests mock
the TLS override point instead of global fetch. Suite is now fully
hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s.

Root cause B (cheaper-inference#7700): tests/unit/usage-analytics-route-extra.test.ts
was created as a byte-for-byte duplicate of 10 of the 22 tests in
tests/unit/usage-analytics-route.test.ts. cheaper-inference#7300 later fixed a fixture
bug in the retention-window boundary test ("does not double-count raw
and aggregated rows") in the main file only — reading
getUserDatabaseSettings().retention.usageHistory live instead of a
hardcoded 30-day cutoff (default retention is 365 days) — leaving the
duplicate copy on the stale hardcoded value, which now fails (1 !== 2).

Fix: delete the duplicate file. All 10 of its test names exist
verbatim in the main file (verified with comm -12) and that file
passes 22/22:
  - does not double-count raw and aggregated rows
  - does not persist guessed API key attribution
  - does not throw Unknown named parameter on short range (needsAggregated=false)
  - does not throw Unknown named parameter with apiKey filter on long range
  - groups renamed API key usage by stable ID
  - includes activityMap for heatmap
  - includes cost by API key
  - omits global aggregates when filtering by API key
  - returns 500 on database errors
  - returns weeklyPattern for the costs dashboard
No coverage loss — same production code, same assertions, one fewer
redundant file.

Validation:
  - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail
    (401 !== 200, real network hit), 31.0s
  - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail
    (1 !== 2), 18.7s
  - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s,
    hermetic (no network)
  - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37
    pass, byte-identical diff
  - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass,
    byte-identical diff
  - npx eslint on the changed file: clean
  - npm run typecheck:core: clean (exit 0)

Refs cheaper-inference#8159
Refs cheaper-inference#7300
Refs cheaper-inference#7700

* chore(quality): register usage-analytics-route-extra deletion in test-masking allowlist

check:test-masking hard-flags any deleted test file without a
_deletedWithReplacement entry. The deletion is legitimate (100% duplicate
suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts)
-- same registration pattern as the video-dashscope entry.

Refs cheaper-inference#7700
Refs cheaper-inference#7300
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… each request) (cheaper-inference#7900)

* fix(notion-web): reuse threadId across OpenAI multi-turn (no new chat each request)

Root cause: every execute() minted a random threadId with createThread:true,
so each OpenAI messages[] turn became a brand-new Notion AI chat. That broke
multi-turn agent flows (tool result follow-ups looked like cold starts).

- History-keyed in-memory session cache (spaceId + conversation prefix hash)
- First user turn: createThread true + new UUID
- Follow-up with prior turns: createThread false + same threadId
- Optional client continuity: body.notion_thread_id / X-Notion-Thread-Id
- Echo thread id on chat.completion (notion_thread_id + response header)
- Also accept OpenAI content-parts arrays for message content
- Unit tests: 34/34 (session lookup/store + createThread false on turn 2)

* fix(notion-web): read X-Notion-Thread-Id from clientHeaders

ExecuteInput exposes client request headers as clientHeaders, not headers.
input.headers was always undefined so client-supplied thread pins were ignored.

* fix(notion-web): prefer clientHeaders with defensive headers fallback

* fix(notion-web): sticky threads on errors + partial follow-ups

- Bind conversation root (first user) to a threadId *before* upstream call so
  temporarily-unavailable / empty replies never mint a new Notion chat on retry
- Persist sticky map under DATA_DIR so multi-turn survives process restarts
- Follow-ups use createThread:false, isPartialTranscript:true, and only the
  steps after the last assistant (full re-transcript was overloading Notion)
- Detect in-band Notion error objects (subType temporarily-unavailable) and
  retry once with the same threadId
- Keep custom-agent workflowId support and clientHeaders thread pin

* refactor(notion-web): split thread-session/stream-parser/transcript-builder into services

The merged notion-web.ts (1490 lines) and its test file (1000 lines) tripped
the file-size gate (cap 800 for new/uncapped files). Extract three
self-contained pieces into open-sse/services/, no behavior change:

- notionThreadSessions.ts: sticky thread-session cache, disk persistence,
  conversation hashing, client thread-id pin (body/header)
- notionStreamParser.ts: NDJSON runInferenceTranscript response parsing +
  in-band upstream error detection
- notionTranscriptBuilder.ts: config/context/message-step transcript building

Split the corresponding "Notion thread session continuity" describe block
into tests/unit/executor-notion-web-thread-sessions.test.ts. All symbols
previously reachable via the notion-web.ts namespace import stay reachable
(re-exported) so existing test destructuring is unaffected. 44/44 tests pass.

Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: Artur <artur@local>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ient thread ids (IDOR)

Security-review follow-up to cheaper-inference#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>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…alytics suite (cheaper-inference#8392)

* test: hermetic TLS mock for notion thread-session suite + drop duplicated usage-analytics file

Root cause A (cheaper-inference#8159): sendNotionInferenceRequest() in
open-sse/executors/notion-web.ts was migrated from fetch() to
tlsFetchNotion() (open-sse/services/notionTlsClient.ts, native
tls-client-node binary) to get past Notion's Cloudflare TLS
fingerprinting. cheaper-inference#8159 updated the mock in the sibling
tests/unit/executor-notion-web.test.ts (installNotionTlsMock, wired
through __setTlsFetchOverrideForTesting) but never touched
tests/unit/executor-notion-web-thread-sessions.test.ts (split out
earlier by cheaper-inference#7900) — its 3 execute()-driven tests still mocked
globalThis.fetch, which tlsFetchNotion() never calls once the native
TLS client loads successfully. Confirmed live: all 3 tests hit real
https://app.notion.com with a fake cookie and got a real 401
(~8.1-8.5s each here; on a network with blocked/slow egress this
would instead hang up to the client's ~190s timeout+grace per test —
a CI-hang risk).

Fix: replicate installNotionTlsMock verbatim from the sibling file
into executor-notion-web-thread-sessions.test.ts so the 3 tests mock
the TLS override point instead of global fetch. Suite is now fully
hermetic — 8/8 pass, no network I/O, total runtime 31.0s -> 8.0s.

Root cause B (cheaper-inference#7700): tests/unit/usage-analytics-route-extra.test.ts
was created as a byte-for-byte duplicate of 10 of the 22 tests in
tests/unit/usage-analytics-route.test.ts. cheaper-inference#7300 later fixed a fixture
bug in the retention-window boundary test ("does not double-count raw
and aggregated rows") in the main file only — reading
getUserDatabaseSettings().retention.usageHistory live instead of a
hardcoded 30-day cutoff (default retention is 365 days) — leaving the
duplicate copy on the stale hardcoded value, which now fails (1 !== 2).

Fix: delete the duplicate file. All 10 of its test names exist
verbatim in the main file (verified with comm -12) and that file
passes 22/22:
  - does not double-count raw and aggregated rows
  - does not persist guessed API key attribution
  - does not throw Unknown named parameter on short range (needsAggregated=false)
  - does not throw Unknown named parameter with apiKey filter on long range
  - groups renamed API key usage by stable ID
  - includes activityMap for heatmap
  - includes cost by API key
  - omits global aggregates when filtering by API key
  - returns 500 on database errors
  - returns weeklyPattern for the costs dashboard
No coverage loss — same production code, same assertions, one fewer
redundant file.

Validation:
  - RED executor-notion-web-thread-sessions.test.ts: 5 pass / 3 fail
    (401 !== 200, real network hit), 31.0s
  - RED usage-analytics-route-extra.test.ts: 9 pass / 1 fail
    (1 !== 2), 18.7s
  - GREEN executor-notion-web-thread-sessions.test.ts: 8/8 pass, 8.0s,
    hermetic (no network)
  - GREEN executor-notion-web.test.ts (sibling, untouched): 37/37
    pass, byte-identical diff
  - GREEN usage-analytics-route.test.ts (untouched): 22/22 pass,
    byte-identical diff
  - npx eslint on the changed file: clean
  - npm run typecheck:core: clean (exit 0)

Refs cheaper-inference#8159
Refs cheaper-inference#7300
Refs cheaper-inference#7700

* chore(quality): register usage-analytics-route-extra deletion in test-masking allowlist

check:test-masking hard-flags any deleted test file without a
_deletedWithReplacement entry. The deletion is legitimate (100% duplicate
suite, coverage retained verbatim in tests/unit/usage-analytics-route.test.ts)
-- same registration pattern as the video-dashscope entry.

Refs cheaper-inference#7700
Refs cheaper-inference#7300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants