fix(security): close three cross-seat authorization gaps#91
Closed
Sarthib7 wants to merge 3 commits into
Closed
Conversation
/feedback passed body.dataset and body.session_id straight into the durable write, skipping resolve_write_dataset / resolve_session_id. It was the only write-scoped route in its range that did — /ingest and /api/contribute both gate the same fields. A writer token could name any dataset, including another seat's node that enforce_dataset_allowlist is default-deny for, and have both the write and its mesh event attributed there. The MCP tool citadel_record_feedback forwards these fields verbatim, so the gap was reachable from that surface too. Also byte-cap feedback text: it lands in durable storage on a cache miss and FeedbackBody.text carries no max_length, so it bypassed the /ingest limit. Both tests fail against the previous handler (200 instead of 403/413).
The handler called require_access and threw the identity away, then returned get_mesh().timeline(...) unfiltered — so any reader token received every seat's events, including message text, dataset names, and (since the recent error-detail change) the failing operation and its reason. Both sibling projections already scope: /api/mesh via scope_mesh_snapshot and /events via _mesh_dataset_visible. The leak was visible in default `citadel activity` output, which printed other seats' ingest events under the caller's own token. Mesh.timeline() gains an optional `visible` predicate, applied before the limit slice so a caller receives a full page of their own events rather than whatever survives filtering one page of everyone's. latest_event_id stays global: --watch resumes from it, and rewinding it to the last visible id would re-poll forever. Bypass (admin/env) callers pass no predicate. Test fails against the previous handler.
ObsidianVault.owner_actor_id was set at registration and never read again, so
five endpoints were gated by scope alone: manifest, sync/pull, sync/push,
conflicts/{id}/resolve, and the Obsidian branch of /api/documents/{id}. Both
obsidian:sync:pull and kb:read are in the default reader scope set, so no
privilege escalation was needed — only a vault id, which /api/sources
discloses to any sources:read caller.
Reads returned full note bodies and complete revision history; push wrote
revisions into the target vault with actor recorded for attribution only.
All five now go through assert_obsidian_vault_owned, which returns 404 rather
than 403 so a scoped caller cannot use the status code as an existence
oracle — the same rule the cognee drill-down branch already applied ten lines
below the Obsidian one. Admin/env callers bypass.
Adds vault_owner/document_vault_id/conflict_vault_id accessors to the sync
store rather than reaching into its state from the handlers.
Test fails against the previous handlers (200 on another actor's vault).
Sarthib7
added a commit
that referenced
this pull request
Jul 20, 2026
Merge origin/main (docs from #92) and fix/feedback-dataset-scoping (#91) into design/shared-session-index. Resolve doc conflicts keeping v1-implemented versions. Update access tests for session-traces in default allowed_datasets. Co-authored-by: Cursor <[email protected]>
Member
Author
9 tasks
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
1 participant
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.
Closes three cross-seat authorization gaps found in the 2026-07-19 audit. Each is a
read or write that authenticated but never checked whose data it touched — scope
alone was the gate, and the relevant scopes are all in the default reader/writer sets,
so no privilege escalation was needed to reach any of them.
The three
/feedback— unscoped dataset + session (43c30fc)Passed
body.dataset/body.session_idstraight into the durable write, skippingresolve_write_dataset/resolve_session_id. It was the only write-scoped route inits range that did —
/ingestand/api/contributeboth gate the same fields. Awriter token could name another seat's node and have both the write and its mesh event
attributed there. Reachable from MCP too:
citadel_record_feedbackforwards the fieldsverbatim. Also byte-caps feedback text, which had no
max_lengthand so bypassed the/ingestlimit on a cache miss./api/knowledge/events— unfiltered timeline (d73c849)Called
require_access, discarded the identity, and returnedget_mesh().timeline(...)whole — every seat's events, including message text, dataset names, and the failing
operation and reason. Both sibling projections (
/api/mesh,/events) already scope.The leak showed up in plain
citadel activityoutput.Mesh.timeline()gains anoptional
visiblepredicate applied before the limit slice, so a caller gets a fullpage of their own events rather than the remnants of one page of everyone's.
latest_event_idstays global so--watchstill resumes correctly.Obsidian vault ownership never enforced (
ab18631)ObsidianVault.owner_actor_idwas set at registration and never read again, leavingfive endpoints gated by scope alone: manifest, sync/pull, sync/push,
conflicts/{id}/resolve, and the Obsidian branch of
/api/documents/{id}. Reads returnedfull note bodies and revision history; push wrote revisions into the target vault. Only
a vault id was needed, which
/api/sourceshands to anysources:readcaller. All fivenow go through
assert_obsidian_vault_owned.Notes
code as an existence oracle — matching the rule the cognee drill-down branch already
applied ten lines below the Obsidian one.
vault_owner/document_vault_id/conflict_vault_idaccessors on the sync store rather than handlers reaching into its state.
Testing
127 passedintests/test_server.py. Each of the three commits adds tests that failagainst the pre-fix handlers (200 where 403/404/413 is now expected).