From b1bbf3fff16bb66f9a96a4824017c28c312942e3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 21:59:55 +0000 Subject: [PATCH] fix: harden temp dir perms for umask 0002 (landed from #27860 by @stakeswky) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 不做了睡大觉 --- CHANGELOG.md | 1 + src/infra/tmp-openclaw-dir.test.ts | 97 ++++++++++++++++++++++++++++++ src/infra/tmp-openclaw-dir.ts | 39 +++++++++++- 3 files changed, 135 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85fa1ca62a6..2e86ad71394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - Delivery queue/recovery backoff: prevent retry starvation by persisting `lastAttemptAt` on failed sends and deferring recovery retries until each entry's `lastAttemptAt + backoff` window is eligible, while continuing to recover ready entries behind deferred ones. Landed from contributor PR #27710 by @Jimmy-xuzimo. Thanks @Jimmy-xuzimo. - Google Chat/Lifecycle: keep Google Chat `startAccount` pending until abort in webhook mode so startup is no longer interpreted as immediate exit, preventing auto-restart loops and webhook-target churn. (#27384) thanks @junsuwhy. +- Temp dirs/Linux umask: force `0700` permissions after temp-dir creation and self-heal existing writable temp dirs before trust checks so `umask 0002` installs no longer crash-loop on startup. Landed from contributor PR #27860 by @stakeswky. (#27853) Thanks @stakeswky. - Microsoft Teams/File uploads: acknowledge `fileConsent/invoke` immediately (`invokeResponse` before upload + file card send) so Teams no longer shows false "Something went wrong" timeout banners while upload completion continues asynchronously; includes updated async regression coverage. Landed from contributor PR #27641 by @scz2011. - Queue/Drain/Cron reliability: harden lane draining with guaranteed `draining` flag reset on synchronous pump failures, reject new queue enqueues during gateway restart drain windows (instead of silently killing accepted tasks), add `/stop` queued-backlog cutoff metadata with stale-message skipping (while avoiding cross-session native-stop cutoff bleed), and raise isolated cron `agentTurn` outer safety timeout to avoid false 10-minute timeout races against longer agent session timeouts. (#27407, #27332, #27427) - Typing/Main reply pipeline: always mark dispatch idle in `agent-runner` finalization so typing cleanup runs even when dispatcher `onIdle` does not fire, preventing stuck typing indicators after run completion. (#27250) Thanks @Sid-Qin. diff --git a/src/infra/tmp-openclaw-dir.test.ts b/src/infra/tmp-openclaw-dir.test.ts index f3e3fe36299..4c0a68b9037 100644 --- a/src/infra/tmp-openclaw-dir.test.ts +++ b/src/infra/tmp-openclaw-dir.test.ts @@ -27,12 +27,16 @@ function resolveWithMocks(params: { lstatSync: NonNullable; fallbackLstatSync?: NonNullable; accessSync?: NonNullable; + chmodSync?: NonNullable; + warn?: NonNullable; uid?: number; tmpdirPath?: string; }) { const uid = params.uid ?? 501; const fallbackPath = fallbackTmp(uid); const accessSync = params.accessSync ?? vi.fn(); + const chmodSync = params.chmodSync ?? vi.fn(); + const warn = params.warn ?? vi.fn(); const wrappedLstatSync = vi.fn((target: string) => { if (target === POSIX_OPENCLAW_TMP_DIR) { return params.lstatSync(target); @@ -50,10 +54,12 @@ function resolveWithMocks(params: { const tmpdir = vi.fn(() => params.tmpdirPath ?? "/var/fallback"); const resolved = resolvePreferredOpenClawTmpDir({ accessSync, + chmodSync, lstatSync: wrappedLstatSync, mkdirSync, getuid, tmpdir, + warn, }); return { resolved, accessSync, lstatSync: wrappedLstatSync, mkdirSync, tmpdir }; } @@ -208,4 +214,95 @@ describe("resolvePreferredOpenClawTmpDir", () => { expect(resolved).toBe(fallbackTmp()); expect(mkdirSync).toHaveBeenCalledWith(fallbackTmp(), { recursive: true, mode: 0o700 }); }); + + it("repairs fallback directory permissions after create when umask makes it group-writable", () => { + const fallbackPath = fallbackTmp(); + let fallbackMode = 0o40775; + const lstatSync = vi.fn>(() => { + throw nodeErrorWithCode("ENOENT"); + }); + const fallbackLstatSync = vi + .fn>() + .mockImplementationOnce(() => { + throw nodeErrorWithCode("ENOENT"); + }) + .mockImplementation(() => ({ + isDirectory: () => true, + isSymbolicLink: () => false, + uid: 501, + mode: fallbackMode, + })); + const chmodSync = vi.fn((target: string, mode: number) => { + if (target === fallbackPath && mode === 0o700) { + fallbackMode = 0o40700; + } + }); + + const resolved = resolvePreferredOpenClawTmpDir({ + accessSync: vi.fn((target: string) => { + if (target === "/tmp") { + throw new Error("read-only"); + } + }), + lstatSync: vi.fn((target: string) => { + if (target === POSIX_OPENCLAW_TMP_DIR) { + return lstatSync(target); + } + if (target === fallbackPath) { + return fallbackLstatSync(target); + } + return secureDirStat(501); + }), + mkdirSync: vi.fn(), + chmodSync, + getuid: vi.fn(() => 501), + tmpdir: vi.fn(() => "/var/fallback"), + warn: vi.fn(), + }); + + expect(resolved).toBe(fallbackPath); + expect(chmodSync).toHaveBeenCalledWith(fallbackPath, 0o700); + }); + + it("repairs existing fallback directory when permissions are too broad", () => { + const fallbackPath = fallbackTmp(); + let fallbackMode = 0o40775; + const chmodSync = vi.fn((target: string, mode: number) => { + if (target === fallbackPath && mode === 0o700) { + fallbackMode = 0o40700; + } + }); + const warn = vi.fn(); + + const resolved = resolvePreferredOpenClawTmpDir({ + accessSync: vi.fn((target: string) => { + if (target === "/tmp") { + throw new Error("read-only"); + } + }), + lstatSync: vi.fn((target: string) => { + if (target === POSIX_OPENCLAW_TMP_DIR) { + throw nodeErrorWithCode("ENOENT"); + } + if (target === fallbackPath) { + return { + isDirectory: () => true, + isSymbolicLink: () => false, + uid: 501, + mode: fallbackMode, + }; + } + return secureDirStat(501); + }), + mkdirSync: vi.fn(), + chmodSync, + getuid: vi.fn(() => 501), + tmpdir: vi.fn(() => "/var/fallback"), + warn, + }); + + expect(resolved).toBe(fallbackPath); + expect(chmodSync).toHaveBeenCalledWith(fallbackPath, 0o700); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("tightened permissions on temp dir")); + }); }); diff --git a/src/infra/tmp-openclaw-dir.ts b/src/infra/tmp-openclaw-dir.ts index 870720b55f8..7fc43926c5c 100644 --- a/src/infra/tmp-openclaw-dir.ts +++ b/src/infra/tmp-openclaw-dir.ts @@ -7,6 +7,7 @@ const TMP_DIR_ACCESS_MODE = fs.constants.W_OK | fs.constants.X_OK; type ResolvePreferredOpenClawTmpDirOptions = { accessSync?: (path: string, mode?: number) => void; + chmodSync?: (path: string, mode: number) => void; lstatSync?: (path: string) => { isDirectory(): boolean; isSymbolicLink(): boolean; @@ -16,6 +17,7 @@ type ResolvePreferredOpenClawTmpDirOptions = { mkdirSync?: (path: string, opts: { recursive: boolean; mode?: number }) => void; getuid?: () => number | undefined; tmpdir?: () => string; + warn?: (message: string) => void; }; type MaybeNodeError = { code?: string }; @@ -33,8 +35,10 @@ export function resolvePreferredOpenClawTmpDir( options: ResolvePreferredOpenClawTmpDirOptions = {}, ): string { const accessSync = options.accessSync ?? fs.accessSync; + const chmodSync = options.chmodSync ?? fs.chmodSync; const lstatSync = options.lstatSync ?? fs.lstatSync; const mkdirSync = options.mkdirSync ?? fs.mkdirSync; + const warn = options.warn ?? ((message: string) => console.warn(message)); const getuid = options.getuid ?? (() => { @@ -92,6 +96,26 @@ export function resolvePreferredOpenClawTmpDir( } }; + const tryRepairWritableBits = (candidatePath: string): boolean => { + try { + const st = lstatSync(candidatePath); + if (!st.isDirectory() || st.isSymbolicLink()) { + return false; + } + if (uid !== undefined && typeof st.uid === "number" && st.uid !== uid) { + return false; + } + if (typeof st.mode !== "number" || (st.mode & 0o022) === 0) { + return false; + } + chmodSync(candidatePath, 0o700); + warn(`[openclaw] tightened permissions on temp dir: ${candidatePath}`); + return resolveDirState(candidatePath) === "available"; + } catch { + return false; + } + }; + const ensureTrustedFallbackDir = (): string => { const fallbackPath = fallback(); const state = resolveDirState(fallbackPath); @@ -99,14 +123,18 @@ export function resolvePreferredOpenClawTmpDir( return fallbackPath; } if (state === "invalid") { + if (tryRepairWritableBits(fallbackPath)) { + return fallbackPath; + } throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`); } try { mkdirSync(fallbackPath, { recursive: true, mode: 0o700 }); + chmodSync(fallbackPath, 0o700); } catch { throw new Error(`Unable to create fallback OpenClaw temp dir: ${fallbackPath}`); } - if (resolveDirState(fallbackPath) !== "available") { + if (resolveDirState(fallbackPath) !== "available" && !tryRepairWritableBits(fallbackPath)) { throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`); } return fallbackPath; @@ -117,6 +145,9 @@ export function resolvePreferredOpenClawTmpDir( return POSIX_OPENCLAW_TMP_DIR; } if (existingPreferredState === "invalid") { + if (tryRepairWritableBits(POSIX_OPENCLAW_TMP_DIR)) { + return POSIX_OPENCLAW_TMP_DIR; + } return ensureTrustedFallbackDir(); } @@ -124,7 +155,11 @@ export function resolvePreferredOpenClawTmpDir( accessSync("/tmp", TMP_DIR_ACCESS_MODE); // Create with a safe default; subsequent callers expect it exists. mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 }); - if (resolveDirState(POSIX_OPENCLAW_TMP_DIR) !== "available") { + chmodSync(POSIX_OPENCLAW_TMP_DIR, 0o700); + if ( + resolveDirState(POSIX_OPENCLAW_TMP_DIR) !== "available" && + !tryRepairWritableBits(POSIX_OPENCLAW_TMP_DIR) + ) { return ensureTrustedFallbackDir(); } return POSIX_OPENCLAW_TMP_DIR;