Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/genie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,21 @@ program
.option('--staging-root <path>')
.option('--expected-version <version>')
.option('--self-test')
.action((options: InstallPromoteCommandOptions) => installPromoteCommand(options));
.action((options: InstallPromoteCommandOptions) => {
try {
installPromoteCommand(options);
} catch (error) {
// Preflight/link failures are operator-fixable environment problems
// (e.g. a group-writable ~/.local/bin); print the remedy, never a stack.
const name = error instanceof Error ? error.name : '';
if (name === 'CanonicalInstallLinkError' || name === 'InstallPromoteCommandError') {
console.error(`\u2716 ${(error as Error).message}`);
process.exitCode = 1;
return;
}
throw error;
}
});

// Global --no-interactive flag: disables all interactive prompts (scripting safety)
program.option('--no-interactive', 'Disable interactive prompts (exit 2 instead of prompting)');
Expand Down
44 changes: 42 additions & 2 deletions src/lib/install-link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ import {
} from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { CanonicalInstallLinkError, prepareCanonicalInstallLink, verifyCanonicalInstallLink } from './install-link.js';
import {
CanonicalInstallLinkError,
classifyOwnedDirectorySafety,
preflightCanonicalInstallLink,
prepareCanonicalInstallLink,
verifyCanonicalInstallLink,
} from './install-link.js';

const roots: string[] = [];

Expand Down Expand Up @@ -130,6 +136,40 @@ describe('canonical installer link', () => {
});

for (const unsafeAncestor of ['.local', '.local/bin'] as const) {
test('accepts a 0775 ~/.local/bin owned by the user and their effective group (user-private-group umask 002)', () => {
const f = fixture();
const localBin = join(f.home, '.local', 'bin');
mkdirSync(localBin, { recursive: true, mode: 0o755 });
chmodSync(join(f.home, '.local'), 0o775);
chmodSync(localBin, 0o775);
// gid of a fresh dir is the process egid on non-setgid parents, matching the relaxation.
expect(() =>
preflightCanonicalInstallLink({
trustedHome: f.home,
linkPath: join(localBin, 'genie'),
targetPath: join(f.home, '.genie', 'bin', 'genie'),
}),
).not.toThrow();
});

test('classifyOwnedDirectorySafety: pure verdicts', () => {
const uid = 1000n;
const gid = 1000n;
const mk = (mode: number, o: Partial<{ uid: bigint; gid: bigint; nlink: bigint }> = {}) =>
({ uid: o.uid ?? uid, gid: o.gid ?? gid, nlink: o.nlink ?? 2n, mode: BigInt(0o40000 | mode) }) as never;
expect(classifyOwnedDirectorySafety(mk(0o755), { uid, gid })).toEqual({ ok: true });
expect(classifyOwnedDirectorySafety(mk(0o775), { uid, gid })).toEqual({ ok: true });
const foreign = classifyOwnedDirectorySafety(mk(0o775, { gid: 999n }), { uid, gid });
expect(foreign.ok).toBe(false);
if (!foreign.ok) expect(foreign.reason).toContain('group 999');
const world = classifyOwnedDirectorySafety(mk(0o777), { uid, gid });
expect(world.ok).toBe(false);
if (!world.ok) expect(world.reason).toContain('world-writable (mode 777)');
const sudo = classifyOwnedDirectorySafety(mk(0o755, { uid: 0n }), { uid, gid });
expect(sudo.ok).toBe(false);
if (!sudo.ok) expect(sudo.reason).toContain('owned by uid 0');
});

test(`rejects a group/world-writable ${unsafeAncestor} PATH ancestor`, () => {
const f = fixture();
const localBin = join(f.home, '.local', 'bin');
Expand All @@ -138,7 +178,7 @@ describe('canonical installer link', () => {

expect(() =>
prepareCanonicalInstallLink({ trustedHome: f.home, linkPath: f.link, targetPath: f.target }),
).toThrow('safe permissions');
).toThrow('writable');
expect(existsSync(f.link)).toBe(false);
});
}
Expand Down
47 changes: 45 additions & 2 deletions src/lib/install-link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,48 @@ function currentUid(): bigint {
return BigInt(process.getuid());
}

function currentGid(): bigint {
if (process.getegid === undefined)
throw new CanonicalInstallLinkError('canonical link requires a POSIX group identity');
return BigInt(process.getegid());
}

/**
* Pure safety classifier for a parent directory of the canonical install link.
* Ownership contract: owned by the current uid, at least one hard link, never
* world-writable, and group-writable ONLY when the directory's group is the
* process's effective group — the Debian/Ubuntu user-private-group layout
* (umask 002 → `~/.local/bin` is 0775 <user>:<user>), which is as private as
* 0755. A group-writable directory owned by any other group stays rejected:
* unknown group members could swap the `genie` link.
*/
export function classifyOwnedDirectorySafety(
stat: Pick<BigIntStats, 'uid' | 'gid' | 'nlink' | 'mode'>,
identity: { uid: bigint; gid: bigint },
): { ok: true } | { ok: false; reason: string; remedy: string } {
const mode = Number(stat.mode & 0o777n);
const octal = mode.toString(8).padStart(3, '0');
if (stat.uid !== identity.uid) {
return {
ok: false,
reason: `is owned by uid ${stat.uid}, not the current user (uid ${identity.uid})`,
remedy: 'chown it to your user (it may have been created with sudo), then retry',
};
}
if (stat.nlink < 1n) return { ok: false, reason: 'has no hard links', remedy: 'recreate the directory, then retry' };
if ((mode & 0o002) !== 0) {
return { ok: false, reason: `is world-writable (mode ${octal})`, remedy: 'run: chmod o-w <path>, then retry' };
}
if ((mode & 0o020) !== 0 && stat.gid !== identity.gid) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prove the effective group is private before accepting 0775

On systems where the user's effective group is shared, a 0775 parent has stat.gid === identity.gid even though other group members can rename or replace ~/.local/bin/genie. This condition therefore removes the previous protection and permits a local group member to substitute the executable reached through the user's PATH; matching the effective GID proves membership, not that this is a user-private group, so the relaxation must verify exclusivity or continue rejecting group-write permission.

Useful? React with 👍 / 👎.

return {
ok: false,
reason: `is writable by group ${stat.gid}, which is not your effective group (gid ${identity.gid}); mode ${octal}`,
remedy: 'run: chmod g-w <path>, then retry',
};
}
return { ok: true };
}

function fdReferencePath(fd: number): string {
if (process.platform === 'linux') return `/proc/self/fd/${fd}`;
if (process.platform === 'darwin') return `/dev/fd/${fd}`;
Expand Down Expand Up @@ -127,8 +169,9 @@ function assertSafeOwnedDirectoryStat(stat: BigIntStats, label: string): void {
if (!stat.isDirectory() || stat.isSymbolicLink()) {
throw new CanonicalInstallLinkError(`${label} is not a physical directory`);
}
if (stat.uid !== currentUid() || stat.nlink < 1n || Number(stat.mode & 0o022n) !== 0) {
throw new CanonicalInstallLinkError(`${label} is not current-user-owned with safe permissions`);
const verdict = classifyOwnedDirectorySafety(stat, { uid: currentUid(), gid: currentGid() });
if (!verdict.ok) {
throw new CanonicalInstallLinkError(`${label} ${verdict.reason} — ${verdict.remedy.replace('<path>', label)}`);
}
}

Expand Down
Loading