Default host path binds to read-only

[kgprs]

Summary

• normalize host-path bind mounts without an explicit mode to :ro by default
• add MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS as a path-scoped override for writable host binds
• keep explicitly writable host-path binds blocked unless their source is under a configured writable root
• update bind validation errors to point users at the read-only or writable env var as appropriate
• add coverage for filesystem-style {{paths|volume|into}} profile config

Why This Is Needed

v0.43.0 hardened Docker volume handling so host-path binds must be read-only. That is a good default, but catalog entries such as the Filesystem server use the volume template helper:

volumes:
  - "{{filesystem.paths|volume|into}}"

With filesystem.paths=["/Users/user/Documents"], that evaluates to:

/Users/user/Documents:/Users/user/Documents

Before this PR, even when the user explicitly allowed the host root:

MCP_GATEWAY_DOCKER_BIND_ALLOWED_PATHS=/Users/user \
  docker mcp gateway run --transport streaming --profile test

the gateway rejected the bind before startup:

Can't start filesystem: validate volume for filesystem: unsafe docker volume "/Users/user/Documents:/Users/user/Documents": host path bind mounts must be read-only

This PR preserves the hardening by converting omitted host bind modes to read-only:

/Users/user/Documents:/Users/user/Documents:ro

Some servers do need write access to user-selected host paths. For that case, the new writable override is explicit and path-scoped:

MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS=/Users/user/Documents \
  docker mcp gateway run --transport streaming --profile test

When the bind source is under MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS, an omitted mode becomes writable:

/Users/user/Documents:/Users/user/Documents:rw

Explicit source:target:rw binds are also allowed under the configured writable root. Writable roots also count as allowed bind roots, so users do not need to set both env vars for the writable case. Sensitive system and credential paths remain blocked.

Validation

• go test ./pkg/gateway -run 'TestApplyConfig(VolumeFilter|LongLivedRejectsWritableHostBind|ShortLivedRejectsWritableHostBind)|TestValidateDockerVolumeBinds'
• go test ./pkg/gateway
• go test ./pkg/eval
• Attempted go test ./...; local integration tests in pkg failed before completion with environment/tooling issues including unknown flag: --gateway-arg and MCP initialize EOF. The directly touched gateway package passed.

[saucow]

Security review — host-path writable binds

Thanks for this — the default-to-:ro normalization and (importantly) keeping the sensitive-path denylist ahead of the new writable decision are both the right shape. I traced the new logic in docker_binds.go and the safe properties do hold:

• disallowedDockerHostPath runs at :57, before the writable check at :60, so /etc, ~/.ssh, ~/.docker, and /var/run/docker.sock stay blocked even when a writable root is configured (verified).
• dockerBindWritableAllowedRoots() returns nil unless the env is set — a catalog can't set it — so with no env, no-mode binds default to :ro and explicit :rw is rejected.
• / is dropped from roots and prefix matching is segment-aware (/user ≠ /user-evil).

So there's no default-path regression and no credential/socket/system escalation.

Recommendation: request-changes — not for a default defect, but because this moves a trust boundary the bind hardening deliberately set, and that should be an explicit, documented decision plus a regression test. Summary (details inline):

1. (High) The writable grant applies to catalog/profile-controlled source paths. Once an operator sets MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS=/root, any catalog/profile can name a writable source anywhere under /root. The hardening's sanctioned writable carve-out was a gateway-chosen source (the embeddings dataDir), not a catalog-chosen path under an operator root. The denylist floors the blast radius away from credentials/system, but …/project/.git/hooks (not denylisted) → write → code-exec on next build is in scope. Preferred fix: gate writability on source provenance or an exact-path allowlist rather than a prefix root.
2. (Docs) The env var is undocumented. Please spell out that opting a root in means trusting every catalog/profile you load to write anywhere under it; recommend a dedicated data dir, not a source tree.
3. (Test gap) No negative test that the denylist still blocks writable binds. The denylist-before-writable ordering is load-bearing and correct today but untested — see the inline ask on clientpool_test.go.

Lower priority (no inline — the code is unchanged by this PR): symlink resolution happens at validation (resolveDockerHostPath → EvalSymlinks), but a non-existent leaf is re-appended literally, leaving a resolve-then-mount gap. Pre-existing, but this PR escalates its consequence from a read-only to a writable mount for paths under a writable root. Worth a follow-up (re-validate immediately before docker run, or reject a synthesized missing leaf on the writable path).

CI note: the two TestValidateDockerVolumeBinds/rejects_relative_* failures I saw locally are a /tmp-checkout artifact — they fail identically on main and are green in real CI. Not from this PR.

[saucow]

(High) Writable grant keys on a catalog-controlled source path.

bind.sourcePath here originates from catalog/profile volumes (clientpool.go:268 / :418), which the bind hardening treats as attacker-influenceable. Once an operator sets MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS=/root, any catalog/profile can name a writable source anywhere under /root — e.g. /root/project/.git/hooks (not on the denylist) → write → code execution on the next build.

The denylist at :57 still blocks /etc, ~/.ssh, docker.sock even when writable (verified), so the blast radius is floored away from credentials/system paths. But this still moves the trust boundary the hardening set: writable was meant for a gateway-chosen source (the embeddings dataDir, which bypasses this validator), not a catalog-chosen path under an operator root — and an operator is unlikely to realize the grant covers every catalog source under that root.

Consider gating writability on source provenance (only a gateway-selected source) or an exact-path allowlist (source == target full match) rather than a prefix root, so a catalog can't pick an arbitrary sub-path under a trusted parent. At minimum, see the docs ask above.

[kgprs]

Addressed in afa00206. MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS now matches only exact resolved host source paths instead of prefix roots, so listing a trusted parent no longer grants arbitrary child paths writable access. The docs now call out the catalog/profile trust implication as well.

[saucow]

A no-mode bind under a writable root is silently promoted to :rw here. I realize this is load-bearing for the {{filesystem.paths|volume|into}} case (it expands to host:host with no mode), so I'm not suggesting you require an explicit :rw token outright — that would defeat the PR's goal.

The implication to be aware of: a catalog supplying volumes: ['/root/victim:/data'] with no mode also becomes writable, not just the server you intended to grant. And dockerBindWritableAllowedRoots() is process-global (both call sites share it, no per-server scoping), so one writable root grants every server's no-mode binds under it. If you keep silent promotion, scoping the writable-roots set per-server would tighten this, and either way it's worth calling out in the env-var docs.

[kgprs]

Addressed in afa00206. No-mode promotion to :rw now happens only when the resolved bind source exactly matches a configured writable path. Child paths under a listed parent do not inherit writable access, and configured writable paths do not allow child paths pins that behavior.

[saucow]

This env var currently has no documentation outside this comment (the docs build doesn't reference it). Since it relaxes the read-only hardening, it deserves operator-facing docs that make the trust implication explicit:

Setting MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS means any catalog/profile-supplied volume source that falls under one of these roots will be mounted writable — i.e. you are trusting every catalog you load not to name a malicious source under that root.

Recommend advising operators to scope writable roots to a dedicated data directory rather than a project/source tree.

[kgprs]

Addressed in afa00206. Added a Host Bind Mount Safety section to docs/server-entry-spec.md covering default read-only behavior, exact-path writable entries, the OS path-list separator, and the recommendation to use dedicated data directories rather than source or broad project directories.

[saucow]

These new subtests cover the happy paths. The highest-risk property of this PR is that the sensitive-path denylist still wins over the writable allowance (the :57-before-:60 ordering), and that isn't tested — a future reorder could silently re-open writable mounts of /etc/~/.ssh/docker.sock with no failing test.

Could you add negative subtests with t.Setenv(dockerBindWritableAllowedPathsEnv, root) asserting these stay blocked even under a writable root:

• filepath.Join(root, ".ssh") + ":/ssh:rw" → ErrorContains credential path
• a symlink root/etc-link → /etc mounted :rw → ErrorContains sensitive system path
• /var/run/docker.sock:/var/run/docker.sock:rw → still blocked

All three pass today, so they're safe to add and they lock the denylist-before-writable ordering against regressions.

[kgprs]

Addressed in afa00206. Added negative subtests for a credential path, a symlink resolving to /etc, and /var/run/docker.sock while MCP_GATEWAY_DOCKER_BIND_ALLOW_WRITABLE_PATHS is set, so the denylist-before-writable ordering is now covered.

[saucow]

Re-reviewed — this resolves the findings, and the security property is actually tightened:

• The writable grant is now an exact-path allowlist (slices.Contains on the resolved source) instead of a prefix root, so a catalog/profile can no longer name an arbitrary child under a trusted parent.
• The new negative tests (credential path, symlink→/etc, docker.sock, and child-path rejection — all under a configured writable path) lock the denylist-before-writable ordering.
• The docs now spell out the exact-match semantics and the trust implication.

Verified locally: the bind tests pass; the only failing subtests are the pre-existing /tmp-checkout relative-path artifacts, which fail identically on main and are unrelated to this PR. Approving.