feat(support): opaque per-conversation customer reconnect token (US-071)#34
Merged
Conversation
…hed storage, resume (US-071)
The anonymous support-widget customer's reconnect credential. Cryptographically
-random opaque bytes (NOT a Supabase JWT, signed with nothing - distinct from
US-068's bot JWT), returned in the clear exactly once at conversation creation
and stored only as a SHA-256 hash. Keeps the customer structurally off the
Supabase trust surface (ADR-0008, amends ADR-0004) with NO server-side
customer-identity table - continuity comes solely from the iframe-stored token.
- migration 20260623140000_conversation_tokens.sql: conversation_tokens
(token_hash pk, conversation_id fk, expires_at, created_at) with RLS on + ZERO
policies (deny-all to anon/authenticated); a service-role-only SECURITY DEFINER
resume_conversation(p_token_hash) RPC that resolves the hash to its bound
conversation iff not-expired AND status<>'resolved', slides the 24h window, and
returns the ONE bound conversation (no caller-supplied id, so X cannot read Y);
and an AFTER-UPDATE purge trigger that deletes a conversation's tokens on
resolve (literal invalidation). Revokes EXECUTE from public/anon/authenticated
by name - Supabase grants it directly via default privileges, so revoking PUBLIC
alone is not enough.
- backend/conversation_tokens.py: pure primitives (generate_conversation_token,
hash_conversation_token, 24h TTL).
- main.py: _issue_conversation_token (for US-078's first-message flow),
POST /widget/conversations/resume, GET /widget/conversations/{id}/transcript -
authed by the X-Conversation-Token header (never Authorization); transcript
enforces the token<->id binding so a token for X cannot read Y's transcript.
- test_us071_conversation_tokens.py: unit layer (always runs) + security
integration layer (DATABASE_URL-gated, skips cleanly) encoding the PRD
validation test.
Typecheck/lint clean (no new mypy errors vs HEAD; flake8 clean); migration applies
cleanly; all 13 US-071 assertions plus US-066/067/068 tests pass.
…anscript roles to user/assistant
…pt GET, extract service-role 503 helper
…REST in US-071 test
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Retrieval eval — PR vs
|
| Mode | recall@5 | MRR | nDCG@5 |
|---|---|---|---|
| vector | 0.860 (±0.000) | 0.772 (±0.000) | 0.779 (±0.000) |
| keyword | 0.110 (±0.000) | 0.120 (±0.000) | 0.112 (±0.000) |
| hybrid | 0.860 (±0.000) | 0.759 (±0.000) | 0.769 (±0.000) |
Per-category recall@5
| Mode | single_chunk | multi_hop | adversarial | paraphrase |
|---|---|---|---|---|
| vector | 0.900 (±0.000) | 0.933 (±0.000) | 0.600 (±0.000) | 1.000 (±0.000) |
| keyword | 0.250 (±0.000) | 0.033 (±0.000) | 0.000 (±0.000) | 0.000 (±0.000) |
| hybrid | 0.900 (±0.000) | 0.933 (±0.000) | 0.600 (±0.000) | 1.000 (±0.000) |
Comment is updated in place on each push by .github/workflows/retrieval-eval.yml (US-035). Comment-only — never blocks the build.
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.
Intent
The developer set out to implement US-071, an opaque per-conversation customer token for the Agentic_RAG support-widget surface, providing anonymous customers a reconnect credential that deliberately stays off the Supabase JWT trust surface. They required the token to be cryptographically-random opaque bytes (not signed with the JWT secret, distinct from US-068's bot JWT), returned in the clear exactly once at conversation creation and stored server-side only as a SHA-256 hash in a dedicated conversation_tokens(token_hash, conversation_id, expires_at, created_at) table, with no server-side customer-identity table. They specified a ~24h lifetime refreshed on activity while status != 'resolved', invalidation on resolve, each token bound to exactly one conversation, plus resume and transcript-GET endpoints that reject missing/expired/resolved tokens cleanly without creating rows. A security-critical automated test was mandated to prove a token for conversation X returns only X, cannot read conversation Y, and stops resuming once X is resolved, mirroring the existing asyncpg/DATABASE_URL-gated test style. Constraints included using a strictly-later migration timestamp, not merging the two existing conversation trust models, never putting workspace_membership.role in a visibility predicate, never logging/returning the raw token, scoping to issuance/storage/resume (leaving US-078's lazy creation flow out), and not wiping the shared local test DB while ensuring typecheck/lint pass and the migration applies cleanly.
What Changed
backend/conversation_tokens.pywith the token primitives:generate_conversation_token()(256-bitsecrets.token_urlsafe, opaque and signed with nothing — distinct from US-068's bot JWT) andhash_conversation_token()(SHA-256 hex, the only representation ever stored), plus the ~24h TTL constant.20260624160000_conversation_tokens.sql: aconversation_tokens(token_hash, conversation_id, expires_at, created_at)table with RLS enabled and zero policies (anon/authenticated denied wholesale), aresume_conversation(text, boolean)SECURITY DEFINER RPC granted toservice_roleonly that resolves a hash to its single bound conversation whileexpires_at > now() AND status <> 'resolved'(thep_slideflag gates the 24h activity refresh so a read-only GET never extends lifetime), and an AFTER-UPDATE trigger that purges a conversation's tokens on transition intoresolved.POST /widget/conversations/resumeandGET /widget/conversations/{id}/transcriptinbackend/main.py, both authed by anX-Conversation-Tokenheader hashed and resolved via the service-role RPC (binding-enforced, transcript limited touser/assistantroles, service-role-unset fail-closed to 503), with the_issue_conversation_tokenissuance helper for US-078, plus the security-criticalbackend/test_us071_conversation_tokens.pyand doc updates (AGENTS.md, README,.env.example).Risk Assessment
✅ Low: A well-bounded, additive feature (new migration, pure token primitives, two public widget endpoints) whose security-critical binding invariant holds; the two actionable prior-round findings are applied and verified, and the two remaining items are deliberate author-accepted decisions.
Testing
Ran the committed
backend/test_us071_conversation_tokens.py(unit + integration) against a running local Supabase with the US-071 migration already applied; both layers pass and prove the DB-level token boundary. Because unit/DB tests alone don't show the end-user surface, I additionally drove the real FastAPI widget endpoints over HTTP with tokens issued via the production backend helper, capturing a request→response transcript that demonstrates the customer reconnect flow: opaque token issued once and stored hashed, valid resume of X, transcript limited to user/assistant roles, a token for X rejected (401) when reaching for Y (with Ty→Y positive control), missing/invalid/resolved tokens cleanly 401 without creating rows, and resolve invalidating the token. All seed data and the transient harness were cleaned up; the working tree is clean.Evidence: US-071 widget endpoints end-to-end HTTP transcript (customer reconnect flow)
POST /widget/conversations/resume (missing)->401; (invalid)->401; (valid Tx)->200 {id:X, status:active} workspace_id hidden; token rows 2->2 (resume never creates a row) GET /widget/conversations/{X}/transcript (Tx)->200 messages roles=[user,assistant] (system/tool filtered) GET /widget/conversations/{Y}/transcript (Tx)->401 cross-conversation reject; positive control Ty->Y 200 Refresh: GET leaves expiry unchanged; POST slides 24h window forward Resolve(X): 0 token rows survive (purge trigger); POST resume Tx ->401; GET transcript Tx ->401 Issuance: raw token opaque (no dot segments), DB stores only sha256 hash != raw, raw not a key (0 rows)Evidence: US-071 committed security test output (DB RPC + trigger boundary)
unit: opaque/unique/URL-safe/not-JWT; hash deterministic 64-hex one-way != raw; empty rejected; TTL 24h schema: RLS on + 0 policies; resume_conversation EXECUTE = service_role only; purge trigger present step1 resume(Tx)->X only, resume(Ty)->Y control; step2 Tx X-bound, 'Tx requesting Y' rejected; step2b anon RPC denied, service-role->X refresh: POST slides expiry, GET (slide=False) does not; expiry: expired rejected & not resurrected step3: resolve(X) purges Tx and rejects resume(Tx) OK: 9 exact assertions, zero cross-conversation leakPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed ✅
supabase/migrations/20260624160000_conversation_tokens.sql:67- The migration's RPC signature changed across the branch's commits: e922fc7 createdresume_conversation(text), e61eb01 changed it toresume_conversation(text, boolean). Editing an already-applied migration file is a hazard: any environment that applied the earlier version - notably the shared local test DB the author was told not to wipe - keeps the stale single-arg function, and a plainsupabase migration upwill NOT reconcile it (it tracks applied filenames). On such a DB the mandated security test (select ... from public.resume_conversation($1, $2)) and the backend's 2-arg RPC call would fail with 'function does not exist'. Fresh CI/prod applies are correct; this only bites environments that ran the intermediate version. Recommend adb reset(or reapply) on the local DB before relying on the test.AGENTS.md:116- The verify hint referencespublic.resume_conversation(text), but the function is nowresume_conversation(text, boolean).has_function_privilege('anon','public.resume_conversation(text)','EXECUTE')will raise 'function does not exist' rather than return false. Update the documented signature to(text, boolean)(the test already uses the correct one).supabase/migrations/20260624160000_conversation_tokens.sql:108- The 24h window is encoded in two places that must stay in lockstep but can silently drift:CONVERSATION_TOKEN_TTL_SECONDS = 24*60*60(Python, used only for the initial issuanceexpires_at) and the hardcodedinterval '24 hours'in the RPC's slide-refresh UPDATE. Changing the constant would alter issuance TTL while leaving every slid token at 24h (and vice versa). Consider passing the interval into the RPC, or at least cross-referencing the two so the coupling is explicit.backend/test_us071_conversation_tokens.py:299- Step 2b issues httpx calls to PostgREST (port 54321) but only the asyncpg connect (port 54322) is guarded for clean-skip. If Postgres is up while PostgREST/Kong is down, thehttp.postraises an uncaughthttpx.ConnectErrorand the integration layer errors instead of skipping cleanly, slightly violating the stated skip contract. Low probability sincesupabase startbrings both up together.🔧 Fix: fix stale RPC signature doc and clean-skip PostgREST in US-071 test
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
python3 -m backend.test_us071_conversation_tokens— unit + integration/security layers green against local Supabase (9 integration assertions: RLS deny-all, service_role-only RPC, anon-denied via PostgREST, Tx→X only with Ty→Y control, slide/no-slide refresh, expiry rejection, resolve-purge invalidation)Confirmed the US-071 migration objects exist in the local DB:public.conversation_tokenstable,resume_conversation(text, boolean)RPC, andconversations_purge_tokens_on_resolvetriggerEnd-to-end through the real FastAPI endpoints viaTestClient(transientbackend/_e2e_us071_widget.py, since removed): issued tokens via the real_issue_conversation_tokenhelper; exercisedPOST /widget/conversations/resume(missing→401, invalid→401, valid Tx→200 resuming X with workspace_id hidden, no row created) andGET /widget/conversations/{id}/transcript(Tx reads X with system/tool roles filtered out, Tx→Y rejected 401 with Ty→Y positive control, resolved-token→401)Verified token issuance stores only the SHA-256 hash (raw token never persisted, not JWT-shaped) and that resolving X purges its token row, plus POST slides the 24h expiry while GET transcript does notVerified DB cleanup left no E2E/US-071 seed rows andgit statusis clean✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.