Align authorization and session lifecycle behavior

[JSv4]

Summary

Align API and browser behavior with the shared authorization and session lifecycle contracts. Apply consistent resource checks and cleanup across related API operations, long-lived sessions, and client state, with regression coverage for normal access and state transitions.

Changes

• Use shared visibility rules for related resources and conversation access.
• Preserve ownership and scope consistently across updates and upload workflows.
• Keep browser caches and active sessions aligned with authentication state.
• Bound repeated authorization work during streaming, with regression coverage for multiple viewers and session transitions.
• Refine query validation, diagnostic logging, and local asset handling.
• Add component-level regression tests and architecture assertions.

The GraphQL schema is unchanged and no database migration is required. Document caches are scoped to the current page/session; older incomplete worker uploads without a recorded corpus target may need to be restarted.

Test plan

• Full backend CI suite — 11,460 passed, 26 skipped, and 611 subtests passed; required backend gate passed.
• WebSocket session, consumer, notification, and expiry suites after the streaming follow-up — 128 passed.
• yarn test:coverage:unit — 2,625 passed across 172 files.
• Targeted GraphQL and annotation/note tree suites — 71 passed and 6 subtests passed.
• Frontend component tests, production build, and Docker build — passed in CI.
• Login/navigation, WebSocket auth, and extract-pipeline end-to-end workflows — passed on the latest commit.
• yarn tsc --noEmit — passed.
• yarn lint and yarn any:check:strict — passed.
• All three Codecov patch checks (overall, backend, frontend) — passed.
• pre-commit run --files <changed files> — passed, including the full backend mypy check.

Checklist

• Affected and new tests pass locally
• Pre-commit checks pass for changed files
• TypeScript compiles cleanly
• Changelog fragment added
• No new dependency introduced

[claude]

Review

Large, focused security/session-consistency PR. Overall this is high-quality work — the ownership-takeover fix in _drf_mutation_body, the independent-visibility FK resolvers, the chunked-upload token-swap fix, the WASM dev-server path traversal fix, and the frontend document-cache session isolation are all real bugs fixed with solid, targeted regression tests (test_graphql_resource_contracts.py, test_websocket_session_contracts.py, docxodusWasm.test.ts, documentCacheSession.test.ts). A few things worth a look before/after merge.

Potential bug / performance concern (the one I'd most want addressed)

config/websocket/auth_handshake.py AuthHandshakeMixin.dispatch() now runs ensure_authorized() — which does a fresh JWT→user DB lookup (_get_user_from_token) plus a full resource-visibility recheck (_validate_resource_permissions) — on every non-connect/disconnect/AUTH message the consumer processes, with no throttling.

For ThreadUpdatesConsumer this includes agent_stream_token (config/websocket/consumers/thread_updates.py:218), which is fired once per streamed LLM token via group_send to every socket watching that conversation. _validate_resource_permissions → _check_conversation_access → ConversationService.get_or_none(...) is a real DB query, and it's called without request= so the per-request visibility cache mentioned in CLAUDE.md doesn't help across these calls. So a single streaming response fans out to O(tokens × watchers) DB queries just for the reauth check, on top of the actual message delivery.

The codebase already has precedent for worrying about exactly this class of cost — _MIN_AUTH_FRAME_INTERVAL_SEC was added specifically to stop AUTH-frame spam from "burn[ing] DB queries" (per the PR #1502 comment right above it), but that throttle only covers client-sent AUTH frames, not this new per-dispatch recheck. Given the stated design goal ("the next event... closes... PERMISSION_REVOKED"), instant revocation is clearly intentional, but doing the full check per token rather than e.g. coalescing to something like the existing 1s AUTH-frame floor (or piggybacking on the 30s watchdog cadence with an in-memory "last verified at" timestamp) seems like it could turn typical multi-viewer streaming usage into a DB hotspot. Worth a load-test or at least a deliberate call on whether the cost is acceptable — UnifiedAgentConsumer's own streaming path avoids this by calling self.send() directly instead of routing through dispatch()/group_send, so it isn't universally paid, which suggests it may not have been sized for the ThreadUpdatesConsumer fan-out case specifically.

Code quality / maintainability

• config/graphql/core/mutations.py: the new "don't let a writable parent attach someone else's private resource" check is scoped to a hardcoded {"label_set", "annotation_label"} set inside the generic _drf_mutation_body. It's correct today (verified against the only three pk_fields tuples in the codebase — annotation_label, label_set, categories, with categories deliberately excluded as public vocabulary), but it's an easy-to-forget allowlist: a future pk_fields=(...) addition referencing another privately-scoped model would silently bypass this check unless someone remembers to add it here too. Might be worth a comment pointing future authors at this list, or (longer term) inferring "private-ish" via the target model rather than by field name.

Security (mostly positive)

• The ownership-preservation fix (is_update → pop creator/creator_id before the serializer save) closes a real privilege-escalation bug: an UPDATE grantee could previously become the object's owner (and thus gain DELETE/PERMISSION rights) just by editing a shared object. Good catch, and test_corpus_update_preserves_owner/test_labelset_update_preserves_owner verify it directly.
• TreeTraversalService.get_nodes correctly stops tree traversal at the first invisible ancestor/descendant rather than trusting the root's visibility to imply the whole subtree is visible — good fix for created_by_analysis/created_by_extract private annotations leaking through descendantsTree/fullTree/subtree.
• GremlinEngineType_WRITE.api_key now gated on UPDATE (management) permission rather than being exposed to anyone who can READ (and thus publish) the analyzer — correct tightening, matches test_engine_credentials_require_management_access.
• Removing token/password prefixes from debug logs (config/graphql_api_token_auth/backends.py, config/graphql_auth0_auth/*, config/jwt_utils.py) is a solid hardening pass, and test_authentication_logging.py gives it real regression coverage rather than just trusting the diff.
• frontend/tooling/docxodusWasm.ts: good defense-in-depth — containment check both before and after realpathSync to catch symlink escapes, decode-then-validate ordering to stop %2e%2e%2f traversal, and dropping the blanket Access-Control-Allow-Origin: * that the old inline plugin had. Nice test coverage of the traversal/symlink/query-string cases.
• Frontend document cache: documentCacheState.ts's generation counter + assertDocumentCacheGeneration guards against a real prior gap — in-flight IndexedDB reads/writes and network responses completing after logout could previously write/read another user's cache namespace. The synchronous generation bump in clearCache() (before the async IDB work) means isolation doesn't depend on IDB delete actually succeeding, which is the right failure mode.

Minor

• TreeTraversalService.get_nodes's ancestor walk-up (subtree/full modes) still does one query per ancestor level, and each query now goes through the full filter_visible permission-filtered queryset rather than a plain PK lookup — likely fine for normal annotation nesting depths, but worth keeping an eye on for deeply nested trees.
• opencontractserver/document_imports/services.py: the _get_owned_session corpus-binding check only recognizes DOCUMENT/ZIP_TO_CORPUS kinds in target_fields; this is safe today only because start_chunked_upload already rejects any other kind when access_token is not None — the two are coupled but in different functions, so a future kind addition needs to update both spots consistently.

Test coverage

Test coverage is a real strength of this PR — test_graphql_resource_contracts.py and test_websocket_session_contracts.py are thorough, use realistic private-vs-public fixtures, and assert both the "blocked" and "allowed" sides of each check (not just the negative case). One gap: no test exercises DB query volume/count for the new dispatch()-triggered reauthorization path under a simulated multi-token stream with multiple watchers — given the concern above, an assertNumQueries-style test on agent_stream_token fan-out would make the actual cost visible and catch future regressions either direction.

[claude]

Review: Align authorization and session lifecycle behavior (#2329)

This is a large, security-focused PR and it's executed carefully. I read through the full diff (GraphQL types/mutations, WebSocket auth handshake, chunked-upload worker-token handling, and the frontend document-cache/session isolation work) plus the new/updated test suites. Overall this looks correct, well-tested, and consistent with the repo's permissioning conventions (`BaseService`, `visible_to_user`, `resolve_visible_fk`). Nice work tightening a lot of real IDOR-shaped gaps.

Highlights

• `resolve_visible_fk` conversion for nullable FKs (`annotation_types.py`, `extract_types.py`, `document_types.py`, `corpus_types.py`, `conversation_types.py`, `research_types.py`, `agent_types.py`): converts a long list of previously-plain `strawberry.field` FK attributes (e.g. `Annotation.parent`, `Annotation.analysis`/`created_by_analysis`/`created_by_extract`, `Analyzer.hostGremlin`, `Corpus.labelSet`, `Note.parent`/`corpus`/`annotation`, etc.) to go through per-target visibility checks instead of default `getattr` resolution. This closes a real class of leaks where an intrinsically-private related object (private analysis/extract, private label set, a Gremlin engine with credentials) was reachable through a public parent. The new architecture tests (`test_nullable_resource_links_do_not_use_default_fk_resolution`, `test_independent_fk_targets_have_permission_hooks`) are a good mechanical guardrail against regressions here — that's exactly the kind of enforcement this codebase already leans on (E001 check, schema parity test) and it should keep this class of bug from reappearing.
• `GremlinEngine.apiKey` gated behind UPDATE, not READ (`extract_types.py`): correctly distinguishes "can see this engine" from "can see its credentials." `test_engine_credentials_require_management_access` verifies both branches.
• `_drf_mutation_body` ownership fix (`config/graphql/core/mutations.py`): stripping `creator`/`creator_id` from update kwargs so an UPDATE-permission grantee can never flip into the owner, plus gating `label_set`/`annotation_label` FK attachment on READ of the target. `test_corpus_update_preserves_owner` / `test_labelset_update_preserves_owner` / `test_labelset_assignment_requires_read` cover this well. The scoping of the "writable parent isn't authority over the linked object" check to just `{"label_set", "annotation_label"}` (the only `pk_fields` currently routed through `drf_mutation`) rather than a generic blanket rule is reasonable and matches actual usage today.
• WebSocket auth watchdog (`config/websocket/auth_handshake.py`): the periodic `_watch_authorization` + `dispatch()`-level revalidation is a meaningful hardening — previously a socket only re-checked authorization when the client chose to send an AUTH frame, so a corpus/document/conversation whose permissions were revoked mid-stream could keep receiving broadcasts indefinitely. The 1-second `allow_recent` window is scoped specifically to the `agent_stream_token` ASGI message type (not the client-controlled JSON payload), so it can't be gamed by a client frame — confirmed by `test_client_frames_recheck_within_stream_window`. Token-expiry is deliberately excluded from that caching window (checked every event), which is correct given expiry is a hard deadline rather than a revocable permission.
• Worker-token corpus-scope binding for chunked uploads (`document_imports/services.py`): recording `add_to_corpus_id` at `start_chunked_upload` time and re-validating it against the token's bound corpus in `_get_owned_session` closes a token-swap path where a worker token for corpus B could complete/inspect/append to a session started under corpus A's token. Good regression test (`test_chunked_operations_enforce_corpus_scope_for_same_worker`) exercising the full view-layer stack (404s on wrong token, 200 on right token, part contents unaffected).
• Frontend document-cache generation/session isolation (`documentCacheState.ts`, `documentCacheManager.ts`, `cachedRest.ts`, `authSession.ts`): the "assert generation hasn't changed" pattern sprinkled through the async IndexedDB paths correctly handles the case where a fetch/cache read is in-flight when a logout/account-switch happens — previously a late-resolving cache read/write could persist or return another account's document bytes. Nice coverage of the "late response after logout" scenarios in `documentCacheSession.test.ts`.
• `docxodusWasm` path traversal fix (`frontend/tooling/docxodusWasm.ts`): moving from ad hoc `path.join` to a symlink-aware containment check (`fs.realpathSync` both on the root and the resolved candidate) is the right fix for a dev-server static file handler; good that it's covered by explicit traversal/symlink/query-string tests.

Minor / non-blocking

1. Dead constant left behind: `TOKEN_LOG_PREFIX_LENGTH` (`opencontractserver/constants/auth.py:6`) was used exclusively by the token-prefix debug logging that this PR removes from `graphql_auth0_auth/utils.py` (and effectively superseded elsewhere by the same "don't log partial secrets" fix in `graphql_api_token_auth/backends.py` and `jwt_utils.py`). After this PR it has no remaining production call sites — only `opencontractserver/tests/test_constants.py` still imports and asserts on it. Per the repo's "no dead code" guideline, consider deleting the constant and its two tests in this PR (or a fast follow) rather than leaving a constant whose only purpose was the pattern you just intentionally removed.
2. `config/websocket/auth_handshake.py::_fail_auth`: sets `_handshake_connected = False` and calls `self.close()`. Since both the periodic watchdog (`_watch_authorization`) and `dispatch()`'s per-event `ensure_authorized()` check can independently detect a revoked/expired session, there's a narrow theoretical window for `_fail_auth` to be invoked twice in close succession (e.g., a broadcast arrives in the same tick the watchdog's sleep expires). `close()` being called twice is likely harmless with Channels/Daphne, but if you haven't already, worth a quick check that a double-close doesn't surface as a noisy `RuntimeError` in logs under load — not something I could construct a failing test for from reading alone.
3. Everything else I looked at closely (the `TreeTraversalService` recursive CTE mode handling for `descendants`/`full`/`subtree`, the fragment-depth memoization in `config/graphql/security.py`, and the worker-token kind gating in `start_chunked_upload`) checked out logically against their test coverage — no issues found.

Test coverage

Strong — the new `test_graphql_resource_contracts.py`, `test_websocket_session_contracts.py`, and `documentCacheSession.test.ts` suites specifically target the classes of bugs this PR fixes (anonymous/cross-user visibility leaks through FKs, ownership preservation on update, worker-token corpus scoping, stream-window vs. expiry timing, cache bleed across auth sessions) rather than just re-testing happy paths. The PR description's test-run summary (1,131-test broad run + full suite passes) is consistent with the scope of the change.

Nothing here blocks merging from my read — the one actionable item is the dead-code cleanup (#1 above).

[codecov]

Codecov Report

❌ Patch coverage is 91.39241% with 34 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
config/graphql/agent_types.py	79.16%	5 Missing ⚠️
config/graphql/annotation_types.py	91.66%	5 Missing ⚠️
config/websocket/auth_handshake.py	94.18%	5 Missing ⚠️
config/websocket/middleware.py	77.77%	4 Missing ⚠️
config/graphql/document_types.py	75.00%	3 Missing ⚠️
frontend/src/services/documentCacheManager.ts	91.42%	2 Missing and 1 partial ⚠️
config/graphql/extract_types.py	86.66%	2 Missing ⚠️
config/graphql/conversation_types.py	83.33%	1 Missing ⚠️
config/graphql/core/mutations.py	92.85%	1 Missing ⚠️
config/graphql/corpus_types.py	88.88%	1 Missing ⚠️
... and 4 more

📢 Thoughts on this report? Let us know!

[claude]

Review

Reviewed the full diff (39 files, GraphQL resolvers, WebSocket auth, DRF mutations, chunked-upload/worker-token auth, and frontend document-cache/dev-server hardening). This is a large, well-scoped security-hardening PR and the fixes look correct and are backed by strong regression coverage. Notes below are mostly non-blocking observations.

Strengths

• FK-visibility fix pattern is systematic, not ad hoc. Swapping raw strawberry.field FK attributes for resolve_visible_fk(...) resolvers across agent_types.py, annotation_types.py, conversation_types.py, corpus_types.py, document_types.py, extract_types.py, and research_types.py closes a real class of IDOR: a visible parent object (e.g. a Relationship or CorpusAction) could leak the contents of a private child object (Analysis, Extract, LabelSet, AgentConfiguration, GremlinEngine.api_key, etc.) via default getattr FK resolution. Locking this in with an AST-based architecture test (test_nullable_resource_links_do_not_use_default_fk_resolution in test_graphql_service_layer.py) so future nullable FK fields on the independent-types allowlist can't regress to plain attribute resolution is a nice piece of defense-in-depth.
• config/graphql/security.py::_measure_depth fragment-depth fix is a genuine correctness fix, not just perf. The old code mutated a single visited_fragments set across the whole recursive tree, so a legitimately-reused fragment at a second location would be treated as "already visited" and silently skipped — under-counting real depth (a query-depth-limit bypass), while a fragment DAG could also blow up exponentially. The new per-path visited_fragments (cycle guard) + fragment_depths cache (memoized relative depth) fixes both: test_reused_fragment_is_counted_at_each_depth and test_fragment_dag_is_measured_without_exponential_expansion directly pin this.
• DRF mutation ownership fix (core/mutations.py::_drf_mutation_body): popping creator/creator_id on UPDATE (vs. only setting it on CREATE) closes a bug where editing a shared object with UPDATE-only permission could silently reassign ownership. The added label_set/annotation_label visibility gate on pk_fields closes the companion IDOR (attaching someone else's private label set/label via a writable parent). Cross-checked against current pk_fields usage (grep -rn "pk_fields=" config/graphql/) — the hardcoded {"label_set", "annotation_label"} allowlist currently covers exactly the two private-resource fields in use; categories is correctly excluded as install-wide vocabulary.
• WebSocket auth (config/websocket/auth_handshake.py): the new dispatch()/ensure_authorized()/watchdog design closes a real gap — previously, permission revocation was only re-checked on the next client AUTH frame, so a socket bound to a corpus/conversation that had access revoked mid-stream could keep receiving server-pushed events indefinitely if the client never re-sent AUTH. Now every dispatched event (except the AUTH frame itself) revalidates, with a bounded 1s reuse window specifically for agent_stream_token fan-out (so a token-per-event burst doesn't multiply DB permission queries) and a background watchdog for idle sockets. test_stream_bursts_bound_permission_queries_per_viewer and test_idle_socket_expires_without_client_frames in test_websocket_session_contracts.py cover this well, including the "client can't forge the cache window via agent_stream_token-typed frames" case.
• Chunked upload / worker-token fixes: binding add_to_corpus_id to the access token at session start, then re-validating that binding in _get_owned_session, closes a token-swap IDOR. Nice catch that the old test expectations (ChunkedUploadStatus.FAILED) documented a side-effect bug — a rejected swap attempt used to mark the victim's legitimate session as FAILED (self-inflicted DoS); the new behavior (404, session left PENDING) is strictly better and the updated tests reflect that.
• Frontend: documentCacheState.ts's generation/scope-based cache invalidation is a solid fix for document content leaking across auth sessions in the same tab (logout/login, account switch), including the race where a stale read resolves after the session already changed. The docxodusWasm.ts extraction is a real path-traversal + symlink fix for the Vite dev server (previously path.join(__dirname, ..., match[1]) with no containment check, plus a wildcard Access-Control-Allow-Origin: *); good containment tests including the encoded (%2e%2e%2f) and symlink-escape cases.

Suggestions (non-blocking)

1. Potential N+1 from resolve_visible_fk in list contexts. Each FK now costs a separate BaseService.get_or_none (permission-checked) lookup instead of a free attribute access / prefetch. For high-cardinality connections (e.g. corpusActions with fieldset/analyzer/agentConfig, or extracts with parentExtract), this is now effectively one extra query+permission-check per row per FK field. This matches CLAUDE.md's own "Permission N+1 queries" pitfall callout. It's the correct trade-off for closing the IDOR, but might be worth a follow-up to batch these (e.g. a per-request/per-type dataloader keyed by (node_type_name, fk_pk)) if these connections show up in profiling.
2. core/mutations.py's pk_field in {"label_set", "annotation_label"} visibility check assumes a scalar pk. By that point in the loop, kwargs[pk_field] could in principle be a list (the code above it explicitly handles isinstance(raw_value, list) for other pk_fields like categories). If a future private list-type FK is ever added to this allowlist, BaseService.get_or_none(related_model, kwargs[pk_field], ...) would be called with a list and misbehave. Not a bug today (both current entries are scalar), but worth a comment or an explicit isinstance(..., list) guard so it fails loudly instead of subtly if that changes.
3. Minor: verified_token_expiry() in config/websocket/middleware.py re-decodes the JWT payload (unverified) a second time right after _get_user_from_token already verified it. Documented as intentional/safe (same token string, post-verification), so this is just a "could save a decode call" micro-nit, not a correctness issue.

Overall: this reads as a careful, well-tested pass closing several real authorization/session-lifecycle gaps (GraphQL FK leakage, WebSocket permission-revocation lag, upload token-swap, dev-server path traversal, cross-session document cache leakage). Nice work.