From b55d214e97cebd5cb15cd9edf03a4eed51a218f9 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 20 Apr 2026 21:29:13 -0400 Subject: [PATCH] fix: tighten read-only audit coverage --- src/plugins/channel-plugin-ids.test.ts | 30 ++++++++++++++++ src/plugins/channel-plugin-ids.ts | 4 +++ src/secrets/channel-env-var-names.ts | 26 ++++++++++++++ src/secrets/channel-env-vars.ts | 1 + .../audit-plugin-readonly-scope.test.ts | 34 ++++++++++++++++++- src/security/audit.ts | 5 ++- 6 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 src/secrets/channel-env-var-names.ts diff --git a/src/plugins/channel-plugin-ids.test.ts b/src/plugins/channel-plugin-ids.test.ts index 3166cdb3189..0f101b80941 100644 --- a/src/plugins/channel-plugin-ids.test.ts +++ b/src/plugins/channel-plugin-ids.test.ts @@ -115,6 +115,17 @@ function createManifestRegistryFixture() { providers: [], cliBackends: [], }, + { + id: "ambient-env-channel-plugin", + channels: ["ambient-env-channel"], + channelEnvVars: { + "ambient-env-channel": ["HOME", "PATH", "lowercase_token"], + }, + origin: "config", + enabledByDefault: undefined, + providers: [], + cliBackends: [], + }, { id: "voice-call", channels: [], @@ -661,6 +672,25 @@ describe("listConfiguredChannelIdsForReadOnlyScope", () => { ).toEqual(["external-env-channel"]); }); + it("ignores ambient or malformed manifest env vars as read-only configured channel triggers", () => { + expect( + listConfiguredChannelIdsForReadOnlyScope({ + config: { + plugins: { + allow: ["ambient-env-channel-plugin"], + }, + } as OpenClawConfig, + workspaceDir: "/tmp", + env: { + HOME: "/tmp/user", + PATH: "/usr/bin", + lowercase_token: "token", + } as NodeJS.ProcessEnv, + includePersistedAuthState: false, + }), + ).toEqual([]); + }); + it("uses manifest env vars for read-only channel presence checks", () => { listPotentialConfiguredChannelIds.mockReturnValue([]); hasPotentialConfiguredChannels.mockReturnValue(false); diff --git a/src/plugins/channel-plugin-ids.ts b/src/plugins/channel-plugin-ids.ts index 7edbfa1dec2..d4ee7e5ebbd 100644 --- a/src/plugins/channel-plugin-ids.ts +++ b/src/plugins/channel-plugin-ids.ts @@ -10,6 +10,7 @@ import { resolveMemoryDreamingPluginConfig, resolveMemoryDreamingPluginId, } from "../memory-host-sdk/dreaming.js"; +import { isSafeChannelEnvVarTriggerName } from "../secrets/channel-env-var-names.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; import { resolveManifestActivationPluginIds } from "./activation-planner.js"; import { @@ -66,6 +67,9 @@ function normalizeChannelIds(channelIds: Iterable): string[] { } function hasNonEmptyEnvValue(env: NodeJS.ProcessEnv, key: string): boolean { + if (!isSafeChannelEnvVarTriggerName(key)) { + return false; + } const value = env[key]; return typeof value === "string" && value.trim().length > 0; } diff --git a/src/secrets/channel-env-var-names.ts b/src/secrets/channel-env-var-names.ts new file mode 100644 index 00000000000..ac82cdac47e --- /dev/null +++ b/src/secrets/channel-env-var-names.ts @@ -0,0 +1,26 @@ +const UNSAFE_CHANNEL_ENV_VAR_TRIGGER_NAMES = new Set([ + "CI", + "HOME", + "LANG", + "LC_ALL", + "LC_CTYPE", + "LOGNAME", + "NODE_ENV", + "OLDPWD", + "PATH", + "PWD", + "SHELL", + "SSH_AUTH_SOCK", + "TEMP", + "TERM", + "TMP", + "TMPDIR", + "USER", +]); + +export function isSafeChannelEnvVarTriggerName(key: string): boolean { + const normalized = key.trim(); + return ( + /^[A-Z][A-Z0-9_]*$/.test(normalized) && !UNSAFE_CHANNEL_ENV_VAR_TRIGGER_NAMES.has(normalized) + ); +} diff --git a/src/secrets/channel-env-vars.ts b/src/secrets/channel-env-vars.ts index b76aebb06f0..98a7976bcac 100644 --- a/src/secrets/channel-env-vars.ts +++ b/src/secrets/channel-env-vars.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { loadPluginManifestRegistry } from "../plugins/manifest-registry.js"; +export { isSafeChannelEnvVarTriggerName } from "./channel-env-var-names.js"; type ChannelEnvVarLookupParams = { config?: OpenClawConfig; diff --git a/src/security/audit-plugin-readonly-scope.test.ts b/src/security/audit-plugin-readonly-scope.test.ts index fcab0d3704d..8fe631c2c19 100644 --- a/src/security/audit-plugin-readonly-scope.test.ts +++ b/src/security/audit-plugin-readonly-scope.test.ts @@ -40,7 +40,7 @@ describe("security audit read-only plugin scope", () => { resolveConfiguredChannelPluginIdsMock.mockReturnValue([]); }); - it("removes configured channel owner plugin ids before loading audit collectors", async () => { + it("keeps configured channel owner collectors when the provided channel plugin list omits them", async () => { const sourceConfig = { plugins: { allow: ["external-channel-plugin", "audit-plugin"], @@ -72,6 +72,38 @@ describe("security audit read-only plugin scope", () => { env: {}, }), ); + expect(loadPluginMetadataRegistrySnapshotMock).toHaveBeenCalledWith( + expect.objectContaining({ + onlyPluginIds: ["external-channel-plugin", "audit-plugin"], + }), + ); + }); + + it("removes configured channel owner collectors only when channel security will audit them", async () => { + const sourceConfig = { + plugins: { + allow: ["external-channel-plugin", "audit-plugin"], + }, + }; + applyPluginAutoEnableMock.mockReturnValue({ + config: sourceConfig, + changes: [], + autoEnabledReasons: { + "external-channel-plugin": ["channel:external"], + "audit-plugin": ["explicit"], + }, + }); + resolveConfiguredChannelPluginIdsMock.mockReturnValue(["external-channel-plugin"]); + + await runSecurityAudit({ + config: sourceConfig, + sourceConfig, + env: {} as NodeJS.ProcessEnv, + includeFilesystem: false, + includeChannelSecurity: true, + plugins: [{ id: "external-channel-plugin" }] as never, + }); + expect(loadPluginMetadataRegistrySnapshotMock).toHaveBeenCalledWith( expect.objectContaining({ onlyPluginIds: ["audit-plugin"], diff --git a/src/security/audit.ts b/src/security/audit.ts index 1bb8267114c..31493bca751 100644 --- a/src/security/audit.ts +++ b/src/security/audit.ts @@ -358,13 +358,16 @@ async function collectPluginSecurityAuditFindings( } } if (context.includeChannelSecurity && context.plugins !== undefined) { + const auditedChannelPluginIds = new Set(context.plugins.map((plugin) => plugin.id)); for (const pluginId of resolveConfiguredChannelPluginIds({ config: autoEnabled.config, activationSourceConfig: context.sourceConfig, workspaceDir: context.workspaceDir, env: context.env, })) { - requestedPluginIds.delete(pluginId); + if (auditedChannelPluginIds.has(pluginId)) { + requestedPluginIds.delete(pluginId); + } } } if (requestedPluginIds.size === 0) {