Felis/docs/changes/2026-07-08-backup-restore-mutation-audit.md

Backup/restore data-safety mutation audit (round 2) — 7 gates pinned, 1 gap closed

• Type: test-quality audit + one test added (code change: internal/api/handlers_backups_test.go)
• Date: 2026-07-08
• Area: internal/api (handlers_backups.go), internal/backupjob
• Task: continues the #82 test-quality integrity audit onto the on-demand backup/restore surface, which postdates the 18-gate round-1 audit (mutation audit 4626ab5). The backup/restore endpoints (7a7c0d5 / f2fc57c / fc748d3) were not in that pass.

Method

Same as round 1: apply a one-line mutation to a fail-open gate in the source, run the package tests, confirm the specifically-named test reddens with an assertion failure (--- FAIL: <subtest>), then revert. A build break (declared and not used, undefined) is not a valid verdict, so mutations are operator-flips that keep every operand referenced (!=→==, drop a !, a literal→true, or if false && <orig> to disable a gate without orphaning its variables). Airtightness: each mutation is re-run with -run scoped to the intended subtest and grep -- "--- FAIL: <subtest>", so a reddening sibling can't be mistaken for the gate under test. Oracle: WSL Fedora-44, go1.26.4.

Fail-open gates mutation-verified (all CAUGHT at the named subtest)

#	Gate (file:line)	What it guards	Mutation	Subtest that reddened
A	handlers_backups.go:282 enqueueBackup stopped-gate	RWO double-mount / torn archive while the world is up	!=→==	TestBackupNow/starting_server_->_409_not_stopped
B	:151 restore stopped-gate	restore Job can't mount a live world's RWO PVC	!=→==	TestRestoreBackup/starting_server_->_409_not_stopped
C	:136 restore former-owner match	a fresh claimant resurrecting the previous owner's world	!=→==	TestRestoreBackup/current_owner_who_is_not_former_owner_->_403
D	:116 restore cross-server guard	restoring server A's backup onto server B	!=→==	TestRestoreBackup/restore_by_backup_id_cross-server_->_403
E	:214 backup owner-or-admin authz	a stranger backing up someone else's world	drop !	TestBackupNow/non-owner_->_403,_no_backup
F	:26 list cross-user scope	a user seeing other tenants' backups	p.IsAdmin()→true	TestListBackups/user_sees_only_own_former-owned_present_backups

The gap this audit found — and closed

Restore's owner-or-admin gate (handlers_backups.go:85) was not pinned by any test. It is the twin of gate E, but the two are not symmetric. Disabling it (if false && !a.isOwnerOrAdmin(p, rec), which keeps rec referenced so the package still builds) reddened nothing — go test ./internal/api/ stayed ok. The same disable applied to backup's L214 (gate E) reddened non-owner immediately, proving the technique valid and the asymmetry real.

Root cause: the former-owner gate at L136 backstops every non-owner case the suite exercised (a stranger and a wrong-backup current owner both fail L136 and L85, so L136's 403 masks a broken L85). The one case only L85 catches went untested: a superseded former owner — a user who owned a server, took this backup (FormerOwner=them), then released it to a new owner. They still pass L136 (they are the former owner) but must be stopped by L85, or they could roll the new owner's live server back onto their old world (cross-tenant clobber). The handler comment names this the "must re-claim first" rule (handlers_backups.go:77-79).

Fix (code): added TestRestoreBackup/former owner after release -> 403, no restore, the mirror of the existing L136 test. Verified both directions: green on the clean tree, and it is the sole subtest that reddens when L85 is disabled — so it now pins the owner gate specifically, not L136. No production code changed; handlers_backups.go is a pure test addition away from where it was.

Enumeration — covered vs. scoped (so "the gates" means all of them)

• Fail-open data-safety gates — all pinned: A–F above, plus L85 (now closed). 7/7.
• Accountability, not fail-open (verified non-vacuous): the os_user attribution at handlers_backups.go:254 — a supplied operator name overrides the default break-glass audit actor. Mutating u != ""→u == "" reddens TestInternalBackup/os_user_body_attributes_the_audit_to_the_operator, so the attribution test isn't vacuous. A failure here degrades the audit actor; it grants no bypass, so it is out of the fail-open bucket.
• Contract/behavioral (tested, out of mutation scope): nil Backuper/Restorer → 503; a failed backup/restore → 500 not audited; backup_ref never serialized to the wire; the internal-face actor defaults to break-glass. Each has a direct test; none is a fail-open safety gate.
• internal/backupjob (glanced, not mutated): orchestration only — each backup gets a unique Job name (BackupJobName + random suffix) so a repeat "立即备份" tap can't collide with a just-finished Job still inside its TTL; ErrAlreadyExists is a defensive no-op; Backup returns once the Job is created (the async 202 is honest). Unit-tested against a fake Jobs; the controller-runtime k8sjobs.go and the jobspec.go Pod shape (weak SA with its token un-mounted, config Secret mounted for the self-recorded row, read-only world mount) are integration-verified per the package doc — not fail-open API gates.

Coverage nuance (documented, not a gate failure)

The stopped-gate is if info.Ready || info.DesiredState != DesiredStopped. The !=→== mutation pins the DesiredState operand (both A and B reddened), but info.Ready is not independently pinned: no test sets Ready=true together with DesiredState=Stopped — the stopping-but-still-up race. Low risk because in practice Ready drops as DesiredState leaves Stopped, but the belt-and-suspenders Ready operand rides on coverage of the operand beside it rather than its own case.

Verdict

The backup/restore data-safety surface is a coherent unit, and this closes it: 7/7 fail-open gates pinned (6 pre-existing, 1 added this round), one accountability gate shown non-vacuous, one coverage edge documented. Not extended to every handler — that would be an unbounded "continue the audit."