fix(media): merge iMessage attachmentRoots into image tool local roots#30363
fix(media): merge iMessage attachmentRoots into image tool local roots#30363sxu75374 wants to merge 11 commits into
Conversation
openclaw-barnacle
Bot
added
channel: whatsapp-web
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc9c597540
ℹ️ 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".
| // validated by the inbound channel. (Fixes #30170) | ||
| const cfg = options?.config; | ||
| const roots = cfg | ||
| ? Array.from(getAgentScopedMediaLocalRoots(cfg, agentDir)) |
There was a problem hiding this comment.
createImageTool now calls getAgentScopedMediaLocalRoots(cfg, agentDir), but that helper expects an agent ID, not the agent directory path. Because resolveAgentWorkspaceDir normalizes this argument as an ID, a typical path-like agentDir resolves to a synthetic workspace-... directory instead of the actual agent workspace, so the image tool can miss valid workspace files (or allow an unintended root) whenever workspaceDir is absent or not aligned.
Useful? React with 👍 / 👎.
Greptile SummaryMerges iMessage Key changes:
The fix is additive and preserves existing roots while preventing duplicates. Confidence Score: 5/5
Last reviewed commit: cc9c597 |
This comment was marked as spam.
This comment was marked as spam.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82e4ab3996
ℹ️ 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".
|
|
||
| // Merge iMessage attachment roots so tools (e.g. image) can read | ||
| // attachments that were already validated by the iMessage channel. | ||
| const iMessageRoots = resolveIMessageAttachmentRoots({ cfg }); |
There was a problem hiding this comment.
getAgentScopedMediaLocalRoots now unconditionally appends resolveIMessageAttachmentRoots({ cfg }), but that helper always falls back to DEFAULT_IMESSAGE_ATTACHMENT_ROOTS when channels.imessage is unset (src/media/inbound-path-policy.ts). Because this root builder is reused across non‑iMessage flows (tool/media loading in other channels), the change silently broadens local file access to /Users/*/Library/Messages/Attachments even when iMessage is not configured, which is a permission expansion from prior behavior.
Useful? React with 👍 / 👎.
openclaw-barnacle
Bot
added
docs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9896c3d67d
ℹ️ 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".
| // iMessage attachmentRoots) remain readable by the image tool (#30170). | ||
| const cfg = options?.config; | ||
| if (cfg) { | ||
| return Array.from(getAgentScopedMediaLocalRoots(cfg, agentDir)); |
There was a problem hiding this comment.
When options.config is present, this branch returns getAgentScopedMediaLocalRoots(...) and drops options.workspaceDir entirely. That regresses the previous behavior where an explicit workspace root was always allowlisted, so image paths in the caller-provided workspace (for example the default coding workspace/cwd) can now be rejected as path-not-allowed unless that directory also matches the agent workspace resolved from config.
Useful? React with 👍 / 👎.
The image tool's path allowlist (mediaLocalRoots) did not include iMessage channel attachment roots, causing a 'not under an allowed directory' error when attempting to analyze iMessage attachments. This regression was introduced by the security hardening in 2026.2.26. Changes: - local-roots.ts: merge iMessage attachmentRoots into getAgentScopedMediaLocalRoots so tools can access validated attachments - image-tool.ts: use getAgentScopedMediaLocalRoots (with config) instead of getDefaultLocalRoots, picking up channel attachment roots - web/media.ts: add wildcard segment matching to assertLocalMediaAllowed so glob-pattern roots like /Users/*/Library/Messages/Attachments work - local-roots.test.ts: add unit tests for iMessage root merging Fixes openclaw#30170 Signed-off-by: sxu75374 <[email protected]> Signed-off-by: Small <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79b7f9d6ce
ℹ️ 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".
| const roots = cfg | ||
| ? Array.from(getAgentScopedMediaLocalRoots(cfg, agentDir)) | ||
| : Array.from(getDefaultLocalRoots()); |
There was a problem hiding this comment.
This new root-building branch ignores fsPolicy.workspaceOnly, so createImageTool(..., fsPolicy: { workspaceOnly: true }) still allowlists default state/media roots (and now merged attachment roots) instead of restricting reads to the workspace. In non-sandbox runs this weakens the intended filesystem boundary and regresses the prior resolveMediaToolLocalRoots(..., { workspaceOnly: true }) behavior that image tool callers and tests rely on.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d031c45217
ℹ️ 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".
| const roots = cfg | ||
| ? Array.from(getAgentScopedMediaLocalRoots(cfg, agentDir)) | ||
| : Array.from(getDefaultLocalRoots()); | ||
| const workspaceDir = normalizeWorkspaceDir(options?.workspaceDir); |
There was a problem hiding this comment.
The new localRoots construction no longer consults options.fsPolicy?.workspaceOnly, so non-sandbox image loads now always include default host roots instead of being restricted to the workspace. Before this change, resolveMediaToolLocalRoots(..., { workspaceOnly: true }) returned only the workspace path (or none), but with this code a workspace-restricted session can read files from default roots like temp/state directories, which weakens the intended filesystem isolation.
Useful? React with 👍 / 👎.
|
|
||
| // Merge iMessage attachment roots so tools (e.g. image) can read | ||
| // attachments that were already validated by the iMessage channel. | ||
| const iMessageRoots = resolveIMessageAttachmentRoots({ cfg }); |
There was a problem hiding this comment.
This unconditionally merges iMessage attachment roots for any agent-scoped call, but resolveIMessageAttachmentRoots falls back to DEFAULT_IMESSAGE_ATTACHMENT_ROOTS even when iMessage is not configured. As a result, deployments with no iMessage channel configured still get /Users/*/Library/Messages/Attachments added to allowed local media roots, broadening file-read scope beyond the stated "when iMessage is configured" behavior.
Useful? React with 👍 / 👎.
2 participants
Summary
Fixes #30170 — after 2026.2.26, the
imagetool can't read iMessage attachment paths becausemediaLocalRootsdoesn't includeattachmentRoots.Changes
src/media/local-roots.ts: Merge iMessageattachmentRootsintogetAgentScopedMediaLocalRoots()so the image tool can resolve attachment pathssrc/agents/tools/image-tool.ts: UsegetAgentScopedMediaLocalRootswith config to pick up merged rootssrc/web/media.ts: UseisInboundPathAllowedfor wildcard rootssrc/media/local-roots.test.ts: 5 unit tests covering merge behavior, deduplication, and empty casesHow it works
When iMessage is configured, its
attachmentRoots(e.g.~/Library/Messages/Attachments) are now included in the set of allowed local roots that the image tool checks against. This restores the pre-2026.2.26 behavior where iMessage attachments could be analyzed via theimagetool.The merge is additive — existing roots are preserved, and duplicates are deduplicated.
Signed-off-by: sxu75374 [email protected]