diff --git a/package.json b/package.json index 75a71b8d..1606f46c 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "preinstall": "node -e \"var v = +process.versions.node.split('.')[0]; if (v < 22) { console.error('Node 22 or newer required (got ' + process.versions.node + '). Run: nvm use 22'); process.exit(1); }\"", "gen:index": "bun run ./scripts/gen-index.ts", "fix:env": "bun run ./scripts/fix-env.ts", + "doctor": "bun run --env-file=.env ./scripts/doctor.ts", "secret:set": "bun run --env-file=.env ./scripts/secret-set.ts", "notify:test": "bun run --env-file=.env ./scripts/notify-test.ts", "gen:ecosystem": "bun run --env-file=.env ./scripts/gen-ecosystem.ts", diff --git a/scripts/doctor.ts b/scripts/doctor.ts new file mode 100644 index 00000000..28438ca8 --- /dev/null +++ b/scripts/doctor.ts @@ -0,0 +1,86 @@ +#!/usr/bin/env bun +/** + * `bun run doctor` — check the invariants that provisioning cannot check for itself. + * + * ── Why ── + * + * Every step in `provisionOsAccount` is deliberately non-fatal, so "nothing reported an error" is much + * weaker than it sounds. The 2026-08-19 case: a fresh server, an Admin granted files, chat and terminal, + * every named ACL entry correct, provisioning clean — and the account reached nothing, because a later + * `chmod 700` had recomputed the ACL mask to `---`. It was found by a person, three days and one + * confusing UI message later. + * + * This is the same posture as `assertPermissionTotality`: state the invariant, verify it, name the fix. + * The difference is that this reports rather than refusing to boot — a broken member must not stop the + * platform serving everyone else. + * + * Read-only. It changes nothing, so it is safe on a production box. + * + * Exit 1 if anything is wrong, so it can gate the end of a setup run. + */ +import { getUsers, getAllRoleGrants, OWNER_USER_ID } from 'officerdb'; +import { checkOsAccount } from '../src/servers/os-user-verify'; +import { PERMISSIONS } from '../src/servers/permissions/registry'; + +const red = (s: string) => `\x1b[31m${s}\x1b[0m`; +const green = (s: string) => `\x1b[32m${s}\x1b[0m`; +const dim = (s: string) => `\x1b[2m${s}\x1b[0m`; + +let problems = 0; +const fail = (what: string, fix: string) => { + problems++; + console.log(`${red('✗')} ${what}`); + console.log(` ${dim(fix)}`); +}; +const pass = (what: string) => console.log(`${green('✓')} ${what}`); + +console.log('\n── Accounts ──\n'); + +const users = await getUsers(); + +for (const user of users) { + if (user.id === OWNER_USER_ID) { + // The owner runs AS the service user, in the service user's own home. There is no Linux account to + // check and no ACL to get wrong — `osUser: null` is correct here, not a finding. + pass(`${user.email} ${dim('(owner — runs as the service account)')}`); + continue; + } + + const check = await checkOsAccount(user.email, user.osUser ?? null); + if (check.ok) { + pass(`${user.email} ${dim(`(${user.role}, os_user=${check.osUser}, home readable)`)}`); + } else { + fail(`${user.email} (${user.role}) — ${check.problem}`, `home: ${check.home}`); + } +} + +console.log('\n── Grants that cannot take effect ──\n'); + +// A `confined` grant is silently dropped for an account with no Linux user. That is correct behaviour and +// completely invisible: the grant is in the table, the UI shows it ticked, and it resolves to nothing. +const confinedKeys = new Set(PERMISSIONS.filter((p) => p.kind === 'confined').map((p) => p.key)); +const grants = await getAllRoleGrants(); +const unconfined = users.filter((u) => u.id !== OWNER_USER_ID && !u.osUser); + +if (!unconfined.length) { + pass('every non-owner account has a Linux user'); +} else { + for (const user of unconfined) { + const dropped = grants + .filter((g) => g.role === user.role && confinedKeys.has(g.permission)) + .map((g) => g.permission); + if (!dropped.length) continue; + fail( + `${user.email} (${user.role}) holds ${dropped.join(', ')} — all dropped, because the account has no Linux user`, + 'Fix: Settings → User management → provision, or POST /api/users//provision-linux', + ); + } +} + +console.log(''); +if (problems === 0) { + console.log(green('No problems found.\n')); + process.exit(0); +} +console.log(red(`${problems} problem${problems === 1 ? '' : 's'} found.\n`)); +process.exit(1); diff --git a/src/servers/api/users/provision-os.ts b/src/servers/api/users/provision-os.ts index 4cf77cfc..feb5b630 100644 --- a/src/servers/api/users/provision-os.ts +++ b/src/servers/api/users/provision-os.ts @@ -1,6 +1,7 @@ import type { UserRole } from 'officerdb'; import { updateUser } from 'officerdb'; import { ensureOsUser, hardenOwnerHome, osUserHome, reassertHomeMask } from '@@/os-user'; +import { checkOsAccount } from '@@/os-user-verify'; import { provisionSshAccess } from '@@/os-user-ssh'; import { seedShellConfig } from '@@/os-user-shell'; import { provisionClaudeCli } from '@@/os-user-claude'; @@ -140,6 +141,18 @@ export async function provisionOsAccount(params: { const mask = await reassertHomeMask(osUserHome(params.email)); if (!mask.ok) console.warn(`[users] ${params.email}: ${mask.error}`); + // And then CHECK it, rather than trusting that no step reported failure. + // + // Every step above is non-fatal by design, so "nothing returned an error" is a much weaker statement + // than it looks — the ACL-mask bug produced a clean run and an account that could reach nothing. This + // makes the same call the file browser makes, which is the only version of the question that cannot be + // satisfied by something merely looking correct. + // + // Reported first when it fails, ahead of keys and shells: an account that cannot read its own home is + // broken in a way that no amount of working SSH makes up for. + const verified = await checkOsAccount(params.email, account.osUser); + if (!verified.ok) console.warn(`[provision] ${params.email}: ${verified.problem}`); + // The Linux account is recorded either way: it exists, it is confined, and a member's terminal can run as // it. Only the keys are missing, and that is what the error says. const sshPublicKey = ssh.ok ? ssh.publicKey : null; @@ -151,16 +164,18 @@ export async function provisionOsAccount(params: { for (const step of [claude, shell, postgres, docker]) { if (step && !step.ok) console.warn(`[users] ${params.email}: ${step.error}`); } - const error = !ssh.ok - ? ssh.error - : !claude.ok - ? claude.error - : !shell.ok - ? shell.error - : postgres && !postgres.ok - ? postgres.error - : docker && !docker.ok - ? docker.error - : null; + const error = !verified.ok + ? verified.problem + : !ssh.ok + ? ssh.error + : !claude.ok + ? claude.error + : !shell.ok + ? shell.error + : postgres && !postgres.ok + ? postgres.error + : docker && !docker.ok + ? docker.error + : null; return { osUser: account.osUser, sshPublicKey, error }; } diff --git a/src/servers/os-user-verify.ts b/src/servers/os-user-verify.ts new file mode 100644 index 00000000..f89ada57 --- /dev/null +++ b/src/servers/os-user-verify.ts @@ -0,0 +1,79 @@ +import { readdir } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; +import { osUserHome } from './os-user'; + +// Checking that a member's Linux account actually WORKS, rather than that provisioning reported success. +// +// ── Why this exists ── +// +// Every step in `provisionOsAccount` is deliberately non-fatal: a member whose account provisioned fine +// should not be rolled back because their shell config failed. That is right, and it has a cost — the +// failures accumulate silently, and the owner learns about them from a file browser saying "Officer +// cannot read this folder", which is three steps removed from the cause. +// +// The 2026-08-19 case was the sharp version. Every named ACL entry was correct. The Linux user existed. +// The home existed. The grants were right. And the platform still could not read the home, because a +// later `chmod 700` had recomputed the ACL mask to `---` and clamped the entries to nothing. Nothing in +// the pipeline was in a position to notice, because nothing tried the thing that actually matters. +// +// So: try it. `readdir` on the member's home, as the process that will do it for real. It is the same +// call the file browser makes, which is the point — a check that tests something ADJACENT to the failure +// is how you get a green report and a broken server. +// +// This is the same posture as `assertPermissionTotality` and `assertInstallLayout`: state the invariant, +// verify it, and say plainly what is wrong. The difference is that those refuse to boot and this does +// not — a member with a broken home must not stop the platform serving everyone else. + +export type OsAccountCheck = { + email: string; + osUser: string | null; + home: string; + ok: boolean; + /** Null when `ok`. Otherwise what is wrong and what fixes it. */ + problem: string | null; +}; + +/** + * Can the platform actually read this member's home? + * + * Deliberately not a permissions inspection. Reading `getfacl` and reasoning about masks is how this was + * missed in the first place: the ACL *looked* right. The only trustworthy question is whether the call + * the file browser makes succeeds. + */ +export async function checkOsAccount(email: string, osUser: string | null): Promise { + const home = osUserHome(email); + const base = { email, osUser, home }; + + if (!osUser) { + return { + ...base, + ok: false, + problem: + 'no Linux user — every `confined` permission (files, terminal, chat) is dropped for this account, ' + + 'whatever the role grants. Fix: POST /api/users//provision-linux, or Settings → User management.', + }; + } + + if (!existsSync(home)) { + return { ...base, ok: false, problem: `home does not exist at ${home}. Fix: re-provision the Linux account.` }; + } + + try { + await readdir(home); + return { ...base, ok: true, problem: null }; + } catch (ex) { + const code = (ex as { code?: string }).code; + if (code === 'EACCES' || code === 'EPERM') { + return { + ...base, + ok: false, + problem: + `the platform cannot read ${home} (${code}). The named ACL entries may look correct and still ` + + `grant nothing: a chmod recomputes the ACL mask from the group bits, so a directory closed to ` + + `700 gets \`mask::---\` and clamps every entry to \`#effective:---\`. ` + + `Fix: re-provision the Linux account, or \`sudo setfacl -R -m m::rwx -m d:m::rwx '${home}'\`.`, + }; + } + return { ...base, ok: false, problem: `could not read ${home}: ${ex instanceof Error ? ex.message : ex}` }; + } +}