Skip to content

fix(update): confine rollback targets to backup roots - #3542

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/update-rollback-target-confinement
Open

fix(update): confine rollback targets to backup roots#3542
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/update-rollback-target-confinement

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate explicit ods-update.sh rollback <target> input before backup lookup
  • reject path separators, dot segments, and control characters before any Docker or file mutation
  • preserve named general backups containing spaces and wire the regression into make test

Why this matters

ods-update.sh rollback ../external previously resolved data/backups/../external, treated that sibling directory as a legacy backup, stopped the live stack, copied its files into the install, and restarted services. A typo or path-like target could therefore replace live configuration from outside both supported backup roots. Explicit rollback targets must be identifiers, not paths.

Root cause and invariant

cmd_rollback joined the raw target to ROLLBACK_DIR and BACKUP_DIR before validating its shape. The invariant is that an explicit target is one path segment and is rejected before lifecycle mutation. Automatic latest-snapshot selection is unchanged.

Overlap check

Searched open and closed PRs for rollback target, rollback path, rollback traversal, backup ID rollback, and PRs changing ods/ods-update.sh. No PR constrains the explicit target. PRs #3338 (backup listing), #3347 (rotation path tokenization), #3379 (compose flag parsing), #3386 (Hermes token migration), and #3502 (health service iteration) are independent stages; #3504 changes the outer ods-cli rollback restart path. Pairwise synthetic merges with all of them plus #3247 were conflict-free. A combined synthetic head with tang-vu PRs #3347 and #3386 plus #3504 passed the new target suite and all affected backup/rollback/Hermes suites.

Validation

  • Red repro on upstream main: boundary test failed because ../external completed rollback
  • bash tests/test-update-rollback-target-boundary.sh ? 5 passed
  • bash tests/test-rollback-compose-stack.sh ? all 7 assertions passed
  • bash tests/test-update-rollback-contract.sh ? full backup?mutation?rollback contract passed
  • synthetic integration: boundary 5/5, rollback stack 7/7, update backup 6/6, Hermes migration 2/2
  • make lint
  • shellcheck --rcfile=/dev/null --severity=error ods-update.sh tests/test-update-rollback-target-boundary.sh
  • git diff --check

The batch-level make gate, dashboard lint/build, and all-files pre-commit results are recorded in PR #3540. The only gate blockers were pre-existing Windows checkout CRLF/private-key fixture/tool-environment issues; focused checks for this diff pass.

Tradeoffs and rollback

Named general backups with spaces remain supported because spaces are valid inside one segment. Backslashes are rejected consistently for Windows/WSL portability. No snapshot format, lookup order, automatic selection, or Docker behavior changes for valid targets. Reverting restores permissive path joining.

Generated with Codex

Batch compatibility update

A synthetic integration head built from upstream/main plus #3540, #3541, and #3542 merged without conflicts. It passed all three new boundary suites, the adjacent preset/backup/rollback suites, make lint, and git diff --check. The PRs change independent command boundaries; no merge order is required.

CI receipt

Head a44e79998f011377ce46826a869576b6e712bfd0 completed with 28 successful checks, 4 conditionally skipped Claude jobs, 0 pending checks, and 0 failures. GitHub reports the diff mergeable; branch protection is waiting only for required human review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant