Skip to content

Commit 349d354

Browse files
amtellezfernandezsteipete
amtellezfernandez
and
steipete
authored
fix(security): align browser audit with plugin policy (#97732)
* fix(security): respect plugin policy in browser audit summary * fix(security): reuse browser plugin activation policy Co-authored-by: Alba María Téllez Fernández <[email protected]> * fix(security): keep browser config behind plugin policy Co-authored-by: Alba María Téllez Fernández <[email protected]> * chore(security): keep release notes in PR body --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent 82106a1 commit 349d354

3 files changed

Lines changed: 99 additions & 2 deletions

File tree

‎src/security/audit-extra.summary.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { normalizeOptionalLowercaseString } from "@openclaw/normalization-core/string-coerce";
12
// Summarizes extra security audit findings for user-facing output.
23
import {
34
resolveConfiguredToolPolicies,
@@ -10,6 +11,8 @@ import { isToolAllowedByPolicies } from "../agents/tool-policy-match.js";
1011
import type { OpenClawConfig } from "../config/types.openclaw.js";
1112
import type { AgentToolsConfig } from "../config/types.tools.js";
1213
import { hasConfiguredInternalHooks } from "../hooks/configured.js";
14+
import { normalizePluginsConfigWithResolver } from "../plugins/config-normalization-shared.js";
15+
import { passesManifestOwnerBasePolicy } from "../plugins/manifest-owner-policy.js";
1316
import { hasConfiguredWebSearchCredential } from "../plugins/web-search-credential-presence.js";
1417
import { inferParamBFromIdOrName } from "../shared/model-param-b.js";
1518
import { collectAuditModelRefs } from "./audit-model-refs.js";
@@ -114,7 +117,17 @@ function isWebFetchEnabled(cfg: OpenClawConfig): boolean {
114117
}
115118

116119
function isBrowserEnabled(cfg: OpenClawConfig): boolean {
117-
return cfg.browser?.enabled !== false;
120+
if (cfg.browser?.enabled === false) {
121+
return false;
122+
}
123+
return passesManifestOwnerBasePolicy({
124+
plugin: { id: "browser" },
125+
normalizedConfig: normalizePluginsConfigWithResolver(
126+
cfg.plugins,
127+
(pluginId) => normalizeOptionalLowercaseString(pluginId) ?? "",
128+
),
129+
// Browser config selects behavior; it must not weaken global plugin policy.
130+
});
118131
}
119132

120133
/** Produce a concise inventory of major security-relevant surfaces. */
@@ -123,7 +136,7 @@ export function collectAttackSurfaceSummaryFindings(cfg: OpenClawConfig): Securi
123136
const elevated = cfg.tools?.elevated?.enabled !== false;
124137
const webhooksEnabled = cfg.hooks?.enabled === true;
125138
const internalHooksEnabled = hasConfiguredInternalHooks(cfg);
126-
const browserEnabled = cfg.browser?.enabled ?? true;
139+
const browserEnabled = isBrowserEnabled(cfg);
127140

128141
const detail =
129142
`groups: open=${group.open}, allowlist=${group.allowlist}` +

‎src/security/audit-extra.sync.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,14 @@ describe("collectSmallModelRiskFindings", () => {
6868
agents: { defaults: { model: { primary: "ollama/mistral-8b" } } },
6969
tools: { web: { fetch: { enabled: false } } },
7070
} satisfies OpenClawConfig;
71+
const browserBlockedByPluginPolicyCfg = {
72+
...browserDefaultCfg,
73+
plugins: { allow: ["openai"] },
74+
} satisfies OpenClawConfig;
75+
const configuredBrowserBlockedByPluginPolicyCfg = {
76+
...browserBlockedByPluginPolicyCfg,
77+
browser: { enabled: true },
78+
} satisfies OpenClawConfig;
7179

7280
it.each([
7381
{
@@ -86,6 +94,22 @@ describe("collectSmallModelRiskFindings", () => {
8694
detailIncludes: ["web=[browser]"],
8795
detailExcludes: ["No web/browser tools detected"],
8896
},
97+
{
98+
name: "treats browser as disabled when restrictive plugin policy excludes it",
99+
cfg: browserBlockedByPluginPolicyCfg,
100+
env: {},
101+
expectedSeverity: "info",
102+
detailIncludes: ["web=[off]", "No web/browser tools detected"],
103+
detailExcludes: ["web=[browser]"],
104+
},
105+
{
106+
name: "does not let browser config bypass restrictive plugin policy",
107+
cfg: configuredBrowserBlockedByPluginPolicyCfg,
108+
env: {},
109+
expectedSeverity: "info",
110+
detailIncludes: ["web=[off]", "No web/browser tools detected"],
111+
detailExcludes: ["web=[browser]"],
112+
},
89113
])("$name", ({ cfg, env, expectedSeverity, detailIncludes, detailExcludes }) => {
90114
const finding = requireFirstFinding(
91115
collectSmallModelRiskFindings({

‎src/security/audit-summary.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,64 @@ describe("security audit attack surface summary", () => {
3838
].join("\n"),
3939
);
4040
});
41+
42+
it.each([
43+
{
44+
name: "restrictive plugin allowlist excludes browser and no browser config is present",
45+
cfg: {
46+
plugins: { allow: ["openai"] },
47+
} satisfies OpenClawConfig,
48+
expected: "browser control: disabled",
49+
},
50+
{
51+
name: "explicit browser config does not bypass a restrictive plugin allowlist",
52+
cfg: {
53+
browser: { enabled: true },
54+
plugins: { allow: ["openai"] },
55+
} satisfies OpenClawConfig,
56+
expected: "browser control: disabled",
57+
},
58+
{
59+
name: "plugin ids use the same case-insensitive canonical form as startup",
60+
cfg: {
61+
plugins: { allow: ["Browser"] },
62+
} satisfies OpenClawConfig,
63+
expected: "browser control: enabled",
64+
},
65+
{
66+
name: "plugin deny policy wins over explicit browser config",
67+
cfg: {
68+
browser: { enabled: true },
69+
plugins: { allow: ["browser"], deny: ["browser"] },
70+
} satisfies OpenClawConfig,
71+
expected: "browser control: disabled",
72+
},
73+
{
74+
name: "disabled browser plugin entry wins over explicit browser config",
75+
cfg: {
76+
browser: { enabled: true },
77+
plugins: { allow: ["browser"], entries: { browser: { enabled: false } } },
78+
} satisfies OpenClawConfig,
79+
expected: "browser control: disabled",
80+
},
81+
{
82+
name: "browser.enabled=false disables browser control",
83+
cfg: {
84+
browser: { enabled: false },
85+
plugins: { allow: ["browser"] },
86+
} satisfies OpenClawConfig,
87+
expected: "browser control: disabled",
88+
},
89+
{
90+
name: "case-normalized plugin deny policy disables browser control",
91+
cfg: {
92+
plugins: { deny: ["BROWSER"] },
93+
} satisfies OpenClawConfig,
94+
expected: "browser control: disabled",
95+
},
96+
])("reports browser control from effective plugin policy: $name", ({ cfg, expected }) => {
97+
const summary = requireAttackSurfaceSummary(collectAttackSurfaceSummaryFindings(cfg));
98+
99+
expect(summary.detail).toContain(expected);
100+
});
41101
});

0 commit comments

Comments
 (0)