From c850d1bb0ddf7197246859f7ad879207ef8ae9c0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 3 May 2026 12:07:36 +0100 Subject: [PATCH] fix(qqbot): harden clientSecret SecretRefs --- CHANGELOG.md | 1 + docs/channels/qqbot.md | 2 + .../reference/secretref-credential-surface.md | 2 - ...tref-user-supplied-credentials-matrix.json | 14 --- extensions/qqbot/src/bridge/config.ts | 12 ++ extensions/qqbot/src/config.test.ts | 15 +++ extensions/qqbot/src/secret-contract.test.ts | 110 ++++++++++++++++++ extensions/qqbot/src/secret-contract.ts | 36 +++--- 8 files changed, 158 insertions(+), 34 deletions(-) create mode 100644 extensions/qqbot/src/secret-contract.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a0f2831f22e..3e142f32bf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Plugins/tools: keep disabled bundled tool plugins out of explicit runtime allowlist ownership and fall back from loaded-but-empty channel registries to tool-bearing plugin registries, so Active Memory can use bundled `memory-core` search/get tools even when `memory-lancedb` is disabled. Fixes #76603. Thanks @jwong-art. +- Channels/QQ Bot: resolve structured `clientSecret` SecretRefs before QQ token exchange, expose the QQ Bot secret contract to secrets tooling, and reject legacy `secretref:/...` marker strings. (#74772) Thanks @xialonglee. - Plugins/externalization: keep official ACPX, Google Chat, and LINE install specs on production package names, leaving beta-tag probing to the explicit OpenClaw beta update channel. Thanks @vincentkoc. - CLI/doctor: keep missing-plugin repair from overriding official catalog metadata with runtime fallbacks, so ACPX repairs preserve the official npm spec during the externalization rollout. Thanks @vincentkoc. - Plugins/catalog: preserve ClawHub install specs when generating the packaged channel catalog so future storepack-first channel plugins keep their remote source instead of becoming npm-only. Thanks @vincentkoc. diff --git a/docs/channels/qqbot.md b/docs/channels/qqbot.md index eb5e1c70680..9e15e6c59b6 100644 --- a/docs/channels/qqbot.md +++ b/docs/channels/qqbot.md @@ -102,6 +102,8 @@ Notes: - `openclaw channels add --channel qqbot --token-file ...` provides the AppSecret only; the AppID must already be set in config or `QQBOT_APP_ID`. - `clientSecret` also accepts SecretRef input, not just a plaintext string. +- Legacy `secretref:/...` marker strings are not valid `clientSecret` values; + use structured SecretRef objects like the example above. ### Multi-account setup diff --git a/docs/reference/secretref-credential-surface.md b/docs/reference/secretref-credential-surface.md index 045ec2128f6..5e75dbeb51e 100644 --- a/docs/reference/secretref-credential-surface.md +++ b/docs/reference/secretref-credential-surface.md @@ -90,8 +90,6 @@ Scope intent: - `channels.feishu.accounts.*.appSecret` - `channels.feishu.accounts.*.encryptKey` - `channels.feishu.accounts.*.verificationToken` -- `channels.qqbot.clientSecret` -- `channels.qqbot.accounts.*.clientSecret` - `channels.msteams.appPassword` - `channels.mattermost.botToken` - `channels.mattermost.accounts.*.botToken` diff --git a/docs/reference/secretref-user-supplied-credentials-matrix.json b/docs/reference/secretref-user-supplied-credentials-matrix.json index da473149004..995094fcb93 100644 --- a/docs/reference/secretref-user-supplied-credentials-matrix.json +++ b/docs/reference/secretref-user-supplied-credentials-matrix.json @@ -281,20 +281,6 @@ "secretShape": "secret_input", "optIn": true }, - { - "id": "channels.qqbot.accounts.*.clientSecret", - "configFile": "openclaw.json", - "path": "channels.qqbot.accounts.*.clientSecret", - "secretShape": "secret_input", - "optIn": true - }, - { - "id": "channels.qqbot.clientSecret", - "configFile": "openclaw.json", - "path": "channels.qqbot.clientSecret", - "secretShape": "secret_input", - "optIn": true - }, { "id": "channels.slack.accounts.*.appToken", "configFile": "openclaw.json", diff --git a/extensions/qqbot/src/bridge/config.ts b/extensions/qqbot/src/bridge/config.ts index 497832b0f85..c751734cf52 100644 --- a/extensions/qqbot/src/bridge/config.ts +++ b/extensions/qqbot/src/bridge/config.ts @@ -19,6 +19,16 @@ interface QQBotChannelConfig extends QQBotAccountConfig { defaultAccount?: string; } +function assertNotLegacySecretRefMarker(value: unknown, path: string): void { + const normalized = normalizeSecretInputString(value); + if (!normalized || !/^secretref(?:-env)?:/i.test(normalized)) { + return; + } + throw new Error( + `${path}: legacy SecretRef marker strings are not valid QQ Bot clientSecret values; use a structured SecretRef object instead.`, + ); +} + function resolveEnvSecretRefValue(params: { cfg: OpenClawConfig; value: unknown; @@ -55,6 +65,8 @@ function resolveQQBotClientSecretInput(params: { value: unknown; path: string; }): string | undefined { + assertNotLegacySecretRefMarker(params.value, params.path); + const envSecret = resolveEnvSecretRefValue({ cfg: params.cfg, value: params.value, diff --git a/extensions/qqbot/src/config.test.ts b/extensions/qqbot/src/config.test.ts index de818feeb33..9c5eedd595a 100644 --- a/extensions/qqbot/src/config.test.ts +++ b/extensions/qqbot/src/config.test.ts @@ -214,6 +214,21 @@ describe("qqbot config", () => { ); }); + it("rejects legacy SecretRef marker strings before QQ token exchange", () => { + const cfg = { + channels: { + qqbot: { + appId: "123456", + clientSecret: "secretref:/QQBOT_CLIENT_SECRET", + }, + }, + } as OpenClawConfig; + + expect(() => resolveQQBotAccount(cfg, DEFAULT_ACCOUNT_ID)).toThrow( + "channels.qqbot.clientSecret: legacy SecretRef marker strings are not valid QQ Bot clientSecret values; use a structured SecretRef object instead.", + ); + }); + it("allows unresolved SecretRefs for setup/status flows", () => { const cfg = makeQqbotSecretRefConfig(); diff --git a/extensions/qqbot/src/secret-contract.test.ts b/extensions/qqbot/src/secret-contract.test.ts new file mode 100644 index 00000000000..0b12103555d --- /dev/null +++ b/extensions/qqbot/src/secret-contract.test.ts @@ -0,0 +1,110 @@ +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-types"; +import { + applyResolvedAssignments, + createResolverContext, + resolveSecretRefValues, +} from "openclaw/plugin-sdk/runtime-secret-resolution"; +import { describe, expect, it } from "vitest"; +import { collectRuntimeConfigAssignments } from "./secret-contract.js"; + +async function resolveQqbotSecretAssignments( + sourceConfig: OpenClawConfig, + env: NodeJS.ProcessEnv, +): Promise { + const resolvedConfig: OpenClawConfig = structuredClone(sourceConfig); + const context = createResolverContext({ sourceConfig, env }); + + collectRuntimeConfigAssignments({ + config: resolvedConfig, + defaults: sourceConfig.secrets?.defaults, + context, + }); + + const resolved = await resolveSecretRefValues( + context.assignments.map((assignment) => assignment.ref), + { + config: sourceConfig, + env: context.env, + cache: context.cache, + }, + ); + applyResolvedAssignments({ assignments: context.assignments, resolved }); + + expect(context.warnings).toEqual([]); + return resolvedConfig; +} + +describe("qqbot secret contract", () => { + it("resolves top-level clientSecret SecretRefs even when clientSecretFile is configured", async () => { + const resolvedConfig = await resolveQqbotSecretAssignments( + { + channels: { + qqbot: { + enabled: true, + appId: "123456", + clientSecret: { source: "env", provider: "default", id: "QQBOT_CLIENT_SECRET" }, + clientSecretFile: "/ignored/by/runtime", + }, + }, + } as OpenClawConfig, + { QQBOT_CLIENT_SECRET: "resolved-top-level-secret" }, + ); + + expect(resolvedConfig.channels?.qqbot?.clientSecret).toBe("resolved-top-level-secret"); + }); + + it("resolves account clientSecret SecretRefs even when account clientSecretFile is configured", async () => { + const resolvedConfig = await resolveQqbotSecretAssignments( + { + channels: { + qqbot: { + enabled: true, + accounts: { + bot2: { + enabled: true, + appId: "654321", + clientSecret: { source: "env", provider: "default", id: "QQBOT_BOT2_SECRET" }, + clientSecretFile: "/ignored/by/runtime", + }, + }, + }, + }, + } as OpenClawConfig, + { QQBOT_BOT2_SECRET: "resolved-bot2-secret" }, + ); + + expect(resolvedConfig.channels?.qqbot?.accounts?.bot2?.clientSecret).toBe( + "resolved-bot2-secret", + ); + }); + + it("keeps the implicit default account top-level clientSecret active with named accounts", async () => { + const resolvedConfig = await resolveQqbotSecretAssignments( + { + channels: { + qqbot: { + enabled: true, + appId: "123456", + clientSecret: { source: "env", provider: "default", id: "QQBOT_DEFAULT_SECRET" }, + accounts: { + bot2: { + enabled: true, + appId: "654321", + clientSecret: { source: "env", provider: "default", id: "QQBOT_BOT2_SECRET" }, + }, + }, + }, + }, + } as OpenClawConfig, + { + QQBOT_DEFAULT_SECRET: "resolved-default-secret", + QQBOT_BOT2_SECRET: "resolved-bot2-secret", + }, + ); + + expect(resolvedConfig.channels?.qqbot?.clientSecret).toBe("resolved-default-secret"); + expect(resolvedConfig.channels?.qqbot?.accounts?.bot2?.clientSecret).toBe( + "resolved-bot2-secret", + ); + }); +}); diff --git a/extensions/qqbot/src/secret-contract.ts b/extensions/qqbot/src/secret-contract.ts index 0aba4c60812..7d3ae6006c7 100644 --- a/extensions/qqbot/src/secret-contract.ts +++ b/extensions/qqbot/src/secret-contract.ts @@ -2,12 +2,13 @@ import { collectConditionalChannelFieldAssignments, getChannelSurface, hasConfiguredSecretInputValue, - normalizeSecretStringValue, type ResolverContext, type SecretDefaults, type SecretTargetRegistryEntry, } from "openclaw/plugin-sdk/channel-secret-basic-runtime"; +const DEFAULT_ACCOUNT_ID = "default"; + export const secretTargetRegistryEntries = [ { id: "channels.qqbot.accounts.*.clientSecret", @@ -33,8 +34,11 @@ export const secretTargetRegistryEntries = [ }, ] satisfies SecretTargetRegistryEntry[]; -function hasClientSecretFile(value: unknown): boolean { - return normalizeSecretStringValue(value).length > 0; +function hasTopLevelAppId(qqbot: Record): boolean { + if (typeof qqbot.appId === "string") { + return qqbot.appId.trim().length > 0; + } + return typeof qqbot.appId === "number"; } export function collectRuntimeConfigAssignments(params: { @@ -48,9 +52,9 @@ export function collectRuntimeConfigAssignments(params: { } const { channel: qqbot, surface } = resolved; - const baseClientSecretFile = hasClientSecretFile(qqbot.clientSecretFile); - const accountClientSecretFile = (account: Record) => - hasClientSecretFile(account.clientSecretFile); + const hasExplicitDefaultAccount = surface.accounts.some( + ({ accountId }) => accountId === DEFAULT_ACCOUNT_ID, + ); collectConditionalChannelFieldAssignments({ channelKey: "qqbot", @@ -59,20 +63,16 @@ export function collectRuntimeConfigAssignments(params: { surface, defaults: params.defaults, context: params.context, - topLevelActiveWithoutAccounts: !baseClientSecretFile, - topLevelInheritedAccountActive: ({ account, enabled }) => { - if (!enabled || baseClientSecretFile) { - return false; + topLevelActiveWithoutAccounts: true, + topLevelInheritedAccountActive: ({ accountId, account, enabled }) => { + if (accountId === DEFAULT_ACCOUNT_ID) { + return enabled && !hasConfiguredSecretInputValue(account.clientSecret, params.defaults); } - return ( - !hasConfiguredSecretInputValue(account.clientSecret, params.defaults) && - !accountClientSecretFile(account) - ); + return !hasExplicitDefaultAccount && hasTopLevelAppId(qqbot); }, - accountActive: ({ account, enabled }) => enabled && !accountClientSecretFile(account), - topInactiveReason: - "no enabled QQBot surface inherits this top-level clientSecret (clientSecretFile is configured).", - accountInactiveReason: "QQBot account is disabled or clientSecretFile is configured.", + accountActive: ({ enabled }) => enabled, + topInactiveReason: "no enabled QQ Bot default surface uses this top-level clientSecret.", + accountInactiveReason: "QQ Bot account is disabled.", }); }