fix(slack): skip monitor startup for disabled accounts [AI-assisted] by liuxiaopai-ai · Pull Request #30592 · openclaw/openclaw

liuxiaopai-ai · 2026-03-01T11:29:58Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: Slack socket-mode monitor could still be started for configs with channels.slack.enabled=false, causing unwanted websocket activity.
Why it matters: disabled channels should be inert; unexpected socket connections create log noise and can destabilize other providers.
What changed: added a fail-safe early guard in monitorSlackProvider that skips startup when resolved account is disabled and waits only for abort; added regression test asserting no auth.test call and no handler registration.
What did NOT change (scope boundary): no changes to Slack message handling, routing, or auth semantics for enabled accounts.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes Gateway opens Slack socket-mode connections when channel is disabled #30586
Related #

User-visible / Behavior Changes

Slack account with enabled=false no longer starts socket monitor even if tokens are still present in config.

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: local node/pnpm workspace
Model/provider: N/A
Integration/channel (if any): Slack socket-mode monitor
Relevant config (redacted): channels.slack.enabled=false, socket mode with tokens present

Steps

Configure Slack with enabled=false and valid-looking botToken/appToken.
Start monitor.
Observe whether socket initialization side effects run.

Expected

No startup auth/bootstrap calls and no event handlers registered for Slack monitor.

Actual

Matches expected after fix (covered by regression test).

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)
pnpm oxfmt --check src/slack/monitor/provider.ts src/slack/monitor.tool-result.test.ts CHANGELOG.md
pnpm oxlint --type-aware src/slack/monitor/provider.ts src/slack/monitor.tool-result.test.ts
pnpm vitest src/slack/monitor.tool-result.test.ts

Human Verification (required)

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

Verified scenarios: disabled Slack config short-circuits monitor startup and does not call auth.test or register event handlers.
Edge cases checked: function still blocks on abort signal in disabled mode, avoiding tight restart loops for long-running lifecycle callers.
What you did not verify: live Slack websocket behavior against real Slack credentials.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert this PR.
Files/config to restore: src/slack/monitor/provider.ts guard block.
Known bad symptoms reviewers should watch for: enabled Slack accounts unexpectedly not starting.

Risks and Mitigations

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

Risk: guard could be too broad and suppress startup in valid enabled cases.
- Mitigation: guard checks resolved account.enabled only; existing enabled-account tests continue to pass.

greptile-apps · 2026-03-01T11:36:01Z

Greptile Summary

This PR adds an early guard in monitorSlackProvider that prevents Slack socket monitor initialization when channels.slack.enabled=false, even if valid tokens are still present in the configuration.

Changes Made:

Added fail-safe guard after account resolution (lines 68-79 in provider.ts) that checks !account.enabled and skips all initialization
Moved runtime variable declaration earlier to enable logging from the guard
Guard waits for abort signal instead of returning immediately, preventing tight restart loops
Added comprehensive regression test verifying no auth.test call and no event handler registration when account is disabled

Impact:

Disabled Slack accounts no longer open websocket connections or perform auth bootstrap
Eliminates log noise and potential instability from disabled channels attempting startup
Change is backward compatible - existing enabled-account tests continue to pass

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The fix is well-implemented with a simple, focused guard that addresses the specific issue. The guard is placed at the optimal location (after account resolution, before any initialization), follows existing code patterns (same wait-for-abort pattern used elsewhere), and includes comprehensive test coverage. The 24 existing tests for enabled accounts verify no regression, and the new test confirms disabled accounts are properly handled. The CHANGELOG is appropriately updated.
No files require special attention

_{Last reviewed commit: d5b8b06}

aisle-research-bot · 2026-03-01T15:54:51Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🔵 Low	Indefinite await when Slack account disabled and abortSignal missing (startup hang/DoS)

1. 🔵 Indefinite await when Slack account disabled and abortSignal missing (startup hang/DoS)

Property	Value
Severity	Low
CWE	CWE-400
Location	`src/slack/monitor/provider.ts:68-77`

Description

In monitorSlackProvider, a new disabled-account branch awaits a Promise that only resolves when opts.abortSignal fires.

abortSignal is optional in MonitorSlackOpts.
If opts.abortSignal is undefined, the optional chaining prevents registering the listener, leaving the Promise unresolved forever.
Any caller that awaits monitorSlackProvider() (e.g., during startup/monitor supervision) can hang indefinitely, tying up worker resources and preventing clean shutdown/restart.

Vulnerable code:

if (!account.enabled) {
  runtime.log?.(`[${account.accountId}] slack account disabled; monitor startup skipped`);
  if (opts.abortSignal?.aborted) {
    return;
  }
  await new Promise<void>((resolve) => {
    opts.abortSignal?.addEventListener("abort", () => resolve(), { once: true });
  });
  return;
}

Call-site review in this repo shows some callers pass an abort signal (tests and the Slack plugin gateway), but the exported API still allows usage without one, and opts defaults to {}.

Recommendation

Avoid awaiting a Promise that cannot resolve.

Safer patterns:

Return immediately when disabled (simplest):

if (!account.enabled) {
  runtime.log?.(`[${account.accountId}] slack account disabled; monitor startup skipped`);
  return;
}

If you intentionally want to block until shutdown to prevent supervisor restart loops, ensure it only blocks when a real signal is provided (otherwise resolve immediately):

if (!account.enabled) {
  runtime.log?.(`[${account.accountId}] slack account disabled; monitor startup skipped`);
  const signal = opts.abortSignal;
  if (!signal) return; // no way to be aborted
  if (signal.aborted) return;
  await new Promise<void>((resolve) => {
    signal.addEventListener("abort", () => resolve(), { once: true });
  });
  return;
}

Optionally factor this into a helper like onceAborted(signal?: AbortSignal): Promise<void> that returns immediately when signal is missing/already aborted.

Analyzed PR: #30592 at commit f32876b

Takhoffman · 2026-03-01T16:20:06Z

PR #30592 - fix(slack): skip monitor startup for disabled accounts [AI-assisted] (#30592)

Merged via squash.

Merge commit: 265b22c
Verified: pnpm install --frozen-lockfile, pnpm build, pnpm check, pnpm test:macmini
Changes made:
Rebased PR head onto latest main; no additional functional code edits beyond the PR content.
Why these changes were made:
Rebase was required to produce a clean merge commit after main moved.
Changelog: CHANGELOG.md updated=true required=true opt_out=false

Thanks @liuxiaopai-ai!

@liuxiaopai-ai

…penclaw#30592) thanks @liuxiaopai-ai Cherry-pick of upstream 265b22c.

@liuxiaopai-ai