diff --git a/modules/agent-box.nix b/modules/agent-box.nix index 009bbd81..ca0923a9 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -4628,20 +4628,26 @@ in PrivateTmp = true; PrivateDevices = true; # keeps pty subsystem; blocks /dev/mem etc. ProtectSystem = "strict"; # entire fs read-only except explicit RW paths - # ~/sites is a SYMLINK to /var/lib/agent-box-sites/, so a write - # through it resolves outside /home and ProtectSystem=strict denied it - # with EROFS — the self-serve vhost flow the guide documents was - # impossible from inside the agent's own namespace. ReadWritePaths - # matches on the resolved path, so the target has to be listed too. - # The dir is tmpfiles-created (sysinit, well before this unit), so it - # always exists by unit start and needs no `-` prefix — but ONLY when - # web.enable is on, since that is what gates the tmpfiles rule. Listing - # it unconditionally fails the whole namespace setup with 226/NAMESPACE - # on a default (web-less) box, so the unit never starts at all: no `-` - # here, an explicit guard instead, so a dir that is genuinely missing - # while web is on stays loud rather than silently reverting to EROFS. + # ~/sites and ~/downloads are SYMLINKS to /var/lib/agent-box-{sites, + # downloads}/, so a write through either resolves outside /home + # and ProtectSystem=strict denied it with EROFS — both flows the guide + # documents (serve a vhost by writing a snippet; hand the user a file + # by moving it into ~/downloads) were impossible from inside the + # agent's own namespace. ReadWritePaths matches on the resolved path, + # so the targets have to be listed too. + # Both dirs are tmpfiles-created (sysinit, well before this unit), so + # they always exist by unit start and need no `-` prefix — but ONLY + # when web.enable is on, since that is what gates the tmpfiles rules. + # Listing them unconditionally fails the whole namespace setup with + # 226/NAMESPACE on a default (web-less) box, so the unit never starts + # at all: no `-` here, an explicit guard instead, so a dir that is + # genuinely missing while web is on stays loud rather than silently + # reverting to EROFS. ReadWritePaths = [ "/home/${name}" ] - ++ lib.optional cfg.web.enable "/var/lib/agent-box-sites/${name}"; + ++ lib.optionals cfg.web.enable [ + "/var/lib/agent-box-sites/${name}" + (downloadsDirOf name) + ]; ProtectKernelTunables = true; ProtectKernelModules = true; ProtectControlGroups = true; diff --git a/modules/agent-box.nix.in b/modules/agent-box.nix.in index ccaf7beb..1202f9d7 100644 --- a/modules/agent-box.nix.in +++ b/modules/agent-box.nix.in @@ -1714,20 +1714,26 @@ in PrivateTmp = true; PrivateDevices = true; # keeps pty subsystem; blocks /dev/mem etc. ProtectSystem = "strict"; # entire fs read-only except explicit RW paths - # ~/sites is a SYMLINK to /var/lib/agent-box-sites/, so a write - # through it resolves outside /home and ProtectSystem=strict denied it - # with EROFS — the self-serve vhost flow the guide documents was - # impossible from inside the agent's own namespace. ReadWritePaths - # matches on the resolved path, so the target has to be listed too. - # The dir is tmpfiles-created (sysinit, well before this unit), so it - # always exists by unit start and needs no `-` prefix — but ONLY when - # web.enable is on, since that is what gates the tmpfiles rule. Listing - # it unconditionally fails the whole namespace setup with 226/NAMESPACE - # on a default (web-less) box, so the unit never starts at all: no `-` - # here, an explicit guard instead, so a dir that is genuinely missing - # while web is on stays loud rather than silently reverting to EROFS. + # ~/sites and ~/downloads are SYMLINKS to /var/lib/agent-box-{sites, + # downloads}/, so a write through either resolves outside /home + # and ProtectSystem=strict denied it with EROFS — both flows the guide + # documents (serve a vhost by writing a snippet; hand the user a file + # by moving it into ~/downloads) were impossible from inside the + # agent's own namespace. ReadWritePaths matches on the resolved path, + # so the targets have to be listed too. + # Both dirs are tmpfiles-created (sysinit, well before this unit), so + # they always exist by unit start and need no `-` prefix — but ONLY + # when web.enable is on, since that is what gates the tmpfiles rules. + # Listing them unconditionally fails the whole namespace setup with + # 226/NAMESPACE on a default (web-less) box, so the unit never starts + # at all: no `-` here, an explicit guard instead, so a dir that is + # genuinely missing while web is on stays loud rather than silently + # reverting to EROFS. ReadWritePaths = [ "/home/${name}" ] - ++ lib.optional cfg.web.enable "/var/lib/agent-box-sites/${name}"; + ++ lib.optionals cfg.web.enable [ + "/var/lib/agent-box-sites/${name}" + (downloadsDirOf name) + ]; ProtectKernelTunables = true; ProtectKernelModules = true; ProtectControlGroups = true; diff --git a/tests/golden/web/units/agent-box-agent.service b/tests/golden/web/units/agent-box-agent.service index 45e49cfc..232dace6 100644 --- a/tests/golden/web/units/agent-box-agent.service +++ b/tests/golden/web/units/agent-box-agent.service @@ -41,6 +41,7 @@ ProtectKernelTunables=true ProtectSystem=strict ReadWritePaths=/home/agent ReadWritePaths=/var/lib/agent-box-sites/agent +ReadWritePaths=/var/lib/agent-box-downloads/agent Restart=always RestartSec=2s RestrictRealtime=true diff --git a/tests/golden/web/units/agent-box-robot.service b/tests/golden/web/units/agent-box-robot.service index 07b2a459..04e9b32e 100644 --- a/tests/golden/web/units/agent-box-robot.service +++ b/tests/golden/web/units/agent-box-robot.service @@ -41,6 +41,7 @@ ProtectKernelTunables=true ProtectSystem=strict ReadWritePaths=/home/robot ReadWritePaths=/var/lib/agent-box-sites/robot +ReadWritePaths=/var/lib/agent-box-downloads/robot Restart=always RestartSec=2s RestrictRealtime=true diff --git a/tests/web-surface.nix b/tests/web-surface.nix index 6402c364..8a94f774 100644 --- a/tests/web-surface.nix +++ b/tests/web-surface.nix @@ -107,6 +107,16 @@ client_ip = client.succeed("ip -4 -o addr show eth1 | head -1").split()[3].split("/")[0] curl = f"curl -sk --resolve box.test:443:{machine_ip}" + # Writes that the guide tells an agent to make (~/downloads, ~/sites) must be + # exercised INSIDE the agent unit's mount namespace. Writing as the agent uid + # from the driver's root shell skips ProtectSystem entirely, which is how a + # read-only ~/downloads shipped under a green test (issue #316). + agent_pid = machine.succeed( + "systemctl show -p MainPID --value agent-box-agent.service" + ).strip() + assert agent_pid not in ("", "0"), "agent unit has no main PID" + in_session = f"nsenter -t {agent_pid} -m -- runuser -u agent --" + with subtest("~/downloads is a per-user file drop served behind the auth gate"): # The tmpfiles-created symlink from ~agent/downloads into the backing dir. machine.succeed("test -L /home/agent/downloads") @@ -121,10 +131,19 @@ "stat -c '%U:%G %a' /var/lib/agent-box-downloads/agent | grep -x 'agent:caddy 750'" ) + # ~/downloads resolves to /var/lib/agent-box-downloads/agent, outside + # /home — so ProtectSystem=strict denies it with EROFS unless the target + # is named in ReadWritePaths, exactly as for ~/sites below (issue #316). + machine.succeed( + "systemctl show agent-box-agent --property=ReadWritePaths --value " + "| grep /var/lib/agent-box-downloads/agent >/dev/null" + ) + # The agent drops a file through the ~/downloads symlink (never touches - # /var/lib directly), exactly as AGENTS.md instructs. + # /var/lib directly), exactly as AGENTS.md instructs — and from inside + # the unit's namespace, which is the only place that proves it. machine.succeed( - "sudo -u agent tee /home/agent/downloads/report.txt > /dev/null <<'EOF'\n" + f"{in_session} tee /home/agent/downloads/report.txt > /dev/null <<'EOF'\n" "hello from the box\n" "EOF" ) @@ -182,12 +201,8 @@ # nsenter joins the running unit's mount namespace so the write is subject # to the same read-only remount a tool shell inside the session gets; # runuser then drops to the agent uid for the ownership check below. - agent_pid = machine.succeed( - "systemctl show -p MainPID --value agent-box-agent.service" - ).strip() - assert agent_pid not in ("", "0"), "agent unit has no main PID" machine.succeed( - f"nsenter -t {agent_pid} -m -- runuser -u agent -- " + f"{in_session} " "tee /home/agent/sites/mysite.caddy > /dev/null <<'CFG'\n" "mysite.test {\n" " tls internal\n"