This commit was merged in pull request #811.
This commit is contained in:
@@ -214,3 +214,85 @@ Re-ran codex again; it found two more rollback-path gaps `set -E` cannot catch.
|
||||
- Gates: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1094 · HARD GATE 193/193 ·
|
||||
rollback 28/28 (was 14, +14 for D1/D2 with controls) · migration 21/21. shellcheck clean on new lines.
|
||||
No --no-verify. Committing FORWARD (no rebase of 34e55d4a/0a5e703a).
|
||||
|
||||
## Session 3 (2026-07-16) — PR1 MERGED, starting PR2 (durable snapshot + restore + secrev)
|
||||
PR1 (#802) squash-merged → main `32a0ffba`; issue #791 stays open (3-PR DAG umbrella). Independent Opus
|
||||
adversarial/security review APPROVED at head `af627e75` (Gitea RoR cmt 17892); lead ran rollback 28/28 +
|
||||
HARD GATE 193/193 green; CI #1877 green. PR2 UNBLOCKED.
|
||||
|
||||
PR2 branch: `feat/791-pr2-snapshot-restore` off `origin/main` 32a0ffba. Same treatment applies:
|
||||
tests-first red-first, independent review + durable Gitea Reviewer-of-Record comment BEFORE MS-LEAD runs
|
||||
the queue guard/merge. Report PR2 number + exact head when ready. PR body: `Part of #791` (NOT Fixes).
|
||||
|
||||
### PR2 scope (ratified §3/§5 of design doc, Mos-approved — do NOT re-litigate)
|
||||
- **(a) Durable pre-update snapshot** to `${XDG_STATE_HOME:-~/.local/state}/mosaic/backups/pre-update-<UTC-ts>/`
|
||||
— OUTSIDE ~/.config/mosaic and any repo. Perms dir 0700 / files 0600 (umask 077 + explicit chmod).
|
||||
Scope = operator-owned surface that EXISTS (operatorReserved paths), not the framework tree. Taken
|
||||
BEFORE any mutation. Retention N=5 (`MOSAIC_BACKUP_RETENTION`), prune older.
|
||||
- **Post-sync verify + selective restore**: diff operator surface vs snapshot; (b) should never touch
|
||||
operator paths, so ANY diff = manifest bug → restore affected paths + warn loudly. (a) catches a (b) miss.
|
||||
- **`mosaic restore`** (TS CLI): `--list` (default, dry-run) enumerates snapshots by ts; `--from <ts>`
|
||||
restores over operator surface, confirmation-gated. Counts/paths only.
|
||||
- **Secret-safety (secrev)**: snapshot/restore NEVER emit file contents; only paths/counts. Tests assert
|
||||
0700/0600 AND that a secret value seeded in tools/_lib/credentials.json never appears in any output.
|
||||
|
||||
### PR2 implementation status (2026-07-16, ready-for-review)
|
||||
All three tasks implemented, red-first proven, unit-green:
|
||||
- **Task #10 — durable snapshot (install.sh)**: `backup_root()`/`enumerate_operator_files()`/
|
||||
`prune_durable_snapshots()`/`make_durable_snapshot()` wired into keep-mode main() after `manifest_load`,
|
||||
before any mutation. umask 077 + explicit chmod 700/600. UTC ts, collision suffix. FAIL-OPEN (a backup
|
||||
failure never aborts the upgrade it protects). Retention `MOSAIC_BACKUP_RETENTION` (default 5), in-place
|
||||
`sort -r -o` prune (no `mv` — stays inside the rsync-absent coreutils whitelist).
|
||||
- **Task #11 — post-sync verify net (install.sh)**: `verify_operator_surface()` runs after sync (trap
|
||||
disarmed), `cmp -s` each snapshot file vs target; restores any diverged/missing operator file + warns
|
||||
loudly (a divergence = manifest bug). VERIFY-NET wired before `cleanup_snapshot`.
|
||||
- **Task #12 — `mosaic restore` (TS)**: `src/commands/restore.ts` + co-located spec (19 tests).
|
||||
`--list` default (dry-run enumerate), `--from <ts>` confirmation-gated restore, `--dry-run`, `--yes`/
|
||||
`MOSAIC_ASSUME_YES`. Injectable `confirm` for testability (proceed/decline/env-bypass covered). Restored
|
||||
files forced 0600. Registered in `cli.ts`. Path convention mirrors install.sh `backup_root()`.
|
||||
- **CI**: `.woodpecker/ci.yml` upgrade-guard runs the new `test-upgrade-durable-snapshot.sh` gate.
|
||||
- **Gates green**: typecheck ✓ lint ✓ format:check ✓ · full mosaic vitest 1241 (+5) ·
|
||||
durable-snapshot 26/26 · manifest-guard 193/193 · rollback 28/28 · migration 21/21.
|
||||
Est. new-code coverage ≈93% (only the interactive readline default + process.exit-on-error uncovered).
|
||||
- Regression fixed: PR2's `date`/`sort`/`mv` broke the rsync-absent manifest-guard PATH whitelist →
|
||||
made date/sort fail-open, replaced `mv` with in-place `sort -o`, added `date sort` to the test whitelist
|
||||
+ isolated `XDG_STATE_HOME`. All 193 manifest-guard assertions green under restricted PATH.
|
||||
- Codex code-review + security-review (secrev) run on the uncommitted diff before commit.
|
||||
|
||||
### PR2 review round 1 — findings + remediations (2026-07-16, pre-PR)
|
||||
Codex code-review returned **request-changes** (1 blocker + 3 should-fix); Codex security-review returned
|
||||
**high** (1 high + 1 medium). Deduped to 5 distinct defects, ALL legitimate, ALL fixed FORWARD, each with
|
||||
a red-first regression test whose control neuters exactly the guard under test:
|
||||
|
||||
- **A · BLOCKER — verify net undid the legacy bin/ migration (install.sh).** On a pre-v2 install `bin/**`
|
||||
is operator-classified, so the durable snapshot captured it; `run_migrations()` deletes bin/ on purpose,
|
||||
but `verify_operator_surface()` then saw it "missing" and healed it back — the migration would be silently
|
||||
undone forever once the version stamps. **Fix:** `MIGRATION_REMOVED_PATHS[]` recorded by run_migrations
|
||||
(`bin`,`rails`) + `is_migration_removed()` skip in the verify loop (`# MIGRATION-SKIP-GUARD`).
|
||||
**Test:** Part 6 — v1 fixture with bin/; shipped keeps it removed + stamps v3; control (guard stripped)
|
||||
wrongly restores bin/tool.sh.
|
||||
- **B · HIGH (CWE-59) — restore/verify wrote secrets THROUGH a symlink (install.sh + restore.ts).** An
|
||||
attacker swapping an operator path (e.g. tools/_lib/credentials.json) for a symlink after the snapshot
|
||||
would make `cp`/`copyFileSync` write the snapshot's secret out through the link. **Fix (bash):** refuse a
|
||||
symlinked ancestor (`has_symlinked_parent`), drop a symlinked leaf before restore
|
||||
(`# SYMLINK-LEAF-GUARD`). **Fix (TS):** reuse audited `secure-file.ts` — `assertCanonicalContainment`
|
||||
+ `ensureManagedDirectory` on every dst, open the leaf `O_NOFOLLOW|O_CREAT|O_TRUNC` 0600 (ELOOP =
|
||||
fail-closed). **Tests:** Part 7 (shipped leaves external exfil target untouched, restores a real 0600
|
||||
file; control leaks the secret through the link) + restore.spec symlinked-leaf/ancestor cases (red-first).
|
||||
- **C · MEDIUM/should-fix (CWE-22) — `--from` traversal escaped the backup root (restore.ts).**
|
||||
`join(root, from)` accepted `../poison`. **Fix:** validate the selector against
|
||||
`^\d{8}T\d{6}Z(?:-\d+)?$`, build exactly `join(root,'pre-update-'+ts)`, `lstat` (reject symlinked snap
|
||||
dir). **Test:** restore.spec `it.each` of 6 malformed selectors + `--from ../poison` fail-closed (red-first).
|
||||
- **D · should-fix — verify `mkdir -p` unguarded under set -e (install.sh).** A parent replaced by a
|
||||
regular file aborted the installer before the recovery pointer printed. **Fix:** guard `mkdir -p`, warn
|
||||
+ `continue` on failure (keeps healing remaining files).
|
||||
- **E · should-fix — snapshot `umask 077` leaked process-global (install.sh).** Later sync copies/dirs
|
||||
inherited 0600/0700. **Fix:** save `old_umask`, restore on EVERY return path (`# UMASK-RESTORE-NORMAL`).
|
||||
**Test:** Part 8 — synced framework file is 0644 while the secret backup stays 0600; control (restore
|
||||
stripped) makes the synced file 0600.
|
||||
|
||||
**Full gate suite re-run after fixes (all green):** typecheck ✓ · lint ✓ · format:check ✓ · full mosaic
|
||||
vitest **1252** · restore.spec **30** · durable-snapshot **41** · manifest-guard 193 · rollback 28 ·
|
||||
migration 21. shellcheck clean on all new lines; new test markers mirror the existing `# VERIFY-NET`
|
||||
anchor convention. NOTE: codex self-review does NOT satisfy the independent-review gate — an independent
|
||||
(author≠reviewer) review + durable Gitea Reviewer-of-Record comment is still required before MS-LEAD merges.
|
||||
|
||||
Reference in New Issue
Block a user