Slack: honor SLACK_USER_TOKEN in account resolution and audit#32570
Slack: honor SLACK_USER_TOKEN in account resolution and audit#32570zwright8 wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR extends Slack's
Confidence Score: 5/5
Last reviewed commit: 5fb517a |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fb517a299
ℹ️ 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".
| "DISCORD_BOT_TOKEN", | ||
| "SLACK_BOT_TOKEN", | ||
| "SLACK_APP_TOKEN", | ||
| "SLACK_USER_TOKEN", |
There was a problem hiding this comment.
Exclude SLACK_USER_TOKEN from fallback short-circuit keys
loadShellEnvFallback skips probing the login shell as soon as any expected key is already present (src/infra/shell-env.ts, hasAnyKey guard), so adding SLACK_USER_TOKEN here changes behavior: if a process has only SLACK_USER_TOKEN preset (for example via config.env) but relies on shell fallback for other credentials like OPENAI_API_KEY, fallback is now skipped and those keys stay unset. Before this change, that setup still triggered shell fallback, so this introduces a regression for GUI/launchd-style environments that depend on fallback hydration.
Useful? React with 👍 / 👎.
5fb517a to
1996f6f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1996f6f2b9
ℹ️ 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".
| const slackConfigured = | ||
| hasSecretInput(params.cfg.channels?.slack?.botToken) || | ||
| hasSecretInput(params.cfg.channels?.slack?.appToken) || | ||
| hasSecretInput(params.cfg.channels?.slack?.userToken) || |
There was a problem hiding this comment.
Require bot/app token for Slack exposure classification
In collectPluginsTrustFindings, counting channels.slack.userToken as enough to set slackConfigured means skillCommandsLikelyExposed can become true when only a user token is present, which can escalate plugins.extensions_no_allowlist to critical when commands.nativeSkills is enabled. That is inconsistent with runtime behavior: Slack startup still hard-fails without a bot token (and app token outside HTTP mode) in src/slack/monitor/provider.ts:109-115, so user-token-only configs do not actually expose native skill commands on Slack.
Useful? React with 👍 / 👎.
Summary
SLACK_USER_TOKENin expected shell-env key fallback listresolveSlackUserTokenfrom Slack index and add account-resolution coverage testsValidation
pnpm exec vitest run src/slack/accounts.test.tspnpm exec vitest run src/security/audit.test.ts -t "flags extensions without plugins.allow"Context
This PR is one focused slice extracted from: