fix(clawhub): prevent temp workspace from chmod'ing shared /tmp to 0700 by chenxiaoyu209 · Pull Request #101361 · openclaw/openclaw

chenxiaoyu209 · 2026-07-07T04:42:11Z

What Problem This Solves

Fixes #101224 — Running openclaw skills install silently changes
/tmp permissions from 1777 to 0700, breaking every service that
depends on world-writable /tmp (MySQL, PHP-FPM, bt-monitor, system
tools, etc.). The issue is recurring when skill updates run on a
schedule.

Why This Change Was Made

The root cause is that several ClawHub download functions in src/infra/clawhub.ts
explicitly pass tmpDir: os.tmpdir() to createTempDownloadTarget.
The underlying @openclaw/fs-safe temp workspace helper calls
ensurePrivateDirectory(root, 0o700), which unconditionally chmods
the root directory. When root is /tmp, the shared system directory
gets chmod'd to 0700.

The fix is simple: remove the explicit os.tmpdir() overrides, letting
createTempDownloadTarget and withTempDir use their existing safe
default resolvePreferredOpenClawTmpDir(), which resolves to the
OpenClaw-owned /tmp/openclaw directory.

User Impact

Before: openclaw skills install could break /tmp permissions,
silently causing other services to fail with "permission denied" errors.
Recovery required terminal-level chmod 1777 /tmp knowledge.
After: Skill installs/updates use the OpenClaw-owned /tmp/openclaw
temp root. /tmp permissions are never touched.

Evidence

Source repro: Verified source-level root cause in both
@openclaw/fs-safe (external) and the 5 ClawHub call sites.
Tests:
- pnpm test src/infra/install-source-utils.test.ts — 17/17 passed
  (includes new regression test verifying temp dirs are under the
  OpenClaw-owned root)
- pnpm test src/infra/clawhub.test.ts — 66/68 passed (2 skipped, pre-existing)
- pnpm test src/infra/tmp-openclaw-dir.test.ts — 22/22 passed
Upstream follow-up: @openclaw/fs-safe (https://github.com/openclaw/fs-safe)
should also fix ensurePrivateDirectory to not chmod pre-existing
directories, but this PR provides the immediate fix in OpenClaw itself.

Fixes #101224

🤖 Generated with Claude Code

The PR refactored pre-flight setup inside the try block, and the catch block no longer references jobTimeoutMs (it cannot, due to block scoping). executeJobCoreWithTimeout internally computes its own timeout via resolveCronJobTimeoutMs, so this variable was declared but never read. Fixes TS6133 error in check-prod-types. Co-Authored-By: Claude <[email protected]>

When createTempDownloadTarget is called with tmpDir: os.tmpdir(), the @openclaw/fs-safe tempWorkspace helper calls ensurePrivateDirectory which unconditionally chmods the root directory to 0700. Passing /tmp as rootDir causes the shared system /tmp to be chmod'd, breaking all other services that depend on world-writable /tmp (MySQL, PHP-FPM, etc.). - Remove 5 explicit tmpDir: os.tmpdir() overrides from clawhub.ts download functions, letting createTempDownloadTarget use its safe default resolvePreferredOpenClawTmpDir() which resolves to the OpenClaw-owned /tmp/openclaw directory. - Change withTempDir default rootDir from os.tmpdir() to resolvePreferredOpenClawTmpDir() so all callers that don't pass an explicit rootDir are safe. - Add regression test verifying temp dirs are created under the OpenClaw-owned temp root, not directly under /tmp. Fixes openclaw#101224 Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-07T05:08:49Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 1:13 AM ET / 05:13 UTC.

Summary
The branch changes ClawHub and install-source temp workspace roots to use the OpenClaw-owned temp directory, adds a regression test, and also carries cron timer/job persistence changes.

PR surface: Source -12, Tests +123. Total +111 across 6 files.

Reproducibility: yes. The linked issue has auditd evidence, current main passes raw /tmp into the temp workspace dependency, and the dependency source chmods the selected root directory.

Review metrics: 1 noteworthy metric.

Unrelated runtime surface: 3 cron files changed. The cron timer/job-state changes target a separate issue and need review outside this ClawHub temp-root fix.

Stored data model
Persistent data-model change detected: serialized state: src/cron/service/timer.regression.test.ts, serialized state: src/infra/install-source-utils.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #101224
Summary: This PR is a candidate fix for the open issue reporting that OpenClaw skill install/update can change /tmp permissions from 1777 to 0700; another open PR targets the same issue but is not a safe supersession target yet.

Members:

canonical: Bug: openclaw skills install changes /tmp permissions from 1777 to 0700 #101224 - The PR body uses closing syntax for the issue, and the issue describes the same ClawHub skill install/update /tmp permission mutation.
same_root_cause: fix(security): route temp workspaces through private OpenClaw temp root (#101224) #101246 - This is another open candidate fix for the same /tmp permission mutation, but its current review context shows unrelated-payload blockers.
adjacent_distinct: Cron running_at_ms not cleared after agentTurn run #100471 - The PR branch carries cron commits referencing this separate running-marker issue, but that issue has a different root cause and should not ride along with this fix.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

[P1] Drop or split the cron-service commits so the PR is only the ClawHub/install-source temp-root fix.
[P1] Add redacted live terminal output or logs showing /tmp before and after a real openclaw skills install or update path.
After narrowing, keep the focused infra tests in the PR body so reviewers can connect proof to the changed files.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body lists source review and tests, but it does not show an after-fix live install/update with /tmp permissions preserved; a redacted terminal transcript or logs are needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] Merging the current head would ship cron timer/job-state behavior for Cron running_at_ms not cleared after agentTurn run #100471 inside an unrelated ClawHub /tmp permissions fix.
[P1] The external PR lacks after-fix real behavior proof, so reviewers still need redacted live output showing /tmp remains 1777 after the affected install/update path.

Maintainer options:

Narrow this branch before merge (recommended)
Drop the cron-service commits so this PR contains only the ClawHub/install-source temp-root fix and its proof.
Split the cron work separately
Move the cron due-job persistence changes to a separate PR tied to Cron running_at_ms not cleared after agentTurn run #100471 with its own owner review and validation.
Accept the combined runtime change intentionally
Merge the current head only if maintainers deliberately review and own both the P0 temp-root fix and the separate cron scheduler behavior change together.

Next step before merge

[P1] The PR needs contributor or maintainer follow-up to narrow the branch and add real behavior proof; that proof gap is not something an automated repair lane can supply for the contributor.

Security
Cleared: No new security or supply-chain concern was found in the intended temp-root fix; the main blocker is unrelated runtime scope and missing real behavior proof.

Review findings

[P1] Drop the unrelated cron-service changes — src/cron/service/timer.ts:1257-1268

Review details

Best possible solution:

Keep the private-temp-root ClawHub/install-source fix, split the cron changes into their own PR, and add redacted live stat /tmp before/after proof for a real skill install or update.

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

Yes. The linked issue has auditd evidence, current main passes raw /tmp into the temp workspace dependency, and the dependency source chmods the selected root directory.

Is this the best way to solve the issue?

No for the current branch. The private OpenClaw temp-root approach is the right fix shape, but the branch must drop unrelated cron changes and add after-fix real behavior proof before merge.

Full review comments:

[P1] Drop the unrelated cron-service changes — src/cron/service/timer.ts:1257-1268
This PR is about preventing ClawHub temp workspaces from chmoding shared /tmp, but the branch also changes cron timer/job-state persistence for Cron running_at_ms not cleared after agentTurn run #100471. Current main does not contain those cron changes, so merging this head would ship unrelated scheduled-job behavior inside a P0 temp-permission fix; please split or drop the cron commits before merge.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6192b037bbb5.

Label changes

Label changes:

add P0: The PR is paired with a P0 issue where skill install/update can chmod the shared /tmp directory and break unrelated services.
add merge-risk: 🚨 compatibility: The current head changes cron job persistence behavior in addition to the ClawHub temp-root fix, which could alter existing scheduled-job behavior.
add merge-risk: 🚨 availability: The unrelated cron-service changes affect running-marker cleanup and due-job persistence, a scheduled-job availability path that green CI alone does not settle.
add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists source review and tests, but it does not show an after-fix live install/update with /tmp permissions preserved; a redacted terminal transcript or logs are needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P0: The PR is paired with a P0 issue where skill install/update can chmod the shared /tmp directory and break unrelated services.
merge-risk: 🚨 compatibility: The current head changes cron job persistence behavior in addition to the ClawHub temp-root fix, which could alter existing scheduled-job behavior.
merge-risk: 🚨 availability: The unrelated cron-service changes affect running-marker cleanup and due-job persistence, a scheduled-job availability path that green CI alone does not settle.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists source review and tests, but it does not show an after-fix live install/update with /tmp permissions preserved; a redacted terminal transcript or logs are needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source -12, Tests +123. Total +111 across 6 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	4	78	90	-12
Tests	2	123	0	+123
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	6	201	90	+111

What I checked:

Current main still uses raw /tmp for install-source temp workspaces: withTempDir still defaults rootDir to os.tmpdir() and passes that into the private temp workspace wrapper on current main. (src/infra/install-source-utils.ts:144, 6192b037bbb5)
Current main still passes raw /tmp into ClawHub download staging: Five ClawHub package/skill download paths explicitly pass tmpDir: os.tmpdir() into createTempDownloadTarget, including the package archive path at line 1388 and the skill archive path at line 1478. (src/infra/clawhub.ts:1388, 6192b037bbb5)
Existing safe temp-root seam exists: When callers omit tmpDir, createTempDownloadTarget resolves the temp root through resolvePreferredOpenClawTmpDir(), so the intended ClawHub fix reuses an existing OpenClaw-owned temp-root path. (src/infra/temp-download.ts:23, 6192b037bbb5)
Dependency contract confirms root chmod behavior: Published @openclaw/fs-safe v0.4.1 createTempWorkspace calls ensurePrivateDirectory(root, dirMode), and that helper runs mkdir(..., mode) followed by best-effort chmod(dir, mode) on the selected root. (openclaw/fs-safe/src/private-temp-workspace.ts:78, de07f4f69176)
Branch includes unrelated cron changes: The PR diff includes three cron-service files from commits labeled for a separate cron due-job/running marker problem, not the ClawHub /tmp permissions bug. (src/cron/service/timer.ts:1257, c085a773a0d2)
Related issue remains the canonical user problem: The linked issue is open, P0-labeled, and reports that openclaw skills install can change /tmp from 1777 to 0700; it lists this PR and fix(security): route temp workspaces through private OpenClaw temp root (#101224) #101246 as closing candidates.

Likely related people:

steipete: Authored the fs-safe extraction that introduced OpenClaw's published temp workspace dependency and recently changed ClawHub archive download handling. (role: feature-history owner and recent ClawHub infra contributor; confidence: high; commits: 538605ff44d2, 2655bf4dac8f; files: src/infra/private-temp-workspace.ts, src/infra/temp-download.ts, src/infra/clawhub.ts)
Patrick-Erichsen: Authored the merged GitHub-backed ClawHub skill install feature that uses the affected skill lifecycle and archive extraction path. (role: ClawHub skill install feature contributor; confidence: medium; commits: 8f85f949465a; files: src/skills/lifecycle/clawhub.ts, src/cli/skills-cli.ts, src/infra/clawhub.ts)
vincentkoc: Recent merged work refactored ClawHub lifecycle internals near the affected skill install path. (role: recent ClawHub skills area contributor; confidence: medium; commits: cb1e4356aafc; files: src/skills/lifecycle/clawhub.ts, src/infra/clawhub.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.

chenxiaoyu209 and others added 4 commits July 6, 2026 12:21

fix(cron): persist due-job outcomes incrementally

4d84ceb

Merge branch 'main' into fix/cron-incremental-outcomes-100471

7b71c10

chenxiaoyu209 requested a review from a team as a code owner July 7, 2026 04:42

openclaw-barnacle Bot added the size: M label Jul 7, 2026

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(security): route temp workspaces through private OpenClaw temp root (#101224) #101246

Merged

chenxiaoyu209 closed this Jul 8, 2026

chenxiaoyu209 deleted the fix/tmp-chmod-skills-install-101224 branch July 8, 2026 07:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(clawhub): prevent temp workspace from chmod'ing shared /tmp to 0700#101361

fix(clawhub): prevent temp workspace from chmod'ing shared /tmp to 0700#101361
chenxiaoyu209 wants to merge 4 commits into
openclaw:mainfrom
chenxiaoyu209:fix/tmp-chmod-skills-install-101224

chenxiaoyu209 commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

chenxiaoyu209 commented Jul 7, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading