add deny flags for privileged, cap-add, and host namespaces#139
Closed
jalevin wants to merge 1 commit into
Closed
Conversation
Body-inspecting bind mount validation already rejects unsafe `HostConfig.Binds`
sources, but the same code path silently accepts other `HostConfig` fields
that punch through container isolation: `Privileged`, `CapAdd`, and the
host-namespace modes (`NetworkMode`/`PidMode`/`IpcMode`/`UTSMode`/`UsernsMode`).
This adds three opt-in deny flags that extend the existing body inspection
in `POST /containers/create` and `POST /containers/{id}/update`. Each
defaults to off, so upgraders see no behavior change.
- `-denyprivileged` / `SP_DENYPRIVILEGED` — rejects `Privileged=true`
- `-denycapadd` / `SP_DENYCAPADD` — rejects non-empty `CapAdd`
- `-denyhostnamespaces` / `SP_DENYHOSTNAMESPACES` — rejects `host` (or
`host:*`) values in any of the namespace mode fields
The deny flags are also exposed as per-container labels with the new
`socket-proxy.deny.` prefix (e.g. `socket-proxy.deny.privileged=true`)
to match the existing per-container allowlist pattern.
Swarm services do not surface these `HostConfig` fields in their API,
so the new flags have no effect on service create/update; only the bind
mount filter applies there as before.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThis pull request introduces opt-in deny flags that allow socket-proxy to reject container creation and update requests containing unsafe ChangesHost config deny policy enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
Closing — my claude session got a little overzealous. I'm testing locally. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds opt-in Docker HostConfig deny controls to socket-proxy so operators can block privileged containers, capability additions, and selected host namespace modes alongside existing bind-mount filtering.
Changes:
- Adds CLI/env/config/label wiring for new deny flags.
- Expands HostConfig request inspection to enforce bind-mount and security policies.
- Documents the new controls and adds unit coverage for config parsing and policy checks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents new deny flags, env vars, labels, and examples. |
| internal/config/config.go | Adds deny policy fields, flag/env parsing, and label extraction. |
| internal/config/config_test.go | Adds tests for deny flag/env/default and label parsing behavior. |
| cmd/socket-proxy/handlehttprequest.go | Passes deny policy into HostConfig restriction checks. |
| cmd/socket-proxy/bindmount.go | Implements HostConfig security checks and broadens bind-mount validation path. |
| cmd/socket-proxy/bindmount_test.go | Updates existing tests and adds HostConfig policy coverage. |
Comments suppressed due to low confidence (1)
internal/config/config.go:557
- The event-driven path has the same policy bypass as the initial population path: a container with per-container allow labels gets an AllowList whose deny flags come only from labels, so globally enabled deny flags stop applying after this allowlist is added. Merge the default deny flags into this AllowList so start/restart updates cannot weaken the global policy.
ID: cntr.ID,
AllowedRequests: allowedRequests,
AllowedBindMounts: allowedBindMounts,
DenyPrivileged: deny.Privileged,
DenyCapAdd: deny.CapAdd,
DenyHostNamespaces: deny.HostNamespaces,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+482
to
+484
| DenyPrivileged: deny.Privileged, | ||
| DenyCapAdd: deny.CapAdd, | ||
| DenyHostNamespaces: deny.HostNamespaces, |
Comment on lines
+769
to
+778
| parsedVal, err := strconv.ParseBool(labelValue) | ||
| if err != nil { | ||
| return nil, nil, denyLabels{}, fmt.Errorf("invalid boolean value %q for label %s", labelValue, labelName) | ||
| } | ||
| switch denySpec { | ||
| case "privileged": | ||
| deny.Privileged = parsedVal | ||
| case "capadd": | ||
| deny.CapAdd = parsedVal | ||
| case "hostnamespaces": |
Comment on lines
+55
to
+63
| Binds []string // List of volume bindings for this container. | ||
| Mounts []mountMount `json:",omitempty"` // Mounts specs used by the container. | ||
| Privileged bool `json:",omitempty"` // Is the container in privileged mode. | ||
| CapAdd []string `json:",omitempty"` // List of kernel capabilities to add to the container. | ||
| NetworkMode string `json:",omitempty"` // Network namespace ("host" gives host networking). | ||
| PidMode string `json:",omitempty"` // PID namespace ("host" gives host PID). | ||
| IpcMode string `json:",omitempty"` // IPC namespace ("host" gives host IPC). | ||
| UTSMode string `json:",omitempty"` // UTS namespace ("host" gives host UTS). | ||
| UsernsMode string `json:",omitempty"` // User namespace mode ("host" disables user namespace remapping). |
Comment on lines
+150
to
+153
| // checkService checks bind mounts and host config in service creation/update requests. | ||
| // Swarm services only allow specifying Mounts (not Binds) and do not expose the | ||
| // host-namespace/privileged fields, so only the Mounts list is forwarded for validation. | ||
| func checkService(allowedBindMounts []string, policy hostConfigPolicy, r *http.Request) error { |
Comment on lines
+150
to
+153
| // checkService checks bind mounts and host config in service creation/update requests. | ||
| // Swarm services only allow specifying Mounts (not Binds) and do not expose the | ||
| // host-namespace/privileged fields, so only the Mounts list is forwarded for validation. | ||
| func checkService(allowedBindMounts []string, policy hostConfigPolicy, r *http.Request) error { |
| | --- | --- | --- | | ||
| | `-denyprivileged` | `SP_DENYPRIVILEGED` | `HostConfig.Privileged == true` | | ||
| | `-denycapadd` | `SP_DENYCAPADD` | `HostConfig.CapAdd` is non-empty (any added kernel capability) | | ||
| | `-denyhostnamespaces` | `SP_DENYHOSTNAMESPACES` | `HostConfig.NetworkMode` / `PidMode` / `IpcMode` / `UTSMode` / `UsernsMode` equals `host` (or a `host:*` form for modes that accept it) | |
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.
Motivation
-allowbindmountfromalready body-inspectsPOST /containers/createandPOST /containers/{id}/updatefor unsafe bind mount sources. But the same code path silently accepts otherHostConfigfields that punch through container isolation:HostConfig.Privileged=true— grants near-root inside the container, capability to access host devices, etc.HostConfig.CapAdd— arbitrary kernel capabilities (SYS_ADMIN,NET_ADMIN, etc.)HostConfig.NetworkMode=host/PidMode=host/IpcMode=host/UTSMode=host/UsernsMode=host— shares host namespacesWhen socket-proxy is used to constrain what an untrusted client can ask the Docker daemon to do, these fields can defeat the bind-mount allowlist (e.g.
--privilegedplus a workdir bind, or--network=hostto reach loopback services). This PR adds opt-in deny flags for each.What changes
Three new flags, three matching env vars, three matching per-container labels. All default to off, so upgraders see no behavior change.
-denyprivilegedSP_DENYPRIVILEGEDsocket-proxy.deny.privilegedHostConfig.Privileged=true-denycapaddSP_DENYCAPADDsocket-proxy.deny.capaddHostConfig.CapAddnon-empty-denyhostnamespacesSP_DENYHOSTNAMESPACESsocket-proxy.deny.hostnamespacesNetworkMode/PidMode/IpcMode/UTSMode/UsernsModeequalshost(orhost:*for modes that accept it)Internally:
containerHostConfigis extended with the new fields, matching their JSON tags fromgithub.com/docker/docker/api/types/container.HostConfig.checkBindMountRestrictions(allowedBindMounts, r)is renamed and broadened tocheckHostConfigRestrictions(allowedBindMounts, policy, r). Bind mount validation continues to work identically; the newhostConfigPolicyis purely additive.checkHostConfigSecurityruns in the same body-inspection pass — no extra JSON parse, no extra socket roundtrip.AllowListgainsDenyPrivileged,DenyCapAdd,DenyHostNamespacesso per-container labels work alongside the default-allowlist flags.Endpoint coverage
Applied to the same endpoints that already get bind-mount inspection:
POST /vX.YY/containers/createPOST /vX.YY/containers/{id}/updateSwarm service create/update only surface a
Mountslist (not the host namespace / privileged fields), so the new checks intentionally do not fire there. The bind mount filter still applies to services as before.Tests
TestCheckHostConfigSecurity— 14 cases covering each field individually plus combinations and the zero-policy no-op.TestCheckHostConfigRestrictionsWithPolicy— 8 cases covering the HTTP dispatch path through container create/update and Swarm services, including the no-policy-and-no-bind-mounts early-return.TestHostConfigPolicyIsZero— sanity check on the policy helper.TestInitConfig_DenyHostConfigFlags,TestInitConfig_DenyHostConfigEnvVars,TestInitConfig_DenyHostConfigDefaultsFalse— config wiring for flags, env vars, and defaults.Test_extractLabelData_DenyLabels— per-container label parsing including invalid value and unknown-key handling.Existing tests are updated for the renamed
checkBindMountRestrictions→checkHostConfigRestrictionssignature and pass unchanged otherwise.go test ./...,go vet ./..., andgofmt -lall clean.Backwards compatibility
Out of scope / follow-up ideas
validateBindMountSourceusesfilepath.Clean. A symlink planted inside an allowed dir pointing outside it is followed by the daemon at mount time, which is a known class of escape (cf. OpenClaw OC-13).EvalSymlinkswould close this but requires the proxy container to have read access to the bind sources, which is a meaningful deployment change. Worth a separate discussion / issue.docker exec/--mounton running containers. Not currently surfaced in this PR.Happy to split into smaller commits, rename flags (e.g.
-deny-privilegedwith dash), or invert the semantics (-allowprivilegeddefaulting to allow) if it fits the project's preferences better. The current style mirrors-allowGET/-allowbindmountfrom(one word, no dash) and the opt-in deny direction.Summary by CodeRabbit
Release Notes
New Features
Documentation