Skip to content

Slack: honor SLACK_USER_TOKEN in account resolution and audit#32570

Open
zwright8 wants to merge 2 commits intoopenclaw:mainfrom
zwright8:codex/pr26712-slack-user-token-account-resolution
Open

Slack: honor SLACK_USER_TOKEN in account resolution and audit#32570
zwright8 wants to merge 2 commits intoopenclaw:mainfrom
zwright8:codex/pr26712-slack-user-token-account-resolution

Conversation

@zwright8
Copy link
Copy Markdown

@zwright8 zwright8 commented Mar 3, 2026

Summary

  • split from Fix status/token regressions and Teams large-file uploads #26712 into a focused Slack token-handling PR
  • include SLACK_USER_TOKEN in expected shell-env key fallback list
  • account for Slack user-token configuration/env in plugin trust audit exposure checks
  • export resolveSlackUserToken from Slack index and add account-resolution coverage tests

Validation

  • pnpm exec vitest run src/slack/accounts.test.ts
  • pnpm exec vitest run src/security/audit.test.ts -t "flags extensions without plugins.allow"

Context

This PR is one focused slice extracted from:

@openclaw-barnacle openclaw-barnacle bot added channel: slack Channel integration: slack size: S labels Mar 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR extends Slack's SLACK_USER_TOKEN support across three areas: it adds the env key to the shell-env allowlist in src/config/io.ts, expands the slackConfigured detection in the audit module to cover userToken at the config, per-account, and environment levels, and exports resolveSlackUserToken from the Slack public index. New account-resolution tests verify env fallback for the default account (with whitespace trimming), config-over-env precedence, and non-default account isolation.

  • src/config/io.ts: SLACK_USER_TOKEN added to SHELL_ENV_EXPECTED_KEYS — consistent with SLACK_BOT_TOKEN/SLACK_APP_TOKEN.
  • src/security/audit-extra.async.ts: userToken coverage added at all three tiers of the slackConfigured check (top-level secret input, per-account secret input, process.env), keeping the logic parity with botToken/appToken.
  • src/security/audit.test.ts: The existing "flags extensions without plugins.allow" test now correctly saves, clears, and restores SLACK_USER_TOKEN in its try/finally block — no positive audit path test for SLACK_USER_TOKEN → critical severity, but this is a minor gap since the accounts.test.ts covers token resolution thoroughly.
  • src/slack/accounts.test.ts: Three new well-structured tests with a proper afterEach teardown.
  • src/slack/index.ts: resolveSlackUserToken cleanly added to the existing token export line.

Confidence Score: 5/5

  • This PR is safe to merge — all changes are additive, consistent with existing patterns, and accompanied by targeted tests.
  • All changes are mechanical extensions of established patterns (adding a new token type alongside two existing ones). The audit logic, config allowlist, and account resolution are updated consistently. Test coverage in accounts.test.ts is thorough for the new token-resolution behavior. The only minor gap is that no audit test exercises the SLACK_USER_TOKEN → critical-severity path, but this doesn't affect correctness.
  • No files require special attention.

Last reviewed commit: 5fb517a

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: 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",
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 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 👍 / 👎.

@zwright8 zwright8 force-pushed the codex/pr26712-slack-user-token-account-resolution branch from 5fb517a to 1996f6f Compare March 3, 2026 05:07
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: 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) ||
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 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: slack Channel integration: slack size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant