Repository navigation
fix: deny empty interior path segments; allow HEAD on named containers in Rust (#55) - #71
Merged
Merged
Conversation
1 task done
This was referenced Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part of #55 (Task 3 of the plan: the empty-segment and HEAD items). Follow-up for dot segments: #70.
What changes
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 …/execstays denied in all three.spec/router.qnt): flagALLOW_EMPTY_INTERIOR, propertiesemptyInteriorDeniedandemptySegmentsHideNoExec(an exec path once its empty segments are dropped is denied), each with its ownrun, new rows, HEAD routed like GET, and a new instancerouter_pre55that shows the old unsoundness.make test-specruns it.spec/README.mdmodelling notes,AGENTS.mdinstance 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
/execas a substring of the raw target and denied it) through an absolute-form request target. Node'sreq.urlis the raw target, so forGET http://docker/exec/<id>/jsonthe TS router's first segment washttp:, 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.main(absolute form)GET http://docker/exec/<id>/jsonGET http://docker/containers/jsonPOST …/containers/<c>/exec,POST …/exec/<id>/start,POST …/containers/create(privileged),DELETE …/containers/<c>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
bump=patch tag=v0.3.4././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//.AGENTS.md/README.mdare updated in Task 6, as planned.Verification
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//jsonand//_pingwere allowed; POST/DELETE//rows were denied with the wrong message; Rust also deniedHEAD /containers/mycontainerand…/json.router'sALLOW_EMPTY_INTERIOR = truefails exactlyemptyLeadingExecGetDeniedTest,emptyInteriorGetDeniedTestandemptyInteriorSoundTest.router_pre55asserts both properties fail there.mainwith the new checks (TS):FAIL: GET //exec/*/json -> 403 (empty segment) (expected 403, got 301)andFAIL: GET http://docker/exec/*/json -> 403 (absolute-form target) (expected 403, got 404).make test-all: Go ok, Rust146 passed, TS163 tests, 162 pass, 1 skipped.make lint-allpasses;make test-spec: all 6 runs pass.go ALL 45 TESTS PASSED,rs ALL 45 TESTS PASSED,ts ALL 45 TESTS PASSED; socket suiteALL 15 TESTS PASSED.Type of change
Implementation(s) changed
Testing
make test-all)make test-integration,-rs,-ts: 45 each; socket suite 15)make test-spec)Checklist