mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:00:44 +00:00
fix: tighten read-only audit coverage
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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>): 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;
|
||||
}
|
||||
|
||||
26
src/secrets/channel-env-var-names.ts
Normal file
26
src/secrets/channel-env-var-names.ts
Normal file
@@ -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)
|
||||
);
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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"],
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user