-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(channels): prevent Slack auth failure from crashing gateway (#2340) #2355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5f44cde
bd61cc1
34009b3
70ce88e
2f7ac9b
9cb663e
3b2e325
e335a1b
21bd3c0
16bd748
169564b
4f12cab
e378e72
c145798
bdf4f25
70f2206
cd8ca6b
0dc68d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -389,12 +389,13 @@ PYCORS | |
| apply_slack_token_override() { | ||
| [ -n "${SLACK_BOT_TOKEN:-}" ] || return 0 | ||
|
|
||
| # SECURITY: Only root can write to /sandbox/.openclaw (root:root 444). | ||
| # Non-root with SLACK_BOT_TOKEN set means the placeholder can never be resolved — | ||
| # Bolt will crash with invalid_auth. Fail fast rather than silently skip. | ||
| # Non-root cannot write to /sandbox/.openclaw (root:root 444), so the | ||
| # placeholder token cannot be resolved here. Log a warning and continue — | ||
| # the Slack channel guard will catch the inevitable auth failure at runtime | ||
| # without crashing the gateway. Ref: #2340 | ||
| if [ "$(id -u)" -ne 0 ]; then | ||
| printf '[SECURITY] Slack Socket Mode requires a root container — SLACK_BOT_TOKEN is set but token placeholder resolution needs root. Run the container as root or remove SLACK_BOT_TOKEN.\n' >&2 | ||
| return 1 | ||
| printf '[channels] Slack token override skipped (non-root) — channel guard will handle auth failure at runtime\n' >&2 | ||
| return 0 | ||
| fi | ||
|
|
||
| local config_file="/sandbox/.openclaw/openclaw.json" | ||
|
|
@@ -465,6 +466,131 @@ PYSLACK | |
| printf '[channels] Config hash recomputed after Slack token override\n' >&2 | ||
| } | ||
|
|
||
| # ── Slack channel guard (unhandled-rejection safety net) ───────── | ||
| # Prevents the gateway from crashing when a Slack channel fails to | ||
| # initialize (e.g., invalid_auth, token_revoked, unresolved placeholder | ||
| # tokens). Instead of modifying openclaw.json (which is Landlock | ||
| # read-only at runtime), this injects a Node.js preload via | ||
| # NODE_OPTIONS that catches unhandled promise rejections originating | ||
| # from Slack channel initialization and logs them as warnings instead | ||
| # of letting Node v22 treat them as fatal. | ||
| # | ||
| # Same pattern as the HTTP proxy fix (_PROXY_FIX_SCRIPT) and the | ||
| # WebSocket CONNECT fix (_WS_FIX_SCRIPT). | ||
| # | ||
| # Ref: https://github.com/NVIDIA/NemoClaw/issues/2340 | ||
| _SLACK_GUARD_SCRIPT="/tmp/nemoclaw-slack-channel-guard.js" | ||
|
|
||
| install_slack_channel_guard() { | ||
| local config_file="/sandbox/.openclaw/openclaw.json" | ||
|
|
||
| # Only install if a Slack channel is configured | ||
| if ! grep -q '"slack"' "$config_file" 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
|
|
||
| printf '[channels] Installing Slack channel guard (unhandled-rejection safety net)\n' >&2 | ||
|
|
||
| emit_sandbox_sourced_file "$_SLACK_GUARD_SCRIPT" <<'SLACK_GUARD_EOF' | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // slack-channel-guard.js — catches unhandled promise rejections from Slack | ||
| // channel initialization so a single channel auth failure does not crash | ||
| // the entire OpenClaw gateway. Node v22 treats unhandled rejections as | ||
| // fatal (--unhandled-rejections=throw is the default), taking down | ||
| // inference, chat, and TUI alongside the failed Slack channel. | ||
| // | ||
| // This preload installs a process-level handler that detects Slack-specific | ||
| // rejections (by error code or stack trace) and logs a warning instead of | ||
| // crashing. Non-Slack rejections are re-thrown to preserve normal behavior. | ||
| // | ||
| // Ref: https://github.com/NVIDIA/NemoClaw/issues/2340 | ||
|
|
||
| (function () { | ||
| 'use strict'; | ||
|
|
||
| // Slack-specific error codes from @slack/web-api that indicate auth failure. | ||
| // These appear as error.code on the WebAPIRequestError or CodedError objects. | ||
| var SLACK_AUTH_ERRORS = [ | ||
| 'slack_webapi_platform_error', | ||
| 'slack_webapi_request_error', | ||
| 'slackbot_error', | ||
| ]; | ||
|
|
||
| // Slack-specific error messages that indicate auth/token problems. | ||
| var SLACK_AUTH_MESSAGES = [ | ||
| 'invalid_auth', | ||
| 'not_authed', | ||
| 'token_revoked', | ||
| 'token_expired', | ||
| 'account_inactive', | ||
| 'missing_scope', | ||
| 'not_allowed_token_type', | ||
| 'An API error occurred: invalid_auth', | ||
| ]; | ||
|
|
||
| function isSlackRejection(reason) { | ||
| if (!reason) return false; | ||
|
|
||
| // Check error code (Slack SDK sets .code on its errors) | ||
| var code = reason.code || ''; | ||
| for (var i = 0; i < SLACK_AUTH_ERRORS.length; i++) { | ||
| if (code === SLACK_AUTH_ERRORS[i]) return true; | ||
| } | ||
|
|
||
| // Check error message | ||
| var msg = String(reason.message || reason); | ||
| for (var j = 0; j < SLACK_AUTH_MESSAGES.length; j++) { | ||
| if (msg.indexOf(SLACK_AUTH_MESSAGES[j]) !== -1) return true; | ||
| } | ||
|
|
||
| // Check stack trace for @slack/ packages | ||
| var stack = reason.stack || ''; | ||
| if (stack.indexOf('@slack/') !== -1 || stack.indexOf('slack-') !== -1) { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| function handleSlackError(reason, source) { | ||
| if (isSlackRejection(reason)) { | ||
| var msg = (reason && reason.message) ? reason.message : String(reason); | ||
| process.stderr.write( | ||
| '[channels] [slack] provider failed to start: ' + msg + | ||
| ' \u2014 ' + source + ' caught by safety net, gateway continues\n' | ||
| ); | ||
| return true; // handled | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Catch async Slack errors (rejected promises from @slack/web-api). | ||
| process.on('unhandledRejection', function (reason, promise) { | ||
| if (handleSlackError(reason, 'unhandledRejection')) return; | ||
| // Non-Slack: re-throw to preserve default --unhandled-rejections=throw. | ||
| throw reason; | ||
| }); | ||
|
|
||
| // Catch sync Slack errors (e.g., Bolt token format validation throws | ||
| // synchronously when appToken doesn't start with xapp-). | ||
| process.on('uncaughtException', function (err, origin) { | ||
| if (handleSlackError(err, 'uncaughtException')) return; | ||
| // Non-Slack: re-throw to preserve normal crash behavior. | ||
| // Print the error first since re-throw inside uncaughtException handler | ||
| // may not print the original stack. | ||
| process.stderr.write(err.stack || String(err)); | ||
| process.stderr.write('\n'); | ||
| process.exit(1); | ||
| }); | ||
| })(); | ||
| SLACK_GUARD_EOF | ||
|
|
||
| export NODE_OPTIONS="${NODE_OPTIONS:+$NODE_OPTIONS }--require $_SLACK_GUARD_SCRIPT" | ||
| printf '[channels] Slack channel guard installed (NODE_OPTIONS updated)\n' >&2 | ||
|
Comment on lines
+590
to
+591
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate Slack preload file permissions before launching Node processes. At Line 589-590, the new Suggested hardening patch@@
- validate_tmp_permissions "$_PROXY_FIX_SCRIPT"
+ validate_tmp_permissions "$_PROXY_FIX_SCRIPT"
+ if [ -f "$_SLACK_GUARD_SCRIPT" ]; then
+ validate_tmp_permissions "$_SLACK_GUARD_SCRIPT"
+ fi
@@
-validate_tmp_permissions "$_PROXY_FIX_SCRIPT"
+validate_tmp_permissions "$_PROXY_FIX_SCRIPT"
+if [ -f "$_SLACK_GUARD_SCRIPT" ]; then
+ validate_tmp_permissions "$_SLACK_GUARD_SCRIPT"
+fi🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| _read_gateway_token() { | ||
| python3 - <<'PYTOKEN' | ||
| import json | ||
|
|
@@ -1129,6 +1255,7 @@ if [ "$(id -u)" -ne 0 ]; then | |
| export_gateway_token | ||
| install_configure_guard | ||
| configure_messaging_channels | ||
| install_slack_channel_guard | ||
| validate_openclaw_symlinks | ||
|
|
||
| # Ensure writable state directories exist and are owned by the current user. | ||
|
|
@@ -1248,6 +1375,7 @@ install_configure_guard | |
| # Must run AFTER integrity check (to detect build-time tampering) and | ||
| # BEFORE chattr +i (which locks the config permanently). | ||
| configure_messaging_channels | ||
| install_slack_channel_guard | ||
|
|
||
| # Write auth profile as sandbox user (needs writable .openclaw-data) | ||
| # and recursively re-tighten any auth-profiles.json files under ~/.openclaw. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,10 @@ | |
| # TELEGRAM_ALLOWED_IDS — comma-separated Telegram user IDs for DM allowlisting | ||
| # TELEGRAM_BOT_TOKEN_REAL — optional: enables Phase 6 real round-trip | ||
| # DISCORD_BOT_TOKEN_REAL — optional: enables Phase 6 real round-trip | ||
| # SLACK_BOT_TOKEN — defaults to fake token (xoxb-fake-...) | ||
| # SLACK_APP_TOKEN — defaults to fake token (xapp-fake-...) | ||
| # SLACK_BOT_TOKEN_REVOKED — optional: revoked xoxb- token to test auth pre-validation (#2340) | ||
| # SLACK_APP_TOKEN_REVOKED — optional: paired xapp- token for the revoked bot token | ||
|
Comment on lines
+45
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation for Slack environment variables is incomplete. The documented 🤖 Prompt for AI Agents |
||
| # TELEGRAM_CHAT_ID_E2E — optional: enables sendMessage test | ||
| # NEMOCLAW_E2E_STRICT_DISCORD_GATEWAY — fail instead of skip on known Discord gateway blockers | ||
| # | ||
|
|
@@ -98,9 +102,13 @@ register_sandbox_for_teardown "$SANDBOX_NAME" | |
| # Default to fake tokens if not provided | ||
| TELEGRAM_TOKEN="${TELEGRAM_BOT_TOKEN:-test-fake-telegram-token-e2e}" | ||
| DISCORD_TOKEN="${DISCORD_BOT_TOKEN:-test-fake-discord-token-e2e}" | ||
| SLACK_TOKEN="${SLACK_BOT_TOKEN:-xoxb-fake-slack-token-e2e}" | ||
| SLACK_APP="${SLACK_APP_TOKEN:-xapp-fake-slack-app-token-e2e}" | ||
| TELEGRAM_IDS="${TELEGRAM_ALLOWED_IDS:-123456789,987654321}" | ||
| export TELEGRAM_BOT_TOKEN="$TELEGRAM_TOKEN" | ||
| export DISCORD_BOT_TOKEN="$DISCORD_TOKEN" | ||
| export SLACK_BOT_TOKEN="$SLACK_TOKEN" | ||
| export SLACK_APP_TOKEN="$SLACK_APP" | ||
| export TELEGRAM_ALLOWED_IDS="$TELEGRAM_IDS" | ||
|
|
||
| # Run a command inside the sandbox via stdin (avoids exposing sensitive args in process list) | ||
|
|
@@ -164,6 +172,8 @@ pass "Docker is running" | |
|
|
||
| info "Telegram token: ${TELEGRAM_TOKEN:0:10}... (${#TELEGRAM_TOKEN} chars)" | ||
| info "Discord token: ${DISCORD_TOKEN:0:10}... (${#DISCORD_TOKEN} chars)" | ||
| info "Slack bot token: configured (${#SLACK_TOKEN} chars)" | ||
| info "Slack app token: configured (${#SLACK_APP} chars)" | ||
| info "Sandbox name: $SANDBOX_NAME" | ||
| STRICT_DISCORD_GATEWAY="${NEMOCLAW_E2E_STRICT_DISCORD_GATEWAY:-0}" | ||
|
|
||
|
|
@@ -559,6 +569,37 @@ print(account.get('groupPolicy', '')) | |
| else | ||
| skip "M11d: Telegram groupPolicy not set (channel may not be configured)" | ||
| fi | ||
|
|
||
| # M11e: Slack channel configured — gateway must survive auth failure (#2340) | ||
| # The Slack channel has placeholder tokens that will fail auth. The channel | ||
| # guard preload (NODE_OPTIONS --require) should catch the error. We can't | ||
| # verify the guard file via SSH (different container), but we CAN check the | ||
| # gateway port from here. This is tested more thoroughly in Phase 7. | ||
| slack_configured=$(echo "$channel_json" | python3 -c " | ||
| import json, sys | ||
| d = json.load(sys.stdin) | ||
| print('yes' if 'slack' in d else 'no') | ||
| " 2>/dev/null || true) | ||
| if [ "$slack_configured" = "yes" ]; then | ||
| pass "M11e: Slack channel configured with placeholder tokens (guard needed)" | ||
|
|
||
| # Diagnostics: check if the guard was installed and what NODE_OPTIONS looks like | ||
| info "Checking guard installation diagnostics (via openshell exec as root):" | ||
| guard_exists=$(openshell sandbox exec --name "$SANDBOX_NAME" -- ls -la /tmp/nemoclaw-slack-channel-guard.js 2>/dev/null || echo "EXEC_FAILED") | ||
| info " Guard file: $guard_exists" | ||
| node_opts=$(openshell sandbox exec --name "$SANDBOX_NAME" -- bash -c 'echo "$NODE_OPTIONS"' 2>/dev/null || echo "EXEC_FAILED") | ||
| info " NODE_OPTIONS: $node_opts" | ||
| proxy_fix=$(openshell sandbox exec --name "$SANDBOX_NAME" -- ls -la /tmp/nemoclaw-http-proxy-fix.js 2>/dev/null || echo "EXEC_FAILED") | ||
| info " Proxy fix file: $proxy_fix" | ||
| # Check what processes are running | ||
| procs=$(openshell sandbox exec --name "$SANDBOX_NAME" -- ps aux 2>/dev/null | head -10 || echo "EXEC_FAILED") | ||
| info " Processes:" | ||
| echo "$procs" | while IFS= read -r line; do | ||
| info " $line" | ||
| done | ||
| else | ||
| skip "M11e: No Slack channel in config" | ||
| fi | ||
| fi | ||
|
|
||
| # ══════════════════════════════════════════════════════════════════ | ||
|
|
@@ -1043,9 +1084,56 @@ else | |
| fi | ||
|
|
||
| # ══════════════════════════════════════════════════════════════════ | ||
| # Phase 7: Cleanup | ||
| # Phase 7: Slack channel guard (#2340) | ||
| # | ||
| # The sandbox was installed with fake Slack tokens. The channel guard | ||
| # preload (NODE_OPTIONS --require) should catch the unhandled rejection | ||
| # from @slack/web-api and keep the gateway alive. | ||
| # ══════════════════════════════════════════════════════════════════ | ||
| section "Phase 7: Slack channel guard (#2340)" | ||
|
|
||
| # S1: Gateway is serving on port 18789 — the guard caught the Slack rejection | ||
| gw_port=$(sandbox_exec 'node -e " | ||
| const net = require(\"net\"); | ||
| const sock = net.connect(18789, \"127.0.0.1\"); | ||
| sock.on(\"connect\", () => { console.log(\"OPEN\"); sock.end(); }); | ||
| sock.on(\"error\", () => console.log(\"CLOSED\")); | ||
| setTimeout(() => { console.log(\"TIMEOUT\"); sock.destroy(); }, 5000); | ||
| "' 2>/dev/null || true) | ||
| if echo "$gw_port" | grep -q "OPEN"; then | ||
| pass "S1: Gateway is serving on port 18789 — Slack auth failure did not crash it" | ||
| else | ||
| fail "S1: Gateway is not serving on port 18789 (${gw_port:0:200})" | ||
| fi | ||
|
|
||
| # S2: Dump gateway.log for diagnostics (must use openshell exec — SSH user | ||
| # cannot read the file because it's 600 gateway:gateway). | ||
| gw_log=$(openshell sandbox exec --name "$SANDBOX_NAME" -- cat /tmp/gateway.log 2>/dev/null || true) | ||
| if [ -z "$gw_log" ]; then | ||
| # Container may have already exited | ||
| gw_log=$(nemoclaw "$SANDBOX_NAME" logs 2>&1 | tail -200 || true) | ||
| fi | ||
|
|
||
| info "Gateway log (last 30 lines):" | ||
| echo "$gw_log" | tail -30 | while IFS= read -r line; do | ||
| info " $line" | ||
| done | ||
|
|
||
| if echo "$gw_log" | grep -q "provider failed to start:.*gateway continues"; then | ||
| pass "S2: Gateway log shows Slack rejection was caught by channel guard" | ||
| elif echo "$gw_log" | grep -qi "slack"; then | ||
| info "Slack-related lines: $(echo "$gw_log" | grep -i slack | head -5)" | ||
| skip "S2: Gateway log has Slack output but not the guard catch message" | ||
| elif [ -z "$gw_log" ]; then | ||
| skip "S2: Could not read gateway log (container may have exited)" | ||
| else | ||
| skip "S2: No Slack-related output in gateway log" | ||
| fi | ||
|
|
||
| # ══════════════════════════════════════════════════════════════════ | ||
| # Phase 8: Cleanup | ||
| # ══════════════════════════════════════════════════════════════════ | ||
| section "Phase 7: Cleanup" | ||
| section "Phase 8: Cleanup" | ||
|
|
||
| info "Destroying sandbox '$SANDBOX_NAME'..." | ||
| [[ "${NEMOCLAW_E2E_KEEP_SANDBOX:-}" = "1" ]] || nemoclaw "$SANDBOX_NAME" destroy --yes 2>/dev/null || true | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In@slack/web-api, what conditions produceslack_webapi_request_errorversusslack_webapi_platform_error, and which of those represent transient/network failures instead of definitive auth failures?💡 Result:
In
@slack/web-api, slack_webapi_request_error (WebAPIRequestError) occurs when the HTTP request fails to send or complete due to client-side issues before reaching Slack's servers. This includes network connectivity problems (e.g., cannot reach api.slack.com), DNS resolution failures (ENOTFOUND), connection resets (ECONNRESET), timeouts (ETIMEDOUT), or other Axios HTTP client errors. The error wraps an original Error object with details like error.original.code. slack_webapi_platform_error (WebAPIPlatformError) occurs when Slack's API returns a response with ok: false and an error field (e.g., 'invalid_auth', 'not_authed', 'invalid_arguments', 'account_inactive'). This indicates business logic failures like invalid tokens, missing scopes, invalid parameters, or resource issues. The error includes data with the full Slack response. slack_webapi_http_error (WebAPIHTTPError) is for unexpected HTTP status codes (non-200/429 responses, e.g., proxy issues), and slack_webapi_rate_limited_error (WebAPIRateLimitedError) for 429 rate limits. Transient/network failures are represented by slack_webapi_request_error (e.g., timeouts, connection issues that may resolve quickly) and slack_webapi_http_error (if 5xx server errors, though rare as Slack uses 200 for errors). The SDK automatically retries these with exponential backoff (up to 10 attempts over ~30 minutes by default). Definitive auth failures are slack_webapi_platform_error with specific data.error values like 'invalid_auth', 'not_authed', or 'missing_scope'. These should not be retried without fixing the token/scopes, as they indicate permanent client-side issues rather than transient problems. The SDK does not retry PlatformErrors.Citations:
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 86
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 91
🏁 Script executed:
sed -n '512,553p' ./scripts/nemoclaw-start.shRepository: NVIDIA/NemoClaw
Length of output: 1342
🏁 Script executed:
Repository: NVIDIA/NemoClaw
Length of output: 1293
🏁 Script executed:
sed -n '500,560p' ./scripts/nemoclaw-start.shRepository: NVIDIA/NemoClaw
Length of output: 2054
🏁 Script executed:
sed -n '490,510p' ./scripts/nemoclaw-start.shRepository: NVIDIA/NemoClaw
Length of output: 1041
🏁 Script executed:
rg "slack_webapi_request_error|slack_webapi_platform_error" ./scripts/nemoclaw-start.sh -B 2 -A 2Repository: NVIDIA/NemoClaw
Length of output: 263
Restrict error detection to definitive auth failures only.
isSlackRejection()includesslack_webapi_request_errorinSLACK_AUTH_ERRORS, but according to the official@slack/web-apidocumentation,slack_webapi_request_errorrepresents transient client-side HTTP failures (network timeouts, DNS resolution failures, connection resets) that the SDK automatically retries with exponential backoff. Onlyslack_webapi_platform_errorrepresents definitive auth failures likeinvalid_auth,not_authed, ormissing_scopewhere the SDK does not retry.By treating transient failures as auth failures, this code suppresses recoverable network issues and prevents the SDK's built-in retry mechanism. Additionally, the stack trace check (
@slack/orslack-) is overly broad and will catch non-auth SDK bugs, swallowing failures that should propagate normally.Remove
slack_webapi_request_errorfromSLACK_AUTH_ERRORS. Keep only the explicit auth/token messages inSLACK_AUTH_MESSAGESand remove the stack trace check, letting transient failures be retried by the SDK and non-auth failures crash as expected.🤖 Prompt for AI Agents