Update slack app permissions#2366
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Added new permissions: - Join public channels (channels:join) - Read files shared in conversations (files:read) - Read and write group direct messages (mpim:history, mpim:read, mpim:write)
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(4) Total Issues | Risk: Medium
🟠⚠️ Major (2) 🟠⚠️
🟠 1) slack-app-manifest.json Missing changeset for user-facing permission change
Issue: This PR adds 5 new OAuth bot scopes (channels:join, files:read, mpim:history, mpim:read, mpim:write) but has no changeset. Per AGENTS.md, changes to agents-work-apps that affect runtime behavior require changesets since the package ships to users.
Why: Self-hosted customers copying this manifest will be prompted to re-authorize their Slack app for new permissions. Without a changeset, they won't see this change in release notes and may be surprised by the expanded permission request. Enterprise customers with Slack app review processes may need to re-approve.
Fix: Create a changeset describing what scopes are added and why:
pnpm bump patch --pkg agents-work-apps "Add Slack OAuth scopes for multi-party DMs, channel joining, and file reading"Refs:
🟠 2) installation.mdx Documentation doesn't reflect new permissions
Issue: The Slack installation docs (lines 31-36) list permissions customers see during OAuth. The new scopes aren't documented:
channels:join— "Join public channels"files:read— "View files shared in channels"mpim:*— "View/send messages in group DMs"
Why: Customers following the docs will see additional permission prompts during re-authorization that aren't explained. This creates a documentation-reality mismatch at a high-stakes touchpoint (OAuth consent screen).
Fix: Update agents-docs/content/talk-to-your-agents/slack/installation.mdx lines 31-36 to include the new permissions with brief explanations of why each is needed.
Refs:
🟡 Minor (1) 🟡
🟡 1) slack-app-manifest.json Speculative permissions without active code usage
Issue: None of these new scopes are currently used by the codebase:
- No calls to
conversations.join(channels:join) - No file reading APIs (
files:read) — onlyfiles.uploadV2which uses existingfiles:write - Dispatcher only handles
channel_type === 'im', not group DMs (mpim:*)
Why: Granting OAuth scopes before code needs them expands the attack surface if credentials are compromised and may trigger additional security review from enterprise customers. The principle of least privilege suggests requesting permissions when features ship, not speculatively.
Fix: Either:
- Remove scopes until the corresponding features are implemented, OR
- Document in the PR description what upcoming feature requires these permissions
Refs:
Inline Comments:
- 🟠 Major:
slack-app-manifest.json:63-65MPIM scopes without event subscription
💭 Consider (1) 💭
💭 1) commit history Vague commit message
Issue: The commit message is simply "update" which doesn't describe the nature of the change.
Why: For security-relevant changes like OAuth permission expansion, commit messages should help future developers understand intent without reading the diff.
Fix: Consider amending to something descriptive: "Add Slack OAuth scopes for group DM support, channel joining, and file reading"
🚫 REQUEST CHANGES
Summary: This PR expands Slack OAuth permissions but lacks the required changeset and documentation updates. The new scopes will be visible to customers during Slack app installation/re-authorization, so they need to be documented. Additionally, clarification is needed on whether these permissions are for an upcoming feature (and if so, what feature) or if there's missing code that should accompany this change.
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
slack-app-manifest.json |
Standards reviewer found no issues | Valid — JSON is correct, scopes are valid Slack scopes, alphabetically ordered |
slack-app-manifest.json:52 |
channels:join without handler |
Lower priority than other findings; same class of issue as mpim scopes |
Reviewers (4)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-product |
3 | 1 | 0 | 0 | 0 | 0 | 2 |
pr-review-devops |
5 | 1 | 1 | 0 | 1 | 0 | 2 |
pr-review-security-iam |
2 | 1 | 0 | 0 | 0 | 0 | 1 |
| Total | 10 | 3 | 1 | 0 | 1 | 0 | 5 |
Note: Findings were deduplicated across reviewers (changeset missing was flagged by 3 reviewers, docs drift by 2).
| "mpim:history", | ||
| "mpim:read", | ||
| "mpim:write", |
There was a problem hiding this comment.
🟠 MAJOR: MPIM scopes without event subscription
Issue: These mpim:* scopes grant access to group DMs but the manifest's bot_events array (line 75) doesn't include message.mpim, so the bot won't receive group DM messages.
Why: If the intent is to enable group DM support, the bot needs both the OAuth scopes AND the event subscription. Currently the bot subscribes to message.groups but not message.mpim — and even message.groups events are ignored by the dispatcher.
Fix: If group DM support is intended, add to bot_events:
"bot_events": ["app_mention", "message.channels", "message.groups", "message.im", "message.mpim"]If these scopes are for API access only (reading history on-demand rather than real-time events), this may be intentional — please clarify in the PR description.
Refs:
- Slack MPIM events docs
- dispatcher.ts:150-192 — only handles
channel_type === 'im'
Added new permissions: - Join public channels (channels:join) - Read files shared in conversations (files:read) - Read and write group direct messages (mpim:history, mpim:read, mpim:write) Co-authored-by: inkeep[bot] <257615677+inkeep[bot]@users.noreply.github.com>
1 participant
No description provided.