Skip to content

Conversation

@AnthonyRonning
Copy link
Contributor

@AnthonyRonning AnthonyRonning commented Nov 23, 2025

  • feat: add endpoint to delete all kv items
  • Update PCRs

Summary by CodeRabbit

  • New Features

    • Added bulk deletion feature enabling users to delete all stored key-value pairs in a single operation.
  • Chores

    • Updated PCR history records for development and production environments.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 23, 2025

Walkthrough

PCR history entries appended to development and production history files. A new bulk delete feature for user key-value pairs is implemented across the database, service, and API layers, introducing a DELETE endpoint and supporting functions to remove all KV entries for a given user.

Changes

Cohort / File(s) Summary
PCR History Records
pcrDevHistory.json, pcrProdHistory.json
New PCR history entries appended with timestamps 1763921652 (dev) and 1763921684 (prod), including PCR0, PCR1, PCR2, and signature fields
Database & Service Layer
src/models/user_kv.rs, src/kv.rs, src/main.rs
Bulk delete functionality added: UserKV::delete_all_for_user() removes all KV records for a user; kv::delete_all() obtains DB connection and delegates; AppState::delete_all() provides async interface
API Endpoint
src/web/protected_routes.rs
New DELETE route at /protected/kv with delete_all_kv() handler; deletes all user KV pairs with encrypted JSON response

Sequence Diagram

sequenceDiagram
    actor Client
    participant API as DELETE /protected/kv
    participant AppState
    participant kv as kv::delete_all()
    participant DB as UserKV::delete_all_for_user()
    participant Postgres

    Client->>API: DELETE request + encrypted body
    API->>API: Decrypt request (middleware)
    API->>AppState: delete_all(user_id)
    AppState->>kv: delete_all(pool, user_id)
    kv->>kv: Acquire DB connection
    kv->>DB: delete_all_for_user(conn, user_id)
    DB->>Postgres: DELETE FROM user_kv WHERE user_id = ?
    Postgres-->>DB: Rows deleted
    DB-->>kv: Result<(), Error>
    kv-->>AppState: StoreResult<()>
    AppState-->>API: Result
    API->>API: Encrypt response
    API-->>Client: JSON response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the delete_all implementation flow across four related files to ensure consistent error handling and connection management
  • Verify that the DELETE endpoint properly validates user context via extensions
  • Confirm PCR history entry timestamps and signatures are correctly formatted

Possibly related PRs

  • Better health checking #74: Appends new PCR history entries to the same dev/prod history files, indicating iterative PCR record maintenance
  • Revert llama33 to tinfoil primary #96: Also modifies pcrDevHistory.json and pcrProdHistory.json with new PCR entries following the same update pattern
  • Jwt hs256 #40: Appends PCR history entries to the identical history files with the same structural format

Poem

🐰 A rabbit hops through data flows so clean,
Deleting KV pairs with DELETE between,
PCR records marked with cryptographic flair,
From endpoint to database, no key left there! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat/delete all kv' accurately summarizes the main change: adding functionality to delete all key-value pairs for users, as evidenced by the new DELETE route and delete_all methods across the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/delete-all-kv

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e51a3c and a8815fc.

⛔ Files ignored due to path filters (2)
  • pcrDev.json is excluded by !pcrDev.json
  • pcrProd.json is excluded by !pcrProd.json
📒 Files selected for processing (6)
  • pcrDevHistory.json (1 hunks)
  • pcrProdHistory.json (1 hunks)
  • src/kv.rs (1 hunks)
  • src/main.rs (1 hunks)
  • src/models/user_kv.rs (1 hunks)
  • src/web/protected_routes.rs (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/models/user_kv.rs (5)
src/models/users.rs (2)
  • diesel (203-205)
  • delete (152-158)
src/models/oauth.rs (4)
  • diesel (94-96)
  • diesel (218-220)
  • delete (65-71)
  • delete (178-184)
src/models/email_verification.rs (2)
  • diesel (110-112)
  • delete (69-75)
src/models/enclave_secrets.rs (2)
  • diesel (80-85)
  • delete (58-64)
src/kv.rs (1)
  • delete (100-120)
src/main.rs (1)
src/kv.rs (1)
  • delete_all (122-131)
src/web/protected_routes.rs (4)
src/kv.rs (1)
  • delete (100-120)
src/models/user_kv.rs (1)
  • delete (69-75)
src/models/orgs.rs (1)
  • delete (73-79)
src/web/encryption_middleware.rs (1)
  • encrypt_response (93-107)
src/kv.rs (2)
src/main.rs (1)
  • delete_all (990-992)
src/models/user_kv.rs (2)
  • diesel (107-112)
  • delete_all_for_user (77-86)
⏰ Context from checks skipped due to timeout of 100000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Development Reproducible Build
🔇 Additional comments (7)
pcrProdHistory.json (1)

442-448: Appended PCR entry matches existing structure

New history entry keeps the same field schema and ordering (PCR0/1/2, timestamp, signature) and is correctly appended to the array. No issues from an application perspective.

pcrDevHistory.json (1)

442-448: Dev PCR history append looks consistent

The new dev PCR entry mirrors existing entries and the prod addition (same keys, value formats). JSON remains valid; no functional concerns.

src/main.rs (1)

990-992: AppState::delete_all correctly delegates to KV layer

Wrapper mirrors the existing get/put/delete/list helpers and cleanly forwards to kv::delete_all using the shared pool. This keeps the KV API surface in one place; no issues spotted.

src/kv.rs (1)

122-131: Bulk delete implementation is correct and consistent with existing KV helpers

delete_all acquires a connection, scopes the delete by user_id, and maps UserKVError into StoreError in the same way as other helpers. Returning Ok(()) regardless of how many rows were removed is appropriate for a “delete all” operation.

src/web/protected_routes.rs (2)

470-474: DELETE /protected/kv route wiring is aligned with existing patterns

The new route reuses the same decrypt middleware pattern as other protected endpoints and cleanly separates list (GET /protected/kv) from bulk delete (DELETE /protected/kv). No routing or middleware issues evident.


704-725: delete_all_kv handler cleanly exposes bulk delete semantics

Handler behavior matches delete_kv: logs entry/exit, calls data.delete_all(user.uuid), and wraps a simple status message via encrypt_response. Error mapping to InternalServerError on any StoreError is consistent with the rest of the KV API.

src/models/user_kv.rs (1)

77-86: Model-level bulk delete follows established Diesel patterns

delete_all_for_user is a straightforward diesel::delete(..).filter(user_id == ...) and maps errors via UserKVError::DatabaseError, matching other model delete helpers. Semantics and style look good.


Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Nov 23, 2025

Greptile Overview

Greptile Summary

This PR adds a new endpoint to delete all key-value pairs for an authenticated user. The implementation follows the same pattern as the existing delete_all_conversations endpoint, adding the functionality across all layers from database model to REST API.

  • Added delete_all_for_user method to UserKV model that uses Diesel ORM to delete all records for a given user_id
  • Created corresponding delete_all functions in the kv service layer and AppState
  • Registered new DELETE /protected/kv route alongside existing GET (list) route on the same path
  • Updated PCR measurements for both dev and prod environments to reflect the binary changes

The implementation is clean, follows established patterns in the codebase, and properly leverages the authentication middleware to ensure users can only delete their own data.

Confidence Score: 5/5

  • This PR is safe to merge with no identified issues
  • The implementation mirrors the existing delete_all_conversations pattern, uses proper authentication and authorization through middleware, correctly filters deletions by user_id to prevent cross-user data deletion, follows Rust best practices with proper error handling, and updates all necessary PCR attestation measurements
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/models/user_kv.rs 5/5 Added delete_all_for_user method that deletes all KV pairs for a user using Diesel ORM with proper user_id filtering
src/kv.rs 5/5 Added delete_all function that wraps database call, properly handles connection pooling and error mapping
src/main.rs 5/5 Added delete_all method to AppState that delegates to kv::delete_all without requiring encryption key
src/web/protected_routes.rs 5/5 Added DELETE /protected/kv route and handler that deletes all KV pairs for authenticated user with proper response encryption

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as Protected Routes
    participant AppState
    participant KV as kv::delete_all
    participant Model as UserKV Model
    participant DB as PostgreSQL

    Client->>API: DELETE /protected/kv
    Note over API: decrypt_request middleware
    API->>API: Extract user from Extension
    API->>AppState: delete_all(user.uuid)
    AppState->>KV: delete_all(pool, user_id)
    KV->>KV: Get DB connection from pool
    KV->>Model: delete_all_for_user(conn, user_id)
    Model->>DB: DELETE FROM user_kv WHERE user_id = ?
    DB-->>Model: Success
    Model-->>KV: Ok(())
    KV-->>AppState: Ok(())
    AppState-->>API: Ok(())
    API->>API: encrypt_response with success message
    API-->>Client: JSON response with encrypted confirmation
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@AnthonyRonning AnthonyRonning merged commit 8d575a8 into master Nov 23, 2025
8 checks passed
@AnthonyRonning AnthonyRonning deleted the feat/delete-all-kv branch November 23, 2025 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants