From 1c75eddbfac1b32b2b54e69af45188ced1f2c07d Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 15 Jun 2026 23:22:44 +0200 Subject: [PATCH] fix: secure skill ssh config temp files --- src/lib/actions/sandbox/skill-install.test.ts | 22 +++++++ src/lib/actions/sandbox/skill-install.ts | 57 ++++++++++++------- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/lib/actions/sandbox/skill-install.test.ts b/src/lib/actions/sandbox/skill-install.test.ts index 7c9c3cb7e24..0ccafbd7125 100644 --- a/src/lib/actions/sandbox/skill-install.test.ts +++ b/src/lib/actions/sandbox/skill-install.test.ts @@ -162,8 +162,14 @@ describe("sandbox skill action orchestration", () => { skillInstall.checkExisting.mockImplementation((ctx, resolvedPaths) => { tempConfig = ctx.configFile; expect(resolvedPaths).toBe(paths); + expect(path.basename(path.dirname(tempConfig)).startsWith("nemoclaw-ssh-skill-")).toBe(true); + expect(path.basename(tempConfig)).toBe("config.conf"); + expect(fs.statSync(path.dirname(tempConfig)).mode & 0o777).toBe(0o700); + expect(fs.statSync(tempConfig).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(tempConfig, "utf-8")).toBe("Host openshell-alpha\n"); return true; }); + const writeFile = vi.spyOn(fs, "writeFileSync"); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); await removeSandboxSkill("alpha", { name: "demo-skill" }); @@ -180,7 +186,12 @@ describe("sandbox skill action orchestration", () => { paths, ); expect(log).toHaveBeenCalledWith(expect.stringContaining("Skill 'demo-skill' removed")); + expect(writeFile).toHaveBeenCalledWith(tempConfig, "Host openshell-alpha\n", { + mode: 0o600, + flag: "wx", + }); expect(fs.existsSync(tempConfig)).toBe(false); + expect(fs.existsSync(path.dirname(tempConfig))).toBe(false); expect(process.exitCode).toBeUndefined(); }); @@ -189,8 +200,14 @@ describe("sandbox skill action orchestration", () => { let tempConfig = ""; skillInstall.checkExisting.mockImplementation((ctx) => { tempConfig = ctx.configFile; + expect(path.basename(path.dirname(tempConfig)).startsWith("nemoclaw-ssh-skill-")).toBe(true); + expect(path.basename(tempConfig)).toBe("config.conf"); + expect(fs.statSync(path.dirname(tempConfig)).mode & 0o777).toBe(0o700); + expect(fs.statSync(tempConfig).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(tempConfig, "utf-8")).toBe("Host openshell-alpha\n"); return null; }); + const writeFile = vi.spyOn(fs, "writeFileSync"); const error = vi.spyOn(console, "error").mockImplementation(() => undefined); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -215,7 +232,12 @@ describe("sandbox skill action orchestration", () => { paths, ); expect(log).toHaveBeenCalledWith(expect.stringContaining("Skill 'demo-skill' installed")); + expect(writeFile).toHaveBeenCalledWith(tempConfig, "Host openshell-alpha\n", { + mode: 0o600, + flag: "wx", + }); expect(fs.existsSync(tempConfig)).toBe(false); + expect(fs.existsSync(path.dirname(tempConfig))).toBe(false); expect(process.exitCode).toBeUndefined(); }); }); diff --git a/src/lib/actions/sandbox/skill-install.ts b/src/lib/actions/sandbox/skill-install.ts index c570ff785eb..014affe05ea 100644 --- a/src/lib/actions/sandbox/skill-install.ts +++ b/src/lib/actions/sandbox/skill-install.ts @@ -72,6 +72,35 @@ export type SkillRemoveRequest = { extraArgs?: string[]; }; +type TemporarySshConfig = { + configFile: string; + dir: string; +}; + +function createTemporarySshConfig(contents: string): TemporarySshConfig { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-ssh-skill-")); + const configFile = path.join(dir, "config.conf"); + try { + fs.writeFileSync(configFile, contents, { mode: 0o600, flag: "wx" }); + } catch (err) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + throw err; + } + return { configFile, dir }; +} + +function removeTemporarySshConfig(temp: TemporarySshConfig): void { + try { + fs.rmSync(temp.dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } +} + export function printPluginInstallHint(): void { console.error(" This looks like an OpenClaw plugin, not a SKILL.md agent skill."); console.error(" `skill install` only accepts skill directories or direct SKILL.md paths."); @@ -123,14 +152,10 @@ export async function removeSandboxSkill( process.exit(1); } - const tmpSshConfig = path.join( - os.tmpdir(), - `nemoclaw-ssh-skill-${process.pid}-${Date.now()}.conf`, - ); - fs.writeFileSync(tmpSshConfig, sshConfigResult.output, { mode: 0o600 }); + const tmpSshConfig = createTemporarySshConfig(sshConfigResult.output); try { - const ctx = { configFile: tmpSshConfig, sandboxName }; + const ctx = { configFile: tmpSshConfig.configFile, sandboxName }; const existsCheck = skillInstall.checkExisting(ctx, paths); if (existsCheck === null) { @@ -165,11 +190,7 @@ export async function removeSandboxSkill( return; } } finally { - try { - fs.unlinkSync(tmpSshConfig); - } catch { - /* ignore */ - } + removeTemporarySshConfig(tmpSshConfig); } } @@ -291,14 +312,10 @@ export async function installSandboxSkill( process.exit(1); } - const tmpSshConfig = path.join( - os.tmpdir(), - `nemoclaw-ssh-skill-${process.pid}-${Date.now()}.conf`, - ); - fs.writeFileSync(tmpSshConfig, sshConfigResult.output, { mode: 0o600 }); + const tmpSshConfig = createTemporarySshConfig(sshConfigResult.output); try { - const ctx = { configFile: tmpSshConfig, sandboxName }; + const ctx = { configFile: tmpSshConfig.configFile, sandboxName }; // 5. Check if skill already exists (update vs fresh install). This probe is // advisory for install only: stale SSH config files and transient remote @@ -349,10 +366,6 @@ export async function installSandboxSkill( process.exit(1); } } finally { - try { - fs.unlinkSync(tmpSshConfig); - } catch { - /* ignore */ - } + removeTemporarySshConfig(tmpSshConfig); } }