diff --git a/sdk/src/runner.ts b/sdk/src/runner.ts index fb58c965..02317038 100644 --- a/sdk/src/runner.ts +++ b/sdk/src/runner.ts @@ -13,13 +13,15 @@ // background work is invisible to supervision; export a plugin to be supervised). // // Trust model (RFC §9.4): a unit is code the operator chose to run — no sandbox is pretended. -// The same sshd rule as hooks applies: a world-writable unit file is refused loudly. +// The same sshd rule as hooks applies on BOTH platforms: a unit file a non-privileged principal +// could have written is refused loudly (mode bits on Unix, owner + DACL on Windows). import { Cause, Duration, Effect, Schedule, } from "effect"; +import { spawnSync } from "node:child_process"; import * as fs from "node:fs"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; @@ -53,9 +55,217 @@ export interface Unit { const defaultLog = (line: string) => console.log(`${new Date().toISOString()} ${line}`); -/** The sshd rule (RFC §9.1/§9.4): refuse group/world-writable unit files, loudly. */ +// ---- unit-file trust (the sshd rule, both halves) --------------------------------------------- + +// SDDL access-mask bits that let a principal change the file's content or its protection: +// write/append data + EAs, DELETE (delete + recreate), WRITE_DAC / WRITE_OWNER (rewrite the +// protection itself), and the generic write/all bits. FILE_WRITE_ATTRIBUTES (0x100) is +// deliberately NOT here: it only toggles timestamps/readonly/hidden — never content — and the +// runner's own service principal legitimately holds it (bun's module loader opens unit files +// requesting RX+WA; `plugins enable` grants exactly that on the plugins/scripts dirs — counting +// WA as tampering would make the runner refuse every unit it is supposed to run). +const SDDL_WRITE_BITS = + 0x2 | 0x4 | 0x10 | 0x10000 | 0x40000 | 0x80000 | 0x10000000 | 0x40000000; + +// SDDL two-letter rights tokens → access-mask bits (generic, standard, file-specific, and the +// low object-rights aliases hex masks sometimes render as). Anything unrecognized is treated as +// write-capable — fail closed. +const SDDL_RIGHT_TOKENS: Record = { + GA: 0x10000000, + GX: 0x20000000, + GW: 0x40000000, + GR: 0x80000000, + RC: 0x20000, + SD: 0x10000, + WD: 0x40000, + WO: 0x80000, + FA: 0x1f01ff, + FR: 0x120089, + FW: 0x120116, + FX: 0x1200a0, + CC: 0x1, + DC: 0x2, + LC: 0x4, + SW: 0x8, + RP: 0x10, + WP: 0x20, + DT: 0x40, + LO: 0x80, + CR: 0x100, +}; + +// SDDL two-letter account abbreviations we may meet on a unit file, → full SIDs. +const SDDL_SID_ABBREV: Record = { + SY: "S-1-5-18", // NT AUTHORITY\SYSTEM + BA: "S-1-5-32-544", // BUILTIN\Administrators + OW: "S-1-3-4", // OWNER RIGHTS + CO: "S-1-3-0", // CREATOR OWNER + LS: "S-1-5-19", // NT AUTHORITY\LOCAL SERVICE + NS: "S-1-5-20", // NT AUTHORITY\NETWORK SERVICE + BU: "S-1-5-32-545", // BUILTIN\Users + AU: "S-1-5-11", // Authenticated Users + IU: "S-1-5-4", // INTERACTIVE + WD: "S-1-1-0", // Everyone +}; + +const TRUSTED_OWNER_SIDS = new Set([ + "S-1-5-18", // SYSTEM + "S-1-5-32-544", // Administrators + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", // TrustedInstaller +]); + +/** An SDDL rights field as an access mask; -1 when it can't be fully understood (fail closed). */ +const sddlRightsMask = (rights: string): number => { + if (rights.startsWith("0x") || rights.startsWith("0X")) { + const n = Number.parseInt(rights, 16); + return Number.isNaN(n) ? -1 : n; + } + if (rights.length % 2 !== 0) return -1; + let mask = 0; + for (let i = 0; i < rights.length; i += 2) { + const bits = SDDL_RIGHT_TOKENS[rights.slice(i, i + 2)]; + if (bits === undefined) return -1; + mask = (mask | bits) >>> 0; + } + return mask; +}; + +/** + * The Windows half of the sshd rule, as a pure function over the file's SDDL (exported for + * tests). Returns `null` when the descriptor is trustworthy — owner is + * SYSTEM/Administrators/TrustedInstaller (or `extraTrustedSid`, the account running the runner: + * the Unix rule's "your own file is fine") and no other principal holds a write-capable allow + * ACE — else a human-readable refusal reason. Unknown ACE shapes count as write-capable. + */ +export const windowsSddlUnsafeReason = ( + sddl: string, + extraTrustedSid?: string, +): string | null => { + const trusted = new Set(TRUSTED_OWNER_SIDS); + trusted.add("S-1-3-4"); // OWNER RIGHTS — constrained by the owner check below + if (extraTrustedSid) trusted.add(extraTrustedSid); + + const owner = /^O:(S-[0-9-]+|[A-Z]{2})/.exec(sddl)?.[1]; + const ownerSid = + owner === undefined + ? undefined + : owner.startsWith("S-") + ? owner + : SDDL_SID_ABBREV[owner]; + if (ownerSid === undefined || !trusted.has(ownerSid)) { + return `owner ${owner ?? "unknown"} is not SYSTEM/Administrators/TrustedInstaller`; + } + + const daclAt = sddl.indexOf("D:"); + if (daclAt < 0) return "no DACL in the security descriptor"; + // ACE format: (type;flags;rights;objectGuid;inheritGuid;sid[;condition]). SACL ACEs after + // "S:" match the regex too but are audit types, filtered by the allow-type check. + for (const [, ace] of sddl.slice(daclAt + 2).matchAll(/\(([^)]*)\)/g)) { + const [type, flags = "", rights = "", , , sid = ""] = ace.split(";"); + if (type !== "A" && type !== "XA") continue; // deny/audit ACEs only ever tighten + // Inherit-only ACEs are templates for children; they grant nothing on this file. Flags + // come in two-letter tokens — compare exactly, not by substring. + const flagTokens: string[] = flags.match(/.{2}/g) ?? []; + if (flagTokens.includes("IO")) continue; + const mask = sddlRightsMask(rights); + if (mask !== -1 && (mask & SDDL_WRITE_BITS) === 0) continue; // read-only ACE + const resolved = sid.startsWith("S-") ? sid : (SDDL_SID_ABBREV[sid] ?? sid); + if (!trusted.has(resolved)) { + return `${sid} can write it (only SYSTEM/Administrators may)`; + } + } + return null; +}; + +/** The SID this process runs as, fetched once (`undefined` when it can't be determined). */ +let processSidCache: string | undefined | false; +const processSid = (): string | undefined => { + if (processSidCache === undefined) { + const res = spawnSync( + windowsPowershell(), + [ + "-NoProfile", + "-NonInteractive", + "-Command", + "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", + ], + { + encoding: "utf8", + windowsHide: true, + timeout: 15_000, + env: windowsPowershellEnv(), + }, + ); + const sid = res.status === 0 ? (res.stdout ?? "").trim() : ""; + processSidCache = /^S-[0-9-]+$/.test(sid) ? sid : false; + } + return processSidCache === false ? undefined : processSidCache; +}; + +// Full System32 path, never PATH — a planted powershell.exe must not run with our privileges +// (mirrors the host CLI's powershell_path, security-review 2026-07-17). +const windowsPowershell = (): string => + path.join( + process.env.SystemRoot ?? "C:\\Windows", + "System32", + "WindowsPowerShell", + "v1.0", + "powershell.exe", + ); + +// Spawn env for Windows PowerShell 5.1 with PSModulePath STRIPPED (5.1 rebuilds its default). +// An inherited PSModulePath that includes a PowerShell 7 module dir (pwsh adds a machine-scope +// entry) makes 5.1 fail to autoload Microsoft.PowerShell.Security — type-data conflict +// ("AuditToString is already present") — so Get-Acl dies and every unit is refused. Seen live +// on the .173 host box; intermittent per spawn, deterministic once stripped. +const windowsPowershellEnv = (): Record => { + const env: Record = { ...process.env }; + delete env.PSModulePath; + return env; +}; + +/** Read a file's SDDL and apply [`windowsSddlUnsafeReason`]. Unreadable ACL ⇒ refuse. */ +const windowsFileIsSafe = (file: string, log: (l: string) => void): boolean => { + const escaped = file.replace(/'/g, "''"); + const res = spawnSync( + windowsPowershell(), + [ + "-NoProfile", + "-NonInteractive", + "-Command", + `(Get-Acl -LiteralPath '${escaped}').Sddl`, + ], + { + encoding: "utf8", + windowsHide: true, + timeout: 15_000, + env: windowsPowershellEnv(), + }, + ); + const sddl = res.status === 0 ? (res.stdout ?? "").trim() : ""; + if (!sddl) { + log(`[runner] REFUSING ${file} — could not read its ACL`); + return false; + } + const reason = windowsSddlUnsafeReason(sddl, processSid()); + if (reason !== null) { + log( + `[runner] REFUSING ${file} — ${reason}. Reinstall the plugin with ` + + `\`punktfunk-host plugins add\`, or re-own the file to Administrators and strip ` + + `non-admin write ACEs (icacls).`, + ); + return false; + } + return true; +}; + +/** + * The sshd rule (RFC §9.1/§9.4): refuse a unit file anyone less privileged than the operator + * could have written — group/world-writable mode on Unix; on Windows, an owner outside + * SYSTEM/Administrators/TrustedInstaller or a write-capable ACE for a non-admin principal. + */ const fileIsSafe = (file: string, log: (l: string) => void): boolean => { - if (process.platform === "win32") return true; // config dir is DACL'd; ACL check is a follow-up + if (process.platform === "win32") return windowsFileIsSafe(file, log); try { const mode = fs.statSync(file).mode & 0o022; if (mode !== 0) { diff --git a/sdk/test/runner.test.ts b/sdk/test/runner.test.ts index 77a8d55c..ed529775 100644 --- a/sdk/test/runner.test.ts +++ b/sdk/test/runner.test.ts @@ -5,7 +5,12 @@ import { afterAll, describe, expect, test } from "bun:test"; import { Effect, Fiber } from "effect"; import * as fs from "node:fs"; import * as path from "node:path"; -import { discoverUnits, runner, superviseUnit } from "../src/runner.js"; +import { + discoverUnits, + runner, + superviseUnit, + windowsSddlUnsafeReason, +} from "../src/runner.js"; const TOKEN = "runner-token"; // Fixtures live under sdk/ so the generated plugin files can resolve "effect" and the SDK. @@ -104,6 +109,86 @@ describe("discovery", () => { }); }); +describe("windowsSddlUnsafeReason (the sshd rule's Windows half, pure)", () => { + // What a file under the host's ACL'd %ProgramData%\punktfunk actually looks like: owned by + // Administrators, protected DACL, admin/SYSTEM/OWNER-RIGHTS full + Users read-execute. + const LOCKED = + "O:BAG:SYD:PAI(A;;FA;;;SY)(A;;FA;;;BA)(A;;FA;;;OW)(A;;0x1200a9;;;BU)"; + + test("accepts the host's locked-down layout", () => { + expect(windowsSddlUnsafeReason(LOCKED)).toBeNull(); + }); + + test("accepts inherited allow ACEs spelled with token runs", () => { + expect( + windowsSddlUnsafeReason("O:SYG:SYD:(A;ID;FA;;;SY)(A;ID;FA;;;BA)(A;ID;FR;;;BU)"), + ).toBeNull(); + }); + + test("refuses an untrusted owner", () => { + expect( + windowsSddlUnsafeReason("O:BUG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)"), + ).toContain("owner"); + expect( + windowsSddlUnsafeReason( + "O:S-1-5-21-1111111111-2222222222-3333333333-1001G:SYD:(A;;FA;;;SY)", + ), + ).toContain("owner"); + }); + + test("accepts the running account as owner and writer (the Unix 'your own file' rule)", () => { + const me = "S-1-5-21-1111111111-2222222222-3333333333-1001"; + const sddl = `O:${me}G:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;FA;;;${me})`; + expect(windowsSddlUnsafeReason(sddl)).toContain("owner"); + expect(windowsSddlUnsafeReason(sddl, me)).toBeNull(); + }); + + test("refuses write-capable ACEs for non-admin principals, by token and by hex", () => { + // BUILTIN\Users with modify (hex, as Windows renders 0x1301bf) + expect( + windowsSddlUnsafeReason(`${LOCKED}(A;;0x1301bf;;;BU)`), + ).toContain("BU"); + // Everyone with generic write + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;GW;;;WD)`)).toContain("WD"); + // Authenticated Users with file-write + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;FW;;;AU)`)).toContain("AU"); + // DELETE alone is enough (delete + recreate) + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;SD;;;BU)`)).toContain("BU"); + // WRITE_DAC alone is enough (rewrite the protection) + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;WD;;;BU)`)).toContain("BU"); + // an explicit user SID + expect( + windowsSddlUnsafeReason( + `${LOCKED}(A;;FA;;;S-1-5-21-1111111111-2222222222-3333333333-1001)`, + ), + ).toContain("S-1-5-21"); + }); + + test("accepts the runner principal's RX+WA grant, refuses real writes for it", () => { + // `plugins enable` grants LocalService (RX,WA) on the unit dirs — bun's module loader + // opens files requesting FILE_WRITE_ATTRIBUTES on top of read+execute (0x1201a9). + // WA can't alter content, so it must not read as tamper-capable… + expect( + windowsSddlUnsafeReason(`${LOCKED}(A;OICIID;0x1201a9;;;LS)`), + ).toBeNull(); + // …but actual write-data for the service principal is still refused. + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;FW;;;LS)`)).toContain("LS"); + }); + + test("inherit-only ACEs and deny ACEs don't trip it; unknown shapes fail closed", () => { + // inherit-only: a template for children, grants nothing on this file + expect( + windowsSddlUnsafeReason(`${LOCKED}(A;OICIIO;FA;;;BU)`), + ).toBeNull(); + // deny ACEs only tighten + expect(windowsSddlUnsafeReason(`${LOCKED}(D;;FA;;;BU)`)).toBeNull(); + // unknown rights token → treated as write-capable + expect(windowsSddlUnsafeReason(`${LOCKED}(A;;ZZ;;;BU)`)).toContain("BU"); + // no DACL at all → refused + expect(windowsSddlUnsafeReason("O:BAG:SY")).toContain("DACL"); + }); +}); + describe("supervision", () => { test("async-fn plugin runs with a facade client; clean return completes", async () => { const server = mockHost();