Skip to content

fix: false error of Windows path when binding the host path to the sandbox.#42174

Merged
BradGroux merged 13 commits intoopenclaw:mainfrom
6607changchun:fix-windowsbind
May 5, 2026
Merged

fix: false error of Windows path when binding the host path to the sandbox.#42174
BradGroux merged 13 commits intoopenclaw:mainfrom
6607changchun:fix-windowsbind

Conversation

@6607changchun
Copy link
Copy Markdown
Contributor

@6607changchun 6607changchun commented Mar 10, 2026

Summary

Describe the problem and fix in 2–5 bullets:

  • Problem: The drive letter of the path on Windows does not start with the slash, leading to false error of the sandbox security. So I added the passby branch to avoid it. When launching the gateway with the custom bindings on Windows, the drive letter is recoginzed as a relative path, leading to false error of the sand security.
  • Why it matters: This bug affects the usage of the custom binding features on Windows, which causes the custom binding feature to be completely unusable on Windows, as absolute Windows paths (e.g., D:/path) are incorrectly rejected by the security check.
  • What changed: Adding the branch of Windows path support to the sandbox security validation.
  • What did NOT change (scope boundary): The current permission check and the posix-like path validation are not changed.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #
  • Related #

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.
The host path on Windows can be binded to the sandbox.

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: Windows 11 26200.7840
  • Runtime/container: Local Machine for the host, and Docker Desktop 4.34.3 with WSL2 backend for docker.
  • Model/provider: deepseek-V3.2 reasoner
  • Integration/channel (if any):
  • Relevant config (redacted):

Steps

"agents": {
    "defaults": {
      "sandbox": {
        "mode": "all",
        "scope": "session",
        "workspaceAccess": "ro",
        "docker": {
          "image": "192.168.1.102:8080/library/ubuntu:latest",
          "network": "bridge",
          "binds": ["D:/data/openclaw/src:/src:ro", "D:/data/openclaw/output:/output:rw"]
        }
      }
    }
  },
  1. Modify the openclaw.json like the above fragment.
  2. Launch the gateway

Expected

  • The gateway starts successfully, and the skills are able to executed in the docker container with proper binding.

Actual

  • The gateway fails to start.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

before

🦞 OpenClaw 2026.3.8 (3a12cf5) — One CLI to rule them all, and one more restart because you changed the port.


◇ Config ───────────────────────────────────────────────────╮
│ │
│ Config invalid; doctor will run with best-effort config. │
│ │
├────────────────────────────────────────────────────────────╯
Config invalid
File: ~.openclaw\openclaw.json
Problem:

  • agents.defaults.sandbox.docker.binds.0: Sandbox security: bind mount "D:/data/openclaw/src:/src:ro" uses a non-absolute source path "D". Only absolute POSIX paths are supported for sandbox binds.
  • agents.defaults.sandbox.docker.binds.1: Sandbox security: bind mount "D:/data/openclaw/output:/output:rw" uses a non-absolute source path "D". Only absolute POSIX paths are supported for sandbox binds.

Run: openclaw doctor --fix
Gateway aborted: config is invalid.
agents.defaults.sandbox.docker.binds.0: Sandbox security: bind mount "D:/data/openclaw/src:/src:ro" uses a non-absolute source path "D". Only absolute POSIX paths are supported for sandbox binds.
agents.defaults.sandbox.docker.binds.1: Sandbox security: bind mount "D:/data/openclaw/output:/output:rw" uses a non-absolute source path "D". Only absolute POSIX paths are supported for sandbox binds.
Fix the config and retry, or run "openclaw doctor" to repair.

after

🦞 OpenClaw 2026.3.8 (3a12cf5) — We ship features faster than Apple ships calculator updates.

Restarted Scheduled Task: OpenClaw Gateway

Human Verification (required)

What you personally verified (not just CI), and how: I put some files in the src directory, and then launched the gateway and tested the agent with several prompts. The agent successfully obtained these files and read the correct content of the files. Besides, I have checked the container, the src and output directory are correctly binded. The src directory is read-only and the output directory is read-write. The original permission check is not corrupted.

  • Verified scenarios: Agent successfully reads files from the read‑only /src directory. Agent writes files to the read‑write /output directory and the files persist on the host. Inside the container, /src is mounted as ro and /output as rw (confirmed via docker inspect). Bind mounts using D:/... syntax are accepted by the security validation and work as expected.
  • Edge cases checked:
  • What you did not verify: The regression testing of the original posix-like path validation as this patch is only a side-enhancement of the original conditions.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: Directly revert this commit and reinstall.
  • Files/config to restore: None
  • Known bad symptoms reviewers should watch for: Windows paths of custom bindings will be rejected (if regression occurs)

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.
None

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Mar 10, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27f22a17dd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/zod-schema.agent-runtime.ts Outdated
Comment thread src/config/zod-schema.agent-runtime.ts Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 10, 2026

Greptile Summary

This PR attempts to fix a false-positive sandbox security error that rejects Windows-style drive-letter paths (e.g., D:/data/openclaw/src) as non-absolute paths when configuring Docker bind mounts on Windows.

What works:

  • The change to validate-sandbox-security.ts (getBlockedBindReason) is sound. It correctly delegates to the existing parseBindSourcePath utility and adds a Windows path check alongside the POSIX path check.

What is broken:

  • The change to zod-schema.agent-runtime.ts contains a critical bug in the colonCount loop (lines 151-156). The loop attempts to count colons by calling bind.indexOf(bindIndex) where bindIndex is a loop counter (number), which causes JavaScript to coerce it to a string and search for digit characters ("0", "1", etc.) rather than the ":" character. Combined with a type mismatch in the comparison (=== ':'), the loop always results in colonCount = 0, so the Windows path detection logic never activates. The source path is extracted as just "D", the regex test fails, and the original Zod validation error is still raised for Windows paths.

Secondary issue (both files):

  • Both modified files use /^[A-Z]:/ (uppercase-only) for Windows drive-letter detection, while the existing bind-spec.ts utility uses /^[A-Za-z]:[\\/]/ (case-insensitive). Lowercase drive letters like d:/path would still be rejected.

Result: The Zod validation layer (which produces the gateway-startup error users encounter) remains broken despite the attempted fix. Only the runtime enforcement path is improved.

Confidence Score: 1/5

  • Not safe to merge — the Zod config-validation layer (where the user-facing gateway-startup error originates) remains broken despite the attempted fix.
  • The critical issue is in zod-schema.agent-runtime.ts, which is the code path that produces the original error users reported. The colonCount loop contains compounding bugs that make it non-functional, so Windows paths will still fail Zod validation at gateway startup. While validate-sandbox-security.ts is correctly implemented, it only guards the runtime enforcement path, not the config-load-time validation that prevents the gateway from starting. The PR's stated goal (accepting Windows paths at config load time) is not achieved.
  • src/config/zod-schema.agent-runtime.ts — the colonCount logic must be rewritten or replaced with the existing splitSandboxBindSpec utility.

Last reviewed commit: 27f22a1

Comment thread src/config/zod-schema.agent-runtime.ts Outdated
Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d49d4b883

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/config/zod-schema.agent-runtime.ts Outdated
Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1093fdf7c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f535f36932

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
@6607changchun
Copy link
Copy Markdown
Contributor Author

Oh no! Help!
I think that some previous merged PRs have corrupted the environment. The locations of the error log are never changed by my commits and there is no related issues with that in my commits.
I have no idea why this problems occur as the 2edab13 almost succeed except for the linter.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ebb1b9dcde

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
Comment thread src/agents/sandbox/validate-sandbox-security.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54eee3fbbd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/agents/sandbox/host-paths.ts Outdated
@openclaw-barnacle
Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 27, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 27, 2026

Codex review: found issues before merge.

Summary
The PR changes sandbox Docker bind parsing and validation so Windows drive-letter host sources can pass config and runtime sandbox checks.

Reproducibility: yes. source-level. The PR body gives Windows 11 Docker Desktop steps, and current main still reduces D:/... to D in schema validation while runtime validation rejects non-/ sources; I did not run a live Windows gateway in this read-only review.

Next step before merge
Needs secops/maintainer policy review because this changes sandbox host-path admission and overlaps active Windows bind-security work in #63074.

Security
Needs attention: The patch expands sandbox bind-source admission and leaves Windows path policy comparisons case-sensitive.

Review findings

  • [P1] Compare Windows sandbox policy paths case-insensitively — src/agents/sandbox/host-paths.ts:33
Review details

Best possible solution:

Centralize a Windows-aware sandbox bind policy that parses bind specs once, admits only drive-absolute host sources, compares policy keys with Windows case semantics, preserves Docker-facing paths, and covers config validation, runtime validation, audit, Docker create args, tests, and changelog.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level. The PR body gives Windows 11 Docker Desktop steps, and current main still reduces D:/... to D in schema validation while runtime validation rejects non-/ sources; I did not run a live Windows gateway in this read-only review.

Is this the best way to solve the issue?

No. The direction is useful, but this patch spreads Windows-path handling across schema/runtime helpers and does not give sandbox policy comparisons Windows path semantics; a shared policy helper is safer.

Full review comments:

  • [P1] Compare Windows sandbox policy paths case-insensitively — src/agents/sandbox/host-paths.ts:33
    After this PR admits drive-letter bind sources, normalizeSandboxHostPath only normalizes separators and the drive letter while blocked credential roots and allowed roots are still checked with case-sensitive prefix logic. On Windows, casing variants can refer to the same directory, so paths like c:/Users/Alice/.ssh can miss a C:/Users/Alice/.ssh deny entry unless policy comparisons use a Windows-aware key while preserving the Docker-facing path.
    Confidence: 0.78

Overall correctness: patch is incorrect
Overall confidence: 0.84

Security concerns:

  • [medium] Windows path case can bypass bind policy checks — src/agents/sandbox/host-paths.ts:33
    Once drive-letter paths are accepted, blocked credential roots and allowed roots are compared with POSIX-style case-sensitive prefixes. Windows paths are normally case-insensitive, so equivalent paths with different segment casing can miss blocked-home or allowed-root checks.
    Confidence: 0.78

Acceptance criteria:

  • pnpm test src/config/config.sandbox-docker.test.ts src/agents/sandbox/bind-spec.test.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts
  • pnpm exec oxfmt --check --threads=1 src/config/zod-schema.agent-runtime.ts src/agents/sandbox/bind-spec.ts src/agents/sandbox/host-paths.ts src/agents/sandbox/validate-sandbox-security.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts

What I checked:

  • Current config validation still mis-parses Windows drive binds: SandboxDockerSchema uses bind.indexOf(":"), so a bind like D:/data/openclaw/src:/src:ro is reduced to source D and fails the absolute-source check before gateway startup. (src/config/zod-schema.agent-runtime.ts:161, 8a1e22027344)
  • Current runtime validation remains POSIX-only: getBlockedBindReason returns non_absolute for any parsed source that does not start with /, so current main does not already implement Windows drive-letter bind support. (src/agents/sandbox/validate-sandbox-security.ts:117, 8a1e22027344)
  • Lower-level parser already preserves Windows bind syntax: splitSandboxBindSpec skips a C: drive-letter prefix when finding the host/container separator, which is the right primitive for config and runtime parsing. (src/agents/sandbox/bind-spec.ts:27, 8a1e22027344)
  • Current tests document Windows rejection: The Windows branch of the symlink-escape test asserts that Windows source paths are rejected as non-POSIX, so the PR changes an existing security contract rather than only fixing a typo. (src/agents/sandbox/validate-sandbox-security.test.ts:195, 8a1e22027344)
  • Sandbox policy still uses case-sensitive prefix checks: Blocked paths and allowed roots are compared with exact string equality/prefix logic; once drive-letter sources are admitted, Windows-equivalent casing variants can be classified incorrectly unless a Windows-aware policy key is used. (src/agents/sandbox/validate-sandbox-security.ts:144, 8a1e22027344)
  • Sandbox code is security-owned: CODEOWNERS routes src/agents/sandbox/ and sandbox docs to @openclaw/openclaw-secops, which is relevant because this PR changes which host paths can be mounted into sandbox containers. (.github/CODEOWNERS:31, 8a1e22027344)

Likely related people:

  • openclaw/openclaw-secops: CODEOWNERS explicitly owns the sandbox source and sandbox docs affected by host bind admission policy. (role: CODEOWNERS owner; confidence: high; files: .github/CODEOWNERS, src/agents/sandbox/, docs/gateway/sandboxing.md)
  • steipete: Git history shows Peter Steinberger landed the original sandbox binds work, tightened sandbox bind validation, and owns the current file snapshot in this partial checkout. (role: introduced behavior and recent maintainer; confidence: high; commits: bbc34215a240, a7cbce1b3de2, ca620fbd4f9f; files: src/config/zod-schema.agent-runtime.ts, src/agents/sandbox/validate-sandbox-security.ts, src/agents/sandbox/host-paths.ts)
  • jacobtomlinson: Jacob Tomlinson authored and reverted the recent sensitive external bind-source hardening in the same validator, so he is relevant context for allowed-root and credential-root policy. (role: adjacent security-path contributor; confidence: medium; commits: 8db20c196598, 14a779ee8db9; files: src/agents/sandbox/validate-sandbox-security.ts, src/agents/sandbox/validate-sandbox-security.test.ts)

Remaining risk / open question:

  • Windows-equivalent path casing can bypass or over-enforce sandbox allow/deny roots after drive-letter sources are admitted.
  • The PR changes sandbox host-path admission without regression coverage for Windows credential roots, allowed roots, symlink/junction canonicalization, config validation, and audit output.
  • fix(security): classify dangerous Windows sandbox binds first #63074 is still open and only covers Windows dangerous-bind classification while preserving generic Windows rejection, so the desired Windows bind policy needs reconciliation before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8a1e22027344.

6607changchun and others added 8 commits May 4, 2026 23:23
…ndbox container.

The drive letter of the path in Windows does not start with the slash, leading to false error of the sandbox security. So I added the passby branch to avoid it.
replace with existing function

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
adding lowercase drive letter support

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@BradGroux BradGroux requested a review from a team as a code owner May 5, 2026 04:26
@openclaw-barnacle openclaw-barnacle Bot added docker Docker and sandbox tooling size: S and removed size: XS labels May 5, 2026
@BradGroux
Copy link
Copy Markdown
Member

Maintainer prep update:

  • Rebased onto current main and pushed the prepared branch at 079b675bbd7f.
  • Reworked the Windows bind handling so config/runtime validation share drive-absolute source detection.
  • Added Windows-aware sandbox policy comparison keys so blocked paths and allowed roots compare case-insensitively for drive-letter paths while preserving Docker-facing bind paths.
  • Added regression coverage for Windows drive-letter config validation, drive-relative rejection, allowed-root casing, audit handling, and host-path policy keys.
  • Added the required changelog entry with attribution.

Local verification after the final rebase:

  • pnpm test src/config/config.sandbox-docker.test.ts src/agents/sandbox/bind-spec.test.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts
  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/config/zod-schema.agent-runtime.ts src/agents/sandbox/bind-spec.ts src/agents/sandbox/host-paths.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts
  • pnpm check:changed

I also ran the full maintainer wrapper gate. pnpm build and pnpm check passed, but full pnpm test hit unrelated baseline failures outside this PR's touched surface: managed child process timeout, gateway session-utils timeout, disabled compat endpoint assertion, plugin registry snapshot assertions, and a Zalo temp-dir ENOTEMPTY. I am leaving this open for GitHub required checks before merge.

@openclaw-barnacle openclaw-barnacle Bot added the channel: whatsapp-web Channel integration: whatsapp-web label May 5, 2026
@BradGroux BradGroux merged commit d02fbc6 into openclaw:main May 5, 2026
109 of 110 checks passed
@BradGroux
Copy link
Copy Markdown
Member

Merged after maintainer prep.

  • Prepared head: 79428188bdb2bb59d382a7a4985a4d7e7d91b8f7
  • Merge commit: d02fbc6116ed9cbd501ad6a1e4d08f3fc71c1dd8
  • GitHub checks: green at the prepared head before merge
  • Local targeted verification: pnpm test src/config/config.sandbox-docker.test.ts src/agents/sandbox/bind-spec.test.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts; pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/config/zod-schema.agent-runtime.ts src/agents/sandbox/bind-spec.ts src/agents/sandbox/host-paths.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.ts src/agents/sandbox/validate-sandbox-security.test.ts src/security/audit-sandbox-docker-config.test.ts; pnpm check:changed; pnpm lint:extensions; pnpm lint:extensions:bundled
  • Maintainer wrapper: scripts/pr-prepare gates 42174 completed build/check, then hit unrelated full-test baseline failures outside this PR surface; CI was used as the merge gate after the targeted local pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling channel: whatsapp-web Channel integration: whatsapp-web docker Docker and sandbox tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants