Harden chat thread_id against squatting and schema-boundary abuse - #1517
Open
daiv-agent[bot] wants to merge 1 commit into
Open
Harden chat thread_id against squatting and schema-boundary abuse#1517daiv-agent[bot] wants to merge 1 commit into
daiv-agent[bot] wants to merge 1 commit into
Conversation
…rottle the SSE stream DAIV-Session: https://daivagent.com/dashboard/sessions/db6d0b29-ffe0-4113-87e6-53a127f3d2bc/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the chat conversation thread-id derivation and validation against two attack/robustness vectors: a caller replaying a deterministic webhook thread id to squat another repo's conversation, and oversized thread/run ids reaching the DB as DataError-500s. Webhook thread ids are now HMAC-SHA256 keyed by a server secret (not the unsalted MD5 they replaced), the chat API rejects digest-shaped and over-long ids at the schema boundary, and the SSE stream endpoint gains a per-user rate throttle plus a bounded async Redis pool.
Key Changes:
codebase.utils.compute_thread_idnow derives webhook thread ids viacore.utils.compute_keyed_uuid(HMAC-SHA256 keyed by a server secret) instead of the unsaltedgenerate_uuidMD5, so the id is deterministic per(repo_slug, scope, entity_iid)but not computable from those public inputs alone.core.encryption.get_thread_id_signing_key(HKDF-SHA256-derived 32-byte key fromDAIV_ENCRYPTION_KEY/DJANGO_SECRET_KEY) andcore.utils.compute_keyed_uuid;slash_commands/actions/clear.pynow usescompute_thread_idfor its thread-id derivation._validate_thread_or_run_idindaiv/chat/api/views.py, applied tocreate_chat_completion,cancel_chat_run, andstream_run_events: rejects empty values, values over 64 chars (400 instead of DataError-500 on theCharField(64)PKs), and deterministic-digest-shaped ids (32/64-char pure hex) to block thread_id squatting.ChatStreamRateThrottle(30/min per authenticated user) onGET /chat/streamso an unbounded open rate cannot exhaust worker threads or the async Redis pool; newtests/unit_tests/core/test_throttling.pycovers the rate, per-user budget, and 429 path.ASYNC_MAX_CONNECTIONS = 50incore/redis.py(mirrors the cache pool cap, stays aboveDB_POOL_MAX_SIZE= 15).SessionAdoptionErrorinsessions/services.py:aget_or_create_sessionnow refuses to graft a webhook-origin run onto a pre-existing non-webhook session on the same thread id (a possible squat), while still adopting existing webhook-origin sessions.💡 Instructions for the reviewer: