[security] fix(agent-route): reject unsafe forwarded attachments#2468
Open
Hinotoi-agent wants to merge 1 commit into
Open
[security] fix(agent-route): reject unsafe forwarded attachments#2468Hinotoi-agent wants to merge 1 commit into
Hinotoi-agent wants to merge 1 commit into
Conversation
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 agent-to-agent attachment forwarding so the host does not follow container-controlled symlinks when copying files from a source agent outbox into a target agent inbox.
Before this change,
forwardAttachedFiles(...)accepted safe-looking attachment basenames and then usedfs.copyFileSync(src, dst). BecausecopyFileSyncfollows symlinks, a compromised or malicious agent could place a symlink in its writable outbox and ask the host to forward it as an attachment. The host would then copy the symlink target bytes into another agent's inbox if the host process could read the target.The patch makes the source outbox and each forwarded file subject to host-side
lstat/realpathchecks before copying.Security issues covered
copyFileSyncBefore this PR
isSafeAttachmentName(filename).src = path.join(sourceDir, filename)and copied it withfs.copyFileSync(src, dst).After this PR
realpathSyncand must remain inside the resolved source outbox before it is copied.Why this matters
Agent workspaces/outboxes are intentionally writable by the agent container, but host-side file forwarding runs outside that container boundary. A filename-only check is not sufficient when the directory entry itself is controlled by the container.
Without this guard, an agent that can write its outbox can turn an apparently benign attachment name such as
safe-name.txtinto a symlink to another host-readable path. The host then becomes the component that dereferences and copies the target file.How this differs from #2001
This is the same trust-boundary family as #2001, but it is not the same code path.
#2001 hardened normal outbound attachment handling and cleanup in
src/session-manager.ts, where the host reads container outbox files and removes message outbox directories. This PR hardens the separate agent-to-agent forwarding path insrc/modules/agent-to-agent/agent-route.ts.The distinction is:
session-manager.ts.forwardAttachedFiles(...)inagent-route.ts.copyFileSync.Both paths need their own boundary checks because agent-to-agent forwarding reimplements the file-copy step instead of going through the already-hardened normal outbox reader.
Attack flow
safe-name.txt.files: ["safe-name.txt"].fs.copyFileSyncfollows the symlink and copies the target bytes into the target inbox.Affected code
src/modules/agent-to-agent/agent-route.tsforwardAttachedFiles(...)src/modules/agent-to-agent/agent-route.test.tsRoot cause
isSafeAttachmentName(filename)as sufficient validation for a container-controlled source file.fs.copyFileSync, which follows symlinks.lstat/realpath/containment checks used by related hardened outbox handling.CVSS assessment
CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:N/A:NSafe reproduction steps
A focused regression test can exercise the vulnerable behavior without using real secrets:
files: ["safe-name.txt"].Expected vulnerable behavior
On vulnerable code, the target inbox receives an attachment whose contents are the sentinel bytes from the symlink target. That proves the host followed the source attachment symlink while forwarding.
After this patch, the symlinked attachment is skipped and no copied attachment contains the sentinel bytes.
Changes in this PR
source.messageIdbefore using it as an outbox path segment.realpathSyncand required them to remain inside the resolved source outbox.Files changed
src/modules/agent-to-agent/agent-route.tslstatchecks, realpath containment, safe message id validation, and symlink/non-regular rejectionsrc/modules/agent-to-agent/agent-route.test.tsMaintainer impact
Fix rationale
The host should validate the filesystem object it is about to copy, not only the string used to name it. Rejecting symlinks and requiring the resolved source file to stay inside the resolved source outbox makes the security boundary explicit at the point where host-side copying occurs.
Type of change
.claude/skills/<name>/, no source changes)Test plan
Local validation run:
Notes:
pnpm run lintwas also attempted, but the repository-level lint script reported pre-existing warnings outside this patch. The touched-file ESLint command above passed.Disclosure notes
This PR is intentionally bounded to the agent-to-agent attachment forwarding path. It does not claim to cover every host/container filesystem interaction in the project. It specifically closes the symlink-follow variant in
forwardAttachedFiles(...)by validating the source outbox and source attachment filesystem objects immediately before host-side copying.