From 4efc2552d0f6e45606568aebbab781bf7ab5fc6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 21 Aug 2026 00:15:38 +0000 Subject: [PATCH] os-deps: a dependency declares who has to be able to run it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe asked the owner's login shell where a binary was, and the install reported that as a fact about the machine. For a confined plugin the work happens as the member's own Linux user, in their own home, where the owner's ~/.local/bin is neither on PATH nor reliably reachable. On alpha it worked — because /home/pastilhas happens to be mode 751, world-traversable, so an absolute path resolved. At 750, the more usual default, every member would have lost every binary with nothing reporting it: the install would still have said satisfied, having only ever asked the owner. Nobody chose 751, and that is the part worth fixing. `scope: 'owner' | 'shared'` on an osDependency, defaulting to owner, an unrecognised value being a manifest error rather than a silent fall back to the permissive one. Shared changes three things. It is searched for on system directories only — asking the owner's shell would find the copy in the owner's home and call it satisfied, the exact false pass this exists to stop. Reachability is checked as world-executable AND every parent traversable to the root, because a binary at 755 inside a home at 700 is unreachable and stat on the file alone says it is fine. And a release binary goes to /usr/local/bin through sudo -n install, staged in the owner's space first so the download itself never runs as root. An unreachable shared dependency gets its own state and refuses the install ahead of every other check, because no package manager fixes it: the binary is installed and works, and what is wrong is where it lives — reinstalling would report success and change nothing. Not a security boundary, and docs/plugin-dependency-scope.md says so: anything in /usr/local/bin is runnable by any account with a shell, grant or no grant. That is already true of ffmpeg. The plugin's data is gated; its executable is not. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 4 + docs/plugin-dependency-scope.md | 91 ++++++++++++++++ src/servers/plugins/manifest.ts | 24 ++++- src/servers/plugins/os-deps.test.ts | 76 ++++++++++++++ src/servers/plugins/os-deps.ts | 155 ++++++++++++++++++++++++---- 5 files changed, 331 insertions(+), 19 deletions(-) create mode 100644 docs/plugin-dependency-scope.md diff --git a/CLAUDE.md b/CLAUDE.md index f42a751d..6e3907c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -438,6 +438,10 @@ explaining why it was safe. mounted and what it knows, the URL-vs-channel split for panel-to-panel communication, and the persistence key families. Read before building a panel app. Its defect list is `docs/workspace-panel-todo.md`. +- `docs/plugin-dependency-scope.md` — a plugin declares WHO must be able to run each host binary. Why the + owner's login shell is the wrong question for a `confined` plugin, the two-part reachability check + (world-executable AND every parent traversable), and why members' binaries on alpha worked only because + one home directory happened to be mode 751 - `docs/plugin-route-authorization.md` — **design, not built**: making a plugin's settings routes owner-only while the plugin itself stays grantable. What the proxy should pass (`isOwner` and a level, never the role name), why declaring owner-only paths in the manifest beats trusting every plugin to diff --git a/docs/plugin-dependency-scope.md b/docs/plugin-dependency-scope.md new file mode 100644 index 00000000..91345717 --- /dev/null +++ b/docs/plugin-dependency-scope.md @@ -0,0 +1,91 @@ +# Plugin dependency scope + +**Built 2026-08-21.** A plugin declares WHO has to be able to run each host binary, and the platform +installs and checks accordingly. + +## The bug + +`os-deps.ts` probes a dependency by asking the owner's login shell where the binary is. That is the right +question for most plugins and the wrong one for some, and nothing distinguished them — a per-user answer +was reported as a fact about the machine. + +It matters because `confined` plugins do their work as **the member's own Linux user**, in their own home. +A binary in the owner's `~/.local/bin` is not on that user's PATH, and reaching it means traversing the +owner's home directory. + +On alpha it happened to work. `/home/pastilhas` is mode **751** — world-traversable, not readable — so an +absolute path resolved for members. Mode 750, which is the more usual default, would have broken every +member's binary with nothing reporting anything: the install would still say satisfied, because the +install only ever asked the owner. + +Working by accident is the part worth fixing. Nobody chose 751. + +## The field + +```ts +osDependencies: [{ binary: 'iris', reason: '…', packages: {…}, scope: 'shared' }] +``` + +`owner` is the default and stays right for most things — anything an `execution` plugin uses runs as the +owner in the owner's home by definition. `shared` says every account with a Linux user must be able to run +it, which changes both the install target and the check. + +An unrecognised value is a manifest error rather than a silent fallback to `owner`, because the fallback +is the permissive one. + +## What changes when it is `shared` + +**Where it is looked for.** System directories only — `/usr/local/bin`, `/usr/bin`, `/bin`, and the sbin +trio. Deliberately not the owner's login shell: that shell finds the copy in the owner's home and reports +the dependency satisfied, which is the exact false answer this exists to stop. + +**What counts as reachable.** Two things, and checking only the first is how this hid: + +- the file is world-executable (`0o001` — group is not enough, members are not in the owner's group) +- **every directory above it is traversable**, walked to the root + +A binary at 755 inside a home at 700 is unreachable, and `stat` on the file alone says it is fine. + +**Where a release binary is installed.** `/usr/local/bin`, via `sudo -n install -m 0755`. The download is +still staged in the owner's own space — fetching straight into a root-owned directory would mean running +the download as root, which is a bigger ask than moving the result. `-n` so a plugin install never blocks +on an invisible password prompt. + +**What blocks an install.** A shared dependency that is present and working but unreachable gets its own +state, `unreachable`, and its own refusal — checked before everything else, because no package manager +fixes it. The binary is installed; what is wrong is where it lives. Reinstalling the package would report +success and change nothing. + +## What this deliberately does not do + +**It does not gate execution on a grant.** A binary in `/usr/local/bin` is runnable by any account with a +shell, whether or not they hold the plugin's permission. The kernel does not know about `role_permissions` +and never will. + +That is the same boundary `ffmpeg` and everything else in `/usr/bin` already has, and it is the right one: +the plugin's **data** is gated by the permission system, its **executable** is not. For iris it is benign — +a member who runs it captures their own `$HOME` profile and their own sessions, which is the isolation the +wrapper already depends on. + +If a binary must not merely be _reachable_ but _unusable_ without a grant, `scope` is not the mechanism. +That needs the plugin's own surface behind the permission gate, and the binary not on anyone's PATH. + +## Iris + +The motivating case, and still the only `shared` one. `PLUGIN.md` had already recorded it as open item 3 — +"the binary lands in the owner's `~/.local/bin`, and a confined member's agent cannot read it there" — but +that had not been connected to the probe being owner-scoped, so the install reported success either way. + +`iris` and `iris-browser` are built by the sidecar rather than fetched, so declaring `scope: 'shared'` +makes the probe honest about who can run them; moving the sidecar's build output into `/usr/local/bin` is +the remaining half, and it is a change to the sidecar rather than to this. + +## Status + +| | | +| ----- | -------------------------------------------------------------------------- | +| Built | `scope` on a dependency, validated, defaulting to `owner` | +| Built | shared probing on system paths, with a traversal walk to the root | +| Built | shared release binaries installed to `/usr/local/bin` via `sudo -n` | +| Built | an unreachable shared dependency refuses the install, naming the directory | +| Open | iris's sidecar still builds into `~/.local/bin` — see above | diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index d3bbf902..178b67fb 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -106,6 +106,25 @@ export type OsDependency = { * to the owner rather than a guess. */ packages: Partial>; + /** + * WHO has to be able to run this. + * + * `owner` (the default) — only the server owner runs it. Correct for anything an `execution` plugin + * uses, since those run as the owner in the owner's home by definition. + * + * `shared` — every account with a Linux user must be able to run it, because a `confined` plugin's + * work happens as THEM. That is a different install target and a different check: a binary in the + * owner's `~/.local/bin` is neither on a member's PATH nor guaranteed readable, since reaching it + * means traversing the owner's home. + * + * The distinction was invisible until it was looked for. On alpha the members' binaries "worked" + * only because `/home/` happened to be mode 751 — world-traversable — so an absolute path + * resolved. Mode 750 would have broken every member silently, and no probe would have noticed, + * because the probe only ever asked the owner. + * + * See `docs/plugin-dependency-scope.md`. + */ + scope?: 'owner' | 'shared'; /** * A release binary, for a dependency no package manager carries. * @@ -466,7 +485,7 @@ function osDependencyProblems(deps: unknown): string[] { problems.push(`${at} must be an object`); continue; } - const { binary, reason, packages } = dep as Partial; + const { binary, reason, packages, scope } = dep as Partial; if (typeof binary !== 'string' || !BINARY_RE.test(binary)) { problems.push(`${at}.binary must be a plain command name — letters, digits, dot, dash, underscore`); @@ -476,6 +495,9 @@ function osDependencyProblems(deps: unknown): string[] { if (typeof reason !== 'string' || !reason.trim()) { problems.push(`${at}.reason is required — say what stops working without it`); } + if (scope !== undefined && scope !== 'owner' && scope !== 'shared') { + problems.push(`${at}.scope must be "owner" or "shared" when present`); + } if (!packages || typeof packages !== 'object') { problems.push(`${at}.packages must be an object keyed by package manager`); continue; diff --git a/src/servers/plugins/os-deps.test.ts b/src/servers/plugins/os-deps.test.ts index 3ee3d967..9ef409b2 100644 --- a/src/servers/plugins/os-deps.test.ts +++ b/src/servers/plugins/os-deps.test.ts @@ -228,3 +228,79 @@ describe('identity — presence is not the same as being the right program', () expect(status.imposter).toBe(false); }); }); + +// ── scope: who has to be able to run it ── +// +// The bug this pins is a false PASS, which is the hard kind to notice. The probe asked the owner's login +// shell where a binary was; for a `confined` plugin the work happens as somebody else's Linux user, and +// nothing checked whether they could reach it. On alpha they could — but only because `/home/` +// happened to be mode 751. At 750, the more usual default, every member would have lost every binary +// with no probe reporting anything. + +describe('osDependencies scope validation', () => { + test('absent is fine — owner-scoped is the default and most dependencies are', () => { + expect( + manifestProblems('probe', withDeps([{ binary: 'ffmpeg', reason: 'r', packages: { apt: 'ffmpeg' } }])), + ).toEqual([]); + }); + + test('owner and shared are accepted', () => { + for (const scope of ['owner', 'shared']) { + expect( + manifestProblems('probe', withDeps([{ binary: 'ffmpeg', reason: 'r', packages: { apt: 'ffmpeg' }, scope }])), + ).toEqual([]); + } + }); + + test('anything else is a manifest error, not a silent default', () => { + const problems = manifestProblems( + 'probe', + withDeps([{ binary: 'ffmpeg', reason: 'r', packages: { apt: 'ffmpeg' }, scope: 'everyone' }]), + ); + expect(problems.some((p) => p.includes('scope must be "owner" or "shared"'))).toBe(true); + }); +}); + +describe('a shared dependency is judged on where it lives, not just whether it exists', () => { + const scratch = join(tmpdir(), `os-deps-scope-${process.pid}`); + afterAll(() => rmSync(scratch, { recursive: true, force: true })); + + const pluginWith = (dep: OsDependency): DiscoveredPlugin => + ({ + appName: 'probe', + dir: scratch, + manifest: { ...base, osDependencies: [dep] }, + }) as unknown as DiscoveredPlugin; + + test('a shared binary that exists only in the owner’s home reads as missing', () => { + // The owner's login shell would find this and call the dependency satisfied. A member's shell would + // not: it is not on their PATH, and reaching it means traversing a home that may refuse them. + const ownerBin = join(homedir(), '.local', 'bin'); + mkdirSync(ownerBin, { recursive: true }); + const name = `os-deps-scope-probe-${process.pid}`; + const path = join(ownerBin, name); + writeFileSync(path, '#!/bin/sh\necho probe\n', { mode: 0o755 }); + + try { + const owned = reportDependencies(pluginWith({ binary: name, reason: 'r', packages: {} })); + expect(owned.dependencies[0]!.present).toBe(true); + + const shared = reportDependencies(pluginWith({ binary: name, reason: 'r', packages: {}, scope: 'shared' })); + expect(shared.dependencies[0]!.present).toBe(false); + // Not "unreachable" — from a shared point of view it was never found at all, because the search + // never leaves the system directories. + expect(shared.dependencies[0]!.path).toBe(null); + expect(shared.dependencies[0]!.scope).toBe('shared'); + } finally { + rmSync(path, { force: true }); + } + }); + + test('a real system binary satisfies a shared dependency', () => { + // `sh` is in /bin on every machine this runs on, world-executable, behind nothing. + const report = reportDependencies(pluginWith({ binary: 'sh', reason: 'r', packages: {}, scope: 'shared' })); + expect(report.dependencies[0]!.present).toBe(true); + expect(report.dependencies[0]!.unreachable).toBe(false); + expect(report.canInstall).toBe(true); + }); +}); diff --git a/src/servers/plugins/os-deps.ts b/src/servers/plugins/os-deps.ts index beeb1654..3fd5071a 100644 --- a/src/servers/plugins/os-deps.ts +++ b/src/servers/plugins/os-deps.ts @@ -1,6 +1,6 @@ -import { chmodSync, existsSync, mkdirSync, renameSync } from 'node:fs'; +import { chmodSync, existsSync, mkdirSync, renameSync, rmSync, statSync } from 'node:fs'; import { homedir, userInfo } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { PACKAGE_MANAGERS, type DiscoveredPlugin, type OsDependency, type PackageManager } from './manifest'; // Host binaries a plugin needs, and getting them onto the machine. @@ -69,6 +69,18 @@ export type DependencyStatus = { * succeeds and every call afterwards fails strangely. */ imposter: boolean; + /** Who has to run it. From the manifest; `owner` when unstated. */ + scope: 'owner' | 'shared'; + /** + * Declared `shared`, found, and NOT reachable by other accounts — wrong permissions, or living behind + * a directory they cannot traverse. + * + * Its own state rather than folded into `present`, because the remedy is different and so is the + * blame: the binary is installed and works for the owner, and what is broken is where it was put. + */ + unreachable: boolean; + /** Why it is unreachable, for the install log. */ + unreachableReason: string | null; }; /** `linux`/`darwin` and `amd64`/`arm64` — the spelling Go releases use, which is who publishes this way. */ @@ -166,18 +178,80 @@ export function locateBinary(binary: string): string | null { ); } +/** + * Directories every account's PATH has, and that no one's home stands in front of. + * + * `/usr/local/bin` first: it is where a shared dependency belongs, and where `installRelease` puts one. + */ +const SHARED_DIRS = ['/usr/local/bin', '/usr/bin', '/bin', '/usr/local/sbin', '/usr/sbin', '/sbin']; + +/** + * Can an account that is not the owner actually run this? + * + * Two things have to hold, and only checking the first is how this hid: the file must be + * world-executable, AND every directory above it must be traversable. A binary at 755 inside a home at + * 700 is unreachable, and `stat` on the file alone says it is fine. + * + * On alpha the members' binaries worked only because `/home/` happened to be 751. Mode 750 — + * which is the more usual default — would have broken every member with nothing reporting it. + */ +function reachableByOthers(path: string): { ok: boolean; reason: string | null } { + try { + const mode = statSync(path).mode; + // 0o001 — the execute bit for "other". Group is not enough: members are not in the owner's group. + if (!(mode & 0o001)) return { ok: false, reason: `${path} is not world-executable` }; + } catch { + return { ok: false, reason: `${path} cannot be read` }; + } + + let dir = dirname(path); + while (true) { + try { + if (!(statSync(dir).mode & 0o001)) { + return { ok: false, reason: `${dir} is not traversable by other accounts` }; + } + } catch { + return { ok: false, reason: `${dir} cannot be read` }; + } + const parent = dirname(dir); + if (parent === dir) return { ok: true, reason: null }; + dir = parent; + } +} + +/** + * Where a SHARED binary is — searched on system directories only. + * + * Deliberately not the owner's login shell. That shell would happily find the copy in the owner's + * `~/.local/bin` and report the dependency satisfied, which is the exact false answer this scope exists + * to stop: true for one account, presented as true for the machine. + */ +function locateShared(binary: string): string | null { + for (const dir of SHARED_DIRS) { + const candidate = join(dir, binary); + if (existsSync(candidate)) return candidate; + } + return null; +} + /** Probe one dependency the way the owner's shell would see it. */ function probe(dep: OsDependency, manager: PackageManager | null): DependencyStatus { - const path = locateBinary(dep.binary); + const scope = dep.scope ?? 'owner'; + const path = scope === 'shared' ? locateShared(dep.binary) : locateBinary(dep.binary); const genuine = path ? identifies(dep, path) : false; + const reach = path && scope === 'shared' ? reachableByOthers(path) : { ok: true, reason: null }; + return { binary: dep.binary, reason: dep.reason, path, - // Present means USABLE. A namesake on PATH is not the dependency, and calling it present is how the - // wrong program gets used for a week before anyone works out why. - present: !!path && genuine, + // Present means USABLE — by whoever has to use it. A namesake on PATH is not the dependency, and for + // a shared dependency neither is a copy only the owner can reach. + present: !!path && genuine && reach.ok, imposter: !!path && !genuine, + scope, + unreachable: !!path && genuine && !reach.ok, + unreachableReason: reach.reason, packageName: (manager && dep.packages[manager]) || null, releaseUrl: releaseUrlFor(dep), }; @@ -236,7 +310,22 @@ export function reportDependencies(plugin: DiscoveredPlugin): DependencyReport { let canInstall = true; let blockedReason: string | null = null; - if (missing.length === 0) { + // A shared dependency that IS installed but sits somewhere only the owner can reach. Checked before + // everything else because no package manager fixes it: the binary is present and working, and what is + // wrong is where it lives. Reinstalling the package would report success and change nothing. + const unreachable = dependencies.filter((d) => d.unreachable); + + if (unreachable.length) { + canInstall = false; + blockedReason = + `${unreachable.map((d) => d.binary).join(', ')} must be runnable by every account, and ` + + `${unreachable.length === 1 ? 'is' : 'are'} not: ` + + `${unreachable + .map((d) => d.unreachableReason) + .filter(Boolean) + .join('; ')}. ` + + `Move ${unreachable.length === 1 ? 'it' : 'them'} to /usr/local/bin.`; + } else if (missing.length === 0) { // Nothing to do, so nothing can block it. Said explicitly because the checks below would otherwise // report "no package manager" on a machine that needs nothing — true, and useless. } else if (!manager) { @@ -292,12 +381,15 @@ export function manualInstallHint(manager: PackageManager | null, packages: stri } /** - * Fetch a release binary into `~/.local/bin`. + * Fetch a release binary — into `~/.local/bin`, or `/usr/local/bin` when the dependency is `shared`. * - * For the dependency no package manager carries. Not root, not `/usr/local/bin`: a binary a plugin - * fetched from the internet has no business sitting beside what the distribution installed, and needing - * sudo for it would be a worse trade than needing PATH to include `~/.local/bin` — which it already does - * on every machine this platform sets up. + * For the dependency no package manager carries. Owner-scoped ones stay out of root's directories: a + * binary a plugin fetched from the internet has no business sitting beside what the distribution + * installed, and needing sudo for it would be a worse trade than needing PATH to include `~/.local/bin`. + * + * A `shared` dependency has no such choice. Members' accounts cannot rely on reaching anything inside + * the owner's home — see `scope` in manifest.ts — so it goes to `/usr/local/bin` through the same + * `sudo -n` the package path already uses, and the install says so rather than silently needing root. * * ── What is checked, and what is not ── * @@ -313,19 +405,46 @@ export function manualInstallHint(manager: PackageManager | null, packages: stri export async function installRelease(dep: DependencyStatus): Promise<{ ok: boolean; path?: string; error?: string }> { if (!dep.releaseUrl) return { ok: false, error: `${dep.binary} ships no release binary` }; - const dir = join(homedir(), '.local', 'bin'); + const shared = dep.scope === 'shared'; + const dir = shared ? '/usr/local/bin' : join(homedir(), '.local', 'bin'); const target = join(dir, dep.binary); try { - mkdirSync(dir, { recursive: true }); + // Staged in the owner's own space either way. Downloading straight into a root-owned directory would + // need sudo for the fetch as well, and running a download as root is a bigger ask than moving one. + const staging = join(homedir(), '.local', 'bin'); + mkdirSync(staging, { recursive: true }); + const tmp = join(staging, `${dep.binary}.download`); + const res = await fetch(dep.releaseUrl, { redirect: 'follow', signal: AbortSignal.timeout(120_000) }); if (!res.ok) return { ok: false, error: `${dep.releaseUrl} answered ${res.status}` }; - // Written to a temporary name and moved into place, so a half-downloaded file is never on PATH under - // the real name — the failure that leaves a machine looking installed and behaving broken. - const tmp = `${target}.download`; await Bun.write(tmp, res); + // 0o755 for both: a shared binary has to be executable by other accounts, and an owner-scoped one is + // no worse off for it — what keeps that one private is the home directory above it. chmodSync(tmp, 0o755); - renameSync(tmp, target); + + if (!shared) { + // Moved rather than copied, so a half-downloaded file is never on PATH under the real name. + renameSync(tmp, target); + return { ok: true, path: target }; + } + + // `-n`: never prompt. A plugin install that silently blocks on a password nobody can see is worse + // than one that fails saying it needed root — the same reason the package path passes it. + const moved = Bun.spawnSync(['sudo', '-n', 'install', '-m', '0755', tmp, target], { + stdout: 'pipe', + stderr: 'pipe', + }); + rmSync(tmp, { force: true }); + if (moved.exitCode !== 0) { + const why = new TextDecoder().decode(moved.stderr).trim(); + return { + ok: false, + error: `${dep.binary} must go in ${dir} because every account has to run it, and that needs root: ${ + why || 'sudo refused' + }`, + }; + } return { ok: true, path: target }; } catch (err) { return { ok: false, error: err instanceof Error ? err.message : String(err) };