Skip to content

fix(backup): dereference archive hardlinks#83938

Merged
steipete merged 3 commits into
openclaw:mainfrom
jason-allen-oneal:fix/issue-54242
May 23, 2026
Merged

fix(backup): dereference archive hardlinks#83938
steipete merged 3 commits into
openclaw:mainfrom
jason-allen-oneal:fix/issue-54242

Conversation

@jason-allen-oneal

@jason-allen-oneal jason-allen-oneal commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • disable tar hardlink emission during backup creation so hardlinked files are archived as regular entries
  • include hardlink targets in backup verification and reject traversal/absolute targets
  • add regression coverage for hardlinked backup payloads and unsafe Link headers

Verification

  • pnpm exec oxfmt --check --threads=1 src/infra/backup-create.ts src/infra/backup-create.test.ts src/commands/backup-verify.ts src/commands/backup-verify.test.ts src/commands/backup.ts src/commands/backup.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.atomic.test.ts CHANGELOG.md
  • node scripts/run-vitest.mjs src/infra/backup-create.test.ts src/commands/backup-verify.test.ts src/commands/backup.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.atomic.test.ts
  • pnpm check:changed
  • git diff --check

Not Run

  • Live macOS bsdtar restore smoke; this environment is Linux.

Real behavior proof

  • Behavior or issue addressed: Backup archives should dereference source hardlinked files so a backup containing two hardlinked source paths stores two regular file entries under the archive root, not a tar hardlink entry whose link target can be outside or invalid after archive path remapping.
  • Real environment tested: Linux 6.19.14+kali-amd64, Node v22.22.2, pnpm 10.33.2; isolated worktree at PR head 53802c6a93f6; real filesystem hardlink created with fs.link(...) inside an isolated OPENCLAW_HOME/OPENCLAW_STATE_DIR.
  • Exact steps or command run after this patch:
    1. Prepared the isolated PR worktree with git fetch origin pull/83938/head:refs/heads/proof-83938 and git worktree add /tmp/openclaw-proof-83938 proof-83938.
    2. Ran the real backup/tar smoke: node --import /home/rev/projects/openclaw/node_modules/tsx/dist/loader.mjs /tmp/openclaw-proof-83938-smoke.mjs.
    3. Cross-checked the generated archive with system tar: tar -tvf /tmp/openclaw-backup-hardlink-proof-zGSJ13/out/hardlink-proof.tar.gz.
  • Evidence after fix: Terminal output from the smoke command:
    source_hardlink_check same_inode=true nlink_original=2 nlink_alias=2
    Backup archive: /tmp/openclaw-backup-hardlink-proof-zGSJ13/out/hardlink-proof.tar.gz
    Archive verification: passed
    create_result archive=/tmp/openclaw-backup-hardlink-proof-zGSJ13/out/hardlink-proof.tar.gz verified=true assetCount=1
    verify_result ok=true archiveRoot=2026-05-19T00-00-00.000Z-openclaw-backup entries=8
    archive_hardlink_entry_count=0
    archive_original_entry type=File linkpath=none size=35
    archive_alias_entry type=File linkpath=none size=35
    extract_content_equal=true content="{"event":"hardlink-proof","seq":1}"
    extract_inode_check same_inode=false nlink_original=1 nlink_alias=1
    
    System tar listed both hardlink-alias.dat and hardlink-original.dat as -rw-r--r-- regular file entries with size 35, with no link to hardlink entry.
  • Observed result after fix: The smoke command exited 0. The source files were real hardlinks with the same inode and nlink=2. The archive contained no tar hardlink entries, both source names were archived as regular File entries with no linkpath, OpenClaw backup verification passed, and extraction produced equal file contents but separate inodes with nlink=1.
  • What was not tested: Did not test Windows/macOS filesystems, the restore command/UI, --include-workspace custom workspace assets, concurrent live writers, or very large backup payloads.

Fixes #54242

Current-head proof refresh (2026-05-19)

Reran the hardlink backup proof at current PR head 04ee028b52077175c3d732e70e1574d45bd9ef6c.

Real hardlink smoke output summary:

{
  "pr": 83938,
  "head": "04ee028b52077175c3d732e70e1574d45bd9ef6c",
  "hardlinkFixture": {
    "sameDevice": true,
    "sameInode": true,
    "nlinkTarget": 2,
    "nlinkHardlink": 2
  },
  "archive": {
    "totalEntries": 15,
    "linkEntryCount": 0,
    "esbuildPayloadEntries": [
      { "path": "payload/.../node_modules/esbuild/bin/esbuild", "type": "File", "linkpath": null },
      { "path": "payload/.../node_modules/@esbuild/darwin-arm64/bin/esbuild", "type": "File", "linkpath": null }
    ]
  },
  "verification": { "ok": true, "reason": null }
}

Commands run at current head:

  • Hardlink smoke via node --import tsx using createBackupArchive(...) + backupVerifyCommand(...) -> source paths were true hardlinks (sameInode: true, nlink: 2), archive emitted 0 Link entries, both payload entries were regular File entries, verification passed.
  • node scripts/run-vitest.mjs src/infra/backup-create.test.ts src/commands/backup-verify.test.ts src/commands/backup.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.restore.test.ts -> 4 test files passed, 45 tests passed.
  • git diff --check -> passed.

Not run: live macOS bsdtar restore smoke; this host is Linux.

Copilot AI review requested due to automatic review settings May 19, 2026 02:51
@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR disables node-tar hardlink emission during backup creation, validates hardlink targets during backup verify, adds hardlink regression tests, and records the fix in the changelog.

Reproducibility: yes. The current-main source plus node-tar 7.5.15 source show a high-confidence reproduction path with hardlinked files in a backup source: node-tar can emit Link entries while OpenClaw remaps only entry.path, and the linked issue supplies the macOS bsdtar failure shape.

PR rating
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Summary: The PR has strong terminal proof and a focused implementation, with normal maintainer review needed for the archive-size compatibility tradeoff.

Rank-up moves:

  • Add macOS bsdtar broad-extraction proof only if maintainers require exact reporter-platform coverage before merge.
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.

Real behavior proof
Sufficient (terminal): The PR body includes terminal proof using real filesystem hardlinks, showing zero Link entries, regular File entries for both paths, successful verification, and separate extracted inodes; the later merge did not change the backup runtime files.

Risk before merge

  • Dereferencing hardlinks can make hardlink-dense backups larger and increase archive write/read I/O, especially for dependency trees or large workspaces.
  • Existing archives with unsafe or missing hardlink targets that previously passed backup verify will now fail verification; that is likely the desired safety behavior, but it is still user-visible stricter validation.
  • The supplied real behavior proof is Linux-based; the exact reporter-platform macOS bsdtar broad-extraction path was not rerun.

Maintainer options:

  1. Accept restore-safe regular file entries (recommended)
    Maintainers can accept larger hardlink-dense archives in exchange for archives that avoid restore-hostile hardlink targets and stricter verification of old Link entries.
  2. Ask for macOS bsdtar smoke first
    If exact reporter-platform proof is required, request a current-head macOS broad-extraction smoke showing the generated archive extracts without the Path contains '..' failure.
  3. Pause if archive-size growth is unacceptable
    If preserving tar hardlink compaction is a product requirement, pause this PR and design a safer hardlink target rewrite instead of unconditional dereferencing.

Next step before merge
No automated repair is indicated; the remaining action is maintainer acceptance of the archive-size/strict-verification compatibility tradeoff and any optional macOS proof request.

Security
Cleared: No security or supply-chain regression found; the patch narrows backup archive trust by rejecting unsafe hardlink targets and does not change dependencies, workflows, scripts, or secret handling.

Review details

Best possible solution:

Land the focused backup fix if maintainers accept the larger-archive tradeoff, keeping the hardlink regression tests as the contract for future tar upgrades.

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

Yes. The current-main source plus node-tar 7.5.15 source show a high-confidence reproduction path with hardlinked files in a backup source: node-tar can emit Link entries while OpenClaw remaps only entry.path, and the linked issue supplies the macOS bsdtar failure shape.

Is this the best way to solve the issue?

Yes. Disabling the node-tar hardlink cache for backup creation and validating any remaining Link targets during verification is a narrow fix for the restore-safety bug; the main tradeoff is larger hardlink-dense archives.

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof using real filesystem hardlinks, showing zero Link entries, regular File entries for both paths, successful verification, and separate extracted inodes; the later merge did not change the backup runtime files.

Label justifications:

  • P2: This fixes a concrete backup/restore correctness problem with limited CLI backup surface and focused tests.
  • merge-risk: 🚨 compatibility: The PR intentionally changes archive shape by storing hardlinked source files as separate regular entries and makes verification stricter for existing archives with unsafe hardlink targets.
  • rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🦞 diamond lobster, patch quality is 🐚 platinum hermit, and The PR has strong terminal proof and a focused implementation, with normal maintainer review needed for the archive-size compatibility tradeoff.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof using real filesystem hardlinks, showing zero Link entries, regular File entries for both paths, successful verification, and separate extracted inodes; the later merge did not change the backup runtime files.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof using real filesystem hardlinks, showing zero Link entries, regular File entries for both paths, successful verification, and separate extracted inodes; the later merge did not change the backup runtime files.

Acceptance criteria:

  • node scripts/run-vitest.mjs src/infra/backup-create.test.ts src/commands/backup-verify.test.ts src/commands/backup.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.restore.test.ts
  • git diff --check
  • Optional: macOS system tar broad extraction smoke for a hardlink-heavy backup archive

What I checked:

  • Current main emits hardlinks before remapping paths: On current main, createBackupArchive calls tar.c with portable, preservePaths, filter, and onWriteEntry remapping, but no custom linkCache, so node-tar can still choose Link entries before OpenClaw remaps archive paths. (src/infra/backup-create.ts:540, a00e7d3898cf)
  • Current main verifier ignores hardlink targets: On current main, backup verification lists only entry.path and then verifies manifest payloads; it does not inspect entry.type or entry.linkpath, so unsafe tar hardlink targets are outside the current verification contract. (src/commands/backup-verify.ts:172, a00e7d3898cf)
  • PR disables node-tar hardlink cache for backup archives: At PR head, BackupLinkCache makes get always return undefined and set a no-op, and createBackupArchive passes it as linkCache, causing hardlinked source files to be written as regular file entries. (src/infra/backup-create.ts:28, 7191d5cb59da)
  • PR validates hardlink targets in backup verify: At PR head, verification records path, linkpath, and type, normalizes Link targets, requires them inside the declared archive root, and requires the target entry to exist. (src/commands/backup-verify.ts:175, 7191d5cb59da)
  • Regression coverage exercises real fs hardlinks and unsafe Link headers: The PR adds a create-side test using fs.link and asserting zero Link entries, plus verify-side tar fixtures rejecting traversal and missing hardlink targets. (src/infra/backup-create.test.ts:428, 7191d5cb59da)
  • node-tar dependency contract supports the fix shape: tar 7.5.15 documents linkCache as an advanced/internal Map for files with nlink > 1; its WriteEntry source checks stat.nlink > 1, reads linkCache.get, emits type = 'Link' when a cached path is found, and otherwise calls linkCache.set. ([email protected]/dist/commonjs/write-entry.js:310)

Likely related people:

  • shichangs: Introduced the local backup CLI and initial backup verification surface in feat: add local backup CLI. (role: feature introducer; confidence: high; commits: 0ecfd37b4465; files: src/commands/backup.ts, src/commands/backup-verify.ts, src/commands/backup-verify.test.ts)
  • Gustavo Madeira Santana: Reviewed the original backup CLI commit and authored the verify path-hardening and backup helper extraction commits that shaped this surface. (role: reviewer and hardening/refactor contributor; confidence: high; commits: 0ecfd37b4465, 09acbe65289d, 3ba64916599b; files: src/commands/backup-verify.ts, src/infra/backup-create.ts, src/commands/backup.ts)
  • Peter Steinberger: Recent history and shortlog show repeated backup command, verification, and test refactors around this area. (role: recent area contributor; confidence: high; commits: c2096897bb23, b8451e26a398, f97b1fa0c3a3; files: src/commands/backup-verify.ts, src/commands/backup-verify.test.ts, src/commands/backup.test.ts)

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 19, 2026
@jason-allen-oneal

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: M and removed size: S proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 19, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 19, 2026
@jason-allen-oneal

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 19, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 19, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🥚 common Gilded Lint Imp

       _..------.._          
    .-'  .-.  .-.  '-.       
   /    ( * )( * )    \      
  |        .--.        |     
  |   <\   ====   />   |     
   \    '.______.'    /      
    '-._   ____   _.-'       
        `-.____.-'           
       __/|_||_|\__          
      /__.'    '.__\         
       .-----------.         
      '-------------'        

Rarity: 🥚 common.
Trait: guards the happy path.
Image traits: location green-check meadow; accessory review stamp; palette seafoam, black, and opal; mood watchful; pose guarding a tiny green check; shell soft speckled shell; lighting moonlit rim light; background subtle branch markers.
How to hatch it: once this PR reaches status: 👀 ready for maintainer look or status: 🚀 automerge armed, the PR author or a maintainer can comment @clawsweeper hatch to turn this ASCII egg into its generated creature image.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Gilded Lint Imp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchable usually means sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@steipete steipete self-assigned this May 23, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 23, 2026
@steipete
steipete merged commit 846f566 into openclaw:main May 23, 2026
11 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Landed via temp rebase onto main.

  • Focused proof: node scripts/run-vitest.mjs src/infra/backup-create.test.ts src/commands/backup-verify.test.ts src/commands/backup.test.ts src/commands/backup.create-verify.test.ts src/commands/backup.atomic.test.ts src/commands/backup.restore.test.ts -> 5 files passed, 49 tests passed.
  • Sanity: git diff --check temp/landpr-83938...HEAD -> passed.
  • Format: pnpm exec oxfmt --check --threads=1 src/infra/backup-create.ts src/infra/backup-create.test.ts src/commands/backup-verify.ts src/commands/backup-verify.test.ts CHANGELOG.md -> passed; after the changelog thanks fixup, pnpm exec oxfmt --check --threads=1 CHANGELOG.md -> passed.
  • PR head pushed: 1917532.
  • Landed on main: d8a2cd5, 77d1157, 846f566.

Thanks @jason-allen-oneal!

SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 24, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
SebTardif pushed a commit to SebTardif/openclaw that referenced this pull request May 26, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
SYU8384 pushed a commit to SYU8384/openclaw that referenced this pull request Jun 3, 2026
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: backup archives can contain hardlink targets with '..' that fail broad extraction on macOS tar

3 participants