fix: lock analysis-run retention purge to a granted admin role - #137
Closed
cursor[bot] wants to merge 4 commits into
Closed
Conversation
Operators can empty a run-bearing analysis-run registry with
purge_analysis_run_registry('approved-retention-purge') instead of a
superuser trigger disable. Repeated citation chips and close buttons
use named design tokens and a Storybook catalog on Node 24.
Co-authored-by: Seongho Bae <[email protected]>
#125 already uses ADR 0017 and package 0.85.0 for POST /api/analysis-runs. Keep this retention-purge slice on the next free numbers. Co-authored-by: Seongho Bae <[email protected]>
The documented approval phrase is a procedure name, not a secret. REVOKE PUBLIC execute, grant only analysis_run_retention_admin, and record the invoking session role on each retention event so a runtime DATABASE_URL login cannot empty the registry. Co-authored-by: Seongho Bae <[email protected]>
Co-authored-by: Seongho Bae <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
Review
Do not merge this head. It is a conflicting duplicate of the #129 privilege fix.
Next action: take #134 (analysis_run_retention_grant + REVOKE PUBLIC, rebased onto current #74, ADR 0019 / v0.87.0). This branch still sits on dea0b3a and conflicts with #74 after #124 / #127 / #125 landed. The role-only analysis_run_retention_admin model is weaker than #134's revocable grant table once the compose app login is the migration owner (a superuser bypasses REVOKE; the in-function grant check still runs).
#134's remaining gap is test cleanup (DROP OWNED BY before DROP ROLE), not the product control. Do not open a third purge successor.
Sent by Cursor Automation: Fix Issues
Contributor
5 tasks
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
2 participants
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.
Buyer impact
Operators empty a run-bearing analysis-run registry only after
GRANT analysis_run_retention_adminandSET ROLE analysis_run_retention_admin. Then runselect purge_analysis_run_registry('approved-retention-purge'), exportanalysis_run_retention_event(it now records the invoking session role), delete those rows, and roll back 0019 then 0018. A runtime login that only knows the public phrase cannot empty the registry. A rawDELETEstill fails.Designers still change citation-chip and close-button appearance in
frontend/src/styles/tokens.cssand click those controls in Storybook (cd frontend && pnpm run storybook).Why this PR
Successor to #129. That slice added the approved purge, but
SECURITY DEFINERplus PostgreSQL's defaultPUBLICexecute grant meant any SQL login could wipe the registry with the documented phrase. NIST SP 800-53 Rev. 5 AC-3 treats that phrase as a procedure name, not an authorization secret.Stacked on #74 / ADR 0018 / package 0.86.0. Do not merge before #74. Do not merge #105 onto protected
main.What shipped
analysis_run_retention_admin(NOLOGIN) receivesEXECUTE;PUBLICdoes notanalysis_run_retention_eventrecordsinvoking_session_role,invoking_current_role, and optionalclient_network_addressVerification
python3 -m pytest -q tests/test_analysis_run_registry_schema.py tests/test_documentation_hygiene.py— 15 passed against local PostgreSQL 16cd frontend && pnpm run lint && pnpm run test && pnpm run build && pnpm run build-storybook— 45 tests passed; Storybook 10.5.8 built