[jwvanderstam]

QP-10 item 5, first part: SECURITY.md's R-9 (QP-9 review, Low, admin-only).

What was wrong

The five single-object purges checked references and nothing else:

• A live document, conversation, workspace, connector or user could be hard-deleted in one call. That skips the reversible Retire step that Clark-Wilson puts before Destroy.
• The only active admin could be purged if it had no memberships. The review of fix(security): R-10 — admin user routes validate their bodies; the last-admin rule counts active admins #525 (its L-2) found this again.
• A missing id gave the wrong answer. A missing document reported success. A missing connector got a 409 about synced documents.

The change

• require_retired(cur, table, id) in src/db/connection.py runs in each purge's own transaction, before the existing reference check:
  • SELECT deleted_at IS NOT NULL ... FOR UPDATE, so a restore cannot slip in between the check and the DELETE;
  • the table name goes through sql.Identifier;
  • it raises NotRetiredError when the row is missing or still live.
• purge_refusals(what) in src/services/errors.py turns that error into a ServiceError: 404 for a missing row, 409 "… must be retired before it is purged" for a live one. The five purge methods in the services use it.
• The routes for users, conversations, workspaces and connectors map ServiceError; the document route already did.
• Unchanged: purge-all already deletes retired documents only. The Users screen only offers purge on a retired user, so the UI path is unchanged.
• CLAUDE.md's Clark-Wilson section now names the precondition: the row is retired, and no active references exist.

Tests

• tests/integration/test_purge_requires_retired.py: all five purges against Postgres.
  • A live row is refused and survives.
  • A missing row is a 404.
  • A retired row is destroyed.
  • 10 of the 15 tests fail on the code before this change. The 5 retired-row cases pass both ways; they are the permitted case.
• tests/unit/test_purge_requires_retired.py: the check (including the FOR UPDATE statement), the 404/409 mapping, and all five routes.
• Five existing repository tests answered the new retired check on their mocked cursors.
• Locally:
  • ruff, mypy, bandit and lint-imports are clean.
  • Fast suite: 3703 passed, 22 skipped.
  • The new integration file passes on a throwaway pgvector.

Notes

CHANGELOG.md [Unreleased] and the SECURITY.md R-9 row will conflict with #523, #524 and #525. #525 also edits R-9's row; whichever merges later needs a rebase.

This touches src/services/ and every purge path, so CONTRIBUTING.md asks for the separate agent-session review before merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[jwvanderstam]

Separate security review (CONTRIBUTING / QP-9)

A separate agent session reviewed this PR read-only. No finding at Medium or above. All five single-object purges lock and check the row as the first statement of their own transaction. The reference checks still follow. All six routes still require a global admin. The integration tests fail on the old code beside the permitted case. There is no restore path anywhere in src/, so the check-then-delete race cannot occur today, and the lock covers it if one is ever added.

Low and Info findings:

Finding	Answer
Low: a withdrawn contribution sets archived_at, not deleted_at, so it can no longer be purged (409), and purge_all skips it too.	Fails safe; recorded. Whether "archived" counts as retired for a global document is a product decision. Until it is made, archived contributions stay.
Low (pre-existing): a malformed UUID in a purge path is a 500, not a 404/422.	Recorded for a follow-up: type the path parameters as uuid.UUID. Nothing leaks.
Info: purge_all runs two DELETEs without a lock.	Harmless without a restore TP; to be revisited if one is added.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud