From e26fc17c77ed737bc2a672b2343b891980253bb6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Jul 2026 03:19:07 -0700 Subject: [PATCH] fix(security): report canonical agents.entries paths in audit and diagnostics (#113603) Audit findings, dangerous-flag labels, exec-approval provenance, filesystem policy drift, and the skill-workshop tool diagnostic printed agents.list.* paths users cannot paste into openclaw.json: dotted agents.list. is valid in no shape, and index-keyed agents.list. only names the internal validation projection. All display surfaces now emit agents.entries.; the summary agent-id extractor regex follows the new labels. Roster-aware builders keep an indexed fallback only for id-less malformed legacy rows, and validation/doctor/legacy-migration paths intentionally keep the legacy form because they point into the user's actual file. Follow-up deferred from #113160. --- src/infra/exec-approvals-effective.ts | 2 +- src/infra/exec-approvals-policy.test.ts | 15 +++--- src/security/audit-config-basics.test.ts | 16 +++--- src/security/audit-exec-safe-bins.test.ts | 29 +++++------ src/security/audit-exec-surface.test.ts | 21 +++++++- src/security/audit-extra.summary.ts | 2 +- src/security/audit-extra.sync.ts | 8 +-- src/security/audit-model-refs.ts | 6 +-- src/security/audit-plugins-trust.test.ts | 13 +++++ src/security/audit-plugins-trust.ts | 2 +- .../audit-sandbox-docker-config.test.ts | 30 +++++++++++- src/security/audit-small-model-risk.test.ts | 49 +++++++++++++++++++ src/security/audit-trust-model.test.ts | 4 +- src/security/audit.ts | 10 ++-- src/security/dangerous-config-flags-core.ts | 18 ++++--- src/security/dangerous-config-flags.test.ts | 21 ++++---- src/security/exec-filesystem-policy.ts | 2 +- .../workshop/tool-policy-diagnostic.test.ts | 34 ++++++------- src/skills/workshop/tool-policy-diagnostic.ts | 7 ++- 19 files changed, 197 insertions(+), 92 deletions(-) diff --git a/src/infra/exec-approvals-effective.ts b/src/infra/exec-approvals-effective.ts index 793e23248f7f..fbabc741695d 100644 --- a/src/infra/exec-approvals-effective.ts +++ b/src/infra/exec-approvals-effective.ts @@ -333,7 +333,7 @@ export function collectExecPolicyScopeSnapshots(params: { approvals: params.approvals, scopeExecConfig: agentConfig?.tools?.exec, globalExecConfig, - configPath: `agents.list.${agentId}.tools.exec`, + configPath: `agents.entries.${agentId}.tools.exec`, hostPath: params.hostPath, hostDefaults: params.hostDefaults, hostDefaultSource: params.hostDefaultSource, diff --git a/src/infra/exec-approvals-policy.test.ts b/src/infra/exec-approvals-policy.test.ts index 0cdcdedc4678..d01610a31e2b 100644 --- a/src/infra/exec-approvals-policy.test.ts +++ b/src/infra/exec-approvals-policy.test.ts @@ -829,7 +829,7 @@ describe("exec approvals policy helpers", () => { }, }, agents: { - list: [{ id: "runner", default: true }], + entries: { runner: { default: true } }, }, } satisfies OpenClawConfig, approvals: { @@ -869,7 +869,7 @@ describe("exec approvals policy helpers", () => { ask: "off", }, }, - agents: { list: [{ id: DEFAULT_AGENT_ID, default: true }] }, + agents: { entries: { [DEFAULT_AGENT_ID]: { default: true } } }, } satisfies OpenClawConfig, approvals: { version: 1, @@ -903,9 +903,8 @@ describe("exec approvals policy helpers", () => { }, }, agents: { - list: [ - { - id: DEFAULT_AGENT_ID, + entries: { + [DEFAULT_AGENT_ID]: { default: true, tools: { exec: { @@ -913,7 +912,7 @@ describe("exec approvals policy helpers", () => { }, }, }, - ], + }, }, } satisfies OpenClawConfig, approvals: { @@ -924,7 +923,7 @@ describe("exec approvals policy helpers", () => { expect(snapshots.map((snapshot) => snapshot.scopeLabel)).toEqual(["tools.exec", "agent:main"]); expectFields(snapshots[1]?.ask, { requested: "always", - requestedSource: "agents.list.main.tools.exec.ask", + requestedSource: "agents.entries.main.tools.exec.ask", }); }); @@ -947,7 +946,7 @@ describe("exec approvals policy helpers", () => { ]); expectFields(snapshots[1]?.ask, { requested: "always", - requestedSource: "agents.list.runner.tools.exec.ask", + requestedSource: "agents.entries.runner.tools.exec.ask", }); }); }); diff --git a/src/security/audit-config-basics.test.ts b/src/security/audit-config-basics.test.ts index 756930bee889..5a0a61f9bb5b 100644 --- a/src/security/audit-config-basics.test.ts +++ b/src/security/audit-config-basics.test.ts @@ -49,21 +49,17 @@ describe("security audit config basics", () => { profile: "minimal", }, agents: { - list: [ - { - id: "owner", + entries: { + owner: { tools: { profile: "full" }, }, - ], + }, }, }); - expect( - findings.some( - (finding) => - finding.checkId === "tools.profile_minimal_overridden" && finding.severity === "warn", - ), - ).toBe(true); + const finding = findings.find((entry) => entry.checkId === "tools.profile_minimal_overridden"); + expect(finding?.severity).toBe("warn"); + expect(finding?.detail).toContain("agents.entries.owner=full"); }); it("flags tools.elevated allowFrom wildcard as critical", async () => { diff --git a/src/security/audit-exec-safe-bins.test.ts b/src/security/audit-exec-safe-bins.test.ts index d540e160e145..94086fd119f6 100644 --- a/src/security/audit-exec-safe-bins.test.ts +++ b/src/security/audit-exec-safe-bins.test.ts @@ -36,9 +36,8 @@ describe("security audit exec safe-bin findings", () => { }, }, agents: { - list: [ - { - id: "ops", + entries: { + ops: { default: true, tools: { exec: { @@ -46,7 +45,7 @@ describe("security audit exec safe-bin findings", () => { }, }, }, - ], + }, }, } satisfies OpenClawConfig, expected: true, @@ -65,9 +64,8 @@ describe("security audit exec safe-bin findings", () => { }, }, agents: { - list: [ - { - id: "ops", + entries: { + ops: { default: true, tools: { exec: { @@ -80,7 +78,7 @@ describe("security audit exec safe-bin findings", () => { }, }, }, - ], + }, }, } satisfies OpenClawConfig, expected: false, @@ -101,7 +99,7 @@ describe("security audit exec safe-bin findings", () => { { name: "jq configured globally", cfg: { - agents: { list: [{ id: "main", default: true }] }, + agents: { entries: { main: { default: true } } }, tools: { exec: { safeBins: ["jq"], @@ -113,7 +111,7 @@ describe("security audit exec safe-bin findings", () => { { name: "jq not configured", cfg: { - agents: { list: [{ id: "main", default: true }] }, + agents: { entries: { main: { default: true } } }, tools: { exec: { safeBins: ["cut"], @@ -143,9 +141,8 @@ describe("security audit exec safe-bin findings", () => { }, }, agents: { - list: [ - { - id: "ops", + entries: { + ops: { default: true, tools: { exec: { @@ -153,7 +150,7 @@ describe("security audit exec safe-bin findings", () => { }, }, }, - ], + }, }, } satisfies OpenClawConfig); @@ -161,7 +158,7 @@ describe("security audit exec safe-bin findings", () => { expect(riskyFinding.severity).toBe("warn"); expect(riskyFinding.detail).toContain(riskyGlobalTrustedDirs[0]); expect(riskyFinding.detail).toContain(riskyGlobalTrustedDirs[1]); - expect(riskyFinding.detail).toContain("agents.list.ops.tools.exec"); + expect(riskyFinding.detail).toContain("agents.entries.ops.tools.exec"); }); it("ignores non-risky absolute dirs", async () => { @@ -169,7 +166,7 @@ describe("security audit exec safe-bin findings", () => { hasFinding( "tools.exec.safe_bin_trusted_dirs_risky", await collectSecurityAuditFindings({ - agents: { list: [{ id: "main", default: true }] }, + agents: { entries: { main: { default: true } } }, tools: { exec: { safeBinTrustedDirs: ["/usr/libexec"], diff --git a/src/security/audit-exec-surface.test.ts b/src/security/audit-exec-surface.test.ts index f020508ff389..6caa11290903 100644 --- a/src/security/audit-exec-surface.test.ts +++ b/src/security/audit-exec-surface.test.ts @@ -108,7 +108,7 @@ describe("security audit exec surface findings", () => { "warn", await collectSecurityAuditFindings({ agents: { - list: [{ id: "ops" }], + entries: { ops: {} }, }, } satisfies OpenClawConfig), ), @@ -253,6 +253,25 @@ describe("security audit exec surface findings", () => { expect(finding.remediation).toContain("deny exec and process"); }); + it("reports canonical agent paths for filesystem policy drift", async () => { + const findings = await collectSecurityAuditFindings({ + agents: { + entries: { + ops: { + default: true, + tools: { + allow: ["read", "exec", "process"], + deny: ["write", "edit", "apply_patch"], + }, + }, + }, + }, + } satisfies OpenClawConfig); + + const finding = requireFinding("tools.exec.fs_tools_disabled_but_exec_enabled", findings); + expect(finding.detail).toContain("agents.entries.ops.tools"); + }); + it("does not warn when sandbox filesystem policy constrains exec", async () => { const findings = await collectSecurityAuditFindings({ agents: { diff --git a/src/security/audit-extra.summary.ts b/src/security/audit-extra.summary.ts index 63440a96b4dc..01d917fb3528 100644 --- a/src/security/audit-extra.summary.ts +++ b/src/security/audit-extra.summary.ts @@ -59,7 +59,7 @@ function summarizeGroupPolicy(cfg: OpenClawConfig): { } function extractAgentIdFromSource(source: string): string | null { - const match = source.match(/^agents\.list\.([^.]*)\./); + const match = source.match(/^agents\.entries\.([^.]*)\./); return match?.[1] ?? null; } diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index ec22fcfaea61..2cb31c95b473 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -487,7 +487,7 @@ function listAuditAgentToolContexts(cfg: OpenClawConfig): AuditAgentToolContext[ continue; } contexts.push({ - label: `agents.list.${agent.id}`, + label: `agents.entries.${agent.id}`, agentId: agent.id, tools: agent.tools, }); @@ -824,7 +824,7 @@ export function collectSandboxDockerNoopFindings(cfg: OpenClawConfig): SecurityA continue; } if (resolveSandboxConfigForAgent(cfg, entry.id).mode === "off") { - configuredPaths.push(`agents.list.${entry.id}.sandbox.docker`); + configuredPaths.push(`agents.entries.${entry.id}.sandbox.docker`); } } @@ -865,7 +865,7 @@ export function collectSandboxDangerousConfigFindings(cfg: OpenClawConfig): Secu const agentDocker = entry.sandbox?.docker; if (agentDocker && typeof agentDocker === "object") { configs.push({ - source: `agents.list.${entry.id}.sandbox.docker`, + source: `agents.entries.${entry.id}.sandbox.docker`, docker: agentDocker as Record, }); } @@ -1083,7 +1083,7 @@ export function collectMinimalProfileOverrideFindings(cfg: OpenClawConfig): Secu title: "Global tools.profile=minimal is overridden by agent profiles", detail: "Global minimal profile is set, but these agent profiles take precedence:\n" + - overrides.map((entry) => `- agents.list.${entry}`).join("\n"), + overrides.map((entry) => `- agents.entries.${entry}`).join("\n"), remediation: 'Set those agents to `tools.profile="minimal"` (or remove the agent override) if you want minimal tools enforced globally.', }); diff --git a/src/security/audit-model-refs.ts b/src/security/audit-model-refs.ts index 626d2bd3c49c..2a39d12e0985 100644 --- a/src/security/audit-model-refs.ts +++ b/src/security/audit-model-refs.ts @@ -88,13 +88,13 @@ export function collectAuditModelRefs(cfg: OpenClawConfig): AuditModelRef[] { typeof (agent as { id?: unknown }).id === "string" ? (agent as { id: string }).id : ""; const model = (agent as { model?: unknown }).model; if (typeof model === "string") { - add(model, `agents.list.${id}.model`); + add(model, `agents.entries.${id}.model`); } else if (model && typeof model === "object") { - add((model as { primary?: unknown }).primary, `agents.list.${id}.model.primary`); + add((model as { primary?: unknown }).primary, `agents.entries.${id}.model.primary`); const fallbacks = (model as { fallbacks?: unknown }).fallbacks; if (Array.isArray(fallbacks)) { for (const fallback of fallbacks) { - add(fallback, `agents.list.${id}.model.fallbacks`); + add(fallback, `agents.entries.${id}.model.fallbacks`); } } } diff --git a/src/security/audit-plugins-trust.test.ts b/src/security/audit-plugins-trust.test.ts index bb634aa39574..66ff4d60089f 100644 --- a/src/security/audit-plugins-trust.test.ts +++ b/src/security/audit-plugins-trust.test.ts @@ -544,6 +544,19 @@ describe("security audit extension tool reachability findings", () => { ).toBe(true); }, }, + { + name: "reports canonical agent paths for permissive tool policy", + cfg: { + plugins: { allow: ["some-plugin"] }, + agents: { entries: { ops: { tools: { profile: "full" } } } }, + } satisfies OpenClawConfig, + assert: (findings: Awaited>) => { + const finding = findings.find( + (entry) => entry.checkId === "plugins.tools_reachable_permissive_policy", + ); + expect(finding?.detail).toContain("- agents.entries.ops"); + }, + }, { name: "does not flag plugin tool reachability when profile is restrictive", cfg: { diff --git a/src/security/audit-plugins-trust.ts b/src/security/audit-plugins-trust.ts index c5924c1fe397..df773e6a7311 100644 --- a/src/security/audit-plugins-trust.ts +++ b/src/security/audit-plugins-trust.ts @@ -363,7 +363,7 @@ export async function collectPluginsTrustFindings(params: { continue; } contexts.push({ - label: `agents.list.${entry.id}`, + label: `agents.entries.${entry.id}`, agentId: entry.id, tools: entry.tools, }); diff --git a/src/security/audit-sandbox-docker-config.test.ts b/src/security/audit-sandbox-docker-config.test.ts index 9670b71be5fd..1b2cf9143a3c 100644 --- a/src/security/audit-sandbox-docker-config.test.ts +++ b/src/security/audit-sandbox-docker-config.test.ts @@ -72,7 +72,7 @@ describe("security audit sandbox docker config", () => { docker: { image: "ghcr.io/example/sandbox:latest" }, }, }, - list: [{ id: "ops", sandbox: { mode: "all" } }], + entries: { ops: { sandbox: { mode: "all" } } }, }, } as OpenClawConfig, expectedFindings: [], @@ -190,4 +190,32 @@ describe("security audit sandbox docker config", () => { ); }); }); + + it("reports canonical agent paths for docker sandbox findings", () => { + const config = { + agents: { + entries: { + ops: { + sandbox: { + mode: "off", + docker: { + image: "ghcr.io/example/sandbox:latest", + binds: ["/etc/passwd:/mnt/passwd:ro"], + }, + }, + }, + }, + }, + } satisfies OpenClawConfig; + + const noOpFinding = collectSandboxDockerNoopFindings(config).find( + (entry) => entry.checkId === "sandbox.docker_config_mode_off", + ); + expect(noOpFinding?.detail).toContain("agents.entries.ops.sandbox.docker"); + + const dangerousFinding = collectSandboxDangerousConfigFindings(config).find( + (entry) => entry.checkId === "sandbox.dangerous_bind_mount", + ); + expect(dangerousFinding?.detail).toContain("agents.entries.ops.sandbox.docker.binds"); + }); }); diff --git a/src/security/audit-small-model-risk.test.ts b/src/security/audit-small-model-risk.test.ts index 0ca5c8dabb75..ea87d496154c 100644 --- a/src/security/audit-small-model-risk.test.ts +++ b/src/security/audit-small-model-risk.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { collectSmallModelRiskFindings } from "./audit-extra.summary.js"; +import { collectAuditModelRefs } from "./audit-model-refs.js"; function requireFirstSmallModelFinding( findings: ReturnType, @@ -15,6 +16,54 @@ function requireFirstSmallModelFinding( } describe("security audit small-model risk findings", () => { + it("reports canonical paths for agent model references", () => { + expect( + collectAuditModelRefs({ + agents: { + entries: { + simple: { model: "ollama/mistral-8b" }, + structured: { + model: { + primary: "ollama/gemma-4b", + fallbacks: ["ollama/phi-3b"], + }, + }, + }, + }, + } satisfies OpenClawConfig), + ).toEqual([ + { id: "ollama/mistral-8b", source: "agents.entries.simple.model" }, + { id: "ollama/gemma-4b", source: "agents.entries.structured.model.primary" }, + { id: "ollama/phi-3b", source: "agents.entries.structured.model.fallbacks" }, + ]); + }); + + it("preserves agent policy context for canonical model source paths", () => { + const finding = requireFirstSmallModelFinding( + collectSmallModelRiskFindings({ + cfg: { + agents: { + entries: { + ops: { + default: true, + model: { primary: "ollama/mistral-8b" }, + tools: { deny: ["web_search", "web_fetch", "browser"] }, + }, + }, + }, + tools: { web: { search: { enabled: true }, fetch: { enabled: true } } }, + browser: { enabled: true }, + } satisfies OpenClawConfig, + env: {}, + }), + "agent policy context", + ); + + expect(finding.severity).toBe("info"); + expect(finding.detail).toContain("@ agents.entries.ops.model.primary"); + expect(finding.detail).toContain("web=[off]"); + }); + it("scores small-model risk by tool/sandbox exposure", () => { const cases: Array<{ name: string; diff --git a/src/security/audit-trust-model.test.ts b/src/security/audit-trust-model.test.ts index b5bc58466031..eff13f12967a 100644 --- a/src/security/audit-trust-model.test.ts +++ b/src/security/audit-trust-model.test.ts @@ -288,7 +288,7 @@ describe("security audit trust model findings", () => { channels: { whatsapp: { groupPolicy: "open" } }, tools: { elevated: { enabled: false }, profile: "messaging" }, agents: { - list: [{ id: "ops", tools: { profile: "messaging", alsoAllow: ["gateway"] } }], + entries: { ops: { tools: { profile: "messaging", alsoAllow: ["gateway"] } } }, }, } satisfies OpenClawConfig, assert: (findings: ReturnType) => { @@ -296,7 +296,7 @@ describe("security audit trust model findings", () => { (entry) => entry.checkId === "security.exposure.open_groups_with_control_plane_tools", ); expect(finding?.detail).toContain( - "agents.list.ops (profile=messaging; controlPlane=[gateway])", + "agents.entries.ops (profile=messaging; controlPlane=[gateway])", ); expect(finding?.detail).not.toContain("agents.defaults (profile=messaging"); }, diff --git a/src/security/audit.ts b/src/security/audit.ts index 8c57b550ac39..4312a168ff3c 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -668,7 +668,7 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[] severity: "warn", title: "Agent exec host uses sandbox while sandbox mode is off", detail: - `agents.list.*.tools.exec.host is set to sandbox for: ${riskyAgents.join(", ")}. ` + + `agents.entries.*.tools.exec.host is set to sandbox for: ${riskyAgents.join(", ")}. ` + "With sandbox mode off, exec fails closed for those agents.", remediation: 'Enable sandbox mode for these agents (`agents.entries.*.sandbox.mode`) or set their tools.exec.host to "gateway".', @@ -849,7 +849,7 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[] continue; } collectRiskyTrustedDirHits( - `agents.list.${entry.id}.tools.exec`, + `agents.entries.${entry.id}.tools.exec`, entry.tools?.exec?.safeBinTrustedDirs, ); } @@ -886,17 +886,17 @@ function collectExecRuntimeFindings(cfg: OpenClawConfig): SecurityAuditFinding[] if (interpreters.length === 0) { for (const hit of listRiskyConfiguredSafeBins(agentSafeBins)) { riskySemanticSafeBinHits.push( - `- agents.list.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`, + `- agents.entries.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`, ); } continue; } interpreterHits.push( - `- agents.list.${entry.id}.tools.exec.safeBins: ${interpreters.join(", ")}`, + `- agents.entries.${entry.id}.tools.exec.safeBins: ${interpreters.join(", ")}`, ); for (const hit of listRiskyConfiguredSafeBins(agentSafeBins)) { riskySemanticSafeBinHits.push( - `- agents.list.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`, + `- agents.entries.${entry.id}.tools.exec.safeBins: ${hit.bin} (${hit.warning})`, ); } } diff --git a/src/security/dangerous-config-flags-core.ts b/src/security/dangerous-config-flags-core.ts index d7f62377cc00..f67abcc2d4e8 100644 --- a/src/security/dangerous-config-flags-core.ts +++ b/src/security/dangerous-config-flags-core.ts @@ -1,5 +1,5 @@ // Defines core dangerous config flag metadata for security audits. -import { listAgentEntriesWithSource } from "../agents/agent-scope-config.js"; +import { listAgentEntriesWithSource, type ListedAgentEntry } from "../agents/agent-scope-config.js"; import { DANGEROUS_SANDBOX_DOCKER_BOOLEAN_KEYS } from "../agents/sandbox/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { isRecord } from "../utils.js"; @@ -41,10 +41,13 @@ function formatDangerousConfigFlagValue(value: DangerousFlagValue): string { return value === null ? "null" : String(value); } -function getAgentDangerousFlagPathSegment( - source: ReturnType[number]["source"], -): string { - return source.kind === "entries" ? `agents.entries.${source.key}` : `agents.list.${source.index}`; +function getAgentDangerousFlagPathSegment(listed: ListedAgentEntry): string { + if (listed.source.kind === "entries") { + return `agents.entries.${listed.source.key}`; + } + return typeof listed.entry.id === "string" && listed.entry.id.length > 0 + ? `agents.entries.${listed.entry.id}` + : `agents.list.${listed.source.index}`; } function collectExactPluginConfigContractMatches({ @@ -98,10 +101,11 @@ export function collectEnabledInsecureOrDangerousFlagsFromContracts( : undefined, "agents.defaults.sandbox.docker", ); - for (const { entry: agent, source } of listAgentEntriesWithSource(cfg)) { + for (const listed of listAgentEntriesWithSource(cfg)) { + const agent = listed.entry; collectSandboxDockerDangerousFlags( isRecord(agent?.sandbox?.docker) ? agent.sandbox.docker : undefined, - `${getAgentDangerousFlagPathSegment(source)}.sandbox.docker`, + `${getAgentDangerousFlagPathSegment(listed)}.sandbox.docker`, ); } diff --git a/src/security/dangerous-config-flags.test.ts b/src/security/dangerous-config-flags.test.ts index 4010ef4e9544..4ebdb7d83652 100644 --- a/src/security/dangerous-config-flags.test.ts +++ b/src/security/dangerous-config-flags.test.ts @@ -198,16 +198,15 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { }, }, }, - list: [ - { - id: "worker", + entries: { + worker: { sandbox: { docker: { dangerouslyAllowExternalBindSources: true, }, }, }, - ], + }, }, hooks: { allowRequestSessionKey: true, @@ -231,7 +230,7 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { "tools.fs.workspaceOnly=false", "agents.defaults.sandbox.docker.dangerouslyAllowReservedContainerTargets=true", "agents.defaults.sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true", - "agents.list.0.sandbox.docker.dangerouslyAllowExternalBindSources=true", + "agents.entries.worker.sandbox.docker.dangerouslyAllowExternalBindSources=true", ]); }); @@ -249,7 +248,7 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { ).toContain("security.audit.suppressions configured (1)"); }); - it("uses legacy list indices for list-shaped dangerous sandbox flags", () => { + it("uses canonical entry paths for id-bearing legacy list rows", () => { expect( collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ @@ -263,25 +262,23 @@ describe("collectEnabledInsecureOrDangerousFlags", () => { }, }, }, - { - id: "helper", - }, ], }, }), ), - ).toContain("agents.list.0.sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true"); + ).toContain("agents.entries.worker.sandbox.docker.dangerouslyAllowContainerNamespaceJoin=true"); + }); + it("keeps legacy list indices for id-less dangerous sandbox rows", () => { expect( collectEnabledInsecureOrDangerousFlagsFromContracts( asConfig({ agents: { list: [ { - id: "helper", + id: "worker", }, { - id: "worker", sandbox: { docker: { dangerouslyAllowContainerNamespaceJoin: true, diff --git a/src/security/exec-filesystem-policy.ts b/src/security/exec-filesystem-policy.ts index 7656aa5e212d..ee0dab82967f 100644 --- a/src/security/exec-filesystem-policy.ts +++ b/src/security/exec-filesystem-policy.ts @@ -57,7 +57,7 @@ export function collectExecFilesystemPolicyDriftHits( continue; } contexts.push({ - scopeLabel: `agents.list.${agent.id}.tools`, + scopeLabel: `agents.entries.${agent.id}.tools`, agentId: agent.id, tools: agent.tools, }); diff --git a/src/skills/workshop/tool-policy-diagnostic.test.ts b/src/skills/workshop/tool-policy-diagnostic.test.ts index 2da2cd40697f..cc5699e69545 100644 --- a/src/skills/workshop/tool-policy-diagnostic.test.ts +++ b/src/skills/workshop/tool-policy-diagnostic.test.ts @@ -46,17 +46,17 @@ describe("detectSkillWorkshopToolPolicyDiagnostic", () => { agents: { list: [{ id: "main", default: true, tools: { profile: "messaging" } }] }, }), ).toMatchObject({ - source: "agents.list[0].tools.profile", - fix: 'Add agents.list[0].tools.alsoAllow: ["skill_workshop"].', + source: "agents.entries.main.tools.profile", + fix: 'Add agents.entries.main.tools.alsoAllow: ["skill_workshop"].', }); expect( detect({ - agents: { list: [{ id: "main", default: true, tools: { allow: ["read"] } }] }, + agents: { entries: { main: { default: true, tools: { allow: ["read"] } } } }, }), ).toMatchObject({ - source: "agents.list[0].tools.allow", - fix: 'Add "skill_workshop" to agents.list[0].tools.allow.', + source: "agents.entries.main.tools.allow", + fix: 'Add "skill_workshop" to agents.entries.main.tools.allow.', }); }); @@ -64,11 +64,11 @@ describe("detectSkillWorkshopToolPolicyDiagnostic", () => { expect( detect({ tools: { profile: "messaging" }, - agents: { list: [{ id: "main", default: true, tools: { alsoAllow: ["read"] } }] }, + agents: { entries: { main: { default: true, tools: { alsoAllow: ["read"] } } } }, }), ).toMatchObject({ source: "tools.profile", - fix: 'Add agents.list[0].tools.alsoAllow: ["skill_workshop"].', + fix: 'Add agents.entries.main.tools.alsoAllow: ["skill_workshop"].', }); }); @@ -89,19 +89,18 @@ describe("detectSkillWorkshopToolPolicyDiagnostic", () => { detect({ agents: { defaults: { model: { primary: "openai/gpt-5.5" } }, - list: [ - { - id: "main", + entries: { + main: { default: true, tools: { byProvider: { openai: { alsoAllow: ["read"] } } }, }, - ], + }, }, tools: { byProvider: { openai: { profile: "messaging" } } }, }), ).toMatchObject({ source: 'tools.byProvider["openai"].profile', - fix: 'Add agents.list[0].tools.byProvider["openai"].alsoAllow: ["skill_workshop"].', + fix: 'Add agents.entries.main.tools.byProvider["openai"].alsoAllow: ["skill_workshop"].', }); }); @@ -110,18 +109,17 @@ describe("detectSkillWorkshopToolPolicyDiagnostic", () => { detect({ agents: { defaults: { model: { primary: "openai/gpt-5.5" } }, - list: [ - { - id: "main", + entries: { + main: { default: true, tools: { byProvider: { openai: { allow: ["read"] } } }, }, - ], + }, }, }), ).toMatchObject({ - source: 'agents.list[0].tools.byProvider["openai"].allow', - fix: 'Add "skill_workshop" to agents.list[0].tools.byProvider["openai"].allow.', + source: 'agents.entries.main.tools.byProvider["openai"].allow', + fix: 'Add "skill_workshop" to agents.entries.main.tools.byProvider["openai"].allow.', }); }); diff --git a/src/skills/workshop/tool-policy-diagnostic.ts b/src/skills/workshop/tool-policy-diagnostic.ts index f4b32ee44b27..b013a86d5325 100644 --- a/src/skills/workshop/tool-policy-diagnostic.ts +++ b/src/skills/workshop/tool-policy-diagnostic.ts @@ -36,10 +36,15 @@ function findAgentTools(config: OpenClawConfig, agentId: string): AgentToolsLoca if (!listed?.entry.tools) { return undefined; } + // Report the canonical entries path; an id-less legacy list row (which + // normalizeAgentId maps to the default id) keeps its indexed path so the + // remediation never points at agents.entries.undefined. const path = listed.source.kind === "entries" ? `agents.entries.${listed.source.key}.tools` - : `agents.list[${listed.source.index}].tools`; + : typeof listed.entry.id === "string" && listed.entry.id.length > 0 + ? `agents.entries.${listed.entry.id}.tools` + : `agents.list[${listed.source.index}].tools`; return { path, tools: listed.entry.tools }; }