From 3bd3caa101cc4ea80a759d946e1877bf8b8dcc25 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 17 Aug 2026 12:51:30 -0700 Subject: [PATCH 1/5] Ship a pia-vpn skill with the VPN support toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The toggle grants CAP_NET_ADMIN and /dev/net/tun and stops there, which users reasonably read as "turn the VPN on" — the gap between the two is the reported bug that the default network does not route through a VPN. Close it by giving the container an agent-usable way to build the tunnel, rather than leaving each project to rediscover it. container/skills/ is baked to /opt/triple-c-skills and installed into ~/.claude/skills/ by entrypoint.sh from VPN_SUPPORT_ENABLED, mirroring how Mission Control installs its own. Staged under /opt because ~/.claude is a volume mount that would mask an image copy from first start. Three details that are not incidental: - The variable is sent as 0 rather than omitted when off, because ~/.claude persists: entrypoint has to be *told* to remove a skill left by an earlier run with the toggle on, and an absent variable cannot say that. A stale skill is worse than none, since it instructs an agent to use a capability the container no longer has. - It is reserved in RESERVED_ENV_EXACT alongside MISSION_CONTROL_ENABLED, or a custom env var of the same name could claim the skill without the capability behind it. Covered by a test. - The skill is re-copied on every start, rm -rf'd first, so fixes reach existing projects and files dropped from a later version do not linger. The skill itself carries the three things that are easy to get wrong: that a full tunnel captures the Docker resolver and takes DNS down with it, that an IP-literal health check cannot see a dead resolver, and that no tunnel survives a restart while /run state riding the snapshot makes it look as though one did. It also states what it deliberately does not do — no killswitch, no autostart — so an agent proposes those as decisions rather than improvising them. pia-wg.sh preflights CAP_NET_ADMIN by capability bit rather than letting the first `ip` call fail with a bare EPERM that points nowhere near the setting that needs changing. Credentials stay in a file (~/pia-creds, PIA_CREDS to override) rather than the environment, where docker inspect and every process in the container would see them. Tested: install/refresh/remove/no-op paths of install_feature_skill against the real function; preflight with and without the capability; and a full up --full / down round trip, confirming DNS via PIA's resolvers, api.anthropic.com reachable through the exit, and routes and resolv.conf restored on teardown. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 8 ++ HOW-TO-USE.md | 7 + app/src-tauri/src/docker/container.rs | 27 ++++ container/Dockerfile | 9 ++ container/entrypoint.sh | 29 +++++ container/skills/pia-vpn/SKILL.md | 149 +++++++++++++++++++++ container/skills/pia-vpn/pia-wg.sh | 178 ++++++++++++++++++++++++++ 7 files changed, 407 insertions(+) create mode 100644 container/skills/pia-vpn/SKILL.md create mode 100644 container/skills/pia-vpn/pia-wg.sh diff --git a/CLAUDE.md b/CLAUDE.md index 251f2c5..22dba43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -341,6 +341,14 @@ container is created once by a very long function where a dropped capability is breaks split tunnels too; `openresolv` has no candidate on noble and `resolvconf` drags in systemd-resolved, so that one is documented rather than fixed. Driving `wg` and `ip route` directly avoids both, which is what the skill does. +- **The `pia-vpn` skill is installed *and removed* from `VPN_SUPPORT_ENABLED`.** `container/skills/` + is baked to `/opt/triple-c-skills` and `install_feature_skill()` in `entrypoint.sh` copies it into + `~/.claude/skills/` on every start — refreshed each time, so a fix reaches existing projects, and + `rm -rf`'d first, so files dropped from a later version do not linger. The removal branch matters + as much as the install: `~/.claude` is a persisted volume, so a skill left behind after the toggle + goes off would keep instructing an agent to use a capability the container no longer has. Which is + also why the variable is sent as `0` rather than omitted, and why it is in `RESERVED_ENV_EXACT` — + a custom env var of that name could otherwise claim the skill without the capability behind it. ### Container Lifecycle diff --git a/HOW-TO-USE.md b/HOW-TO-USE.md index 85f53b6..21436ca 100644 --- a/HOW-TO-USE.md +++ b/HOW-TO-USE.md @@ -488,6 +488,13 @@ redirected, and no tunnel is configured or started on your behalf. Enabling it a container's traffic to start leaving through a VPN is the most common misreading of what it does — configuring a tunnel and routing traffic into it remains yours to do. +To make that second half easier, enabling this also installs a **`pia-vpn` skill** into the +container's `~/.claude/skills/`, so Claude Code can bring up a Private Internet Access tunnel over +WireGuard for you — ask it to connect the VPN and it will. The skill carries the parts that are +easy to get wrong (see the DNS note below), and it is removed again when you turn the setting off. +It needs your PIA credentials in `~/pia-creds`, two lines, username then password. If you use a +different provider, ignore it and set up your own client; nothing else depends on it. + With the setting **off**, a client such as PIA or OpenVPN installs and its daemon starts normally, but the connection attempt **hangs until it times out** — a default container has no tun device to open and no permission to add an interface or a route, and most clients report that as a generic timeout diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index e856577..9a5e864 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -233,6 +233,7 @@ const RESERVED_ENV_EXACT: &[&str] = &[ "MCP_SERVERS_JSON", "CLAUDE_CODE_SETTINGS_JSON", "MISSION_CONTROL_ENABLED", + "VPN_SUPPORT_ENABLED", "TRIPLE_C_PERMISSION_MODE", CLAUDE_OAUTH_TOKEN_ENV, // The model-alias vars are already covered by the `ANTHROPIC_` prefix @@ -1275,6 +1276,15 @@ pub async fn create_container( env_vars.push("MISSION_CONTROL_ENABLED=1".to_string()); } + // Drives the pia-vpn skill install in entrypoint.sh. Sent as 0 rather than + // omitted when off, because ~/.claude is a persisted volume: entrypoint has + // to be told to *remove* a skill left there by an earlier run with the + // toggle on, and an absent variable cannot say that. + env_vars.push(format!( + "VPN_SUPPORT_ENABLED={}", + u8::from(project.vpn_support_enabled) + )); + // Permission mode — read by triple-c-task-runner for scheduled (headless) // Claude Code runs. Interactive terminals get the flags directly instead. env_vars.push(format!( @@ -2789,6 +2799,23 @@ mod tests { assert_eq!(cap_add.unwrap(), vec!["NET_ADMIN"]); } + #[test] + fn the_vpn_skill_flag_is_reserved_from_custom_env() { + // entrypoint.sh installs and removes the pia-vpn skill from this + // variable. A custom env var of the same name would let a project claim + // the skill without the capability behind it — or keep it after the + // toggle is off — so it has to be unsettable like the others. + assert!(is_reserved_env_key("VPN_SUPPORT_ENABLED")); + assert!(is_reserved_env_key("vpn_support_enabled")); + assert_eq!( + compute_env_fingerprint(&[EnvVar { + key: "VPN_SUPPORT_ENABLED".to_string(), + value: "1".to_string(), + }]), + "" + ); + } + /// What bollard actually hands us when a tun-less host rejects the device. /// /// Captured verbatim from Docker 29.7: `docker create` with a missing diff --git a/container/Dockerfile b/container/Dockerfile index b94b76c..b980a3d 100644 --- a/container/Dockerfile +++ b/container/Dockerfile @@ -394,6 +394,15 @@ RUN chmod +x /usr/local/bin/triple-c-sso-refresh COPY mission-control /opt/mission-control +# Skills that ship with a Triple-C feature rather than with Mission Control. +# entrypoint.sh installs them into ~/.claude/skills/ when the feature that owns +# them is enabled, and removes them when it is not — a skill telling an agent to +# build a tunnel in a container that no longer has CAP_NET_ADMIN is worse than +# no skill at all. Staged in /opt because ~/.claude is a volume mount: an image +# copy underneath it would be masked from the project's first start onward. +COPY skills /opt/triple-c-skills +RUN chmod +x /opt/triple-c-skills/*/*.sh + COPY entrypoint.sh /usr/local/bin/entrypoint.sh RUN chmod +x /usr/local/bin/entrypoint.sh COPY triple-c-scheduler /usr/local/bin/triple-c-scheduler diff --git a/container/entrypoint.sh b/container/entrypoint.sh index 5f7535e..92435d7 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -338,6 +338,35 @@ if [ "$MISSION_CONTROL_ENABLED" = "1" ]; then unset MISSION_CONTROL_ENABLED fi +# ── Feature skills ────────────────────────────────────────────────────────── +# Skills owned by a Triple-C feature rather than by Mission Control. Installed +# when the feature is on, removed when it is off: ~/.claude is a persisted +# volume, so a skill left behind after its feature is disabled would keep +# telling an agent to use a capability the container no longer has. +# +# Copied on every start rather than only when absent, so a fix to a skill +# reaches projects that already have the old copy. Local edits under these +# directories do not survive — treat /opt/triple-c-skills as the source. +install_feature_skill() { + _name=$1 + _enabled=$2 + _dest="/home/claude/.claude/skills/$_name" + if [ "$_enabled" = "1" ]; then + [ -d "/opt/triple-c-skills/$_name" ] || return 0 + mkdir -p /home/claude/.claude/skills + rm -rf "$_dest" + cp -r "/opt/triple-c-skills/$_name" "$_dest" + chown -R claude:claude "$_dest" + echo "entrypoint: $_name skill installed to ~/.claude/skills/" + elif [ -d "$_dest" ]; then + rm -rf "$_dest" + echo "entrypoint: $_name skill removed (feature disabled)" + fi +} + +install_feature_skill pia-vpn "${VPN_SUPPORT_ENABLED:-0}" +unset VPN_SUPPORT_ENABLED + # ── Claude Code settings ──────────────────────────────────────────────────── # Merge Claude Code settings into ~/.claude/settings.json (preserves existing # keys). Creates the file if it doesn't exist. These control TUI mode, effort diff --git a/container/skills/pia-vpn/SKILL.md b/container/skills/pia-vpn/SKILL.md new file mode 100644 index 0000000..78fb200 --- /dev/null +++ b/container/skills/pia-vpn/SKILL.md @@ -0,0 +1,149 @@ +--- +name: pia-vpn +description: Connect this container's traffic through a PIA VPN tunnel over WireGuard, or diagnose one that is not working. Use when asked to enable, route through, check, or tear down a VPN, when traffic needs to leave from a different location, or when DNS or connectivity broke after a VPN was brought up. +--- + +# PIA VPN + +Bring this container's traffic out through Private Internet Access over +WireGuard, using the API PIA documents for headless use. + +Run `sudo ~/.claude/skills/pia-vpn/pia-wg.sh` with `up`, `up --full`, `down` or +`status`. Read the rest of this page before the first `up --full` — two of the +behaviours below are actively misleading if you meet them without warning. + +## Before anything else: what the toggle does not do + +Triple-C's **VPN support** setting grants three things — `CAP_NET_ADMIN`, the +`/dev/net/tun` device, and the `net.ipv4.conf.all.src_valid_mark` sysctl — and +stops there. It starts no client, builds no tunnel and changes no route. + +So "the VPN is enabled but traffic isn't going through it" is normally not a +fault. It means the capability is present and nothing has used it yet. Check +with `status` before assuming something is broken. + +If the toggle is off, the script says so and names the setting. It cannot be +turned on from inside the container; the user changes it in Config → Runtime, +and it recreates the container on the next start (home and `.claude` volumes +are preserved — it is not a Reset). + +## Two modes + +| | routes | use when | +|---|---|---| +| `up` | only `1.1.1.1/32` | verifying the tunnel works without disturbing anything | +| `up --full` | all public traffic | you actually want traffic leaving via PIA | + +Prefer `up` first. It proves the handshake, credentials and region are good +while your own connectivity is untouched, so a failure is cheap. + +**`up --full` routes Claude Code's own API traffic through PIA.** If the tunnel +drops, that traffic stops until it recovers or you run `down`. Say so before +running it — the user may be mid-session, and they will experience the failure +as Claude going away, not as a VPN problem. + +## Trap 1: a full tunnel takes DNS with it + +The container resolves through an address on the Docker network — under Docker +Desktop, `192.168.65.7` — which sits **outside** the container's own subnet. A +default route of `0.0.0.0/0`, or the `0.0.0.0/1` + `128.0.0.0/1` pair, captures +it and posts every lookup into a tunnel that cannot carry private traffic. + +Nothing resolves after that. The visible symptom is Claude Code reporting it +cannot connect, because `api.anthropic.com` no longer resolves: + +``` +$ curl https://api.anthropic.com/v1/messages +* Could not resolve host: api.anthropic.com (rc=6) +``` + +`pia-wg.sh` already handles this: it routes `10.0.0.0/8`, `172.16.0.0/12`, +`192.168.0.0/16` and `169.254.0.0/16` back via the original gateway, then pins +PIA's own resolvers through the tunnel with `/32` routes that outrank the +`10/8` exclusion. If you ever route traffic by hand, you owe both halves — the +exclusions *and* a resolver reachable from wherever you pointed the default. + +## Trap 2: an IP-literal health check cannot see a dead resolver + +`curl https://1.1.1.1/cdn-cgi/trace` needs no DNS, so it returns a cheerful +PIA exit address while name resolution is entirely broken. A tunnel verified +that way looks perfect and works for nothing. + +`status` resolves a real name for this reason. Trust its `DNS:` line, and if +you check by hand, resolve a name rather than fetching an address. + +## Trap 3: no tunnel survives a restart, and it fails open + +The network namespace is rebuilt every time the container starts, and nothing +inside reconnects anything. After a stop/start, Reset or any config change that +recreates the container, the interface and its routes are gone. + +State under `/run/pia-wg` rides the snapshot and persists, so leftover files +make it look as though the tunnel is still configured. It is not. Traffic goes +out the real address with no error and nothing visibly different. + +Never infer from `/run/pia-wg` that a tunnel is up. Run `status` — if the +handshake line is missing, there is no tunnel. Re-run `up` after every start. + +## Credentials + +Two lines in `~/pia-creds` — username, then password: + +``` +p1234567 +your-password +``` + +Set `PIA_CREDS` to use a different path. Treat the contents as secret: never +print the file, never echo the values, and never include them in a commit, a +log or a message. The script reads it directly and does not echo it. + +## Regions + +Defaults to `us_chicago`. Override with `PIA_REGION`: + +```bash +sudo PIA_REGION=uk_london ~/.claude/skills/pia-vpn/pia-wg.sh up --full +``` + +List the ids: + +```bash +curl -s https://serverlist.piaservers.net/vpninfo/servers/v6 \ + | head -1 | jq -r '.regions[].id' +``` + +## Verifying + +`status` prints three things — handshake, DNS, and the public address: + +``` + latest handshake: 2 seconds ago + transfer: 92 B received, 180 B sent +DNS: ok (via 10.0.0.243 10.0.0.242) +public IP: 64.113.5.244 +``` + +All three matter. A handshake with `DNS: BROKEN` is trap 1. A handshake with an +unchanged public address means routing did not take — you are probably in `up` +rather than `up --full`. + +## Tearing down + +`down` restores `resolv.conf` from its backup and removes exactly the routes +that were added, in reverse order, then deletes the interface. It is safe to +run when nothing is up. Confirm afterwards that the public address is back to +the container's own. + +## What this deliberately does not do + +- **No killswitch.** Blocking non-tunnel egress needs `iptables`, which is not + in the image, and would cut Claude Code's API traffic whenever the tunnel is + down. If the user needs guaranteed egress rather than convenient egress, say + so plainly rather than improvising one — it is a real design decision. +- **No autostart.** There is no service manager in the container and Triple-C + has no start hook, so nothing can re-establish the tunnel automatically. +- **Not PIA's desktop client.** `pia-daemon` and `piactl` are installable but + cannot work headless: the daemon never accepts a client connection without + the GUI, and `piactl --help` states that connecting requires it. If you find + one installed, it is not a working alternative to this script. diff --git a/container/skills/pia-vpn/pia-wg.sh b/container/skills/pia-vpn/pia-wg.sh new file mode 100644 index 0000000..d1b5928 --- /dev/null +++ b/container/skills/pia-vpn/pia-wg.sh @@ -0,0 +1,178 @@ +#!/usr/bin/env bash +# PIA over WireGuard, headless. +# +# PIA's desktop client (pia-daemon + piactl) cannot work here: its daemon never +# accepts a client connection without the GUI running, and `piactl --help` says +# as much. This talks to PIA's public API directly instead, which is the path +# PIA themselves document for headless use. +# +# sudo pia-wg.sh up tunnel up, only 1.1.1.1 routed through it (safe test) +# sudo pia-wg.sh up --full tunnel up, all *public* traffic exits via PIA +# sudo pia-wg.sh down tear down, restoring DNS and routes +# sudo pia-wg.sh status handshake, DNS and current public IP +# +# Requires the project's "VPN support" setting (Config -> Runtime) to be on. +# +# PIA_CREDS credentials file, two lines: username, then password +# (default ~/pia-creds; never echoed by this script) +# PIA_REGION region id (default us_chicago). List them with: +# curl -s https://serverlist.piaservers.net/vpninfo/servers/v6 \ +# | head -1 | jq -r '.regions[].id' +set -euo pipefail + +CREDS=${PIA_CREDS:-/home/claude/pia-creds} +REGION=${PIA_REGION:-us_chicago} +IFACE=pia0 +STATE=/run/pia-wg + +# Kept off the tunnel in --full mode. The container's DNS resolver, the Docker +# host network (host.docker.internal, any host-side Ollama), sibling containers +# and the LAN all live in here. PIA cannot route any of it, so without these +# exclusions the container reaches the public internet and nothing else -- +# including, fatally, its own resolver. +PRIVATE_NETS="10.0.0.0/8 172.16.0.0/12 192.168.0.0/16 169.254.0.0/16" + +# Args are joined with spaces so a long message can be written as several +# source lines without the indentation ending up in the output. +die() { echo "pia-wg: $*" >&2; exit 1; } + +preflight() { + [ "$(id -u)" = 0 ] || die "run with sudo" + # CAP_NET_ADMIN is bit 12. Checking it by name gives a usable error; without + # it the first `ip` call fails with a bare "Operation not permitted" that + # points nowhere near the setting that actually needs changing. + local caps + caps=$(awk '/^CapEff:/{print $2}' /proc/self/status) + if [ $(( 0x$caps & 0x1000 )) -eq 0 ]; then + die "this container has no CAP_NET_ADMIN." \ + "Turn on \"VPN support\" in Config -> Runtime and start the project" \ + "again. That recreates the container; the home and .claude volumes" \ + "are preserved, so nothing in them is lost." + fi + [ -e /dev/net/tun ] || \ + die "/dev/net/tun is missing." \ + "Same fix: turn on \"VPN support\" in Config -> Runtime. If it is" \ + "already on, the Docker host's kernel is missing the tun module." + command -v wg >/dev/null || die "wireguard-tools is not installed." + [ -r "$CREDS" ] || \ + die "no credentials at $CREDS." \ + "Two lines are expected: username, then password." \ + "Set PIA_CREDS to read them from somewhere else." +} + +# Record every route we add so teardown removes exactly those and nothing else. +add_route() { ip route add $1 2>/dev/null && echo "$1" >> "$STATE/routes" || true; } + +up() { + preflight + mkdir -p "$STATE"; cd "$STATE" + [ -f ca.rsa.4096.crt ] || curl -sf -m 20 -o ca.rsa.4096.crt \ + https://raw.githubusercontent.com/pia-foss/manual-connections/master/ca.rsa.4096.crt \ + || die "could not fetch PIA's CA certificate" + + local u p tok srv sip scn priv pub resp ep gw dns + u=$(sed -n 1p "$CREDS"); p=$(sed -n 2p "$CREDS") + tok=$(curl -sf -m 25 -u "$u:$p" \ + https://www.privateinternetaccess.com/gtoken/generateToken | jq -r .token) + [ -n "$tok" ] && [ "$tok" != null ] || die "PIA authentication failed - check $CREDS" + + curl -sf -m 30 https://serverlist.piaservers.net/vpninfo/servers/v6 | head -1 > servers.json + srv=$(jq -r --arg r "$REGION" '.regions[] | select(.id==$r) | .servers.wg[0]' servers.json) + sip=$(echo "$srv" | jq -r .ip); scn=$(echo "$srv" | jq -r .cn) + [ -n "$sip" ] && [ "$sip" != null ] || die "no WireGuard server for region $REGION" + + priv=$(wg genkey); pub=$(echo "$priv" | wg pubkey) + printf '%s' "$priv" > wg.priv; chmod 600 wg.priv + + # PIA pins its certificate to the server's common name, which is why this + # connects by CN and lets --connect-to point that name at the real address. + resp=$(curl -sf -m 25 -G --connect-to "$scn::$sip:" --cacert ca.rsa.4096.crt \ + --data-urlencode "pt=$tok" --data-urlencode "pubkey=$pub" \ + "https://$scn:1337/addKey") + [ "$(echo "$resp" | jq -r .status)" = OK ] || die "key registration failed: $resp" + + : > "$STATE/routes" + ip link del "$IFACE" 2>/dev/null || true + ip link add "$IFACE" type wireguard + wg set "$IFACE" private-key wg.priv \ + peer "$(echo "$resp" | jq -r .server_key)" \ + endpoint "$(echo "$resp" | jq -r .server_ip):$(echo "$resp" | jq -r .server_port)" \ + allowed-ips 0.0.0.0/0 persistent-keepalive 25 + ip addr add "$(echo "$resp" | jq -r .peer_ip)/32" dev "$IFACE" + ip link set "$IFACE" up + + if [ "${1:-}" = "--full" ]; then + # Pin the endpoint to the pre-existing gateway first, so the tunnel's own + # packets do not try to route through the tunnel. Then beat the default + # route with two half-routes rather than replacing it -- nothing to restore + # on teardown, and the container keeps working if this script dies midway. + ep=$(echo "$resp" | jq -r .server_ip) + gw=$(ip route show default | awk '{print $3; exit}') + add_route "$ep/32 via $gw" + add_route "0.0.0.0/1 dev $IFACE" + add_route "128.0.0.0/1 dev $IFACE" + + # Keep container, host and LAN traffic off the tunnel. Longer prefixes than + # the two halves above, so these win. + for n in $PRIVATE_NETS; do add_route "$n via $gw"; done + + # PIA's resolver lives inside 10/8, so pin it back through the tunnel with a + # /32 -- longer still, so it beats the 10.0.0.0/8 exclusion just added. + # Using PIA's resolver rather than the container's keeps DNS from leaking, + # and the container's own resolver is unreachable from inside the tunnel. + dns=$(echo "$resp" | jq -r '.dns_servers[]? // empty' | head -2) + if [ -n "$dns" ]; then + cp /etc/resolv.conf "$STATE/resolv.conf.bak" + for d in $dns; do add_route "$d/32 dev $IFACE"; done + # resolv.conf is a bind mount: write through it, never replace it. + for d in $dns; do echo "nameserver $d"; done > /etc/resolv.conf + else + echo "pia-wg: warning - PIA returned no DNS servers; leaving resolv.conf alone" >&2 + fi + echo "full tunnel: public traffic exits via PIA; private ranges stay local" + else + add_route "1.1.1.1/32 dev $IFACE" + echo "test route only: 1.1.1.1 goes via PIA, everything else unchanged" + fi + + sleep 2 + status +} + +down() { + [ "$(id -u)" = 0 ] || die "run with sudo" + if [ -f "$STATE/resolv.conf.bak" ]; then + cat "$STATE/resolv.conf.bak" > /etc/resolv.conf + rm -f "$STATE/resolv.conf.bak" + fi + if [ -f "$STATE/routes" ]; then + # Reverse order: the specific overrides go before the ranges they sit in. + tac "$STATE/routes" | while read -r r; do + [ -n "$r" ] && ip route del $r 2>/dev/null || true + done + rm -f "$STATE/routes" + fi + ip link del "$IFACE" 2>/dev/null || true + echo "tunnel down" +} + +status() { + wg show "$IFACE" 2>/dev/null | grep -E "latest handshake|transfer" || echo "no tunnel up" + # Resolve a name, not an IP literal. A curl to 1.1.1.1 succeeds while DNS is + # completely broken, which is exactly how a dead resolver goes unnoticed. + printf 'DNS: ' + if timeout 10 getent hosts api.anthropic.com >/dev/null 2>&1; then + echo "ok (via $(sed -n 's/^nameserver //p' /etc/resolv.conf | tr '\n' ' '))" + else + echo "BROKEN - cannot resolve api.anthropic.com" + fi + echo -n "public IP: " + curl -s -m 20 https://1.1.1.1/cdn-cgi/trace | sed -n 's/^ip=//p' +} + +case "${1:-}" in + up) shift; up "${1:-}" ;; + down) down ;; + status) status ;; + *) sed -n '2,20p' "$0" | sed 's/^# \{0,1\}//'; exit 1 ;; +esac -- 2.52.0 From 7a8bbcbef706f039dfdfbda091a9810597d6c4f8 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 17 Aug 2026 12:54:17 -0700 Subject: [PATCH 2/5] Report which exit is which, instead of one ambiguous "public IP" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `status` probed https://1.1.1.1/cdn-cgi/trace and printed the answer as "public IP". In test mode 1.1.1.1 is the *only* address routed into the tunnel, so that line reported a PIA exit while every other packet left directly — a test tunnel reading exactly like a full one. Found on a live container: default route still via eth0, one 1.1.1.1/32 route through pia0, and the old status line claiming a PIA public IP. This is a plausible route to concluding the VPN is on when it is not, which is close to the confusion this skill exists to prevent. Status now names the mode and, in test mode, prints both exits with the real address called out. 1.0.0.1 serves the same trace endpoint as 1.1.1.1 and is never routed into the tunnel, so the direct exit can be probed without DNS. Verified against all four states: no tunnel, test mode on a live tunnel that was already up, full tunnel, and after teardown. Co-Authored-By: Claude Opus 5 (1M context) --- container/skills/pia-vpn/SKILL.md | 42 ++++++++++++++++++++++++------ container/skills/pia-vpn/pia-wg.sh | 27 +++++++++++++++++-- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/container/skills/pia-vpn/SKILL.md b/container/skills/pia-vpn/SKILL.md index 78fb200..d7554ad 100644 --- a/container/skills/pia-vpn/SKILL.md +++ b/container/skills/pia-vpn/SKILL.md @@ -9,8 +9,10 @@ Bring this container's traffic out through Private Internet Access over WireGuard, using the API PIA documents for headless use. Run `sudo ~/.claude/skills/pia-vpn/pia-wg.sh` with `up`, `up --full`, `down` or -`status`. Read the rest of this page before the first `up --full` — two of the -behaviours below are actively misleading if you meet them without warning. +`status`. Read the rest of this page before the first `up --full` — three of the +behaviours below are actively misleading if you meet them without warning, and +each one presents as "the VPN is fine" or "Claude is broken" rather than as +what it is. ## Before anything else: what the toggle does not do @@ -72,7 +74,27 @@ that way looks perfect and works for nothing. `status` resolves a real name for this reason. Trust its `DNS:` line, and if you check by hand, resolve a name rather than fetching an address. -## Trap 3: no tunnel survives a restart, and it fails open +## Trap 3: in test mode, the obvious probe is the one thing tunnelled + +`up` routes `1.1.1.1` and nothing else. So checking your address by fetching +`https://1.1.1.1/cdn-cgi/trace` reports a **PIA** address — not because your +traffic is going through PIA, but because that single probe is. Everything else +still leaves directly. + +This reads exactly like a working full tunnel, and it is the likeliest reason +someone concludes the VPN is on when it is not. `status` prints both exits in +test mode for this reason: + +``` +mode: test route only (1.1.1.1 through the tunnel, nothing else) + through the tunnel: 64.113.5.73 + everything else: 172.116.197.166 <- your real address +``` + +Two different addresses there is correct and expected in test mode. If you want +the second line to change, you want `up --full`. + +## Trap 4: no tunnel survives a restart, and it fails open The network namespace is rebuilt every time the container starts, and nothing inside reconnects anything. After a stop/start, Reset or any config change that @@ -115,18 +137,22 @@ curl -s https://serverlist.piaservers.net/vpninfo/servers/v6 \ ## Verifying -`status` prints three things — handshake, DNS, and the public address: +`status` prints the handshake, DNS, and which address traffic actually leaves +from — labelled by mode, so the answer cannot be misread: ``` latest handshake: 2 seconds ago transfer: 92 B received, 180 B sent DNS: ok (via 10.0.0.243 10.0.0.242) -public IP: 64.113.5.244 +mode: full tunnel + all traffic exits: 64.113.5.244 ``` -All three matter. A handshake with `DNS: BROKEN` is trap 1. A handshake with an -unchanged public address means routing did not take — you are probably in `up` -rather than `up --full`. +All of it matters. A handshake with `DNS: BROKEN` is trap 1. `mode: test route +only` with two different addresses is trap 3, and is correct — it means the +tunnel works and you have not asked for it to carry anything yet. Report the +mode line when telling someone the VPN is on; "the public IP is a PIA one" is +true in test mode too, and means much less than it sounds like. ## Tearing down diff --git a/container/skills/pia-vpn/pia-wg.sh b/container/skills/pia-vpn/pia-wg.sh index d1b5928..8e5c8c6 100644 --- a/container/skills/pia-vpn/pia-wg.sh +++ b/container/skills/pia-vpn/pia-wg.sh @@ -156,8 +156,17 @@ down() { echo "tunnel down" } +# Both are Cloudflare and both answer /cdn-cgi/trace over their bare address, so +# neither needs DNS. Only 1.1.1.1 is ever routed into the tunnel, which is what +# lets status tell the two exits apart. +TRACE_TUNNELLED=https://1.1.1.1/cdn-cgi/trace +TRACE_DIRECT=https://1.0.0.1/cdn-cgi/trace + +exit_ip() { curl -s -m 20 "$1" | sed -n 's/^ip=//p'; } + status() { wg show "$IFACE" 2>/dev/null | grep -E "latest handshake|transfer" || echo "no tunnel up" + # Resolve a name, not an IP literal. A curl to 1.1.1.1 succeeds while DNS is # completely broken, which is exactly how a dead resolver goes unnoticed. printf 'DNS: ' @@ -166,8 +175,22 @@ status() { else echo "BROKEN - cannot resolve api.anthropic.com" fi - echo -n "public IP: " - curl -s -m 20 https://1.1.1.1/cdn-cgi/trace | sed -n 's/^ip=//p' + + # Report the exit per mode. In test mode the probe address is itself the one + # thing inside the tunnel, so a single "public IP" line would print a PIA + # address while every other packet leaves directly -- the exact reading that + # makes a test tunnel look like a full one. + if ip route show 0.0.0.0/1 2>/dev/null | grep -q "$IFACE"; then + echo "mode: full tunnel" + echo " all traffic exits: $(exit_ip "$TRACE_TUNNELLED")" + elif ip link show "$IFACE" >/dev/null 2>&1; then + echo "mode: test route only (1.1.1.1 through the tunnel, nothing else)" + echo " through the tunnel: $(exit_ip "$TRACE_TUNNELLED")" + echo " everything else: $(exit_ip "$TRACE_DIRECT") <- your real address" + else + echo "mode: no tunnel" + echo " all traffic exits: $(exit_ip "$TRACE_DIRECT")" + fi } case "${1:-}" in -- 2.52.0 From dcb13d23ea735a4ae54e6f6d4edbf4f5ccb5bc15 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 17 Aug 2026 14:04:39 -0700 Subject: [PATCH 3/5] Fix what review found in the skill: five real defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of #29 found bugs I confirmed by reproducing each one. **Every hand-written error message was unreachable.** `tok=$(curl ...)` is a plain assignment, so `set -e` acts on the command substitution before the following `|| die` can run. A wrong password produced exit 22 and no output at all — the most likely way this gets used wrongly, and the least explained. All four captures now go through a `run` helper that takes a *description* rather than echoing the command, because one of them carries the account password in `-u`. **`up` was not idempotent, and the second run destroyed DNS.** The resolv.conf backup was copied unconditionally, so `up --full` twice overwrote the good backup with PIA's own resolvers; the later `down` then "restored" those and left the container with no working DNS and no way back. `up` now runs `down` first. Verified: two `up --full` runs, then `down`, and the backup still holds the original 192.168.65.7. **An empty gateway produced total connectivity loss, reported as healthy.** `$gw` was never validated and `add_route` swallowed every failure to /dev/null. The two half-routes need no gateway and would succeed, so the tunnel captured everything while the exclusions keeping DNS and the Docker host reachable silently did not exist — and `status` still printed "full tunnel". Routes are now fatal on failure, and a via-less default (`$3` is the literal "eth0") is rejected. **The PIA session token was in the process arguments** — confirmed in `ps` and /proc/*/cmdline, a ~24h bearer credential for the account readable by anything in the container. It now goes to curl on stdin as a config. Verified: 60 polls across a full `up`, zero sightings. **The preflight diagnosed the wrong kernel module.** It checked /dev/net/tun and blamed the tun module, but kernel WireGuard is a netlink interface and does not use it — verified by creating one with NET_ADMIN and no tun device. The check is dropped (the container could not have started without the device anyway) and `ip link add` now reports the real dependency. Also: a full tunnel with no DNS servers from PIA used to warn and carry on, which is a tunnel leaking every lookup while reporting itself healthy — now fatal. `down` validates the backup before restoring it, so a truncated one cannot leave the container with no resolver at all. `wg.priv` is shredded on teardown and created under umask 077, because /run rides `docker commit` into the snapshot image. A mistyped `up --ful` is rejected instead of silently giving a test route. entrypoint: `install_feature_skill` gets `local`, a blank-name guard (the disabled branch would otherwise `rm -rf` the whole skills directory under a persisted volume), `-e`/`-L` so a leftover *file* at the destination is cleaned up, and a chown of the parent so `claude` can still add skills of their own when Mission Control is off. When the base image predates the skill it now says so instead of returning silently — and `/opt/triple-c-skills` joins FEATURE_PROBES so the migration pre-flight reports it. Docs corrected to match: neither half reaches an existing project without a migration. `vpn_env_var` extracted and tested, pinning the property the whole removal path rests on — that the variable is emitted as 0 rather than omitted. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 20 ++- HOW-TO-USE.md | 4 + app/src-tauri/src/docker/container.rs | 37 ++++-- app/src-tauri/src/docker/migration.rs | 1 + container/Dockerfile | 4 +- container/entrypoint.sh | 31 ++++- container/skills/pia-vpn/SKILL.md | 52 ++++++-- container/skills/pia-vpn/pia-wg.sh | 169 +++++++++++++++++++------- 8 files changed, 244 insertions(+), 74 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 22dba43..f8ef401 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -343,12 +343,20 @@ container is created once by a very long function where a dropped capability is directly avoids both, which is what the skill does. - **The `pia-vpn` skill is installed *and removed* from `VPN_SUPPORT_ENABLED`.** `container/skills/` is baked to `/opt/triple-c-skills` and `install_feature_skill()` in `entrypoint.sh` copies it into - `~/.claude/skills/` on every start — refreshed each time, so a fix reaches existing projects, and - `rm -rf`'d first, so files dropped from a later version do not linger. The removal branch matters - as much as the install: `~/.claude` is a persisted volume, so a skill left behind after the toggle - goes off would keep instructing an agent to use a capability the container no longer has. Which is - also why the variable is sent as `0` rather than omitted, and why it is in `RESERVED_ENV_EXACT` — - a custom env var of that name could otherwise claim the skill without the capability behind it. + `~/.claude/skills/` on every start — refreshed each time, so a fix reaches any project whose base + image has the source, and `rm -rf`'d first, so files dropped from a later version do not linger. + The removal branch matters as much as the install: `~/.claude` is a persisted volume, so a skill + left behind after the toggle goes off would keep instructing an agent to use a capability the + container no longer has. Which is also why the variable is sent as `0` rather than omitted (see + `vpn_env_var`, tested), and why it is in `RESERVED_ENV_EXACT` — a custom env var of that name + could otherwise claim the skill without the capability behind it. +- **Both halves of that live in the base image, so neither reaches an existing project.** A + recreation builds from the project's *own snapshot*, which has no `/opt/triple-c-skills` and no + updated `entrypoint.sh`; only a migration or a Reset delivers them. The install path says so out + loud rather than returning silently, and `/opt/triple-c-skills` is in `FEATURE_PROBES` so the + migration pre-flight lists it as missing. Worth knowing before adding anything else behind an + existing toggle: the label fingerprints *the setting*, not the set of things the setting drives, + so a project already at `true` gets no recreation at all on upgrade. ### Container Lifecycle diff --git a/HOW-TO-USE.md b/HOW-TO-USE.md index 21436ca..643b022 100644 --- a/HOW-TO-USE.md +++ b/HOW-TO-USE.md @@ -495,6 +495,10 @@ easy to get wrong (see the DNS note below), and it is removed again when you tur It needs your PIA credentials in `~/pia-creds`, two lines, username then password. If you use a different provider, ignore it and set up your own client; nothing else depends on it. +Like the VPN tooling above, the skill ships in the container image, so a project whose container +predates it will not get one by toggling the setting — **migrate the project** and it appears; the +migration pre-flight lists it among what you would gain. + With the setting **off**, a client such as PIA or OpenVPN installs and its daemon starts normally, but the connection attempt **hangs until it times out** — a default container has no tun device to open and no permission to add an interface or a route, and most clients report that as a generic timeout diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index 9a5e864..f4ca0c0 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -841,6 +841,23 @@ type VpnHostConfigParts = ( /// host-kernel module auto-loading. It is also enough to flush netfilter rules /// inside the container, so pair it with `sandbox_mode_enabled` advisedly. /// Hence opt-in, per project, rather than on for everyone. +/// The env var `entrypoint.sh` installs and removes the `pia-vpn` skill from. +/// +/// **Emitted either way, never omitted.** `~/.claude` is a persisted volume, so +/// turning the toggle off has to actively tell entrypoint to remove a skill an +/// earlier run left there, and an absent variable cannot say that. It is also +/// what stops a `=1` baked into a snapshot by `docker commit` from outliving +/// the setting — the explicit `=0` overwrites it. +/// +/// Extracted for the same reason as [`vpn_host_config`]: the emitting code sits +/// in a very long function where a dropped or inverted value is invisible, and +/// `MISSION_CONTROL_ENABLED` twenty lines above shows the failure this avoids — +/// it is pushed only when true, so a snapshot's baked `=1` survives the toggle +/// going off. +fn vpn_env_var(enabled: bool) -> String { + format!("VPN_SUPPORT_ENABLED={}", u8::from(enabled)) +} + fn vpn_host_config(enabled: bool) -> VpnHostConfigParts { if !enabled { return (None, None, None); @@ -1276,14 +1293,7 @@ pub async fn create_container( env_vars.push("MISSION_CONTROL_ENABLED=1".to_string()); } - // Drives the pia-vpn skill install in entrypoint.sh. Sent as 0 rather than - // omitted when off, because ~/.claude is a persisted volume: entrypoint has - // to be told to *remove* a skill left there by an earlier run with the - // toggle on, and an absent variable cannot say that. - env_vars.push(format!( - "VPN_SUPPORT_ENABLED={}", - u8::from(project.vpn_support_enabled) - )); + env_vars.push(vpn_env_var(project.vpn_support_enabled)); // Permission mode — read by triple-c-task-runner for scheduled (headless) // Claude Code runs. Interactive terminals get the flags directly instead. @@ -2799,6 +2809,17 @@ mod tests { assert_eq!(cap_add.unwrap(), vec!["NET_ADMIN"]); } + #[test] + fn the_vpn_skill_flag_is_emitted_either_way_never_omitted() { + // The whole removal path depends on this. If `false` ever became "emit + // nothing", a project that had the toggle on would keep the skill + // forever: the container recreates from a snapshot whose baked + // VPN_SUPPORT_ENABLED=1 would then go unchallenged, and entrypoint + // would reinstall a skill for a capability the container no longer has. + assert_eq!(vpn_env_var(true), "VPN_SUPPORT_ENABLED=1"); + assert_eq!(vpn_env_var(false), "VPN_SUPPORT_ENABLED=0"); + } + #[test] fn the_vpn_skill_flag_is_reserved_from_custom_env() { // entrypoint.sh installs and removes the pia-vpn skill from this diff --git a/app/src-tauri/src/docker/migration.rs b/app/src-tauri/src/docker/migration.rs index a08776a..8c39346 100644 --- a/app/src-tauri/src/docker/migration.rs +++ b/app/src-tauri/src/docker/migration.rs @@ -136,6 +136,7 @@ pub const FEATURE_PROBES: &[(&str, &str)] = &[ ("/usr/local/bin/triple-c-sso-refresh", "AWS SSO auto-refresh"), ("/opt/mission-control", "Mission Control (Flight Control)"), ("/usr/bin/wg", "VPN tooling for the VPN Support toggle (WireGuard)"), + ("/opt/triple-c-skills", "Bundled skills for the VPN Support toggle (PIA VPN)"), ]; /// Headroom demanded on Docker's storage backend on top of the measured diff --git a/container/Dockerfile b/container/Dockerfile index b980a3d..8b4cc41 100644 --- a/container/Dockerfile +++ b/container/Dockerfile @@ -401,7 +401,9 @@ COPY mission-control /opt/mission-control # no skill at all. Staged in /opt because ~/.claude is a volume mount: an image # copy underneath it would be masked from the project's first start onward. COPY skills /opt/triple-c-skills -RUN chmod +x /opt/triple-c-skills/*/*.sh +# `find`, not a `*/*.sh` glob: the glob fails the build the day a skill ships +# without a script, which is a legitimate thing for a skill to do. +RUN find /opt/triple-c-skills -name '*.sh' -exec chmod +x {} + COPY entrypoint.sh /usr/local/bin/entrypoint.sh RUN chmod +x /usr/local/bin/entrypoint.sh diff --git a/container/entrypoint.sh b/container/entrypoint.sh index 92435d7..f4e049d 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -347,18 +347,37 @@ fi # Copied on every start rather than only when absent, so a fix to a skill # reaches projects that already have the old copy. Local edits under these # directories do not survive — treat /opt/triple-c-skills as the source. +# +# The source lives in the *base image*, so a project whose container predates it +# recreates from its own snapshot and has no /opt/triple-c-skills to copy from. +# That case says so rather than returning silently: the toggle is on, the +# capability is there, and the skill simply never appears — which is impossible +# to work out from the outside. install_feature_skill() { - _name=$1 - _enabled=$2 - _dest="/home/claude/.claude/skills/$_name" + local _name="$1" + local _enabled="$2" + local _src="/opt/triple-c-skills/$1" + local _dest="/home/claude/.claude/skills/$1" + + # A blank name would make the disabled branch `rm -rf` the whole skills + # directory, Mission Control's included, under a persisted volume. + [ -n "$_name" ] || { echo "entrypoint: install_feature_skill called with no name"; return 1; } + if [ "$_enabled" = "1" ]; then - [ -d "/opt/triple-c-skills/$_name" ] || return 0 + if [ ! -d "$_src" ]; then + echo "entrypoint: $_name skill unavailable — this container's base image predates it; migrate the project to get it" + return 0 + fi mkdir -p /home/claude/.claude/skills + # Not just $_dest: when Mission Control is off nothing else creates the + # parent, so root would own it and `claude` could not add a skill there. + chown claude:claude /home/claude/.claude/skills rm -rf "$_dest" - cp -r "/opt/triple-c-skills/$_name" "$_dest" + cp -r "$_src" "$_dest" chown -R claude:claude "$_dest" echo "entrypoint: $_name skill installed to ~/.claude/skills/" - elif [ -d "$_dest" ]; then + elif [ -e "$_dest" ] || [ -L "$_dest" ]; then + # -e/-L rather than -d: a leftover *file* at that path must go too. rm -rf "$_dest" echo "entrypoint: $_name skill removed (feature disabled)" fi diff --git a/container/skills/pia-vpn/SKILL.md b/container/skills/pia-vpn/SKILL.md index d7554ad..52cffc0 100644 --- a/container/skills/pia-vpn/SKILL.md +++ b/container/skills/pia-vpn/SKILL.md @@ -65,6 +65,19 @@ PIA's own resolvers through the tunnel with `/32` routes that outrank the `10/8` exclusion. If you ever route traffic by hand, you owe both halves — the exclusions *and* a resolver reachable from wherever you pointed the default. +The failure has a quiet twin. Do only the first half — exclude the private +ranges, leave the resolver alone — and everything *works*, while every DNS +query travels outside the tunnel to your ISP. A VPN that leaks the full list of +what you looked up is worse than one that is visibly broken, so `up --full` +refuses to proceed if PIA does not hand back resolvers rather than carrying on +without them. + +The mechanism above is Docker Desktop's. On a user-defined Docker network the +resolver is `127.0.0.11`, which is loopback and never captured by a default +route — the trap still exists there (that resolver forwards upstream from +inside the container's namespace) but arrives by a different path. Check +`/etc/resolv.conf` rather than assuming which case you are in. + ## Trap 2: an IP-literal health check cannot see a dead resolver `curl https://1.1.1.1/cdn-cgi/trace` needs no DNS, so it returns a cheerful @@ -116,9 +129,20 @@ p1234567 your-password ``` -Set `PIA_CREDS` to use a different path. Treat the contents as secret: never -print the file, never echo the values, and never include them in a commit, a -log or a message. The script reads it directly and does not echo it. +Treat the contents as secret: never print the file, never echo the values, and +never include them in a commit, a log or a message. The script reads it directly +and does not echo it, and passes PIA's session token to `curl` on stdin rather +than in the argv, where `ps` would expose it to everything in the container. + +`PIA_CREDS` points somewhere else — but `sudo` resets the environment, so it +only takes effect **after** the word `sudo`: + +```bash +sudo PIA_CREDS=/path/to/creds ~/.claude/skills/pia-vpn/pia-wg.sh up # works +PIA_CREDS=/path/to/creds sudo ~/.claude/skills/pia-vpn/pia-wg.sh up # ignored +``` + +The second form fails silently back to the default path. Same for `PIA_REGION`. ## Regions @@ -156,10 +180,16 @@ true in test mode too, and means much less than it sounds like. ## Tearing down -`down` restores `resolv.conf` from its backup and removes exactly the routes -that were added, in reverse order, then deletes the interface. It is safe to -run when nothing is up. Confirm afterwards that the public address is back to -the container's own. +`down` restores `resolv.conf` from its backup (only if that backup still looks +like a resolver file — restoring a truncated one would leave the container with +no DNS at all), removes exactly the routes that were added, in reverse order, +deletes the interface, and shreds the WireGuard private key. It is safe to run +when nothing is up, and `up` runs it first so a repeat `up` cannot stack state. +Confirm afterwards that the public address is back to the container's own. + +The key deletion is not housekeeping: `/run` is in the container's writable +layer, so `docker commit` bakes whatever is there into the project's snapshot +image. A key left behind rides that image into every future container. ## What this deliberately does not do @@ -173,3 +203,11 @@ the container's own. cannot work headless: the daemon never accepts a client connection without the GUI, and `piactl --help` states that connecting requires it. If you find one installed, it is not a working alternative to this script. +- **Not `wg-quick`.** Its `Table=auto` full-tunnel mode routes by firewall mark + and needs `xt_CONNMARK` from the host kernel, which Docker Desktop for + Windows (WSL2) does not have and a container cannot load. This script adds + the routes with `ip route` directly, which works on every host. +- **IPv4 only.** The `0.0.0.0/1` + `128.0.0.0/1` pair covers v4. A container + with a global IPv6 address and a v6 default route would leak all v6 traffic + outside the tunnel; Triple-C's containers do not have one by default, but + check `ip -6 route show default` before relying on this where it matters. diff --git a/container/skills/pia-vpn/pia-wg.sh b/container/skills/pia-vpn/pia-wg.sh index 8e5c8c6..3b1ce4f 100644 --- a/container/skills/pia-vpn/pia-wg.sh +++ b/container/skills/pia-vpn/pia-wg.sh @@ -13,13 +13,20 @@ # # Requires the project's "VPN support" setting (Config -> Runtime) to be on. # +# Settings are read from the environment, but note that sudo resets it: they +# have to be passed *through* sudo, after the word `sudo`, not before it. +# +# sudo PIA_REGION=uk_london pia-wg.sh up --full # works +# PIA_REGION=uk_london sudo pia-wg.sh up --full # silently ignored +# # PIA_CREDS credentials file, two lines: username, then password -# (default ~/pia-creds; never echoed by this script) +# (default /home/claude/pia-creds; never echoed by this script) # PIA_REGION region id (default us_chicago). List them with: # curl -s https://serverlist.piaservers.net/vpninfo/servers/v6 \ # | head -1 | jq -r '.regions[].id' set -euo pipefail +# Not ~/pia-creds: under sudo, HOME is /root. CREDS=${PIA_CREDS:-/home/claude/pia-creds} REGION=${PIA_REGION:-us_chicago} IFACE=pia0 @@ -36,11 +43,28 @@ PRIVATE_NETS="10.0.0.0/8 172.16.0.0/12 192.168.0.0/16 169.254.0.0/16" # source lines without the indentation ending up in the output. die() { echo "pia-wg: $*" >&2; exit 1; } +# `x=$(cmd)` is a plain assignment, so `set -e` kills the script on a non-zero +# cmd *before* any `[ -z "$x" ] || die` line can run. Every capture below +# therefore goes through `run`; without it a wrong password exits 22 with no +# output at all, which is the most likely way this is used wrongly and was the +# least explained. +# +# It takes a description rather than reporting the command it ran: one of these +# invocations carries the account password in `-u`, and an error message is +# exactly the wrong place for that to surface. +run() { local what=$1; shift; "$@" || die "$what (exit $?)"; } + preflight() { [ "$(id -u)" = 0 ] || die "run with sudo" # CAP_NET_ADMIN is bit 12. Checking it by name gives a usable error; without # it the first `ip` call fails with a bare "Operation not permitted" that # points nowhere near the setting that actually needs changing. + # + # Deliberately NOT checking /dev/net/tun: kernel WireGuard is a netlink + # interface and does not use it (verified -- `ip link add type wireguard` + # succeeds with NET_ADMIN and no tun device). It is OpenVPN and userspace + # wireguard-go that need it. The real kernel dependency here is the + # `wireguard` module, which `ip link add` below reports on directly. local caps caps=$(awk '/^CapEff:/{print $2}' /proc/self/status) if [ $(( 0x$caps & 0x1000 )) -eq 0 ]; then @@ -49,51 +73,88 @@ preflight() { "again. That recreates the container; the home and .claude volumes" \ "are preserved, so nothing in them is lost." fi - [ -e /dev/net/tun ] || \ - die "/dev/net/tun is missing." \ - "Same fix: turn on \"VPN support\" in Config -> Runtime. If it is" \ - "already on, the Docker host's kernel is missing the tun module." - command -v wg >/dev/null || die "wireguard-tools is not installed." + command -v wg >/dev/null || \ + die "wireguard-tools is not installed." \ + "If this project's container was built from an older base image," \ + "migrate it onto the current one -- that is what ships \`wg\`." [ -r "$CREDS" ] || \ die "no credentials at $CREDS." \ "Two lines are expected: username, then password." \ - "Set PIA_CREDS to read them from somewhere else." + "Set PIA_CREDS (after the word \`sudo\`) to read them elsewhere." } -# Record every route we add so teardown removes exactly those and nothing else. -add_route() { ip route add $1 2>/dev/null && echo "$1" >> "$STATE/routes" || true; } +# Routes that must work. A silent failure here is the worst state this script +# can reach: the two half-routes need no gateway and would succeed, so the +# tunnel captures everything while the exclusions that keep DNS and the Docker +# host reachable are quietly missing -- and `status` still says "full tunnel". +add_route() { + ip route add "$@" || die "could not add route '$*'. Run 'down' to undo the partial setup." + printf '%s\n' "$*" >> "$STATE/routes" +} up() { + case "${1:-}" in + ""|--full) ;; + *) die "unknown option '$1' (expected --full or nothing)." \ + "Refusing rather than silently giving you a test route." ;; + esac preflight + + # Always start from a known state. Without this a second `up` overwrites the + # saved resolv.conf with PIA's own resolvers, so the later `down` "restores" + # those and leaves the container with no working DNS and no way back. + down >/dev/null 2>&1 || true + mkdir -p "$STATE"; cd "$STATE" - [ -f ca.rsa.4096.crt ] || curl -sf -m 20 -o ca.rsa.4096.crt \ - https://raw.githubusercontent.com/pia-foss/manual-connections/master/ca.rsa.4096.crt \ - || die "could not fetch PIA's CA certificate" + + # `curl -o` creates the file before it knows the request failed, so a plain + # `[ -f ]` cache check can pin a truncated cert forever -- and /run rides the + # snapshot, so "forever" outlives the container. Fetch to a temp name and + # rename only on success. + if [ ! -s ca.rsa.4096.crt ]; then + run "could not download PIA's CA certificate" \ + curl -sf -m 20 -o ca.crt.part \ + https://raw.githubusercontent.com/pia-foss/manual-connections/master/ca.rsa.4096.crt + [ -s ca.crt.part ] || die "PIA's CA certificate downloaded empty" + mv ca.crt.part ca.rsa.4096.crt + fi local u p tok srv sip scn priv pub resp ep gw dns u=$(sed -n 1p "$CREDS"); p=$(sed -n 2p "$CREDS") - tok=$(curl -sf -m 25 -u "$u:$p" \ - https://www.privateinternetaccess.com/gtoken/generateToken | jq -r .token) - [ -n "$tok" ] && [ "$tok" != null ] || die "PIA authentication failed - check $CREDS" + [ -n "$u" ] && [ -n "$p" ] || die "$CREDS needs two lines: username, then password" - curl -sf -m 30 https://serverlist.piaservers.net/vpninfo/servers/v6 | head -1 > servers.json + tok=$(run "PIA rejected the credentials in $CREDS, or could not be reached" \ + curl -sf -m 25 -u "$u:$p" \ + https://www.privateinternetaccess.com/gtoken/generateToken | jq -r .token) + [ -n "$tok" ] && [ "$tok" != null ] || die "PIA returned no token - check the credentials in $CREDS" + + run "could not fetch PIA's server list" \ + curl -sf -m 30 https://serverlist.piaservers.net/vpninfo/servers/v6 \ + | head -1 > servers.json srv=$(jq -r --arg r "$REGION" '.regions[] | select(.id==$r) | .servers.wg[0]' servers.json) sip=$(echo "$srv" | jq -r .ip); scn=$(echo "$srv" | jq -r .cn) - [ -n "$sip" ] && [ "$sip" != null ] || die "no WireGuard server for region $REGION" + [ -n "$sip" ] && [ "$sip" != null ] || die "no WireGuard server for region '$REGION'" - priv=$(wg genkey); pub=$(echo "$priv" | wg pubkey) - printf '%s' "$priv" > wg.priv; chmod 600 wg.priv + # umask, not a later chmod: the file is created under the inherited 0022 + # otherwise, so the key is world-readable for the moment in between. + ( umask 077; priv=$(wg genkey); printf '%s' "$priv" > wg.priv ) + priv=$(cat wg.priv); pub=$(printf '%s' "$priv" | wg pubkey) + # The token goes in on stdin as a curl config rather than in the argv, where + # `ps` and /proc/*/cmdline expose it to every process in the container -- + # verified. It is a ~24h bearer credential for the whole PIA account. # PIA pins its certificate to the server's common name, which is why this # connects by CN and lets --connect-to point that name at the real address. - resp=$(curl -sf -m 25 -G --connect-to "$scn::$sip:" --cacert ca.rsa.4096.crt \ - --data-urlencode "pt=$tok" --data-urlencode "pubkey=$pub" \ - "https://$scn:1337/addKey") + resp=$(printf -- '--data-urlencode "pt=%s"\n--data-urlencode "pubkey=%s"\n' "$tok" "$pub" \ + | run "could not register the key with $scn" \ + curl -sf -m 25 -G -K - --connect-to "$scn::$sip:" \ + --cacert ca.rsa.4096.crt "https://$scn:1337/addKey") [ "$(echo "$resp" | jq -r .status)" = OK ] || die "key registration failed: $resp" : > "$STATE/routes" - ip link del "$IFACE" 2>/dev/null || true - ip link add "$IFACE" type wireguard + ip link add "$IFACE" type wireguard 2>/dev/null || \ + die "could not create a WireGuard interface." \ + "The Docker host's kernel has no 'wireguard' module." wg set "$IFACE" private-key wg.priv \ peer "$(echo "$resp" | jq -r .server_key)" \ endpoint "$(echo "$resp" | jq -r .server_ip):$(echo "$resp" | jq -r .server_port)" \ @@ -102,36 +163,41 @@ up() { ip link set "$IFACE" up if [ "${1:-}" = "--full" ]; then + ep=$(echo "$resp" | jq -r .server_ip) + gw=$(ip route show default | awk '{print $3; exit}') + # `default dev eth0` with no `via` yields the literal "eth0" here, which + # would make every exclusion below a malformed no-op. + [[ $gw =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]] || \ + die "no usable default gateway to pin the tunnel against (got '${gw:-none}')" + + # PIA's resolvers are required in --full. Without them the 10/8 exclusion + # below is already in place, so every lookup would go to the container's + # own resolver *outside* the tunnel -- a full tunnel leaking all its DNS, + # reported by `status` as perfectly healthy. + dns=$(echo "$resp" | jq -r '.dns_servers[]? // empty' | head -2) + [ -n "$dns" ] || die "PIA returned no DNS servers; refusing a full tunnel that would leak every lookup" + # Pin the endpoint to the pre-existing gateway first, so the tunnel's own # packets do not try to route through the tunnel. Then beat the default # route with two half-routes rather than replacing it -- nothing to restore # on teardown, and the container keeps working if this script dies midway. - ep=$(echo "$resp" | jq -r .server_ip) - gw=$(ip route show default | awk '{print $3; exit}') - add_route "$ep/32 via $gw" - add_route "0.0.0.0/1 dev $IFACE" - add_route "128.0.0.0/1 dev $IFACE" + add_route "$ep/32" via "$gw" + add_route 0.0.0.0/1 dev "$IFACE" + add_route 128.0.0.0/1 dev "$IFACE" # Keep container, host and LAN traffic off the tunnel. Longer prefixes than # the two halves above, so these win. - for n in $PRIVATE_NETS; do add_route "$n via $gw"; done + for n in $PRIVATE_NETS; do add_route "$n" via "$gw"; done - # PIA's resolver lives inside 10/8, so pin it back through the tunnel with a - # /32 -- longer still, so it beats the 10.0.0.0/8 exclusion just added. - # Using PIA's resolver rather than the container's keeps DNS from leaking, - # and the container's own resolver is unreachable from inside the tunnel. - dns=$(echo "$resp" | jq -r '.dns_servers[]? // empty' | head -2) - if [ -n "$dns" ]; then - cp /etc/resolv.conf "$STATE/resolv.conf.bak" - for d in $dns; do add_route "$d/32 dev $IFACE"; done - # resolv.conf is a bind mount: write through it, never replace it. - for d in $dns; do echo "nameserver $d"; done > /etc/resolv.conf - else - echo "pia-wg: warning - PIA returned no DNS servers; leaving resolv.conf alone" >&2 - fi + # PIA's resolvers live inside 10/8, so pin them back through the tunnel with + # /32s -- longer still, so they beat the exclusion just added. + cp /etc/resolv.conf "$STATE/resolv.conf.bak" + for d in $dns; do add_route "$d/32" dev "$IFACE"; done + # resolv.conf is a bind mount: write through it, never replace it. + for d in $dns; do echo "nameserver $d"; done > /etc/resolv.conf echo "full tunnel: public traffic exits via PIA; private ranges stay local" else - add_route "1.1.1.1/32 dev $IFACE" + add_route 1.1.1.1/32 dev "$IFACE" echo "test route only: 1.1.1.1 goes via PIA, everything else unchanged" fi @@ -141,8 +207,15 @@ up() { down() { [ "$(id -u)" = 0 ] || die "run with sudo" + # Only restore something that actually looks like a resolver file. Restoring + # an empty or truncated backup leaves the container with no DNS at all, which + # is worse than leaving the current one alone. if [ -f "$STATE/resolv.conf.bak" ]; then - cat "$STATE/resolv.conf.bak" > /etc/resolv.conf + if grep -q '^nameserver' "$STATE/resolv.conf.bak" 2>/dev/null; then + cat "$STATE/resolv.conf.bak" > /etc/resolv.conf + else + echo "pia-wg: warning - saved resolv.conf looks empty; leaving the current one alone" >&2 + fi rm -f "$STATE/resolv.conf.bak" fi if [ -f "$STATE/routes" ]; then @@ -153,6 +226,10 @@ down() { rm -f "$STATE/routes" fi ip link del "$IFACE" 2>/dev/null || true + # /run is in the writable layer and `docker commit` bakes it into the + # project's snapshot image, so a key left here rides that image into every + # future container. Verified: a snapshot already carried one. + rm -f "$STATE/wg.priv" echo "tunnel down" } @@ -197,5 +274,5 @@ case "${1:-}" in up) shift; up "${1:-}" ;; down) down ;; status) status ;; - *) sed -n '2,20p' "$0" | sed 's/^# \{0,1\}//'; exit 1 ;; + *) sed -n '2,27p' "$0" | sed 's/^# \{0,1\}//'; exit 1 ;; esac -- 2.52.0 From 5b96ad48233db976e2d44f0e410741bf43f3c52b Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 17 Aug 2026 16:29:30 -0700 Subject: [PATCH 4/5] Stop a failed `up` from tearing down a working tunnel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit moved `down` to the top of `up` to fix a resolv.conf idempotency bug, and in doing so put it *before* every network fetch that can fail. Re-review caught it and I reproduced it: with a tunnel up, an `up` that fails on bad credentials left `pia0` gone and traffic silently back on the real address, while the error talked only about credentials. A privacy regression introduced by a correctness fix. `down` now runs after the token, server list and key registration have all succeeded — nothing above that line touches the network stack — and still clears the stale backup it was added for. Everything after it is covered by a rollback. Note this is an EXIT trap with a flag, not `trap ... ERR`: my first attempt used ERR and did not fire at all, because ERR is not inherited by shell functions without `set -E`, so a failure inside add_route missed it, and `die` exits explicitly, which is not an error. Verified by forcing a route collision — the tunnel is torn down and DNS is intact, where before the fix it was left half-configured with DNS dead. Also from the review: - **The private key lived on disk for the whole life of the tunnel.** `commit_container_snapshot` blanks env vars, never files, and nothing tears the tunnel down before a recreate or migrate — so the `down`-time cleanup never covered the path that put a key in a snapshot in the first place. It is now deleted the moment `wg set` has read it; the kernel keeps its own copy, verified by checking the interface still works afterwards. - **`up` claimed success without a handshake.** An unreachable peer still routes — into a black hole — so `up --full` could exit 0 having pointed all traffic and resolv.conf at a peer that never answered, with `status` printing "mode: full tunnel". Now polls for a handshake and rolls back if none arrives. - **`status` needed root and did not check.** `wg show` fails unprivileged and was swallowed, so an unprivileged run printed "no tunnel up" and then "mode: full tunnel" in the same breath. An agent reading the first line would re-run `up` — which, before the fix above, destroyed the tunnel it failed to see. - **The killswitch bullet was false.** It said `iptables` is not in the image; it is, so a killswitch is buildable. It stays unbuilt because it would cut Claude Code's own API traffic — an honest reason, unlike the previous one. - `install_feature_skill` rejects path-traversal names, not just blank ones — verified `../skills` would have deleted the whole skills directory including Mission Control's — and reports `mkdir`/`cp` failures instead of printing a success line regardless. - The usage text ended by printing `set -euo pipefail`, off by one line. - HOW-TO-USE claimed "the container says so on start". entrypoint prints to PID 1's stdout, which no terminal or UI surfaces — `docker logs` appears nowhere in the repo. Now points at the migration pre-flight, which does. Co-Authored-By: Claude Opus 5 (1M context) --- container/entrypoint.sh | 20 ++++++++--- container/skills/pia-vpn/SKILL.md | 34 ++++++++++++------ container/skills/pia-vpn/pia-wg.sh | 56 +++++++++++++++++++++++++----- 3 files changed, 86 insertions(+), 24 deletions(-) diff --git a/container/entrypoint.sh b/container/entrypoint.sh index f4e049d..2bb9d95 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -359,21 +359,31 @@ install_feature_skill() { local _src="/opt/triple-c-skills/$1" local _dest="/home/claude/.claude/skills/$1" - # A blank name would make the disabled branch `rm -rf` the whole skills - # directory, Mission Control's included, under a persisted volume. - [ -n "$_name" ] || { echo "entrypoint: install_feature_skill called with no name"; return 1; } + # Reject anything that is not a plain directory name. The disabled branch + # `rm -rf`s $_dest under a *persisted volume*, so a blank name would take the + # whole skills directory (Mission Control's included) and `../x` would escape + # it entirely. Only the literal `pia-vpn` is passed today; this is so that + # stays true. + case "$_name" in + ''|*/*|.*) echo "entrypoint: install_feature_skill: bad skill name '$_name'"; return 1 ;; + esac if [ "$_enabled" = "1" ]; then if [ ! -d "$_src" ]; then echo "entrypoint: $_name skill unavailable — this container's base image predates it; migrate the project to get it" return 0 fi - mkdir -p /home/claude/.claude/skills + # Checked, not assumed: with no `set -e` in this script every step here + # can fail (full volume, read-only mount, a file where the directory + # should be) and the success line would still print. + mkdir -p /home/claude/.claude/skills || { + echo "entrypoint: $_name skill install FAILED (cannot create ~/.claude/skills)"; return 1; } # Not just $_dest: when Mission Control is off nothing else creates the # parent, so root would own it and `claude` could not add a skill there. chown claude:claude /home/claude/.claude/skills rm -rf "$_dest" - cp -r "$_src" "$_dest" + cp -r "$_src" "$_dest" || { + echo "entrypoint: $_name skill install FAILED (copy from $_src)"; return 1; } chown -R claude:claude "$_dest" echo "entrypoint: $_name skill installed to ~/.claude/skills/" elif [ -e "$_dest" ] || [ -L "$_dest" ]; then diff --git a/container/skills/pia-vpn/SKILL.md b/container/skills/pia-vpn/SKILL.md index 52cffc0..7dea400 100644 --- a/container/skills/pia-vpn/SKILL.md +++ b/container/skills/pia-vpn/SKILL.md @@ -183,22 +183,34 @@ true in test mode too, and means much less than it sounds like. `down` restores `resolv.conf` from its backup (only if that backup still looks like a resolver file — restoring a truncated one would leave the container with no DNS at all), removes exactly the routes that were added, in reverse order, -deletes the interface, and shreds the WireGuard private key. It is safe to run -when nothing is up, and `up` runs it first so a repeat `up` cannot stack state. -Confirm afterwards that the public address is back to the container's own. +and deletes the interface. It is safe to run when nothing is up. Confirm +afterwards that the public address is back to the container's own. -The key deletion is not housekeeping: `/run` is in the container's writable -layer, so `docker commit` bakes whatever is there into the project's snapshot -image. A key left behind rides that image into every future container. +`up` calls it too, but only *after* every network fetch has succeeded, so a +failed `up` leaves an existing tunnel alone rather than tearing it down to +report a bad password. From that point on a rollback is armed: if any step of +the setup fails, the tunnel is torn down rather than left half-configured. + +The private key is deleted earlier still — the moment `wg set` has read it, +while the tunnel is being built. That is not housekeeping: `/run` is in the +container's writable layer, and recreating or migrating the project runs +`docker commit` over it *without* tearing the tunnel down first. A key that +lived for the tunnel's lifetime would be baked into the snapshot image and +copied forward from then on. The kernel keeps its own copy, so nothing is lost. ## What this deliberately does not do -- **No killswitch.** Blocking non-tunnel egress needs `iptables`, which is not - in the image, and would cut Claude Code's API traffic whenever the tunnel is - down. If the user needs guaranteed egress rather than convenient egress, say - so plainly rather than improvising one — it is a real design decision. +- **No killswitch.** `iptables` *is* in the image, so one is buildable — this + is a deliberate omission, not a missing dependency. Blocking non-tunnel egress + cuts Claude Code's own API traffic the moment the tunnel drops, which ends the + session that would otherwise fix it. If the user needs guaranteed egress + rather than convenient egress, say so plainly and let them decide, rather than + improvising one. - **No autostart.** There is no service manager in the container and Triple-C - has no start hook, so nothing can re-establish the tunnel automatically. + has no start hook, so nothing re-establishes the tunnel on its own. `cron` is + in the image and `triple-c-scheduler` runs on it, so a scheduled reconnect is + possible if the user wants one — it is just not set up, and a tunnel that + reconnects unattended deserves an explicit decision. - **Not PIA's desktop client.** `pia-daemon` and `piactl` are installable but cannot work headless: the daemon never accepts a client connection without the GUI, and `piactl --help` states that connecting requires it. If you find diff --git a/container/skills/pia-vpn/pia-wg.sh b/container/skills/pia-vpn/pia-wg.sh index 3b1ce4f..50bea37 100644 --- a/container/skills/pia-vpn/pia-wg.sh +++ b/container/skills/pia-vpn/pia-wg.sh @@ -27,6 +27,9 @@ set -euo pipefail # Not ~/pia-creds: under sudo, HOME is /root. +# Read by up()'s EXIT trap, which runs after the function's locals are gone. +SETUP_OK=0 + CREDS=${PIA_CREDS:-/home/claude/pia-creds} REGION=${PIA_REGION:-us_chicago} IFACE=pia0 @@ -88,7 +91,7 @@ preflight() { # tunnel captures everything while the exclusions that keep DNS and the Docker # host reachable are quietly missing -- and `status` still says "full tunnel". add_route() { - ip route add "$@" || die "could not add route '$*'. Run 'down' to undo the partial setup." + ip route add "$@" || die "could not add route '$*'" printf '%s\n' "$*" >> "$STATE/routes" } @@ -100,11 +103,6 @@ up() { esac preflight - # Always start from a known state. Without this a second `up` overwrites the - # saved resolv.conf with PIA's own resolvers, so the later `down` "restores" - # those and leaves the container with no working DNS and no way back. - down >/dev/null 2>&1 || true - mkdir -p "$STATE"; cd "$STATE" # `curl -o` creates the file before it knows the request failed, so a plain @@ -135,6 +133,27 @@ up() { sip=$(echo "$srv" | jq -r .ip); scn=$(echo "$srv" | jq -r .cn) [ -n "$sip" ] && [ "$sip" != null ] || die "no WireGuard server for region '$REGION'" + # Only now tear down any previous tunnel. Doing it up front (as an earlier + # version did) meant a failed token fetch or an unreachable server list took + # a *working* tunnel down with it and silently reverted the container to its + # real address, while the error talked about credentials. Everything above + # this line can fail; nothing above it has touched the network stack. + # + # It also still does the job it was added for: clearing a stale resolv.conf + # backup so a second `up` cannot save PIA's own resolvers over the real ones. + down >/dev/null 2>&1 || true + + # From here on the network stack is being modified, so any failure has to put + # it back rather than exit half-configured. `down` is idempotent and restores + # routes and resolv.conf exactly. + # + # EXIT rather than ERR, and a flag rather than the trap's own exit status: an + # ERR trap is not inherited by shell functions without `set -E`, so a failure + # inside add_route would not fire it, and `die` exits explicitly, which is not + # an error and would not fire it either. EXIT catches both. + SETUP_OK=0 + trap '[ "$SETUP_OK" = 1 ] || { echo "pia-wg: setup failed - rolling back" >&2; down >/dev/null 2>&1; }' EXIT + # umask, not a later chmod: the file is created under the inherited 0022 # otherwise, so the key is world-readable for the moment in between. ( umask 077; priv=$(wg genkey); printf '%s' "$priv" > wg.priv ) @@ -159,6 +178,12 @@ up() { peer "$(echo "$resp" | jq -r .server_key)" \ endpoint "$(echo "$resp" | jq -r .server_ip):$(echo "$resp" | jq -r .server_port)" \ allowed-ips 0.0.0.0/0 persistent-keepalive 25 + # The kernel holds the key from here, so the file has no reason to outlive + # this line -- and every reason not to: /run is in the writable layer, and a + # recreate or migrate runs `docker commit` over it without tearing the tunnel + # down first, baking the key into the project's snapshot image. `down` also + # removes it, for the case where `up` never got this far. + rm -f wg.priv ip addr add "$(echo "$resp" | jq -r .peer_ip)/32" dev "$IFACE" ip link set "$IFACE" up @@ -201,7 +226,18 @@ up() { echo "test route only: 1.1.1.1 goes via PIA, everything else unchanged" fi - sleep 2 + # A tunnel with no handshake still routes -- into a black hole. Without this + # `up --full` would exit 0 having pointed all traffic *and* resolv.conf at a + # peer that never answered, and `status` would print "mode: full tunnel". + local waited=0 + until [ "$(wg show "$IFACE" latest-handshakes | awk '{print $2; exit}')" != 0 ]; do + waited=$((waited + 1)) + [ "$waited" -lt 20 ] || die "no handshake from $REGION after 10s - rolled back" + sleep 0.5 + done + + SETUP_OK=1 + trap - EXIT status } @@ -242,6 +278,10 @@ TRACE_DIRECT=https://1.0.0.1/cdn-cgi/trace exit_ip() { curl -s -m 20 "$1" | sed -n 's/^ip=//p'; } status() { + # `wg show` needs root; `ip route`/`ip link` do not. Without this guard an + # unprivileged run prints "no tunnel up" and then "mode: full tunnel" in the + # same breath, and an agent reading the first line re-runs `up`. + [ "$(id -u)" = 0 ] || die "run with sudo" wg show "$IFACE" 2>/dev/null | grep -E "latest handshake|transfer" || echo "no tunnel up" # Resolve a name, not an IP literal. A curl to 1.1.1.1 succeeds while DNS is @@ -274,5 +314,5 @@ case "${1:-}" in up) shift; up "${1:-}" ;; down) down ;; status) status ;; - *) sed -n '2,27p' "$0" | sed 's/^# \{0,1\}//'; exit 1 ;; + *) sed -n '2,26p' "$0" | sed 's/^# \{0,1\}//'; exit 1 ;; esac -- 2.52.0 From 48d0c3249a16184f70ee3ad63018b708b6f60064 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Mon, 17 Aug 2026 17:15:52 -0700 Subject: [PATCH 5/5] Fix two bugs in last round's fixes, and stop --full hiding the Docker host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 found defects in code written an hour earlier. Both reproduced. **The handshake poll accepted empty output as a completed handshake.** `[ "$(… | awk '{print $2}')" != 0 ]` is *true* when `wg show` prints nothing — which it does when the interface has no peer, and when the interface is gone (that message goes to stderr). `until` suspends `set -e` and `pipefail`, so nothing else caught it. The poll added last round to make "success without a tunnel" impossible produced exactly that. Now requires a number greater than zero, and waits 20s rather than 10 so a slow link is not rolled back needlessly. **`down` still sat above the key registration.** Last round moved it below the token and server-list fetches but not below `addKey`, which is the most failure-prone of the three — one gateway, by CN, pinned certificate. So a refused registration still tore down a working tunnel. It now runs after the last fetch; the key is generated before but written after, since `down` deletes it. SKILL.md said "after every network fetch has succeeded", which was false; corrected. **`up --full` made `host.docker.internal` unresolvable — and `status` said DNS was fine.** That name is answered only by the resolver being replaced; it is not in `/etc/hosts`. `gateway.rs` hands it to every container for the LiteLLM gateway, and Ollama and custom endpoints default to it, so an agent running `up --full` silently removed the project's model backend. The route was already excluded; only the name was lost. Now resolved with the old resolver and pinned into `/etc/hosts` before the swap, restored on teardown, and `status` probes it — PIA answers public names happily, which is precisely why probing only `api.anthropic.com` reported "ok". Documented as Trap 4. **The rollback could abort halfway.** The trap's `{ … }` is not exempt from `set -e`, and `down`'s `cat`/`tac`/`rm` had no `|| true` — so one failure left the interface up with all traffic captured, after printing "rolling back". `down` now runs under `set +e`, the trap tolerates its failure, and the interface is deleted *first*, since that removes every route pointing at it. **The account password had a real argv window.** curl does blank `-u`, but only once running: sampling /proc//cmdline caught the plaintext in 2 of 400 tries, between exec and the overwrite. Small, but it is the permanent password and the token already had the fix. Moved onto the same stdin config — 0 of 400. Review reported this as a 25-second exposure; that was a wrapper's argv, not curl's. entrypoint: the skill install stages into `$_dest.new` and swaps, so a failed copy leaves the previous copy intact instead of a truncated SKILL.md and no script, root-owned, on a persisted volume. Verified against a size-limited filesystem. Co-Authored-By: Claude Opus 5 (1M context) --- container/entrypoint.sh | 14 +++- container/skills/pia-vpn/SKILL.md | 27 +++++-- container/skills/pia-vpn/pia-wg.sh | 112 ++++++++++++++++++++--------- 3 files changed, 109 insertions(+), 44 deletions(-) diff --git a/container/entrypoint.sh b/container/entrypoint.sh index 2bb9d95..2e859b2 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -381,10 +381,18 @@ install_feature_skill() { # Not just $_dest: when Mission Control is off nothing else creates the # parent, so root would own it and `claude` could not add a skill there. chown claude:claude /home/claude/.claude/skills + # Stage then swap. Copying over the live path meant a failure (full + # volume, read-only mount) left a truncated SKILL.md and no script + # behind, root-owned, on a persisted volume — which Claude Code then + # discovers and loads. + rm -rf "$_dest.new" + cp -r "$_src" "$_dest.new" || { + rm -rf "$_dest.new" + echo "entrypoint: $_name skill install FAILED (copy from $_src); previous copy left intact" + return 1; } + chown -R claude:claude "$_dest.new" rm -rf "$_dest" - cp -r "$_src" "$_dest" || { - echo "entrypoint: $_name skill install FAILED (copy from $_src)"; return 1; } - chown -R claude:claude "$_dest" + mv "$_dest.new" "$_dest" echo "entrypoint: $_name skill installed to ~/.claude/skills/" elif [ -e "$_dest" ] || [ -L "$_dest" ]; then # -e/-L rather than -d: a leftover *file* at that path must go too. diff --git a/container/skills/pia-vpn/SKILL.md b/container/skills/pia-vpn/SKILL.md index 7dea400..096005b 100644 --- a/container/skills/pia-vpn/SKILL.md +++ b/container/skills/pia-vpn/SKILL.md @@ -9,7 +9,7 @@ Bring this container's traffic out through Private Internet Access over WireGuard, using the API PIA documents for headless use. Run `sudo ~/.claude/skills/pia-vpn/pia-wg.sh` with `up`, `up --full`, `down` or -`status`. Read the rest of this page before the first `up --full` — three of the +`status`. Read the rest of this page before the first `up --full` — four of the behaviours below are actively misleading if you meet them without warning, and each one presents as "the VPN is fine" or "Claude is broken" rather than as what it is. @@ -107,7 +107,21 @@ mode: test route only (1.1.1.1 through the tunnel, nothing else) Two different addresses there is correct and expected in test mode. If you want the second line to change, you want `up --full`. -## Trap 4: no tunnel survives a restart, and it fails open +## Trap 4: a full tunnel hides the Docker host unless the name is pinned + +`host.docker.internal` is answered *only* by the resolver that `up --full` +replaces — it is not in `/etc/hosts`. Triple-C hands that name to the container +for the LiteLLM gateway, and host-side Ollama and custom endpoints default to +it, so losing the name takes the project's model backend down with it. + +The nasty part is what a naive check reports. PIA's resolvers answer public +names perfectly well, so a probe of `api.anthropic.com` says everything is fine +while the Docker host has vanished. `pia-wg.sh` pins the address into +`/etc/hosts` before swapping the resolver and restores the file on teardown, and +`status` probes both names — but if you ever rewrite `resolv.conf` by hand, this +is the one that will not announce itself. + +## Trap 5: no tunnel survives a restart, and it fails open The network namespace is rebuilt every time the container starts, and nothing inside reconnects anything. After a stop/start, Reset or any config change that @@ -186,10 +200,11 @@ no DNS at all), removes exactly the routes that were added, in reverse order, and deletes the interface. It is safe to run when nothing is up. Confirm afterwards that the public address is back to the container's own. -`up` calls it too, but only *after* every network fetch has succeeded, so a -failed `up` leaves an existing tunnel alone rather than tearing it down to -report a bad password. From that point on a rollback is armed: if any step of -the setup fails, the tunnel is torn down rather than left half-configured. +`up` calls it too, but only after the last network fetch — the key registration +— has succeeded, so a failed `up` leaves an existing tunnel alone rather than +tearing it down to report a bad password or an unreachable gateway. From that +point on a rollback is armed: if any step of the setup fails, the tunnel is torn +down rather than left half-configured. The private key is deleted earlier still — the moment `wg set` has read it, while the tunnel is being built. That is not housekeeping: `/run` is in the diff --git a/container/skills/pia-vpn/pia-wg.sh b/container/skills/pia-vpn/pia-wg.sh index 50bea37..8a320ee 100644 --- a/container/skills/pia-vpn/pia-wg.sh +++ b/container/skills/pia-vpn/pia-wg.sh @@ -121,9 +121,15 @@ up() { u=$(sed -n 1p "$CREDS"); p=$(sed -n 2p "$CREDS") [ -n "$u" ] && [ -n "$p" ] || die "$CREDS needs two lines: username, then password" - tok=$(run "PIA rejected the credentials in $CREDS, or could not be reached" \ - curl -sf -m 25 -u "$u:$p" \ - https://www.privateinternetaccess.com/gtoken/generateToken | jq -r .token) + # Via stdin, not `-u`. curl does blank the password in its own argv, but only + # once it is running: sampling /proc//cmdline in a tight loop caught the + # plaintext in 3 of 200 tries, in the window between exec and the overwrite. + # Small, but this is the permanent account password, and the mechanism to + # avoid it entirely is already here for the token. + tok=$(printf -- '--user "%s:%s"\n' "$u" "$p" \ + | run "PIA rejected the credentials in $CREDS, or could not be reached" \ + curl -sf -m 25 -K - \ + https://www.privateinternetaccess.com/gtoken/generateToken | jq -r .token) [ -n "$tok" ] && [ "$tok" != null ] || die "PIA returned no token - check the credentials in $CREDS" run "could not fetch PIA's server list" \ @@ -133,31 +139,9 @@ up() { sip=$(echo "$srv" | jq -r .ip); scn=$(echo "$srv" | jq -r .cn) [ -n "$sip" ] && [ "$sip" != null ] || die "no WireGuard server for region '$REGION'" - # Only now tear down any previous tunnel. Doing it up front (as an earlier - # version did) meant a failed token fetch or an unreachable server list took - # a *working* tunnel down with it and silently reverted the container to its - # real address, while the error talked about credentials. Everything above - # this line can fail; nothing above it has touched the network stack. - # - # It also still does the job it was added for: clearing a stale resolv.conf - # backup so a second `up` cannot save PIA's own resolvers over the real ones. - down >/dev/null 2>&1 || true - - # From here on the network stack is being modified, so any failure has to put - # it back rather than exit half-configured. `down` is idempotent and restores - # routes and resolv.conf exactly. - # - # EXIT rather than ERR, and a flag rather than the trap's own exit status: an - # ERR trap is not inherited by shell functions without `set -E`, so a failure - # inside add_route would not fire it, and `die` exits explicitly, which is not - # an error and would not fire it either. EXIT catches both. - SETUP_OK=0 - trap '[ "$SETUP_OK" = 1 ] || { echo "pia-wg: setup failed - rolling back" >&2; down >/dev/null 2>&1; }' EXIT - - # umask, not a later chmod: the file is created under the inherited 0022 - # otherwise, so the key is world-readable for the moment in between. - ( umask 077; priv=$(wg genkey); printf '%s' "$priv" > wg.priv ) - priv=$(cat wg.priv); pub=$(printf '%s' "$priv" | wg pubkey) + # The key is generated but NOT written yet -- `down` below deletes wg.priv, and + # the teardown has to come after every fetch that can fail. + priv=$(wg genkey); pub=$(printf '%s' "$priv" | wg pubkey) # The token goes in on stdin as a curl config rather than in the argv, where # `ps` and /proc/*/cmdline expose it to every process in the container -- @@ -170,6 +154,32 @@ up() { --cacert ca.rsa.4096.crt "https://$scn:1337/addKey") [ "$(echo "$resp" | jq -r .status)" = OK ] || die "key registration failed: $resp" + # Only now tear down any previous tunnel. Every network call above this line + # can fail, and an earlier version tore down first -- so a failed token fetch, + # an unreachable server list, or a refused key registration took a *working* + # tunnel with it and silently reverted the container to its real address while + # the error talked about credentials. Nothing above this line has touched the + # network stack. addKey is the most failure-prone of the three: it reaches one + # individual gateway by CN with a pinned certificate. + # + # It also still does the job it was added for: clearing a stale resolv.conf + # backup so a second `up` cannot save PIA's own resolvers over the real ones. + down >/dev/null 2>&1 || true + + # From here on the network stack is being modified, so any failure has to put + # it back rather than exit half-configured. + # + # EXIT rather than ERR, and a flag rather than the trap's own exit status: an + # ERR trap is not inherited by shell functions without `set -E`, so a failure + # inside add_route would not fire it, and `die` exits explicitly, which is not + # an error and would not fire it either. EXIT catches both. + SETUP_OK=0 + trap '[ "$SETUP_OK" = 1 ] || { echo "pia-wg: setup failed - rolling back" >&2; down >/dev/null 2>&1 || true; }' EXIT + + # umask, not a later chmod: created under the inherited 0022 otherwise, so the + # key would be world-readable for the moment in between. + ( umask 077; printf '%s' "$priv" > wg.priv ) + : > "$STATE/routes" ip link add "$IFACE" type wireguard 2>/dev/null || \ die "could not create a WireGuard interface." \ @@ -216,6 +226,19 @@ up() { # PIA's resolvers live inside 10/8, so pin them back through the tunnel with # /32s -- longer still, so they beat the exclusion just added. + # `host.docker.internal` is answered only by the resolver about to be + # replaced -- it is not in /etc/hosts. Triple-C hands that name to the + # container for the LiteLLM gateway and defaults host-side Ollama and custom + # endpoints to it, so losing it takes the project's model backend with it. + # The *route* to it is already excluded above; only the name needs pinning. + # Resolve it with the old resolver and write it into /etc/hosts first. + local hdi + hdi=$(getent ahostsv4 host.docker.internal 2>/dev/null | awk '{print $1; exit}') + if [ -n "$hdi" ]; then + cp /etc/hosts "$STATE/hosts.bak" + printf '%s host.docker.internal\n' "$hdi" >> /etc/hosts + fi + cp /etc/resolv.conf "$STATE/resolv.conf.bak" for d in $dns; do add_route "$d/32" dev "$IFACE"; done # resolv.conf is a bind mount: write through it, never replace it. @@ -229,10 +252,16 @@ up() { # A tunnel with no handshake still routes -- into a black hole. Without this # `up --full` would exit 0 having pointed all traffic *and* resolv.conf at a # peer that never answered, and `status` would print "mode: full tunnel". - local waited=0 - until [ "$(wg show "$IFACE" latest-handshakes | awk '{print $2; exit}')" != 0 ]; do + # Demand a number, not just "different from 0". `wg show` prints nothing at + # all when the interface has no peer, and writes to stderr when the interface + # is gone -- both leave $2 empty, and `[ "" != 0 ]` is true, so the original + # form treated a missing tunnel as a completed handshake and exited 0. `until` + # suspends both `set -e` and `pipefail`, so nothing else was going to catch it. + local waited=0 hs + until hs=$(wg show "$IFACE" latest-handshakes 2>/dev/null | awk 'NR==1{print $2}') + [[ $hs =~ ^[0-9]+$ ]] && [ "$hs" -gt 0 ]; do waited=$((waited + 1)) - [ "$waited" -lt 20 ] || die "no handshake from $REGION after 10s - rolled back" + [ "$waited" -lt 40 ] || die "no handshake from $REGION after 20s" sleep 0.5 done @@ -243,6 +272,11 @@ up() { down() { [ "$(id -u)" = 0 ] || die "run with sudo" + # Teardown must finish even if a step fails; a half-rollback is the state this + # exists to prevent. Deliberately not inherited from the caller's `set -e`. + set +e + # First, because removing the interface removes every route that points at it. + ip link del "$IFACE" 2>/dev/null # Only restore something that actually looks like a resolver file. Restoring # an empty or truncated backup leaves the container with no DNS at all, which # is worse than leaving the current one alone. @@ -254,6 +288,10 @@ down() { fi rm -f "$STATE/resolv.conf.bak" fi + if [ -f "$STATE/hosts.bak" ]; then + cat "$STATE/hosts.bak" > /etc/hosts + rm -f "$STATE/hosts.bak" + fi if [ -f "$STATE/routes" ]; then # Reverse order: the specific overrides go before the ranges they sit in. tac "$STATE/routes" | while read -r r; do @@ -261,7 +299,6 @@ down() { done rm -f "$STATE/routes" fi - ip link del "$IFACE" 2>/dev/null || true # /run is in the writable layer and `docker commit` bakes it into the # project's snapshot image, so a key left here rides that image into every # future container. Verified: a snapshot already carried one. @@ -287,10 +324,15 @@ status() { # Resolve a name, not an IP literal. A curl to 1.1.1.1 succeeds while DNS is # completely broken, which is exactly how a dead resolver goes unnoticed. printf 'DNS: ' - if timeout 10 getent hosts api.anthropic.com >/dev/null 2>&1; then - echo "ok (via $(sed -n 's/^nameserver //p' /etc/resolv.conf | tr '\n' ' '))" - else + if ! timeout 10 getent hosts api.anthropic.com >/dev/null 2>&1; then echo "BROKEN - cannot resolve api.anthropic.com" + elif ! timeout 10 getent hosts host.docker.internal >/dev/null 2>&1; then + # PIA's resolvers answer public names happily, so probing only + # api.anthropic.com reports "ok" on a container that has just lost the + # Docker host -- and with it the LiteLLM gateway and any host-side Ollama. + echo "public ok, but host.docker.internal is UNRESOLVABLE (gateway/Ollama backends will fail)" + else + echo "ok (via $(sed -n 's/^nameserver //p' /etc/resolv.conf | tr '\n' ' '))" fi # Report the exit per mode. In test mode the probe address is itself the one -- 2.52.0