mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:20:43 +00:00
fix(slack): tolerate unresolved channel SecretRef on outbound send path (#68954)
Merged via squash.
Prepared head SHA: f13c639c26
Co-authored-by: openperf <80630709+openperf@users.noreply.github.com>
Co-authored-by: openperf <80630709+openperf@users.noreply.github.com>
Reviewed-by: @openperf
This commit is contained in:
@@ -37,6 +37,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Telegram/setup: require numeric `allowFrom` user IDs during setup instead of offering unsupported `@username` DM resolution, and point operators to `from.id`/`getUpdates` for discovery. (#69191) Thanks @obviyus.
|
||||
- GitHub Copilot/onboarding: default GitHub Copilot setup to `claude-opus-4.6` and keep the bundled default model list aligned, so new Copilot setups no longer start on the older `gpt-4o` default. (#69207) Thanks @obviyus.
|
||||
- Gateway/status: separate reachability, capability, and read-probe reporting so connect-only or scope-limited sessions no longer look fully healthy, and normalize SSH targets entered as `ssh user@host`. (#69215) Thanks @obviyus.
|
||||
- Slack: fix outbound replies failing with "unresolved SecretRef" for accounts configured via `file` or `exec` secret sources; the send path now tolerates the runtime snapshot retaining an unresolved channel SecretRef when a boot-resolved token override is already available. (#68954) Thanks @openperf.
|
||||
|
||||
## 2026.4.19-beta.2
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveSlackAccount } from "./accounts.js";
|
||||
|
||||
@@ -107,3 +108,212 @@ describe("resolveSlackAccount allowFrom precedence", () => {
|
||||
expect(resolved.config.dm?.allowFrom).toEqual(["U123"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveSlackAccount tolerateUnresolvedSecrets", () => {
|
||||
// The static `SlackAccountConfig.botToken` type is `string` because it
|
||||
// models the post-resolution shape, but the runtime cfg snapshot can still
|
||||
// hold an unresolved `SecretRef` object for inactive channel targets (per
|
||||
// the inspect/strict separation in #66818). Cast via `unknown` so the test
|
||||
// can construct that runtime-only shape without weakening the production
|
||||
// type. See #68237.
|
||||
const cfgWithUnresolvedBotTokenRef = {
|
||||
channels: {
|
||||
slack: {
|
||||
accounts: {
|
||||
default: {
|
||||
botToken: { source: "exec", provider: "default", id: "slack_bot_token" },
|
||||
allowFrom: ["U999"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as OpenClawConfig;
|
||||
|
||||
it("throws by default when the snapshot still holds an unresolved SecretRef botToken", () => {
|
||||
expect(() =>
|
||||
resolveSlackAccount({
|
||||
cfg: cfgWithUnresolvedBotTokenRef,
|
||||
accountId: "default",
|
||||
}),
|
||||
).toThrowError(/channels\.slack\.accounts\.default\.botToken/);
|
||||
});
|
||||
|
||||
it("returns undefined credentials without throwing when tolerateUnresolvedSecrets is set", () => {
|
||||
const resolved = resolveSlackAccount({
|
||||
cfg: cfgWithUnresolvedBotTokenRef,
|
||||
accountId: "default",
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
|
||||
expect(resolved.botToken).toBeUndefined();
|
||||
expect(resolved.botTokenSource).toBe("none");
|
||||
// Surrounding account info still resolves so callers with an explicit
|
||||
// override (for example sendMessageSlack receiving opts.token) can keep
|
||||
// operating.
|
||||
expect(resolved.accountId).toBe("default");
|
||||
expect(resolved.config.allowFrom).toEqual(["U999"]);
|
||||
});
|
||||
|
||||
it("still returns resolved string credentials in tolerant mode", () => {
|
||||
const resolved = resolveSlackAccount({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: {
|
||||
accounts: {
|
||||
default: { botToken: "xoxb-resolved", appToken: "xapp-resolved" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
accountId: "default",
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
|
||||
expect(resolved.botToken).toBe("xoxb-resolved");
|
||||
expect(resolved.botTokenSource).toBe("config");
|
||||
expect(resolved.appToken).toBe("xapp-resolved");
|
||||
expect(resolved.appTokenSource).toBe("config");
|
||||
});
|
||||
|
||||
it("does not silently fall back to SLACK_*_TOKEN env vars in tolerant mode when all credentials are configured as SecretRef (credential confusion guard)", () => {
|
||||
// Each credential is configured as a SecretRef. In tolerant mode none of
|
||||
// them resolves, so per-credential env gating must block all three env
|
||||
// vars; otherwise a stray `SLACK_*_TOKEN` would silently impersonate the
|
||||
// operator-configured account (CWE-287 credential confusion).
|
||||
const cfgAllSecretRefs = {
|
||||
channels: {
|
||||
slack: {
|
||||
accounts: {
|
||||
default: {
|
||||
botToken: { source: "exec", provider: "default", id: "slack_bot_token" },
|
||||
appToken: { source: "exec", provider: "default", id: "slack_app_token" },
|
||||
userToken: { source: "exec", provider: "default", id: "slack_user_token" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as OpenClawConfig;
|
||||
const previousBotToken = process.env.SLACK_BOT_TOKEN;
|
||||
const previousAppToken = process.env.SLACK_APP_TOKEN;
|
||||
const previousUserToken = process.env.SLACK_USER_TOKEN;
|
||||
process.env.SLACK_BOT_TOKEN = "xoxb-env-fallback";
|
||||
process.env.SLACK_APP_TOKEN = "xapp-env-fallback";
|
||||
process.env.SLACK_USER_TOKEN = "xoxp-env-fallback";
|
||||
try {
|
||||
const resolved = resolveSlackAccount({
|
||||
cfg: cfgAllSecretRefs,
|
||||
accountId: "default",
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
|
||||
expect(resolved.botToken).toBeUndefined();
|
||||
expect(resolved.botTokenSource).toBe("none");
|
||||
expect(resolved.appToken).toBeUndefined();
|
||||
expect(resolved.appTokenSource).toBe("none");
|
||||
expect(resolved.userToken).toBeUndefined();
|
||||
expect(resolved.userTokenSource).toBe("none");
|
||||
} finally {
|
||||
if (previousBotToken === undefined) {
|
||||
delete process.env.SLACK_BOT_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_BOT_TOKEN = previousBotToken;
|
||||
}
|
||||
if (previousAppToken === undefined) {
|
||||
delete process.env.SLACK_APP_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_APP_TOKEN = previousAppToken;
|
||||
}
|
||||
if (previousUserToken === undefined) {
|
||||
delete process.env.SLACK_USER_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_USER_TOKEN = previousUserToken;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves SLACK_BOT_TOKEN env fallback in tolerant mode when no config token is set (env-only setups)", () => {
|
||||
const previousBotToken = process.env.SLACK_BOT_TOKEN;
|
||||
const previousAppToken = process.env.SLACK_APP_TOKEN;
|
||||
process.env.SLACK_BOT_TOKEN = "xoxb-env-only";
|
||||
process.env.SLACK_APP_TOKEN = "xapp-env-only";
|
||||
try {
|
||||
// No SecretRef and no string token configured for the default account:
|
||||
// env fallback must still fire so env-only deployments (relying solely
|
||||
// on SLACK_BOT_TOKEN / SLACK_APP_TOKEN) keep working when callers like
|
||||
// `channel.ts` invoke sendMessageSlack without an explicit override.
|
||||
const resolved = resolveSlackAccount({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: {
|
||||
accounts: {
|
||||
default: { allowFrom: ["U001"] },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
accountId: "default",
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
|
||||
expect(resolved.botToken).toBe("xoxb-env-only");
|
||||
expect(resolved.botTokenSource).toBe("env");
|
||||
expect(resolved.appToken).toBe("xapp-env-only");
|
||||
expect(resolved.appTokenSource).toBe("env");
|
||||
} finally {
|
||||
if (previousBotToken === undefined) {
|
||||
delete process.env.SLACK_BOT_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_BOT_TOKEN = previousBotToken;
|
||||
}
|
||||
if (previousAppToken === undefined) {
|
||||
delete process.env.SLACK_APP_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_APP_TOKEN = previousAppToken;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("blocks env fallback per-credential: unresolved SecretRef on botToken does not leak SLACK_APP_TOKEN", () => {
|
||||
const previousBotToken = process.env.SLACK_BOT_TOKEN;
|
||||
const previousAppToken = process.env.SLACK_APP_TOKEN;
|
||||
process.env.SLACK_BOT_TOKEN = "xoxb-env-bot";
|
||||
process.env.SLACK_APP_TOKEN = "xapp-env-app";
|
||||
try {
|
||||
// botToken has an unresolved SecretRef (env fallback should be
|
||||
// blocked), but appToken is unset (env fallback should still fire).
|
||||
// This proves the gating is per-credential, not whole-account.
|
||||
const resolved = resolveSlackAccount({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: {
|
||||
accounts: {
|
||||
default: {
|
||||
botToken: { source: "exec", provider: "default", id: "slack_bot_token" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as OpenClawConfig,
|
||||
accountId: "default",
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
|
||||
expect(resolved.botToken).toBeUndefined();
|
||||
expect(resolved.botTokenSource).toBe("none");
|
||||
// appToken was never configured → env fallback still fires.
|
||||
expect(resolved.appToken).toBe("xapp-env-app");
|
||||
expect(resolved.appTokenSource).toBe("env");
|
||||
} finally {
|
||||
if (previousBotToken === undefined) {
|
||||
delete process.env.SLACK_BOT_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_BOT_TOKEN = previousBotToken;
|
||||
}
|
||||
if (previousAppToken === undefined) {
|
||||
delete process.env.SLACK_APP_TOKEN;
|
||||
} else {
|
||||
process.env.SLACK_APP_TOKEN = previousAppToken;
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
resolveMergedAccountConfig,
|
||||
type OpenClawConfig,
|
||||
} from "openclaw/plugin-sdk/account-resolution";
|
||||
import { isSecretRef, normalizeSecretInputString } from "openclaw/plugin-sdk/secret-input";
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
import type { SlackAccountSurfaceFields } from "./account-surface-fields.js";
|
||||
import type { SlackAccountConfig } from "./runtime-api.js";
|
||||
@@ -35,10 +36,8 @@ export function mergeSlackAccountConfig(
|
||||
accountId: string,
|
||||
): SlackAccountConfig {
|
||||
return resolveMergedAccountConfig<SlackAccountConfig>({
|
||||
channelConfig: cfg.channels?.slack as SlackAccountConfig | undefined,
|
||||
accounts: cfg.channels?.slack?.accounts as
|
||||
| Record<string, Partial<SlackAccountConfig>>
|
||||
| undefined,
|
||||
channelConfig: cfg.channels?.slack as SlackAccountConfig,
|
||||
accounts: cfg.channels?.slack?.accounts as Record<string, Partial<SlackAccountConfig>>,
|
||||
accountId,
|
||||
});
|
||||
}
|
||||
@@ -46,6 +45,28 @@ export function mergeSlackAccountConfig(
|
||||
export function resolveSlackAccount(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
/**
|
||||
* When true, account-level credential reads (`botToken`, `appToken`,
|
||||
* `userToken`) silently become `undefined` for unresolved `SecretRef`
|
||||
* inputs instead of throwing. Default is false to preserve the strict
|
||||
* behavior expected by boot-time provider initialization, which must
|
||||
* surface unresolved channel SecretRefs loudly.
|
||||
*
|
||||
* Pass `true` from call sites that already have a separately-resolved
|
||||
* credential override (for example `sendMessageSlack` receives an explicit
|
||||
* `opts.token` derived from the boot-time monitor context) and only need
|
||||
* the rest of the account info (account id, dm policy, channel settings,
|
||||
* etc.). The downstream consumer's existing `if (!token)` guard still
|
||||
* surfaces a clean "missing token" error when no explicit override is
|
||||
* supplied either.
|
||||
*
|
||||
* Without this opt-in, an inactive `channels.slack.accounts.*.botToken`
|
||||
* SecretRef left in the runtime snapshot (per the inspect/strict
|
||||
* separation introduced in #66818) blows up the strict resolver path even
|
||||
* though the actual send already has a valid boot-resolved token. See
|
||||
* #68237.
|
||||
*/
|
||||
tolerateUnresolvedSecrets?: boolean;
|
||||
}): ResolvedSlackAccount {
|
||||
const accountId = normalizeAccountId(
|
||||
params.accountId ?? resolveDefaultSlackAccountId(params.cfg),
|
||||
@@ -54,22 +75,36 @@ export function resolveSlackAccount(params: {
|
||||
const merged = mergeSlackAccountConfig(params.cfg, accountId);
|
||||
const accountEnabled = merged.enabled !== false;
|
||||
const enabled = baseEnabled && accountEnabled;
|
||||
const allowEnv = accountId === DEFAULT_ACCOUNT_ID;
|
||||
const envBot = allowEnv ? resolveSlackBotToken(process.env.SLACK_BOT_TOKEN) : undefined;
|
||||
const envApp = allowEnv ? resolveSlackAppToken(process.env.SLACK_APP_TOKEN) : undefined;
|
||||
const envUser = allowEnv ? resolveSlackUserToken(process.env.SLACK_USER_TOKEN) : undefined;
|
||||
const configBot = resolveSlackBotToken(
|
||||
merged.botToken,
|
||||
`channels.slack.accounts.${accountId}.botToken`,
|
||||
);
|
||||
const configApp = resolveSlackAppToken(
|
||||
merged.appToken,
|
||||
`channels.slack.accounts.${accountId}.appToken`,
|
||||
);
|
||||
const configUser = resolveSlackUserToken(
|
||||
merged.userToken,
|
||||
`channels.slack.accounts.${accountId}.userToken`,
|
||||
);
|
||||
// Per-credential env-var fallback gating: in tolerant mode, only block
|
||||
// the `SLACK_*_TOKEN` env fallback for credentials whose configured value
|
||||
// is an unresolved `SecretRef` object. Otherwise (config field is a
|
||||
// resolved string, or unset entirely) keep the original env fallback so
|
||||
// legitimate env-only setups (no per-account config token, just
|
||||
// `SLACK_BOT_TOKEN` in the process env) keep working. This avoids
|
||||
// credential confusion (CWE-287) on misconfigured deployments where an
|
||||
// unresolved SecretRef would otherwise be silently overridden by a stray
|
||||
// env var, while preserving the env-only contract that callers like
|
||||
// `extensions/slack/src/channel.ts` rely on when omitting `opts.token`.
|
||||
const baseAllowEnv = accountId === DEFAULT_ACCOUNT_ID;
|
||||
const tolerantMode = params.tolerateUnresolvedSecrets === true;
|
||||
const blockBotEnv = tolerantMode && isSecretRef(merged.botToken);
|
||||
const blockAppEnv = tolerantMode && isSecretRef(merged.appToken);
|
||||
const blockUserEnv = tolerantMode && isSecretRef(merged.userToken);
|
||||
const envBot =
|
||||
baseAllowEnv && !blockBotEnv ? resolveSlackBotToken(process.env.SLACK_BOT_TOKEN) : undefined;
|
||||
const envApp =
|
||||
baseAllowEnv && !blockAppEnv ? resolveSlackAppToken(process.env.SLACK_APP_TOKEN) : undefined;
|
||||
const envUser =
|
||||
baseAllowEnv && !blockUserEnv ? resolveSlackUserToken(process.env.SLACK_USER_TOKEN) : undefined;
|
||||
const configBot = tolerantMode
|
||||
? normalizeSecretInputString(merged.botToken)
|
||||
: resolveSlackBotToken(merged.botToken, `channels.slack.accounts.${accountId}.botToken`);
|
||||
const configApp = tolerantMode
|
||||
? normalizeSecretInputString(merged.appToken)
|
||||
: resolveSlackAppToken(merged.appToken, `channels.slack.accounts.${accountId}.appToken`);
|
||||
const configUser = tolerantMode
|
||||
? normalizeSecretInputString(merged.userToken)
|
||||
: resolveSlackUserToken(merged.userToken, `channels.slack.accounts.${accountId}.userToken`);
|
||||
const botToken = configBot ?? envBot;
|
||||
const appToken = configApp ?? envApp;
|
||||
const userToken = configUser ?? envUser;
|
||||
|
||||
@@ -170,6 +170,10 @@ async function resolveSlackSendContext(params: {
|
||||
const send =
|
||||
resolveOutboundSendDep<SlackSendFn>(params.deps, "slack") ??
|
||||
(await loadSlackSendRuntime()).sendMessageSlack;
|
||||
// params.cfg is the scoped channel-dispatch config; channel credentials are
|
||||
// expected to be resolved here (not a raw loadConfig() snapshot). Strict mode
|
||||
// is intentional so boot-time misconfigurations surface loudly. See #68237
|
||||
// for the companion tolerant-mode path in sendMessageSlack itself.
|
||||
const account = resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId });
|
||||
const token = getTokenForOperation(account, "write");
|
||||
const botToken = account.botToken?.trim();
|
||||
|
||||
@@ -322,9 +322,20 @@ export async function sendMessageSlack(
|
||||
throw new Error("Slack send requires text, blocks, or media");
|
||||
}
|
||||
const cfg = opts.cfg ?? loadConfig();
|
||||
// Tolerate unresolved channel SecretRefs in the cfg snapshot here: the
|
||||
// send path either receives an explicit `opts.token` (resolved at Slack
|
||||
// monitor boot time and threaded through `ctx.botToken`) or surfaces the
|
||||
// existing "Slack bot token missing" error via `resolveToken` below. The
|
||||
// runtime snapshot can legitimately retain unresolved `channels.slack.*`
|
||||
// SecretRefs (see the inspect/strict separation introduced in #66818) when
|
||||
// the active account's secrets were not part of the agent-runtime base
|
||||
// target set; failing the strict resolver here would block outbound
|
||||
// replies even though `reactions.add` and inbound dispatch (which use the
|
||||
// boot-resolved client/token directly) keep working. See #68237.
|
||||
const account = resolveSlackAccount({
|
||||
cfg,
|
||||
accountId: opts.accountId,
|
||||
tolerateUnresolvedSecrets: true,
|
||||
});
|
||||
const token = resolveToken({
|
||||
explicit: opts.token,
|
||||
|
||||
Reference in New Issue
Block a user