Skip to content

OpenWork — Security & Bug Review #29

Description

@ankurshashwat

Summary

# Severity Category Location Issue
1 Critical Vulnerability electron/main.mjs:1438 (+ app/.../lib/desktop.ts:157, bundles/sources.ts:96) __fetch is an unrestricted server-side HTTP proxy (SSRF) exposed to the renderer
2 High Vulnerability bundles/sources.ts:40,96 + lib/openwork-links.ts:173 Bundle fetch has no origin allowlist when ow_bundle URL is supplied; no integrity/signature check
3 High Vulnerability lib/openwork-links.ts:24-70 connect-remote deep link can auto-connect to an attacker host + token, bypassing the confirm modal
4 High Bug electron/openshell/wsl.mjs:133-144 (trigger client.mjs:98) Killing any one WSL child terminates the entire distro (cross-session DoS)
5 High Vulnerability electron/runtime.mjs:603-606,935-947 Renderer-supplied opencodeBinPath is exec'd with no validation/allowlist
6 High Hardening electron/main.mjs:2014-2118 No will-navigate / will-redirect guard; main frame can be navigated to remote origin keeping the IPC bridge
7 Medium Vulnerability electron/runtime.mjs:912-929 Remote access binds 0.0.0.0 with --cors * and --approval auto
8 Medium Bug/Vuln electron/openshell/openeral-terminal.mjs:132-136 cmd.exe fallback uses shell:true with unescaped metacharacters — broken + injection surface
9 Medium Vulnerability electron/main.mjs:2121 + lib/desktop.ts:184 shell.openExternal invoked with unvalidated URL scheme
10 Medium Vulnerability electron/runtime.mjs:94-102,137-145,784-795 Secrets (hostToken, opencode password, raw child stdout/stderr) returned to renderer / logged
11 Medium Bug/Vuln electron/updater.mjs:163 Any version mismatch (incl. older) reported as "update available" — no semver-greater guard
12 Medium Vulnerability electron/updater.mjs:80-87 No explicit update signature/publisher verification (generic provider over GitHub)
13 Medium Vulnerability lib/openwork-links.ts:19-22,107-135 http(s) accepted as deep-link scheme; den-auth denBaseUrl unpinned
14 Medium Bug electron/openshell/installer.mjs:141-150 Elevated wsl --install spawnSync has no timeout — can hang the installer indefinitely
15 Low Bug electron/workspace-archive.mjs:217,278-287 ZIP import has no decompression/size cap (zip-bomb DoS)
16 Low Vulnerability electron/runtime.mjs:438-472 Server tokens persisted to disk in plaintext (no safeStorage, no explicit 0600)
17 Low Vulnerability electron/main.mjs:2028 sandbox: false disables the Chromium renderer sandbox
18 Low Bug electron/openshell/doctor.mjs:55-70 runPowerShell conflates timeout with normal exit
19 Low Bug electron/openshell/client.mjs:154,162 Positional name arg-injection (--prefixed) into openshell
20 Low Bug electron/updater.mjs:108-135 autoUpdaterLoaded latched before import; transient failure permanently disables updater
21 Low Bug electron/migration.mjs:21-48 Snapshot read/ack not atomic (double-import race); no schema validation or size cap
22 Low Bug electron/openshell/wsl.mjs:72-103 wslRun buffers stdout/stderr unbounded
23 Low Bug electron/openshell/openeral-pty.mjs:150,192 cols/rows not floored/clamped before interpolation into stty (spawn path)
24 Low Vulnerability lib/electron-alpha.ts:34-37 Update artifact URL not pinned to release origin
25 Low Bug electron/runtime.mjs:1690-1698 Detached orchestrator spawn: no error handler; orphan on health-timeout
26 Info Design bundles/schema.ts / apply.ts Bundle skill name not sanitized (Electron sink is safe; Tauri/Rust sink unverified)
27 Info Design cloud/desktop-app-restrictions.ts Model/provider restrictions are client-side advisory only
28 Info Dead code electron/runtime.mjs ~150 lines of unused orchestrator/direct-spawn code + unused consts/helpers
29 Info Dead code electron/openshell/installer.mjs:25, openeral-terminal.mjs:34-54 Unused MIN_FREE_DISK_GB; ineffective detectLinuxTerminal probe (leaks which procs)

Critical

1. __fetch is an unrestricted SSRF proxy exposed to the renderer

  • Files: apps/desktop/electron/main.mjs:1438-1453; reached via apps/app/src/app/lib/desktop.ts:157-182 (desktopFetch) and apps/app/src/app/bundles/sources.ts:96-124 (fetchBundle).
  • Problem: The __fetch IPC command does fetch(url, { method, headers, body }) for any renderer-supplied URL and returns status + headers + full body. No scheme restriction, no host allowlist, no block on private/link-local ranges.
  • Impact: Any renderer code (or injected content) can make the main process fetch arbitrary URLs with main-process network privileges and read the response — cloud metadata (http://169.254.169.254/…), loopback/LAN admin services, file:-adjacent internal APIs — bypassing CORS entirely. This is the linchpin that makes findings Add/new feature #2 and Fix: Memory feature claude. #24 exploitable.
  • Fix: Enforce an allowlist in the main process (renderer-side checks are insufficient):
    case "__fetch": {
      const url = String(args[0] ?? "").trim();
      const parsed = new URL(url);                 // throws on garbage
      if (parsed.protocol !== "https:") throw new Error("Only https is allowed");
      await assertPublicHost(parsed.hostname);     // reject loopback, 10/8, 172.16/12,
                                                   // 192.168/16, 169.254/16, ::1, fc00::/7,
                                                   // and names that resolve into them
      // optionally: require parsed.origin to be on a configured publisher allowlist
      ...
    }
    Also tighten isLoopbackUrl in desktop.ts (it only matches 127.0.0.1/localhost/[::1]; 0.0.0.0, 127.0.0.2, decimal IPs, ::ffff:127.0.0.1 slip through) — but treat that as defense-in-depth, not the primary control.

High

2. Bundle fetch: no origin allowlist for ow_bundle URLs; no integrity check

  • Files: apps/app/src/app/bundles/sources.ts:40-66 (parseBundleDeepLink), 96-124 (fetchBundle), apps/app/src/app/lib/openwork-links.ts:173-192 (regex fallback).
  • Problem: isConfiguredBundlePublisherUrl() is only enforced on the bare-URL branch. When a link carries ow_bundle=<url> (or the regex fallback parses it), any http(s) URL is accepted and fetched — through the __fetch SSRF proxy (Add/new feature #1). Bundle payloads are only shape-validated (parseBundlePayload), never signature/digest-verified, and http: is accepted (cleartext/MITM).
  • Impact: A deep link like openwork://import-bundle?ow_bundle=http://169.254.169.254/… triggers SSRF; a link pointing at an attacker origin imports attacker-authored skills/commands (agent prompt-injection / supply-chain) into the workspace.
  • Fix: Enforce isConfiguredBundlePublisherUrl(bundleUrl) for all branches inside fetchBundle before any network call; require https:; validate the decoded URL in the openwork-links.ts fallback too; verify a publisher signature or content digest before applying.

3. connect-remote deep link auto-connects to an attacker host + token

  • File: apps/app/src/app/lib/openwork-links.ts:24-70.
  • Problem: parseRemoteConnectDeepLink accepts openworkHostUrl + openworkToken and honors autoConnect/bypassModal/bypassAddWorkerModal from the link itself, with no allowlist on the host (only normalization).
  • Impact: A single malicious link (even plain https:// — see Feat/Openclaw #13) silently points the user's worker at an attacker-operated OpenWork host with an attacker-chosen token, skipping confirmation → credential exposure / traffic routing to the attacker.
  • Fix: Never honor autoConnect/bypass* from untrusted deep links (always require explicit user confirmation), and/or restrict openworkHostUrl to a trusted-origin allowlist.

4. Killing one WSL child terminates the entire distro (cross-session DoS)

  • Files: apps/desktop/electron/openshell/wsl.mjs:133-144; trigger at apps/desktop/electron/openshell/client.mjs:96-107 (cleanup()child.kill("SIGTERM")).
  • Problem: wslSpawn overrides child.kill to first run wsl.exe -t <DISTRO_NAME>, which terminates the whole distro utility VM, before signalling the child. Verified in source (spawn(exe, ["-t", DISTRO_NAME], …)).
  • Impact: Tearing down one sandbox session (or a createSandbox cleanup) kills all other running sandboxes, every live sandbox connect PTY, and the gateway.
  • Fix: Do not use wsl -t as the reaper. Kill only the tracked child PID (wsl -d <distro> -- kill <pid> / scoped pkill on the exact command), or drop the reaper and rely on the CLI's own teardown. Only nuke the distro on an explicit, distinct "reset distro" action.

5. Renderer-supplied opencodeBinPath executed without validation

  • File: apps/desktop/electron/runtime.mjs:603-606 (resolveOpencodeBinary), consumed at 935-947, 1031-1053, 1096-1110; renderer-reachable via main.mjs:1234-1237 (engineStart(projectDir, options) where options = args[1]).
  • Problem: A caller-supplied opencodeBinPath is trusted verbatim ({ path: explicitPath, source: "custom" }) — no existsSync, no allowlist — and injected as OPENWORK_OPENCODE_BIN / --opencode-bin for execution.
  • Impact: Under a compromised/XSS'd renderer, this is arbitrary executable execution in the main process's environment (which also hosts a network-exposed service).
  • Fix: Validate before use — require the resolved path to exist and be a regular file, ideally under an allow-listed root; better, don't accept an arbitrary binary path over IPC at all.
    if (explicitPath) {
      const resolved = path.resolve(explicitPath);
      if (!existsSync(resolved) || !statSync(resolved).isFile())
        throw new Error("opencodeBinPath is not a file");
      return { path: resolved, source: "custom" };
    }

6. No will-navigate / will-redirect navigation guard

  • File: apps/desktop/electron/main.mjs:2014-2118 (createMainWindow).
  • Problem: setWindowOpenHandler restricts new windows, but there is no handler for top-level navigation of the existing webContents. A renderer (or injected content/link/redirect) can navigate the main frame to an arbitrary remote origin while keeping the preload contextBridge + all IPC channels exposed.
  • Impact: Turns any navigation-triggering content into a full IPC-bridge takeover (chains into Add/new feature #1, Feat/optimization #5, testing persistence #9). Standard Electron hardening gap.
  • Fix:
    const allowNav = (url) =>
      url.startsWith("file://") || url.startsWith("http://127.0.0.1") ||
      url.startsWith("http://localhost") || (trustedStartOrigin && new URL(url).origin === trustedStartOrigin);
    mainWindow.webContents.on("will-navigate", (e, url) => { if (!allowNav(url)) e.preventDefault(); });
    mainWindow.webContents.on("will-redirect", (e, url) => { if (!allowNav(url)) e.preventDefault(); });

Medium

7. Remote access binds 0.0.0.0 with --cors * and --approval auto

  • File: apps/desktop/electron/runtime.mjs:912, 926-929.
  • Problem: When remoteAccessEnabled, the server binds all interfaces, sets Access-Control-Allow-Origin: *, and auto-approves actions; the only guard is a bearer token. remoteAccessEnabled comes from renderer options.
  • Impact: Any web page the user visits can make cross-origin requests to the LAN-exposed server; combined with auto-approval and any token leak (harden workspace persistence #10/feat: add openwork as subdirectory + credit in README #16), this is remote command execution against the workspace.
  • Fix: Restrict CORS to the desktop origin (never *) when bound to 0.0.0.0; require explicit approval for remote-enabled sessions; make enabling remote access a deliberate, confirmed action.

8. cmd.exe terminal fallback: shell:true + unescaped metacharacters

  • File: apps/desktop/electron/openshell/openeral-terminal.mjs:132-136 (payload built at 115-122).
  • Problem: Fallback spawns cmd.exe with { shell: true }, but the bash payload contains |, &&, 2>/dev/null, > — cmd re-interprets these, tearing the command apart (fallback is non-functional) and creating an injection surface via windowTitle (which embeds sandboxName). Not currently exploitable only because callers pass deriveOpenEralSandboxName output.
  • Fix: Drop shell:true; invoke cmd.exe with an argv array (["/C","start","",windowTitle,"wsl.exe",...wslArgs]); validate sandboxName/windowTitle against /^[a-z0-9_.-]+$/ inside the launcher so it is self-defending.

9. shell.openExternal called with unvalidated URL scheme

  • Files: apps/desktop/electron/main.mjs:2121-2125; apps/app/src/app/lib/desktop.ts:184-196 (openDesktopUrl).
  • Problem: Any non-empty string is passed to shell.openExternal. No scheme validation.
  • Impact: A deep-link/bundle-derived value could open file:, smb:, or custom protocol-handler URIs → local file disclosure or launching arbitrary registered handlers.
  • Fix: Allowlist schemes before opening:
    const u = new URL(url);
    if (!["https:", "http:", "mailto:"].includes(u.protocol)) throw new Error("Blocked URL scheme");

10. Secrets returned to renderer / captured in logs

  • File: apps/desktop/electron/runtime.mjs:94-102, 137-145 (snapshots), 784-795 (appendOutput), 1277 (lastError).
  • Problem: State snapshots returned via runtimeStatus/engineInfo/openworkServerInfo include cleartext opencodePassword, hostToken (mints owner tokens), and raw child lastStdout/lastStderr. Children run with secret env vars, so verbose child output can leak secrets into captured logs surfaced to the renderer.
  • Fix: Return only the minimal bearer/owner token the renderer needs; never expose hostToken/opencodePassword; redact known secrets from captured output before storing/returning.

11. Updater reports any version mismatch (incl. downgrade) as "available"

  • File: apps/desktop/electron/updater.mjs:163.
  • Problem: available: info.version !== currentVersion — an older feed version is presented as an upgrade.
  • Fix: available: Boolean(info?.version && semver.gt(info.version, currentVersion)).

12. No explicit update signature/publisher verification

  • File: apps/desktop/electron/updater.mjs:80-87 (provider: "generic"), feeds at 31-34.
  • Problem: Integrity relies solely on HTTPS-to-GitHub + latest.yml sha512; no verifyUpdateCodeSignature/publisherName pinning, and signature errors are swallowed.
  • Fix: Prefer the GitHub provider (ties to repo/release) or configure/verify code signatures (Windows publisherName); surface verification failures to the user.

13. http(s) accepted as deep-link scheme; den-auth denBaseUrl unpinned

  • File: apps/app/src/app/lib/openwork-links.ts:19-22, 107-135.
  • Problem: Deep-link parsers accept http:/https: alongside the custom schemes, so ordinary web links can drive the connect-remote (Update2.0 #3), den-auth, and bundle (Add/new feature #2) flows. parseDenAuthDeepLink accepts an unpinned denBaseUrl, so a grant can be exchanged against an attacker endpoint.
  • Fix: Restrict deep-link parsing to openwork:/openwork-dev:; allowlist (or ignore) denBaseUrl.

14. Elevated wsl --install spawnSync has no timeout

  • File: apps/desktop/electron/openshell/installer.mjs:141-150.
  • Problem: spawnSync(powershell, ["-Command","Start-Process wsl.exe -Verb RunAs -Wait …"]) blocks indefinitely (no timeout), ignores the phase AbortSignal, and being spawnSync freezes the thread if the elevated install stalls.
  • Fix: Add a bounded timeout (e.g. 15 min) and honor the signal; treat result.signal === "SIGTERM" as the timeout case.

Low

15. ZIP import lacks a decompression/size cap (zip-bomb DoS)

apps/desktop/electron/workspace-archive.mjs:217,278-287. inflateRawSync runs with no output cap and uncompressedSize is never validated against a limit; a crafted archive can exhaust memory. Fix: cap total/per-entry uncompressed bytes and reject archives that exceed it.

16. Server tokens persisted in plaintext

apps/desktop/electron/runtime.mjs:438-472. openwork-server-tokens.json stores clientToken/hostToken/ownerToken in cleartext under userData (token strength is fine — randomUUID/256-bit). Fix: encrypt with Electron safeStorage or set explicit 0600.

17. Renderer Chromium sandbox disabled

apps/desktop/electron/main.mjs:2028 (sandbox: false). contextIsolation:true/nodeIntegration:false are set, but disabling the sandbox weakens the renderer boundary. Fix: set sandbox: true unless a specific preload dependency needs Node (the current preload only uses ipcRenderer/contextBridge, which work under the sandbox).

18. runPowerShell conflates timeout with normal exit

apps/desktop/electron/openshell/doctor.mjs:55-70. On timeout it SIGKILLs but resolves with exitCode:null/empty output instead of rejecting (unlike wslRun), so a hung WMI query is misreported as "missing/unknown". Fix: set a timedOut flag and reject on timeout.

19. Positional name argument-injection into openshell

apps/desktop/electron/openshell/client.mjs:154,162. deleteSandbox/getSandboxStatus place name as a positional arg before flags; a --prefixed name would be parsed as an option. Not currently exploitable (names come from deriveOpenEralSandboxName). Fix: use --name <name> (like createSandbox) or a -- end-of-options separator, or validate the name.

20. Updater permanently disabled after one transient import failure

apps/desktop/electron/updater.mjs:108-135. autoUpdaterLoaded = true is set before await import("electron-updater"); a single import throw latches the updater off for the session. Fix: set the flag only after a successful import (or reset it in catch).

21. Migration snapshot read/ack not atomic; no validation/size cap

apps/desktop/electron/migration.mjs:21-48. read doesn't mark consumed and ack is a separate call, so two windows (or a crash between) can double-import; contents are only version===1 checked and drive engineStart(projectDir…). Fix: make read-and-mark atomic/idempotent, schema-validate, and cap read size.

22. wslRun buffers output unbounded

apps/desktop/electron/openshell/wsl.mjs:72-103. No cap on stdoutChunks/stderrChunks. Fix: track a byte total and kill+reject past a cap (mirror openeral-pty.mjs's BUFFER_CAP_BYTES).

23. cols/rows not clamped before stty interpolation (spawn path)

apps/desktop/electron/openshell/openeral-pty.mjs:150,192. Only Number.isFinite-guarded, not floored/>0 (unlike resizeSession). Numeric-only, so not injection, but inconsistent. Fix: apply Math.floor/>0 clamp in openSession too.

24. Update artifact URL not pinned to release origin

apps/app/src/app/lib/electron-alpha.ts:34-37. If the manifest path is an absolute https:// URL it is used unchanged; a tampered manifest (delivered via the SSRF-capable desktopFetch) could redirect the download. Fix: always resolve path relative to the release base and reject foreign origins.

25. Detached orchestrator spawn: no error handler; orphan on timeout

apps/desktop/electron/runtime.mjs:1690-1698. spawn(…, { detached:true, stdio:"ignore" }) has no 'error' listener (an ENOENT/EACCES throws unhandled in main), and on waitForHttpOk rejection the unref()'d child is never killed. Fix: attach child.on("error", …) and child.kill("SIGKILL") before rethrowing on timeout.


Info / Design notes / Dead code

26. Bundle skill name not sanitized at the schema layer (Electron sink is safe)

apps/app/src/app/bundles/schema.ts / apply.ts accept name with only trim/type checks. Verified: the Electron IPC writers (main.mjs installSkillTemplate:1351, importSkill:1338, findSkillFile:990) all run validateSkillName (/^[a-z0-9]+(?:-[a-z0-9]+)*$/), which blocks ..///\, so the Electron path-traversal chain is not exploitable. Remaining gaps to close for defense-in-depth: (a) validate name in the bundle schema too; (b) confirm the Tauri/Rust sinks (desktop-tauri.tswrite_local_skill/import_skill) enforce basename-only writes — that path was outside this review's scope. Also note importSkill (main.mjs:1331-1347) cps from a renderer-supplied sourceDir with no containment check (can copy arbitrary local files into a workspace) — low impact, worth a guard.

27. Desktop app restrictions are client-side advisory only

apps/app/src/app/cloud/desktop-app-restrictions.ts:13-42. blockZenModel/provider blocks read renderer-held config and gate UI only — trivially bypassed by editing local config/state. Fine as UX/licensing hints; must not be treated as a security control. Enforce server-side at the model/provider gateway if it matters.

28. Dead code in runtime.mjs

Unused/unreachable (never called or exported): startOrchestratorRuntime (1019-1093) + ORCHESTRATOR_RUNTIME (44), startDirectRuntime (1095-1130), resolveOrchestratorBaseUrl (1007-1017), readPreferredOpenworkPort (485-492, ignored by resolveOpenworkPort), portAvailable (243-252), OPENWORK_SERVER_PORT_RANGE_START/END (45-46). ~150 lines including a whole alternate spawn path that will rot out of sync with the live startOpenworkServer. Fix: remove or wire back in.

29. Other dead/ineffective code

  • apps/desktop/electron/openshell/installer.mjs:25MIN_FREE_DISK_GB = 30 declared but never used (no disk preflight exists despite the intent). Implement the check or remove.
  • apps/desktop/electron/openshell/openeral-terminal.mjs:34-54detectLinuxTerminal spawns which <exe> for every candidate, ignores all results, and always returns the full list; it only leaks ~8 short-lived processes. launchLinuxTerminal already tries candidates in order. Fix: delete the probe loop; return LINUX_TERMINAL_CANDIDATES.
  • apps/desktop/electron/openshell/openeral-terminal.mjs:129-144 — wt.exe launch resolves success on a fixed 50 ms timer rather than a real spawn signal (optimistic; races the error→fallback path). Fix: resolve on once("spawn"), fall back only on error.

Suggested remediation order

  1. Add/new feature #1 + Add/new feature #2 (SSRF proxy + bundle allowlist) — one coordinated fix: enforce scheme/host allowlist in the main-process __fetch handler and origin allowlist in fetchBundle. This closes the most impactful chain.
  2. Feat/dev and feat/skills #6 (will-navigate guard) and testing persistence #9 (openExternal scheme allowlist) — cheap, high-value Electron hardening.
  3. Update2.0 #3 + Feat/Openclaw #13 (deep-link trust: drop http(s) schemes, never auto-connect/bypass, pin hosts).
  4. fixed typescript errors #4 (whole-distro kill) — correctness/DoS bug that will bite multi-session users.
  5. Feat/optimization #5, fix double /v1 #7, harden workspace persistence #10 (runtime: validate opencodeBinPath, lock down remote CORS/approval, stop leaking secrets to the renderer).
  6. Feat/persistence #11 + Feat/Openclaw #12 + Fix: Memory feature claude. #24 (update integrity: semver-greater + signature verification + origin pinning).
  7. Remaining Medium/Low items and dead-code cleanup.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions