Repository navigation
fix(kv,blob): close cross-tenant IDOR on KV and Blob data planes - #327
Merged
Merged
Conversation
The KV and Blob data endpoints authenticated the caller but never verified that the client-supplied project_id belonged to them (security review finding #3). For session/API-key auth the project_id was trusted verbatim, so any authenticated principal could read/write/list/copy/delete another project's KV entries and blobs by passing an arbitrary id. Deployment tokens were already constrained; the session/API-key path was the hole. Route every data-plane handler through the team-based project_access_guard! (the same ADR-028 mechanism every other project-scoped resource uses): - KV: fold the guard into the async extract_project_id chokepoint (covers all 7 data handlers). - Blob: fold it into extract_project_id for the body handlers and add an explicit authorize_project_access() call to the path-based blob_head and blob_download handlers. - Wire an optional ProjectAccessChecker into KvAppState/BlobAppState from the plugin context, and register temps-kv/temps-blob in the ADR-028 coverage snapshot. No-op in plain OSS (no checker registered, matching prior behaviour); enforced when a team-access plugin is present. Regression tests cover the denied/allowed/no-checker paths for both data planes.
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Fixed
- **kv,blob:** Confine data-plane access to the caller's projects
### Styling
- **kv,blob:** Apply rustfmt |
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.
Security review finding #3 — Cross-tenant IDOR on the KV and Blob data plane (High)
Location: `crates/temps-kv/src/handlers/handler.rs` (`extract_project_id`), `crates/temps-blob/src/handlers/handler.rs` (`extract_project_id`, `blob_head`, `blob_download`)
These data endpoints authenticated the caller but never verified that the client-supplied `project_id` belonged to them. For session/API-key auth the `project_id` was trusted verbatim, so any authenticated principal could read/write/list/copy/delete another project's KV entries and blobs by passing an arbitrary id. (Deployment tokens were already correctly constrained; the session/API-key path was the hole.)
Fix
Route every data-plane handler through the team-based `project_access_guard!` — the same ADR-028 mechanism every other project-scoped resource already uses:
Behavioural note: no-op in plain OSS (no checker registered — identical to prior behaviour); enforced when a team-access plugin is present. This matches how the entire codebase models multi-tenant project access.
Verification (regression tests only)