Fix backup verifier for root-relative hardlink targets by abnershang · Pull Request #89328 · openclaw/openclaw

abnershang · 2026-06-02T04:50:37Z

Summary

Accept older backup hardlink targets that are stored relative to the archive root when they resolve to an existing entry inside the archive.
Keep absolute, backslash, and traversal hardlink validation intact before the compatibility path.
Add regression coverage for the older internal hardlink shape reported in openclaw backup create --verify exits 13, leaves corrupt .tmp archive; verifier also rejects older backup hardlinks #89257.

AI-assisted: yes, prepared with Codex and checked with codex review before submission.

Verification

node scripts/run-vitest.mjs src/commands/backup-verify.test.ts src/infra/backup-create.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.atomic.test.ts src/cli/program/register.backup.test.ts
git diff --check -- src/commands/backup-verify.ts src/commands/backup-verify.test.ts
pnpm build
codex review --base origin/main

Real behavior proof

Behavior addressed: backup verify now accepts older backup archives whose internal hardlink targets are recorded relative to the archive payload/root, while preserving rejection for unsafe or missing hardlink targets.

Real environment tested: Source checkout on the PR branch, using a synthetic backup archive that matches the reported older internal hardlink shape. Release-branch reproduction of the verifier failure was confirmed separately before the fix.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/commands/backup-verify.test.ts src/infra/backup-create.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.atomic.test.ts src/cli/program/register.backup.test.ts
git diff --check -- src/commands/backup-verify.ts src/commands/backup-verify.test.ts
pnpm build
codex review --base origin/main

Disposable state smoke:

TMP_ROOT=$(mktemp -d)
mkdir -p "$TMP_ROOT/home" "$TMP_ROOT/state" "$TMP_ROOT/backups" "$TMP_ROOT/state/node_modules/a" "$TMP_ROOT/state/node_modules/b"
printf '{}\n' > "$TMP_ROOT/state/openclaw.json"
printf 'shared\n' > "$TMP_ROOT/state/node_modules/a/file.txt"
ln "$TMP_ROOT/state/node_modules/a/file.txt" "$TMP_ROOT/state/node_modules/b/file.txt"
HOME="$TMP_ROOT/home" OPENCLAW_HOME="$TMP_ROOT/home" OPENCLAW_STATE_DIR="$TMP_ROOT/state" OPENCLAW_CONFIG_PATH="$TMP_ROOT/state/openclaw.json" node openclaw.mjs backup create --output "$TMP_ROOT/backups" --verify --no-include-workspace
find "$TMP_ROOT/backups" -maxdepth 1 -name '*.tmp' -print
rm -rf "$TMP_ROOT"

Evidence after fix: Focused backup tests passed, build passed, Codex review reported no actionable regressions, and the disposable create/verify smoke printed Archive verification: passed with no .tmp files listed.

Observed result after fix: The new verifier regression accepts the older internal hardlink target shape, while existing unsafe and missing hardlink target tests continue to pass.

What was not tested: Full pnpm check && pnpm test; the reporter's actual archive; the reported backup create --verify exit-13 temp archive failure; applying this fix to release/2026.5.28.

clawsweeper · 2026-06-02T04:51:58Z

Codex review: needs maintainer review before merge. Reviewed June 2, 2026, 1:02 AM ET / 05:02 UTC.

Summary
The PR updates backup verification to accept older archive-root-relative hardlink linkpaths only when they resolve to an existing entry, with a focused regression test.

PR surface: Source +6, Tests +33. Total +39 across 2 files.

Reproducibility: yes. Source inspection shows current main rejects a hardlink target that is relative to the archive payload/root, and the PR adds a synthetic tar fixture that exercises that exact path.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] The linked issue also reports a backup create --verify exit-13 and corrupt .tmp archive; this PR is a narrow verifier compatibility fix and should not be treated as resolving that entire report.

Maintainer options:

Decide the mitigation before merge
Land the narrow verifier compatibility fix if maintainer review and CI stay clean, while keeping the linked report open for separate live reproduction or repair of the backup-create temp archive failure.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair lane is needed because no concrete patch defect was found; remaining action is maintainer review, CI, and issue-scope handling.

Security
Cleared: The diff narrows verifier compatibility after existing path normalization and containment checks and does not add dependencies, CI, secrets, downloads, or code-execution surfaces.

Review details

Best possible solution:

Land the narrow verifier compatibility fix if maintainer review and CI stay clean, while keeping the linked report open for separate live reproduction or repair of the backup-create temp archive failure.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main rejects a hardlink target that is relative to the archive payload/root, and the PR adds a synthetic tar fixture that exercises that exact path.

Is this the best way to solve the issue?

Yes for the verifier compatibility bug. The hardlink target validator is the narrow owner of this invariant; backup creation already dereferences hardlinks, so changing create logic would not address older archives.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 16808524cbd3.

Label changes

Label changes:

add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

P1: The PR targets a real backup verification regression tied to a current user backup workflow and a linked data-loss-risk issue.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes structured after-fix proof with focused backup tests, build output, and a disposable backup create/verify smoke reporting Archive verification: passed with no .tmp file listed.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes structured after-fix proof with focused backup tests, build output, and a disposable backup create/verify smoke reporting Archive verification: passed with no .tmp file listed.

Evidence reviewed

PR surface:

Source +6, Tests +33. Total +39 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	10	4	+6
Tests	1	33	0	+33
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	43	4	+39

What I checked:

Repository policy read: The full root AGENTS.md was read and applied; no scoped AGENTS.md owns src/commands or src/infra backup paths, and the only maintainer note found was Telegram-only. (AGENTS.md:1, 16808524cbd3)
Current main still rejects the reported shape: Current main requires each normalized hardlink target to already be within the archive root and present exactly in the archive entries, so a root-relative target such as payload/... is rejected before this PR. (src/commands/backup-verify.ts:304, 16808524cbd3)
PR implementation: The PR prefixes archiveRoot only for hardlink targets that are not already root-prefixed, then still checks containment and exact entry presence before accepting them. (src/commands/backup-verify.ts:304, bd501f9ccacd)
Regression coverage: The PR adds a synthetic tar archive with linkpath payload/posix/... and asserts backupVerifyCommand resolves successfully while adjacent unsafe and missing-target tests remain in place. (src/commands/backup-verify.test.ts:361, bd501f9ccacd)
Caller and sibling path: backupCreateCommand calls backupVerifyCommand for --verify, while current archive creation disables tar hardlink emission with BackupLinkCache, so the compatibility repair belongs in the verifier path for older archives. (src/commands/backup.ts:29, 16808524cbd3)
Shipped behavior context: The 2026.5.28 changelog says shipped backup behavior dereferences hardlinks during creation and rejects unsafe hardlink targets during verification; this PR adds a narrower compatibility acceptance for older archives, not a shipped main fix. (CHANGELOG.md:819, 16808524cbd3)

Likely related people:

shichangs: Authored the local backup CLI commit that added backup verify/create surfaces and tests. (role: introduced behavior; confidence: high; commits: 0ecfd37b4465; files: src/commands/backup-verify.ts, src/commands/backup.ts, src/commands/backup-shared.ts)
gumadeiras: Co-authored/reviewed the original backup CLI work and later hardened backup verify path validation and backup infra helpers. (role: reviewer and hardening contributor; confidence: high; commits: 0ecfd37b4465, 09acbe65289d, 3ba64916599b; files: src/commands/backup-verify.ts, src/infra/backup-create.ts, src/commands/backup-verify.test.ts)
Vincent Koc: Current shallow blame/log for the backup verifier and backup-create files points at a recent broad refactor touching this area. (role: recent area contributor; confidence: medium; commits: 340cc2c1e41c; files: src/commands/backup-verify.ts, src/infra/backup-create.ts, src/commands/backup-verify.test.ts)
Peter Steinberger: Recent history shows multiple backup command/test refactors and the latest release commit touching the same backup verification and creation files. (role: recent adjacent contributor; confidence: medium; commits: e93216080aa1, b8451e26a398, f97b1fa0c3a3; files: src/commands/backup-verify.ts, src/infra/backup-create.ts, src/commands/backup-verify.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

RomneyDa

Thanks for the contribution

Follow-up to openclaw#89328. node-tar records a deduplicated hardlink's linkpath as the cwd-relative source path (e.g. .openclaw/state/db), so prepending only the archive-root segment still misses the real <root>/payload/posix/... target and older backups keep failing verify. node-tar dedupes only files under a shared cwd, so the target is <ancestor-of-link>/<linkpath>: walk the link entry's ancestor dirs and require an exact entry match. This also covers the root-relative shape openclaw#89328 added and avoids a loose suffix scan that let an unrelated same-named file mask a dropped/corrupt target.

fix(backup): accept root-relative hardlink targets

bd501f9

openclaw-barnacle Bot added commands Command implementations size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 2, 2026

openclaw-barnacle Bot removed the triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. label Jun 2, 2026

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 2, 2026

hoobnn mentioned this pull request Jun 2, 2026

fix(backup): resolve legacy hardlink targets by link subtree (follow-up to #89328) #89345

Open

RomneyDa approved these changes Jun 2, 2026

View reviewed changes

RomneyDa merged commit 5be282e into openclaw:main Jun 2, 2026
218 of 232 checks passed

github-actions Bot mentioned this pull request Jun 2, 2026

📡 Upstream Digest — 2026-06-02 08:21 UTC curtismercier/openclaw-mods#1001

Open

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 3, 2026

fix(backup): accept root-relative hardlink targets (openclaw#89328)

a9c485b

SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026

fix(backup): accept root-relative hardlink targets (openclaw#89328)

180bbe0

sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026

fix(backup): accept root-relative hardlink targets (openclaw#89328)

4afd4ad

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix backup verifier for root-relative hardlink targets#89328

Fix backup verifier for root-relative hardlink targets#89328
RomneyDa merged 1 commit into
openclaw:mainfrom
abnershang:codex/issue-89257-backup-repro

abnershang commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

RomneyDa left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

abnershang commented Jun 2, 2026

Summary

Verification

Real behavior proof

Uh oh!

clawsweeper Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RomneyDa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading