mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 20:03:59 +00:00
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 <amtellezfernandez@gmail.com> * fix(security): keep browser config behind plugin policy Co-authored-by: Alba María Téllez Fernández <amtellezfernandez@gmail.com> * chore(security): keep release notes in PR body --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
committed by
GitHub
parent
82106a18b3
commit
349d3547cc
@@ -1,3 +1,4 @@
|
||||
import { normalizeOptionalLowercaseString } from "@openclaw/normalization-core/string-coerce";
|
||||
// Summarizes extra security audit findings for user-facing output.
|
||||
import {
|
||||
resolveConfiguredToolPolicies,
|
||||
@@ -10,6 +11,8 @@ import { isToolAllowedByPolicies } from "../agents/tool-policy-match.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { AgentToolsConfig } from "../config/types.tools.js";
|
||||
import { hasConfiguredInternalHooks } from "../hooks/configured.js";
|
||||
import { normalizePluginsConfigWithResolver } from "../plugins/config-normalization-shared.js";
|
||||
import { passesManifestOwnerBasePolicy } from "../plugins/manifest-owner-policy.js";
|
||||
import { hasConfiguredWebSearchCredential } from "../plugins/web-search-credential-presence.js";
|
||||
import { inferParamBFromIdOrName } from "../shared/model-param-b.js";
|
||||
import { collectAuditModelRefs } from "./audit-model-refs.js";
|
||||
@@ -114,7 +117,17 @@ function isWebFetchEnabled(cfg: OpenClawConfig): boolean {
|
||||
}
|
||||
|
||||
function isBrowserEnabled(cfg: OpenClawConfig): boolean {
|
||||
return cfg.browser?.enabled !== false;
|
||||
if (cfg.browser?.enabled === false) {
|
||||
return false;
|
||||
}
|
||||
return passesManifestOwnerBasePolicy({
|
||||
plugin: { id: "browser" },
|
||||
normalizedConfig: normalizePluginsConfigWithResolver(
|
||||
cfg.plugins,
|
||||
(pluginId) => normalizeOptionalLowercaseString(pluginId) ?? "",
|
||||
),
|
||||
// Browser config selects behavior; it must not weaken global plugin policy.
|
||||
});
|
||||
}
|
||||
|
||||
/** Produce a concise inventory of major security-relevant surfaces. */
|
||||
@@ -123,7 +136,7 @@ export function collectAttackSurfaceSummaryFindings(cfg: OpenClawConfig): Securi
|
||||
const elevated = cfg.tools?.elevated?.enabled !== false;
|
||||
const webhooksEnabled = cfg.hooks?.enabled === true;
|
||||
const internalHooksEnabled = hasConfiguredInternalHooks(cfg);
|
||||
const browserEnabled = cfg.browser?.enabled ?? true;
|
||||
const browserEnabled = isBrowserEnabled(cfg);
|
||||
|
||||
const detail =
|
||||
`groups: open=${group.open}, allowlist=${group.allowlist}` +
|
||||
|
||||
@@ -68,6 +68,14 @@ describe("collectSmallModelRiskFindings", () => {
|
||||
agents: { defaults: { model: { primary: "ollama/mistral-8b" } } },
|
||||
tools: { web: { fetch: { enabled: false } } },
|
||||
} satisfies OpenClawConfig;
|
||||
const browserBlockedByPluginPolicyCfg = {
|
||||
...browserDefaultCfg,
|
||||
plugins: { allow: ["openai"] },
|
||||
} satisfies OpenClawConfig;
|
||||
const configuredBrowserBlockedByPluginPolicyCfg = {
|
||||
...browserBlockedByPluginPolicyCfg,
|
||||
browser: { enabled: true },
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
it.each([
|
||||
{
|
||||
@@ -86,6 +94,22 @@ describe("collectSmallModelRiskFindings", () => {
|
||||
detailIncludes: ["web=[browser]"],
|
||||
detailExcludes: ["No web/browser tools detected"],
|
||||
},
|
||||
{
|
||||
name: "treats browser as disabled when restrictive plugin policy excludes it",
|
||||
cfg: browserBlockedByPluginPolicyCfg,
|
||||
env: {},
|
||||
expectedSeverity: "info",
|
||||
detailIncludes: ["web=[off]", "No web/browser tools detected"],
|
||||
detailExcludes: ["web=[browser]"],
|
||||
},
|
||||
{
|
||||
name: "does not let browser config bypass restrictive plugin policy",
|
||||
cfg: configuredBrowserBlockedByPluginPolicyCfg,
|
||||
env: {},
|
||||
expectedSeverity: "info",
|
||||
detailIncludes: ["web=[off]", "No web/browser tools detected"],
|
||||
detailExcludes: ["web=[browser]"],
|
||||
},
|
||||
])("$name", ({ cfg, env, expectedSeverity, detailIncludes, detailExcludes }) => {
|
||||
const finding = requireFirstFinding(
|
||||
collectSmallModelRiskFindings({
|
||||
|
||||
@@ -38,4 +38,64 @@ describe("security audit attack surface summary", () => {
|
||||
].join("\n"),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "restrictive plugin allowlist excludes browser and no browser config is present",
|
||||
cfg: {
|
||||
plugins: { allow: ["openai"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
{
|
||||
name: "explicit browser config does not bypass a restrictive plugin allowlist",
|
||||
cfg: {
|
||||
browser: { enabled: true },
|
||||
plugins: { allow: ["openai"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
{
|
||||
name: "plugin ids use the same case-insensitive canonical form as startup",
|
||||
cfg: {
|
||||
plugins: { allow: ["Browser"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: enabled",
|
||||
},
|
||||
{
|
||||
name: "plugin deny policy wins over explicit browser config",
|
||||
cfg: {
|
||||
browser: { enabled: true },
|
||||
plugins: { allow: ["browser"], deny: ["browser"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
{
|
||||
name: "disabled browser plugin entry wins over explicit browser config",
|
||||
cfg: {
|
||||
browser: { enabled: true },
|
||||
plugins: { allow: ["browser"], entries: { browser: { enabled: false } } },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
{
|
||||
name: "browser.enabled=false disables browser control",
|
||||
cfg: {
|
||||
browser: { enabled: false },
|
||||
plugins: { allow: ["browser"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
{
|
||||
name: "case-normalized plugin deny policy disables browser control",
|
||||
cfg: {
|
||||
plugins: { deny: ["BROWSER"] },
|
||||
} satisfies OpenClawConfig,
|
||||
expected: "browser control: disabled",
|
||||
},
|
||||
])("reports browser control from effective plugin policy: $name", ({ cfg, expected }) => {
|
||||
const summary = requireAttackSurfaceSummary(collectAttackSurfaceSummaryFindings(cfg));
|
||||
|
||||
expect(summary.detail).toContain(expected);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user