Skip to content

refactor: centralize channel ingress access#79092

Merged
steipete merged 1 commit intomainfrom
codex/channel-access-refactor
May 10, 2026
Merged

refactor: centralize channel ingress access#79092
steipete merged 1 commit intomainfrom
codex/channel-access-refactor

Conversation

@steipete
Copy link
Copy Markdown
Contributor

@steipete steipete commented May 7, 2026

Summary

This PR centralizes message-channel ingress authorization behind an experimental openclaw/plugin-sdk/channel-ingress API, then migrates bundled channel hot paths onto the shared decision graph while preserving older plugin SDK compatibility seams.

  • Adds a redacted ingress resolver, ordered gate graph, typed gate selectors, turn-admission mapping, and AccessFacts projection for route, sender, command, event, and mention-activation gates.
  • Exposes SDK helpers for common adapter construction, legacy DM/group access projection, combined ingress access resolution, access-group expansion, and existing compatibility shims.
  • Migrates bundled runtime paths across Discord, Feishu, Google Chat, iMessage, IRC, LINE, Matrix, Mattermost, Microsoft Teams, Nextcloud Talk, Nostr, QA Channel, QQBot, Signal, Slack, Synology Chat, Telegram, Tlon, Twitch, WhatsApp, Zalo, and Zalo Personal.
  • Keeps plugin-owned behavior local: webhook verification, platform API lookups, identity normalization, pairing replies/writes, command replies, reactions, typing, media, history, and user-facing copy.
  • Marks Discord reaction/system-event text from channel events as explicitly untrusted for the system-event queue.

Behavior Fixed

  • Telegram plugin callbacks keep computed command authorization for group inline-button scopes.
  • Signal passes cfg.accessGroups into DM and group ingress state.
  • Signal and iMessage/BlueBubbles-style group allowlists preserve allowFrom fallback where that was the legacy contract.
  • origin-subject event auth matches adapter-normalized identity values, not opaque subject slot ids or raw unnormalized values.
  • QQBot slash/native paths route command authorization through the bridge access port and ingress command gates.
  • Zalo Personal, Telegram, WhatsApp, Mattermost, and Microsoft Teams no longer carry local resolve/decide/project command boilerplate where the shared ingress helper covers it.
  • Slack command ingress fails closed when sender gates stop before command gates, so block actions do not surface denied senders as command-authorized.

API Design

platform event
  -> plugin verifies authenticity and normalizes platform facts
  -> resolveChannelIngressState(...) or resolveChannelIngressAccess(...)
  -> decideChannelIngress(...) builds route/sender/command/event/activation gates
  -> plugin performs pairing/reply/ack/history/media side effects
  -> turn kernel receives projected redacted AccessFacts

Core receives selected facts and policy slices, not whole config, stores, clients, or network hooks. Raw sender ids and raw allowlist entries exist only in resolver input and adapter match material. Serialized state, decisions, diagnostics, snapshots, and AccessFacts use opaque ids, counts, reason codes, and redacted match ids. The new SDK subpath remains experimental while bundled migrations settle; existing SDK helpers remain source-compatible.

Review Guide

The branch is split for review by layer:

  1. refactor: add channel ingress kernel
  2. refactor: expose channel ingress sdk
  3. refactor: migrate bundled channel ingress
  4. fix: mark discord system events untrusted
  5. fix: fail closed slack command ingress

The important split: commit 1 is generic core only, commit 2 is SDK/docs/compatibility surface, commit 3 is bundled plugin adoption, commit 4 is the OpenGrep hardening fix for Discord system events, commit 5 is the Slack command-auth regression fix.

Compatibility / Migration

  • Backward compatible: yes.
  • Config/env migration required: no.
  • Existing SDK exports remain available; deprecated fields stay redacted or empty where raw values are not safe.
  • The new ingress SDK docs explicitly mark the API experimental and intended for migrated channel runtime paths.

Real Behavior Proof

  • Behavior addressed: shared channel ingress must preserve existing channel authorization semantics while removing duplicated gate logic. The affected behavior includes Telegram callback command auth, Signal access-group allowlists, group allowFrom fallback, origin-subject event auth, QQBot slash/native command auth, Discord system-event trust classification, Slack block-action command auth, and bundled plugin command/sender gate separation.
  • Real environment tested: local OpenClaw checkout on macOS, Node 25.9.0/24-compatible repo tooling, pnpm, rebased onto current origin/main, final pushed head 9cc94872559dec61ef264ce4912debbbc3a042f5; remote Linux proof via Crabbox/Blacksmith Testbox leases tbx_01kr2tv446sw5ntj53n3kajxmf, tbx_01kr2y2gt7hwer215nj5cvba32.
  • Exact steps or command run after the patch:
git diff --check
pnpm lint --threads=8
pnpm check:test-types
pnpm test extensions/slack/src/monitor/auth.test.ts extensions/slack/src/monitor/events/interactions.test.ts
env CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test src/channels/message-access/message-access.test.ts src/channels/message-access/projection.test.ts src/channels/message-access/conformance.test.ts src/plugin-sdk/channel-ingress.test.ts extensions/discord/src/monitor/monitor.test.ts extensions/discord/src/monitor/monitor.agent-components.test.ts extensions/discord/src/monitor/message-handler.process.test.ts
pnpm crabbox:run -- --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref main --idle-timeout 90m --ttl 240m --timing-json --shell -- "env CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test src/channels/message-access/message-access.test.ts src/channels/message-access/projection.test.ts src/channels/message-access/conformance.test.ts src/plugin-sdk/channel-ingress.test.ts extensions/discord/src/monitor/monitor.test.ts extensions/discord/src/monitor/monitor.agent-components.test.ts extensions/discord/src/monitor/message-handler.process.test.ts extensions/imessage/src/monitor.gating.test.ts extensions/imessage/src/monitor/inbound-processing.test.ts extensions/mattermost/src/mattermost/monitor-auth.test.ts extensions/mattermost/src/mattermost/monitor.authz.test.ts extensions/msteams/src/monitor-handler/message-handler.authz.test.ts extensions/whatsapp/src/inbound/access-control.test.ts"
pnpm crabbox:run -- --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref main --idle-timeout 90m --ttl 240m --timing-json --shell -- "node -v && env CI=1 OPENCLAW_LOCAL_CHECK=0 FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true pnpm run lint:extensions:bundled"
  • Evidence after fix:
[test] passed 1 Vitest shard in 6.08s (Slack auth/interactions)
pnpm lint --threads=8: Found 0 warnings and 0 errors across core/extensions/scripts
pnpm check:test-types: tsgo:core:test and tsgo:extensions:test passed
[test] passed 2 Vitest shards in 13.01s
Crabbox lease tbx_01kr2tv446sw5ntj53n3kajxmf:
[test] passed 6 Vitest shards in 33.08s
blacksmith run summary sync=delegated command=1m21.556s total=1m23.179s exit=0
Crabbox lease tbx_01kr2y2gt7hwer215nj5cvba32:
v24.13.0
Found 0 warnings and 0 errors. (lint:extensions:bundled)
blacksmith run summary sync=delegated command=1m20.625s total=1m22.256s exit=0
  • Observed result after fix: Local and Crabbox Linux runs both passed for the central ingress kernel, SDK ingress facade, Discord system-event/monitor coverage, and representative migrated channel authorization paths for iMessage, Mattermost, Microsoft Teams, and WhatsApp. The two Discord enqueueSystemEvent callsites found by OpenGrep now pass trusted: false for channel-derived system text. Slack command ingress now reports commandAuthorized: false when no command gate exists because a sender gate blocked first.
  • What was not tested: live Telegram, Signal, or Slack provider credentials against real network callbacks. This PR uses runtime harnesses, source-level ingress coverage, GitHub CI, local changed-gate proof, and Crabbox remote Linux proof.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: line Channel integration: line channel: matrix Channel integration: matrix channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: nextcloud-talk Channel integration: nextcloud-talk channel: signal Channel integration: signal channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo channel: zalouser Channel integration: zalouser scripts Repository scripts channel: irc channel: qa-channel Channel integration: qa-channel extensions: qa-lab size: XL maintainer Maintainer-authored PR labels May 7, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 7, 2026

Codex review: needs real behavior proof before merge.

Summary
The PR adds an experimental channel ingress runtime/SDK, documentation and API baseline updates, and migrations across bundled channel authorization paths.

Reproducibility: no. high-confidence failing reproduction remains from this source review. The previously flagged Teams and Matrix cases match current docs/source behavior by source comparison, but I did not run live callbacks or local tests.

Real behavior proof
Needs stronger real behavior proof before merge: The PR has terminal/Testbox proof for an older SHA; the contributor should update the PR body with redacted current-head proof, after which ClawSweeper should re-review automatically or via @clawsweeper re-review.

Next step before merge
Human follow-up is required for a protected maintainer-labeled XL auth refactor with stale external proof; automation cannot provide the contributor's real-behavior evidence.

Security
Cleared: The diff is security-sensitive because it changes authorization, but this pass did not find a concrete security or supply-chain regression.

Review details

Best possible solution:

Keep the shared ingress direction under maintainer review, require fresh redacted current-head proof, and land only after the remaining checks and owner review are satisfied.

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

No high-confidence failing reproduction remains from this source review. The previously flagged Teams and Matrix cases match current docs/source behavior by source comparison, but I did not run live callbacks or local tests.

Is this the best way to solve the issue?

Unclear as a final merge decision. The centralized ingress direction matches the architecture guidance, but this XL auth refactor still needs maintainer review and fresh current-head real behavior proof before landing.

What I checked:

  • Live PR state: GitHub API reports the PR is open, non-draft, mergeable, labeled maintainer, maintainer_can_modify=false, and currently at head 0c55491. (0c55491abacc)
  • Real behavior proof is stale: The PR body's proof names final tested head 9cc9487, but the live head is 0c55491 after later force-pushes. (0c55491abacc)
  • Teams fallback matches current documented behavior: Current docs say channels.msteams.groupAllowFrom falls back to channels.msteams.allowFrom, so the previously flagged Teams fallback is not a clear regression against the current documented contract. Public docs: docs/channels/msteams.md. (docs/channels/msteams.md:151, 1b3809430cd2)
  • Teams current-main source already uses the fallback-derived group list: Current main computes effectiveGroupAllowFrom before group authorization and later evaluates group sender access from that list, so DM allowFrom fallback into group sender authorization is not unique to the PR. (extensions/msteams/src/monitor-handler/message-handler.ts:343, 1b3809430cd2)
  • Teams PR source uses the shared fallback intentionally: At the PR head, Microsoft Teams passes groupAllowFromFallbackToAllowFrom: true, and the shared helper falls back to the DM allowlist only when no group allowlist is configured. (extensions/msteams/src/monitor-handler/access.ts:112, 0c55491abacc)
  • Matrix current-main room users are a sender gate: Current main blocks a room sender when roomUserMatch exists and does not match, before command authorization runs, so a global group allowlist does not bypass a non-empty per-room user list. (extensions/matrix/src/matrix/monitor/handler.ts:814, 1b3809430cd2)

Likely related people:

  • steipete: Recent main history touches Teams access, Matrix access, shared DM policy, and the channel turn kernel, and this PR also changes the same surfaces. (role: recent area contributor and feature-history owner; confidence: high; commits: 0fe007f71b2e, d47055aa92e5, bd1d1f0f2bb2; files: extensions/msteams/src/monitor-handler/access.ts, extensions/matrix/src/matrix/monitor/access-state.ts, src/security/dm-policy-shared.ts)
  • jacobtomlinson: Recent merged Teams authorization work touched the same monitor-handler access path. (role: Teams adjacent contributor; confidence: medium; commits: c5415a474bb0; files: extensions/msteams/src/monitor-handler/access.ts)
  • pgondhi987: Recent Matrix authorization work changed the same access-state surface for room command authorization boundaries. (role: Matrix adjacent contributor; confidence: medium; commits: f8705f512b09; files: extensions/matrix/src/matrix/monitor/access-state.ts)
  • gumadeiras: Prior Matrix plugin migration work touched the same monitor access-state file. (role: Matrix adjacent contributor; confidence: medium; commits: 94693f7ff036; files: extensions/matrix/src/matrix/monitor/access-state.ts)

Remaining risk / open question:

  • Current-head real behavior proof is stale because the PR body still names an older tested SHA.
  • One Windows node test check was still in progress when inspected.
  • This was a read-only review, so I did not run local Vitest suites or live channel callbacks.

Codex review notes: model gpt-5.5, reasoning high; reviewed against aac9ebd4f3ef.

Re-review progress:

@steipete steipete force-pushed the codex/channel-access-refactor branch 8 times, most recently from 982e23a to 6055c49 Compare May 7, 2026 23:12
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes commands Command implementations agents Agent runtime and tooling extensions: deepseek extensions: lmstudio labels May 8, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9cc9487255

ℹ️ 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".

effectiveRoomUsers,
}),
commandOwner: commandAllowFrom,
commandGroup: effectiveRoomUsers.length > 0 ? effectiveRoomUsers : effectiveGroupAllowFrom,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Merge room and global command allowlists

When both roomUsers and groupAllowFrom are configured for a room, commandGroup is set to effectiveRoomUsers only, so command authorization no longer accepts users who are allowed solely through the global group allowlist. This changes prior OR-style behavior (room users OR global group allowlist) and can block valid control/text commands in existing Matrix setups that rely on both lists.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc378bd7b4

ℹ️ 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".

effectiveRoomUsers,
}),
commandOwner: commandAllowFrom,
commandGroup: effectiveRoomUsers.length > 0 ? effectiveRoomUsers : effectiveGroupAllowFrom,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Merge room and global command allowlists

For room traffic, commandGroup currently picks effectiveRoomUsers whenever that list is non-empty and drops effectiveGroupAllowFrom, which changes command auth from (roomUsers OR groupAllowFrom) to roomUsers only in mixed configurations. This blocks valid control/text commands from users who are allowed via the global group allowlist but not present in the room-specific list. Fresh evidence on this commit: with roomUsers=['@room-only:example.org'], groupAllowFrom=['@admin:example.org'], and sender @admin:example.org, commandAuthorizers[2].allowed is true but resolveMatrixMonitorCommandAccess(...).commandAuthorized is false because only the room list is fed into commandGroup.

Useful? React with 👍 / 👎.

@steipete steipete force-pushed the codex/channel-access-refactor branch from bc378bd to 0a7b5ac Compare May 9, 2026 07:16
@steipete steipete requested a review from a team as a code owner May 9, 2026 07:16
@steipete steipete force-pushed the codex/channel-access-refactor branch from 0a7b5ac to 08db2c3 Compare May 9, 2026 22:41
@openclaw-barnacle openclaw-barnacle Bot removed the agents Agent runtime and tooling label May 9, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08db2c33a7

ℹ️ 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".

Comment on lines +119 to +120
const commandGroupAllowFrom =
state.effectiveRoomUsers.length > 0 ? state.effectiveRoomUsers : state.effectiveGroupAllowFrom;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Merge room and global command allowlists for Matrix commands

When a Matrix room has both roomUsers and groupAllowFrom configured, command authorization currently uses only effectiveRoomUsers and discards effectiveGroupAllowFrom, which changes command auth from an OR condition to room-users-only and blocks valid control/text commands from globally allowlisted senders. Fresh evidence on this commit: commandGroupAllowFrom is assigned with state.effectiveRoomUsers.length > 0 ? state.effectiveRoomUsers : state.effectiveGroupAllowFrom, so any non-empty room user list suppresses the global group allowlist entirely.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1129a2afa

ℹ️ 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".

groupPolicy: senderGroupPolicy,
groupPolicy,
policy: {
groupAllowFromFallbackToAllowFrom: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve Teams group allowlist isolation from DM allowlist

Switching groupAllowFromFallbackToAllowFrom to true makes group sender authorization inherit channels.msteams.allowFrom whenever groupAllowFrom is unset/empty, which broadens access beyond prior behavior. In practice, a user listed only for DMs can now be accepted in group/channel conversations under groupPolicy: "allowlist", whereas the previous implementation (false) treated empty group sender lists as deny/empty-allowlist. This is a regression in authorization scope and can unintentionally expose group conversations.

Useful? React with 👍 / 👎.

@steipete steipete force-pushed the codex/channel-access-refactor branch 2 times, most recently from d49a568 to cfe5616 Compare May 10, 2026 02:49
@openclaw-barnacle openclaw-barnacle Bot added the cli CLI command changes label May 10, 2026
@steipete steipete force-pushed the codex/channel-access-refactor branch from 3a0d830 to dce57e3 Compare May 10, 2026 03:49
@openclaw-barnacle openclaw-barnacle Bot removed the cli CLI command changes label May 10, 2026
@steipete steipete force-pushed the codex/channel-access-refactor branch from dce57e3 to 0c55491 Compare May 10, 2026 04:08
@steipete steipete merged commit a0fb7fb into main May 10, 2026
111 checks passed
@steipete steipete deleted the codex/channel-access-refactor branch May 10, 2026 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui channel: discord Channel integration: discord channel: feishu Channel integration: feishu channel: googlechat Channel integration: googlechat channel: imessage Channel integration: imessage channel: irc channel: line Channel integration: line channel: matrix Channel integration: matrix channel: mattermost Channel integration: mattermost channel: msteams Channel integration: msteams channel: nextcloud-talk Channel integration: nextcloud-talk channel: nostr Channel integration: nostr channel: qa-channel Channel integration: qa-channel channel: qqbot channel: signal Channel integration: signal channel: slack Channel integration: slack channel: synology-chat channel: telegram Channel integration: telegram channel: tlon Channel integration: tlon channel: twitch Channel integration: twitch channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo channel: zalouser Channel integration: zalouser commands Command implementations docs Improvements or additions to documentation extensions: qa-lab gateway Gateway runtime maintainer Maintainer-authored PR scripts Repository scripts size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant