Skip to content

feat: 🐛 implement deletes, purges, apikey for security and update…#590

Merged
marcvergees merged 1 commit into
developmentfrom
588-bug-readme-claim-inaccurate-pii-is-collected-and-retained-locally-without-deletion-or-access-controls
Jun 24, 2026
Merged

feat: 🐛 implement deletes, purges, apikey for security and update…#590
marcvergees merged 1 commit into
developmentfrom
588-bug-readme-claim-inaccurate-pii-is-collected-and-retained-locally-without-deletion-or-access-controls

Conversation

@marcvergees

Copy link
Copy Markdown
Member

Walkthrough — Bug #588: PII Retention & Access Control

What Was Done

1. Fixed broken test imports

  • tests/test_v1_system.py: Fixed all app.api.v1.routes.systemapp.api.routes.system imports and monkeypatch paths.
  • tests/test_api.py, tests/test_jobs.py: Updated all bare /templates, /forms, /jobs paths to /api/v1/… to match the actual prefix.

2. Database — new repository helpers

app/db/repositories.py

  • get_form_submission(session, id) — lookup by PK
  • delete_form_submission(session, submission) — remove row and commit
  • delete_template(session, template) — remove row and commit

3. Config — new environment variables

app/core/config.py

Variable Default Purpose
FIREFORM_API_KEY "" (disabled) API key required on admin endpoints
RETENTION_PERIOD_DAYS 30 Days before submissions are auto-purged

4. Access-control dependency

app/api/deps.py: verify_api_key — when FIREFORM_API_KEY is set, callers must supply it via X-API-Key header or Authorization: Bearer <key>. No-ops when the key is unconfigured.

5. DELETE endpoints

  • DELETE /api/v1/templates/{id} — deletes the template DB record, its source PDF, all linked FormSubmission rows, their output PDFs, and all linked Job rows.
  • DELETE /api/v1/forms/{id} — deletes the submission DB row and its output PDF.
  • POST /api/v1/forms/purge?days=N — bulk-purges all submissions older than N days (defaults to RETENTION_PERIOD_DAYS), including their output PDFs.

All three endpoints require the API key when one is configured.

6. Celery purge task

app/tasks/purge.py: purge_old_submissions Celery task — same logic as the HTTP purge endpoint, scheduled via Celery Beat at 03:00 UTC daily.

app/core/celery.py: registered the new task and added the beat_schedule entry.

7. Tests

tests/test_deletion.py — 21 new tests covering:

  • DELETE template (not found, DB removal, file deletion, cascade to submissions)
  • DELETE submission (not found, DB removal, file deletion)
  • POST purge (age filter, file removal, nothing-to-purge)
  • API-key access control (no key → 401, correct key → 200, wrong key → 401, Bearer token)

8. README

README.md

  • Removed the inaccurate "never collects PII" claim.
  • Added accurate description of local PII storage and operator responsibility.
  • Added Data Deletion & Retention section with curl examples.
  • Added Access Control section with env-var setup.
  • Added Operator Hardening Recommendations section.
  • Updated the "Do No Harm" bullet to reflect operator responsibility.

Test Results

74 passed, 5 warnings in 1.11s

@marcvergees marcvergees merged commit d99f648 into development Jun 24, 2026
@marcvergees marcvergees deleted the 588-bug-readme-claim-inaccurate-pii-is-collected-and-retained-locally-without-deletion-or-access-controls branch June 24, 2026 18:57
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.

Bug: README claim inaccurate — PII is collected and retained locally without deletion or access controls

1 participant