Slack: harden slash and interactions ingress checks by Solvely-Colin · Pull Request #29091 · openclaw/openclaw

Solvely-Colin · 2026-02-27T19:39:03Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: Slack mismatch filtering (api_app_id / team_id) was enforced for event subscriptions, but slash and interaction entry points could process payloads without the same ingress guard.
Why it matters: Slash commands and interactive controls are privileged paths that can drive command/tool execution; mismatch filtering should be consistent before any authorization/dispatch logic.
What changed: Added early mismatch drops in Slack slash command handlers, slash arg menu options/actions, block actions, and modal lifecycle handlers; extended mismatch parsing to support interaction-style nested team.id.
What did NOT change (scope boundary): No changes to Slack token verification primitives, DM/group policy semantics, or tool permission policy models.

Slack now drops mismatched app/team payloads earlier on slash and interaction surfaces (same trust boundary behavior as event handlers).

New permissions/capabilities? (Yes/No): No
Secrets/tokens handling changed? (Yes/No): No
New/changed network calls? (Yes/No): No
Command/tool execution surface changed? (Yes/No): Yes
Data access scope changed? (Yes/No): No
If any Yes, explain risk + mitigation:
- Tightened ingress checks on privileged Slack entry points to fail closed earlier when app/team identity does not match the configured account context.

OS: macOS
Runtime/container: Node 22+, pnpm
Model/provider: N/A
Integration/channel (if any): Slack
Relevant config (redacted): Slack config with teamId/apiAppId context and slash/interactions enabled

Register Slack slash commands and interactions with a context whose mismatch guard returns true (or payload with mismatched app/team identifiers).
Send slash/interaction payloads.
Observe they are acknowledged but not dispatched/enqueued.

Verified in tests: no dispatch/system-event enqueue for mismatched slash/interactions payloads.

Attach at least one:

What you personally verified (not just CI), and how:

Verified scenarios:
- pnpm test -- src/slack/monitor/slash.test.ts src/slack/monitor/events/interactions.test.ts src/slack/monitor/context.test.ts
- Confirmed new tests cover slash, interaction block actions, modal lifecycle drops, and nested team.id mismatch parsing.
Edge cases checked:
- Slash arg-menu options/actions mismatch handling still acks safely without dispatch.
- Existing authorized interaction flow remains intact.
What you did not verify:
- Full repo pnpm check currently fails on unrelated pre-existing type errors outside this change set.

How to disable/revert this change quickly:
- Revert this PR commit.
Files/config to restore:
- src/slack/monitor/slash.ts
- src/slack/monitor/events/interactions.ts
- src/slack/monitor/context.ts
Known bad symptoms reviewers should watch for:
- Legitimate slash or interaction payloads being dropped due to unexpected app/team identifier mismatch.

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk:
- Overly strict mismatch checks could block valid Slack payload variants.
- Mitigation:
  - Added test coverage for nested team.id payload form and preserved non-mismatch paths.
AI-assisted:
- This PR was AI-assisted and manually reviewed/tested as described above.

greptile-apps · 2026-02-27T19:41:37Z

Extended Slack ingress validation to slash commands and interactions, bringing them to parity with event subscription security checks.

Key changes:

Added team.id nested payload format support for interaction-style bodies
Implemented early mismatch drops in slash command handlers (direct commands, arg menu options/actions)
Implemented mismatch guards for block actions and modal lifecycle events (view submission/closed)
Fail-closed approach: payloads acknowledged but silently dropped with server-side logging when app/team IDs don't match configured context

Implementation quality:

Mismatch checks correctly placed after ack() (prevents Slack timeout warnings) but before dispatch/enqueue
Comprehensive test coverage for all new code paths including nested team.id format
Consistent with existing event subscription mismatch handling patterns already in place for messages, app_mentions, and reactions
Edge cases handled properly (missing IDs, empty values, nested formats)

This PR is safe to merge with minimal risk - it's a defensive security hardening change with comprehensive test coverage.
Clean security hardening implementation: extends existing mismatch filtering pattern to slash/interaction entry points, handles edge cases correctly, includes comprehensive tests, no logical errors found, and follows fail-closed security principles. The changes are additive (only dropping mismatched requests) with no modifications to core processing logic.
No files require special attention - all changes follow established patterns and are well-tested.

_{Last reviewed commit: 13a5eb3}

Takhoffman · 2026-03-01T15:41:07Z

PR #29091 - Slack: harden slash and interactions ingress checks (#29091)

Merged via squash.

Merge commit: 0f36ee5
Verified: pnpm build, pnpm check, pnpm test:macmini
Changes made:
M CHANGELOG.md
A src/slack/monitor/context.test.ts
M src/slack/monitor/context.ts
M src/slack/monitor/events/interactions.test.ts
M src/slack/monitor/events/interactions.ts
M src/slack/monitor/slash.test.ts
M src/slack/monitor/slash.ts
Why these changes were made:
Required changelog policy for autoland; code path and tests from contributor branch were preserved after rebase.
Changelog: CHANGELOG.md updated=true required=true opt_out=false

@Solvely-Colin

…thanks @Solvely-Colin Cherry-pick of upstream 0f36ee5.

@Solvely-Colin