From 73ec6ed9ef39dd247bdc8c0939ecf76ba196fc3c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 19 Jul 2026 23:23:48 +0200 Subject: [PATCH] feat(security): give the de-privileged runner a writable plugin-state dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LocalService runner cannot write anywhere under %ProgramData%\punktfunk (the config dir is Users-read-only), so a state-writing plugin's saveCache / config-edit / first-run mkdir all fail EPERM — proven on-glass (rom-manager only looked fine because its state dir was pre-created by an admin run and a 0-title reconcile skipped the write). Add the one writable grant the model was missing, keeping the split crisp — code dirs RX+WA, secrets R, and now a dedicated state root RW: - plugins enable / build-scripting.ps1: create %ProgramData%\punktfunk plugin-state and grant LocalService (OI)(CI)(M); disable revokes. Users stay read-only, so another non-admin still can't tamper with a plugin's launch templates. - SDK: export pluginStateDir(name) -> /plugin-state/. Same path on Linux (the systemd --user runner owns the config dir, writable with no grant), so plugins use one branch-free helper. Plugins must persist under pluginStateDir(), not straight under the config dir. Co-Authored-By: Claude Fable 5 --- crates/punktfunk-host/src/plugins.rs | 39 +++++++++++++++++++++++++++- scripts/windows/build-scripting.ps1 | 7 +++++ sdk/README.md | 21 +++++++++++++++ sdk/src/config.ts | 20 ++++++++++++++ sdk/src/index.ts | 2 ++ sdk/test/config.test.ts | 29 +++++++++++++++++++++ 6 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 sdk/test/config.test.ts diff --git a/crates/punktfunk-host/src/plugins.rs b/crates/punktfunk-host/src/plugins.rs index c69cbf9a..35246010 100644 --- a/crates/punktfunk-host/src/plugins.rs +++ b/crates/punktfunk-host/src/plugins.rs @@ -239,6 +239,16 @@ const RUNNER_SECRET_FILES: [&str; 2] = ["plugin-token", "cert.pem"]; #[cfg(target_os = "windows")] const RUNNER_UNIT_DIRS: [&str; 2] = ["plugins", "scripts"]; +/// The runner's writable state root: `\plugin-state`. A plugin persists its config + +/// cache under `plugin-state\` (`@punktfunk/host`'s `pluginStateDir`), so LocalService needs +/// real **Modify** here — unlike the code dirs (RX,WA) and the secrets (R). This keeps the +/// three-way split crisp: code is read-only (a plugin can't rewrite itself), secrets are +/// read-only, only this one dir is writable. Inheritable so per-plugin subdirs the runner creates +/// carry the grant. Users stay read-only (config-dir default), so another non-admin still can't +/// tamper with a plugin's launch templates. +#[cfg(target_os = "windows")] +const RUNNER_STATE_DIRS: [&str; 1] = ["plugin-state"]; + #[cfg(target_os = "windows")] fn enable() -> Result<()> { // Converge the task principal BEFORE starting it: the installer registers it as LocalService, @@ -330,6 +340,29 @@ fn grant_runner_secret_reads() { ); } } + // The state root: inheritable Modify so plugins can persist config/cache under + // `plugin-state\` (see RUNNER_STATE_DIRS). This is the ONLY writable grant. + for name in RUNNER_STATE_DIRS { + let dir = cfg.join(name); + if let Err(e) = std::fs::create_dir_all(&dir) { + eprintln!("warning: could not create {}: {e}", dir.display()); + continue; + } + let ok = Command::new(icacls_path()) + .arg(&dir) + .args(["/grant:r", &format!("{LOCAL_SERVICE_SID}:(OI)(CI)(M)")]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()); + if !ok { + eprintln!( + "warning: could not grant LocalService write on {} - state-writing plugins \ + (config/cache) may fail to persist", + dir.display() + ); + } + } } /// Best-effort removal of the LocalService read grants when the runner is switched off — the @@ -337,7 +370,11 @@ fn grant_runner_secret_reads() { #[cfg(target_os = "windows")] fn revoke_runner_secret_reads() { let cfg = pf_paths::config_dir(); - for name in RUNNER_SECRET_FILES.iter().chain(RUNNER_UNIT_DIRS.iter()) { + for name in RUNNER_SECRET_FILES + .iter() + .chain(RUNNER_UNIT_DIRS.iter()) + .chain(RUNNER_STATE_DIRS.iter()) + { let path = cfg.join(name); if !path.exists() { continue; diff --git a/scripts/windows/build-scripting.ps1 b/scripts/windows/build-scripting.ps1 index ce6bd6b3..5bc3d8c5 100644 --- a/scripts/windows/build-scripting.ps1 +++ b/scripts/windows/build-scripting.ps1 @@ -139,6 +139,13 @@ if ($existing) { & "$env:SystemRoot\System32\icacls.exe" $dirPath /grant:r '*S-1-5-19:(OI)(CI)(RX,WA)' | Out-Null if ($LASTEXITCODE -ne 0) { Write-Host "warn : icacls grant failed on $dirPath" } } + # State root gets inheritable Modify - the ONE writable grant, so a plugin can persist its + # config/cache under plugin-state\ (@punktfunk/host's pluginStateDir). Code dirs stay + # RX+WA, secrets stay R; only this dir is writable. + $stateDir = Join-Path $cfg 'plugin-state' + New-Item -ItemType Directory -Force -Path $stateDir | Out-Null + & "$env:SystemRoot\System32\icacls.exe" $stateDir /grant:r '*S-1-5-19:(OI)(CI)(M)' | Out-Null + if ($LASTEXITCODE -ne 0) { Write-Host "warn : icacls grant failed on $stateDir" } } # --- 4. the opt-in scheduled task ------------------------------------------------------------- diff --git a/sdk/README.md b/sdk/README.md index 482b650b..3177dd89 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -155,6 +155,27 @@ export default definePlugin({ In v1 a plugin is a script you run (see below); the managed runner package is a later step. +### Persisting state — `pluginStateDir` + +A plugin that keeps config or a cache must write it under `pluginStateDir("")`, **not** +directly under the config dir: + +```ts +import { pluginStateDir } from "@punktfunk/host"; +import * as fs from "node:fs"; +import * as path from "node:path"; + +const dir = pluginStateDir("rom-manager"); // /plugin-state/rom-manager +fs.mkdirSync(dir, { recursive: true }); +fs.writeFileSync(path.join(dir, "cache.json"), data); +``` + +This matters on Windows: the managed runner is de-privileged (`NT AUTHORITY\LocalService`) and the +config dir is locked read-only, so a write straight under it fails with `EPERM`. `punktfunk-host +plugins enable` grants the runner write on exactly `plugin-state` — the config dir and your plugin's +*code* stay read-only. On Linux the runner owns the whole config dir, so the same path is writable +with no special step. + ### A plugin UI in the console — `servePluginUi` A plugin can surface a web UI **inside the punktfunk console** — no second password or port for the diff --git a/sdk/src/config.ts b/sdk/src/config.ts index 370af65d..64478ba7 100644 --- a/sdk/src/config.ts +++ b/sdk/src/config.ts @@ -52,6 +52,26 @@ export const configDir = (): string => { return path.join(base, "punktfunk"); }; +/** + * The writable state directory a plugin should persist its config/cache into: + * `/plugin-state[/]`. + * + * WHY this and not `/` directly: on Windows the managed runner is de-privileged + * (runs as `NT AUTHORITY\LocalService`), and the config dir is locked to Users-read — so a plugin + * writing straight under it fails with EPERM. `punktfunk-host plugins enable` grants the runner + * **Modify** on exactly `plugin-state` (the config dir and the plugin *code* stay read-only), so + * this is the one place a supervised plugin can write. On Linux the runner is a `systemd --user` + * unit owning the whole config dir, so the path is writable there too — same code, no branch. + * + * `name` is a plugin's own kebab-case id; omit it for the shared root. The directory is NOT created + * here (the caller decides permissions/timing) — `fs.mkdirSync(pluginStateDir(name), {recursive: + * true})` from the runner inherits the granted ACL on Windows. + */ +export const pluginStateDir = (name?: string): string => { + const root = path.join(configDir(), "plugin-state"); + return name ? path.join(root, name) : root; +}; + const readIfExists = (p: string): string | undefined => { try { return fs.readFileSync(p, "utf8"); diff --git a/sdk/src/index.ts b/sdk/src/index.ts index c3ee4058..4956b6bc 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -30,6 +30,8 @@ import { export type { HostApi } from "./api.js"; export { HttpStatusError } from "./core.js"; export type { ConnectOptions } from "./config.js"; +// A plugin persists its state here — the one dir the de-privileged Windows runner may write. +export { pluginStateDir } from "./config.js"; export { type PluginUiHandle, type PluginUiOptions, diff --git a/sdk/test/config.test.ts b/sdk/test/config.test.ts new file mode 100644 index 00000000..4c490e24 --- /dev/null +++ b/sdk/test/config.test.ts @@ -0,0 +1,29 @@ +// Connection/config resolution helpers. `pluginStateDir` is the writable location a supervised +// plugin persists into — the one dir the de-privileged Windows runner may write. +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as path from "node:path"; +import { pluginStateDir } from "../src/config.js"; + +describe("pluginStateDir", () => { + let saved: string | undefined; + beforeEach(() => { + saved = process.env.PUNKTFUNK_CONFIG_DIR; + }); + afterEach(() => { + if (saved === undefined) delete process.env.PUNKTFUNK_CONFIG_DIR; + else process.env.PUNKTFUNK_CONFIG_DIR = saved; + }); + + test("resolves /plugin-state[/name] and honors the config-dir override", () => { + process.env.PUNKTFUNK_CONFIG_DIR = path.join("/tmp", "pf-cfg"); + expect(pluginStateDir()).toBe(path.join("/tmp", "pf-cfg", "plugin-state")); + expect(pluginStateDir("rom-manager")).toBe( + path.join("/tmp", "pf-cfg", "plugin-state", "rom-manager"), + ); + }); + + test("the per-plugin dir is nested under the shared root", () => { + process.env.PUNKTFUNK_CONFIG_DIR = path.join("/tmp", "pf-cfg2"); + expect(pluginStateDir("x").startsWith(pluginStateDir())).toBe(true); + }); +});