Add crash-orphan sweep for stale -p port-forward rules

Commit 6/6 (final) of the network isolation feature
(docs/networking-design.md). Landed narrower in scope than originally
planned once the actual orphan surface was worked out: veths need no
sweep at all (the kernel tears down an entire pair once either end's
namespace is destroyed, so nothing survives a crash), and bridges/
persistent namespaces are deliberately meant to always outlive any one
session (the whole point of the reboot-reconciliation design already
built in commit 3). Only -p's iptables rules are host-global state
with no automatic teardown, so that's the entire sweep.

port_forward.{h,cpp}: record_port_forwards()/remove_port_forward_record()
persist a session's active mappings to $XDG_STATE_HOME/slocker-lite/
port-forwards/<container_name>-<pid> -- the exact same naming scheme
as session_pid_file_path() (pid_file.h), so clean_stale_port_forwards()
can cross-reference filenames directly against list_sessions()'s own
liveness check rather than re-deriving it. --clean-processes
(commands.cpp) now also runs this sweep alongside its existing
stale-pid-file one.

Also fixed a real gap in commands.cpp caught while wiring this up:
on_bwrap_pid_known was only set when daemonize_flag ||
!network_specs.empty(), so a bare "-p ... " with no -n or -D would
silently never even attempt to run -- no error, nothing logged, the
whole flag just quietly did nothing.

Verified via a controlled scratch test rather than a literal kill -9
on a root-owned slocker-lite process (not achievable through this
session's scoped doas rule, which only permits running slocker-lite
itself): a fabricated stale port-forward record was correctly
detected, its rule-removal attempted, and its file cleaned up, while a
record matching a real running session was left completely untouched.
This commit is contained in:
2026-08-30 13:33:47 +00:00
parent 5c87cac430
commit 7605831269
6 changed files with 228 additions and 9 deletions
+61 -4
View File
@@ -113,9 +113,13 @@ Source layout (all under `src/`):
success on an empty list. `clean_processes_command()` implements
`--clean-processes` (also long-option only): calls `clean_stale_sessions()`
(`pid_file.{h,cpp}`) and prints one `removed stale pid file for '<name>' (pid
<pid>)` line per file actually removed — nothing is printed for sessions still
running, and an empty result (nothing stale) is silent success, same
convention as the rest of this file's list/delete commands. `Mode::exec`'s
<pid>)` line per file actually removed, then also calls
`clean_stale_port_forwards()` (`port_forward.h`, see below — commit 6 of
`docs/networking-design.md`'s sequence) and prints one `removed stale
port-forward rules for '<name>-<pid>'` line per record actually swept —
nothing is printed for sessions still running, and an empty result
(nothing stale) is silent success, same convention as the rest of this
file's list/delete commands. `Mode::exec`'s
dispatch case is a one-line call to `exec_in_session(*args.exec_pid,
args.command)` (`exec_session.{h,cpp}`, see below) — the pid/command
parsing and validation now happens in `cli_args.cpp`'s `parse_args()`
@@ -244,7 +248,22 @@ Source layout (all under `src/`):
returns) mirrors how `join_networks()`'s own veths don't need an explicit
removal step (the kernel tears them down once the session's namespace
goes away) while port-forward rules — host-global, named, persistent
iptables state — very much do.
iptables state — very much do. `on_bwrap_pid_known` itself is only set at
all when `daemonize_flag || !network_specs.empty() ||
!parsed_port_forwards.empty()` — a real gap caught while wiring this up: an
earlier version only checked the first two, so `-p` given *without* `-n`
or `-D` would silently never even attempt to run (no error, nothing
logged) since the callback that resolves/applies it would never fire at
all. At the end of the callback, `record_port_forwards(container_name,
pid, active_port_forwards)` (`port_forward.h`, see below) persists
whatever actually landed to a small state file — so that a later
`--clean-processes` run can find and remove these rules even if *this*
process crashes before ever reaching its own `remove_port_forward()` calls
after `run_bwrap()` returns; those calls are paired with a
`remove_port_forward_record(container_name, bwrap_pid)` (`bwrap_pid`
captured from the same callback, in a variable declared in
`run_container()`'s own scope) so a cleanly-exiting session's own record
doesn't linger for `--clean-processes` to find later.
- `self_test.{h,cpp}``run_self_tests()` implements `-t/--test`, this
project's own built-in self-test mode (distinct from the Meson-driven
fixture smoke test under `tests/`, described in "Build & test commands"
@@ -729,6 +748,44 @@ Source layout (all under `src/`):
container serving HTTP on an `extern` network with `-p 8080:80` was
reachable via `curl <host's real IP>:8080` from the host; the rule was
confirmed gone (connection refused) after the session was killed.
**Crash-orphan sweep** (commit 6 of `docs/networking-design.md`'s
sequence): unlike `join_networks()`'s veths (torn down automatically by
the kernel once the session's namespace goes away) or the bridges/
persistent namespaces themselves (deliberately meant to outlive any one
session — `network_bridge.h`'s reboot-reconciliation design), a `-p`
mapping's iptables rules are host-global state with no automatic teardown
at all — if `slocker-lite` itself is killed/crashes before reaching its
own `remove_port_forward()` calls, those rules simply outlive the session
forever otherwise (`bwrap` itself dies immediately in that case too, via
`--die-with-parent`, so the *container* never becomes a stray process
needing separate handling — only these rules can). `port_forward_state_path()`
resolves `xdg_state_dir() / "port-forwards" / "<container_name>-<pid>"`
deliberately the *exact* same naming scheme as `session_pid_file_path()`
(`pid_file.h`), so `clean_stale_port_forwards()` can cross-reference this
directory's filenames directly against `list_sessions()`'s own
`SessionInfo::path` to reuse its liveness check, rather than re-deriving
pid liveness a second, drifting way. `record_port_forwards()` writes one
line per mapping (`"<host_port> <container_ip> <container_port>"`) to that
path — a no-op if there's nothing to record. `clean_stale_port_forwards()`
(`commands.cpp`'s `clean_processes_command()`, alongside
`clean_stale_sessions()`) scans that directory: a record whose filename
doesn't match any currently-*running* session is stale — every line is
parsed back into an `ActivePortForward` and removed
(`remove_port_forward()`) before the record file itself is deleted; a
record whose session is still running is left completely untouched.
**Verified via a controlled scratch test** (root wasn't needed for the
logic itself — only the underlying `iptables -D` calls, already proven
working as root above; killing a root-owned `slocker-lite` process
directly, bypassing `--kill`'s own graceful cgroup-based teardown, wasn't
achievable through the scoped `doas` rule this session has, which only
permits running `slocker-lite` itself): a real running session's own pid
file was used to construct a *matching* port-forward record (left
untouched by the sweep, confirmed still present afterward) alongside a
*fabricated* record for a nonexistent pid (correctly identified as stale,
its rule-removal attempted — visibly failing only for lack of root in this
particular rootless test — and its record file actually removed,
reported as `removed stale port-forward rules for 'faketest-999999'`).
- `session_cgroup.{h,cpp}` — gives `--kill` (`kill_session.{h,cpp}`, see
below) a reliable way to find every process a session ever started, however
deeply forked/daemonized/reparented, by putting it in a dedicated cgroup v2