diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index 96b0c8054..ef7d41c06 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "../config/config.js"; import type { ModelDefinitionConfig } from "../config/types.models.js"; +import { coerceSecretRef } from "../config/types.secrets.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { DEFAULT_COPILOT_API_BASE_URL, @@ -357,10 +358,24 @@ function resolveApiKeyFromProfiles(params: { continue; } if (cred.type === "api_key") { - return cred.key; + if (cred.key?.trim()) { + return cred.key; + } + const keyRef = coerceSecretRef(cred.keyRef); + if (keyRef?.source === "env" && keyRef.id.trim()) { + return keyRef.id.trim(); + } + continue; } if (cred.type === "token") { - return cred.token; + if (cred.token?.trim()) { + return cred.token; + } + const tokenRef = coerceSecretRef(cred.tokenRef); + if (tokenRef?.source === "env" && tokenRef.id.trim()) { + return tokenRef.id.trim(); + } + continue; } } return undefined; @@ -1017,7 +1032,13 @@ export async function resolveImplicitCopilotProvider(params: { const profileId = listProfilesForProvider(authStore, "github-copilot")[0]; const profile = profileId ? authStore.profiles[profileId] : undefined; if (profile && profile.type === "token") { - selectedGithubToken = profile.token; + selectedGithubToken = profile.token?.trim() ?? ""; + if (!selectedGithubToken) { + const tokenRef = coerceSecretRef(profile.tokenRef); + if (tokenRef?.source === "env" && tokenRef.id.trim()) { + selectedGithubToken = (env[tokenRef.id] ?? process.env[tokenRef.id] ?? "").trim(); + } + } } } diff --git a/src/agents/models-config.providers.volcengine-byteplus.test.ts b/src/agents/models-config.providers.volcengine-byteplus.test.ts index 9ce3ad892..00dd65e38 100644 --- a/src/agents/models-config.providers.volcengine-byteplus.test.ts +++ b/src/agents/models-config.providers.volcengine-byteplus.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { captureEnv } from "../test-utils/env.js"; +import { upsertAuthProfile } from "./auth-profiles.js"; import { resolveImplicitProviders } from "./models-config.providers.js"; describe("Volcengine and BytePlus providers", () => { @@ -37,4 +38,40 @@ describe("Volcengine and BytePlus providers", () => { envSnapshot.restore(); } }); + + it("includes providers when auth profiles are env keyRef-only", async () => { + const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); + const envSnapshot = captureEnv(["VOLCANO_ENGINE_API_KEY", "BYTEPLUS_API_KEY"]); + delete process.env.VOLCANO_ENGINE_API_KEY; + delete process.env.BYTEPLUS_API_KEY; + + upsertAuthProfile({ + profileId: "volcengine:default", + credential: { + type: "api_key", + provider: "volcengine", + keyRef: { source: "env", provider: "default", id: "VOLCANO_ENGINE_API_KEY" }, + }, + agentDir, + }); + upsertAuthProfile({ + profileId: "byteplus:default", + credential: { + type: "api_key", + provider: "byteplus", + keyRef: { source: "env", provider: "default", id: "BYTEPLUS_API_KEY" }, + }, + agentDir, + }); + + try { + const providers = await resolveImplicitProviders({ agentDir }); + expect(providers?.volcengine?.apiKey).toBe("VOLCANO_ENGINE_API_KEY"); + expect(providers?.["volcengine-plan"]?.apiKey).toBe("VOLCANO_ENGINE_API_KEY"); + expect(providers?.byteplus?.apiKey).toBe("BYTEPLUS_API_KEY"); + expect(providers?.["byteplus-plan"]?.apiKey).toBe("BYTEPLUS_API_KEY"); + } finally { + envSnapshot.restore(); + } + }); }); diff --git a/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts b/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts index 50b80f2eb..2ea2c25da 100644 --- a/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts +++ b/src/agents/models-config.uses-first-github-copilot-profile-env-tokens.test.ts @@ -76,4 +76,38 @@ describe("models-config", () => { }); }); }); + + it("uses tokenRef env var when github-copilot profile omits plaintext token", async () => { + await withTempHome(async (home) => { + await withUnsetCopilotTokenEnv(async () => { + const fetchMock = mockCopilotTokenExchangeSuccess(); + const agentDir = path.join(home, "agent-profiles"); + await fs.mkdir(agentDir, { recursive: true }); + process.env.COPILOT_REF_TOKEN = "token-from-ref-env"; + await fs.writeFile( + path.join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "github-copilot:default": { + type: "token", + provider: "github-copilot", + tokenRef: { source: "env", provider: "default", id: "COPILOT_REF_TOKEN" }, + }, + }, + }, + null, + 2, + ), + ); + + await ensureOpenClawModelsJson({ models: { providers: {} } }, agentDir); + + const [, opts] = fetchMock.mock.calls[0] as [string, { headers?: Record }]; + expect(opts?.headers?.Authorization).toBe("Bearer token-from-ref-env"); + delete process.env.COPILOT_REF_TOKEN; + }); + }); + }); }); diff --git a/src/secrets/apply.test.ts b/src/secrets/apply.test.ts index 5f6fcc2b0..2f398ea0e 100644 --- a/src/secrets/apply.test.ts +++ b/src/secrets/apply.test.ts @@ -320,6 +320,49 @@ describe("secrets apply", () => { expect(rawConfig).not.toContain("sk-skill-plaintext"); }); + it("rejects plan targets that do not match allowed secret-bearing paths", async () => { + const plan: SecretsApplyPlan = { + version: 1, + protocolVersion: 1, + generatedAt: new Date().toISOString(), + generatedBy: "manual", + targets: [ + { + type: "models.providers.apiKey", + path: "models.providers.openai.baseUrl", + pathSegments: ["models", "providers", "openai", "baseUrl"], + providerId: "openai", + ref: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + }, + ], + }; + + await expect(runSecretsApply({ plan, env, write: false })).rejects.toThrow( + "Invalid plan target path", + ); + }); + + it("rejects plan targets with forbidden prototype-like path segments", async () => { + const plan: SecretsApplyPlan = { + version: 1, + protocolVersion: 1, + generatedAt: new Date().toISOString(), + generatedBy: "manual", + targets: [ + { + type: "skills.entries.apiKey", + path: "skills.entries.__proto__.apiKey", + pathSegments: ["skills", "entries", "__proto__", "apiKey"], + ref: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + }, + ], + }; + + await expect(runSecretsApply({ plan, env, write: false })).rejects.toThrow( + "Invalid plan target path", + ); + }); + it("applies provider upserts and deletes from plan", async () => { await fs.writeFile( configPath, diff --git a/src/secrets/apply.ts b/src/secrets/apply.ts index 8c4e2c69c..60a58bb1e 100644 --- a/src/secrets/apply.ts +++ b/src/secrets/apply.ts @@ -15,6 +15,7 @@ import { type SecretsApplyPlan, type SecretsPlanTarget, normalizeSecretsPlanOptions, + resolveValidatedTargetPathSegments, } from "./plan.js"; import { listKnownSecretEnvVarNames } from "./provider-env-vars.js"; import { resolveSecretRefValue } from "./resolve.js"; @@ -52,10 +53,6 @@ export type SecretsApplyResult = { warnings: string[]; }; -function parseDotPath(pathname: string): string[] { - return pathname.split(".").filter(Boolean); -} - function getByPathSegments(root: unknown, segments: string[]): unknown { if (segments.length === 0) { return undefined; @@ -114,15 +111,11 @@ function deleteByPathSegments(root: OpenClawConfig, segments: string[]): boolean } function resolveTargetPathSegments(target: SecretsPlanTarget): string[] { - const explicit = target.pathSegments; - if ( - Array.isArray(explicit) && - explicit.length > 0 && - explicit.every((segment) => typeof segment === "string" && segment.trim().length > 0) - ) { - return [...explicit]; + const resolved = resolveValidatedTargetPathSegments(target); + if (!resolved) { + throw new Error(`Invalid plan target path for ${target.type}: ${target.path}`); } - return parseDotPath(target.path); + return resolved; } function parseEnvValue(raw: string): string { diff --git a/src/secrets/plan.ts b/src/secrets/plan.ts index 9a39f0fa2..0956f9677 100644 --- a/src/secrets/plan.ts +++ b/src/secrets/plan.ts @@ -45,6 +45,15 @@ export type SecretsApplyPlan = { }; const PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; +const FORBIDDEN_PATH_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); + +function isSecretsPlanTargetType(value: unknown): value is SecretsPlanTargetType { + return ( + value === "models.providers.apiKey" || + value === "skills.entries.apiKey" || + value === "channels.googlechat.serviceAccount" + ); +} function isObjectRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); @@ -54,6 +63,104 @@ function isSecretProviderConfigShape(value: unknown): value is SecretProviderCon return SecretProviderSchema.safeParse(value).success; } +function parseDotPath(pathname: string): string[] { + return pathname + .split(".") + .map((segment) => segment.trim()) + .filter((segment) => segment.length > 0); +} + +function hasForbiddenPathSegment(segments: string[]): boolean { + return segments.some((segment) => FORBIDDEN_PATH_SEGMENTS.has(segment)); +} + +function hasMatchingPathShape( + candidate: Pick, + segments: string[], +): boolean { + if (candidate.type === "models.providers.apiKey") { + if ( + segments.length !== 4 || + segments[0] !== "models" || + segments[1] !== "providers" || + segments[3] !== "apiKey" + ) { + return false; + } + return ( + candidate.providerId === undefined || + candidate.providerId.trim().length === 0 || + candidate.providerId === segments[2] + ); + } + if (candidate.type === "skills.entries.apiKey") { + return ( + segments.length === 4 && + segments[0] === "skills" && + segments[1] === "entries" && + segments[3] === "apiKey" + ); + } + if ( + segments.length === 3 && + segments[0] === "channels" && + segments[1] === "googlechat" && + segments[2] === "serviceAccount" + ) { + return candidate.accountId === undefined || candidate.accountId.trim().length === 0; + } + if ( + segments.length === 5 && + segments[0] === "channels" && + segments[1] === "googlechat" && + segments[2] === "accounts" && + segments[4] === "serviceAccount" + ) { + return ( + candidate.accountId === undefined || + candidate.accountId.trim().length === 0 || + candidate.accountId === segments[3] + ); + } + return false; +} + +export function resolveValidatedTargetPathSegments(candidate: { + type?: SecretsPlanTargetType; + path?: string; + pathSegments?: string[]; + providerId?: string; + accountId?: string; +}): string[] | null { + if (!isSecretsPlanTargetType(candidate.type)) { + return null; + } + const path = typeof candidate.path === "string" ? candidate.path.trim() : ""; + if (!path) { + return null; + } + const segments = + Array.isArray(candidate.pathSegments) && candidate.pathSegments.length > 0 + ? candidate.pathSegments.map((segment) => String(segment).trim()).filter(Boolean) + : parseDotPath(path); + if ( + segments.length === 0 || + hasForbiddenPathSegment(segments) || + path !== segments.join(".") || + !hasMatchingPathShape( + { + type: candidate.type, + providerId: candidate.providerId, + accountId: candidate.accountId, + }, + segments, + ) + ) { + return null; + } + return segments; +} + export function isSecretsApplyPlan(value: unknown): value is SecretsApplyPlan { if (!value || typeof value !== "object" || Array.isArray(value)) { return false; @@ -74,12 +181,14 @@ export function isSecretsApplyPlan(value: unknown): value is SecretsApplyPlan { candidate.type !== "channels.googlechat.serviceAccount") || typeof candidate.path !== "string" || !candidate.path.trim() || - (candidate.pathSegments !== undefined && - (!Array.isArray(candidate.pathSegments) || - candidate.pathSegments.length === 0 || - candidate.pathSegments.some( - (segment) => typeof segment !== "string" || segment.trim().length === 0, - ))) || + (candidate.pathSegments !== undefined && !Array.isArray(candidate.pathSegments)) || + !resolveValidatedTargetPathSegments({ + type: candidate.type, + path: candidate.path, + pathSegments: candidate.pathSegments, + providerId: candidate.providerId, + accountId: candidate.accountId, + }) || !ref || typeof ref !== "object" || (ref.source !== "env" && ref.source !== "file" && ref.source !== "exec") ||