CapturedStdout swallows compose_up_command()/compose_down_command()'s own
fmt::print()/spdlog output (spdlog's default sink is stdout) so it doesn't
pollute Catch2's console reporting -- but that also meant a failure had no
diagnostic trail at all. Both captures are now kept and attached via
INFO(), which Catch2 only actually prints alongside a failing assertion in
the same scope, staying silent on a normal passing run.
Also softens the port-forward reply REQUIRE to CHECK: a flaky reply on one
run must never skip the -d/--down cleanup below it, which is exactly what
a REQUIRE there would do -- found while investigating a real failure on
the actual target device (managed networks not torn down by -d), where
losing that diagnostic trail (and, if this were interactive, the -d skip
compounding into a real leaked uplink relay) would have made root-causing
much harder.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
tests/integration/test_compose_orchestrator.cpp ([integration][root][net])
exercises the real, checked-in test-compose/compose.yaml skeleton end to
end: creates test-preexisting-net if needed, -u, confirms both services'
environment/env_file values via their own log files, confirms the server's
port-forward actually relays the worker's reply over a real TCP
connection, -d, confirms managed networks were torn down while the
external network and the managed volume both survived, then deletes the
volume too so a later run can exercise its creation again.
Deliberately doesn't use ScratchXdgDirs (unlike every other [root][net]
test in this suite): test-preexisting-net is external: true, meaning it's
the user's own responsibility to set up once and keep reusing, matching
real Compose's own convention for that field. A daemonized service's own
dispatch_command() call can return before the sandboxed script has
produced any output, so log-content and port-forward checks poll rather
than asserting immediately; the port-forward check connects via the
host's own real global IPv4 (discovered via `ip`, not hardcoded) instead
of 127.0.0.1, since loopback never reaches the container regardless of
whether forwarding itself works (a documented NAT-hairpinning limitation).
Verified end to end on this dev machine (root, via the scoped doas rule),
twice in a row: 27 assertions passed both times, and
--list-containers/--list-networks/--list-volumes/ps all confirmed clean
afterward.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
Running the full device test suite on the real Android target found a
genuine test-harness gap: every extern-network test in
test_network_join_scenarios.cpp failed with "RTNETLINK answers: File
exists" adding its own uplink route. Root cause: each test runs under its
own ScratchXdgDirs, so its own persistent.yaml starts empty every time --
allocate_ipv4_subnet()'s own auto-allocation always picks the very first
slot (10.168.0.0/24) with no way to see whatever real, non-test networks
already exist on the host. The device happens to have a real, long-lived
"extern" network already occupying exactly that subnet from prior manual
testing, and since extern's uplink adds a route back into the (necessarily
shared, host-root) routing table -- unlike intern, whose routing lives
entirely inside its own isolated per-network namespace -- every extern
test collided with it. Not a production bug: a real end user only ever has
one persistent.yaml where allocation correctly sees every existing entry.
Fixed by giving each of the 10 test networks in
test_network_join_scenarios.cpp its own fixed, explicit subnet (--subnet)
in 10.169.0.0/16 -- a different /16 than production's own default
10.168.0.0/16 range, so a test run can't collide with a real network
regardless of how many the host already has.
Also renamed every network/hostname/container-name string literal used
across the test suite (test_network_join_scenarios.cpp,
test_root_networking.cpp, test_rootless_run.cpp,
test_session_cleanup.cpp) from "selftest*" to "test-*", to further reduce
the chance of colliding with anything a real invocation might already be
using. Low-level interface device literals (slkselftest0/thselftest0/
ethselftest in test_root_networking.cpp) are left as-is -- they're
internal identifiers for a throwaway unit test, not network or container
names.
Verified: clean rebuild, meson test, and 3 consecutive
[integration][root] suite runs (61 assertions, 14 test cases) with no
failures.
The tap+relay intern-ping test and the DNS hostname-ping test both
flaked intermittently (roughly 1 in 13-20 runs), always in a "found_success
== false" shape with no assertion-level clue as to why.
Root cause, confirmed via a temporary production-code diagnostic (since
reverted) and cross-referenced against network_join.cpp's own code: the
readiness poll only waited for eth0 to *exist* (`ip link show eth0`), but
an interface can become visible before join_one_network()'s own later
`ip addr add`/`ip link set ... up` steps for it have actually run. A
script that used the interface as soon as it merely existed could ping,
get "Network unreachable" (no address yet), and exit almost immediately
-- and since a session's own sandboxed process is the sole occupant of
its pid/net namespace (--unshare-pid/--unshare-net), its exit destroys
that namespace outright. That, in turn, made whichever other nsenter
call was still in flight against the same namespace -- join_one_network()'s
own remaining steps, or the entirely separate per-session DNS resolver
(network_dns.cpp's start_dns_resolver(), which enters every networked
session's namespace regardless of whether --hostname was given) -- fail
with "No such file or directory" against a namespace that had already
collapsed underneath it.
This is the same general class of race network_join.{h,cpp}'s own
CLAUDE.md entry already documents (a very-short-lived sandboxed command
can outrun its own concurrent network setup), just one step further than
the eth0-existence race already fixed earlier in this file -- a test-code
issue, not a production bug. Fixed by polling for an actually assigned
address on eth0 instead of mere existence, in both wait_for_eth0_then()
and BackgroundPeer's own inline readiness script.
Verified with the diagnostic in place that the DNS resolver's own
namespace lookup was never itself stale, isolating the cause to the
script's own premature exit. Re-verified extensively after the fix:
8/8 isolated repeats of the previously-flaky tap+relay test, and 6
consecutive full [integration][root][net] suite runs (78 test-case
executions total) with no failures.
Two peers on the same intern network, one started with --hostname
peer-a, resolve and ping each other by name via the per-session dnsmasq
resolver (network_dns.cpp). Skips cleanly when dnsmasq isn't available.
wait_for_hostname_then() retries the whole ping-by-name probe (not just
an eth0 existence check) until it succeeds or the bound is hit, since
this races against two independent things starting concurrently with
the sandboxed command: the interface coming up, and the per-session
resolver picking up the peer's own hosts record -- same underlying
join_networks() timing limitation the earlier wait_for_eth0_then() fix
was for. Verified end-to-end as root, both variants.
The positive counterpart to the intern-network isolation tests: a
container joined to an extern network gets a default route through its
uplink and can reach a real outside address. Verified end-to-end as
root, both variants.
Same shape as the intern-network peer-ping pair, on an extern network
instead: confirms same-bridge peer reachability still works once the
extern uplink (network_bridge.cpp's ensure_uplink_provisioned()) is also
provisioned alongside the bridge. Verified end-to-end as root, both
veth and tap+relay variants.
A container joined only to an intern network gets no default route
(network_join.cpp), so pinging a real outside address (8.8.8.8) must
fail outright, not merely succeed slower than an extern join would.
Verified end-to-end as root: both variants correctly report
"Network unreachable" and the tests assert a RESULT= line was seen
(the command actually ran) that isn't RESULT=0.
First of a planned series of end-to-end -n/--network join tests
(tests/integration/test_network_join_scenarios.cpp, [integration][root][net]):
two containers joined to the same intern network ping each other by IP,
run once with a real veth pair and once forced onto the tap+relay
fallback, since the two are genuinely different implementations. IPv6 is
deliberately excluded pending a known device-specific peculiarity.
split_lines_trimmed()/extract_marked_lines() moved from
test_rootless_run.cpp into tests/support/fixtures.{h,cpp} for reuse here.
wait_for_eth0_then() wraps a sandboxed command's own network-touching
script in a poll for eth0 to exist first: join_networks() runs
concurrently with, not before, the sandboxed command starting, so a
near-instant command can otherwise exit before its own join finishes --
the exact limitation already documented in network_join.{h,cpp}'s own
CLAUDE.md entry. Confirmed by testing (not assumed): without this, the
container's own immediate ping-and-exit sometimes raced ahead of the
veth-move step, which then failed outright ("Invalid netns value")
against an already-exited pid.
Same class of test gap as the net-namespace test just fixed: this test
filtered which of pid/uts/ipc/cgroup to check by kernel support alone,
never by the config's own policy, so running the full suite as root with
every unshare-* flag forced off (via -c/--config-file) correctly made
bwrap skip requesting all four -- production code working as configured,
not a bug -- while the test still asserted each was isolated and failed.
Reuses namespace_type_would_isolate() (introduced in the previous commit)
to filter to_check by both gates instead of kernel support alone.
Full suite as root with everything disabled: 78 test cases, 76 passed, 2
skipped (this one and the net-namespace test), 0 failed -- the complete
picture requested.
Found by running the full suite as root with every unshare-* flag forced
off via -c/--config-file: this test only ever checked kernel support for
--unshare-net, never the config's own policy, so disabling it via
global.unshare-net correctly makes bwrap skip requesting the namespace --
production code working exactly as configured, not a bug -- while the test
still asserted isolation and failed.
Generalized the existing pid-only two-gate check into
namespace_type_would_isolate(type), covering any of bwrap.cpp's own
namespace_probes names, and used it here instead of a kernel-support-only
check.
create_session_cgroup() used to be called from run_bwrap()'s on_start
callback, in the parent, concurrently with the just-forked child execing
into bwrap and bwrap then doing its own internal clone() of the sandboxed
target. Without --unshare-pid, bwrap has little enough setup work to do
that it could reliably win that race, cloning its target before the
parent's own write into cgroup.procs completed -- leaving that target, and
everything it later spawns, permanently outside the tracked cgroup, so the
post-exit sweep found nothing to reap. Found via the user's own request to
test "pid namespace off, cgroup on" as root: confirmed directly by
inspecting cgroup.procs mid-session, showing only bwrap's own pid.
Fixed by giving run_process_foreground() a new before_exec hook, invoked in
the child synchronously right before execvp() -- the child cannot proceed
to exec (and thus cannot trigger any of bwrap's own internal forking) until
this has already returned, closing the race structurally rather than by
timing luck. run_bwrap() now creates the session cgroup there instead of in
on_start; the parent side just reconstructs the deterministic path
unconditionally, since the downstream sweep/cleanup functions already
tolerate a nonexistent directory gracefully either way.
Also fixes a false positive found while verifying this: the regression
test's own process-matching did a substring search across a whole cmdline
blob, which matched an unrelated manual `pkill -f 'sleep 137'` diagnostic
command run by hand during the investigation. Tightened to an exact
argv[0]/argv[1] match.
Finally, the regression test now SKIP()s (instead of failing) when neither
a pid namespace nor a working session cgroup is available for the current
effective config -- a documented, known residual limitation, not a
regression -- checked directly via two new helpers rather than assumed from
e.g. geteuid().
run_self_tests() now takes the same effective AppConfig any other command
gets and stashes it into a new g_test_app_config global (tests/support/
fixtures.h) before Catch2 runs anything. test_rootless_run.cpp's own
run_in_fixture() reads a copy of it instead of a hardcoded default
AppConfig{}, so -c actually reaches that test's container creation.
Verified: a config with every unshare-*/with-* key set to false, used via
-c <file> -t -- "[integration][net]~[root]", flips all 3 of that file's
container-creating tests to failing -- including the nohup-straggler
regression test, since with neither a pid namespace nor a cgroup nothing
reaps the backgrounded process -- while [unit] and non-networking
[integration] tests are completely unaffected, as expected.
Reproduces the user's reported real-world shape end to end with a real
busybox container ("nohup sleep 137 & exit"), verifying via the host's own
/proc that the backgrounded process is actually gone afterward -- checked
by cmdline substring, not pid, since a pid seen inside an isolated pid
namespace doesn't correspond to the same-numbered host pid. A bounded 2s
poll guards against the pid namespace's own kernel collapse-on-pid-1-exit
timing (which already covers this case for free on this dev machine).
Complements test_session_cleanup.cpp's existing [integration][root] test,
which exercises kill_via_cgroup() directly -- this one instead proves the
outward, visible contract holds through the real -r/--run path.
config.yaml now holds only the global section (log-level, unshare-*,
with-veth, with-ipv6); a new persistent.yaml holds volumes/networks.
load_config_file()/write_config_file() are replaced by
load_global_config()/load_persistent_config()/write_global_config()/
write_persistent_config(), each touching only their own file.
-c/--config-file <path> lets one invocation use an alternate file for the
global section only -- persistent.yaml is always the one fixed path,
regardless of -c, so an experiment can never affect real volumes/networks
(a -c file's own volumes/networks, if any, are simply never read either).
A -c path that doesn't exist is a hard error, unlike the default path's
existing missing-file leniency.
migrate_legacy_config_if_needed() moves volumes/networks out of an
old-format config.yaml into persistent.yaml on first run after upgrading,
always against the fixed default paths regardless of -c. A name collision
aborts the migration for that run (touching neither file) rather than
risking data loss.
Required reordering main() to parse CLI args before loading config (so
-c's value is known first) -- ParsedArgs::log_level_flag_given tracks
whether --log-level was already given so the config file's own log-level
doesn't clobber it despite the reversed call order.
Replaces --no-ipv6/--no-veth (plain flags) with --with-ipv6/--with-veth,
each taking an explicit true/false value (e.g. --with-veth=false), parsed
via the same parse_bool_flag() the config file itself already uses (now
exported from config_file.h so cli_args.cpp can reuse it).
create_network_command() now resolves ipv6/veth as CLI flag -> config's own
global.with-ipv6/global.with-veth -> true, so a host that always wants the
tap+relay fallback (or no IPv6) can set it once in the config instead of
passing the flag on every network creation. -w/--write-config fills in both
new keys like the existing six unshare-* bools.
test_session_cleanup.cpp exercises kill_via_cgroup() directly against two
plain forked processes (one setsid()-ing away from the other before it
exits), confirming a reparented straggler is actually reaped -- reproducing
the real escape shape (no pid namespace support at all) through a full
mount/bwrap session isn't possible from the CLI on a single run, since
--unshare-pid is a config-file-only setting, not a flag.
Also resolves TODO.md's SIGINT/SIGTERM entry and extends the relevant
CLAUDE.md sections (bwrap.{h,cpp}, session_cgroup.{h,cpp}, kill_session.{h,cpp})
with the fix's rationale and its known residual limitation (a kernel with
neither cgroup v2 nor pid namespace support still can't be reached
automatically).
Found via real-device testing (the actual Android target), not assumed:
both tests hardcoded assumptions that don't hold on every kernel.
1. "a fresh network namespace has only loopback" assumed exactly 3 lines
of /proc/net/dev (2-line header + one "lo" entry) -- the real target
device's kernel auto-creates several harmless placeholder tunnel
interfaces (sit0, ip6tnl0, ip_vti0, ip6_vti0) in *every* fresh network
namespace, alongside loopback. The namespace is still genuinely
isolated (confirmed: none of the *host's* real interfaces leak in) --
the test's assumption was just wrong for this kernel. Replaced the
exact-count check with a readlink-based /proc/self/ns/net identity
comparison (proves genuine isolation regardless of kernel config) plus
a simple "loopback is present" check, dropping the brittle count
assertion entirely.
2. "pid/uts/ipc namespaces differ from this process's own" assumed all
three are always readable via /proc/self/ns/<type>. The real target
device has neither PID nor IPC namespace support *as a kernel feature
at all* -- confirmed directly: even this test process's own `readlink
/proc/self/ns/pid`, run completely outside any container, fails
outright there. This matches this project's own already-documented
standing lesson (neither CONFIG_CHECKPOINT_RESTORE nor pid namespace
support on this target). Comparing against a namespace type the kernel
doesn't expose at all wouldn't prove anything either way.
Both tests now build their expectations from detect_bwrap_unshare_args()
(bwrap.h) -- this host's own live kernel-capability probe, the exact same
one build_bwrap_args() itself already gates on -- rather than assuming a
fixed set of namespace types is always available. "user" is deliberately
excluded from the generic per-type check: build_bwrap_args() never
requests --unshare-user when running as root, so asserting on it would be
wrong specifically when these tests are run as root (as they are on the
real device).
Verified: passes repeatably on this dev machine (all 6 namespace types
supported, 8 assertions/2 test cases either way -- same coverage as
before, just derived instead of hardcoded), full combined suite and
meson test both still clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
tests/integration/test_rootless_run.cpp runs a real busybox image through
the exact real -r/--run dispatch path (dispatch_command(), commands.h) --
mount, resolve, run_bwrap, unmount, cleanup, in-process rather than via a
subprocess -- and confirms bwrap's *default* sandboxing (no -n/-p at all)
is genuinely isolating: a fresh network namespace with nothing but
loopback, and pid/uts/ipc namespaces that differ from this test process's
own. No root needed, same as a plain `-r image.tar -- <command>` already
isn't.
New tests/support helpers: CapturedStdout (RAII, redirects this process's
own fd 1 -- and anything a forked/exec'd child inherits from it -- to a
throwaway temp file for its lifetime) so the sandboxed command's own
output can actually be asserted on.
Two real, non-obvious findings from getting this working, not assumed:
1. bwrap's own sandbox mounts --proc /proc and --dev /dev, but *not*
/sys -- confirmed directly (`ls /sys/class/net` inside the sandbox:
"No such file or directory", reproduced identically via the real CLI,
not just this test). Switched the loopback-only check to
/proc/net/dev instead (two header lines + one "<iface>: ..." line per
interface), which correctly shows only "lo".
2. spdlog's default sink writes to stdout, not stderr, same as the plain
"mounted image at: ..." success line (see CLAUDE.md) -- so a naive
capture-and-line-split mixed slocker-lite's own status/log output in
with the sandboxed command's real output. Fixed by having the
sandboxed command bracket its own output between two unique markers
and extracting only what's strictly between them.
Verified: both tests pass repeatably, 15 stress-test runs of the full
combined [unit]+[integration] suite with zero failures, plus a full run
as root.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
tests/support/fixtures.{h,cpp}: find_busybox_fixture() (images/busybox.tar
relative to cwd, this project's own established manual-testing convention
-- nullopt if absent, so [net] tests can SKIP() rather than fail) and
ScratchXdgDirs, an RAII helper pointing XDG_CONFIG_HOME/XDG_STATE_HOME at a
fresh throwaway mkdtemp() directory for its lifetime, restoring the
previous environment and removing the directory on destruction -- so
integration tests that actually exercise config_file_path()/xdg_state_dir()
never touch the real developer's own config/state.
tests/integration/test_config_bwrap_chain.cpp: the user's own example --
write a config file, load it back, resolve a NamespaceConfig from it the
same way run_container() (commands.cpp) does, and confirm build_bwrap_args()'s
resulting argv actually reflects it (disabled unshare-net/unshare-uts never
requested; an all-default config matches the live host's own
detect_bwrap_unshare_args() probe exactly). Neither test mounts/runs
anything or needs privilege.
Real bug found via ~10-30 repeated combined [unit]+[integration] runs, not
assumed: ScratchXdgDirs's constructor built its mkdtemp() template vector
from two *separate* temporary std::string objects (`.begin()` off one,
`.end()` off the other) -- mixing iterators from different containers is
undefined behavior, here manifesting as an intermittent, heap-address-
dependent `std::length_error: cannot create std::vector larger than
max_size()` inside whichever test happened to run adjacent to it. Fixed by
using a single named string instance for both ends of the range; confirmed
clean across 30 repeated combined runs afterward, plus a full run as root.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
persistent-netns, tap-relay, and dns-resolver (formerly hand-rolled
bool-returning functions in self_test.cpp, called unconditionally by the
old ad hoc run_self_tests()) move to tests/integration/test_root_networking.cpp
as TEST_CASEs tagged [integration][root][net] -- SKIP() (not a whole-suite
skip) when not root, or when dnsmasq isn't installed for the DNS one, so
e.g. -t -- "[unit]" on a rootless machine is unaffected. Every assertion
uses CHECK, not REQUIRE: these tests manage real host-side namespaces,
bridges, and tap devices that must not leak just because an earlier
assertion failed, so execution always falls through to the same
unconditional cleanup at the end (guarded only by simple pid/bool checks
to skip meaningless dependent steps).
Real bug found running the tap-relay test under Catch2, not assumed:
Catch2 installs its own fatal-signal handler around a running TEST_CASE,
which create_tap_relay()'s own forked relay child inherits -- so the
relay's ordinary shutdown SIGTERM (sent by stop_tap_relay()) got caught by
that *inherited* handler in the child instead of terminating it via the
default disposition the relay's own design relies on, producing a
spurious "FAILED ... due to a fatal error condition: SIGTERM" report
interleaved into the real output (confirmed cosmetic only -- exit code
and assertion count were correct either way, just confusing). Fixed by
resetting SIGTERM to SIG_DFL for the narrow window around the
create_tap_relay() call and restoring it right after -- only the
disposition at fork time is inherited, so nothing about how long the
relay then keeps running matters. No production code changed for this;
it's purely an artifact of forking network primitives from within a
Catch2-instrumented process.
meson.build: the new tests/integration/*.cpp sources are only added to
slocker-lite's own source list when enable_tests is true (mirroring
config.h's ENABLE_TESTS runtime guard, added last commit), and src/ is
added to the target's own include_directories so test sources under
tests/ can #include project headers the same way src/*.cpp already does.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
ceamac