feat(domain): soft-delete domains and write a domain.deleted audit record - #1960
Conversation
… deleted domains Delete sets deleted_at through softDelete instead of removing the row, and a domain that is already deleted reports not found. The daily cleanup of expired pending requests skips deleted rows, so it no longer removes them.
The service reads the domain and its organization, marks the domain deleted, then records the delete with the organization as the resource and the domain as the target. A failed record write is logged and does not fail the delete.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: raystack/frontier/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughDomain deletion now uses soft deletion in PostgreSQL and records a ChangesDomain deletion
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk remains after normal checks. Security Architecture ReviewSecurity architecture risk: 🟠 High · up to A code-only rollback could make deleted verified domains usable for organization joining again. Deletions can also succeed without a durable audit record if the subsequent audit write fails. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for CI Build 36551101108Coverage increased (+0.1%) to 52.89%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
This branch was successfully deployed
3 participants
Summary
DeleteOrganizationDomainnow setsdeleted_atinstead of removing the row, and each delete writes adomain.deletedaudit record. Callers get the same responses as before.Changes
DomainRepository.DeleteusessoftDelete(feat(store): add a helper that marks rows as deleted #1959), so the row stays withdeleted_atset. A domain that is already deleted reports not found.DeleteExpiredDomainRequests, skips deleted rows. It still removes old pending domains that nobody deleted.domain.Service.Deletereads the domain and its organization, marks the domain deleted, then writes the audit record. The organization is read withGetRaw, so a superuser can still delete a domain of a disabled organization.domain.deletedand the newdomainentity type. The actor comes from the request. A failed record write is logged and does not fail the delete.OrgServicemock gainsGetRaw, andAuditRecordRepositoryhas a new mock.Technical Details
Reads already skip deleted domains, so get, list, verify, join, and the joinable organizations list treat a deleted domain as gone.
Domain names are unique among live rows only (
uq_domains_org_id_name_live), so a deleted name can be added again.Organization delete is still a hard delete. The
domains.org_idforeign key isON DELETE CASCADE, so it also removes the organization's deleted domain rows.Test Plan
go test ./internal/store/postgres/passes. New cases: the row stays after a delete, a second delete reports not found, the same name can be added again, and the cleanup keeps deleted rows.go test ./core/domain/passes.TestService_Deletechecks the full audit record, and that no record is written when the domain is unknown, the organization lookup fails, or the delete fails.TestOrganizationDomainsAPI: delete, get returns not found, the same name can be created again, exactly onedomain.deletedrecord with the caller as its actor, and the organization delete still works.golangci-lint runreports no issues.mainon the same database, with only the binary swapped.deleted_atsetdomain.deleted, all fields match, actor is the callerThe joinable checks mark the domain verified in the database, because DNS verification cannot run locally. The cleanup rows run the function the daily job calls. On
mainthe deleted row is removed by the delete itself, before the cleanup runs.SQL Safety
?placeholders,goqu.Ex{}, orgoqu.Record{}— neverfmt.Sprintfor+building a query that gets executed.ToSQL()callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Neverquery, _, err := ….?placeholders inside single-quoted SQL literals ingoqu.L.//nolint:forbidigoor// #nosec G20xannotations.