diff --git a/src/servers/plugins/install.ts b/src/servers/plugins/install.ts index f94d7ec6..63f4d7cd 100644 --- a/src/servers/plugins/install.ts +++ b/src/servers/plugins/install.ts @@ -241,7 +241,7 @@ export async function installPlugin( await step(steps, onStep, `dependencies: installing ${deps.packagesToInstall.join(', ')} with ${deps.manager}`); } const installed = deps.packagesToInstall.length - ? await installDependencies(deps.manager!, deps.packagesToInstall) + ? await installDependencies(deps.manager!, deps.packagesToInstall, deps.architectures) : { ok: true as const, installed: [], output: '' }; if (!installed.ok) { return { diff --git a/src/servers/plugins/manifest.ts b/src/servers/plugins/manifest.ts index bec32d5e..e619120f 100644 --- a/src/servers/plugins/manifest.ts +++ b/src/servers/plugins/manifest.ts @@ -105,7 +105,7 @@ export type OsDependency = { * which for a binary that is present anyway is correct and silent, and otherwise becomes an instruction * to the owner rather than a guess. */ - packages: Partial>; + packages: Partial>; /** * WHO has to be able to run this. * @@ -125,6 +125,33 @@ export type OsDependency = { * See `docs/plugin-dependency-scope.md`. */ scope?: 'owner' | 'shared'; + /** + * An architecture this dependency needs enabled before its package can be installed. + * + * Debian and Ubuntu only, and it means exactly one thing: run `dpkg --add-architecture ` followed + * by `apt-get update` before installing, if that architecture is not already enabled. + * + * ── Why this is a field and not a package name ── + * + * Because it cannot be one. A 32-bit package on a 64-bit host is `wine32:i386`, and installing it fails + * outright until the host has been told i386 exists — which is a change to the machine's dpkg + * configuration, not a package to fetch. Minesweeper found this the direct way: `wine` alone installs + * 64-bit-only, the plugin reports a clean install, and the game dies with `Bad EXE format`. + * + * ── Why it is an enum and not a command ── + * + * This runs as root. A manifest that could supply the command would be a manifest that can run anything + * as root on every machine that installs it, which is a far larger grant than "this plugin may install + * wine". The manifest picks a value from `DPKG_ARCHITECTURES`; the platform owns the sentence it turns + * into. Same principle as the plugin websocket naming a sidecar rather than a port. + * + * It is also SYSTEM-WIDE and does not go away when the plugin does. `plugins:delete` will not disable an + * architecture something else may now depend on, so this is shown to the owner before they consent + * rather than mentioned in a log afterwards. + * + * Ignored where it means nothing — a pacman or brew host is already multi-arch or has no such concept. + */ + multiarch?: DpkgArchitecture; /** * A release binary, for a dependency no package manager carries. * @@ -509,7 +536,26 @@ export const PACKAGE_MANAGERS: PackageManager[] = ['apt', 'pacman', 'dnf', 'brew * not a regex that already allows it. */ const BINARY_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$/; -const PACKAGE_RE = /^[a-z0-9][a-z0-9.+-]{0,63}$/; +/** + * A package name, optionally qualified by an architecture. + * + * `wine32:i386` is the motivating case and it is not an edge case: on Debian and Ubuntu a 32-bit package + * on a 64-bit host can only be named that way. The suffix is drawn from `DPKG_ARCHITECTURES` below rather + * than being any word, so this stays a closed vocabulary — the point of a narrow regex here is that a + * manifest cannot get a metacharacter onto an `apt-get` command line, and widening it to `[a-z0-9]+` + * after the colon would still be safe but would stop being a vocabulary. + */ +const PACKAGE_RE = /^[a-z0-9][a-z0-9.+-]{0,63}(:(i386|amd64|arm64|armhf|ppc64el|s390x))?$/; + +/** + * Architectures `dpkg --add-architecture` accepts, and the only values a manifest may ask to enable. + * + * Closed on purpose. `multiarch` turns into a command run as root, so the manifest gets to pick from a + * list rather than to supply a string — the same reasoning as the package regex one line up, applied to + * the one field that would otherwise be a way to say "run this as root" out loud. + */ +export const DPKG_ARCHITECTURES = ['i386', 'amd64', 'arm64', 'armhf', 'ppc64el', 's390x'] as const; +export type DpkgArchitecture = (typeof DPKG_ARCHITECTURES)[number]; /** Reasons an `osDependencies` list is unusable. Empty when it is fine. */ function osDependencyProblems(deps: unknown): string[] { @@ -540,6 +586,13 @@ function osDependencyProblems(deps: unknown): string[] { problems.push(`${at}.packages must be an object keyed by package manager`); continue; } + const multiarch = (dep as { multiarch?: unknown }).multiarch; + if (multiarch !== undefined && !DPKG_ARCHITECTURES.includes(multiarch as never)) { + // Refused rather than ignored: a typo here silently drops the one step that makes the package + // installable at all, and the failure surfaces much later as the plugin not working. + problems.push(`${at}.multiarch must be one of ${DPKG_ARCHITECTURES.join(', ')}`); + } + for (const [manager, name] of Object.entries(packages)) { if (!PACKAGE_MANAGERS.includes(manager as PackageManager)) { problems.push( @@ -547,8 +600,19 @@ function osDependencyProblems(deps: unknown): string[] { ); continue; } - if (typeof name !== 'string' || !PACKAGE_RE.test(name)) { - problems.push(`${at}.packages.${manager} must be a plain package name — lowercase, digits, dot, plus, dash`); + // A LIST is allowed, because one binary genuinely can need more than one package. wine is the case: + // `wine` provides the launcher on PATH and `wine32:i386` provides the 32-bit loader it needs to run + // an i386 executable, and installing either alone produces a wine that cannot run the thing the + // plugin came for. Every element is validated the same way and they reach apt as separate argv + // entries, so a list is no more powerful than the names in it. + const names = Array.isArray(name) ? name : [name]; + const bad = names.length === 0 || names.some((n) => typeof n !== 'string' || !PACKAGE_RE.test(n)); + if (bad) { + problems.push( + `${at}.packages.${manager} must be a plain package name (or a non-empty list of them) — ` + + `lowercase, digits, dot, plus, dash, optionally with an architecture suffix ` + + `(${DPKG_ARCHITECTURES.join(', ')})`, + ); } } } diff --git a/src/servers/plugins/marketplace.ts b/src/servers/plugins/marketplace.ts index 21a05e8b..246362d8 100644 --- a/src/servers/plugins/marketplace.ts +++ b/src/servers/plugins/marketplace.ts @@ -274,7 +274,8 @@ const BUNDLED: CatalogueEntry[] = [ binary: 'wine', reason: 'Runs the game. Minesweeper X is a 32-bit Windows executable, so this host also needs i386 multiarch (`dpkg --add-architecture i386`) and the 32-bit wine package — a 64-bit-only wine cannot load it.', - packages: { apt: 'wine', dnf: 'wine', pacman: 'wine', brew: 'wine-stable' }, + multiarch: 'i386', + packages: { apt: ['wine', 'wine32:i386'], dnf: 'wine', pacman: 'wine', brew: 'wine-stable' }, }, { binary: 'Xvfb', diff --git a/src/servers/plugins/multiarch.test.ts b/src/servers/plugins/multiarch.test.ts new file mode 100644 index 00000000..38392e82 --- /dev/null +++ b/src/servers/plugins/multiarch.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, test } from 'bun:test'; +import { DPKG_ARCHITECTURES, manifestProblems } from './manifest'; +import { reportDependencies } from './os-deps'; +import type { DiscoveredPlugin, OsDependency } from './manifest'; + +/** + * A package cannot be installed for an architecture the host does not know about, and no package name can + * say so — which is why `multiarch` is a field rather than something clever in the package string. + * + * Minesweeper is the case that produced it: `apt-get install wine` on a 64-bit host succeeds, reports a + * clean plugin install, and leaves a 32-bit game dying with `Bad EXE format`. Nothing in that chain + * mentions architectures, so it is worth pinning the two properties that keep it working. + */ + +const dep = (over: Partial = {}): OsDependency => ({ + binary: 'definitely-not-a-real-binary-xyzzy', + reason: 'test', + packages: { apt: 'some-package' }, + ...over, +}); + +const plugin = (deps: OsDependency[]): DiscoveredPlugin => + ({ appName: 'probe', dir: '/tmp/probe', manifest: { osDependencies: deps } }) as unknown as DiscoveredPlugin; + +describe('multiarch is a closed vocabulary, not a command', () => { + test('a valid architecture is accepted', () => { + for (const arch of DPKG_ARCHITECTURES) { + const problems = manifestProblems('probe', { + publisher: 'x', + version: '1.0.0', + platform: '>=1.0.0', + label: 'X', + summary: 'x', + color: '#000000', + permissions: [], + osDependencies: [dep({ multiarch: arch })], + } as never); + expect(problems.filter((p) => p.includes('multiarch'))).toEqual([]); + } + }); + + // The whole point of the enum. A manifest that could supply the string could run it as root on every + // machine that installs the plugin, which is a far larger grant than "may install wine". + test('anything else is refused, including something shell-shaped', () => { + for (const bad of ['i486', 'I386', 'i386; rm -rf /', '$(whoami)', '']) { + const problems = manifestProblems('probe', { + publisher: 'x', + version: '1.0.0', + platform: '>=1.0.0', + label: 'X', + summary: 'x', + color: '#000000', + permissions: [], + osDependencies: [dep({ multiarch: bad as never })], + } as never); + expect(problems.some((p) => p.includes('multiarch'))).toBe(true); + } + }); + + test('a package name may carry a known architecture suffix, and only a known one', () => { + const ok = manifestProblems('probe', { + publisher: 'x', + version: '1.0.0', + platform: '>=1.0.0', + label: 'X', + summary: 'x', + color: '#000000', + permissions: [], + osDependencies: [dep({ packages: { apt: 'wine32:i386' } })], + } as never); + expect(ok.filter((p) => p.includes('packages'))).toEqual([]); + + // The list form, which is what wine actually needs. + const list = manifestProblems('probe', { + publisher: 'x', + version: '1.0.0', + platform: '>=1.0.0', + label: 'X', + summary: 'x', + color: '#000000', + permissions: [], + osDependencies: [dep({ packages: { apt: ['wine', 'wine32:i386'] } })], + } as never); + expect(list.filter((p) => p.includes('packages'))).toEqual([]); + + const bad = manifestProblems('probe', { + publisher: 'x', + version: '1.0.0', + platform: '>=1.0.0', + label: 'X', + summary: 'x', + color: '#000000', + permissions: [], + osDependencies: [dep({ packages: { apt: 'wine32:sparc' } })], + } as never); + expect(bad.some((p) => p.includes('packages'))).toBe(true); + }); +}); + +describe('the report tells the owner what will CHANGE', () => { + test('a missing dependency surfaces its architecture', () => { + const report = reportDependencies(plugin([dep({ multiarch: 'i386' })])); + expect(report.missing.length).toBe(1); + // Reported on every host. Whether it is ACTED on is apt-only — see `enableArchitectures` — but the + // owner is told what the plugin asks for regardless of what this machine will do about it. + expect(report.architectures).toEqual(['i386']); + }); + + // A binary already on PATH had its architecture enabled at some point in the past. Re-reporting it would + // tell the owner the machine is about to change when it is not. + test('a dependency that is already present does not', () => { + const report = reportDependencies(plugin([dep({ binary: 'sh', multiarch: 'i386' })])); + expect(report.missing.length).toBe(0); + expect(report.architectures).toEqual([]); + }); + + test('two dependencies wanting the same architecture report it once', () => { + const report = reportDependencies( + plugin([dep({ binary: 'nope-a-xyzzy', multiarch: 'i386' }), dep({ binary: 'nope-b-xyzzy', multiarch: 'i386' })]), + ); + expect(report.architectures).toEqual(['i386']); + }); +}); diff --git a/src/servers/plugins/os-deps.ts b/src/servers/plugins/os-deps.ts index 3fd5071a..0f7d6e80 100644 --- a/src/servers/plugins/os-deps.ts +++ b/src/servers/plugins/os-deps.ts @@ -1,7 +1,7 @@ import { chmodSync, existsSync, mkdirSync, renameSync, rmSync, statSync } from 'node:fs'; import { homedir, userInfo } from 'node:os'; import { dirname, join } from 'node:path'; -import { PACKAGE_MANAGERS, type DiscoveredPlugin, type OsDependency, type PackageManager } from './manifest'; +import { PACKAGE_MANAGERS, type DiscoveredPlugin, type DpkgArchitecture, type OsDependency, type PackageManager } from './manifest'; // Host binaries a plugin needs, and getting them onto the machine. // @@ -58,7 +58,13 @@ export type DependencyStatus = { path: string | null; present: boolean; /** What would be installed on THIS machine, or null when this manager has no entry for it. */ - packageName: string | null; + /** + * The package(s) that would provide this binary here, or null when this manager has no entry for it. + * + * A LIST because one binary can need more than one package — wine needs both its launcher and the 32-bit + * loader, and installing either alone gives a wine that cannot run what the plugin came for. + */ + packageName: string[] | null; /** Resolved download URL when the plugin ships a release binary, or null. */ releaseUrl: string | null; /** @@ -81,8 +87,21 @@ export type DependencyStatus = { unreachable: boolean; /** Why it is unreachable, for the install log. */ unreachableReason: string | null; + /** + * Carried through from the manifest so the report can say what installing this will CHANGE, not just + * what it will fetch. Enabling an architecture is system-wide and permanent. + */ + multiarch: DpkgArchitecture | null; }; +/** A dependency's packages for one manager, always as a list — the manifest allows either form. */ +export function packageNames(dep: OsDependency, manager: PackageManager): string[] | null { + const entry = dep.packages[manager]; + if (!entry) return null; + const names = Array.isArray(entry) ? entry : [entry]; + return names.length ? names : null; +} + /** `linux`/`darwin` and `amd64`/`arm64` — the spelling Go releases use, which is who publishes this way. */ export function hostTarget(): { os: string; arch: string } { const os = process.platform === 'darwin' ? 'darwin' : 'linux'; @@ -252,8 +271,9 @@ function probe(dep: OsDependency, manager: PackageManager | null): DependencySta scope, unreachable: !!path && genuine && !reach.ok, unreachableReason: reach.reason, - packageName: (manager && dep.packages[manager]) || null, + packageName: manager ? packageNames(dep, manager) : null, releaseUrl: releaseUrlFor(dep), + multiarch: dep.multiarch ?? null, }; } @@ -268,6 +288,15 @@ export type DependencyReport = { * has. These are the owner's to resolve by hand, and the install says so rather than guessing. */ unresolvable: DependencyStatus[]; + /** + * Architectures the missing packages need enabled first — `dpkg --add-architecture`, apt hosts only. + * + * Reported rather than done quietly because it is SYSTEM-WIDE and permanent: nothing here ever disables + * one, including uninstalling the plugin. The owner should see "this will change how this machine + * resolves packages" on the screen where they consent, not in a log afterwards. + */ + architectures: DpkgArchitecture[]; + /** Can the platform actually run the install — passwordless sudo, or a manager that never needs it. */ canInstall: boolean; /** Why not, when it cannot. Written to be shown to a person. */ @@ -305,7 +334,7 @@ export function reportDependencies(plugin: DiscoveredPlugin): DependencyReport { // Deduped and sorted: ffmpeg and ffprobe are one apt package, and naming it twice on the command line // is the sort of thing that reads as a bug in a log even though apt does not care. - const packagesToInstall = [...new Set(missing.map((d) => d.packageName).filter((n): n is string => !!n))].sort(); + const packagesToInstall = [...new Set(missing.flatMap((d) => d.packageName ?? []))].sort(); let canInstall = true; let blockedReason: string | null = null; @@ -343,7 +372,14 @@ export function reportDependencies(plugin: DiscoveredPlugin): DependencyReport { 'Install the packages yourself and then install the plugin.'; } - return { manager, dependencies, missing, packagesToInstall, unresolvable, canInstall, blockedReason }; + // Only for the ones actually being installed. A binary already present needed its architecture enabled + // at some point in the past, and re-reporting that would tell the owner the machine is about to change + // when it is not. + const architectures = [ + ...new Set(missing.map((d) => d.multiarch).filter((a): a is DpkgArchitecture => !!a)), + ]; + + return { manager, dependencies, missing, packagesToInstall, unresolvable, architectures, canInstall, blockedReason }; } /** The command that would be run, as argv. Exported so the UI can SHOW it — the owner should see it. */ @@ -451,6 +487,81 @@ export async function installRelease(dep: DependencyStatus): Promise<{ ok: boole } } + +/** + * Enable a foreign architecture, so a package that only exists for it can be installed. + * + * ── Why the platform runs this at all ── + * + * Because no package name can express it. `wine32:i386` is not installable until dpkg has been told the + * architecture exists, and that is a change to the machine's configuration rather than a package to + * fetch. Leaving it to the owner means an install that reports success and produces a plugin that does + * not work — which is exactly what happened with Minesweeper: `wine` alone lands 64-bit-only and the game + * dies with `Bad EXE format`, several steps away from anything that mentions architectures. + * + * ── What it will and will not do ── + * + * apt hosts only. pacman and brew have no equivalent, and dnf resolves multilib without being asked. + * + * It is SYSTEM-WIDE and permanent. Nothing here ever removes an architecture, including + * `plugins:delete` — something else may depend on it by then, and disabling one with packages installed + * against it is how a machine ends up unable to run `apt-get -f install`. The manifest field is shown to + * the owner before they consent for this reason. + * + * Already-enabled is success and costs one `dpkg` call. `apt-get update` runs only when something + * actually changed, because it is slow and there is no reason to pay for it otherwise. + */ +async function enableArchitectures( + manager: PackageManager, + architectures: DpkgArchitecture[], +): Promise<{ ok: boolean; output: string; error?: string }> { + const wanted = [...new Set(architectures)]; + if (!wanted.length || manager !== 'apt') return { ok: true, output: '' }; + if (!Bun.which('dpkg')) return { ok: true, output: '' }; + + const current = Bun.spawnSync(['dpkg', '--print-foreign-architectures']); + const enabled = new Set(new TextDecoder().decode(current.stdout).split(/\s+/).filter(Boolean)); + const missing = wanted.filter((a) => !enabled.has(a)); + if (!missing.length) return { ok: true, output: '' }; + + const escalate = sudoAvailable() ? ['sudo', '-n'] : []; + let output = ''; + + for (const arch of missing) { + // argv, never a shell string — and `arch` came from a closed enum in the manifest validator, so this + // is belt and braces rather than the only guard. Both halves are deliberate; see the header comment. + const proc = Bun.spawn([...escalate, 'dpkg', '--add-architecture', arch], { stdout: 'pipe', stderr: 'pipe' }); + const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]); + const code = await proc.exited; + output += `${out}${err}`; + if (code !== 0) { + return { + ok: false, + output: output.trim().slice(-600), + error: + `could not enable the ${arch} architecture, which this plugin's packages need. ` + + `Run it by hand: sudo dpkg --add-architecture ${arch} && sudo apt-get update`, + }; + } + } + + // The package lists have no entries for an architecture that was not enabled a moment ago, so without + // this the install that follows cannot find anything to install. + const upd = Bun.spawn([...escalate, 'apt-get', 'update'], { stdout: 'pipe', stderr: 'pipe' }); + const [uo, ue] = await Promise.all([new Response(upd.stdout).text(), new Response(upd.stderr).text()]); + const updCode = await upd.exited; + output += `${uo}${ue}`; + if (updCode !== 0) { + return { + ok: false, + output: output.trim().slice(-600), + error: `enabled ${missing.join(', ')} but \`apt-get update\` failed, so the new package lists are not available yet`, + }; + } + + return { ok: true, output: output.trim().slice(-600) }; +} + export type InstallDependenciesResult = { ok: boolean; installed: string[]; output: string; error?: string }; /** @@ -463,9 +574,16 @@ export type InstallDependenciesResult = { ok: boolean; installed: string[]; outp export async function installDependencies( manager: PackageManager, packages: string[], + architectures: DpkgArchitecture[] = [], ): Promise { if (!packages.length) return { ok: true, installed: [], output: '' }; + // Architectures FIRST, and the order is not cosmetic: `apt-get install wine32:i386` fails outright on a + // host that has not been told i386 exists, so doing this after would install nothing and report a + // package that does not exist. + const arch = await enableArchitectures(manager, architectures); + if (!arch.ok) return { ok: false, installed: [], output: arch.output, error: arch.error }; + try { const proc = Bun.spawn(installCommand(manager, packages), { stdout: 'pipe', stderr: 'pipe' }); const [out, err] = await Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]);