Skip to content

Confine every non-admin account by default - #57

Merged
ralyodio merged 3 commits into
masterfrom
worktree-user-sandbox
Sep 6, 2026
Merged

Confine every non-admin account by default#57
ralyodio merged 3 commits into
masterfrom
worktree-user-sandbox

Conversation

@ralyodio

@ralyodio ralyodio commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

These are multi-tenant boxes, and the root VPSes we sell hand a customer an account, never the root password. Measured on dev before this change: 2,437 world-readable files under /home/anthony alone — including session tokens in ~/.config — readable by each of the seven non-sudo accounts on the box.

Renamed, and rebased on master

This branch was called "the user sandbox". master has since merged #56, which already owns that word: --sandbox puts a tenant in their own incus container or VM. That is isolation by virtualisation; this is isolation by policy on a shared box. Every identifier collided — SANDBOX, SANDBOX_GROUP, cmd_sandbox, sandbox_usage.

So this is now confinement, which is the better word regardless: it is what AppArmor and SELinux call exactly this, and the table below was headed "confined?" from the first draft.

SANDBOX*   -> CONFINE*        sandbox  -> confine      (subcommand, functions)
sandboxed  -> confined        (the group, and the adjective)

The two features are layers, not rivals — a tenant with their own instance is still a non-admin account out here, so confinement applies to them too. Tenants are excluded from the resource caps (they have nologin and no host processes to cap), but their home mode is still set, because that runs per-home rather than off that list.

The model

Three tiers, and the boundary is group membership:

tier who confined?
root uid 0 never — every cap explicitly lifted
admin sudo, admin no — exempt by uid
confined everybody else, by default yes

Membership is recomputed every run, so a promotion releases someone and a demotion confines them again without anyone remembering which flag was passed when the account was made.

before confined
someone else's home 0751, walk in and read 0700
ps -ef every command line on the box your own processes
memory / tasks unbounded MemoryMax=50%, TasksMax=4096
fork bomb takes the box stopped at nproc
ssh -R relay out under our IP refused (ssh -L still works)
kernel kptr/dmesg/perf readable raised to a floor

nginx has to walk through a home to reach ~/public_html, and the old answer — chmod o+x — opens that path to every account. This grants www-data an execute-only ACL instead, and only where something is actually published.

Never root

user-.slice.d is a prefix drop-in — it applies to user-0.slice too. So the caps are lifted again for uid 0 and each admin uid explicitly, by uid. The account you fix a wedged box with must not be subject to the cap that is wedging it.

It also will not demote an account that already has sudo. Doing that unattended is how you lose a box: it might be a colleague, the only other admin, or the account your automation logs in as. confine names them and stops.

Three things this got wrong first

  • The kernel settings are floors, not values. Ubuntu ships perf_event_paranoid=4 and unprivileged_bpf_disabled=2 already stricter than the target, so writing them unconditionally would have loosened the box in the name of hardening it — silently, on the distro we run everywhere.
  • fs.suid_dumpable does not run in one direction. 0 is safest, 1 is the dangerous one, 2 sits in between. Treating it as a floor left a box on 2 and called it hardened. Caught by dumping the generated files rather than by reading the code.
  • The fstab /proc line is written only after a live remount proves the kernel takes the option. An entry it rejects fails the mount at boot, which is the worst place to find out.

I also claimed in a comment that the drop-in's closing Match all was load-bearing — that sshd would otherwise swallow the directives after the Include. Measured against OpenSSH 10.2p1 and it is not true: a Match is scoped to the file it appears in, and the effective config is byte-identical either way. The line stays (it costs nothing and the file may move), but the comment and test now say what was actually measured.

Three more found while merging

git auto-merged several regions both branches had touched, so those were read rather than trusted — fix_home_permissions, add_user_spec and the stage ordering all keep both sides. On the way through:

  • The accounts table hand-padded "admin " to the width of "sandboxed". Renaming to the shorter confined misaligned the column by one. It pads in the format string now, so neither word's length is load-bearing.
  • _confine_sysctl_floor's want local collided with an array of the same name elsewhere in the file. The linter does not scope locals, so the name cost two false SC2178/SC2128 warnings — renamed to floor.
  • Two tests pinned the entire subcommand alternation and broke the moment sandbox joined it. They match the verb inside the list now, so the next subcommand added does not break them.

One more, self-inflicted and worth knowing: a comment that opens with the linter's own name is parsed as a directive, and the parse error aborts the rest of the scan — so the file looked clean for the wrong reason until the comment was reworded.

Verification

  • pnpm test791 passed, 29 files, including 33 tests for this
  • bash -n clean; shellcheck -S warning clean apart from one SC2088 at line 1116 that predates both branches (verified against master)
  • generated every /etc file into a temp tree and read it; the second run is silent (idempotent)
  • sshd -t and sshd -T against a synthesized Ubuntu-shaped config
  • confine status, sandbox, and groups --help all run side by side after the merge; confine status against this box correctly reports it as currently wide open

test/torrent.test.ts > computes the info hash WebTorrent computes failed once in five runs and passed the other four. It is untouched by this branch — flaky, not a regression, and not chased here.

pnpm typecheck fails in src/free-names.ts, which is pre-existing and untouched (verified identical to master).

Not applied anywhere yet

This is the capability, not a rollout. dev, vienna and the DO droplets all still report wide open; applying it changes home modes and sshd for real users mid-session, so that wants a deliberate run. CONFINE=0 opts out entirely.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y32FRASN3UZhUgwFdbEAzb

These are multi-tenant boxes, and the root VPSes we sell hand a customer
an account rather than the root password. Until now an account on one
could read every other home, watch every command line on the machine,
take the box down with one build, and relay out over ssh under our IP.

Adds a sandbox with three tiers, where the boundary is group membership:
root is untouched always, anyone in sudo/admin is exempt, and everybody
else is confined with nothing to remember.

  home 0700 + an execute-only ACL for www-data, so nginx can still walk
    through to ~/public_html without the path being open to every account
  /proc hidepid, so ps shows you your own processes
  systemd slice caps (MemoryMax/TasksMax) and pam limits (nproc/nofile)
  sshd policy: ssh -L yes, ssh -R no
  kernel floors for kptr/dmesg/perf/protected_*/suid_dumpable

Every cap is lifted again for uid 0 and each admin uid explicitly --
user-.slice.d is a prefix drop-in that otherwise applies to root's own
slice, and the account you fix a wedged box with must not be subject to
the cap that is wedging it.

It deliberately does NOT demote an account that already has sudo:
doing that unattended is how you lose a box. `sandbox` names them.

Three things this got wrong first and now handles:

  * the kernel settings are floors, not values. Ubuntu ships
    perf_event_paranoid=4 and unprivileged_bpf_disabled=2 already
    stricter than the target, so writing them unconditionally would
    loosen the box in the name of hardening it.
  * fs.suid_dumpable does not run in one direction -- 0 is safest, 1 is
    the dangerous one, 2 sits in between -- so it cannot be a floor, and
    treating it as one left a box on 2 and called it hardened.
  * the fstab /proc line is written only after a live remount has proved
    the kernel takes the option; an entry it rejects fails the mount at
    boot, which is the worst place to find out.

DEFAULT_GROUPS now defaults to users rather than sudo,admin while the
sandbox is on -- an unattended run never prompts, so the old default
handed each new tenant the way straight back out. An explicit setting
still wins.

New subcommand: `root-ubuntu.sh sandbox [status|apply]`. status reads
only and needs no root.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8y84poK1amqfctipMbjJm
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

ThreatCrush Security Scan

12 finding(s)

HIGH/CRITICAL: 4 | MEDIUM: 4 | LOW: 4

Severity Rule Location
HIGH sh-remote-script-execution root-ubuntu.sh:3227
HIGH sh-remote-script-execution root-ubuntu.sh:3228
HIGH sh-remote-script-execution root-ubuntu.sh:4840
HIGH sh-remote-script-execution root-ubuntu.sh:4844
MEDIUM sh-remote-script-execution root-ubuntu.sh:4993
MEDIUM sh-remote-script-execution root-ubuntu.sh:6055
MEDIUM redos-nested-quantifier src/domain-free.ts:56
MEDIUM redos-nested-quantifier src/mail.ts:1042
LOW secret-generic-credential src/credentials.ts:36
LOW secret-generic-api-key test/credentials.test.ts:208
LOW secret-generic-credential test/mail.test.ts:135
LOW secret-generic-credential test/shorten.test.ts:36

Snippets are redacted; ThreatCrush never prints matched credential material.

ralyodio and others added 2 commits September 6, 2026 12:23
master has since grown a feature that already owns the word: `--sandbox`
puts a tenant in their own incus container or VM. That is isolation by
virtualisation. This is isolation by policy on a shared box -- home modes,
hidepid, a systemd slice, limits, sshd -- and every identifier here
collided with it: SANDBOX, SANDBOX_GROUP, cmd_sandbox, sandbox_usage.

"Confine" is the better word for it anyway, and this branch was already
using it in prose: it is what AppArmor and SELinux call exactly this, and
the table in the PR was headed "confined?" from the start.

  SANDBOX*        -> CONFINE*
  sandbox         -> confine          (subcommand, functions)
  sandboxed       -> confined         (the group, and the adjective)
  configure_user_sandbox -> configure_confinement

The two features are layers, not rivals: a tenant with an instance is also
a non-admin account out here, so confinement applies to them too.

No behaviour change. 791 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y32FRASN3UZhUgwFdbEAzb
master merged #56, which puts a tenant in their own incus container or VM
under the name `sandbox`. This branch confines accounts in place. They are
layers rather than rivals -- a tenant with an instance is still a non-admin
account out here -- so both survive the merge intact.

The rename in the previous commit did most of the work; what was left:

  * both features had inserted a section at the same point, and git had
    folded the shared "esac }" tail of cmd_confine and cmd_sandbox together.
    cmd_confine's case is closed explicitly and both sections are kept
  * one subcommand list, one dispatch arm each
  * a comment about ssh-agent lingering that #56 orphaned above the sandbox
    stage is back with the stage it describes

Checked rather than assumed, since git auto-merged several regions both
branches had touched:

  * fix_home_permissions keeps BOTH sides -- the _confine_home_mode branch
    and the per-app publishing walk
  * add_user_spec keeps both the tenant-groups shortcut and ask_groups
  * DEFAULT_GROUPS still moves to `users` under confinement
  * stage order is right: incus up, accounts created, THEN confinement,
    which is what makes group membership the thing that decides a tier

Tenants are excluded from _confine_humans (nologin), which is correct: they
have no host processes to cap. Their home mode is still set, because that
runs per-home out of fix_home_permissions rather than off that list.

Three defects fixed on the way through:

  * the accounts table hand-padded "admin" to the width of "sandboxed", so
    the rename to the shorter "confined" misaligned the column. It pads in
    the format string now
  * _confine_sysctl_floor's `want` local collided with an array of the same
    name elsewhere; the linter does not scope locals, so it cost two false
    warnings. Renamed to `floor`
  * two tests pinned the whole subcommand alternation and broke when
    `sandbox` joined it. They match the verb inside the list now, so the
    next subcommand does not break them

791 tests pass; shellcheck clean apart from one SC2088 that predates both
branches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y32FRASN3UZhUgwFdbEAzb
@ralyodio ralyodio changed the title Confine every non-admin account by default (the user sandbox) Confine every non-admin account by default Sep 6, 2026
@ralyodio
ralyodio merged commit 67a2124 into master Sep 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant