refactor(sandbox): slack token override nits from PR #2151

scripts/nemoclaw-start.sh

@@ -415,9 +415,11 @@ 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.
   if [ "$(id -u)" -ne 0 ]; then
-    printf '[SECURITY] Slack token override ignored — requires root (non-root mode cannot write to config)\n' >&2
-    return 0
+    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
   fi
 
   local config_file="/sandbox/.openclaw/openclaw.json"
@@ -455,26 +457,29 @@ apply_slack_token_override() {
   SLACK_BOT_TOKEN="$SLACK_BOT_TOKEN" \
     SLACK_APP_TOKEN="${SLACK_APP_TOKEN:-}" \
     python3 - "$config_file" <<'PYSLACK'
-import json, os, sys
+import re, os, sys
 
 config_file = sys.argv[1]
 bot_token = os.environ["SLACK_BOT_TOKEN"]
 app_token = os.environ.get("SLACK_APP_TOKEN", "")
-placeholder_prefix = "openshell:resolve:env:"
 
 with open(config_file) as f:
-    cfg = json.load(f)
-
-slack = cfg.get("channels", {}).get("slack", {})
-default_acct = slack.get("accounts", {}).get("default", {})
+    content = f.read()
 
-if default_acct.get("botToken", "").startswith(placeholder_prefix):
-    default_acct["botToken"] = bot_token
-if app_token and default_acct.get("appToken", "").startswith(placeholder_prefix):
-    default_acct["appToken"] = app_token
+content = re.sub(
+    r'("botToken"\s*:\s*")openshell:resolve:env:SLACK_BOT_TOKEN(")',
+    lambda m: m.group(1) + bot_token + m.group(2),
+    content,
+)
+if app_token:
+    content = re.sub(
+        r'("appToken"\s*:\s*")openshell:resolve:env:SLACK_APP_TOKEN(")',
+        lambda m: m.group(1) + app_token + m.group(2),
+        content,
+    )
 
 with open(config_file, "w") as f:
-    json.dump(cfg, f, indent=2)
+    f.write(content)
 PYSLACK
 
   (cd /sandbox/.openclaw && sha256sum openclaw.json >"$hash_file")
@@ -959,13 +964,6 @@ if [ "$(id -u)" -ne 0 ]; then
   apply_model_override
   apply_cors_override
   apply_slack_token_override
-  # SECURITY: apply_slack_token_override is a no-op when non-root.
-  # If SLACK_BOT_TOKEN is still set here the placeholder was never resolved —
-  # Bolt will crash with invalid_auth at startup. Fail fast with a clear message.
-  if [ -n "${SLACK_BOT_TOKEN:-}" ]; 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
-    exit 1
-  fi
   export_gateway_token
   install_configure_guard
   configure_messaging_channels

test/nemoclaw-start.test.ts

@@ -510,11 +510,13 @@ describe("Slack token placeholder resolution (#2085)", () => {
     expect(fn[1]).toMatch(/\[ -n "\$\{SLACK_BOT_TOKEN:-\}" \] \|\| return 0/);
   });
 
-  it("only applies override in root mode", () => {
+  it("only applies override in root mode, fails fast when non-root and token is set", () => {
     const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m);
     expect(fn).toBeTruthy();
     expect(fn[1]).toMatch(/id -u.*-ne 0/);
-    expect(fn[1]).toContain("requires root");
+    expect(fn[1]).toContain("requires a root container");
+    // Non-root with SLACK_BOT_TOKEN set must return 1 (not silently skip)
+    expect(fn[1]).toMatch(/requires a root container[\s\S]*?return 1/);
   });
 
   it("guards against symlink attacks", () => {
@@ -568,13 +570,18 @@ describe("Slack token placeholder resolution (#2085)", () => {
   });
 
   it("fails fast when SLACK_BOT_TOKEN is set in non-root mode", () => {
+    // Fail-fast is now folded into apply_slack_token_override itself.
+    const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m);
+    expect(fn).toBeTruthy();
+    // Function must return 1 (not 0) when non-root and SLACK_BOT_TOKEN is set
+    expect(fn[1]).toMatch(/id -u.*-ne 0[\s\S]*?requires a root container[\s\S]*?return 1/);
+
+    // The non-root call site must not have a separate post-call SLACK_BOT_TOKEN check
     const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then([\s\S]*?)# ── Root path/);
     expect(nonRootBlock).toBeTruthy();
-    // After apply_slack_token_override (no-op without root) the non-root path must exit 1
-    expect(nonRootBlock[1]).toMatch(
-      /apply_slack_token_override[\s\S]*?SLACK_BOT_TOKEN[\s\S]*?exit 1/,
+    expect(nonRootBlock[1]).not.toMatch(
+      /apply_slack_token_override[\s\S]*?if \[ -n "\$\{SLACK_BOT_TOKEN/,
     );
-    expect(nonRootBlock[1]).toContain("requires a root container");
   });
 
   it("passes tokens via env prefix, not as positional args", () => {