fix(sandbox): make /workspace bind mount read-only when workspaceAcce… by guanyu-zhang · Pull Request #32227 · openclaw/openclaw

guanyu-zhang · 2026-03-02T22:23:32Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: When workspaceAccess is set to 'ro' or 'none', the /workspace bind mount is still writable instead of read-only
Why it matters: Users expect sandbox workspace to be read-only in ro/none modes for security isolation, as documented
What changed: Fixed the condition for adding :ro suffix - now simply checks if workspaceAccess is not 'rw'
What did NOT change (scope boundary): The /agent mount behavior remains unchanged

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

When workspaceAccess is 'ro' or 'none', the sandbox workspace (/workspace inside container) is now read-only instead of writable. This matches the documented behavior.

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) Yes - agents cannot write to /workspace in ro/none modes
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation: This is a security improvement - reduces attack surface by making sandbox workspace read-only as documented

Repro + Verification

Environment

OS: Linux
Runtime/container: Docker
Model/provider: N/A
Integration/channel (if any): sandbox
Relevant config (redacted): workspaceAccess: "ro"

Steps

Configure agent with workspaceAccess: "ro" in openclaw.json
Start sandbox container
Try to write to /workspace: echo "test" > /workspace/test.txt
Check mount: docker inspect --format '{{json .Mounts}}'

Expected

Write should fail (read-only)
Mount shows RW: false

Actual (before fix)

Write succeeds (writable)
Mount shows RW: true

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets: docker inspect shows RW: true for /workspace
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: Confirmed that with workspaceAccess: "ro", /workspace is still writable
Edge cases checked: Verified all three modes (rw, ro, none)
What you did not verify: Need to rebuild container to verify fix works

Compatibility / Migration

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

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert the code change
Files/config to restore: src/agents/sandbox/docker.ts
Known bad symptoms reviewers should watch for: N/A

Risks and Mitigations

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

Risk: None - this is a security improvement
- Mitigation: N/A

…ss is not rw This ensures that when workspaceAccess is set to 'ro' or 'none', the sandbox workspace (/workspace inside the container) is mounted as read-only, matching the documented behavior. Previously, the condition was: workspaceAccess === 'ro' && workspaceDir === agentWorkspaceDir This was always false in 'ro' mode because workspaceDir equals sandboxWorkspaceDir, not agentWorkspaceDir. Now the logic is simplified: - 'rw': /workspace is writable - 'ro': /workspace is read-only - 'none': /workspace is read-only

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ff002a38c

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-02T22:28:48Z

src/agents/sandbox/docker.ts

  args.push("--workdir", cfg.workdir);
-  const mainMountSuffix =
-    params.workspaceAccess === "ro" && workspaceDir === params.agentWorkspaceDir ? ":ro" : "";
+  const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";


Force container recreation after mount policy change

This mount-mode change only applies when a new container is created, but ensureSandboxContainer reuses existing containers when computeSandboxConfigHash(...) is unchanged; since the hash inputs are config-only, upgrading to this commit leaves already-created session/agent/shared containers with the old writable /workspace mount and the ro/none hardening is silently skipped until manual sandbox recreate. Please version this behavior change (for example via a security epoch in the hash input) so existing containers are automatically rebuilt.

Useful? React with 👍 / 👎.

greptile-apps · 2026-03-02T22:29:51Z

Greptile Summary

This PR correctly fixes a security bug where /workspace bind mounts were writable when they should be read-only in ro and none modes. The fix simplifies the logic and makes it consistent with the expectations of fs-paths.ts (line 65) and fs-bridge.ts (line 488).

Key changes:

Changed condition from workspaceAccess === "ro" && workspaceDir === agentWorkspaceDir to workspaceAccess === "rw" ? "" : ":ro"
The old condition was buggy because when workspaceAccess === "ro", workspaceDir is sandboxWorkspaceDir (not agentWorkspaceDir), so the mount was never read-only

Critical issue:

The identical bug exists in src/agents/sandbox/browser.ts at lines 240-243 and must be fixed for consistency

Confidence Score: 3/5

This PR fixes a security issue but the fix is incomplete
The fix correctly addresses the bug in docker.ts, but the identical bug exists in browser.ts and needs to be fixed. The incomplete fix means browser sandboxes still have the security vulnerability. Once browser.ts is also fixed, this would be safe to merge.
The fix should also be applied to src/agents/sandbox/browser.ts (lines 240-243)

_{Last reviewed commit: 5ff002a}

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-03-02T22:29:55Z

src/agents/sandbox/docker.ts

  args.push("--workdir", cfg.workdir);
-  const mainMountSuffix =
-    params.workspaceAccess === "ro" && workspaceDir === params.agentWorkspaceDir ? ":ro" : "";
+  const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";


same bug exists in browser.ts lines 240-243 and needs the same fix

Suggested change

const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";

const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";

The identical buggy logic exists in src/agents/sandbox/browser.ts at lines 240-243. It should also be changed from:

const mainMountSuffix = params.cfg.workspaceAccess === "ro" && params.workspaceDir === params.agentWorkspaceDir ? ":ro" : "";

to:

const mainMountSuffix = params.cfg.workspaceAccess === "rw" ? "" : ":ro";

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/sandbox/docker.ts Line: 455 Comment: same bug exists in `browser.ts` lines 240-243 and needs the same fix ```suggestion const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro"; ``` The identical buggy logic exists in `src/agents/sandbox/browser.ts` at lines 240-243. It should also be changed from: ```typescript const mainMountSuffix = params.cfg.workspaceAccess === "ro" && params.workspaceDir === params.agentWorkspaceDir ? ":ro" : ""; ``` to: ```typescript const mainMountSuffix = params.cfg.workspaceAccess === "rw" ? "" : ":ro"; ``` How can I resolve this? If you propose a fix, please make it concise.

…ad-only when workspaceAccess is not rw The identical buggy logic from docker.ts also exists in browser.ts. Applying the same fix here.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed98763bff

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-02T22:48:50Z

src/agents/sandbox/browser.ts

-      params.cfg.workspaceAccess === "ro" && params.workspaceDir === params.agentWorkspaceDir
-        ? ":ro"
-        : "";
+    const mainMountSuffix = params.cfg.workspaceAccess === "rw" ? "" : ":ro";


Bump browser hash epoch for mount-mode change

Changing the browser container mount to :ro here does not force existing sandbox browser containers to be recreated, because reuse is still gated by computeSandboxBrowserConfigHash(...) and SANDBOX_BROWSER_SECURITY_HASH_EPOCH, neither of which changed in this commit. In upgrades where a browser container already exists, the old writable /workspace mount is kept and this hardening silently does not apply until a manual recreate, which leaves the intended security fix ineffective for active installs.

Useful? React with 👍 / 👎.

)

steipete · 2026-03-02T22:59:48Z

Follow-up landed in #32274 to complete maintainer fixups after this PR merged during update:

Added regression tests for workspace mount mode behavior in both sandbox create paths:
- src/agents/sandbox/browser.create.test.ts
- src/agents/sandbox/docker.config-hash-recreate.test.ts
Added changelog entries for fix(sandbox): make /workspace bind mount read-only when workspaceAcce… #32227 and fix(signal): fall back to toolContext.currentMessageId for reactions #32217.

This keeps the merged behavior fully covered and changelog-complete.

…uanyu-zhang

…uanyu-zhang)

…uanyu-zhang

…uanyu-zhang)

…uanyu-zhang

…uanyu-zhang)

guanyu-zhang mentioned this pull request Mar 2, 2026

[Question] sandbox workspace bind mount behavior when workspaceAccess is ro or none #32231

Closed

chatgpt-codex-connector bot reviewed Mar 2, 2026

View reviewed changes

greptile-apps bot reviewed Mar 2, 2026

View reviewed changes

openclaw-barnacle bot added agents Agent runtime and tooling size: XS labels Mar 2, 2026

Evan and others added 2 commits March 2, 2026 22:33

fix(sandbox): same fix for browser.ts - make /workspace bind mount re…

71a8352

…ad-only when workspaceAccess is not rw The identical buggy logic from docker.ts also exists in browser.ts. Applying the same fix here.

Merge branch 'main' into fix/sandbox-workspace-readonly

ed98763

chatgpt-codex-connector bot reviewed Mar 2, 2026

View reviewed changes

steipete merged commit 7cbcbbc into openclaw:main Mar 2, 2026
19 of 21 checks passed

steipete added a commit that referenced this pull request Mar 2, 2026

fix: enforce sandbox workspace mount mode (#32227) (thanks @guanyu-zhang

4f9ffea

)

steipete mentioned this pull request Mar 2, 2026

test/changelog: follow up sandbox workspace mount fixes #32274

Merged

steipete added a commit that referenced this pull request Mar 2, 2026

fix: enforce sandbox workspace mount mode (#32227) (thanks @guanyu-zhang

02eeb08

)

steipete mentioned this pull request Mar 2, 2026

fix(signal): fall back to toolContext.currentMessageId for reactions #32217

Merged

dawi369 pushed a commit to dawi369/davis that referenced this pull request Mar 3, 2026

fix: enforce sandbox workspace mount mode (openclaw#32227) (thanks @g…

4b06fb4

…uanyu-zhang)

OWALabuy pushed a commit to kcinzgg/openclaw that referenced this pull request Mar 4, 2026

fix: enforce sandbox workspace mount mode (openclaw#32227) (thanks @g…

8e09561

…uanyu-zhang)

zooqueen pushed a commit to hanzoai/bot that referenced this pull request Mar 6, 2026

fix: enforce sandbox workspace mount mode (openclaw#32227) (thanks @g…

a464897

…uanyu-zhang)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sandbox): make /workspace bind mount read-only when workspaceAcce…#32227

fix(sandbox): make /workspace bind mount read-only when workspaceAcce…#32227
steipete merged 3 commits intoopenclaw:mainfrom
guanyu-zhang:fix/sandbox-workspace-readonly

guanyu-zhang commented Mar 2, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 2, 2026

Uh oh!

greptile-apps bot commented Mar 2, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Mar 2, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 2, 2026

Uh oh!

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";
	const mainMountSuffix = params.workspaceAccess === "rw" ? "" : ":ro";

Uh oh!

Conversation

guanyu-zhang commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual (before fix)

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

guanyu-zhang commented Mar 2, 2026 •

edited

Loading