fix: redact config values in skills status

This commit is contained in:
Peter Steinberger
2026-02-14 17:32:19 +01:00
parent 188c4cd076
commit d3428053d9
7 changed files with 154 additions and 514 deletions

View File

@@ -20,7 +20,6 @@ import { resolveBundledSkillsContext } from "./skills/bundled-context.js";
export type SkillStatusConfigCheck = {
path: string;
value: unknown;
satisfied: boolean;
};
@@ -216,7 +215,6 @@ function buildSkillStatus(
skillConfig?.env?.[envName] ||
(skillConfig?.apiKey && entry.metadata?.primaryEnv === envName),
),
resolveConfigValue: (pathStr) => resolveConfigPath(config, pathStr),
isConfigSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr),
});
const eligible = !disabled && !blockedByAllowlist && requirementsSatisfied;

View File

@@ -0,0 +1,72 @@
import path from "node:path";
import { describe, expect, it } from "vitest";
import {
connectOk,
installGatewayTestHooks,
rpcReq,
startServerWithClient,
} from "./test-helpers.js";
installGatewayTestHooks({ scope: "suite" });
async function withServer<T>(
run: (ws: Awaited<ReturnType<typeof startServerWithClient>>["ws"]) => Promise<T>,
) {
const { server, ws, prevToken } = await startServerWithClient("secret");
try {
return await run(ws);
} finally {
ws.close();
await server.close();
if (prevToken === undefined) {
delete process.env.OPENCLAW_GATEWAY_TOKEN;
} else {
process.env.OPENCLAW_GATEWAY_TOKEN = prevToken;
}
}
}
describe("gateway skills.status", () => {
it("does not expose raw config values to operator.read clients", async () => {
const prevBundledSkillsDir = process.env.OPENCLAW_BUNDLED_SKILLS_DIR;
process.env.OPENCLAW_BUNDLED_SKILLS_DIR = path.join(process.cwd(), "skills");
const secret = "discord-token-secret-abc";
const { writeConfigFile } = await import("../config/config.js");
await writeConfigFile({
session: { mainKey: "main-test" },
channels: {
discord: {
token: secret,
},
},
});
try {
await withServer(async (ws) => {
await connectOk(ws, { token: "secret", scopes: ["operator.read"] });
const res = await rpcReq<{
skills?: Array<{
name?: string;
configChecks?: Array<{ path?: string; satisfied?: boolean } & Record<string, unknown>>;
}>;
}>(ws, "skills.status", {});
expect(res.ok).toBe(true);
expect(JSON.stringify(res.payload)).not.toContain(secret);
const discord = res.payload?.skills?.find((s) => s.name === "discord");
expect(discord).toBeTruthy();
const check = discord?.configChecks?.find((c) => c.path === "channels.discord.token");
expect(check).toBeTruthy();
expect(check?.satisfied).toBe(true);
expect(check && "value" in check).toBe(false);
});
} finally {
if (prevBundledSkillsDir === undefined) {
delete process.env.OPENCLAW_BUNDLED_SKILLS_DIR;
} else {
process.env.OPENCLAW_BUNDLED_SKILLS_DIR = prevBundledSkillsDir;
}
}
});
});

View File

@@ -8,7 +8,6 @@ import { loadWorkspaceHookEntries } from "./workspace.js";
export type HookStatusConfigCheck = {
path: string;
value: unknown;
satisfied: boolean;
};
@@ -124,7 +123,6 @@ function buildHookStatus(
localPlatform: process.platform,
remotePlatforms: eligibility?.remote?.platforms,
isEnvSatisfied: (envName) => Boolean(process.env[envName] || hookConfig?.env?.[envName]),
resolveConfigValue: (pathStr) => resolveConfigPath(config, pathStr),
isConfigSatisfied: (pathStr) => isConfigPathTruthy(config, pathStr),
});

View File

@@ -52,14 +52,13 @@ describe("requirements helpers", () => {
).toEqual(["A"]);
});
it("buildConfigChecks includes value+status", () => {
it("buildConfigChecks includes status", () => {
expect(
buildConfigChecks({
required: ["a.b"],
resolveValue: (p) => (p === "a.b" ? 1 : null),
isSatisfied: (p) => p === "a.b",
}),
).toEqual([{ path: "a.b", value: 1, satisfied: true }]);
).toEqual([{ path: "a.b", satisfied: true }]);
});
it("evaluateRequirementsFromMetadata derives required+missing", () => {
@@ -72,7 +71,6 @@ describe("requirements helpers", () => {
hasLocalBin: (bin) => bin === "a",
localPlatform: "linux",
isEnvSatisfied: (name) => name === "E",
resolveConfigValue: () => "x",
isConfigSatisfied: () => false,
});

View File

@@ -8,7 +8,6 @@ export type Requirements = {
export type RequirementConfigCheck = {
path: string;
value: unknown;
satisfied: boolean;
};
@@ -84,13 +83,11 @@ export function resolveMissingEnv(params: {
export function buildConfigChecks(params: {
required: string[];
resolveValue: (pathStr: string) => unknown;
isSatisfied: (pathStr: string) => boolean;
}): RequirementConfigCheck[] {
return params.required.map((pathStr) => {
const value = params.resolveValue(pathStr);
const satisfied = params.isSatisfied(pathStr);
return { path: pathStr, value, satisfied };
return { path: pathStr, satisfied };
});
}
@@ -103,7 +100,6 @@ export function evaluateRequirements(params: {
localPlatform: string;
remotePlatforms?: string[];
isEnvSatisfied: (envName: string) => boolean;
resolveConfigValue: (pathStr: string) => unknown;
isConfigSatisfied: (pathStr: string) => boolean;
}): { missing: Requirements; eligible: boolean; configChecks: RequirementConfigCheck[] } {
const missingBins = resolveMissingBins({
@@ -127,7 +123,6 @@ export function evaluateRequirements(params: {
});
const configChecks = buildConfigChecks({
required: params.required.config,
resolveValue: params.resolveConfigValue,
isSatisfied: params.isConfigSatisfied,
});
const missingConfig = configChecks.filter((check) => !check.satisfied).map((check) => check.path);
@@ -162,7 +157,6 @@ export function evaluateRequirementsFromMetadata(params: {
localPlatform: string;
remotePlatforms?: string[];
isEnvSatisfied: (envName: string) => boolean;
resolveConfigValue: (pathStr: string) => unknown;
isConfigSatisfied: (pathStr: string) => boolean;
}): {
required: Requirements;
@@ -187,7 +181,6 @@ export function evaluateRequirementsFromMetadata(params: {
localPlatform: params.localPlatform,
remotePlatforms: params.remotePlatforms,
isEnvSatisfied: params.isEnvSatisfied,
resolveConfigValue: params.resolveConfigValue,
isConfigSatisfied: params.isConfigSatisfied,
});
return { required, ...result };