Explain a missing tun device where the failure actually happens
Build App (Preview) / compute-version (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m37s
Build App (Preview) / build-linux (pull_request) Successful in 5m30s
Build App (Preview) / build-windows (pull_request) Successful in 5m55s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m37s
Build App (Preview) / build-linux (pull_request) Successful in 5m30s
Build App (Preview) / build-windows (pull_request) Successful in 5m55s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Review caught that the device guard was wired to the wrong call. The
daemon does not resolve `--device` at create: verified against Docker
29.7, `docker create --device /dev/does-not-exist` succeeds and prints an
id, and runc only resolves the device — and validates sysctls — when it
builds the container. So on a host with no tun module the create returns
fine and `start` fails, which means the explanation never ran and the
user saw the raw daemon string naming a path they would go looking for on
the wrong machine. The unit tests fed the create-side string straight in,
so they confirmed a function no real failure could reach.
Move the guard onto `start_container`, covering create as well in case a
future daemon checks earlier. It no longer takes `vpn_support_enabled` —
`start_container` has a container id and no project, and nothing else in
Triple-C ever requests a device, so an error naming /dev/net/tun is
unambiguous on its own. The test now uses the daemon's verbatim message
via bollard's real Display format.
Also from review:
* Soften the security claim. Docker does not enable user-namespace
remapping by default, so this is a real CAP_NET_ADMIN in the initial
user namespace with only the network namespace confining it. It
cannot touch host interfaces, but "confers no authority outside the
container" was too strong: within its namespace it can set
promiscuous mode and add addresses, routes and NAT on the shared
docker0 segment, which puts sibling containers — the LiteLLM gateway
among them — within ARP-spoofing reach, and it can flush netfilter
rules sandbox mode may rely on. Said plainly in the code, CLAUDE.md
and HOW-TO-USE.
* Drop Tailscale from the list of clients needing this. Its
--tun=userspace-networking mode needs neither the capability nor the
device, and listing it invites granting NET_ADMIN for nothing.
* Say in the toggle's own hint that changing it recreates the
container, matching how every other recreation-triggering setting is
labelled. The tab's generic "stop the container first" chip does not
tell the user what is about to happen.
* Add RuntimeSection tests: saves on, saves off explicitly rather than
dropping the key, reflects state, is disabled while running, and
carries the recreation warning.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -287,10 +287,21 @@ container is created once by a very long function where a dropped capability is
|
||||
Any two without the third still presents as a connection that hangs to a timeout, which is why
|
||||
the tests assert the whole set.
|
||||
- **The device is passed through from the host, never `mknod`-ed inside.** The kernel's `tun`
|
||||
module has to back it. When the host has no such device the failure lands at *creation* — the
|
||||
project simply won't start — so `explain_create_failure()` rewrites that one error to name the
|
||||
switch and the Docker-Desktop-VM-vs-your-machine distinction. Do not let it degrade to a raw
|
||||
bollard string.
|
||||
module has to back it.
|
||||
- **A missing device fails at `start`, not `create` — verified against Docker 29.7.** `docker
|
||||
create --device /dev/does-not-exist` succeeds and prints an id; runc resolves the device (and
|
||||
validates sysctls) only when it builds the container. So the guard belongs on the start path:
|
||||
`explain_container_failure()` covers both and is called from `start_container`, where it has a
|
||||
container id and no project — which is why it keys off the error naming `/dev/net/tun` rather
|
||||
than off `vpn_support_enabled`. Nothing else in Triple-C requests a device, so that is
|
||||
unambiguous. A version of this check wired to `create` alone is dead code that looks correct.
|
||||
- **`NET_ADMIN` here is not user-namespaced.** Docker does not enable userns remapping by default,
|
||||
so only the *network* namespace confines it: no reach onto host interfaces, but promiscuous
|
||||
mode, arbitrary addresses/routes/NAT on the shared `docker0` segment (sibling containers, the
|
||||
LiteLLM gateway among them, are ARP-spoofable), netlink-triggered host module auto-load, and
|
||||
enough authority to flush in-container netfilter rules that sandbox mode may rely on. Keep the
|
||||
code comments honest about this — an earlier draft claimed it "confers no authority" outside the
|
||||
container, which is too strong.
|
||||
- **`triple-c.vpn-support` is written unconditionally, including `false`.** The usual
|
||||
`docker commit` reason: a `true` stamped once would ride the snapshot image into every future
|
||||
container and make the switch impossible to turn off.
|
||||
|
||||
Reference in New Issue
Block a user