SecretRef: harden custom/provider secret persistence and reuse (#42554)

* Models: gate custom provider keys by usable secret semantics

* Config: project runtime writes onto source snapshot

* Models: prevent stale apiKey preservation for marker-managed providers

* Runner: strip SecretRef marker headers from resolved models

* Secrets: scan active agent models.json path in audit

* Config: guard runtime-source projection for unrelated configs

* Extensions: fix onboarding type errors in CI

* Tests: align setup helper account-enabled expectation

* Secrets audit: harden models.json file reads

* fix: harden SecretRef custom/provider secret persistence (#42554) (thanks @joshavant)
This commit is contained in:
Josh Avant
2026-03-10 18:46:47 -05:00
committed by Peter Steinberger
parent 20237358d9
commit 36d2ae2a22
40 changed files with 651 additions and 73 deletions

View File

@@ -16,6 +16,7 @@ type AuditFixture = {
};
const OPENAI_API_KEY_MARKER = "OPENAI_API_KEY"; // pragma: allowlist secret
const MAX_AUDIT_MODELS_JSON_BYTES = 5 * 1024 * 1024;
async function writeJsonFile(filePath: string, value: unknown): Promise<void> {
await fs.writeFile(filePath, `${JSON.stringify(value, null, 2)}\n`, "utf8");
@@ -482,6 +483,73 @@ describe("secrets audit", () => {
).toBe(true);
});
it("reports non-regular models.json files as unresolved findings", async () => {
await fs.rm(fixture.modelsPath, { force: true });
await fs.mkdir(fixture.modelsPath, { recursive: true });
const report = await runSecretsAudit({ env: fixture.env });
expect(
hasFinding(
report,
(entry) => entry.code === "REF_UNRESOLVED" && entry.file === fixture.modelsPath,
),
).toBe(true);
});
it("reports oversized models.json as unresolved findings", async () => {
const oversizedApiKey = "a".repeat(MAX_AUDIT_MODELS_JSON_BYTES + 256);
await writeJsonFile(fixture.modelsPath, {
providers: {
openai: {
baseUrl: "https://api.openai.com/v1",
api: "openai-completions",
apiKey: oversizedApiKey,
models: [{ id: "gpt-5", name: "gpt-5" }],
},
},
});
const report = await runSecretsAudit({ env: fixture.env });
expect(
hasFinding(
report,
(entry) => entry.code === "REF_UNRESOLVED" && entry.file === fixture.modelsPath,
),
).toBe(true);
});
it("scans active agent-dir override models.json even when outside state dir", async () => {
const externalAgentDir = path.join(fixture.rootDir, "external-agent");
const externalModelsPath = path.join(externalAgentDir, "models.json");
await fs.mkdir(externalAgentDir, { recursive: true });
await writeJsonFile(externalModelsPath, {
providers: {
openai: {
baseUrl: "https://api.openai.com/v1",
api: "openai-completions",
apiKey: "sk-external-plaintext", // pragma: allowlist secret
models: [{ id: "gpt-5", name: "gpt-5" }],
},
},
});
const report = await runSecretsAudit({
env: {
...fixture.env,
OPENCLAW_AGENT_DIR: externalAgentDir,
},
});
expect(
hasFinding(
report,
(entry) =>
entry.code === "PLAINTEXT_FOUND" &&
entry.file === externalModelsPath &&
entry.jsonPath === "providers.openai.apiKey",
),
).toBe(true);
expect(report.filesScanned).toContain(externalModelsPath);
});
it("does not flag non-sensitive routing headers in openclaw config", async () => {
await writeJsonFile(fixture.configPath, {
models: {

View File

@@ -97,6 +97,7 @@ type AuditCollector = {
};
const REF_RESOLVE_FALLBACK_CONCURRENCY = 8;
const MAX_AUDIT_MODELS_JSON_BYTES = 5 * 1024 * 1024;
const ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES = new Set([
"authorization",
"proxy-authorization",
@@ -369,7 +370,10 @@ function collectModelsJsonSecrets(params: {
return;
}
params.collector.filesScanned.add(params.modelsJsonPath);
const parsedResult = readJsonObjectIfExists(params.modelsJsonPath);
const parsedResult = readJsonObjectIfExists(params.modelsJsonPath, {
requireRegularFile: true,
maxBytes: MAX_AUDIT_MODELS_JSON_BYTES,
});
if (parsedResult.error) {
addFinding(params.collector, {
code: "REF_UNRESOLVED",
@@ -630,7 +634,7 @@ export async function runSecretsAudit(
defaults,
});
}
for (const modelsJsonPath of listAgentModelsJsonPaths(config, stateDir)) {
for (const modelsJsonPath of listAgentModelsJsonPaths(config, stateDir, env)) {
collectModelsJsonSecrets({
modelsJsonPath,
collector,

View File

@@ -32,11 +32,25 @@ export function listLegacyAuthJsonPaths(stateDir: string): string[] {
return out;
}
export function listAgentModelsJsonPaths(config: OpenClawConfig, stateDir: string): string[] {
const paths = new Set<string>();
paths.add(path.join(resolveUserPath(stateDir), "agents", "main", "agent", "models.json"));
function resolveActiveAgentDir(stateDir: string, env: NodeJS.ProcessEnv = process.env): string {
const override = env.OPENCLAW_AGENT_DIR?.trim() || env.PI_CODING_AGENT_DIR?.trim();
if (override) {
return resolveUserPath(override);
}
return path.join(resolveUserPath(stateDir), "agents", "main", "agent");
}
const agentsRoot = path.join(resolveUserPath(stateDir), "agents");
export function listAgentModelsJsonPaths(
config: OpenClawConfig,
stateDir: string,
env: NodeJS.ProcessEnv = process.env,
): string[] {
const resolvedStateDir = resolveUserPath(stateDir);
const paths = new Set<string>();
paths.add(path.join(resolvedStateDir, "agents", "main", "agent", "models.json"));
paths.add(path.join(resolveActiveAgentDir(stateDir, env), "models.json"));
const agentsRoot = path.join(resolvedStateDir, "agents");
if (fs.existsSync(agentsRoot)) {
for (const entry of fs.readdirSync(agentsRoot, { withFileTypes: true })) {
if (!entry.isDirectory()) {
@@ -48,7 +62,7 @@ export function listAgentModelsJsonPaths(config: OpenClawConfig, stateDir: strin
for (const agentId of listAgentIds(config)) {
if (agentId === "main") {
paths.add(path.join(resolveUserPath(stateDir), "agents", "main", "agent", "models.json"));
paths.add(path.join(resolvedStateDir, "agents", "main", "agent", "models.json"));
continue;
}
const agentDir = resolveAgentDir(config, agentId);
@@ -58,14 +72,51 @@ export function listAgentModelsJsonPaths(config: OpenClawConfig, stateDir: strin
return [...paths];
}
export type ReadJsonObjectOptions = {
maxBytes?: number;
requireRegularFile?: boolean;
};
export function readJsonObjectIfExists(filePath: string): {
value: Record<string, unknown> | null;
error?: string;
};
export function readJsonObjectIfExists(
filePath: string,
options: ReadJsonObjectOptions,
): {
value: Record<string, unknown> | null;
error?: string;
};
export function readJsonObjectIfExists(
filePath: string,
options: ReadJsonObjectOptions = {},
): {
value: Record<string, unknown> | null;
error?: string;
} {
if (!fs.existsSync(filePath)) {
return { value: null };
}
try {
const stats = fs.statSync(filePath);
if (options.requireRegularFile && !stats.isFile()) {
return {
value: null,
error: `Refusing to read non-regular file: ${filePath}`,
};
}
if (
typeof options.maxBytes === "number" &&
Number.isFinite(options.maxBytes) &&
options.maxBytes >= 0 &&
stats.size > options.maxBytes
) {
return {
value: null,
error: `Refusing to read oversized JSON (${stats.size} bytes): ${filePath}`,
};
}
const raw = fs.readFileSync(filePath, "utf8");
const parsed = JSON.parse(raw) as unknown;
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {