fix(imessage): require authorization for group actions by eleqtrizit · Pull Request #97961 · openclaw/openclaw

eleqtrizit · 2026-06-30T00:22:48Z

Summary

Restricts iMessage group-management actions to owner-authorized message turns or Gateway callers with operator.admin scope.

What Problem This Solves

Fixes an issue where callers with generic message-action write access could invoke iMessage group membership and metadata mutations without owner or administrator authorization.

Changes

Require trusted requester identity for group-management actions originating from iMessage turns.
Reject group mutations unless the requester is the owner or has operator.admin scope.
Cover rename, icon, participant, and leave-group actions with focused regression tests.

Why This Change Was Made

The implementation follows the existing channel-owned authorization pattern used for privileged Microsoft Teams actions. It keeps ordinary iMessage actions unchanged and enforces the authorization check before loading or invoking native mutation functions.

User Impact

Authorized owner and administrator flows continue to work. Non-owner, non-admin callers can no longer mutate iMessage group metadata or membership through the shared message-action surface.

This fail-closed change is intentional and compatibility-sensitive: existing Gateway or automation clients that invoke renameGroup, setGroupIcon, addParticipant, removeParticipant, or leaveGroup with only operator.write will now be rejected. Those callers must provide verified owner provenance or connect with operator.admin.

Validation

node scripts/run-vitest.mjs extensions/imessage/src/actions.test.ts — 37 passed
node scripts/run-vitest.mjs src/channels/plugins/message-actions.security.test.ts — 3 passed
Temporary real Gateway WebSocket harness on Linux/Node 22 — operator.write denied before any bridge process; operator.admin succeeded and emitted chat-add-member
node_modules/.bin/oxfmt --check extensions/imessage/src/actions.ts extensions/imessage/src/actions.test.ts
git diff upstream/main...HEAD --check
.agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main --stream-engine-output — clean, no actionable findings

Evidence

Regression tests verify all five privileged group actions are rejected before their native runtime functions run for an operator.write non-owner. Separate assertions verify owner and operator.admin requests remain authorized.

A full Gateway WebSocket proof on the exact PR head used separately paired write-only and admin clients against the same message.action payload. The write-only response was rejected with the new authorization error and the bridge log did not exist, proving no subprocess ran. The admin response succeeded and the bridge log recorded the expected status --json probe followed by chat-add-member --chat <redacted> --address <redacted> --json. The deterministic bridge executable avoided mutating a real Messages group while exercising the production Gateway, plugin dispatch, lazy runtime load, probe, subprocess, and command construction paths.

Notes

AI-assisted. No configuration, protocol, dependency, or documentation surface changes.

clawsweeper · 2026-06-30T00:23:42Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 8:39 PM ET / 00:39 UTC.

Summary
The PR adds an iMessage-owned owner/admin authorization guard and tests for rename, icon, participant add/remove, and leave-group message actions.

PR surface: Source +28, Tests +104. Total +132 across 2 files.

Reproducibility: yes. from source inspection: current main reaches the native iMessage group mutation branches without an owner/admin authorization check, and Gateway write-only callers are forced to senderIsOwner=false. I did not run a live mutation because this cleanup review is read-only and the contributor supplied exact-head Gateway/log proof for the fix.

Review metrics: 1 noteworthy metric.

Privileged action policy changed: 5 iMessage group actions gated. Rename, icon, participant add/remove, and leave now fail closed for non-owner/non-admin callers, which is the compatibility-sensitive surface maintainers should review before merge.

Root-cause cluster
Relationship: canonical
Canonical: #97961
Summary: This PR is the active iMessage-specific hardening item; related merged PRs provide feature and sibling-invariant context but do not supersede it.

Members:

partial_overlap: feat(imessage): add native imsg message actions #79234 - This merged PR introduced the iMessage native message-action surface that this PR hardens.
same_root_cause: fix(msteams): require admin for group actions #91746 - This merged PR applied the same owner-or-admin authorization invariant to Microsoft Teams group actions.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] Existing Gateway or automation clients using only operator.write for iMessage group rename, icon, participant, or leave actions will fail closed after this change unless they provide owner provenance or operator.admin.
[P1] The PR has a protected maintainer label, so even with sufficient proof and no code findings, maintainers need to explicitly accept the compatibility-sensitive authorization boundary before merge.

Maintainer options:

Accept the fail-closed iMessage auth boundary (recommended)
Merge after a maintainer explicitly accepts that write-only iMessage group-management automation must move to owner provenance or operator.admin.
Stage the operator impact
Require an upgrade note or narrower compatibility plan before landing if maintainers want a softer rollout for existing automation clients.
Pause for channel or security ownership
Pause this PR if the owner/admin policy for iMessage group administration still needs explicit channel or security owner direction.

Next step before merge

[P1] Protected-label and compatibility-sensitive fail-closed authorization changes need maintainer acceptance, not an automated repair.

Security
Cleared: The diff tightens an iMessage authorization boundary and does not add dependencies, workflows, package execution, secrets handling, or supply-chain surface.

Review details

Best possible solution:

Merge the plugin-owned authorization gate after a maintainer accepts the fail-closed operator.write impact; no automated repair is needed.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main reaches the native iMessage group mutation branches without an owner/admin authorization check, and Gateway write-only callers are forced to senderIsOwner=false. I did not run a live mutation because this cleanup review is read-only and the contributor supplied exact-head Gateway/log proof for the fix.

Is this the best way to solve the issue?

Yes, this is the best fix shape: iMessage owns these native mutations, while the shared dispatcher and Gateway already carry trusted sender and scope facts. A core hardcoded iMessage policy would be the wrong boundary; the remaining question is maintainer acceptance of the compatibility impact.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 455f813d6ee6.

Label changes

Label justifications:

P0: The PR addresses an authorization bypass where non-admin message-action callers could mutate iMessage group metadata or membership.
merge-risk: 🚨 compatibility: The new guard intentionally rejects existing iMessage group-management callers that only have operator.write unless they provide owner provenance or operator.admin.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The contributor posted exact-head Gateway/log proof showing write-only rejection before subprocess execution and admin success through the native command path.
proof: sufficient: Contributor real behavior proof is sufficient. The contributor posted exact-head Gateway/log proof showing write-only rejection before subprocess execution and admin success through the native command path.

Evidence reviewed

PR surface:

Source +28, Tests +104. Total +132 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	29	1	+28
Tests	1	104	0	+104
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	133	1	+132

What I checked:

Repository policy applied: Root and scoped extension/channel policy were read; the relevant guidance treats fail-closed authorization and compatibility-sensitive plugin behavior as maintainer-review risk. (AGENTS.md:1, 455f813d6ee6)
Current main lacks the owner/admin gate: Current main reaches the native iMessage group mutation branches for rename, icon, add/remove participant, and leave without checking senderIsOwner or gatewayClientScopes. (extensions/imessage/src/actions.ts:633, 455f813d6ee6)
Native runtime mutations are privileged: The iMessage runtime maps the affected actions to imsg chat-name, chat-photo, chat-add-member, chat-remove-member, and chat-leave commands. (extensions/imessage/src/actions.runtime.ts:476, 455f813d6ee6)
PR head adds the plugin-owned gate: PR head defines the five group-management actions, declares trusted-sender requirements for iMessage turns, and throws before loading the native runtime unless the sender is owner or scopes include operator.admin. (extensions/imessage/src/actions.ts:44, 66ca31b0f555)
PR tests cover denial and allowed paths: The added tests require trusted requester identity for the five actions, reject non-owner operator.write calls before native mutation, and allow owner plus operator.admin calls. (extensions/imessage/src/actions.test.ts:196, 66ca31b0f555)
Gateway scope facts are trustworthy: Gateway message.action forwards client scopes and only honors request-provided senderIsOwner for admin-scoped clients; write-only callers are forced to false. (src/gateway/server-methods/send.ts:537, 455f813d6ee6)

Likely related people:

steipete: Authored and merged PR 79234, which introduced the iMessage native message-action surface that includes the group-management actions hardened here. (role: introduced behavior; confidence: high; commits: 7c66bf3592e3, fe79d85ae0cb; files: extensions/imessage/src/actions.ts, extensions/imessage/src/message-tool-api.ts, extensions/imessage/src/actions.test.ts)
Omar Shahine: Recent commits changed the same iMessage action module, runtime boundary, and tests around private-API execution and reply attachment handling. (role: recent area contributor; confidence: medium; commits: bb49f93e8684, 2a5bf2758e3a, 513453079a2b; files: extensions/imessage/src/actions.ts, extensions/imessage/src/actions.runtime.ts, extensions/imessage/src/actions.test.ts)
eleqtrizit: Previously authored and merged PR 91746, which added the same owner-or-operator.admin group-management authorization pattern for Microsoft Teams. (role: adjacent invariant contributor; confidence: high; commits: 07905fde41d1, d2ddc26e89bf; files: extensions/msteams/src/channel.ts, extensions/msteams/src/channel.actions.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

eleqtrizit · 2026-06-30T00:29:38Z

Behavioral proof on exact head 66ca31b0f555e1d7ed44eb5a25ac300f3bb6f99a:

Environment: Linux x86_64, Node v22.22.2. I started the real loopback Gateway and connected two separately paired WebSocket clients through the normal connect/auth path. The iMessage channel used a deterministic bridge executable that logged every invocation, allowing the production Gateway → channel dispatch → lazy iMessage runtime → subprocess path to run without changing a real Messages group.

Same message.action request in both cases: iMessage addParticipant, with group and address values redacted below.

operator.write response:
ok=false
error="iMessage group management requires an owner or operator.admin requester."

bridge log immediately after rejection:
ENOENT (zero bridge subprocess invocations)

operator.admin response:
ok=true

bridge log:
["status","--json"]
["send-rich","--help"]
["chat-add-member","--chat","<redacted-group>","--address","<redacted-address>","--json"]

This proves the changed behavior at the public Gateway boundary: write-only automation fails closed before runtime loading or native mutation, while the admin path still reaches the expected native group-management command.

Focused regression validation was rerun after the behavioral scenario:

node scripts/run-vitest.mjs extensions/imessage/src/actions.test.ts src/channels/plugins/message-actions.security.test.ts --reporter=verbose
Test Files  2 passed (2)
Tests       40 passed (40)

The compatibility impact is intentional and is now explicit in the PR body: existing callers using only operator.write for the five group-management actions must move to verified owner provenance or operator.admin.

eleqtrizit · 2026-06-30T00:29:46Z

@clawsweeper re-review

clawsweeper · 2026-06-30T00:29:49Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

(cherry picked from commit 587eefe)

fix(imessage): require authorization for group actions

66ca31b

openclaw-barnacle Bot added channel: imessage Channel integration: imessage size: S maintainer Maintainer-authored PR labels Jun 30, 2026

eleqtrizit self-assigned this Jun 30, 2026

eleqtrizit merged commit 587eefe into openclaw:main Jun 30, 2026
170 of 184 checks passed

github-actions Bot mentioned this pull request Jun 30, 2026

📡 Upstream Digest — 2026-06-30 02:37 UTC curtismercier/openclaw-mods#1176

Open

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 30, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

8dbdc7b

eleqtrizit deleted the 793 branch June 30, 2026 19:27

QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

0d6d0df

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

bbaad9c

clawsweeper Bot mentioned this pull request Jul 2, 2026

Fix Codex dynamic tools returning raw payloads #97916

Closed

chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

e958daa

Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 6, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

8e528e8

(cherry picked from commit 587eefe)

Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 8, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

ec9c21e

(cherry picked from commit 587eefe)

wheakerd pushed a commit to wheakerd/clawdbot that referenced this pull request Jul 15, 2026

fix(imessage): require authorization for group actions (openclaw#97961)

391b5d3

(cherry picked from commit 587eefe)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(imessage): require authorization for group actions#97961

fix(imessage): require authorization for group actions#97961
eleqtrizit merged 1 commit into
openclaw:mainfrom
eleqtrizit:793

eleqtrizit commented Jun 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

eleqtrizit commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Problem This Solves

Changes

Why This Change Was Made

User Impact

Validation

Evidence

Notes

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

eleqtrizit commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

eleqtrizit commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading