slocker-lite/TODO.md

TODO

Resolved: Ctrl-C/SIGTERM (or a normal exit) could leave session stragglers running

Reported by the user while device-testing the signal-exit-logging fix, then fixed on request: forward_signal_to_foreground_child() (process.cpp, installed as the SIGINT/SIGTERM handler around run_process_foreground()'s own waitpid()) only ever does a single, simple kill(g_foreground_child_pid, sig) -- just the one pid run_bwrap() tracks (bwrap's own outer process). If anything inside the sandbox daemonizes/double-forks and calls setsid() (escaping into a new session, the same shape --kill's own investigation found for a real php-fpm --daemonize + exec caddy init script -- see kill_session.{h,cpp}'s own three-strategy design in CLAUDE.md), that signal forward alone could never reach it -- and, on reflection, a normal exit (or -D/--daemonize) had exactly the same gap, since nothing swept the session's own leftovers in any of those cases either.

Fix, per the user's own suggested direction: rather than teaching the signal handler itself to do kill_session()-style work (genuinely awkward -- kill_session() does blocking polling/waitpid()s, unsafe to call directly from a signal handler, and re-derives the session via list_sessions(), which would see run_bwrap()'s own still-open SessionLock fd as "still running" since flock() ownership is per open file description, not per process), kill_via_cgroup() (kill_session.cpp's own cgroup-based strategy) was exported and is now called directly from run_bwrap() itself, right after run_process_foreground() returns and before the session cgroup is torn down -- unconditionally, regardless of why it returned (normal exit, a forwarded SIGINT/SIGTERM), so a container that daemonizes and exits successfully can no longer leave a background process running either. -D/--daemonize needed no special-casing at all: it re-enters this exact same run_bwrap() call from within its own already-forked/setsid()'d child, so the sweep runs there too, for free. See bwrap.{h,cpp}'s and kill_session.{h,cpp}'s own entries in CLAUDE.md for the full detail.

Verified on this dev machine, root, via the scoped doas rule: a new [integration][root] regression test (tests/integration/test_session_cleanup.cpp) confirms kill_via_cgroup() actually reaps a process that forks, setsid()s away, and outlives its own parent's exit -- the exact escape shape this fix targets -- and a live -r/--run session backgrounding a long-running process and exiting normally left no leftover process and no leftover cgroup directory afterward. This dev machine's own kernel supports pid namespaces, though, so the default case (--unshare-pid requested) already gets this for free from the kernel itself (killing a pid namespace's own pid 1 -- whether via a normal exit or a forced kill -- collapses the whole namespace regardless of this fix); the scenario this fix specifically targets -- --unshare-pid unavailable or disabled, so a daemonizing process reparents completely outside any namespace -- originally couldn't be exercised end-to-end locally (no CLI-level way to force that off for a single run; it was a config-file-only NamespaceConfig field).

Since resolved: -c/--config-file (added later) made this directly testable locally after all, and doing exactly that (per the user's own explicit request) found a real, separate bug -- a race between this fix's own create_session_cgroup() call and bwrap's own internal forking, which could leave the sweep unable to reach the straggler at all specifically without a pid namespace, even as root with cgroup v2 genuinely available. See CLAUDE.md's session_cgroup.{h,cpp} entry for the full root-cause and fix (process.h's new before_exec hook). Re-verified end-to-end (root, this dev machine, -c <unshare-pid: false>) after the fix: 3/3 isolated trials and repeated full-category runs all correctly reaped the straggler. On-device re-confirmation (the real target device has neither pid namespace nor previously-confirmed cgroup delegation) is still worthwhile but no longer the only way to exercise this path locally.

Known residual limitation: this only helps when create_session_cgroup() actually succeeded (cgroup v2 mounted and writable). A kernel with neither cgroup v2 nor pid namespace support still has no way to reach a reparented straggler automatically -- the same fundamental gap kill_via_tracked_pid() (the weakest of --kill's own three strategies) already represents.

Resolved: config/state paths sometimes resolved relative to cwd

Found on the real target device (a Gentoo chroot on Android), two separate ways: (1) running slocker-lite from different working directories once produced two separate state trees -- /root/.config/slocker-lite + /root/.local/state/slocker-lite vs. /root/src/slocker-lite/.config/... + .../.local/state/... -- so two independently-invoked processes had disjoint config.yaml/session/network state and both auto-allocated the same 10.168.0.0/24 subnet, stepping on each other's host-level ip/iptables state; (2) later, network_dns.cpp's dnsmasq failed outright with failed to open pidfile .local/state/slocker-lite/dns-resolvers/...: No such file or directory -- a visibly relative path, not the expected absolute one.

Root cause, confirmed by the user: HOME was set in the invoking bash session but never exported (declare -p HOME showed declare -- HOME="/root", not declare -x HOME="/root") -- a non-exported shell variable is visible to the shell itself (prompts, tab completion, declare -p) and to anything that reads it back via the shell's own variable table, but is not part of the environment execve() passes to a child process, so getenv("HOME") in slocker-lite itself returned nullptr. A first environment dump the user pasted to help debug this showed HOME=/root and looked like it ruled this out -- it didn't, since that dump was a declare -p-style listing of all shell variables, not strictly the exported environment a child process actually receives.

Fixed: xdg_state_dir()/config_file_path() (pid_file.{h,cpp}/ config_file.cpp) now fall back to the passwd database entry for the current uid when $HOME itself is unset (resolve_home_dir()) -- exactly this scenario -- and both additionally make their own final return value absolute (std::filesystem::absolute()) regardless of which piece was relative, so a relative $XDG_STATE_HOME/$XDG_CONFIG_HOME override would be caught the same way even though it wasn't the actual cause here.

Security: run the DNS resolver (dnsmasq) as a low-privilege user

start_dns_resolver() (src/network_dns.cpp) currently runs dnsmasq with --user=root --group=root, explicitly overriding dnsmasq's own default privilege-drop behavior. This was done because dnsmasq's default drop-to- unprivileged-user broke reading $XDG_STATE_HOME (typically /root/.local/state/slocker-lite/..., mode 0700 -- unreadable/ untraversable by a non-root user) for its own --hostsdir/--addn-hosts files, causing every query to come back REFUSED. Keeping dnsmasq at root was the fastest correct fix, but it's a real, deliberately-accepted regression from dnsmasq's own security posture: this project's DNS resolver process now runs as root for its entire (per-session) lifetime, with no privilege drop at all, purely to work around a directory permissions mismatch.

Future direction: let dnsmasq actually drop to a low-privilege user, and instead make the files it needs to read reachable by that user -- e.g. move (or additionally expose) the relevant per-network dns-hosts/, dns-internal-hosts/, and dns-resolvers/ state directories somewhere a non-root user can traverse into (not nested under /root), with permissions scoped narrowly to just what the resolver needs. Needs some design thought about where that location should live consistently with this project's existing $XDG_STATE_HOME conventions, and whether the rest of this project's own state should stay root-only or move too. Not urgent -- this project's networking is already root-only in every other respect (bridges, iptables, etc.), so this is a narrower, lower-priority hardening pass for later, not a blocker for anything currently in progress.

-x/--exec/--kill's resolve_namespace_pid() can stop one level too shallow when --user used priv-drop

Found while verifying compose's new user: support (root, via the scoped doas rule): a session started with --user (root, --unshare-pid in effect, the default) has a real process tree of outer bwrap -> bwrap's own pid-1 supervisor for the new pid namespace (never itself execs into anything -- it's what keeps the namespace alive, since the kernel requires some process to be pid 1) -> the actual target, forked by that supervisor, which execs slocker-lite-priv-drop and then the real command. Confirmed via ps -eo pid,ppid,uid,cmd that the real command genuinely runs as the resolved uid/gid (priv-drop itself is correct, not the bug) -- but resolve_namespace_pid() (sandbox_process.{h,cpp}), used by both -x/--exec's own default-identity resolution and --kill, stops at the pid-1 supervisor (its namespaces already differ from the true outer pid, satisfying that function's own single-hop comparison) instead of walking one level deeper to the real target. Concretely: -x/--exec <pid> -- id with no explicit --user on a session that resolved a non-root user via priv-drop incorrectly reports root (reads the supervisor's own still-root /proc/.../status), even though the session's real command is genuinely running as the resolved non-root user the whole time.

Not urgent: -x/--exec --user <explicit> and --kill are unaffected in practice (--kill's own cgroup/pid-namespace strategies already reach every process regardless of tree depth; only -x/--exec's default identity guess is off). Needs a real fix to resolve_namespace_pid() (or a documented, narrower workaround) to correctly walk past bwrap's own pid-1 supervisor when --unshare-pid is in effect, rather than assuming the first namespace-differing child is the real target.