Secrets: avoid header marker collisions and audit false positives
This commit is contained in:
parent
5a3284ef9a
commit
f4c116ac0b
@ -180,7 +180,7 @@ describe("buildInlineProviderModels", () => {
|
||||
expect(result[0].headers).toBeUndefined();
|
||||
});
|
||||
|
||||
it("drops SecretRef marker headers when building inline provider models", () => {
|
||||
it("preserves literal marker-shaped headers in inline provider models", () => {
|
||||
const providers: Parameters<typeof buildInlineProviderModels>[0] = {
|
||||
custom: {
|
||||
headers: {
|
||||
@ -195,7 +195,11 @@ describe("buildInlineProviderModels", () => {
|
||||
const result = buildInlineProviderModels(providers);
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].headers).toEqual({ "X-Static": "tenant-a" });
|
||||
expect(result[0].headers).toEqual({
|
||||
Authorization: "secretref-env:OPENAI_HEADER_TOKEN",
|
||||
"X-Managed": "secretref-managed",
|
||||
"X-Static": "tenant-a",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -241,7 +245,7 @@ describe("resolveModel", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("drops marker-backed provider headers in fallback models", () => {
|
||||
it("preserves literal marker-shaped provider headers in fallback models", () => {
|
||||
const cfg = {
|
||||
models: {
|
||||
providers: {
|
||||
@ -262,10 +266,35 @@ describe("resolveModel", () => {
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect((result.model as unknown as { headers?: Record<string, string> }).headers).toEqual({
|
||||
Authorization: "secretref-env:OPENAI_HEADER_TOKEN",
|
||||
"X-Managed": "secretref-managed",
|
||||
"X-Custom-Auth": "token-123",
|
||||
});
|
||||
});
|
||||
|
||||
it("drops marker headers from discovered models.json entries", () => {
|
||||
mockDiscoveredModel({
|
||||
provider: "custom",
|
||||
modelId: "listed-model",
|
||||
templateModel: {
|
||||
...makeModel("listed-model"),
|
||||
provider: "custom",
|
||||
headers: {
|
||||
Authorization: "secretref-env:OPENAI_HEADER_TOKEN",
|
||||
"X-Managed": "secretref-managed",
|
||||
"X-Static": "tenant-a",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = resolveModel("custom", "listed-model", "/tmp/agent");
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect((result.model as unknown as { headers?: Record<string, string> }).headers).toEqual({
|
||||
"X-Static": "tenant-a",
|
||||
});
|
||||
});
|
||||
|
||||
it("prefers matching configured model metadata for fallback token limits", () => {
|
||||
const cfg = {
|
||||
models: {
|
||||
|
||||
@ -23,13 +23,19 @@ type InlineProviderConfig = {
|
||||
headers?: unknown;
|
||||
};
|
||||
|
||||
function sanitizeModelHeaders(headers: unknown): Record<string, string> | undefined {
|
||||
function sanitizeModelHeaders(
|
||||
headers: unknown,
|
||||
opts?: { stripSecretRefMarkers?: boolean },
|
||||
): Record<string, string> | undefined {
|
||||
if (!headers || typeof headers !== "object" || Array.isArray(headers)) {
|
||||
return undefined;
|
||||
}
|
||||
const next: Record<string, string> = {};
|
||||
for (const [headerName, headerValue] of Object.entries(headers)) {
|
||||
if (typeof headerValue !== "string" || isSecretRefHeaderValueMarker(headerValue)) {
|
||||
if (typeof headerValue !== "string") {
|
||||
continue;
|
||||
}
|
||||
if (opts?.stripSecretRefMarkers && isSecretRefHeaderValueMarker(headerValue)) {
|
||||
continue;
|
||||
}
|
||||
next[headerName] = headerValue;
|
||||
@ -63,11 +69,14 @@ function applyConfiguredProviderOverrides(params: {
|
||||
if (!providerConfig) {
|
||||
return {
|
||||
...discoveredModel,
|
||||
headers: sanitizeModelHeaders(discoveredModel.headers),
|
||||
// Discovered models originate from models.json and may contain persistence markers.
|
||||
headers: sanitizeModelHeaders(discoveredModel.headers, { stripSecretRefMarkers: true }),
|
||||
};
|
||||
}
|
||||
const configuredModel = providerConfig.models?.find((candidate) => candidate.id === modelId);
|
||||
const discoveredHeaders = sanitizeModelHeaders(discoveredModel.headers);
|
||||
const discoveredHeaders = sanitizeModelHeaders(discoveredModel.headers, {
|
||||
stripSecretRefMarkers: true,
|
||||
});
|
||||
const providerHeaders = sanitizeModelHeaders(providerConfig.headers);
|
||||
const configuredHeaders = sanitizeModelHeaders(configuredModel?.headers);
|
||||
if (!configuredModel && !providerConfig.baseUrl && !providerConfig.api && !providerHeaders) {
|
||||
|
||||
@ -88,12 +88,12 @@ describe("runCapability deepgram provider options", () => {
|
||||
expect(seenBaseUrl).toBe("https://entry.example");
|
||||
expect(seenHeaders).toMatchObject({
|
||||
"X-Provider": "1",
|
||||
"X-Provider-Managed": "secretref-managed",
|
||||
"X-Config": "2",
|
||||
"X-Config-Managed": "secretref-env:DEEPGRAM_HEADER_TOKEN",
|
||||
"X-Entry": "3",
|
||||
"X-Entry-Managed": "secretref-managed",
|
||||
});
|
||||
expect((seenHeaders as Record<string, string>)["X-Provider-Managed"]).toBeUndefined();
|
||||
expect((seenHeaders as Record<string, string>)["X-Config-Managed"]).toBeUndefined();
|
||||
expect((seenHeaders as Record<string, string>)["X-Entry-Managed"]).toBeUndefined();
|
||||
expect(seenQuery).toMatchObject({
|
||||
detect_language: false,
|
||||
punctuate: false,
|
||||
|
||||
@ -4,7 +4,6 @@ import {
|
||||
collectProviderApiKeysForExecution,
|
||||
executeWithApiKeyRotation,
|
||||
} from "../agents/api-key-rotation.js";
|
||||
import { isSecretRefHeaderValueMarker } from "../agents/model-auth-markers.js";
|
||||
import { requireApiKey, resolveApiKeyForProvider } from "../agents/model-auth.js";
|
||||
import type { MsgContext } from "../auto-reply/templating.js";
|
||||
import { applyTemplate } from "../auto-reply/templating.js";
|
||||
@ -49,7 +48,7 @@ function sanitizeProviderHeaders(
|
||||
}
|
||||
const next: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
if (typeof value !== "string" || isSecretRefHeaderValueMarker(value)) {
|
||||
if (typeof value !== "string") {
|
||||
continue;
|
||||
}
|
||||
next[key] = value;
|
||||
|
||||
@ -322,6 +322,33 @@ describe("secrets audit", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does not flag non-sensitive routing headers in models.json", async () => {
|
||||
await writeJsonFile(fixture.modelsPath, {
|
||||
providers: {
|
||||
openai: {
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
api: "openai-completions",
|
||||
apiKey: "OPENAI_API_KEY",
|
||||
headers: {
|
||||
"X-Proxy-Region": "us-west",
|
||||
},
|
||||
models: [{ id: "gpt-5", name: "gpt-5" }],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const report = await runSecretsAudit({ env: fixture.env });
|
||||
expect(
|
||||
hasFinding(
|
||||
report,
|
||||
(entry) =>
|
||||
entry.code === "PLAINTEXT_FOUND" &&
|
||||
entry.file === fixture.modelsPath &&
|
||||
entry.jsonPath === "providers.openai.headers.X-Proxy-Region",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not flag models.json marker values as plaintext", async () => {
|
||||
await writeJsonFile(fixture.modelsPath, {
|
||||
providers: {
|
||||
@ -420,4 +447,38 @@ describe("secrets audit", () => {
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does not flag non-sensitive routing headers in openclaw config", async () => {
|
||||
await writeJsonFile(fixture.configPath, {
|
||||
models: {
|
||||
providers: {
|
||||
openai: {
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
api: "openai-completions",
|
||||
apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" },
|
||||
headers: {
|
||||
"X-Proxy-Region": "us-west",
|
||||
},
|
||||
models: [{ id: "gpt-5", name: "gpt-5" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
await writeJsonFile(fixture.authStorePath, {
|
||||
version: 1,
|
||||
profiles: {},
|
||||
});
|
||||
await fs.writeFile(fixture.envPath, "", "utf8");
|
||||
|
||||
const report = await runSecretsAudit({ env: fixture.env });
|
||||
expect(
|
||||
hasFinding(
|
||||
report,
|
||||
(entry) =>
|
||||
entry.code === "PLAINTEXT_FOUND" &&
|
||||
entry.file === fixture.configPath &&
|
||||
entry.jsonPath === "models.providers.openai.headers.X-Proxy-Region",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@ -97,6 +97,40 @@ type AuditCollector = {
|
||||
};
|
||||
|
||||
const REF_RESOLVE_FALLBACK_CONCURRENCY = 8;
|
||||
const ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES = new Set([
|
||||
"authorization",
|
||||
"proxy-authorization",
|
||||
"x-api-key",
|
||||
"api-key",
|
||||
"apikey",
|
||||
"x-auth-token",
|
||||
"auth-token",
|
||||
"x-access-token",
|
||||
"access-token",
|
||||
"x-secret-key",
|
||||
"secret-key",
|
||||
]);
|
||||
const SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS = [
|
||||
"api-key",
|
||||
"apikey",
|
||||
"token",
|
||||
"secret",
|
||||
"password",
|
||||
"credential",
|
||||
];
|
||||
|
||||
function isLikelySensitiveModelProviderHeaderName(value: string): boolean {
|
||||
const normalized = value.trim().toLowerCase();
|
||||
if (!normalized) {
|
||||
return false;
|
||||
}
|
||||
if (ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES.has(normalized)) {
|
||||
return true;
|
||||
}
|
||||
return SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS.some((fragment) =>
|
||||
normalized.includes(fragment),
|
||||
);
|
||||
}
|
||||
|
||||
function addFinding(collector: AuditCollector, finding: SecretsAuditFinding): void {
|
||||
collector.findings.push(finding);
|
||||
@ -198,6 +232,12 @@ function collectConfigSecrets(params: {
|
||||
target.value,
|
||||
target.entry.expectedResolvedValue,
|
||||
);
|
||||
if (
|
||||
target.entry.id === "models.providers.*.headers.*" &&
|
||||
!isLikelySensitiveModelProviderHeaderName(target.pathSegments.at(-1) ?? "")
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
if (!hasPlaintext) {
|
||||
continue;
|
||||
}
|
||||
@ -393,6 +433,9 @@ function collectModelsJsonSecrets(params: {
|
||||
if (isSecretRefHeaderValueMarker(headerValue)) {
|
||||
continue;
|
||||
}
|
||||
if (!isLikelySensitiveModelProviderHeaderName(headerKey)) {
|
||||
continue;
|
||||
}
|
||||
addFinding(params.collector, {
|
||||
code: "PLAINTEXT_FOUND",
|
||||
severity: "warn",
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user