Repository navigation
fix: check the ports the stack publishes in the rootless guard - #7313
Conversation
|
Claude finished @otavio's task in 3m 38s —— View job Code Review CompleteReviewed 3 files. 3 inline issues posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 2. StandardsJudgement call (Duplicated Code): the test stub for CorrectnessWorst: the new "a rootlesskit off PATH warns instead of aborting" case fails on any host with a real SpecThe Spec axis did not run because the PR references no issue to review against. Additional notesNo cap dropped any findings. Two low-value judgement calls were filtered out:
To request another review round, comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
3c7bc23 to
19c9069
Compare
|
/review |
|
Claude finished @otavio's task in 6m 30s —— View job Code Review CompleteReviewed 3 files. 2 inline issues posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 1. StandardsJudgement call (Mysterious Name): "the ssh port counts too" ( CorrectnessLast round's SpecThe Spec axis did not run because the PR references no issue to review against. Additional notesNo cap dropped any findings. Four low-value judgement calls were filtered out:
To request another review round, comment |
19c9069 to
f5e9723
Compare
|
/review |
|
Claude finished @otavio's task in 3m 32s —— View job Code Review CompleteReviewed 3 files. 1 inline issue posted. Standards: 0 violations, 1 judgement call. Spec: 0. Correctness: 0. StandardsJudgement call (test coverage, SpecThe Spec axis did not run because the PR references no issue to review against. Additional notesThe last round's fixes hold up. No cap dropped any findings. These were filtered out:
To request another review round, comment |
The guard took the lowest of SHELLHUB_HTTP_PORT, default 80, and SHELLHUB_SSH_PORT, whether the stack publishes them or not. A worktree instance resets the gateway's ports in compose.instance.yml, since Traefik serves it, so it publishes only SSH on 2200+N; under rootless every wrapper command there still aborted with "cannot publish port 80", including runs that publish nothing at all. The guard now reads the lowest published port from docker compose config, with the files, profiles and overrides the wrapper selected, and passes when nothing below the unprivileged start is published. That needs COMPOSE_FILE, so the check moves below its export; the socket stays where it was. The rendered config also carries the auto-SSL overlay's HTTPS port, so the advice names SHELLHUB_HTTPS_PORT next to the other two. On NixOS rootlesskit is not on PATH, it sits inside the docker package, so getcap "" failed, the guard read the failure as a missing capability and printed a setcap command for an empty path. A missing rootlesskit now warns that the guard cannot tell, as a missing getcap already did. The lookup goes through ROOTLESSKIT when it is set, an empty value meaning missing, so a test can hide the binary without hiding the directory around it. The test stub's compose config now prints the ports each case names in STUB_PUBLISHED, shaped as Compose writes them, a bind address on its own host_ip line, instead of rebuilding them from the env files the way the old guard did. The rootlesskit-off-PATH case sets ROOTLESSKIT empty, so a rootless developer's own /usr/bin/rootlesskit cannot satisfy the lookup, and PATH keeps coreutils. The stub's config also fails until the wrapper has exported COMPOSE_FILE, so moving the guard back above that export turns the guard cases red.
f5e9723 to
2d3a61b
Compare
Summary
Under rootless Docker, the guard from #7309 aborted every wrapper command in a worktree instance with "rootless Docker cannot publish port 80". The instance publishes nothing on port 80.
compose.instance.ymlresets the gateway's ports because Traefik serves it, and only SSH is published, on 2200+N. The guard took the lowest ofSHELLHUB_HTTP_PORT, which defaults to 80, andSHELLHUB_SSH_PORT, without asking Compose what it actually publishes.The check moves below
export COMPOSE_FILE, because it needs the files the wrapper selected. The socket detection stays where it was.On NixOS,
rootlesskitlives inside the docker package and is not onPATH. Sogetcap ""failed, the guard read the failure as a missing capability, and it printed asetcapcommand for an empty path. A missingrootlesskitnow warns that the guard cannot tell, as a missinggetcapalready did. The lookup goes throughROOTLESSKITwhen it is set; an empty value means missing.Evidence
DOCKER_HOST=unix:///run/user/1000/docker.sock ./bin/docker-compose config --servicesexited with🚫 ERROR: rootless Docker cannot publish port 80.docker compose configshows the instance publishes only2210.After: the same command lists the services. With feat: run the testcontainers and e2e suites in containers #7306 on top, on the same rootless daemon, through the wrapper:
run test go test -run '^TestLoginWithoutANamespace$'intests/passesui/scripts/e2e.sh test auth.spec.tspasses all 8 specsbats tests/compose/reports 46 ok, including with arootlesskitin/usr/bin. The stub'sdocker compose configprints the ports each case names inSTUB_PUBLISHED, shaped as Compose writes them, with a bind address on its ownhost_ip:line. Four cases cover the changed behaviour:rootlesskitwarns instead of abortingSHELLHUB_HTTP_PORT,SHELLHUB_HTTPS_PORTandSHELLHUB_SSH_PORTMerge Danger
Door: two-way
Blast Radius: rootless dev stacks
The guard now runs
docker compose configonce per wrapper call, but only in development and only on a rootless daemon. A stack that does publish a privileged port still aborts with the samesetcapadvice.