Skip to content

Commit b7589b3

Browse files
LiaoyuanNingjoshavant
LiaoyuanNing
and
joshavant
authored
fix(feishu): support SecretRef-style env credentials in account resolver (#30903)
Merged via squash. Prepared head SHA: d3d0a18 Co-authored-by: LiaoyuanNing <[email protected]> Co-authored-by: joshavant <[email protected]> Reviewed-by: @joshavant
1 parent 21e8d88 commit b7589b3

File tree

5 files changed

+422
-25
lines changed

5 files changed

+422
-25
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,7 @@ Docs: https://docs.openclaw.ai
530530
- Slack/Legacy streaming config: map boolean `channels.slack.streaming=false` to unified streaming mode `off` (with `nativeStreaming=false`) so legacy configs correctly disable draft preview/native streaming instead of defaulting to `partial`. (#25990) Thanks @chilu18.
531531
- Slack/Socket reconnect reliability: reconnect Socket Mode after disconnect/start failures using bounded exponential backoff with abort-aware waits, while preserving clean shutdown behavior and adding disconnect/error helper tests. (#27232) Thanks @pandego.
532532
- Memory/QMD update+embed output cap: discard captured stdout for `qmd update` and `qmd embed` runs (while keeping stderr diagnostics) so large index progress output no longer fails sync with `produced too much output` during boot/refresh. (#28900; landed from contributor PR #23311 by @haitao-sjsu) Thanks @haitao-sjsu.
533+
- Feishu/Onboarding SecretRef guards: avoid direct `.trim()` calls on object-form `appId`/`appSecret` in onboarding credential checks, keep status semantics strict when an account explicitly sets empty `appId` (no fallback to top-level `appId`), recognize env SecretRef `appId`/`appSecret` as configured so readiness is accurate, and preserve unresolved SecretRef errors in default account resolution for actionable diagnostics. (#30903) Thanks @LiaoyuanNing.
533534
- Onboarding/Custom providers: raise default custom-provider model context window to the runtime hard minimum (16k) and auto-heal existing custom model entries below that threshold during reconfiguration, preventing immediate `Model context window too small (4096 tokens)` failures. (#21653) Thanks @r4jiv007.
534535
- Web UI/Assistant text: strip internal `<relevant-memories>...</relevant-memories>` scaffolding from rendered assistant messages (while preserving code-fence literals), preventing memory-context leakage in chat output for models that echo internal blocks. (#29851) Thanks @Valkster70.
535536
- Dashboard/Sessions: allow authenticated Control UI clients to delete and patch sessions while still blocking regular webchat clients from session mutation RPCs, fixing Dashboard session delete failures. (#21264) Thanks @jskoiz.

‎extensions/feishu/src/accounts.test.ts‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import {
33
resolveDefaultFeishuAccountId,
44
resolveDefaultFeishuAccountSelection,
55
resolveFeishuAccount,
6+
resolveFeishuCredentials,
67
} from "./accounts.js";
8+
import type { FeishuConfig } from "./types.js";
9+
10+
const asConfig = (value: Partial<FeishuConfig>) => value as FeishuConfig;
711

812
describe("resolveDefaultFeishuAccountId", () => {
913
it("prefers channels.feishu.defaultAccount when configured", () => {
@@ -98,6 +102,148 @@ describe("resolveDefaultFeishuAccountId", () => {
98102
});
99103
});
100104

105+
describe("resolveFeishuCredentials", () => {
106+
it("throws unresolved SecretRef errors by default for unsupported secret sources", () => {
107+
expect(() =>
108+
resolveFeishuCredentials(
109+
asConfig({
110+
appId: "cli_123",
111+
appSecret: { source: "file", provider: "default", id: "path/to/secret" } as never,
112+
}),
113+
),
114+
).toThrow(/unresolved SecretRef/i);
115+
});
116+
117+
it("returns null (without throwing) when unresolved SecretRef is allowed", () => {
118+
const creds = resolveFeishuCredentials(
119+
asConfig({
120+
appId: "cli_123",
121+
appSecret: { source: "file", provider: "default", id: "path/to/secret" } as never,
122+
}),
123+
{ allowUnresolvedSecretRef: true },
124+
);
125+
126+
expect(creds).toBeNull();
127+
});
128+
129+
it("throws unresolved SecretRef error when env SecretRef points to missing env var", () => {
130+
const key = "FEISHU_APP_SECRET_MISSING_TEST";
131+
const prev = process.env[key];
132+
delete process.env[key];
133+
try {
134+
expect(() =>
135+
resolveFeishuCredentials(
136+
asConfig({
137+
appId: "cli_123",
138+
appSecret: { source: "env", provider: "default", id: key } as never,
139+
}),
140+
),
141+
).toThrow(/unresolved SecretRef/i);
142+
} finally {
143+
if (prev === undefined) {
144+
delete process.env[key];
145+
} else {
146+
process.env[key] = prev;
147+
}
148+
}
149+
});
150+
151+
it("resolves env SecretRef objects when unresolved refs are allowed", () => {
152+
const key = "FEISHU_APP_SECRET_TEST";
153+
const prev = process.env[key];
154+
process.env[key] = " secret_from_env ";
155+
156+
try {
157+
const creds = resolveFeishuCredentials(
158+
asConfig({
159+
appId: "cli_123",
160+
appSecret: { source: "env", provider: "default", id: key } as never,
161+
}),
162+
{ allowUnresolvedSecretRef: true },
163+
);
164+
165+
expect(creds).toEqual({
166+
appId: "cli_123",
167+
appSecret: "secret_from_env",
168+
encryptKey: undefined,
169+
verificationToken: undefined,
170+
domain: "feishu",
171+
});
172+
} finally {
173+
if (prev === undefined) {
174+
delete process.env[key];
175+
} else {
176+
process.env[key] = prev;
177+
}
178+
}
179+
});
180+
181+
it("resolves env SecretRef with custom provider alias when unresolved refs are allowed", () => {
182+
const key = "FEISHU_APP_SECRET_CUSTOM_PROVIDER_TEST";
183+
const prev = process.env[key];
184+
process.env[key] = " secret_from_env_alias ";
185+
186+
try {
187+
const creds = resolveFeishuCredentials(
188+
asConfig({
189+
appId: "cli_123",
190+
appSecret: { source: "env", provider: "corp-env", id: key } as never,
191+
}),
192+
{ allowUnresolvedSecretRef: true },
193+
);
194+
195+
expect(creds?.appSecret).toBe("secret_from_env_alias");
196+
} finally {
197+
if (prev === undefined) {
198+
delete process.env[key];
199+
} else {
200+
process.env[key] = prev;
201+
}
202+
}
203+
});
204+
205+
it("preserves unresolved SecretRef diagnostics for env refs in default mode", () => {
206+
const key = "FEISHU_APP_SECRET_POLICY_TEST";
207+
const prev = process.env[key];
208+
process.env[key] = "secret_from_env";
209+
try {
210+
expect(() =>
211+
resolveFeishuCredentials(
212+
asConfig({
213+
appId: "cli_123",
214+
appSecret: { source: "env", provider: "default", id: key } as never,
215+
}),
216+
),
217+
).toThrow(/unresolved SecretRef/i);
218+
} finally {
219+
if (prev === undefined) {
220+
delete process.env[key];
221+
} else {
222+
process.env[key] = prev;
223+
}
224+
}
225+
});
226+
227+
it("trims and returns credentials when values are valid strings", () => {
228+
const creds = resolveFeishuCredentials(
229+
asConfig({
230+
appId: " cli_123 ",
231+
appSecret: " secret_456 ",
232+
encryptKey: " enc ",
233+
verificationToken: " vt ",
234+
}),
235+
);
236+
237+
expect(creds).toEqual({
238+
appId: "cli_123",
239+
appSecret: "secret_456",
240+
encryptKey: "enc",
241+
verificationToken: "vt",
242+
domain: "feishu",
243+
});
244+
});
245+
});
246+
101247
describe("resolveFeishuAccount", () => {
102248
it("uses top-level credentials with configured default account id even without account map entry", () => {
103249
const cfg = {
@@ -158,4 +304,45 @@ describe("resolveFeishuAccount", () => {
158304
expect(account.selectionSource).toBe("explicit");
159305
expect(account.appId).toBe("cli_default");
160306
});
307+
308+
it("surfaces unresolved SecretRef errors in account resolution", () => {
309+
expect(() =>
310+
resolveFeishuAccount({
311+
cfg: {
312+
channels: {
313+
feishu: {
314+
accounts: {
315+
main: {
316+
appId: "cli_123",
317+
appSecret: { source: "file", provider: "default", id: "path/to/secret" },
318+
} as never,
319+
},
320+
},
321+
},
322+
} as never,
323+
accountId: "main",
324+
}),
325+
).toThrow(/unresolved SecretRef/i);
326+
});
327+
328+
it("does not throw when account name is non-string", () => {
329+
expect(() =>
330+
resolveFeishuAccount({
331+
cfg: {
332+
channels: {
333+
feishu: {
334+
accounts: {
335+
main: {
336+
name: { bad: true },
337+
appId: "cli_123",
338+
appSecret: "secret_456",
339+
} as never,
340+
},
341+
},
342+
},
343+
} as never,
344+
accountId: "main",
345+
}),
346+
).not.toThrow();
347+
});
161348
});

‎extensions/feishu/src/accounts.ts‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -129,27 +129,54 @@ export function resolveFeishuCredentials(
129129
verificationToken?: string;
130130
domain: FeishuDomain;
131131
} | null {
132-
const appId = cfg?.appId?.trim();
133-
const appSecret = options?.allowUnresolvedSecretRef
134-
? normalizeSecretInputString(cfg?.appSecret)
135-
: normalizeResolvedSecretInputString({
136-
value: cfg?.appSecret,
137-
path: "channels.feishu.appSecret",
138-
});
132+
const normalizeString = (value: unknown): string | undefined => {
133+
if (typeof value !== "string") {
134+
return undefined;
135+
}
136+
const trimmed = value.trim();
137+
return trimmed ? trimmed : undefined;
138+
};
139+
140+
const resolveSecretLike = (value: unknown, path: string): string | undefined => {
141+
const asString = normalizeString(value);
142+
if (asString) {
143+
return asString;
144+
}
145+
146+
// In relaxed/onboarding paths only: allow direct env SecretRef reads for UX.
147+
// Default resolution path must preserve unresolved-ref diagnostics/policy semantics.
148+
if (options?.allowUnresolvedSecretRef && typeof value === "object" && value !== null) {
149+
const rec = value as Record<string, unknown>;
150+
const source = normalizeString(rec.source)?.toLowerCase();
151+
const id = normalizeString(rec.id);
152+
if (source === "env" && id) {
153+
const envValue = normalizeString(process.env[id]);
154+
if (envValue) {
155+
return envValue;
156+
}
157+
}
158+
}
159+
160+
if (options?.allowUnresolvedSecretRef) {
161+
return normalizeSecretInputString(value);
162+
}
163+
return normalizeResolvedSecretInputString({ value, path });
164+
};
165+
166+
const appId = resolveSecretLike(cfg?.appId, "channels.feishu.appId");
167+
const appSecret = resolveSecretLike(cfg?.appSecret, "channels.feishu.appSecret");
168+
139169
if (!appId || !appSecret) {
140170
return null;
141171
}
142172
return {
143173
appId,
144174
appSecret,
145-
encryptKey: cfg?.encryptKey?.trim() || undefined,
146-
verificationToken:
147-
(options?.allowUnresolvedSecretRef
148-
? normalizeSecretInputString(cfg?.verificationToken)
149-
: normalizeResolvedSecretInputString({
150-
value: cfg?.verificationToken,
151-
path: "channels.feishu.verificationToken",
152-
})) || undefined,
175+
encryptKey: normalizeString(cfg?.encryptKey),
176+
verificationToken: resolveSecretLike(
177+
cfg?.verificationToken,
178+
"channels.feishu.verificationToken",
179+
),
153180
domain: cfg?.domain ?? "feishu",
154181
};
155182
}
@@ -186,13 +213,14 @@ export function resolveFeishuAccount(params: {
186213

187214
// Resolve credentials from merged config
188215
const creds = resolveFeishuCredentials(merged);
216+
const accountName = (merged as FeishuAccountConfig).name;
189217

190218
return {
191219
accountId,
192220
selectionSource,
193221
enabled,
194222
configured: Boolean(creds),
195-
name: (merged as FeishuAccountConfig).name?.trim() || undefined,
223+
name: typeof accountName === "string" ? accountName.trim() || undefined : undefined,
196224
appId: creds?.appId,
197225
appSecret: creds?.appSecret,
198226
encryptKey: creds?.encryptKey,

0 commit comments

Comments
 (0)