Repository navigation
fix(retention): keep support reports until they are deleted - #10
Merged
Merged
Conversation
Reports, their conversations, and their encrypted diagnostic objects are
no longer deleted because of their age.
- Remove the per-product retention period. Migration 005 makes
retention_until nullable and clears it for every existing row; new
reports store NULL.
- The hourly purge, renamed PurgeDeleted, only hard-deletes rows whose
deleted_at is set by a reporter, a cancellation, or a maintainer. A
failed object deletion still keeps the row for a later retry.
- Add DELETE /api/v1/admin/reports/{id}, which requires the admin
session and CSRF token, is audited as report_deletion_requested, and
reuses the reporter deletion path. The console gains a confirmed
"Delete report" action.
- Refuse to start when the private object root is not writable, and log
the object root and the number of live reports whose diagnostic object
is missing. Missing objects never change or delete a report.
- Keep retentionUntil in every contract version 1 response as
createdAt + 30 days, because released clients require it. It is
documented as a legacy informational field, not a deletion date.
- Show "Kept until deleted" in the portal and document indefinite
retention, manual deletion, the persistent /data requirement, and a
runbook for restoring missing diagnostic objects.
- Run the PostgreSQL migration and store tests in CI.
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
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.
Outcome
Support reports, their private conversations, and their encrypted diagnostic ZIPs are now kept until they are deleted. Nothing is deleted because of its age.
RetentionDayssetting is removed from product registration and fromGET /api/v1/products.PurgeDeleted, also run at startup) only hard-deletes rows whosedeleted_atis set by a reporter deletion, a submission cancellation, or a maintainer deletion. If removing the diagnostic object fails, the row is kept so the deletion can be retried.DELETE /api/v1/admin/reports/{id}: requires the admin session and theX-CSRF-Tokenheader, is written tosupport_admin_auditasreport_deletion_requestedbefore it runs (fails closed if the audit cannot be written), soft-deletes the report (revoking the reporter's link immediately), and removes the diagnostic object through the same path as reporter deletion. Returns204, or404for an unknown or already deleted report. The console has a confirmed Delete report button.docs/operations.md,docs/threat-model.md, andopenapi.yamldescribe the new retention, manual deletion, the persistent/datarequirement, and a runbook for restoring missing diagnostic objects. The expired-backup deletion schedule and the "retention purge" recovery step are removed.Compatibility constraint:
retentionUntilstays in contract v1Installed nati.ve apps (support contract version 1) parse
retentionUntilin receipts and private status responses as a required, non-null string, and reject a receipt unless it is a timestamp no earlier thancreatedAtand at most 30 days after it (for a new submission: also after now and at most now + 30 days). Returningnull, omitting the field, or returning a later date would make every installed app treat new submissions as failed and break status checks.So every contract v1 response (receipt, reconcile, private status, reporter reply, and the admin list, detail, update, and message responses) still contains
retentionUntilas an RFC 3339 timestamp equal tocreatedAt + 30 days, computed in the response mapping. This is the same value existing reports already had.openapi.yamldocuments it as a deprecated, informational legacy field that is not a deletion date. The contract version is unchanged. A new HTTP contract test pins the field as present, non-null, and equal tocreatedAt + 30 dayson all those endpoints, including for a report created in 2016 whose value passed long ago, and checks that the purge leaves that report in place.The portal no longer shows
retentionUntilas an expiry.Database migration
005_retain_until_deleted.sql(idempotent, since migrations run on every startup):support_reports.retention_untilnullable and sets it to NULL for every row, so no existing report is queued for deletion again;deleted_atfor the purge.The column is kept rather than dropped: if an older image is started against the migrated database, its purge cannot select NULL rows by age, while a dropped column would add a second, irreversible schema change for no benefit. New reports store NULL. Rows that were already purged are gone and do not come back.
Rolling back to an earlier image after this migration is not supported. The old image cannot read NULL
retention_untilvalues, so its report reads fail, but it deletes nothing because of age.Deployment note
The production
/datavolume must be a persistent volume that the platform reattaches on every redeploy, together with the same database andSUPPORT_DATA_KEY. The DockerfileVOLUMEline alone does not guarantee this. After deploying, check the startup log forprivate diagnostic objects verifiedwithmissing0. A warning that names the object root withmissingabove zero means objects are absent; follow "Restoring missing diagnostic objects" indocs/operations.md.Validation
Run locally on Windows with Go 1.25.5 and Node 22.13.0:
go vet ./...: passed.go test -count=1 ./...: passed. Skipped locally: the two PostgreSQL tests ininternal/store/postgres_test.go(no database or Docker daemon available) and the read-only directory probe test (not applicable on Windows). CI now runs the PostgreSQL tests against apostgres:17-alpineservice throughSUPPORT_TEST_DATABASE_URL, and runs the read-only probe test on Linux.frontend:npm ci,npm run lint(vue-tsc),npm test(16 tests, 3 files),npm run build: passed.62672e0:service(including both PostgreSQL tests againstpostgres:17-alpine),portal, andcontainerpassed. The first CI run caught a fixture bug in the new PostgreSQL tests (hashes shorter than the 32-byte check constraint), fixed in the second commit.New tests cover the migration clearing existing deadlines (and running twice); long-expired live reports never being purged (memory and PostgreSQL); reporter deletions being purged; an object-delete failure keeping the row and succeeding on retry; admin deletion (session, CSRF, audit entry, object removal, revoked link, repeated, unknown, and malformed IDs); the startup probe refusing an unwritable root; missing-object count logging without changing reports; the
retentionUntilv1 contract; and the portal showing "Kept until deleted", the intake sentence, and the confirmed and cancelled admin delete flow.Limitations
retentionUntil. Keeping it longer needs a client change and a new contract version.