Skip to content

Robust deduplication - #1046

Merged
imDarshanGK merged 17 commits into
imDarshanGK:mainfrom
shantanushok:security/dedupe
Aug 1, 2026
Merged

Robust deduplication #1046
imDarshanGK merged 17 commits into
imDarshanGK:mainfrom
shantanushok:security/dedupe

Conversation

@shantanushok

Copy link
Copy Markdown
Contributor

feat(security): upgrade request deduplication from in-memory to SQLite-backed persistent cache

Description

Solves #918

The initial implementation of this issue introduced basicin-memory dictionary for request deduplication. This PR completes the feature by replacing that in-memory dict with a persistent, SQLite-backed dedupe_cache table,
making deduplication survive server restarts and adding three new correctness guarantees:

Problem with in-memory dict : Fix in this PR
1] Reset on every server restart - a crash mid-request left a silent gap : Startup dedupe_clear_orphaned_processing() sweeps stale sentinels
2] A duplicate arriving while the first was processing was silently passed through : 'processing' sentinel + HTTP 409 Conflict during in-flight window
3] A failed response (4xx/5xx) was cached like a success - client retried into a replayed error : Error responses are never persisted; sentinel cleared immediately


Changes Made

backend/middleware/csrf.py

  • Renamed OriginValidationMiddlewareSecurityMiddleware to reflect its dual role.
  • Added in-flight duplicate detection: a 'processing' sentinel is inserted before the
    handler runs; a matching request that arrives during that window receives HTTP 409 Conflict.
  • Error responses (4xx/5xx) are NOT cached. Sentinel is deleted on error so the client
    can retry immediately without hitting a replayed failure.
  • Added two new Prometheus counters:
    • csrf_requests_total{status="deduplicated"} — served from cache
    • csrf_requests_total{status="deduplicated_in_progress"} — rejected as in-flight duplicate
  • Added Known Limitations section to the class docstring.

backend/services/db_service.py

  • Added dedupe_cache table to init_db() (with idx_dedupe_expires index).
  • Added five new helper functions:
    • dedupe_get / dedupe_set_processing / dedupe_set_done / dedupe_delete
    • dedupe_purge_expired — TTL cleanup feeding _maybe_vacuum()
    • dedupe_clear_orphaned_processing — startup-time stale sentinel removal

backend/app.py

  • Called dedupe_clear_orphaned_processing() in the lifespan startup hook before the
    server begins accepting traffic.

backend/tests/test_csrf.py

  • Added import hashlib, import json, import time and _TESTCLIENT_HOST constant.

  • Extended test suite from 32 → 36 tests:

    | Test | What it verifies |
    | test_request_deduplication | Second identical POST within 5 s returns identical cached body |
    | test_in_progress_sentinel_returns_409 | Planted 'processing' sentinel → 409 + Prometheus counter |
    | test_orphaned_processing_rows_cleared_on_startup | Startup cleanup removes stale row; next request succeeds |
    | test_error_response_not_cached_allows_immediate_retry | 422 response → sentinel deleted, immediate retry succeeds |
    | test_successful_response_is_still_cached_after_error_guard | Regression: 200 responses still cached after error guard |

docs/csrf-protection.md

  • Renamed to Security Middleware — CSRF & Deduplication.
  • Added Phase 2 deduplication section and Single-Worker Deduplication Design note.

Verification

pytest backend/tests/test_csrf.py -v
tests/test_csrf.py::test_get_no_origin_allowed PASSED
tests/test_csrf.py::test_post_no_origin_allowed PASSED
tests/test_csrf.py::test_post_valid_origin_allowed[http://localhost:3000] PASSED
tests/test_csrf.py::test_post_attacker_origin_blocked[https://evil.com] PASSED
...
tests/test_csrf.py::test_prometheus_metrics PASSED
tests/test_csrf.py::test_request_deduplication PASSED
tests/test_csrf.py::test_in_progress_sentinel_returns_409 PASSED
tests/test_csrf.py::test_orphaned_processing_rows_cleared_on_startup PASSED
tests/test_csrf.py::test_error_response_not_cached_allows_immediate_retry PASSED
tests/test_csrf.py::test_successful_response_is_still_cached_after_error_guard PASSED

Design Decisions

Why SQLite instead of the in-memory dict?

The in-memory dict was reset on every restart. A crash mid-request left no sentinel,
but the handler had already started — a silent idempotency gap. SQLite is already used
for all other app state in LocalMind and persists across restarts. The startup orphan
cleanup keeps the table consistent every time the server comes up.

Why are error responses not cached?

Replaying a transient error hides a real bug from the client and prevents a legitimate
retry from succeeding. Clearing the sentinel on error gives the client a clean slate
immediately.

Known limitations

  • Single-worker only. INSERT OR REPLACE is last-write-wins. Two Uvicorn workers
    racing on the same request could overwrite each other's cached responses. Acceptable
    for LocalMind's single-user, single-worker design; would need a shared cache (Redis)
    to scale correctly.

Checklist

  • All existing tests pass (36/36, 0 regressions)
  • New tests cover all new behaviour paths (in-flight 409, orphan cleanup, error guard)
  • No external dependencies added
  • docs/csrf-protection.md updated
  • Class and function docstrings updated
  • No unrelated files modified

Copilot AI review requested due to automatic review settings July 30, 2026 07:08
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

@shantanushok is attempting to deploy a commit to the Darshan's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@shantanushok

Copy link
Copy Markdown
Contributor Author

I found out the error I will push it right now ( I forgot to add Prometheus client package in the dependency list )

@shantanushok
shantanushok requested a review from imDarshanGK July 30, 2026 08:20
adikulkarni006
adikulkarni006 previously approved these changes Jul 30, 2026
adikulkarni006 and others added 2 commits July 30, 2026 14:14
…ow CI expiry

Also optimizes response byte accumulation in csrf.py using b''.join
@shantanushok

Copy link
Copy Markdown
Contributor Author

I have tried to solve the ci issues so whenever @adikulkarni006 and @imDarshanGK are available feel free to re-approve the workflows.

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

shantanushok and others added 3 commits July 30, 2026 18:47
- Extract compute_request_hash to use as the single source of truth for both middleware and tests.
- Update 	est_csrf.py to use httpx.Request serialization to match the middleware byte-for-byte, fixing the sentinel mismatch in CI.
- Add an �utouse fixture to 	est_bulk_export.py to clear the sessions table before each test, preventing global database state bleed during randomized test execution.
@shantanushok
shantanushok requested a review from imDarshanGK July 31, 2026 10:27
@shantanushok

Copy link
Copy Markdown
Contributor Author

@imDarshanGK can you reapprove the only the backend workflow .

@adikulkarni006
adikulkarni006 self-requested a review July 31, 2026 17:36
adikulkarni006
adikulkarni006 previously approved these changes Jul 31, 2026
@shantanushok

Copy link
Copy Markdown
Contributor Author

hey @imDarshanGK I have resolved the merge conflicts can you reapprove the workflows ?

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@shantanushok

Copy link
Copy Markdown
Contributor Author

@imDarshanGK could you please merge this

@imDarshanGK imDarshanGK added SSoC26 Part of Social Summer of Code 2026 Medium Feature or backend work with moderate scope labels Aug 1, 2026
@imDarshanGK
imDarshanGK merged commit 1a67ea7 into imDarshanGK:main Aug 1, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Medium Feature or backend work with moderate scope SSoC26 Part of Social Summer of Code 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants