[security] fix(container): prevent host file read/delete via container-controlled outbox paths#2001
Merged
gavrielc merged 2 commits intoApr 30, 2026
Conversation
dim0627
added a commit
to dim0627/nanoclaw
that referenced
this pull request
Apr 27, 2026
…ments Stacked on nanocoai#2001. That PR confines outbound attachment reads and post-delivery cleanup; this commit extends the same guard to the mirror-image inbound path. `extractAttachmentFiles` writes channel-supplied attachment bytes under `inbox/<messageId>/<filename>` via host-side `mkdirSync` + `writeFileSync`. Both `messageId` (carried through from the channel adapter — typically a platform-supplied id) and `att.name` (a free-form field a malicious user can craft in a Slack/Discord/etc. message) are untrusted. Without validation, a traversal sequence in either segment overwrites arbitrary host-writable files, and a compromised container with /workspace write access can pre-place a symlink at the inbox dir or filePath to redirect the write. Mirror the four defenses nanocoai#2001 added on the outbound side: - `isSafePathSegment(messageId)` before any inbox path is built. - `isSafePathSegment(filename)` before `att.name` is used as a path segment; preserve the host-generated `attachment-${Date.now()}` fallback for attachments with no name. - `lstatSync(inboxDir)` to refuse a pre-placed symlink before `mkdirSync` would silently follow it; `realpathSync` + `isPathInside` to assert the resolved dir still sits inside the session's inbox root. - `writeFileSync(..., { flag: 'wx' })` to refuse following a pre-existing symlink at the target file path or overwriting any existing file. Surface `EEXIST` as a logged skip. Tests (added under the existing `describe('session manager', ...)`): - traversal in `att.name` does not write to the resolved escape path; - a symlinked inbox subdir is rejected and the symlink target is left untouched; - a pre-existing symlink at the file path is not followed; - traversal in `messageId` materialises no inbox subtree; - safe basenames still land at `inbox/<msgId>/<name>`. `pnpm test` (23 files, 206 tests) and `pnpm exec tsc --noEmit` clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closed
6 tasks
This was referenced Apr 29, 2026
bcbed31 to
3502d9b
Compare
…ments
Mirrors the four defenses on the outbound side onto extractAttachmentFiles:
1. Reject unsafe messageId via isSafeAttachmentName before any inbox path
is built. WhatsApp passes msg.key.id through raw and that field is
client generated, so a peer can craft it; future end to end encrypted
adapters will have the same property.
2. lstatSync on the inbox dir refuses a pre placed symlink before
mkdirSync would silently follow it.
3. realpathSync + isPathInside contains the resolved dir under the
session inbox root.
4. writeFileSync uses the wx flag so a pre placed symlink at the file
path is refused atomically by the kernel; EEXIST surfaces as a
logged skip.
Threat: the session dir is mounted writable into the container at
/workspace, so a compromised agent can pre place inbox/<future msgId>/
as a symlink and wait for a chat message with a matching id to redirect
the host write. The four guards together close that window.
Consolidates with the existing isSafeAttachmentName helper from
attachment-safety.ts rather than introducing a duplicate basename
validator inside session-manager.
Co-Authored-By: Daisuke Tsuji <dim0627@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3502d9b to
fc3c11b
Compare
gavrielc
approved these changes
Apr 30, 2026
Collaborator
gavrielc
left a comment
There was a problem hiding this comment.
Rebased on current main, deduplicated path-segment helper against existing isSafeAttachmentName. Added a follow-up commit applying the same defenses to the inbound side, co-authored with Daisuke Tsuji from #2053. Tests green (234).
This was referenced May 1, 2026
mzazon
added a commit
to mzazon/nanoclaw
that referenced
this pull request
May 6, 2026
Brings in 202 upstream commits since our last sync. Key additions: - Circuit breaker for crash loop protection - Outbox path-confinement security fix (nanocoai#2001) - Inbound DB fresh-open fix for virtiofs/NFS (nanocoai#2160) - Orphan processing_ack cleanup (nanocoai#2151) - Pre-task scripts on follow-up poll injections (nanocoai#2114) - Attachment naming/safety refactor - Setup flow improvements (splash, headless, env reuse) - Channel approval flow enhancements Conflict resolution: - Dockerfile: kept OPENCODE_VERSION, adopted upstream's pinned Vercel 52.2.1 - poll-loop.ts: took upstream's async pre-task handling (subsumes our try/catch guard) - agent-route.ts: took upstream's factored-out isSafeAttachmentName - src/index.ts: kept both readEnvFile and new circuit-breaker imports - setup/verify.ts, agent-ping.ts: took upstream's simplified verify flow - package.json: took upstream version 2.0.25
6 tasks
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.
Summary
This PR hardens NanoClaw's host/container filesystem boundary for outbound attachments and outbox cleanup.
Before this change, the agent container controlled both the outbound message rows and the writable session outbox mounted at
/workspace. The host later trusted container-suppliedmessages_out.idandcontent.filesvalues as path components when reading attachments and recursively cleaningoutbox/<messageId>/. A compromised or prompt-injected container could use traversal or symlinks to make the host read or delete paths outside the intended message outbox.This patch confines outbound attachment handling to a single message outbox directory by validating path segments, rejecting symlinks, and realpath-checking host reads and cleanup targets before performing file I/O.
Security issues covered
outbox/<messageId>/and deliver them as attachmentslstatrejection of symlinked/non-file attachment pathsBefore this PR
readOutboxFiles()joined each container-supplied filename directly underoutbox/<messageId>/.readOutboxFiles()followed symlinks because it usedexistsSync()andreadFileSync()on the joined path.clearOutbox()used the container-controlled outbound message id directly in a recursivermSync()path.After this PR
messageIdvalues must be simple path segments before file read or cleanup operations proceed.lstat()and symlinks/non-files are skipped.Why this matters
The session directory is intentionally writable by the agent container:
That writable mount is expected for the container-owned
outbound.dband outbox files. The security boundary problem is that host code later performed file reads and recursive deletion using path strings from that container-owned state.If those strings are not constrained at the host-side sink, the container can turn the host process into a confused deputy. The host has the NanoClaw user's filesystem privileges, so reading or deleting outside the intended outbox can expose or corrupt host-side state that was not supposed to be mounted into the container.
How this differs from #1967 and #1999
This is related to, but distinct from, two existing hardening areas:
isSafeAttachmentName()for agent-to-agent forwarding. That protects the path where one agent copies attachments into another agent's inbox. It does not protect normal channel delivery, wheredelivery.tsstill callsreadOutboxFiles()withcontent.filesfrom the container-owned outbound row..claude-fragmentsand.claude-shared/skills. This PR fixes a different sink: container-controlled outbound attachment reads and post-delivery recursive cleanup under session outboxes.Both fixes are needed because they protect different host operations over container-writable trees.
Attack flow
Affected code
src/session-manager.tsreadOutboxFiles()andclearOutbox()host-side outbox file operationssrc/delivery.tsreadOutboxFiles()usingcontent.filesfrom container-owned outbound messagessrc/container-runner.tssrc/host-core.test.tsRoot cause
existsSync()/readFileSync()followed symlinks at the read sink.CVSS assessment
Issue: host/container filesystem boundary bypass via outbound outbox paths
CVSS v3.1: 8.8 High
Vector:
CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:HRationale: exploitation requires control of an agent container or its container-owned outbound state, but once that precondition is met the vulnerable host process can read or delete files outside the intended outbox boundary with the NanoClaw host user's privileges. Scope changes from container-controlled state to host-side filesystem effects.
Safe reproduction steps
A safe local proof can be constructed with a temporary NanoClaw data directory:
data/v2-sessions/ag-poc/sess-poc/outbox/msg-poc/.data/outside.txt.readOutboxFiles('ag-poc', 'sess-poc', 'msg-poc', ['../../../../../outside.txt'])or createsafe-name.txtas a symlink to the marker and request['safe-name.txt'].data/victim-dir/keep.txtand callclearOutbox('ag-poc', 'sess-poc', '../../../../victim-dir').Expected vulnerable behavior
On vulnerable code:
readOutboxFiles()as outbound file data;clearOutbox()remove a directory outside the intended message outbox.Changes in this PR
lstat()to reject symlinked/non-file attachment paths.Files changed
src/session-manager.tslstat()andrealpath()checkssrc/host-core.test.tsMaintainer impact
This should be low-impact for legitimate attachments because normal outbox attachment names are simple filenames. The change intentionally rejects path-like attachment names and symlinked attachments from the container-owned outbox.
If a future feature needs nested outbox paths, it should add an explicit safe abstraction rather than passing arbitrary container-controlled path strings to host file APIs.
Fix rationale
The host-side file operation is the correct place to enforce this boundary. Container-side validation is useful defense-in-depth, but the host must treat outbound DB rows and outbox files as untrusted because they are written from inside the container.
Basename-only validation matches the existing attachment model and the validation already used in the agent-to-agent forwarding path.
lstat()plusrealpath()containment ensures that safe-looking names cannot escape through symlinks.Type of change
Test plan
Ran:
Notes:
corepack pnpm run lintstill reports unrelated pre-existing errors insrc/channels/cli.tsandsrc/container-runner.ts; this PR does not touch those files.pnpm, which is not on this automation PATH. The commit was created withHUSKY=0after the Corepack validation above passed.Disclosure notes
This PR is intentionally bounded to host-side file read/delete sinks reached from container-owned outbox state. It does not claim a Docker runtime breakout such as privileged-container escape, Docker socket access, host PID namespace access, or added Linux capabilities. The issue is a NanoClaw host/container confused-deputy boundary bypass through path handling in the host process.