fix(update): confine rollback targets to backup roots - #3542
Open
tang-vu wants to merge 1 commit into
Open
Conversation
This was referenced Aug 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ods-update.sh rollback <target>input before backup lookupmake testWhy this matters
ods-update.sh rollback ../externalpreviously resolveddata/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_rollbackjoined the raw target toROLLBACK_DIRandBACKUP_DIRbefore 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 changingods/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 outerods-clirollback 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
../externalcompleted rollbackbash tests/test-update-rollback-target-boundary.sh? 5 passedbash tests/test-rollback-compose-stack.sh? all 7 assertions passedbash tests/test-update-rollback-contract.sh? full backup?mutation?rollback contract passedmake lintshellcheck --rcfile=/dev/null --severity=error ods-update.sh tests/test-update-rollback-target-boundary.shgit diff --checkThe 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/mainplus #3540, #3541, and #3542 merged without conflicts. It passed all three new boundary suites, the adjacent preset/backup/rollback suites,make lint, andgit diff --check. The PRs change independent command boundaries; no merge order is required.CI receipt
Head
a44e79998f011377ce46826a869576b6e712bfd0completed 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.