feat(gateway): capability-gated outbound Slack — reactions, file upload, thread replies#1851
Merged
Merged
Conversation
kgabryje
added a commit
that referenced
this pull request
Jul 8, 2026
…oad escape rejections, dedupe channel_history onto shared resolver Addresses Codex review on #1851: - Add direct (non-mocked) escape tests for agor_gateway_slack_file_upload: relative ".." traversal through the real branch-workspace resolver, an absolute-path symlink escaping the upload directory, and a null-byte path — proving the rejections hold end-to-end through the tool. - Add cheap regex format validation for slackChannelId/ts/emoji on the reaction tools so malformed input is rejected by the schema before any Slack call. - Refactor agor_gateway_slack_channel_history_get onto the shared resolveGatewaySlackToolTarget resolver, removing its now-duplicate inline capability-gate + branch-binding logic (verified risk-free: existing tests only assert error-message substrings shared by both code paths).
kgabryje
force-pushed
the
gateway-slack-read-tools-capabilities
branch
from
fd491e4 to
5962fd0
Compare
…ad, thread replies Extends the Slack agent-tool capability model (#1848) with reactions and file_upload, mirroring channel_history's pattern exactly: opt-in toggles on SlackAgentToolsConfig, matching OAuth scopes in SLACK_AGENT_TOOL_SCOPES, and capability-gated + branch-bound MCP tools (agor_gateway_slack_reaction_add/ _remove, agor_gateway_slack_file_upload). file_upload constrains its path argument to the daemon upload directory or the calling session's branch workspace, reusing resolveBranchWorkspacePath's containment checks — no arbitrary host filesystem reads. Also exposes threadTs on agor_gateway_emit_message so agents can reply into existing Slack threads (pure exposure of the connector's existing thread_ts support, no new scope). Interactive Block Kit components are out of scope (#1729) — deferred to a follow-up needing Socket Mode interactivity handling.
…oad escape rejections, dedupe channel_history onto shared resolver Addresses Codex review on #1851: - Add direct (non-mocked) escape tests for agor_gateway_slack_file_upload: relative ".." traversal through the real branch-workspace resolver, an absolute-path symlink escaping the upload directory, and a null-byte path — proving the rejections hold end-to-end through the tool. - Add cheap regex format validation for slackChannelId/ts/emoji on the reaction tools so malformed input is rejected by the schema before any Slack call. - Refactor agor_gateway_slack_channel_history_get onto the shared resolveGatewaySlackToolTarget resolver, removing its now-duplicate inline capability-gate + branch-binding logic (verified risk-free: existing tests only assert error-message substrings shared by both code paths).
kgabryje
force-pushed
the
gateway-slack-outbound-richness
branch
from
7f23788 to
e07df58
Compare
…M exemption Addresses dual-review (Codex + Fable) on #1851: - Major: agor_gateway_slack_reaction_add/_remove and agor_gateway_slack_file_upload now honor the gateway channel's allowed_channel_ids whitelist, mirroring the inbound listening semantics — DMs (Slack IDs starting with "D") are always exempt, since these tools default to the calling session's own conversation, which is frequently a DM. New isSlackDirectMessageId/isSlackWriteTargetAllowed helpers in slack.ts, enforced once in the shared resolveGatewaySlackToolTarget resolver, scoped to the 'reactions'/'file_upload' capabilities only — channel_history's existing (already-reviewed) connector-level enforcement is untouched. - Minor: slackFileUploadSchema's slackChannelId/threadTs and agor_gateway_emit_message's threadTs now use the same regex format validation the reaction tools already had. - Minor: the file-upload escape-rejection error no longer echoes the resolved daemon upload root's absolute path.
kgabryje
added
ai-reviewed-claude-fable-5
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
2 participants
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
Adds richer OUTBOUND Slack capabilities for gateway agent sessions (#1729), reusing #1848's capability/branch-binding model exactly — no parallel pattern.
1. Capability model (
packages/core/src/types/gateway.ts,slack-manifest.ts): addsreactionsandfile_uploadtoSlackAgentToolsConfig/SLACK_AGENT_TOOL_DEFAULTS(both default disabled — opt-in, add write scopes) andSLACK_AGENT_TOOL_SCOPES(reactions→reactions:write,file_upload→files:write). Wired throughagor_gateway_slack_manifest_generateand the create-wizard/edit-form UI toggles, same single-source-of-truth aschannel_history.2. Connector methods (
packages/core/src/gateway/connectors/slack.ts):addReaction/removeReaction(reactions.add/reactions.remove) anduploadFile(files.uploadV2), mirroringsendSlackMessage's error-handling style.3. New MCP tools (
apps/agor-daemon/src/mcp/tools/gateway-channels.ts), each capability-gated + branch-bound via a sharedresolveGatewaySlackToolTargetresolver:agor_gateway_slack_reaction_add/agor_gateway_slack_reaction_remove(capabilityreactions)agor_gateway_slack_file_upload(capabilityfile_upload) —pathis constrained to either the daemon upload directory or a path relative to the calling session's branch workspace (reusingresolveBranchWorkspacePath's containment checks, same pattern as other branch-workspace file tools). No arbitrary host filesystem reads.agor_gateway_slack_channel_history_get(#1848) was also refactored onto this same shared resolver — it previously had its own inline copy of the identical capability-gate + branch-binding logic; that duplication is now gone and all four Slack conversation-targeting tools enforce the rules from one place. Verified risk-free: every existingchannel_history_gettest only asserts error-message substrings, all of which are shared between the old and new error text (confirmed by the full 66-test suite passing unchanged).4. Thread replies on emit:
agor_gateway_emit_messagegains an optionalthreadTsparam, plumbed straight to the connector's existingthread_tssupport — pure exposure, no new scope, no new capability. Omitting it keeps current behavior (fresh thread/DM per emit).5. Input hardening (post-review):
slackChannelId/ts/emojion the reaction tools now get cheap regex format validation (^[A-Z0-9]+$,^\d+\.\d+$,^[a-z0-9_+'-]+$) so malformed input is rejected by the schema before any Slack call, instead of just a non-empty check.Out of scope
Interactive Block Kit components (buttons/select menus) are deferred — they need Socket Mode interactivity handling, a separate feature. Reactions cover #1729's reaction/voting use case. A raw
blockspassthrough on emit was also skipped to keep this PR's surface tight.Stack
Stacked on #1848 → #1847. Base is
gateway-slack-read-tools-capabilities, notmain— this branch will be rebased ontomainonce both merge.Test plan
pnpm typecheck/pnpm lint/pnpm check:shortid— greenturbo run build— greenfile_uploadescape rejections (relative..traversal, absolute-path symlink escape, null-byte path) are exercised end-to-end through the real validation code (not mocked) — added per Codex reviewcheck:multitenancy-boundaries5-file baseline drift (register-hooks.ts, health-monitor.test.ts, branches.test.ts, tenant-db-scope.test.ts, widgets/env-vars/index.ts) and one pre-existing unrelatedpackages/executorOpenCode MCP-header test failure — neither touches files changed by this PR🤖 Generated with Claude Code