diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 9a77f6ac1a3..5cad5acea9d 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -2713,6 +2713,7 @@ Validation: - `source: "env"` id pattern: `^[A-Z][A-Z0-9_]{0,127}$` - `source: "file"` id: absolute JSON pointer (for example `"/providers/openai/apiKey"`) - `source: "exec"` id pattern: `^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$` +- `source: "exec"` ids must not contain `.` or `..` slash-delimited path segments (for example `a/../b` is rejected) ### Supported credential surface diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 213c98f9f14..76b89a0f28a 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -114,6 +114,7 @@ Validation: - `provider` must match `^[a-z][a-z0-9_-]{0,63}$` - `id` must match `^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$` +- `id` must not contain `.` or `..` as slash-delimited path segments (for example `a/../b` is rejected) ## Provider config diff --git a/docs/help/testing.md b/docs/help/testing.md index 9e965b4c769..6580de4da20 100644 --- a/docs/help/testing.md +++ b/docs/help/testing.md @@ -409,3 +409,6 @@ When you fix a provider/model issue discovered in live: - Prefer targeting the smallest layer that catches the bug: - provider request conversion/replay bug → direct models test - gateway session/history/tool pipeline bug → gateway live smoke or CI-safe gateway mock test +- SecretRef traversal guardrail: + - `src/secrets/exec-secret-ref-id-parity.test.ts` derives one sampled target per SecretRef class from registry metadata (`listSecretTargetRegistryEntries()`), then asserts traversal-segment exec ids are rejected. + - If you add a new `includeInPlan` SecretRef target family in `src/secrets/target-registry-data.ts`, update `classifyTargetClass` in that test. The test intentionally fails on unclassified target ids so new classes cannot be skipped silently. diff --git a/src/commands/auth-choice.apply-helpers.ts b/src/commands/auth-choice.apply-helpers.ts index 122be392153..32c6ac82786 100644 --- a/src/commands/auth-choice.apply-helpers.ts +++ b/src/commands/auth-choice.apply-helpers.ts @@ -8,6 +8,8 @@ import { import { encodeJsonPointerToken } from "../secrets/json-pointer.js"; import { PROVIDER_ENV_VARS } from "../secrets/provider-env-vars.js"; import { + formatExecSecretRefIdValidationMessage, + isValidExecSecretRefId, isValidFileSecretRefId, resolveDefaultSecretProviderAlias, } from "../secrets/ref-contract.js"; @@ -238,6 +240,9 @@ export async function promptSecretRefForOnboarding(params: { ) { return 'singleValue mode expects id "value".'; } + if (providerEntry.source === "exec" && !isValidExecSecretRefId(candidate)) { + return formatExecSecretRefIdValidationMessage(); + } return undefined; }, }); diff --git a/src/config/config.secrets-schema.test.ts b/src/config/config.secrets-schema.test.ts index 196bb50ace4..e3c236fb15b 100644 --- a/src/config/config.secrets-schema.test.ts +++ b/src/config/config.secrets-schema.test.ts @@ -1,4 +1,8 @@ import { describe, expect, it } from "vitest"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../test-utils/secret-ref-test-vectors.js"; import { validateConfigObjectRaw } from "./validation.js"; function validateOpenAiApiKeyRef(apiKey: unknown) { @@ -173,4 +177,31 @@ describe("config secret refs schema", () => { ).toBe(true); } }); + + it("accepts valid exec secret reference ids", () => { + for (const id of VALID_EXEC_SECRET_REF_IDS) { + const result = validateOpenAiApiKeyRef({ + source: "exec", + provider: "vault", + id, + }); + expect(result.ok, `expected valid exec ref id: ${id}`).toBe(true); + } + }); + + it("rejects invalid exec secret reference ids", () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + const result = validateOpenAiApiKeyRef({ + source: "exec", + provider: "vault", + id, + }); + expect(result.ok, `expected invalid exec ref id: ${id}`).toBe(false); + if (!result.ok) { + expect( + result.issues.some((issue) => issue.path.includes("models.providers.openai.apiKey")), + ).toBe(true); + } + } + }); }); diff --git a/src/config/zod-schema.core.ts b/src/config/zod-schema.core.ts index 23accd81637..066a33f0f4f 100644 --- a/src/config/zod-schema.core.ts +++ b/src/config/zod-schema.core.ts @@ -1,14 +1,17 @@ import path from "node:path"; import { z } from "zod"; import { isSafeExecutableValue } from "../infra/exec-safety.js"; -import { isValidFileSecretRefId } from "../secrets/ref-contract.js"; +import { + formatExecSecretRefIdValidationMessage, + isValidExecSecretRefId, + isValidFileSecretRefId, +} from "../secrets/ref-contract.js"; import { MODEL_APIS } from "./types.models.js"; import { createAllowDenyChannelRulesSchema } from "./zod-schema.allowdeny.js"; import { sensitive } from "./zod-schema.sensitive.js"; const ENV_SECRET_REF_ID_PATTERN = /^[A-Z][A-Z0-9_]{0,127}$/; const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; -const EXEC_SECRET_REF_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$/; const WINDOWS_ABS_PATH_PATTERN = /^[A-Za-z]:[\\/]/; const WINDOWS_UNC_PATH_PATTERN = /^\\\\[^\\]+\\[^\\]+/; @@ -65,12 +68,7 @@ const ExecSecretRefSchema = z SECRET_PROVIDER_ALIAS_PATTERN, 'Secret reference provider must match /^[a-z][a-z0-9_-]{0,63}$/ (example: "default").', ), - id: z - .string() - .regex( - EXEC_SECRET_REF_ID_PATTERN, - 'Exec secret reference id must match /^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$/ (example: "vault/openai/api-key").', - ), + id: z.string().refine(isValidExecSecretRefId, formatExecSecretRefIdValidationMessage()), }) .strict(); diff --git a/src/gateway/protocol/primitives.secretref.test.ts b/src/gateway/protocol/primitives.secretref.test.ts new file mode 100644 index 00000000000..67f8304d48e --- /dev/null +++ b/src/gateway/protocol/primitives.secretref.test.ts @@ -0,0 +1,34 @@ +import AjvPkg from "ajv"; +import { describe, expect, it } from "vitest"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../../test-utils/secret-ref-test-vectors.js"; +import { SecretInputSchema, SecretRefSchema } from "./schema/primitives.js"; + +describe("gateway protocol SecretRef schema", () => { + const Ajv = AjvPkg as unknown as new (opts?: object) => import("ajv").default; + const ajv = new Ajv({ allErrors: true, strict: false }); + const validateSecretRef = ajv.compile(SecretRefSchema); + const validateSecretInput = ajv.compile(SecretInputSchema); + + it("accepts valid source-specific refs", () => { + expect(validateSecretRef({ source: "env", provider: "default", id: "OPENAI_API_KEY" })).toBe( + true, + ); + expect( + validateSecretRef({ source: "file", provider: "filemain", id: "/providers/openai/apiKey" }), + ).toBe(true); + for (const id of VALID_EXEC_SECRET_REF_IDS) { + expect(validateSecretRef({ source: "exec", provider: "vault", id }), id).toBe(true); + expect(validateSecretInput({ source: "exec", provider: "vault", id }), id).toBe(true); + } + }); + + it("rejects invalid exec refs", () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + expect(validateSecretRef({ source: "exec", provider: "vault", id }), id).toBe(false); + expect(validateSecretInput({ source: "exec", provider: "vault", id }), id).toBe(false); + } + }); +}); diff --git a/src/gateway/protocol/schema/primitives.ts b/src/gateway/protocol/schema/primitives.ts index 2268d1bde50..6ac6a71b64a 100644 --- a/src/gateway/protocol/schema/primitives.ts +++ b/src/gateway/protocol/schema/primitives.ts @@ -1,4 +1,10 @@ import { Type } from "@sinclair/typebox"; +import { ENV_SECRET_REF_ID_RE } from "../../../config/types.secrets.js"; +import { + EXEC_SECRET_REF_ID_JSON_SCHEMA_PATTERN, + FILE_SECRET_REF_ID_PATTERN, + SECRET_PROVIDER_ALIAS_PATTERN, +} from "../../../secrets/ref-contract.js"; import { SESSION_LABEL_MAX_LENGTH } from "../../../sessions/session-label.js"; import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../client-info.js"; @@ -27,13 +33,41 @@ export const SecretRefSourceSchema = Type.Union([ Type.Literal("exec"), ]); -export const SecretRefSchema = Type.Object( +const SecretProviderAliasString = Type.String({ + pattern: SECRET_PROVIDER_ALIAS_PATTERN.source, +}); + +const EnvSecretRefSchema = Type.Object( { - source: SecretRefSourceSchema, - provider: NonEmptyString, - id: NonEmptyString, + source: Type.Literal("env"), + provider: SecretProviderAliasString, + id: Type.String({ pattern: ENV_SECRET_REF_ID_RE.source }), }, { additionalProperties: false }, ); +const FileSecretRefSchema = Type.Object( + { + source: Type.Literal("file"), + provider: SecretProviderAliasString, + id: Type.String({ pattern: FILE_SECRET_REF_ID_PATTERN.source }), + }, + { additionalProperties: false }, +); + +const ExecSecretRefSchema = Type.Object( + { + source: Type.Literal("exec"), + provider: SecretProviderAliasString, + id: Type.String({ pattern: EXEC_SECRET_REF_ID_JSON_SCHEMA_PATTERN }), + }, + { additionalProperties: false }, +); + +export const SecretRefSchema = Type.Union([ + EnvSecretRefSchema, + FileSecretRefSchema, + ExecSecretRefSchema, +]); + export const SecretInputSchema = Type.Union([Type.String(), SecretRefSchema]); diff --git a/src/gateway/server.reload.test.ts b/src/gateway/server.reload.test.ts index b3a603fa287..d62a3e90968 100644 --- a/src/gateway/server.reload.test.ts +++ b/src/gateway/server.reload.test.ts @@ -8,6 +8,7 @@ import { installGatewayTestHooks, rpcReq, startServerWithClient, + testState, withGatewayServer, } from "./test-helpers.js"; @@ -242,6 +243,94 @@ describe("gateway hot reload", () => { ); } + async function writeTalkApiKeyEnvRefConfig(refId = "TALK_API_KEY_REF") { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH is not set"); + } + await fs.writeFile( + configPath, + `${JSON.stringify( + { + talk: { + apiKey: { source: "env", provider: "default", id: refId }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + } + + async function writeGatewayTraversalExecRefConfig() { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH is not set"); + } + await fs.writeFile( + configPath, + `${JSON.stringify( + { + gateway: { + auth: { + mode: "token", + token: { source: "exec", provider: "vault", id: "a/../b" }, + }, + }, + secrets: { + providers: { + vault: { + source: "exec", + command: process.execPath, + }, + }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + } + + async function writeGatewayTokenExecRefConfig(params: { + resolverScriptPath: string; + modePath: string; + tokenValue: string; + }) { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH is not set"); + } + await fs.writeFile( + configPath, + `${JSON.stringify( + { + gateway: { + auth: { + mode: "token", + token: { source: "exec", provider: "vault", id: "gateway/token" }, + }, + }, + secrets: { + providers: { + vault: { + source: "exec", + command: process.execPath, + allowSymlinkCommand: true, + args: [params.resolverScriptPath, params.modePath, params.tokenValue], + }, + }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + } + async function writeDisabledSurfaceRefConfig() { const configPath = process.env.OPENCLAW_CONFIG_PATH; if (!configPath) { @@ -485,6 +574,13 @@ describe("gateway hot reload", () => { ); }); + it("fails startup when an active exec ref id contains traversal segments", async () => { + await writeGatewayTraversalExecRefConfig(); + await expect(withGatewayServer(async () => {})).rejects.toThrow( + /must not include "\." or "\.\." path segments/i, + ); + }); + it("allows startup when unresolved refs exist only on disabled surfaces", async () => { await writeDisabledSurfaceRefConfig(); delete process.env.DISABLED_TELEGRAM_STARTUP_REF; @@ -650,6 +746,154 @@ describe("gateway hot reload", () => { await server.close(); } }); + + it("keeps last-known-good snapshot active when secrets.reload fails over RPC", async () => { + const refId = "RUNTIME_LKG_TALK_API_KEY"; + const previousRefValue = process.env[refId]; + process.env[refId] = "talk-key-before-reload-failure"; // pragma: allowlist secret + await writeTalkApiKeyEnvRefConfig(refId); + + const { server, ws } = await startServerWithClient(); + try { + await connectOk(ws); + const preResolve = await rpcReq<{ + assignments?: Array<{ path: string; pathSegments: string[]; value: unknown }>; + }>(ws, "secrets.resolve", { + commandName: "runtime-lkg-test", + targetIds: ["talk.apiKey"], + }); + expect(preResolve.ok).toBe(true); + expect(preResolve.payload?.assignments?.[0]?.path).toBe("talk.apiKey"); + expect(preResolve.payload?.assignments?.[0]?.value).toBe("talk-key-before-reload-failure"); + + delete process.env[refId]; + const reload = await rpcReq<{ warningCount?: number }>(ws, "secrets.reload", {}); + expect(reload.ok).toBe(false); + expect(reload.error?.code).toBe("UNAVAILABLE"); + expect(reload.error?.message ?? "").toContain(refId); + + const postResolve = await rpcReq<{ + assignments?: Array<{ path: string; pathSegments: string[]; value: unknown }>; + }>(ws, "secrets.resolve", { + commandName: "runtime-lkg-test", + targetIds: ["talk.apiKey"], + }); + expect(postResolve.ok).toBe(true); + expect(postResolve.payload?.assignments?.[0]?.path).toBe("talk.apiKey"); + expect(postResolve.payload?.assignments?.[0]?.value).toBe("talk-key-before-reload-failure"); + } finally { + if (previousRefValue === undefined) { + delete process.env[refId]; + } else { + process.env[refId] = previousRefValue; + } + ws.close(); + await server.close(); + } + }); + + it("keeps last-known-good auth snapshot active when gateway auth token exec reload fails", async () => { + const stateDir = process.env.OPENCLAW_STATE_DIR; + if (!stateDir) { + throw new Error("OPENCLAW_STATE_DIR is not set"); + } + const resolverScriptPath = path.join(stateDir, "gateway-auth-token-resolver.cjs"); + const modePath = path.join(stateDir, "gateway-auth-token-resolver.mode"); + const tokenValue = "gateway-auth-exec-token"; + await fs.mkdir(path.dirname(resolverScriptPath), { recursive: true }); + await fs.writeFile( + resolverScriptPath, + `const fs = require("node:fs"); +let input = ""; +process.stdin.setEncoding("utf8"); +process.stdin.on("data", (chunk) => { + input += chunk; +}); +process.stdin.on("end", () => { + const modePath = process.argv[2]; + const token = process.argv[3]; + const mode = fs.existsSync(modePath) ? fs.readFileSync(modePath, "utf8").trim() : "ok"; + let ids = ["gateway/token"]; + try { + const parsed = JSON.parse(input || "{}"); + if (Array.isArray(parsed.ids) && parsed.ids.length > 0) { + ids = parsed.ids.map((entry) => String(entry)); + } + } catch {} + + if (mode === "fail") { + const errors = {}; + for (const id of ids) { + errors[id] = { message: "forced failure" }; + } + process.stdout.write(JSON.stringify({ protocolVersion: 1, values: {}, errors }) + "\\n"); + return; + } + + const values = {}; + for (const id of ids) { + values[id] = token; + } + process.stdout.write(JSON.stringify({ protocolVersion: 1, values }) + "\\n"); +}); +`, + "utf8", + ); + await fs.writeFile(modePath, "ok\n", "utf8"); + await writeGatewayTokenExecRefConfig({ + resolverScriptPath, + modePath, + tokenValue, + }); + + const previousGatewayAuth = testState.gatewayAuth; + const previousGatewayTokenEnv = process.env.OPENCLAW_GATEWAY_TOKEN; + testState.gatewayAuth = undefined; + delete process.env.OPENCLAW_GATEWAY_TOKEN; + + const started = await startServerWithClient(); + const { server, ws, envSnapshot } = started; + try { + await connectOk(ws, { + token: tokenValue, + }); + const preResolve = await rpcReq<{ + assignments?: Array<{ path: string; pathSegments: string[]; value: unknown }>; + }>(ws, "secrets.resolve", { + commandName: "runtime-lkg-auth-test", + targetIds: ["gateway.auth.token"], + }); + expect(preResolve.ok).toBe(true); + expect(preResolve.payload?.assignments?.[0]?.path).toBe("gateway.auth.token"); + expect(preResolve.payload?.assignments?.[0]?.value).toBe(tokenValue); + + await fs.writeFile(modePath, "fail\n", "utf8"); + const reload = await rpcReq<{ warningCount?: number }>(ws, "secrets.reload", {}); + expect(reload.ok).toBe(false); + expect(reload.error?.code).toBe("UNAVAILABLE"); + expect(reload.error?.message ?? "").toContain("forced failure"); + + const postResolve = await rpcReq<{ + assignments?: Array<{ path: string; pathSegments: string[]; value: unknown }>; + }>(ws, "secrets.resolve", { + commandName: "runtime-lkg-auth-test", + targetIds: ["gateway.auth.token"], + }); + expect(postResolve.ok).toBe(true); + expect(postResolve.payload?.assignments?.[0]?.path).toBe("gateway.auth.token"); + expect(postResolve.payload?.assignments?.[0]?.value).toBe(tokenValue); + } finally { + testState.gatewayAuth = previousGatewayAuth; + if (previousGatewayTokenEnv === undefined) { + delete process.env.OPENCLAW_GATEWAY_TOKEN; + } else { + process.env.OPENCLAW_GATEWAY_TOKEN = previousGatewayTokenEnv; + } + envSnapshot.restore(); + ws.close(); + await server.close(); + } + }); }); describe("gateway agents", () => { diff --git a/src/plugin-sdk/secret-input-schema.test.ts b/src/plugin-sdk/secret-input-schema.test.ts new file mode 100644 index 00000000000..1a4463c830a --- /dev/null +++ b/src/plugin-sdk/secret-input-schema.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../test-utils/secret-ref-test-vectors.js"; +import { buildSecretInputSchema } from "./secret-input-schema.js"; + +describe("plugin-sdk secret input schema", () => { + const schema = buildSecretInputSchema(); + + it("accepts plaintext and valid refs", () => { + expect(schema.safeParse("sk-plain").success).toBe(true); + expect( + schema.safeParse({ source: "env", provider: "default", id: "OPENAI_API_KEY" }).success, + ).toBe(true); + expect( + schema.safeParse({ source: "file", provider: "filemain", id: "/providers/openai/apiKey" }) + .success, + ).toBe(true); + for (const id of VALID_EXEC_SECRET_REF_IDS) { + expect(schema.safeParse({ source: "exec", provider: "vault", id }).success, id).toBe(true); + } + }); + + it("rejects invalid exec refs", () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + expect(schema.safeParse({ source: "exec", provider: "vault", id }).success, id).toBe(false); + } + }); +}); diff --git a/src/plugin-sdk/secret-input-schema.ts b/src/plugin-sdk/secret-input-schema.ts index d5eb3a0767e..579d80df441 100644 --- a/src/plugin-sdk/secret-input-schema.ts +++ b/src/plugin-sdk/secret-input-schema.ts @@ -1,12 +1,48 @@ import { z } from "zod"; +import { ENV_SECRET_REF_ID_RE } from "../config/types.secrets.js"; +import { + formatExecSecretRefIdValidationMessage, + isValidExecSecretRefId, + isValidFileSecretRefId, + SECRET_PROVIDER_ALIAS_PATTERN, +} from "../secrets/ref-contract.js"; export function buildSecretInputSchema() { + const providerSchema = z + .string() + .regex( + SECRET_PROVIDER_ALIAS_PATTERN, + 'Secret reference provider must match /^[a-z][a-z0-9_-]{0,63}$/ (example: "default").', + ); + return z.union([ z.string(), - z.object({ - source: z.enum(["env", "file", "exec"]), - provider: z.string().min(1), - id: z.string().min(1), - }), + z.discriminatedUnion("source", [ + z.object({ + source: z.literal("env"), + provider: providerSchema, + id: z + .string() + .regex( + ENV_SECRET_REF_ID_RE, + 'Env secret reference id must match /^[A-Z][A-Z0-9_]{0,127}$/ (example: "OPENAI_API_KEY").', + ), + }), + z.object({ + source: z.literal("file"), + provider: providerSchema, + id: z + .string() + .refine( + isValidFileSecretRefId, + 'File secret reference id must be an absolute JSON pointer (example: "/providers/openai/apiKey"), or "value" for singleValue mode.', + ), + }), + z.object({ + source: z.literal("exec"), + provider: providerSchema, + id: z.string().refine(isValidExecSecretRefId, formatExecSecretRefIdValidationMessage()), + }), + ]), ]); } diff --git a/src/secrets/configure.ts b/src/secrets/configure.ts index 0934c603c2d..a07d3b45903 100644 --- a/src/secrets/configure.ts +++ b/src/secrets/configure.ts @@ -20,7 +20,12 @@ import { } from "./configure-plan.js"; import type { SecretsApplyPlan } from "./plan.js"; import { PROVIDER_ENV_VARS } from "./provider-env-vars.js"; -import { isValidSecretProviderAlias, resolveDefaultSecretProviderAlias } from "./ref-contract.js"; +import { + formatExecSecretRefIdValidationMessage, + isValidExecSecretRefId, + isValidSecretProviderAlias, + resolveDefaultSecretProviderAlias, +} from "./ref-contract.js"; import { resolveSecretRefValue } from "./resolve.js"; import { assertExpectedResolvedSecretValue } from "./secret-value.js"; import { isRecord } from "./shared.js"; @@ -917,7 +922,16 @@ export async function runSecretsConfigureInteractive( await text({ message: "Secret id", initialValue: suggestedId, - validate: (value) => (String(value ?? "").trim().length > 0 ? undefined : "Required"), + validate: (value) => { + const trimmed = String(value ?? "").trim(); + if (!trimmed) { + return "Required"; + } + if (source === "exec" && !isValidExecSecretRefId(trimmed)) { + return formatExecSecretRefIdValidationMessage(); + } + return undefined; + }, }), "Secrets configure cancelled.", ); diff --git a/src/secrets/exec-secret-ref-id-parity.test.ts b/src/secrets/exec-secret-ref-id-parity.test.ts new file mode 100644 index 00000000000..c3d9cb10fbc --- /dev/null +++ b/src/secrets/exec-secret-ref-id-parity.test.ts @@ -0,0 +1,199 @@ +import AjvPkg from "ajv"; +import { describe, expect, it } from "vitest"; +import { validateConfigObjectRaw } from "../config/validation.js"; +import { SecretRefSchema as GatewaySecretRefSchema } from "../gateway/protocol/schema/primitives.js"; +import { buildSecretInputSchema } from "../plugin-sdk/secret-input-schema.js"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../test-utils/secret-ref-test-vectors.js"; +import { isSecretsApplyPlan } from "./plan.js"; +import { isValidExecSecretRefId } from "./ref-contract.js"; +import { materializePathTokens, parsePathPattern } from "./target-registry-pattern.js"; +import { listSecretTargetRegistryEntries } from "./target-registry.js"; + +describe("exec SecretRef id parity", () => { + const Ajv = AjvPkg as unknown as new (opts?: object) => import("ajv").default; + const ajv = new Ajv({ allErrors: true, strict: false }); + const validateGatewaySecretRef = ajv.compile(GatewaySecretRefSchema); + const pluginSdkSecretInput = buildSecretInputSchema(); + + function configAcceptsExecRef(id: string): boolean { + const result = validateConfigObjectRaw({ + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "exec", provider: "vault", id }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }, + }); + return result.ok; + } + + function planAcceptsExecRef(id: string): boolean { + return isSecretsApplyPlan({ + version: 1, + protocolVersion: 1, + generatedAt: "2026-03-10T00:00:00.000Z", + generatedBy: "manual", + targets: [ + { + type: "talk.apiKey", + path: "talk.apiKey", + pathSegments: ["talk", "apiKey"], + ref: { source: "exec", provider: "vault", id }, + }, + ], + }); + } + + for (const id of [...VALID_EXEC_SECRET_REF_IDS, ...INVALID_EXEC_SECRET_REF_IDS]) { + it(`keeps config/plan/gateway/plugin parity for exec id "${id}"`, () => { + const expected = isValidExecSecretRefId(id); + expect(configAcceptsExecRef(id)).toBe(expected); + expect(planAcceptsExecRef(id)).toBe(expected); + expect(validateGatewaySecretRef({ source: "exec", provider: "vault", id })).toBe(expected); + expect( + pluginSdkSecretInput.safeParse({ source: "exec", provider: "vault", id }).success, + ).toBe(expected); + }); + } + + function classifyTargetClass(id: string): string { + if (id.startsWith("auth-profiles.")) { + return "auth-profiles"; + } + if (id.startsWith("agents.")) { + return "agents"; + } + if (id.startsWith("channels.")) { + return "channels"; + } + if (id.startsWith("cron.")) { + return "cron"; + } + if (id.startsWith("gateway.auth.")) { + return "gateway.auth"; + } + if (id.startsWith("gateway.remote.")) { + return "gateway.remote"; + } + if (id.startsWith("messages.")) { + return "messages"; + } + if (id.startsWith("models.providers.") && id.includes(".headers.")) { + return "models.headers"; + } + if (id.startsWith("models.providers.")) { + return "models.apiKey"; + } + if (id.startsWith("skills.entries.")) { + return "skills"; + } + if (id.startsWith("talk.")) { + return "talk"; + } + if (id.startsWith("tools.web.fetch.")) { + return "tools.web.fetch"; + } + if (id.startsWith("tools.web.search.")) { + return "tools.web.search"; + } + return "unclassified"; + } + + function samplePathSegments(pathPattern: string): string[] { + const tokens = parsePathPattern(pathPattern); + const captures = tokens.flatMap((token) => { + if (token.kind === "literal") { + return []; + } + return [token.kind === "array" ? "0" : "sample"]; + }); + const segments = materializePathTokens(tokens, captures); + if (!segments) { + throw new Error(`failed to sample path segments for pattern "${pathPattern}"`); + } + return segments; + } + + const registryPlanTargets = listSecretTargetRegistryEntries().filter( + (entry) => entry.includeInPlan, + ); + const unclassifiedTargetIds = registryPlanTargets + .filter((entry) => classifyTargetClass(entry.id) === "unclassified") + .map((entry) => entry.id); + const sampledTargetsByClass = [ + ...new Set(registryPlanTargets.map((entry) => classifyTargetClass(entry.id))), + ] + .toSorted((a, b) => a.localeCompare(b)) + .map((className) => { + const candidates = registryPlanTargets + .filter((entry) => classifyTargetClass(entry.id) === className) + .toSorted((a, b) => a.id.localeCompare(b.id)); + const selected = candidates[0]; + if (!selected) { + throw new Error(`missing sampled target for class "${className}"`); + } + const pathSegments = samplePathSegments(selected.pathPattern); + return { + className, + id: selected.id, + type: selected.targetType, + configFile: selected.configFile, + pathSegments, + }; + }); + + function planAcceptsExecRefForSample(params: { + type: string; + configFile: "openclaw.json" | "auth-profiles.json"; + pathSegments: string[]; + id: string; + }): boolean { + return isSecretsApplyPlan({ + version: 1, + protocolVersion: 1, + generatedAt: "2026-03-10T00:00:00.000Z", + generatedBy: "manual", + targets: [ + { + type: params.type, + path: params.pathSegments.join("."), + pathSegments: params.pathSegments, + ref: { source: "exec", provider: "vault", id: params.id }, + ...(params.configFile === "auth-profiles.json" ? { agentId: "main" } : {}), + }, + ], + }); + } + + it("derives sampled class coverage from target registry metadata", () => { + expect(unclassifiedTargetIds).toEqual([]); + expect(sampledTargetsByClass.length).toBeGreaterThan(0); + }); + + for (const sample of sampledTargetsByClass) { + it(`rejects traversal-segment exec ids for sampled class "${sample.className}" (example: "${sample.id}")`, () => { + expect( + planAcceptsExecRefForSample({ + type: sample.type, + configFile: sample.configFile, + pathSegments: sample.pathSegments, + id: "vault/openai/apiKey", + }), + ).toBe(true); + expect( + planAcceptsExecRefForSample({ + type: sample.type, + configFile: sample.configFile, + pathSegments: sample.pathSegments, + id: "vault/../apiKey", + }), + ).toBe(false); + }); + } +}); diff --git a/src/secrets/plan.test.ts b/src/secrets/plan.test.ts index 01ee81ea551..ec4d2c8dcba 100644 --- a/src/secrets/plan.test.ts +++ b/src/secrets/plan.test.ts @@ -1,4 +1,8 @@ import { describe, expect, it } from "vitest"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../test-utils/secret-ref-test-vectors.js"; import { isSecretsApplyPlan, resolveValidatedPlanTarget } from "./plan.js"; describe("secrets plan validation", () => { @@ -98,4 +102,44 @@ describe("secrets plan validation", () => { }); expect(withAgent).toBe(true); }); + + it("accepts valid exec secret ref ids in plans", () => { + for (const id of VALID_EXEC_SECRET_REF_IDS) { + const isValid = isSecretsApplyPlan({ + version: 1, + protocolVersion: 1, + generatedAt: "2026-03-10T00:00:00.000Z", + generatedBy: "manual", + targets: [ + { + type: "talk.apiKey", + path: "talk.apiKey", + pathSegments: ["talk", "apiKey"], + ref: { source: "exec", provider: "vault", id }, + }, + ], + }); + expect(isValid, `expected valid plan exec ref id: ${id}`).toBe(true); + } + }); + + it("rejects invalid exec secret ref ids in plans", () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + const isValid = isSecretsApplyPlan({ + version: 1, + protocolVersion: 1, + generatedAt: "2026-03-10T00:00:00.000Z", + generatedBy: "manual", + targets: [ + { + type: "talk.apiKey", + path: "talk.apiKey", + pathSegments: ["talk", "apiKey"], + ref: { source: "exec", provider: "vault", id }, + }, + ], + }); + expect(isValid, `expected invalid plan exec ref id: ${id}`).toBe(false); + } + }); }); diff --git a/src/secrets/plan.ts b/src/secrets/plan.ts index 3101e1b7828..4f576a4f25f 100644 --- a/src/secrets/plan.ts +++ b/src/secrets/plan.ts @@ -1,6 +1,6 @@ import type { SecretProviderConfig, SecretRef } from "../config/types.secrets.js"; import { SecretProviderSchema } from "../config/zod-schema.core.js"; -import { isValidSecretProviderAlias } from "./ref-contract.js"; +import { isValidExecSecretRefId, isValidSecretProviderAlias } from "./ref-contract.js"; import { parseDotPath, toDotPath } from "./shared.js"; import { isKnownSecretTargetType, @@ -140,7 +140,8 @@ export function isSecretsApplyPlan(value: unknown): value is SecretsApplyPlan { typeof ref.provider !== "string" || ref.provider.trim().length === 0 || typeof ref.id !== "string" || - ref.id.trim().length === 0 + ref.id.trim().length === 0 || + (ref.source === "exec" && !isValidExecSecretRefId(ref.id)) ) { return false; } diff --git a/src/secrets/ref-contract.test.ts b/src/secrets/ref-contract.test.ts new file mode 100644 index 00000000000..2820ee71f46 --- /dev/null +++ b/src/secrets/ref-contract.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { + INVALID_EXEC_SECRET_REF_IDS, + VALID_EXEC_SECRET_REF_IDS, +} from "../test-utils/secret-ref-test-vectors.js"; +import { isValidExecSecretRefId, validateExecSecretRefId } from "./ref-contract.js"; + +describe("exec secret ref id validation", () => { + it("accepts valid exec secret ref ids", () => { + for (const id of VALID_EXEC_SECRET_REF_IDS) { + expect(isValidExecSecretRefId(id), `expected valid id: ${id}`).toBe(true); + expect(validateExecSecretRefId(id)).toEqual({ ok: true }); + } + }); + + it("rejects invalid exec secret ref ids", () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + expect(isValidExecSecretRefId(id), `expected invalid id: ${id}`).toBe(false); + expect(validateExecSecretRefId(id).ok).toBe(false); + } + }); + + it("reports traversal segment failures separately", () => { + expect(validateExecSecretRefId("a/../b")).toEqual({ + ok: false, + reason: "traversal-segment", + }); + expect(validateExecSecretRefId("a/./b")).toEqual({ + ok: false, + reason: "traversal-segment", + }); + }); +}); diff --git a/src/secrets/ref-contract.ts b/src/secrets/ref-contract.ts index cd08b40a847..946e9ca1bb3 100644 --- a/src/secrets/ref-contract.ts +++ b/src/secrets/ref-contract.ts @@ -6,8 +6,21 @@ import { const FILE_SECRET_REF_SEGMENT_PATTERN = /^(?:[^~]|~0|~1)*$/; export const SECRET_PROVIDER_ALIAS_PATTERN = /^[a-z][a-z0-9_-]{0,63}$/; +const EXEC_SECRET_REF_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$/; export const SINGLE_VALUE_FILE_REF_ID = "value"; +export const FILE_SECRET_REF_ID_PATTERN = /^(?:value|\/(?:[^~]|~0|~1)*(?:\/(?:[^~]|~0|~1)*)*)$/; +export const EXEC_SECRET_REF_ID_JSON_SCHEMA_PATTERN = + "^(?!.*(?:^|/)\\.{1,2}(?:/|$))[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$"; + +export type ExecSecretRefIdValidationReason = "pattern" | "traversal-segment"; + +export type ExecSecretRefIdValidationResult = + | { ok: true } + | { + ok: false; + reason: ExecSecretRefIdValidationReason; + }; export type SecretRefDefaultsCarrier = { secrets?: { @@ -69,3 +82,27 @@ export function isValidFileSecretRefId(value: string): boolean { export function isValidSecretProviderAlias(value: string): boolean { return SECRET_PROVIDER_ALIAS_PATTERN.test(value); } + +export function validateExecSecretRefId(value: string): ExecSecretRefIdValidationResult { + if (!EXEC_SECRET_REF_ID_PATTERN.test(value)) { + return { ok: false, reason: "pattern" }; + } + for (const segment of value.split("/")) { + if (segment === "." || segment === "..") { + return { ok: false, reason: "traversal-segment" }; + } + } + return { ok: true }; +} + +export function isValidExecSecretRefId(value: string): boolean { + return validateExecSecretRefId(value).ok; +} + +export function formatExecSecretRefIdValidationMessage(): string { + return [ + "Exec secret reference id must match /^[A-Za-z0-9][A-Za-z0-9._:/-]{0,255}$/", + 'and must not include "." or ".." path segments', + '(example: "vault/openai/api-key").', + ].join(" "); +} diff --git a/src/secrets/resolve.test.ts b/src/secrets/resolve.test.ts index 7b74e582b85..7f906d00919 100644 --- a/src/secrets/resolve.test.ts +++ b/src/secrets/resolve.test.ts @@ -3,7 +3,12 @@ import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { resolveSecretRefString, resolveSecretRefValue } from "./resolve.js"; +import { INVALID_EXEC_SECRET_REF_IDS } from "../test-utils/secret-ref-test-vectors.js"; +import { + resolveSecretRefString, + resolveSecretRefValue, + resolveSecretRefValues, +} from "./resolve.js"; async function writeSecureFile(filePath: string, content: string, mode = 0o600): Promise { await fs.mkdir(path.dirname(filePath), { recursive: true }); @@ -232,12 +237,16 @@ describe("secret ref resolver", () => { expect(value).toBe("plain-secret"); }); - itPosix("ignores EPIPE when exec provider exits before consuming stdin", async () => { - const oversizedId = `openai/${"x".repeat(120_000)}`; - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: oversizedId }, - { + itPosix( + "tolerates stdin write errors when exec provider exits before consuming a large request", + async () => { + const refs = Array.from({ length: 256 }, (_, index) => ({ + source: "exec" as const, + provider: "execmain", + id: `openai/${String(index).padStart(3, "0")}/${"x".repeat(240)}`, + })); + await expect( + resolveSecretRefValues(refs, { config: { secrets: { providers: { @@ -248,10 +257,10 @@ describe("secret ref resolver", () => { }, }, }, - }, - ), - ).rejects.toThrow('Exec provider "execmain" returned empty stdout.'); - }); + }), + ).rejects.toThrow('Exec provider "execmain" returned empty stdout.'); + }, + ); itPosix("rejects symlink command paths unless allowSymlinkCommand is enabled", async () => { const root = await createCaseDir("exec-link-reject"); @@ -432,4 +441,17 @@ describe("secret ref resolver", () => { ), ).rejects.toThrow('has source "env" but ref requests "exec"'); }); + + it("rejects invalid exec ids before provider resolution", async () => { + for (const id of INVALID_EXEC_SECRET_REF_IDS) { + await expect( + resolveSecretRefValue( + { source: "exec", provider: "vault", id }, + { + config: {}, + }, + ), + ).rejects.toThrow(/Exec secret reference id must match|Secret reference id is empty/); + } + }); }); diff --git a/src/secrets/resolve.ts b/src/secrets/resolve.ts index 039875c464c..103075b8cd9 100644 --- a/src/secrets/resolve.ts +++ b/src/secrets/resolve.ts @@ -15,6 +15,8 @@ import { resolveUserPath } from "../utils.js"; import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js"; import { readJsonPointer } from "./json-pointer.js"; import { + formatExecSecretRefIdValidationMessage, + isValidExecSecretRefId, SINGLE_VALUE_FILE_REF_ID, resolveDefaultSecretProviderAlias, secretRefKey, @@ -843,6 +845,11 @@ export async function resolveSecretRefValues( if (!id) { throw new Error("Secret reference id is empty."); } + if (ref.source === "exec" && !isValidExecSecretRefId(id)) { + throw new Error( + `${formatExecSecretRefIdValidationMessage()} (ref: ${ref.source}:${ref.provider}:${id}).`, + ); + } uniqueRefs.set(secretRefKey(ref), { ...ref, id }); } diff --git a/src/secrets/runtime.test.ts b/src/secrets/runtime.test.ts index f03ce73da3e..47628f1bfe2 100644 --- a/src/secrets/runtime.test.ts +++ b/src/secrets/runtime.test.ts @@ -1134,6 +1134,29 @@ describe("secrets runtime snapshot", () => { ).rejects.toThrow(/MISSING_GATEWAY_TOKEN_REF/i); }); + it("fails when an active exec ref id contains traversal segments", async () => { + await expect( + prepareSecretsRuntimeSnapshot({ + config: asConfig({ + talk: { + apiKey: { source: "exec", provider: "vault", id: "a/../b" }, + }, + secrets: { + providers: { + vault: { + source: "exec", + command: process.execPath, + }, + }, + }, + }), + env: {}, + agentDirs: ["/tmp/openclaw-agent-main"], + loadAuthStore: () => ({ version: 1, profiles: {} }), + }), + ).rejects.toThrow(/must not include "\." or "\.\." path segments/i); + }); + it("treats gateway.auth.password ref as inactive when auth mode is trusted-proxy", async () => { const snapshot = await prepareSecretsRuntimeSnapshot({ config: asConfig({ diff --git a/src/test-utils/secret-ref-test-vectors.ts b/src/test-utils/secret-ref-test-vectors.ts new file mode 100644 index 00000000000..7645f4c24f2 --- /dev/null +++ b/src/test-utils/secret-ref-test-vectors.ts @@ -0,0 +1,24 @@ +export const VALID_EXEC_SECRET_REF_IDS = [ + "vault/openai/api-key", + "vault:secret/mykey", + "providers/openai/apiKey", + "a..b/c", + "a/.../b", + "a/.well-known/key", + `a/${"b".repeat(254)}`, +] as const; + +export const INVALID_EXEC_SECRET_REF_IDS = [ + "", + " ", + "a/../b", + "a/./b", + "../b", + "./b", + "a/..", + "a/.", + "/absolute/path", + "bad id", + "a\\b", + `a${"b".repeat(256)}`, +] as const;