From 7643c0a1c4eb8f6d871e748395552dc7e6a9bdcf Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sat, 5 Sep 2026 04:49:35 -0700 Subject: [PATCH 1/2] fix(computer-use): contain HarmonyOS temporary-file cleanup Fixes #5894. Download into an owned mkdtemp directory and clean it in finally so successful reads cannot delete unrelated temporary files and failed transfers do not leak partial downloads. Validation: computer-use suite 34/34 passed. Isolated regression fixture reproduces unrelated-file deletion and leaked partial transfer on the previous implementation. Local file-read smoke passed. No live HarmonyOS device acceptance claimed. Signed-off-by: CodeWhale Bot --- .../plugins/computer-use/src/transport.mjs | 13 +++++--- .../tests/exec-transport.test.mjs | 31 ++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/crates/tui/plugins/computer-use/src/transport.mjs b/crates/tui/plugins/computer-use/src/transport.mjs index 7176de12e4..b6a4f4b110 100644 --- a/crates/tui/plugins/computer-use/src/transport.mjs +++ b/crates/tui/plugins/computer-use/src/transport.mjs @@ -118,11 +118,14 @@ export function hdcExec(computer) { return localPath; }, async readFile(remotePath, opts = {}) { - const tmp = path.join(os.tmpdir(), `cu-hdc-${crypto.randomBytes(4).toString("hex")}`); - await this.pullFile(remotePath, tmp, opts); - const data = await fs.promises.readFile(tmp); - await fs.promises.rm(path.dirname(tmp), { recursive: true, force: true }).catch(() => {}); - return data; + const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "cu-hdc-")); + try { + const tmp = path.join(dir, "out"); + await this.pullFile(remotePath, tmp, opts); + return await fs.promises.readFile(tmp); + } finally { + await fs.promises.rm(dir, { recursive: true, force: true }); + } }, }; } diff --git a/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs b/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs index 9959347c4e..061d966d2c 100644 --- a/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs +++ b/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs @@ -1,8 +1,11 @@ // exec + transport safety tests. import { test } from "node:test"; import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { run, runOk, ExecError, have, trim } from "../src/exec.mjs"; -import { safeRemotePath, b64, localExec } from "../src/transport.mjs"; +import { safeRemotePath, b64, localExec, hdcExec } from "../src/transport.mjs"; test("run captures stdout/stderr and exit codes without a shell", async () => { const r = await run("node", ["-e", "console.log('hello'); console.error('boo')"]); @@ -55,3 +58,29 @@ test("localExec provides run/runOk/tmpFile", async () => { const f = ex.tmpFile("cu-test-"); assert.ok(typeof f === "string"); }); + +for (const outcome of ["success", "transfer failure", "read failure"]) { + test(`hdc readFile cleans only its owned files after ${outcome}`, async (t) => { + // Contain even the old recursive-deletion bug inside this fixture. + const root = await fs.mkdtemp(path.join(os.tmpdir(), "cu-hdc-test-")); + t.after(() => fs.rm(root, { recursive: true, force: true })); + t.mock.method(os, "tmpdir", () => root); + await fs.writeFile(path.join(root, "unrelated"), "keep me"); + const ex = hdcExec({}); + const transferError = new Error("transfer interrupted"); + ex.pullFile = async (remote, local) => { + assert.equal(remote, "fixture.txt"); + if (outcome === "read failure") return; + await fs.writeFile(local, "downloaded bytes"); + if (outcome === "transfer failure") throw transferError; + }; + if (outcome === "success") { + assert.equal((await ex.readFile("fixture.txt")).toString(), "downloaded bytes"); + } else { + await assert.rejects(ex.readFile("fixture.txt"), (error) => + outcome === "transfer failure" ? error === transferError : error.code === "ENOENT"); + } + assert.equal(await fs.readFile(path.join(root, "unrelated"), "utf8"), "keep me"); + assert.deepEqual(await fs.readdir(root), ["unrelated"]); + }); +} From 29bdcf1dcaaa76ec1cf5713fe28826e65f536558 Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Sat, 5 Sep 2026 05:54:09 -0700 Subject: [PATCH 2/2] fix(computer-use): preserve HDC results when temporary cleanup fails Keep recursive cleanup confined to the owned directory and best effort, so cleanup failure cannot mask a transfer/read error or reject a successful download. Validation: 37/37 plugin tests pass, including six success/failure/cleanup combinations; git diff --check passed. Runtime root has no npm test/check:web scripts. Signed-off-by: CodeWhale Bot --- .../plugins/computer-use/src/transport.mjs | 3 +- .../tests/exec-transport.test.mjs | 58 +++++++++++-------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/crates/tui/plugins/computer-use/src/transport.mjs b/crates/tui/plugins/computer-use/src/transport.mjs index b6a4f4b110..5a203645ba 100644 --- a/crates/tui/plugins/computer-use/src/transport.mjs +++ b/crates/tui/plugins/computer-use/src/transport.mjs @@ -124,7 +124,8 @@ export function hdcExec(computer) { await this.pullFile(remotePath, tmp, opts); return await fs.promises.readFile(tmp); } finally { - await fs.promises.rm(dir, { recursive: true, force: true }); + // Cleanup must not replace downloaded bytes or the original I/O error. + await fs.promises.rm(dir, { recursive: true, force: true }).catch(() => {}); } }, }; diff --git a/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs b/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs index 061d966d2c..f51ec676d3 100644 --- a/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs +++ b/crates/tui/plugins/computer-use/tests/exec-transport.test.mjs @@ -59,28 +59,38 @@ test("localExec provides run/runOk/tmpFile", async () => { assert.ok(typeof f === "string"); }); -for (const outcome of ["success", "transfer failure", "read failure"]) { - test(`hdc readFile cleans only its owned files after ${outcome}`, async (t) => { - // Contain even the old recursive-deletion bug inside this fixture. - const root = await fs.mkdtemp(path.join(os.tmpdir(), "cu-hdc-test-")); - t.after(() => fs.rm(root, { recursive: true, force: true })); - t.mock.method(os, "tmpdir", () => root); - await fs.writeFile(path.join(root, "unrelated"), "keep me"); - const ex = hdcExec({}); - const transferError = new Error("transfer interrupted"); - ex.pullFile = async (remote, local) => { - assert.equal(remote, "fixture.txt"); - if (outcome === "read failure") return; - await fs.writeFile(local, "downloaded bytes"); - if (outcome === "transfer failure") throw transferError; - }; - if (outcome === "success") { - assert.equal((await ex.readFile("fixture.txt")).toString(), "downloaded bytes"); - } else { - await assert.rejects(ex.readFile("fixture.txt"), (error) => - outcome === "transfer failure" ? error === transferError : error.code === "ENOENT"); - } - assert.equal(await fs.readFile(path.join(root, "unrelated"), "utf8"), "keep me"); - assert.deepEqual(await fs.readdir(root), ["unrelated"]); - }); +for (const cleanupFails of [false, true]) { + for (const outcome of ["success", "transfer failure", "read failure"]) { + test(`hdc readFile preserves ${outcome} when cleanup ${cleanupFails ? "fails" : "succeeds"}`, async (t) => { + // Contain even the old recursive-deletion bug inside this fixture. + const root = await fs.mkdtemp(path.join(os.tmpdir(), "cu-hdc-test-")); + const realRm = fs.rm.bind(fs); + t.after(() => realRm(root, { recursive: true, force: true })); + if (cleanupFails) { + t.mock.method(fs, "rm", async (dir) => { + assert.equal(path.dirname(dir), root, "cleanup stays inside its fixture"); + assert.ok(path.basename(dir).startsWith("cu-hdc-")); + throw Object.assign(new Error("cleanup denied"), { code: "EACCES" }); + }); + } + t.mock.method(os, "tmpdir", () => root); + await fs.writeFile(path.join(root, "unrelated"), "keep me"); + const ex = hdcExec({}); + const transferError = new Error("transfer interrupted"); + ex.pullFile = async (remote, local) => { + assert.equal(remote, "fixture.txt"); + if (outcome === "read failure") return; + await fs.writeFile(local, "downloaded bytes"); + if (outcome === "transfer failure") throw transferError; + }; + if (outcome === "success") { + assert.equal((await ex.readFile("fixture.txt")).toString(), "downloaded bytes"); + } else { + await assert.rejects(ex.readFile("fixture.txt"), (error) => + outcome === "transfer failure" ? error === transferError : error.code === "ENOENT"); + } + assert.equal(await fs.readFile(path.join(root, "unrelated"), "utf8"), "keep me"); + if (!cleanupFails) assert.deepEqual(await fs.readdir(root), ["unrelated"]); + }); + } }