fix(podmanx,docs): close the host.containers.internal back-channel for container runs #271

Merged

dominik.polakovics merged 1 commit from afk/216 into main 2026-08-03 22:16:33 +02:00

Conversation 1 Commits 1 Files changed 10 +243 -37

dominik.polakovics commented 2026-08-03 21:59:36 +02:00

Owner

Copy link

Closes #216

What was wrong

ADR-0052 claimed pasta gives "full egress with no route back to the host's services … never a host port." That held only for loopback-bound host services. Podman appends --map-guest-addr 169.254.1.2 to its pasta defaults, which maps host.containers.internal onto the host's global address — so every wildcard-bound (0.0.0.0) host service, lab's own operator server on --addr :8080 included, was reachable from inside every run container. --no-map-gw (podman's default) closes only the loopback/gateway path.

Reproduced live from inside a run container on the dev host: host.containers.internal:8080 → HTTP 200, while the guest's own 127.0.0.1:8080 failed.

What changed

RunArgv — the single renderer for all three shapes (run pane, login pane, non-interactive CLI poke) — now pins:

--network=pasta:--map-guest-addr,none
--add-host host.containers.internal:127.0.0.1
--add-host host.docker.internal:127.0.0.1

Both are needed, and this is the one place the PR goes beyond the issue's stated mechanism. The pasta option alone is not sufficient: verified in containers/common + podman source, when no address is mapped podman does not drop the /etc/hosts names — GetHostContainersInternalIP falls through to util.GetLocalIPExcluding, which returns the first other global-unicast host address (RFC1918 included; only loopback/link-local/multicast are ruled out). On the dev host that fallback lands on the bridge address 172.17.0.1, where lab's :8080 also answers — measured, HTTP 200 — so the option by itself would have left the leak intact on this exact host. The --add-host pins claim both names first; podman's addEntriesIfNotExists then skips its automatic entry for names already taken. Two flags rather than the name;name:ip form, since parseExtraHosts splits on ; only on recent podman and the preflight floor is podman >= 4.

The pasta option composes with podman's other defaults rather than replacing them (containers/common skips only the --map-guest-addr it would otherwise append, still adding --no-map-gw, -t/-u/-T/-U none, --dns-forward 169.254.1.1), so egress and DNS forwarding are untouched.

Preflight

The existing pasta check widens in place — same check id, now "missing or too old", mirroring how podman already covers missing/unrunnable/too-old. It runs pasta --map-guest-addr none --version and judges exit status alone; it only runs when pasta is on PATH, so a missing pasta is never blamed twice.

It probes the exact option and value the argv uses, deliberately, because the floor has two stages:

passt	--map-guest-addr	value none
< 2024_08_21	absent	—
2024_08_21 … 2025_03_20	present	rejected (conf_nat() matched the literal without returning, fell into inet_pton, died Invalid address to remap to host: none)
>= 2025_04_15	present	accepted

A version gate — or a pasta --help grep — would call that middle window green, and podman's backwards-compat retry covers only the default it appends, never a user-supplied option, so those hosts would fail at container start, once per spawn, behind a green preflight. The argument order is load-bearing (--version first would exit 0 on every passt ever built); the test suite pins it, verified with a negative control.

Residual, documented not papered over

pasta still grants full egress, so a wildcard-bound host service on a non-shadowed host address stays reachable by raw IP (172.17.0.1:8080 still answers). The host's own primary address is unreachable — the guest shadows it. Binding co-located host services to 127.0.0.1 is therefore the control on a multi-address host, not defense in depth; egress filtering is explicitly out of scope for #216. ADR-0052, docs/ops.md and the DoD all say this plainly now, and the DoD's manual-residue probe records both the expected failure (host.containers.internal) and the expected success (raw IP).

Verification

• go build ./..., go vet ./..., go test ./... — all green
• golangci-lint run ./... at CI's pinned v2.12.2 — 0 issues; gofmt -l clean
• Five golden argv tests updated (podmanx run/login/CLI shapes + providercli login/CLI); internal/instance's argv tests build their expectation through RunArgv, so they cover the change automatically
• Preflight gains two failure cases — "option unknown" and "option known, value rejected" — plus a strengthened "pasta missing" case proving the capability probe is skipped (no double-blame)
• No live container spawn: this sandbox has no podman/pasta, so the loop stays maintainer-validated on the dev host per DoD §11. All upstream behavior above was verified against passt conf.c/passt.1 at the specific release tags and against containers/common + podman source, not from memory.

Closes #216 ## What was wrong ADR-0052 claimed pasta gives *"full egress with no route back to the host's services … never a host port."* That held only for **loopback-bound** host services. Podman appends `--map-guest-addr 169.254.1.2` to its pasta defaults, which maps `host.containers.internal` onto the host's **global** address — so every wildcard-bound (`0.0.0.0`) host service, lab's own operator server on `--addr :8080` included, was reachable from inside every run container. `--no-map-gw` (podman's default) closes only the loopback/gateway path. Reproduced live from inside a run container on the dev host: `host.containers.internal:8080` → **HTTP 200**, while the guest's own `127.0.0.1:8080` failed. ## What changed `RunArgv` — the single renderer for all three shapes (run pane, login pane, non-interactive CLI poke) — now pins: ``` --network=pasta:--map-guest-addr,none --add-host host.containers.internal:127.0.0.1 --add-host host.docker.internal:127.0.0.1 ``` **Both are needed, and this is the one place the PR goes beyond the issue's stated mechanism.** The pasta option alone is *not* sufficient: verified in containers/common + podman source, when no address is mapped podman does not drop the `/etc/hosts` names — `GetHostContainersInternalIP` falls through to `util.GetLocalIPExcluding`, which returns the first **other** global-unicast host address (RFC1918 included; only loopback/link-local/multicast are ruled out). On the dev host that fallback lands on the bridge address `172.17.0.1`, where lab's `:8080` also answers — measured, HTTP 200 — so the option by itself would have left the leak intact on this exact host. The `--add-host` pins claim both names first; podman's `addEntriesIfNotExists` then skips its automatic entry for names already taken. Two flags rather than the `name;name:ip` form, since `parseExtraHosts` splits on `;` only on recent podman and the preflight floor is podman >= 4. The pasta option **composes with** podman's other defaults rather than replacing them (containers/common skips only the `--map-guest-addr` it would otherwise append, still adding `--no-map-gw`, `-t/-u/-T/-U none`, `--dns-forward 169.254.1.1`), so **egress and DNS forwarding are untouched**. ## Preflight The existing **`pasta` check widens in place** — same check id, now "missing *or* too old", mirroring how `podman` already covers missing/unrunnable/too-old. It runs `pasta --map-guest-addr none --version` and judges **exit status alone**; it only runs when pasta is on PATH, so a missing pasta is never blamed twice. It probes the exact option **and value** the argv uses, deliberately, because the floor has two stages: | passt | `--map-guest-addr` | value `none` | |---|---|---| | < `2024_08_21` | absent | — | | `2024_08_21` … `2025_03_20` | **present** | **rejected** (`conf_nat()` matched the literal without returning, fell into `inet_pton`, died `Invalid address to remap to host: none`) | | >= `2025_04_15` | present | accepted | A version gate — or a `pasta --help` grep — would call that middle window green, and podman's backwards-compat retry covers only the default **it** appends, never a user-supplied option, so those hosts would fail at *container start*, once per spawn, behind a green preflight. The argument order is load-bearing (`--version` first would exit 0 on every passt ever built); the test suite pins it, verified with a negative control. ## Residual, documented not papered over pasta still grants full egress, so a wildcard-bound host service on a **non-shadowed** host address stays reachable **by raw IP** (`172.17.0.1:8080` still answers). The host's own primary address is unreachable — the guest shadows it. Binding co-located host services to `127.0.0.1` is therefore *the* control on a multi-address host, not defense in depth; egress filtering is explicitly out of scope for #216. ADR-0052, `docs/ops.md` and the DoD all say this plainly now, and the DoD's manual-residue probe records both the expected failure (`host.containers.internal`) and the expected success (raw IP). ## Verification - `go build ./...`, `go vet ./...`, `go test ./...` — all green - `golangci-lint run ./...` at CI's pinned v2.12.2 — **0 issues**; `gofmt -l` clean - Five golden argv tests updated (podmanx run/login/CLI shapes + providercli login/CLI); `internal/instance`'s argv tests build their expectation through `RunArgv`, so they cover the change automatically - Preflight gains two failure cases — "option unknown" and "option known, value rejected" — plus a strengthened "pasta missing" case proving the capability probe is skipped (no double-blame) - **No live container spawn**: this sandbox has no podman/pasta, so the loop stays maintainer-validated on the dev host per DoD §11. All upstream behavior above was verified against passt `conf.c`/`passt.1` at the specific release tags and against containers/common + podman source, not from memory.

dominik.polakovics added 1 commit 2026-08-03 21:59:36 +02:00

fix(podmanx,docs): close the host.containers.internal back-channel for container runs ... f65c1faf8b

ADR-0052 claimed pasta leaves "no route back to the host's services ...
never a host port". That held only for loopback-bound services: podman
appends --map-guest-addr 169.254.1.2 to its pasta defaults, mapping
host.containers.internal onto the host's *global* address, so every
wildcard-bound host service — lab's own operator server on --addr :8080
included — was reachable from inside every run container. Reproduced live
from a run container: host.containers.internal:8080 answered 200 while the
guest's own 127.0.0.1:8080 did not.

The argv now pins, in all three shapes RunArgv renders (run pane, login
pane, non-interactive CLI poke):

  --network=pasta:--map-guest-addr,none
  --add-host host.containers.internal:127.0.0.1
  --add-host host.docker.internal:127.0.0.1

Both are needed. The pasta option drops the mapping and composes with
podman's remaining defaults (--no-map-gw, -t/-u/-T/-U none, --dns-forward)
rather than replacing them, so egress and DNS are untouched — but podman
does not then leave the names alone: with no mapped address it falls back
to the first OTHER global-unicast host address it finds, so on a host with
a bridge, a second NIC or a VPN the name merely re-points and stays live.
Measured on the dev host, that fallback lands on the bridge address, where
:8080 also answers. The --add-host pins claim both names first, and
podman's addEntriesIfNotExists then skips its automatic entry.

Preflight's existing `pasta` check widens in place from "missing" to
"missing or too old", probing `pasta --map-guest-addr none --version` and
judging exit status alone. It runs the exact option AND value the argv
uses, because the floor has two stages: passt 2024_08_21 added
--map-guest-addr, but conf_nat() matched the "none" literal without
returning and died "Invalid address to remap to host: none" until
2025_04_15 added the missing return. A version gate or a `pasta --help`
grep would call that window green and then fail every spawn at container
start — podman's back-compat retry covers only the default it appends
itself, never a user-supplied option.

Residual, documented rather than papered over: pasta still grants full
egress, so a wildcard-bound service on a non-shadowed host address stays
reachable by raw IP. Binding co-located services to 127.0.0.1 remains the
control there; egress filtering is out of scope for #216.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented 2026-08-03 22:16:26 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

PASS — validated against the validation core.

Signal relied on: labctl pr checks 271 --wait → aggregate success (ci / native (pull_request), 12m17s). That job is unconditional on pull_request and runs the SPA lint/test/build, go build+go test (tags ui), and golangci-lint v2.12.2 — so its reach covers every file in this diff. ci-nix is path-gated on **/*.nix/flake.lock/go.mod and agent-tools on containers/**; this diff touches none of those, so their absence is correct, not a coverage gap. No re-run of what CI already ran.

Conventions: Conventional Commits title (fix(podmanx,docs):) ✓ · Closes #216 present in the body ✓ · head afk/216 matches the linked issue ✓ · 1 commit ahead of origin/main, 0 behind, merge-base is main's tip — no conflict.

Diff-scope: matches #216's agent brief. All three RunArgv shapes lose the mapping, the five golden argv tests are updated (podmanx run/login/CLI + providercli login/CLI), the preflight widens in place under the same pasta check id, and ADR-0052 / ops.md / the DoD are rewritten. The --add-host pins go beyond the brief's literal mechanism, but they serve its stated desired behavior ('no host.containers.internal mapping at all') and the PR flags them explicitly — not a drive-by.

Independently verified every load-bearing upstream claim against real source rather than taking the PR's word:

1. passt conf.c — --version (case 14, l.1478) and --map-guest-addr (case 22 → conf_nat, l.1530) are handled in the SAME getopt_long pass, which ends case '?': default: usage(argv[0], stderr, EXIT_FAILURE) (l.1819). So the probe's argument order really is load-bearing exactly as documented: old passt → '?' → non-zero; middle-window passt → conf_nat dies before --version can exit 0; modern passt → parses none, then exits 0. Reversed, it would be a guaranteed false green. conf_nat's none branch has the early return, and the fall-through die("Invalid address to remap to host: %s") matches the test fixture's string verbatim.

2. containers/common libnetwork/pasta/pasta_linux.go — the composition claim holds: podman appends its --map-guest-addr 169.254.1.2 only when the user supplied none (l.272-275), while --no-map-gw (l.261) is cleared solely by the pseudo-option --map-gw, never by --map-guest-addr; --dns-forward and the -t/-u/-T/-U none defaults are untouched. Egress and DNS are genuinely unaffected. The backwards-compat retry (l.76-84) is explicitly gated on mapGuestAddrIPs[0] == mapGuestAddrIpv4, so it never fires for a user-supplied value — which is precisely why the preflight check has to exist.

3. containers/common libnetwork/etchosts/hosts.go — writeHostFile writes user --add-host entries first and registers their names, then addEntriesIfNotExists filters podman's automatic entry down to its free names. Podman's entry carries both host.containers.internal and host.docker.internal, so pinning both leaves freeNames empty and podman writes nothing — pinning only one would have left the other live on the fallback address. The two-flag form also sidesteps parseExtraHosts' ; splitting, consistent with the podman >= 4 floor.

Test coverage is proportionate: preflight gains both floor cases (option unknown / value rejected) plus a strengthened pasta-missing case proving the probe is skipped so one host is never blamed twice under one check id.

The documented residual (raw-IP reachability of wildcard-bound services on non-shadowed host addresses) is stated plainly in ADR-0052, ops.md and the DoD rather than papered over, and egress filtering is explicitly out of scope for #216. No findings rise to CONCERNS.

[autoland] verdict: pass PASS — validated against the validation core. Signal relied on: `labctl pr checks 271 --wait` → aggregate **success** (`ci / native (pull_request)`, 12m17s). That job is unconditional on `pull_request` and runs the SPA lint/test/build, `go build`+`go test` (tags ui), and golangci-lint v2.12.2 — so its reach covers every file in this diff. `ci-nix` is path-gated on `**/*.nix`/`flake.lock`/`go.mod` and `agent-tools` on `containers/**`; this diff touches none of those, so their absence is correct, not a coverage gap. No re-run of what CI already ran. Conventions: Conventional Commits title (`fix(podmanx,docs):`) ✓ · `Closes #216` present in the body ✓ · head `afk/216` matches the linked issue ✓ · 1 commit ahead of `origin/main`, 0 behind, merge-base is main's tip — no conflict. Diff-scope: matches #216's agent brief. All three `RunArgv` shapes lose the mapping, the five golden argv tests are updated (podmanx run/login/CLI + providercli login/CLI), the preflight widens in place under the same `pasta` check id, and ADR-0052 / ops.md / the DoD are rewritten. The `--add-host` pins go beyond the brief's literal mechanism, but they serve its stated desired behavior ('no host.containers.internal mapping at all') and the PR flags them explicitly — not a drive-by. Independently verified every load-bearing upstream claim against real source rather than taking the PR's word: 1. **passt `conf.c`** — `--version` (case 14, l.1478) and `--map-guest-addr` (case 22 → `conf_nat`, l.1530) are handled in the SAME getopt_long pass, which ends `case '?': default: usage(argv[0], stderr, EXIT_FAILURE)` (l.1819). So the probe's argument order really is load-bearing exactly as documented: old passt → '?' → non-zero; middle-window passt → `conf_nat` dies before `--version` can exit 0; modern passt → parses `none`, then exits 0. Reversed, it would be a guaranteed false green. `conf_nat`'s `none` branch has the early `return`, and the fall-through `die("Invalid address to remap to host: %s")` matches the test fixture's string verbatim. 2. **containers/common `libnetwork/pasta/pasta_linux.go`** — the composition claim holds: podman appends its `--map-guest-addr 169.254.1.2` only when the user supplied none (l.272-275), while `--no-map-gw` (l.261) is cleared solely by the pseudo-option `--map-gw`, never by `--map-guest-addr`; `--dns-forward` and the `-t/-u/-T/-U none` defaults are untouched. Egress and DNS are genuinely unaffected. The backwards-compat retry (l.76-84) is explicitly gated on `mapGuestAddrIPs[0] == mapGuestAddrIpv4`, so it never fires for a user-supplied value — which is precisely why the preflight check has to exist. 3. **containers/common `libnetwork/etchosts/hosts.go`** — `writeHostFile` writes user `--add-host` entries first and registers their names, then `addEntriesIfNotExists` filters podman's automatic entry down to its *free* names. Podman's entry carries both `host.containers.internal` and `host.docker.internal`, so pinning both leaves `freeNames` empty and podman writes nothing — pinning only one would have left the other live on the fallback address. The two-flag form also sidesteps `parseExtraHosts`' `;` splitting, consistent with the podman >= 4 floor. Test coverage is proportionate: preflight gains both floor cases (option unknown / value rejected) plus a strengthened pasta-missing case proving the probe is skipped so one host is never blamed twice under one check id. The documented residual (raw-IP reachability of wildcard-bound services on non-shadowed host addresses) is stated plainly in ADR-0052, ops.md and the DoD rather than papered over, and egress filtering is explicitly out of scope for #216. No findings rise to CONCERNS.

dominik.polakovics merged commit a41950d688 into main 2026-08-03 22:16:33 +02:00

dominik.polakovics referenced this pull request from a commit 2026-08-03 22:16:33 +02:00

Merge pull request 'fix(podmanx,docs): close the host.containers.internal back-channel for container runs' (#271) from afk/216 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is broken

  

chore

  

enhancement

New feature or improvement

  

needs-info

Waiting on reporter for more information

  

needs-triage

Maintainer needs to evaluate this issue

  

ready-for-agent

Fully specified, ready for an AFK agent

  

ready-for-human

Requires human implementation

  

wontfix

Will not be actioned

No labels

bug

chore

enhancement

needs-info

needs-triage

ready-for-agent

ready-for-human

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Cloonar/coding-lab!271

No description provided.