From e9a4c4a601314206a3514d3807e6be68d7d15294 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 20 Jul 2026 00:35:47 +0200 Subject: [PATCH] feat(security): add a user-writable plugin ingest inbox for cross-account data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LocalService runner can't traverse the interactive user's profile the way the old SYSTEM runner could — so a plugin can no longer read a file an app running as *you* produced (the acute case: the Playnite exporter's library JSON under %APPDATA%). Confirmed on-glass: as LocalService the glob finds nothing and the profile is un-traversable. Add the inverse of plugin-state: \ingest, granted BUILTIN\Users Modify by plugins enable (disable reverts to inherited Users:RX). An interactive-user app drops ingest\\… and the de-privileged runner reads it there — the one Users-writable carve-out in the otherwise Users-read-only tree. SDK exports pluginIngestDir(name) to resolve it; on Linux the systemd --user runner owns the config dir so same-user producers write there with no grant. Accepted tradeoff: the inbox is writable by any local user (trusted-single- user model; it feeds only a LocalService runner). Consumers must treat ingest data as lower trust than their own state. Co-Authored-By: Claude Fable 5 --- crates/punktfunk-host/src/plugins.rs | 54 ++++++++++++++++++++++++++++ scripts/windows/build-scripting.ps1 | 7 ++++ sdk/README.md | 16 +++++++++ sdk/src/config.ts | 21 +++++++++++ sdk/src/index.ts | 2 ++ sdk/test/config.test.ts | 23 +++++++++++- 6 files changed, 122 insertions(+), 1 deletion(-) diff --git a/crates/punktfunk-host/src/plugins.rs b/crates/punktfunk-host/src/plugins.rs index 35246010..c0940497 100644 --- a/crates/punktfunk-host/src/plugins.rs +++ b/crates/punktfunk-host/src/plugins.rs @@ -249,6 +249,22 @@ const RUNNER_UNIT_DIRS: [&str; 2] = ["plugins", "scripts"]; #[cfg(target_os = "windows")] const RUNNER_STATE_DIRS: [&str; 1] = ["plugin-state"]; +/// The plugin **ingest** inbox: `\ingest`. The INVERSE grant of `plugin-state` — +/// `BUILTIN\Users` gets **Modify**, so an app running as the interactive user (e.g. the Playnite +/// exporter, a Playnite extension) can drop data (`ingest\\…`) that the de-privileged +/// LocalService runner then READS (LocalService is a member of Users, so it inherits read here). +/// This is the one place a plugin can receive data produced by *another* account — the runner can +/// no longer traverse the interactive user's profile the way the old SYSTEM runner could. Scoped +/// to this one inbox: the rest of the config tree stays Users-read-only, so the widening is a +/// well-defined drop box, not a general write hole. (Accepted tradeoff: any local user can drop a +/// file here — trusted-single-user model, and the runner it feeds is only LocalService.) +#[cfg(target_os = "windows")] +const RUNNER_INGEST_DIRS: [&str; 1] = ["ingest"]; + +/// `BUILTIN\Users` (S-1-5-32-545) in icacls SID form — the ingest inbox's writer. +#[cfg(target_os = "windows")] +const USERS_SID: &str = "*S-1-5-32-545"; + #[cfg(target_os = "windows")] fn enable() -> Result<()> { // Converge the task principal BEFORE starting it: the installer registers it as LocalService, @@ -363,6 +379,30 @@ fn grant_runner_secret_reads() { ); } } + // The ingest inbox: inheritable Modify for BUILTIN\Users, so an interactive-user app (the + // Playnite exporter) can drop `ingest\\…` for the LocalService runner to read (see + // RUNNER_INGEST_DIRS). The one Users-writable carve-out in the otherwise Users-read-only tree. + for name in RUNNER_INGEST_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!("{USERS_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 open the ingest inbox {} for writes - a plugin fed by an \ + interactive-user app (e.g. playnite) may see no data", + dir.display() + ); + } + } } /// Best-effort removal of the LocalService read grants when the runner is switched off — the @@ -386,6 +426,20 @@ fn revoke_runner_secret_reads() { .stderr(std::process::Stdio::null()) .status(); } + // The ingest inbox was opened to Users, not LocalService — remove that explicit grant (the + // inherited Users:RX from the config dir remains, so it reverts to read-only, not orphaned). + for name in RUNNER_INGEST_DIRS { + let path = cfg.join(name); + if !path.exists() { + continue; + } + let _ = Command::new(icacls_path()) + .arg(&path) + .args(["/remove:g", USERS_SID]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status(); + } } /// Resolve icacls by full System32 path rather than PATH — same planted-binary reasoning as diff --git a/scripts/windows/build-scripting.ps1 b/scripts/windows/build-scripting.ps1 index 5bc3d8c5..ae63a3ea 100644 --- a/scripts/windows/build-scripting.ps1 +++ b/scripts/windows/build-scripting.ps1 @@ -146,6 +146,13 @@ if ($existing) { 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" } + # Ingest inbox gets inheritable Modify for BUILTIN\Users - the INVERSE grant, so an + # interactive-user app (the Playnite exporter) can drop ingest\\ data the LocalService + # runner reads. The one Users-writable carve-out in the otherwise Users-read-only tree. + $ingestDir = Join-Path $cfg 'ingest' + New-Item -ItemType Directory -Force -Path $ingestDir | Out-Null + & "$env:SystemRoot\System32\icacls.exe" $ingestDir /grant:r '*S-1-5-32-545:(OI)(CI)(M)' | Out-Null + if ($LASTEXITCODE -ne 0) { Write-Host "warn : icacls grant failed on $ingestDir" } } # --- 4. the opt-in scheduled task ------------------------------------------------------------- diff --git a/sdk/README.md b/sdk/README.md index 3177dd89..06844c96 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -176,6 +176,22 @@ plugins enable` grants the runner write on exactly `plugin-state` — the config *code* stay read-only. On Linux the runner owns the whole config dir, so the same path is writable with no special step. +### Receiving data from an interactive-user app — `pluginIngestDir` + +If a plugin needs data produced by a **different account** — e.g. a desktop app running as the +logged-in user, like the Playnite exporter — it can't read it from that user's profile: the +de-privileged Windows runner can't traverse `C:\Users\\…`. `pluginIngestDir("")` +resolves an inbox (`/ingest/`) that `plugins enable` makes **user-writable**, so +your app drops a file there and the runner reads it: + +```ts +import { pluginIngestDir } from "@punktfunk/host"; +const inbox = pluginIngestDir("playnite"); // /ingest/playnite (your app writes here) +``` + +Treat what you read from it as lower trust than your own state — the inbox is writable by any local +user. + ### 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 64478ba7..43eb4800 100644 --- a/sdk/src/config.ts +++ b/sdk/src/config.ts @@ -72,6 +72,27 @@ export const pluginStateDir = (name?: string): string => { return name ? path.join(root, name) : root; }; +/** + * The ingest inbox a plugin reads data DROPPED BY ANOTHER ACCOUNT from: + * `/ingest[/]`. + * + * The mirror of {@link pluginStateDir}, and the answer to a problem the de-privileging creates on + * Windows: the LocalService runner can no longer traverse the interactive user's profile, so a + * plugin can't read a file an app running as *you* produced (e.g. the Playnite exporter's library + * JSON under your `%APPDATA%`). `punktfunk-host plugins enable` grants `BUILTIN\Users` **write** on + * exactly `ingest` — so your app drops `ingest//…` and the runner reads it there. On Linux + * the runner is a `systemd --user` unit owning the config dir, so a same-user producer writes here + * with no special step. + * + * The dir is NOT created here (a producer running as the interactive user creates its own + * `ingest/` subdir under the host-granted `ingest`). Treat anything read from it as + * lower-trust than your own state: the inbox is writable by any local user. + */ +export const pluginIngestDir = (name?: string): string => { + const root = path.join(configDir(), "ingest"); + 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 4956b6bc..b492235f 100644 --- a/sdk/src/index.ts +++ b/sdk/src/index.ts @@ -32,6 +32,8 @@ 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"; +// A plugin reads cross-account data (dropped by an interactive-user app) from here. +export { pluginIngestDir } from "./config.js"; export { type PluginUiHandle, type PluginUiOptions, diff --git a/sdk/test/config.test.ts b/sdk/test/config.test.ts index 4c490e24..75fdd456 100644 --- a/sdk/test/config.test.ts +++ b/sdk/test/config.test.ts @@ -2,7 +2,7 @@ // 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"; +import { pluginIngestDir, pluginStateDir } from "../src/config.js"; describe("pluginStateDir", () => { let saved: string | undefined; @@ -27,3 +27,24 @@ describe("pluginStateDir", () => { expect(pluginStateDir("x").startsWith(pluginStateDir())).toBe(true); }); }); + +describe("pluginIngestDir", () => { + 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 /ingest[/name], distinct from plugin-state", () => { + process.env.PUNKTFUNK_CONFIG_DIR = path.join("/tmp", "pf-cfg3"); + expect(pluginIngestDir()).toBe(path.join("/tmp", "pf-cfg3", "ingest")); + expect(pluginIngestDir("playnite")).toBe( + path.join("/tmp", "pf-cfg3", "ingest", "playnite"), + ); + // the inbox (Users-write) is a different tree from state (LocalService-write) + expect(pluginIngestDir("playnite")).not.toBe(pluginStateDir("playnite")); + }); +});