Skip to content

Commit 6bfe7a2

Browse files
vincentkocvincentkoc
committed
fix(secrets): enforce canonical secret refs
1 parent 2babcf0 commit 6bfe7a2

6 files changed

Lines changed: 122 additions & 36 deletions

File tree

‎src/plugin-sdk/secret-input-schema.ts‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,31 +30,37 @@ const secretInputSchema = z
3030
.union([
3131
z.string(),
3232
z.discriminatedUnion("source", [
33-
z.object({
34-
source: z.literal("env"),
35-
provider: providerSchema,
36-
id: z
37-
.string()
38-
.regex(
39-
ENV_SECRET_REF_ID_RE,
40-
'Env secret reference id must match /^[A-Z][A-Z0-9_]{0,127}$/ (example: "OPENAI_API_KEY").',
41-
),
42-
}),
43-
z.object({
44-
source: z.literal("file"),
45-
provider: providerSchema,
46-
id: z
47-
.string()
48-
.refine(
49-
isValidFileSecretRefId,
50-
'File secret reference id must be an absolute JSON pointer (example: "/providers/openai/apiKey"), or "value" for singleValue mode.',
51-
),
52-
}),
53-
z.object({
54-
source: z.literal("exec"),
55-
provider: providerSchema,
56-
id: z.string().refine(isValidExecSecretRefId, formatExecSecretRefIdValidationMessage()),
57-
}),
33+
z
34+
.object({
35+
source: z.literal("env"),
36+
provider: providerSchema,
37+
id: z
38+
.string()
39+
.regex(
40+
ENV_SECRET_REF_ID_RE,
41+
'Env secret reference id must match /^[A-Z][A-Z0-9_]{0,127}$/ (example: "OPENAI_API_KEY").',
42+
),
43+
})
44+
.strict(),
45+
z
46+
.object({
47+
source: z.literal("file"),
48+
provider: providerSchema,
49+
id: z
50+
.string()
51+
.refine(
52+
isValidFileSecretRefId,
53+
'File secret reference id must be an absolute JSON pointer (example: "/providers/openai/apiKey"), or "value" for singleValue mode.',
54+
),
55+
})
56+
.strict(),
57+
z
58+
.object({
59+
source: z.literal("exec"),
60+
provider: providerSchema,
61+
id: z.string().refine(isValidExecSecretRefId, formatExecSecretRefIdValidationMessage()),
62+
})
63+
.strict(),
5864
]),
5965
])
6066
.register(sensitive);

‎src/plugins/provider-auth-env-trust.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,34 @@ describe("provider auth env trust", () => {
4747
});
4848
});
4949

50+
it("buildApiKeyCredential rejects malformed object SecretRefs", async () => {
51+
const { buildApiKeyCredential } = await import("./provider-auth-helpers.js");
52+
const malformedRefs = [
53+
{ source: "env", provider: "default", id: "OPENAI_API_KEY", extra: "x" },
54+
{ source: "env", provider: "Default", id: "OPENAI_API_KEY" },
55+
{ source: "env", provider: "default", id: "openai_api_key" },
56+
{ source: "file", provider: "default", id: "providers/openai/apiKey" },
57+
{ source: "exec", provider: "vault", id: "vault/../api-key" },
58+
];
59+
60+
for (const ref of malformedRefs) {
61+
expect(() => buildApiKeyCredential("openai", ref as never)).toThrow(
62+
"API key SecretRef is invalid.",
63+
);
64+
}
65+
});
66+
67+
it("buildApiKeyCredential keeps invalid env-template strings as plaintext", async () => {
68+
const { buildApiKeyCredential } = await import("./provider-auth-helpers.js");
69+
const overlongEnvRef = `\${A${"B".repeat(128)}}`;
70+
71+
expect(buildApiKeyCredential("openai", overlongEnvRef)).toEqual({
72+
type: "api_key",
73+
provider: "openai",
74+
key: overlongEnvRef,
75+
});
76+
});
77+
5078
it("resolveRefFallbackInput excludes untrusted workspace plugin env vars", async () => {
5179
const { resolveRefFallbackInput } = await import("./provider-auth-ref.js");
5280
const config = { plugins: {} };

‎src/plugins/provider-auth-helpers.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@ import type { OpenClawConfig } from "../config/types.openclaw.js";
1111
import {
1212
coerceSecretRef,
1313
DEFAULT_SECRET_PROVIDER_ALIAS,
14+
parseEnvTemplateSecretRef,
1415
type SecretInput,
1516
type SecretRef,
1617
} from "../config/types.secrets.js";
1718
import type { OAuthCredentials } from "../llm/oauth.js";
1819
import { getProviderEnvVars } from "../secrets/provider-env-vars.js";
20+
import { isValidSecretRef } from "../secrets/ref-contract.js";
1921
import { normalizeSecretInput } from "../utils/normalize-secret-input.js";
2022
import type { SecretInputMode } from "./provider-auth-types.js";
2123

22-
const ENV_REF_PATTERN = /^\$\{([A-Z][A-Z0-9_]*)\}$/;
2324
type UpsertAuthProfileParams = Parameters<typeof upsertAuthProfileWithLock>[0];
2425

2526
const resolveAuthAgentDir = (agentDir?: string, config?: OpenClawConfig) =>
@@ -40,14 +41,6 @@ function buildEnvSecretRef(id: string): SecretRef {
4041
return { source: "env", provider: DEFAULT_SECRET_PROVIDER_ALIAS, id };
4142
}
4243

43-
function parseEnvSecretRef(value: string): SecretRef | null {
44-
const match = ENV_REF_PATTERN.exec(value);
45-
if (!match) {
46-
return null;
47-
}
48-
return buildEnvSecretRef(match[1]);
49-
}
50-
5144
function resolveProviderDefaultEnvSecretRef(provider: string, config?: OpenClawConfig): SecretRef {
5245
const envVars = getProviderEnvVars(provider, {
5346
...(config ? { config } : {}),
@@ -67,15 +60,25 @@ function resolveApiKeySecretInput(
6760
input: SecretInput,
6861
options?: ApiKeyStorageOptions,
6962
): SecretInput {
63+
if (input !== null && typeof input === "object") {
64+
const coercedRef = coerceSecretRef(input);
65+
if (!coercedRef || !isValidSecretRef(coercedRef)) {
66+
throw new Error("API key SecretRef is invalid.");
67+
}
68+
return coercedRef;
69+
}
7070
if (options?.secretInputMode === "plaintext") {
7171
return normalizeSecretInput(input);
7272
}
7373
const coercedRef = coerceSecretRef(input);
7474
if (coercedRef) {
75+
if (!isValidSecretRef(coercedRef)) {
76+
throw new Error("API key SecretRef is invalid.");
77+
}
7578
return coercedRef;
7679
}
7780
const normalized = normalizeSecretInput(input);
78-
const inlineEnvRef = parseEnvSecretRef(normalized);
81+
const inlineEnvRef = parseEnvTemplateSecretRef(normalized, DEFAULT_SECRET_PROVIDER_ALIAS);
7982
if (inlineEnvRef) {
8083
return inlineEnvRef;
8184
}

‎src/secrets/exec-secret-ref-id-parity.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ describe("exec SecretRef id parity", () => {
5757
return result.ok;
5858
}
5959

60+
function configAcceptsRef(ref: unknown): boolean {
61+
const result = validateConfigObjectRaw({
62+
models: {
63+
providers: {
64+
openai: {
65+
baseUrl: "https://api.openai.com/v1",
66+
apiKey: ref,
67+
models: [{ id: "gpt-5", name: "gpt-5" }],
68+
},
69+
},
70+
},
71+
});
72+
return result.ok;
73+
}
74+
6075
function planAcceptsExecRef(id: string): boolean {
6176
return isSecretsApplyPlan({
6277
version: 1,
@@ -75,7 +90,7 @@ describe("exec SecretRef id parity", () => {
7590
});
7691
}
7792

78-
function planAcceptsRef(ref: { source: "env" | "file" | "exec"; provider: string; id: string }) {
93+
function planAcceptsRef(ref: unknown) {
7994
return isSecretsApplyPlan({
8095
version: 1,
8196
protocolVersion: 1,
@@ -128,6 +143,19 @@ describe("exec SecretRef id parity", () => {
128143
expect(pluginSdkSecretInput.safeParse(ref).success).toBe(false);
129144
});
130145

146+
for (const ref of [
147+
{ source: "env", provider: "default", id: "OPENAI_API_KEY", extra: "x" },
148+
{ source: "file", provider: "default", id: "value", extra: "x" },
149+
{ source: "exec", provider: "vault", id: "vault/openai/api-key", extra: "x" },
150+
]) {
151+
it(`rejects non-canonical ${ref.source} refs with extra properties across config/plan/gateway/plugin`, () => {
152+
expect(configAcceptsRef(ref)).toBe(false);
153+
expect(planAcceptsRef(ref)).toBe(false);
154+
expect(validateGatewaySecretRef.Check(ref)).toBe(false);
155+
expect(pluginSdkSecretInput.safeParse(ref).success).toBe(false);
156+
});
157+
}
158+
131159
for (const id of [...VALID_EXEC_SECRET_REF_IDS, ...INVALID_EXEC_SECRET_REF_IDS]) {
132160
it(`keeps config/plan/gateway/plugin parity for exec id "${id}"`, () => {
133161
const expected = isValidExecSecretRefId(id);

‎src/secrets/ref-contract.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import {
1010
isValidExecSecretRefId,
1111
isValidFileSecretRefId,
12+
isValidSecretRef,
1213
validateExecSecretRefId,
1314
} from "./ref-contract.js";
1415

@@ -52,3 +53,19 @@ describe("exec secret ref id validation", () => {
5253
});
5354
});
5455
});
56+
57+
describe("secret ref validation", () => {
58+
it("rejects non-canonical refs with extra properties", () => {
59+
expect(isValidSecretRef({ source: "env", provider: "default", id: "OPENAI_API_KEY" })).toBe(
60+
true,
61+
);
62+
expect(
63+
isValidSecretRef({
64+
source: "env",
65+
provider: "default",
66+
id: "OPENAI_API_KEY",
67+
extra: "x",
68+
} as never),
69+
).toBe(false);
70+
});
71+
});

‎src/secrets/ref-contract.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/** Shared SecretRef grammar and validation helpers for config, schema, SDK, and gateway parity. */
22
import {
33
DEFAULT_SECRET_PROVIDER_ALIAS,
4+
isSecretRef,
45
isValidEnvSecretRefId,
56
type SecretRef,
67
type SecretRefSource,
@@ -135,6 +136,9 @@ export function isValidExecSecretRefId(value: string): boolean {
135136

136137
/** Validates a complete SecretRef against the shared provider/source/id grammar. */
137138
export function isValidSecretRef(ref: SecretRef): boolean {
139+
if (!isSecretRef(ref)) {
140+
return false;
141+
}
138142
if (!isValidSecretProviderAlias(ref.provider)) {
139143
return false;
140144
}

0 commit comments

Comments
 (0)