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) };