fix: tighten read-only plugin triage paths

This commit is contained in:
Gustavo Madeira Santana
2026-04-20 20:13:44 -04:00
parent 0e30b8df34
commit a7f0ae9c07
8 changed files with 240 additions and 55 deletions

View File

@@ -1,12 +1,14 @@
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../../agents/agent-scope.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { resolveDiscoverableScopedChannelPluginIds } from "../../plugins/channel-plugin-ids.js";
import {
listConfiguredChannelIdsForPluginScope,
resolveDiscoverableScopedChannelPluginIds,
} from "../../plugins/channel-plugin-ids.js";
import { loadOpenClawPlugins } from "../../plugins/loader.js";
import {
loadPluginManifestRegistry,
type PluginManifestRecord,
} from "../../plugins/manifest-registry.js";
import { listPotentialConfiguredChannelIds } from "../config-presence.js";
import { getBundledChannelSetupPlugin } from "./bundled.js";
import { listChannelPlugins } from "./registry.js";
import type { ChannelPlugin } from "./types.plugin.js";
@@ -48,11 +50,6 @@ function addChannelPlugins(
}
}
function hasNonEmptyEnvValue(env: NodeJS.ProcessEnv, key: string): boolean {
const value = env[key];
return typeof value === "string" && value.trim().length > 0;
}
function resolveReadOnlyWorkspaceDir(
cfg: OpenClawConfig,
options: ReadOnlyChannelPluginOptions,
@@ -74,22 +71,6 @@ function listExternalChannelManifestRecords(params: {
}).plugins.filter((plugin) => plugin.origin !== "bundled" && plugin.channels.length > 0);
}
function listExternalEnvConfiguredChannelIds(params: {
records: readonly PluginManifestRecord[];
env: NodeJS.ProcessEnv;
}): string[] {
const channelIds = new Set<string>();
for (const record of params.records) {
for (const channelId of record.channels) {
const envVars = record.channelEnvVars?.[channelId] ?? [];
if (envVars.some((envVar) => hasNonEmptyEnvValue(params.env, envVar))) {
channelIds.add(channelId);
}
}
}
return [...channelIds].toSorted((left, right) => left.localeCompare(right));
}
function resolveExternalReadOnlyChannelPluginIds(params: {
cfg: OpenClawConfig;
activationSourceConfig?: OpenClawConfig;
@@ -148,15 +129,16 @@ export function listReadOnlyChannelPluginsForConfig(
cache: options.cache,
});
const configuredChannelIds = [
...new Set([
...listPotentialConfiguredChannelIds(cfg, env, {
includePersistedAuthState: options.includePersistedAuthState,
}),
...listExternalEnvConfiguredChannelIds({
records: externalManifestRecords,
...new Set(
listConfiguredChannelIdsForPluginScope({
config: cfg,
workspaceDir,
env,
cache: options.cache,
includePersistedAuthState: options.includePersistedAuthState,
manifestRecords: externalManifestRecords,
}),
]),
),
];
const byId = new Map<string, ChannelPlugin>();

View File

@@ -69,4 +69,28 @@ describe("resolveProviderAuths plugin boundary", () => {
expect(resolveProviderUsageAuthWithPluginMock).not.toHaveBeenCalled();
expect(ensureAuthProfileStoreMock).not.toHaveBeenCalled();
});
it("skips plugin usage auth per provider when only another provider has direct credentials", async () => {
await expect(
resolveProviderAuths({
providers: ["anthropic", "zai"],
skipPluginAuthWithoutCredentialSource: true,
env: {
ANTHROPIC_API_KEY: "sk-ant",
},
}),
).resolves.toEqual([
{
provider: "anthropic",
token: "sk-ant",
},
]);
expect(resolveProviderUsageAuthWithPluginMock).toHaveBeenCalledTimes(1);
expect(resolveProviderUsageAuthWithPluginMock).toHaveBeenCalledWith(
expect.objectContaining({
provider: "anthropic",
}),
);
});
});

View File

@@ -242,25 +242,25 @@ export async function resolveProviderAuths(params: {
env: params.env ?? process.env,
agentDir: params.agentDir,
};
const hasDirectCredentialSource = params.providers.some((provider) =>
Boolean(
const hasAuthProfileStoreSource = hasAnyAuthProfileStoreSource(params.agentDir);
const auths: ProviderAuth[] = [];
for (const provider of params.providers) {
const hasDirectCredentialSource = Boolean(
resolveProviderApiKeyFromConfig({
state: { ...stateBase, allowAuthProfileStore: false },
providerIds: [provider],
}),
),
);
const allowAuthProfileStore =
!params.skipPluginAuthWithoutCredentialSource ||
hasDirectCredentialSource ||
hasAnyAuthProfileStoreSource(params.agentDir);
const state: UsageAuthState = {
...stateBase,
allowAuthProfileStore,
};
const auths: ProviderAuth[] = [];
);
const allowAuthProfileStore =
!params.skipPluginAuthWithoutCredentialSource ||
hasDirectCredentialSource ||
hasAuthProfileStoreSource;
const state: UsageAuthState = {
...stateBase,
allowAuthProfileStore,
};
for (const provider of params.providers) {
if (!params.skipPluginAuthWithoutCredentialSource || allowAuthProfileStore) {
const pluginAuth = await resolveProviderUsageAuthViaPlugin({
state,

View File

@@ -102,6 +102,17 @@ function createManifestRegistryFixture() {
providers: [],
cliBackends: [],
},
{
id: "external-env-channel-plugin",
channels: ["external-env-channel"],
channelEnvVars: {
"external-env-channel": ["EXTERNAL_ENV_CHANNEL_TOKEN"],
},
origin: "config",
enabledByDefault: undefined,
providers: [],
cliBackends: [],
},
{
id: "voice-call",
channels: [],
@@ -586,6 +597,22 @@ describe("resolveConfiguredChannelPluginIds", () => {
).toEqual([]);
});
it("includes trusted external channel owners configured only by manifest env vars", () => {
expect(
resolveConfiguredChannelPluginIds({
config: {
plugins: {
allow: ["external-env-channel-plugin"],
},
} as OpenClawConfig,
workspaceDir: "/tmp",
env: {
EXTERNAL_ENV_CHANNEL_TOKEN: "token",
} as NodeJS.ProcessEnv,
}),
).toEqual(["external-env-channel-plugin"]);
});
it("blocks bundled activation owners when explicitly disabled", () => {
expect(
resolveConfiguredChannelPluginIds({

View File

@@ -61,6 +61,56 @@ function normalizeChannelIds(channelIds: Iterable<string>): string[] {
).toSorted((left, right) => left.localeCompare(right));
}
function hasNonEmptyEnvValue(env: NodeJS.ProcessEnv, key: string): boolean {
const value = env[key];
return typeof value === "string" && value.trim().length > 0;
}
function listEnvConfiguredManifestChannelIds(params: {
records: readonly PluginManifestRecord[];
env: NodeJS.ProcessEnv;
}): string[] {
const channelIds = new Set<string>();
for (const record of params.records) {
for (const channelId of record.channels) {
const envVars = record.channelEnvVars?.[channelId] ?? [];
if (envVars.some((envVar) => hasNonEmptyEnvValue(params.env, envVar))) {
channelIds.add(channelId);
}
}
}
return [...channelIds].toSorted((left, right) => left.localeCompare(right));
}
export function listConfiguredChannelIdsForPluginScope(params: {
config: OpenClawConfig;
workspaceDir?: string;
env: NodeJS.ProcessEnv;
cache?: boolean;
includePersistedAuthState?: boolean;
manifestRecords?: readonly PluginManifestRecord[];
}): string[] {
const records =
params.manifestRecords ??
loadPluginManifestRegistry({
config: params.config,
workspaceDir: params.workspaceDir,
env: params.env,
cache: params.cache,
}).plugins;
return [
...new Set([
...listPotentialConfiguredChannelIds(params.config, params.env, {
includePersistedAuthState: params.includePersistedAuthState,
}),
...listEnvConfiguredManifestChannelIds({
records,
env: params.env,
}),
]),
].toSorted((left, right) => left.localeCompare(right));
}
function isChannelPluginEligibleForScopedOwnership(params: {
plugin: PluginManifestRecord;
normalizedConfig: ReturnType<typeof normalizePluginsConfig>;
@@ -222,7 +272,11 @@ export function resolveConfiguredChannelPluginIds(params: {
env: NodeJS.ProcessEnv;
}): string[] {
const configuredChannelIds = new Set(
listPotentialConfiguredChannelIds(params.config, params.env).map((id) => id.trim()),
listConfiguredChannelIdsForPluginScope({
config: params.config,
workspaceDir: params.workspaceDir,
env: params.env,
}).map((id) => id.trim()),
);
if (configuredChannelIds.size === 0) {
return [];

View File

@@ -78,6 +78,7 @@ export function ensurePluginRegistryLoaded(options?: {
config?: OpenClawConfig;
activationSourceConfig?: OpenClawConfig;
env?: NodeJS.ProcessEnv;
workspaceDir?: string;
onlyPluginIds?: string[];
}): void {
const scope = options?.scope ?? "all";

View File

@@ -0,0 +1,81 @@
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
const applyPluginAutoEnableMock = vi.hoisted(() => vi.fn());
const loadPluginMetadataRegistrySnapshotMock = vi.hoisted(() => vi.fn());
const resolveConfiguredChannelPluginIdsMock = vi.hoisted(() => vi.fn());
vi.mock("../config/plugin-auto-enable.js", () => ({
applyPluginAutoEnable: (...args: unknown[]) => applyPluginAutoEnableMock(...args),
}));
vi.mock("../plugins/channel-plugin-ids.js", () => ({
resolveConfiguredChannelPluginIds: (...args: unknown[]) =>
resolveConfiguredChannelPluginIdsMock(...args),
}));
vi.mock("../plugins/runtime/metadata-registry-loader.js", () => ({
loadPluginMetadataRegistrySnapshot: (...args: unknown[]) =>
loadPluginMetadataRegistrySnapshotMock(...args),
}));
let runSecurityAudit: typeof import("./audit.js").runSecurityAudit;
describe("security audit read-only plugin scope", () => {
beforeAll(async () => {
({ runSecurityAudit } = await import("./audit.js"));
});
beforeEach(() => {
applyPluginAutoEnableMock.mockReset();
loadPluginMetadataRegistrySnapshotMock.mockReset();
resolveConfiguredChannelPluginIdsMock.mockReset();
applyPluginAutoEnableMock.mockImplementation((params: { config: unknown }) => ({
config: params.config,
changes: [],
autoEnabledReasons: {},
}));
loadPluginMetadataRegistrySnapshotMock.mockReturnValue({
securityAuditCollectors: [],
});
resolveConfiguredChannelPluginIdsMock.mockReturnValue([]);
});
it("removes configured channel owner plugin ids before loading audit collectors", 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: [],
});
expect(resolveConfiguredChannelPluginIdsMock).toHaveBeenCalledWith(
expect.objectContaining({
config: sourceConfig,
activationSourceConfig: sourceConfig,
env: {},
}),
);
expect(loadPluginMetadataRegistrySnapshotMock).toHaveBeenCalledWith(
expect.objectContaining({
onlyPluginIds: ["audit-plugin"],
}),
);
});
});

View File

@@ -1,9 +1,7 @@
import path from "node:path";
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js";
import { resolveSandboxConfigForAgent } from "../agents/sandbox/config.js";
import {
hasPotentialConfiguredChannels,
listPotentialConfiguredChannelIds,
} from "../channels/config-presence.js";
import { hasPotentialConfiguredChannels } from "../channels/config-presence.js";
import type { listChannelPlugins } from "../channels/plugins/index.js";
import type { ConfigFileSnapshot, OpenClawConfig } from "../config/config.js";
import { resolveConfigPath, resolveStateDir } from "../config/paths.js";
@@ -16,6 +14,7 @@ import {
} from "../infra/exec-safe-bin-runtime-policy.js";
import { listRiskyConfiguredSafeBins } from "../infra/exec-safe-bin-semantics.js";
import { normalizeTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js";
import { resolveConfiguredChannelPluginIds } from "../plugins/channel-plugin-ids.js";
import { getActivePluginRegistry } from "../plugins/runtime.js";
import { DEFAULT_AGENT_ID } from "../routing/session-key.js";
import { asNullableRecord } from "../shared/record-coerce.js";
@@ -73,6 +72,8 @@ export type SecurityAuditOptions = {
codeSafetySummaryCache?: Map<string, Promise<unknown>>;
/** Optional explicit auth for deep gateway probe. */
deepProbeAuth?: { token?: string; password?: string };
/** Override workspace used for workspace plugin discovery. */
workspaceDir?: string;
/** Dependency injection for tests. */
probeGatewayFn?: ProbeGatewayFn;
};
@@ -95,6 +96,7 @@ type AuditExecutionContext = {
configSnapshot: ConfigFileSnapshot | null;
codeSafetySummaryCache: Map<string, Promise<unknown>>;
deepProbeAuth?: { token?: string; password?: string };
workspaceDir?: string;
};
let channelPluginsModulePromise: Promise<typeof import("../channels/plugins/index.js")> | undefined;
@@ -356,11 +358,13 @@ async function collectPluginSecurityAuditFindings(
}
}
if (context.includeChannelSecurity && context.plugins !== undefined) {
for (const channelId of listPotentialConfiguredChannelIds(
context.sourceConfig,
context.env,
)) {
requestedPluginIds.delete(channelId);
for (const pluginId of resolveConfiguredChannelPluginIds({
config: autoEnabled.config,
activationSourceConfig: context.sourceConfig,
workspaceDir: context.workspaceDir,
env: context.env,
})) {
requestedPluginIds.delete(pluginId);
}
}
if (requestedPluginIds.size === 0) {
@@ -372,6 +376,7 @@ async function collectPluginSecurityAuditFindings(
config: autoEnabled.config,
activationSourceConfig: context.sourceConfig,
env: context.env,
workspaceDir: context.workspaceDir,
onlyPluginIds: [...requestedPluginIds],
});
collectors = snapshot.securityAuditCollectors ?? [];
@@ -894,6 +899,8 @@ async function createAuditExecutionContext(
const deepTimeoutMs = Math.max(250, opts.deepTimeoutMs ?? 5000);
const stateDir = opts.stateDir ?? resolveStateDir(env);
const configPath = opts.configPath ?? resolveConfigPath(env, stateDir);
const workspaceDir =
opts.workspaceDir ?? resolveAgentWorkspaceDir(cfg, resolveDefaultAgentId(cfg));
const { readConfigSnapshotForAudit } = await loadAuditNonDeepModule();
const configSnapshot = includeFilesystem
? opts.configSnapshot !== undefined
@@ -915,6 +922,7 @@ async function createAuditExecutionContext(
execDockerRawFn: opts.execDockerRawFn,
probeGatewayFn: opts.probeGatewayFn,
plugins: opts.plugins,
workspaceDir,
configSnapshot,
codeSafetySummaryCache: opts.codeSafetySummaryCache ?? new Map<string, Promise<unknown>>(),
deepProbeAuth: opts.deepProbeAuth,
@@ -997,13 +1005,21 @@ export async function runSecurityAudit(opts: SecurityAuditOptions): Promise<Secu
const shouldAuditChannelSecurity =
context.includeChannelSecurity &&
(context.plugins !== undefined || hasPotentialConfiguredChannels(cfg, env));
(context.plugins !== undefined ||
hasPotentialConfiguredChannels(cfg, env) ||
resolveConfiguredChannelPluginIds({
config: cfg,
activationSourceConfig: context.sourceConfig,
workspaceDir: context.workspaceDir,
env,
}).length > 0);
if (shouldAuditChannelSecurity) {
if (context.plugins === undefined) {
(await loadPluginRegistryLoaderModule()).ensurePluginRegistryLoaded({
scope: "configured-channels",
config: cfg,
activationSourceConfig: context.sourceConfig,
workspaceDir: context.workspaceDir,
env,
});
}