fix(sandbox): allow mkdirp safety checks on existing directory targets by Sid-Qin · Pull Request #28863 · openclaw/openclaw

Sid-Qin · 2026-02-27T15:25:38Z

Summary

Problem: Sandbox boundary checks for directory creation use file-open validation semantics and can reject already-existing in-workspace directories as non-file.
Why it matters: Write flows that prepare directories can fail with boundary-check errors even when path is valid and writable (write fails while edit works on same file path).
What changed: In src/agents/sandbox/fs-bridge.ts, added allowNonFileTarget path-safety option and enabled it for mkdirp. assertPathSafety now allows validation failures for this specific mode while still enforcing canonical mount containment and writable mount checks.
What did NOT change: Alias escape checks, canonical mount-root checks, writable mount enforcement, write/remove/rename safety behavior.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: write fails sandbox boundary check on existing path while edit succeeds #28745

User-visible / Behavior Changes

mkdir -p style operations inside sandbox no longer fail on already-existing valid workspace directories.
Subsequent write flows that depend on directory creation are consistent with edit behavior.

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Linux sandbox host
Runtime: Docker-based sandbox bridge
Integration/channel: tools.fs write/mkdir paths in sandbox mode

Steps

Create an existing in-workspace directory path (e.g., memory/state).
Trigger sandbox mkdirp on the same path.

Expected

Path safety should pass and mkdirp should succeed/no-op.

Actual

Before fix: boundary check failed before mkdirp for existing directory target.
After fix: mkdirp succeeds while mount-root and writable checks remain enforced.

Evidence

Automated checks passed:

pnpm -C /Users/sidqin/Desktop/openclaw-contrib exec vitest run src/agents/sandbox/fs-bridge.test.ts
pnpm -C /Users/sidqin/Desktop/openclaw-contrib exec tsc --noEmit --pretty false

Added regression test:

allows mkdirp on an already-existing directory inside workspace in src/agents/sandbox/fs-bridge.test.ts

Human Verification (required)

Verified scenarios: mkdirp on existing directory; baseline bridge operations still pass.
Edge cases checked: path still must map inside canonical allowed mount and writable mount.
What I did not verify: full live e2e with upstream reporter environment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert: Revert this commit.
Files/config to restore: src/agents/sandbox/fs-bridge.ts, src/agents/sandbox/fs-bridge.test.ts
Known bad symptoms: false-positive boundary rejections for valid existing directories may return.

Risks and Mitigations

Low risk. Relaxation is scoped to mkdirp path precheck only, and canonical mount + writable checks still run for every call.

Sandbox mkdirp validated target paths with file-open boundary checks that expect regular files. When the target directory already existed, validation returned non-file and mkdirp failed before execution, even though the path was inside writable workspace mounts. Allow non-file validation outcomes for mkdirp safety checks while keeping canonical mount-root and writable checks intact. Closes openclaw#28745

greptile-apps · 2026-02-27T15:29:49Z

Greptile Summary

This PR fixes a bug where mkdirp operations failed on already-existing directories due to file-open validation semantics in boundary checks.

Key changes:

Added allowNonFileTarget option to PathSafetyOptions (src/agents/sandbox/fs-bridge.ts:26)
Enabled this option for mkdirp operations (src/agents/sandbox/fs-bridge.ts:139)
Modified assertPathSafety to allow validation failures when allowNonFileTarget is true (src/agents/sandbox/fs-bridge.ts:268-270)
Added regression test verifying mkdirp succeeds on existing directories (src/agents/sandbox/fs-bridge.test.ts:176-198)

Security preserved:
All critical security boundaries remain enforced for mkdirp:

Canonical mount containment checks (verifies path stays within allowed mounts after resolving symlinks)
Writable mount checks (ensures path is in a writable mount)
Alias escape checks (via aliasPolicy parameter)

The only validation skipped is the file-open check from openBoundaryFile, which appropriately allows existing directories for mkdir -p semantics.

Confidence Score: 5/5

This PR is safe to merge with no security concerns
The fix is minimal, well-targeted, and maintains all security boundaries. The change only affects mkdirp operations and correctly allows existing directories while still enforcing canonical mount containment, writable mount checks, and alias escape checks. The test coverage is adequate and directly verifies the bug fix.
No files require special attention

_{Last reviewed commit: 6ae9962}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ae996295b

ℹ️ 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-02-27T15:29:58Z

src/agents/sandbox/fs-bridge.ts

+      const allowedNonFile = guarded.reason === "validation" && options.allowNonFileTarget === true;
+      if (!allowedMissing && !allowedNonFile) {


Restrict mkdirp non-file bypass to directory targets

allowNonFileTarget currently accepts all validation failures from openBoundaryFile, but validation also includes boundary/alias violations from resolveBoundaryPath (not just “target is an existing directory”); as a result, mkdirp can now proceed after symlink-escape validation failures and only rely on later container-path checks, which changes the previous host-boundary behavior for alias paths. This should be narrowed so mkdirp ignores validation only when the target is a safe pre-existing directory, rather than blanket-allowing every validation error.

Useful? React with 👍 / 👎.

nikolasdehor

This and #28853 fix the same issue (#28745) with slightly different implementations. This one (allowNonFileTarget) is the cleaner approach. Heads up to maintainers — only one should be merged.

steipete · 2026-03-02T23:34:37Z

Thanks for the PR! Multiple PRs address the same sandbox mkdirp safety check (#28863, #30521, #31478). Keeping #30521 which has the most refined approach. Closing this one to reduce noise. This is an AI-assisted triage review. If we got this wrong, feel free to reopen or start a new PR — happy to revisit.

openclaw-barnacle bot added agents Agent runtime and tooling size: XS experienced-contributor labels Feb 27, 2026

chatgpt-codex-connector bot reviewed Feb 27, 2026

View reviewed changes

nikolasdehor reviewed Feb 27, 2026

View reviewed changes

steipete mentioned this pull request Mar 2, 2026

fix(sandbox): allow mkdirp for directories within workspace boundary #31478

Closed

steipete closed this Mar 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sandbox): allow mkdirp safety checks on existing directory targets#28863

fix(sandbox): allow mkdirp safety checks on existing directory targets#28863
Sid-Qin wants to merge 1 commit intoopenclaw:mainfrom
Sid-Qin:fix/28745-write-boundary-consistency

Sid-Qin commented Feb 27, 2026 •

edited

Loading

Uh oh!

greptile-apps bot commented Feb 27, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Uh oh!

nikolasdehor left a comment

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const allowedNonFile = guarded.reason === "validation" && options.allowNonFileTarget === true;
		if (!allowedMissing && !allowedNonFile) {

Uh oh!

Conversation

Sid-Qin commented Feb 27, 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

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps bot commented Feb 27, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

nikolasdehor left a comment

Choose a reason for hiding this comment

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sid-Qin commented Feb 27, 2026 •

edited

Loading