venus-db-backup.sh: trash||rm precedence hard-deletes clinical dumps when trash exists and FAILS
venus-db-backup.sh prune() line: 'command -v trash >/dev/null && trash "$f" 2>/dev/null || rm -f "$f"'. Shell && || precedence makes this NOT the intended if/else. It reads as (command -v trash && trash f) || rm -f f, so rm -f fires in TWO cases, not one: (i) trash is absent - intended fallback; (ii) trash EXISTS and FAILS - unintended HARD DELETE. Case (ii) is the dangerous one because it triggers exactly when trash is broken, full, or the target is on a filesystem trash cannot handle - i.e. precisely when a human would want the file kept and recoverable. Violates the fleet rule 'File operations: trash command (never rm/hard delete)' while appearing to honor it, which is why it survived review. The artifacts are complete logical dumps of a live clinical database, so a silent hard delete is unrecoverable data loss with no trace. FIX: real if/else - if command -v trash; then trash "$f" || log-and-skip; else rm -f "$f"; fi - and decide explicitly whether a trash failure should abort the prune rather than escalate to rm. MUST-FAIL FIXTURE REQUIRED: a stub 'trash' on PATH that exits 1 must NOT result in the file being removed; the current code deletes it. Same class as mem:reference-gates-need-a-must-fail-fixture. Blocked while venus-db-backup.sh is evidence in the live VENUS-350 P0. Sibling of VENUS-358 (same function, one push once unblocked).
Activity
-
Blocked by live P0 VENUS-350: same file as VENUS-358, same reason. Fix both in one push once released.
-
RELEASED with VENUS-358, same file, same push. This is the one that matters - trash||rm precedence hard-deletes complete logical dumps of a live clinical database exactly when trash EXISTS and FAILS, which is precisely when a human would want the file kept. MUST-FAIL FIXTURE IS A CONDITION OF THE FIX, not an extra: a stub 'trash' on PATH exiting 1 must leave the file present. Also decide explicitly whether a trash failure aborts the prune rather than escalating to rm - my ruling is abort and log, because a prune that cannot prune safely should stop, not delete.
-
FIXED in venus-db-backup.sh v1.6, sh.git 36624b6. The precedence bug is gone: remove_artifact() is a real if/else - trash present and FAILING now logs the reason and returns non-zero, prune_lane() aborts the whole lane on that, and nothing is hard-deleted. Absent trash still falls back to rm (the intended case (i)) but now logs loudly, since trash missing from PATH on venus is itself an anomaly. Implemented pmmaster's ruling exactly: abort and log, do not escalate. Two things the code revealed that the ticket did not name. (a) THE ORIGINAL LOOP WAS A PIPELINE, so 'while read' ran in a SUBSHELL - an abort inside it could not have stopped the caller and the loop would have carried on deleting after the first refusal. Rewritten as mapfile + a plain for loop; without that, the abort would have been decorative. (b) 'trash 2>/dev/null' discarded the one piece of evidence saying WHY the removal failed, on the branch that now decides whether to stop; stderr is captured into the log. MUST-FAIL FIXTURE SHIPPED as venus-db-backup-prune-test.sh v1.0 (14 assertions, all passing). It includes a CONTROL that runs the same stub against the v1.5 line and asserts the file IS destroyed - without it, test 1 passing would not be attributable to the fix. Also a drift check that greps the shipped script for the decision-carrying lines, because the fixture necessarily tests a COPY: venus-db-backup.sh dumps a live clinical DB from its first line and has no dry-run, so the copy is stated rather than hidden and the drift check is what keeps it honest. THE FIXTURE FAILED THREE TIMES BEFORE IT PASSED, and each failure was in the fixture: a success stub that exited 0 without removing anything made every happy-path assertion fail against correct code; 'set -o pipefail' killed the run on a glob whose NON-match was the passing condition; and the drift detector flagged the defect quoted inside the comment explaining why it was wrong - a detector that cannot tell a defect from its own postmortem would have forced the fix to delete its own rationale.