Skip to content

fix: session-cookie requests partition the response cache without a bearer - #844

Merged
seonghobae merged 2 commits into
mainfrom
fix/session-cache-partition
Aug 25, 2026
Merged

fix: session-cookie requests partition the response cache without a bearer#844
seonghobae merged 2 commits into
mainfrom
fix/session-cache-partition

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Integration regression from landing #772 (distributed response cache) and #788 (opaque admin sessions) on the same main: _cache_partition required a bearer header, so every state-changing admin POST (/api/v1/evaluation_runs, /admin/simulate, agent-pool writes) from a cookie-authenticated operator failed with 401 before reaching handler logic.

Fix

  • An active opaque session id now derives the cache partition (prefixed admin-session:); session ids are random per login, so cross-session cache reuse stays impossible.
  • Bearer path unchanged; missing both still fails closed with 401.
  • Regression test: cookie-authenticated evaluation-run POST returns 201 across two distinct sessions.

Full local suite green (1807 tests).


Open in Devin Review

…earer

Merging #772 (distributed cache) with #788 (opaque admin sessions) left a
gap: _cache_partition required a bearer header, so every state-changing
admin POST from a cookie-authenticated operator failed with 401 before any
handler logic ran. An active opaque session id now derives the partition
(random per login, so cross-session reuse stays impossible), and a
regression test locks the flow in.
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 57 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4e572228-1053-4acb-b709-852e796fcfbd

📥 Commits

Reviewing files that changed from the base of the PR and between e22c826 and 3ca5063.

📒 Files selected for processing (2)
  • contextual_orchestrator/server.py
  • tests/test_security_hardening.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 08:58

@devin-ai-integration devin-ai-integration 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.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines 5977 to 5984
raw = self.headers.get("authorization", "")
token = raw.split(" ", 1)[1].strip() if raw.lower().startswith("bearer ") else ""
if not token: # pragma: no cover - _authorize rejects this first
if not token:
session_id = security._extract_admin_session_cookie(self.headers)
if session_id and security._admin_session_is_active(session_id):
return hashlib.sha256(f"admin-session:{session_id}".encode("utf-8")).hexdigest()
raise RequestError(401, "unauthorized", "bearer token is required")
return hashlib.sha256(token.encode("utf-8")).hexdigest()

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.

📝 Info: Bearer/session priority differs between authorize and partitioner

authorize checks an active admin session before the bearer header (server.py:281), but _cache_partition checks the bearer first and only falls back to the session. A request carrying both is authorized by session yet partitioned by bearer hash. Harmless today since the partition is only an isolation key, but the ordering diverges.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Merge-gate evidence (2026-08-25): Fixes a live main regression — merging #772 with #788 broke every state-changing admin POST for cookie-authenticated operators (401 from the cache partitioner before handler logic). Full local suite green (1807 tests) including the new two-session regression test. Hosted checks are queued behind an exhausted shared runner pool; merging on complete local evidence because the bug degrades production behavior on current main.

@seonghobae
seonghobae merged commit 84ec3e5 into main Aug 25, 2026
23 of 24 checks passed
@seonghobae
seonghobae deleted the fix/session-cache-partition branch August 25, 2026 09:00
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.

1 participant