diff --git a/src/servers/api/users/provision-os.ts b/src/servers/api/users/provision-os.ts index 36a869b1..4cf77cfc 100644 --- a/src/servers/api/users/provision-os.ts +++ b/src/servers/api/users/provision-os.ts @@ -1,6 +1,6 @@ import type { UserRole } from 'officerdb'; import { updateUser } from 'officerdb'; -import { ensureOsUser, hardenOwnerHome, osUserHome } from '@@/os-user'; +import { ensureOsUser, hardenOwnerHome, osUserHome, reassertHomeMask } from '@@/os-user'; import { provisionSshAccess } from '@@/os-user-ssh'; import { seedShellConfig } from '@@/os-user-shell'; import { provisionClaudeCli } from '@@/os-user-claude'; @@ -127,6 +127,19 @@ export async function provisionOsAccount(params: { }) : null; + // LAST, and it has to be last. + // + // Every step above that creates a directory and then closes it — `.ssh` at 700, the claude installer's + // `.config` and `.local` — recomputes that directory's ACL mask from its new group bits. For 700 that is + // `mask::---`, which clamps the platform's own entry to nothing: the grant survives as + // `user:pastilhas:rwx #effective:---`, correct-looking and inert. + // + // Found on a fresh server where an Admin granted files, chat and terminal reached none of them, and the + // ACL looked right everywhere anyone checked. Non-fatal — but it is the difference between an account + // that works and one that only appears to. + const mask = await reassertHomeMask(osUserHome(params.email)); + if (!mask.ok) console.warn(`[users] ${params.email}: ${mask.error}`); + // 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; diff --git a/src/servers/os-user.test.ts b/src/servers/os-user.test.ts index 21893172..d744ea98 100644 --- a/src/servers/os-user.test.ts +++ b/src/servers/os-user.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { chmod, mkdir, mkdtemp, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { findReadableSecrets, osUserNameFor, runAsArgv, secureRuntimeSecrets } from './os-user'; +import { findReadableSecrets, osUserNameFor, runAsArgv, secureRuntimeSecrets, maskArgvFor } 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 @@ -243,3 +243,36 @@ describe('secureRuntimeSecrets', () => { expect(await secureRuntimeSecrets(root)).toEqual({ repaired: [], failed: [] }); }); }); + +describe('ACL mask, after everything else', () => { + // The regression: a chmod recomputes the ACL mask from the GROUP bits, so a directory closed to 700 + // after the platform's grant was set ends up `mask::---`, clamping `user::rwx` to nothing. + // The entry is still there and reads as correct; it just does not grant anything. + // + // This asserts the shape of the repair rather than the filesystem effect — the effect needs root, an + // acl-capable filesystem and a second uid, none of which a unit test has. It was verified live on a + // real member's home instead (2026-08-19): READABLE afterwards, and zero `effective:---` entries left. + test('re-asserts the mask recursively, and touches nothing else', () => { + const argv = maskArgvFor('/data/someone@example.com/home'); + + expect(argv).toEqual([ + 'sudo', + '-n', + 'setfacl', + '-R', + '-m', + 'm::rwx', + '-m', + 'd:m::rwx', + '/data/someone@example.com/home', + ]); + + // Recursive, because every subdirectory created-then-closed after the original grant has the same + // clamped mask — `.ssh`, `.config` and `.local` all did. + expect(argv).toContain('-R'); + // The default mask too, or the next directory the member creates inherits the broken state. + expect(argv).toContain('d:m::rwx'); + // No named entries: those were always right, and re-granting them here would hide it if they were not. + expect(argv.some((a: string) => a.startsWith('u:'))).toBe(false); + }); +}); diff --git a/src/servers/os-user.ts b/src/servers/os-user.ts index 23633f39..f690c08a 100644 --- a/src/servers/os-user.ts +++ b/src/servers/os-user.ts @@ -545,6 +545,49 @@ export async function confineUserTree(params: { const OWNER_HOME_PRIVATE = ['.claude', '.pm2', '.config', '.npm', '.opencode', '.cache', '.tmux']; const OWNER_HOME_PRIVATE_NESTED = ['.local/share', '.local/state']; +/** + * The exact command, split out so a test can assert it without root or an acl-capable filesystem. + * + * `-R` because the same clamp happens to every subdirectory created and closed after the original grant, + * not just to the home itself. Only the mask — the named entries were never the problem. + */ +export const maskArgvFor = (home: string): string[] => [ + 'sudo', + '-n', + 'setfacl', + '-R', + '-m', + 'm::rwx', + '-m', + 'd:m::rwx', + home, +]; + +/** + * Re-assert the ACL mask across a member's home. **Must run after every other provisioning step.** + * + * ── The failure this exists for ── + * + * A POSIX ACL mask is not independent of the mode bits: `chmod` recomputes it from the GROUP bits, and a + * directory chmod'd to 700 gets `mask::---`, which clamps every named entry to nothing. The platform's + * grant then reads `user:pastilhas:rwx #effective:---` — present, and meaningless. + * + * `ensureOsUser` already knows this and orders itself correctly (chmod, then setfacl). What it cannot + * account for is the steps that come AFTER it, each of which creates a directory and then closes it: + * `.ssh` at 700 from the key setup, `.config` and `.local` from the claude installer. Each one inherits + * the default ACL at creation and then clamps its own mask a moment later. + * + * Observed on a fresh server, 2026-08-19: an Admin holding `files`, `chat` and `terminal` reached none of + * them. The file browser's `readdir` raised EACCES on a home whose ACL said it could read it, and the + * grant looked correct in every place anyone thought to check. + * + * Only the mask is touched. The named entries are already right — they were never the problem. + */ +export async function reassertHomeMask(home: string): Promise<{ ok: boolean; error?: string }> { + const done = await run(maskArgvFor(home)); + return done.ok ? { ok: true } : { ok: false, error: `could not re-assert ACL mask on ${home}: ${done.out}` }; +} + /** * Close the owner's own dotfiles to the accounts this platform hands shells to. *