Skip to content

fix: deny empty interior path segments; allow HEAD on named containers in Rust (#55) - #71

Merged
abienkowski merged 6 commits into
mainfrom
fix/empty-segment-head-55
Oct 10, 2026
Merged

abienkowski merged 6 commits into
mainfrom
fix/empty-segment-head-55

Conversation

@abienkowski

@abienkowski abienkowski commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Part of #55 (Task 3 of the plan: the empty-segment and HEAD items). Follow-up for dot segments: #70.

What changes

  • Empty interior segments are denied in Go, Rust and TS, for every method, with empty path segment not allowed. The check runs right after the percent check (Percent-encoded container names: Go routes on the decoded path, Rust/TS on the raw path #53) and before the API-version strip, on the path without its query. Before this, GET //exec/<id>/json, /v1.45//exec/… and //v1.45/exec/… had an empty first segment, so the exec check (Routing parity: TS path-wide exec deny; Go matchEndpoint accepts endpoint subpaths #49) missed them and the GET passthrough forwarded them. A single trailing slash (/containers/) and // inside a query string are unchanged.
  • HEAD on a named container is allowed like GET in Rust. Go and TS already did this; HEAD …/exec stays denied in all three.
  • Spec (spec/router.qnt): flag ALLOW_EMPTY_INTERIOR, properties emptyInteriorDenied and emptySegmentsHideNoExec (an exec path once its empty segments are dropped is denied), each with its own run, new rows, HEAD routed like GET, and a new instance router_pre55 that shows the old unsoundness. make test-spec runs it.
  • Docs: README endpoint table and prose, spec/README.md modelling notes, AGENTS.md instance list.

Security note: absolute-form targets in TypeScript (fixed here)

The review found, and I reproduced end to end against Docker 29.4.0, that TS v0.3.1 through v0.3.3 leak exec inspect (since #49's whole-segment exec match; v0.3.0 and earlier matched /exec as a substring of the raw target and denied it) through an absolute-form request target. Node's req.url is the raw target, so for GET http://docker/exec/<id>/json the TS router's first segment was http:, the exec check missed it, and the GET passthrough forwarded it. The daemon answered 200 with the exec's full command line. Go and Rust route on the parsed path and answered 403.

TS on main (absolute form) Result
GET http://docker/exec/<id>/json 200 (leak)
GET http://docker/containers/json 200
POST …/containers/<c>/exec, POST …/exec/<id>/start, POST …/containers/create (privileged), DELETE …/containers/<c> 403

Only reads leaked; every write was still default-denied. On this branch, every absolute-form target is denied in TS (llhttp requires // after the scheme, so the empty-segment check catches it); the Docker CLI never sends absolute form. A new integration check pins it for all three implementations, and a TS router row (GET http://docker/exec/abc/json → empty path segment not allowed) pins it at unit level. Go and Rust still route non-exec absolute-form targets normally (checked: GET http://docker/containers/json → 200, exec → 403); TS denies them all.

Review notes

  • Release: merging cuts v0.3.4 (patch). Simulated squash: bump=patch tag=v0.3.4.
  • Dot segments (/./exec/…, /x/../exec/…) are still allowed by all three routers; tracked in Router allows exec paths hidden behind dot segments (/./exec, /x/../exec) #70. The daemon redirects them (301), as it did for //.
  • Counts: unit and integration counts in AGENTS.md/README.md are updated in Task 6, as planned.

Verification

  • Test-first, at bd96aae (tests, no fix): the new table fails in all three. GET //exec/abc/json, /v1.45//exec/abc/json, //v1.45/exec/abc/json, /containers//json and //_ping were allowed; POST/DELETE // rows were denied with the wrong message; Rust also denied HEAD /containers/mycontainer and …/json.
  • Spec mutation: setting router's ALLOW_EMPTY_INTERIOR = true fails exactly emptyLeadingExecGetDeniedTest, emptyInteriorGetDeniedTest and emptyInteriorSoundTest. router_pre55 asserts both properties fail there.
  • Integration on main with the new checks (TS): FAIL: GET //exec/*/json -> 403 (empty segment) (expected 403, got 301) and FAIL: GET http://docker/exec/*/json -> 403 (absolute-form target) (expected 403, got 404).
  • This branch:
    • make test-all: Go ok, Rust 146 passed, TS 163 tests, 162 pass, 1 skipped.
    • make lint-all passes; make test-spec: all 6 runs pass.
    • Integration: go ALL 45 TESTS PASSED, rs ALL 45 TESTS PASSED, ts ALL 45 TESTS PASSED; socket suite ALL 15 TESTS PASSED.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Implementation(s) changed

  • Go
  • Rust
  • TypeScript
  • Quint specification
  • CI / infrastructure

Testing

  • Unit tests pass (make test-all)
  • Integration tests pass (make test-integration, -rs, -ts: 45 each; socket suite 15)
  • Quint verification passes (make test-spec)
  • New tests added for the change

Checklist

  • I have read CONTRIBUTING.md
  • My code follows the project's coding style
  • I have updated documentation as needed

@abienkowski abienkowski added Priority: P1 Added to issues and PRs relating to a high severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug labels Oct 10, 2026
@abienkowski
abienkowski merged commit 60a14e0 into main Oct 10, 2026
7 checks passed
@abienkowski
abienkowski deleted the fix/empty-segment-head-55 branch October 10, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P1 Added to issues and PRs relating to a high severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant