linux accounts use the chosen username, and refuse to take one over
Two changes, and the second is what makes the first safe. The officer_ prefix is gone: a member's account is the username the owner typed, so whoami says who they are and a commit from their checkout is attributed to something recognisable. Measured first — useradd on this host accepts everything validateUsername permits, including dots, hyphens, underscores and uppercase. The prefix was also load-bearing, though, and not for looks. ensureOsUser REUSES an existing account, which is what makes it re-runnable, and that was safe by construction while only we created officer_* names. Unprefixed, adoption becomes the dangerous path: a platform account named root would have found root in passwd, and every runAs for that member would have been a root shell. So adoption now requires the existing account's passwd home to be exactly the home we are about to confine — that is what makes it ours — and any uid below 1000 is refused outright. Verified: root and daemon refused as system accounts, and the owner's own username refused by name with its real home quoted back. Also, the ancestor trap from the first real install. A member's home is under DATA_PATH, which is under the OWNER'S home, and /home/<owner> is 750 on Debian and Ubuntu — so every mode bit on the account tree was right, the directory existed, and the member still could not reach it for want of x four levels up. It surfaced as "ssh-keygen: Could not stat …/.ssh: Permission denied", which points at the wrong thing entirely. firstUntraversableAncestor now walks the chain as the member before anything uses the home, and the error names the directory and the chmod. The dev machine was already 751, and the probe used /tmp, so it never crossed the ancestor that mattered. Worth remembering as a shape of mistake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+17
-16
@@ -2,7 +2,7 @@ import { describe, expect, test } from 'bun:test';
|
||||
import { chmod, mkdtemp, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { findReadableSecrets, osUserNameFor, runAsArgv, OS_USER_PREFIX } from './os-user';
|
||||
import { findReadableSecrets, osUserNameFor, runAsArgv } from './os-user';
|
||||
|
||||
// The tests that matter here are the two that prove the MECHANISM rather than the plumbing: that Bun
|
||||
// ignores `uid`, and that `setpriv` does not. Everything else in os-user.ts touches the passwd database
|
||||
@@ -119,28 +119,29 @@ describe('runAsArgv', () => {
|
||||
});
|
||||
|
||||
describe('osUserNameFor', () => {
|
||||
test('prefixes so it cannot collide with a system account', () => {
|
||||
expect(osUserNameFor({ username: 'ana', email: 'ana@example.com' })).toBe(`${OS_USER_PREFIX}ana`);
|
||||
// The chosen username, verbatim — so `whoami` in a member's terminal says who they are. Measured on this
|
||||
// host: useradd accepts dots, hyphens, underscores and uppercase, i.e. everything validateUsername lets
|
||||
// through.
|
||||
test('uses the chosen username as-is', () => {
|
||||
expect(osUserNameFor({ username: 'ana', email: 'ana@example.com' })).toBe('ana');
|
||||
expect(osUserNameFor({ username: 'ana.silva', email: 'a@b.com' })).toBe('ana.silva');
|
||||
expect(osUserNameFor({ username: 'Ana-Silva_2', email: 'a@b.com' })).toBe('Ana-Silva_2');
|
||||
});
|
||||
|
||||
test('falls back to the email local part when there is no username', () => {
|
||||
expect(osUserNameFor({ username: null, email: 'Ana.Silva@example.com' })).toBe(`${OS_USER_PREFIX}ana.silva`);
|
||||
expect(osUserNameFor({ username: null, email: 'Ana.Silva@example.com' })).toBe('ana.silva');
|
||||
expect(osUserNameFor({ username: ' ', email: 'Ana.Silva@example.com' })).toBe('ana.silva');
|
||||
});
|
||||
|
||||
test('sanitises what useradd would refuse', () => {
|
||||
expect(osUserNameFor({ username: 'Ana Silva!', email: 'a@b.com' })).toBe(`${OS_USER_PREFIX}ana_silva_`);
|
||||
test('stays within the 32-character limit useradd enforces', () => {
|
||||
expect(osUserNameFor({ username: 'a'.repeat(40), email: 'a@b.com' })).toHaveLength(32);
|
||||
});
|
||||
|
||||
// The prefix can push an already-32-char sanitised name over the limit, and useradd rejects the whole
|
||||
// name rather than truncating it.
|
||||
test('stays within the 32-character limit', () => {
|
||||
const name = osUserNameFor({ username: 'a'.repeat(40), email: 'a@b.com' });
|
||||
expect(name.length).toBe(32);
|
||||
expect(name.startsWith(OS_USER_PREFIX)).toBe(true);
|
||||
});
|
||||
|
||||
test('a username that tries to shadow root is still prefixed', () => {
|
||||
expect(osUserNameFor({ username: 'root', email: 'r@b.com' })).toBe(`${OS_USER_PREFIX}root`);
|
||||
// No longer defended by a prefix, so it must be defended by adoption rules instead: `ensureOsUser`
|
||||
// refuses a name whose existing passwd home is not the one we are about to confine, and refuses any uid
|
||||
// below 1000 outright. This test records that the NAME itself is no longer the protection.
|
||||
test('does not neutralise a dangerous name — that is ensureOsUser-s job now', () => {
|
||||
expect(osUserNameFor({ username: 'root', email: 'r@b.com' })).toBe('root');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+93
-9
@@ -21,21 +21,35 @@ import { DATA_PATH, HOME_SEED_DIRS, USER_DIRS, toShellUsername } from './data-pa
|
||||
/** Off unless explicitly enabled: this needs root, and a light install has no sudoers entry. */
|
||||
export const OS_USERS_ENABLED = process.env.OFFICER_OS_USERS === 'true' || process.env.OFFICER_OS_USERS === '1';
|
||||
|
||||
/** Prefixed so it cannot collide with a system account, and so `/etc/passwd` shows what we created. */
|
||||
export const OS_USER_PREFIX = 'officer_';
|
||||
|
||||
const MAX_USERNAME = 32;
|
||||
|
||||
/**
|
||||
* The Linux account name for a platform account.
|
||||
* The Linux account name for a platform account: **the username the owner chose**, verbatim.
|
||||
*
|
||||
* Built on `toShellUsername`, which already lowercases, strips anything from `@` on, replaces illegal
|
||||
* characters and truncates. Truncated again after the prefix, because the prefix can push a 32-char
|
||||
* result over the limit and `useradd` would refuse the whole thing.
|
||||
* This carried an `officer_` prefix until 2026-08-11. The prefix bought two things — no collision with a
|
||||
* system account, and a greppable record of what we created — and cost the thing the owner actually wants,
|
||||
* which is that `whoami` in a member's terminal says who they are. Measured before removing it: `useradd`
|
||||
* on this host accepts everything `validateUsername` permits, including dots, hyphens, underscores and
|
||||
* uppercase.
|
||||
*
|
||||
* What replaced the prefix's safety is a stricter ADOPTION rule in `ensureOsUser`: an existing Linux account
|
||||
* is only reused when its passwd home is already the home we expect. Without that, a platform account named
|
||||
* `root` would have adopted root. See there.
|
||||
*
|
||||
* `toShellUsername` still handles the fallback when there is no username, since the email local part can
|
||||
* contain things a Linux name cannot.
|
||||
*/
|
||||
export function osUserNameFor(params: { username: string | null; email: string }): string {
|
||||
const base = toShellUsername(params.username ?? '', params.email);
|
||||
return `${OS_USER_PREFIX}${base}`.slice(0, MAX_USERNAME);
|
||||
const chosen = params.username?.trim();
|
||||
if (chosen) return chosen.slice(0, MAX_USERNAME);
|
||||
return toShellUsername('', params.email).slice(0, MAX_USERNAME);
|
||||
}
|
||||
|
||||
/** The account's home as passwd records it, or null if it has none / does not exist. */
|
||||
async function passwdHome(osUser: string): Promise<string | null> {
|
||||
const result = await run(['getent', 'passwd', osUser]);
|
||||
if (!result.ok) return null;
|
||||
return result.out.split(':')[5] ?? null;
|
||||
}
|
||||
|
||||
export type RunAsOptions = {
|
||||
@@ -120,6 +134,34 @@ export async function lookupOsUser(osUser: string): Promise<{ uid: number; gid:
|
||||
/** The member's home: `DATA_PATH/<email>/home`, where `provisionUserDirs` already put it. */
|
||||
export const osUserHome = (email: string): string => join(DATA_PATH, email, 'home');
|
||||
|
||||
/**
|
||||
* The first directory on the way to `path` that `osUser` cannot traverse, or null if the whole chain is fine.
|
||||
*
|
||||
* Exists because of a real and thoroughly confusing failure. A member's home is under `DATA_PATH`, which on
|
||||
* a normal install is under the OWNER'S home — and `/home/<owner>` is 750 on Debian and Ubuntu. So every
|
||||
* mode bit we set on the account tree was correct, the directory existed, and the member still could not
|
||||
* reach it, because they had no `x` on an ancestor four levels up. What surfaced was
|
||||
* `ssh-keygen: Could not stat …/.ssh: Permission denied`, which points at exactly the wrong place.
|
||||
*
|
||||
* `x` without `r` is the ask: traversal, not listing. Nobody gains the ability to enumerate the owner's home.
|
||||
*/
|
||||
export async function firstUntraversableAncestor(osUser: string, path: string): Promise<string | null> {
|
||||
const parts = path.split('/').filter(Boolean);
|
||||
const chain = parts.map((_, i) => `/${parts.slice(0, i + 1).join('/')}`);
|
||||
|
||||
// One spawn rather than one per level: this runs on every account creation, and the chain is ~6 deep.
|
||||
const proc = runAs(osUser, [
|
||||
'sh',
|
||||
'-c',
|
||||
'for p in "$@"; do [ -x "$p" ] || { printf %s "$p"; exit 0; }; done',
|
||||
'sh',
|
||||
...chain,
|
||||
]);
|
||||
const out = (await new Response(proc.stdout).text()).trim();
|
||||
await proc.exited;
|
||||
return out || null;
|
||||
}
|
||||
|
||||
export type EnsureOsUserResult =
|
||||
| { ok: true; osUser: string; uid: number; gid: number; created: boolean }
|
||||
| { ok: false; error: string };
|
||||
@@ -142,6 +184,34 @@ export async function ensureOsUser(params: { email: string; username: string | n
|
||||
let created = false;
|
||||
let ids = await lookupOsUser(osUser);
|
||||
|
||||
// ── Adoption, and why it is this strict ──
|
||||
//
|
||||
// An account that already exists is REUSED, which is what makes this function re-runnable. While names
|
||||
// carried an `officer_` prefix that was safe by construction: nothing else creates those. Now that the
|
||||
// name is whatever the owner typed, adoption is the dangerous path — a platform account named `root`
|
||||
// would find root in passwd, and every `runAs` for that member would then be a root shell.
|
||||
//
|
||||
// The test is the account's own home. If passwd already points it at exactly the directory we are about
|
||||
// to confine, it is ours (or a previous run's). Anything else is somebody else's account that happens to
|
||||
// share a name, and the answer is to refuse rather than to touch it.
|
||||
//
|
||||
// The uid floor is belt and braces: a system account below 1000 could in principle be created with a
|
||||
// matching home, and none of them should ever be handed to a member.
|
||||
if (ids) {
|
||||
const existingHome = await passwdHome(osUser);
|
||||
if (ids.uid < 1000) {
|
||||
return { ok: false, error: `'${osUser}' is a system account on this machine. Choose another username.` };
|
||||
}
|
||||
if (existingHome !== home) {
|
||||
return {
|
||||
ok: false,
|
||||
error:
|
||||
`'${osUser}' is already a user on this machine, with its home at ${existingHome ?? 'an unknown path'}. ` +
|
||||
`Refusing to take it over — choose another username.`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
if (!ids) {
|
||||
// `-M` because provisionUserDirs already made the directory, and letting useradd create it would
|
||||
// copy /etc/skel in as root-owned. `-s` explicitly: /etc/default/useradd here says /bin/sh, and a
|
||||
@@ -166,6 +236,20 @@ export async function ensureOsUser(params: { email: string; username: string | n
|
||||
const confined = await confineUserTree({ email: params.email, uid: ids.uid, gid: ids.gid });
|
||||
if (!confined.ok) return { ok: false, error: confined.error };
|
||||
|
||||
// Checked before anything tries to USE the home, so the error names the actual problem. Every mode bit on
|
||||
// the account tree can be right while the member still cannot get there, because `DATA_PATH` normally
|
||||
// lives under the owner's home and `/home/<owner>` is 750 on Debian and Ubuntu.
|
||||
const blocked = await firstUntraversableAncestor(osUser, home);
|
||||
if (blocked) {
|
||||
return {
|
||||
ok: false,
|
||||
error:
|
||||
`${osUser} cannot traverse into ${blocked}, so it cannot reach its own home. ` +
|
||||
`Fix with: chmod o+x ${blocked} ` +
|
||||
`(that grants traversal only — the directory stays unlistable.)`,
|
||||
};
|
||||
}
|
||||
|
||||
// Seeded AS the member, because after the chown above their home is 700 and theirs — the platform cannot
|
||||
// write into it, which is exactly the point. Best-effort: an empty file browser is a cosmetic problem, and
|
||||
// failing the whole account creation over Downloads/ would be absurd.
|
||||
|
||||
Reference in New Issue
Block a user