fix(gateway): prevent default built-in plugins from being disabled by explicit allowlists (#737)
This commit is contained in:
committed by
GitHub
Unverified
parent
d34a88e629
commit
ca92d7fa2c
@@ -9,10 +9,11 @@
|
|||||||
* Responding" hangs.
|
* Responding" hangs.
|
||||||
*/
|
*/
|
||||||
import { access, mkdir, readFile, writeFile } from 'fs/promises';
|
import { access, mkdir, readFile, writeFile } from 'fs/promises';
|
||||||
import { constants } from 'fs';
|
import { constants, readdirSync, readFileSync, existsSync } from 'fs';
|
||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { homedir } from 'os';
|
import { homedir } from 'os';
|
||||||
import { listConfiguredAgentIds } from './agent-config';
|
import { listConfiguredAgentIds } from './agent-config';
|
||||||
|
import { getOpenClawResolvedDir } from './paths';
|
||||||
import {
|
import {
|
||||||
getProviderEnvVar,
|
getProviderEnvVar,
|
||||||
getProviderDefaultModel,
|
getProviderDefaultModel,
|
||||||
@@ -227,6 +228,53 @@ const AUTH_PROFILE_PROVIDER_KEY_MAP: Record<string, string> = {
|
|||||||
'google-gemini-cli': 'google',
|
'google-gemini-cli': 'google',
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Scan OpenClaw's bundled extensions directory to find all plugins that have
|
||||||
|
* `enabledByDefault: true` in their `openclaw.plugin.json` manifest.
|
||||||
|
*
|
||||||
|
* When `plugins.allow` is explicitly set (e.g. for third-party channel
|
||||||
|
* plugins), OpenClaw blocks ALL plugins not in the allowlist — even bundled
|
||||||
|
* ones with `enabledByDefault: true`. This function discovers those plugins
|
||||||
|
* so they can be preserved in the allowlist.
|
||||||
|
*
|
||||||
|
* Results are cached for the lifetime of the process since bundled
|
||||||
|
* extensions don't change at runtime.
|
||||||
|
*/
|
||||||
|
let _bundledPluginCache: { all: Set<string>; enabledByDefault: string[] } | null = null;
|
||||||
|
function discoverBundledPlugins(): { all: Set<string>; enabledByDefault: string[] } {
|
||||||
|
if (_bundledPluginCache) return _bundledPluginCache;
|
||||||
|
const all = new Set<string>();
|
||||||
|
const enabledByDefault: string[] = [];
|
||||||
|
try {
|
||||||
|
const extensionsDir = join(getOpenClawResolvedDir(), 'dist', 'extensions');
|
||||||
|
if (!existsSync(extensionsDir)) {
|
||||||
|
_bundledPluginCache = { all, enabledByDefault };
|
||||||
|
return _bundledPluginCache;
|
||||||
|
}
|
||||||
|
for (const entry of readdirSync(extensionsDir, { withFileTypes: true })) {
|
||||||
|
if (!entry.isDirectory()) continue;
|
||||||
|
const manifestPath = join(extensionsDir, entry.name, 'openclaw.plugin.json');
|
||||||
|
if (!existsSync(manifestPath)) continue;
|
||||||
|
try {
|
||||||
|
const manifest = JSON.parse(readFileSync(manifestPath, 'utf-8'));
|
||||||
|
if (typeof manifest.id === 'string') {
|
||||||
|
all.add(manifest.id);
|
||||||
|
if (manifest.enabledByDefault === true) {
|
||||||
|
enabledByDefault.push(manifest.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Malformed manifest — skip silently
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Extension directory not found or unreadable — return empty
|
||||||
|
}
|
||||||
|
_bundledPluginCache = { all, enabledByDefault };
|
||||||
|
return _bundledPluginCache;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
function normalizeAuthProfileProviderKey(provider: string): string {
|
function normalizeAuthProfileProviderKey(provider: string): string {
|
||||||
return AUTH_PROFILE_PROVIDER_KEY_MAP[provider] ?? provider;
|
return AUTH_PROFILE_PROVIDER_KEY_MAP[provider] ?? provider;
|
||||||
}
|
}
|
||||||
@@ -1574,7 +1622,15 @@ export async function sanitizeOpenClawConfig(): Promise<void> {
|
|||||||
modified = true;
|
modified = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
const externalPluginIds = allowArr2.filter((pluginId) => !BUILTIN_CHANNEL_IDS.has(pluginId));
|
// Discover all bundled extension IDs and which ones are enabledByDefault
|
||||||
|
// so we can (a) exclude them from the "external" set (prevents stale
|
||||||
|
// entries surviving across OpenClaw upgrades) and (b) re-add the
|
||||||
|
// enabledByDefault ones to prevent the allowlist from blocking them.
|
||||||
|
const bundled = discoverBundledPlugins();
|
||||||
|
|
||||||
|
const externalPluginIds = allowArr2.filter(
|
||||||
|
(pluginId) => !BUILTIN_CHANNEL_IDS.has(pluginId) && !bundled.all.has(pluginId),
|
||||||
|
);
|
||||||
let nextAllow = [...externalPluginIds];
|
let nextAllow = [...externalPluginIds];
|
||||||
if (externalPluginIds.length > 0) {
|
if (externalPluginIds.length > 0) {
|
||||||
for (const channelId of configuredBuiltIns) {
|
for (const channelId of configuredBuiltIns) {
|
||||||
@@ -1586,6 +1642,20 @@ export async function sanitizeOpenClawConfig(): Promise<void> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Ensure enabledByDefault built-in plugins survive restrictive allowlists ──
|
||||||
|
// OpenClaw's plugin enable logic checks the allowlist BEFORE enabledByDefault,
|
||||||
|
// so any bundled plugin with enabledByDefault: true (e.g. browser, diffs, etc.)
|
||||||
|
// gets blocked when plugins.allow is non-empty. We add them back here.
|
||||||
|
// On upgrade, plugins removed from enabledByDefault are also removed from the
|
||||||
|
// allowlist because they were excluded from externalPluginIds above.
|
||||||
|
if (nextAllow.length > 0) {
|
||||||
|
for (const pluginId of bundled.enabledByDefault) {
|
||||||
|
if (!nextAllow.includes(pluginId)) {
|
||||||
|
nextAllow.push(pluginId);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (JSON.stringify(nextAllow) !== JSON.stringify(allowArr2)) {
|
if (JSON.stringify(nextAllow) !== JSON.stringify(allowArr2)) {
|
||||||
if (nextAllow.length > 0) {
|
if (nextAllow.length > 0) {
|
||||||
pluginsObj.allow = nextAllow;
|
pluginsObj.allow = nextAllow;
|
||||||
|
|||||||
@@ -30,7 +30,10 @@ async function readConfig(): Promise<Record<string, unknown>> {
|
|||||||
* Standalone mirror of the sanitization logic in openclaw-auth.ts.
|
* Standalone mirror of the sanitization logic in openclaw-auth.ts.
|
||||||
* Uses the same blocklist approach as the production code.
|
* Uses the same blocklist approach as the production code.
|
||||||
*/
|
*/
|
||||||
async function sanitizeConfig(filePath: string): Promise<boolean> {
|
async function sanitizeConfig(
|
||||||
|
filePath: string,
|
||||||
|
bundledPlugins?: { all: string[]; enabledByDefault: string[] },
|
||||||
|
): Promise<boolean> {
|
||||||
let raw: string;
|
let raw: string;
|
||||||
try {
|
try {
|
||||||
raw = await readFile(filePath, 'utf-8');
|
raw = await readFile(filePath, 'utf-8');
|
||||||
@@ -143,7 +146,14 @@ async function sanitizeConfig(filePath: string): Promise<boolean> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const externalPluginIds = allow.filter((id) => !BUILTIN_CHANNEL_IDS.has(id));
|
// Mirror production logic: exclude both built-in channels AND bundled
|
||||||
|
// extension IDs from the "external" set, then re-add enabledByDefault ones.
|
||||||
|
const bundledAll = new Set(bundledPlugins?.all ?? []);
|
||||||
|
const bundledEnabledByDefault = bundledPlugins?.enabledByDefault ?? [];
|
||||||
|
|
||||||
|
const externalPluginIds = allow.filter(
|
||||||
|
(id) => !BUILTIN_CHANNEL_IDS.has(id) && !bundledAll.has(id),
|
||||||
|
);
|
||||||
const nextAllow = [...externalPluginIds];
|
const nextAllow = [...externalPluginIds];
|
||||||
if (externalPluginIds.length > 0) {
|
if (externalPluginIds.length > 0) {
|
||||||
for (const channelId of configuredBuiltIns) {
|
for (const channelId of configuredBuiltIns) {
|
||||||
@@ -153,6 +163,15 @@ async function sanitizeConfig(filePath: string): Promise<boolean> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Re-add enabledByDefault plugins when allowlist is non-empty
|
||||||
|
if (nextAllow.length > 0) {
|
||||||
|
for (const pluginId of bundledEnabledByDefault) {
|
||||||
|
if (!nextAllow.includes(pluginId)) {
|
||||||
|
nextAllow.push(pluginId);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (JSON.stringify(nextAllow) !== JSON.stringify(allow)) {
|
if (JSON.stringify(nextAllow) !== JSON.stringify(allow)) {
|
||||||
if (nextAllow.length > 0) {
|
if (nextAllow.length > 0) {
|
||||||
pluginsObj.allow = nextAllow;
|
pluginsObj.allow = nextAllow;
|
||||||
@@ -612,4 +631,117 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
|
|||||||
const modified = await sanitizeConfig(configPath);
|
const modified = await sanitizeConfig(configPath);
|
||||||
expect(modified).toBe(false);
|
expect(modified).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ── enabledByDefault bundled plugin allowlist tests ──────────────
|
||||||
|
|
||||||
|
it('adds enabledByDefault bundled plugins to plugins.allow when allowlist is non-empty', async () => {
|
||||||
|
await writeConfig({
|
||||||
|
plugins: {
|
||||||
|
allow: ['customPlugin'],
|
||||||
|
entries: { customPlugin: { enabled: true } },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const bundled = {
|
||||||
|
all: ['browser', 'openai', 'diffs'],
|
||||||
|
enabledByDefault: ['browser', 'openai'],
|
||||||
|
};
|
||||||
|
|
||||||
|
const modified = await sanitizeConfig(configPath, bundled);
|
||||||
|
expect(modified).toBe(true);
|
||||||
|
|
||||||
|
const result = await readConfig();
|
||||||
|
const plugins = result.plugins as Record<string, unknown>;
|
||||||
|
const allow = plugins.allow as string[];
|
||||||
|
expect(allow).toContain('customPlugin');
|
||||||
|
expect(allow).toContain('browser');
|
||||||
|
expect(allow).toContain('openai');
|
||||||
|
// 'diffs' is bundled but NOT enabledByDefault — should not be added
|
||||||
|
expect(allow).not.toContain('diffs');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('removes stale bundled plugin IDs from allowlist on upgrade', async () => {
|
||||||
|
// Simulate: previous version had 'old-bundled' as enabledByDefault,
|
||||||
|
// new version still has it bundled but no longer enabledByDefault.
|
||||||
|
// Also 'unknown-plugin' is not in bundled.all — it could be a
|
||||||
|
// user-installed third-party plugin, so it must be preserved.
|
||||||
|
await writeConfig({
|
||||||
|
plugins: {
|
||||||
|
allow: ['customPlugin', 'unknown-plugin', 'old-bundled', 'browser'],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const bundled = {
|
||||||
|
all: ['browser', 'openai', 'old-bundled'], // old-bundled still bundled
|
||||||
|
enabledByDefault: ['browser', 'openai'], // but no longer enabledByDefault
|
||||||
|
};
|
||||||
|
|
||||||
|
const modified = await sanitizeConfig(configPath, bundled);
|
||||||
|
expect(modified).toBe(true);
|
||||||
|
|
||||||
|
const result = await readConfig();
|
||||||
|
const plugins = result.plugins as Record<string, unknown>;
|
||||||
|
const allow = plugins.allow as string[];
|
||||||
|
expect(allow).toContain('customPlugin'); // external — preserved
|
||||||
|
expect(allow).toContain('unknown-plugin'); // not bundled — treated as external, preserved
|
||||||
|
expect(allow).toContain('browser'); // still enabledByDefault
|
||||||
|
expect(allow).toContain('openai'); // newly added enabledByDefault
|
||||||
|
expect(allow).not.toContain('old-bundled'); // bundled but demoted — removed
|
||||||
|
});
|
||||||
|
|
||||||
|
it('removes demoted bundled plugin from allowlist when no longer enabledByDefault', async () => {
|
||||||
|
// Simulate: 'diffs' was enabledByDefault in v1, demoted to opt-in in v2
|
||||||
|
await writeConfig({
|
||||||
|
plugins: {
|
||||||
|
allow: ['customPlugin', 'diffs', 'browser'],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const bundled = {
|
||||||
|
all: ['browser', 'diffs', 'openai'],
|
||||||
|
enabledByDefault: ['browser', 'openai'], // diffs no longer enabledByDefault
|
||||||
|
};
|
||||||
|
|
||||||
|
const modified = await sanitizeConfig(configPath, bundled);
|
||||||
|
expect(modified).toBe(true);
|
||||||
|
|
||||||
|
const result = await readConfig();
|
||||||
|
const plugins = result.plugins as Record<string, unknown>;
|
||||||
|
const allow = plugins.allow as string[];
|
||||||
|
expect(allow).toContain('customPlugin');
|
||||||
|
expect(allow).toContain('browser');
|
||||||
|
expect(allow).toContain('openai');
|
||||||
|
expect(allow).not.toContain('diffs'); // demoted — removed
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not add enabledByDefault plugins when allowlist is empty (no external plugins)', async () => {
|
||||||
|
// When no external plugins exist, allowlist should be dropped entirely
|
||||||
|
await writeConfig({
|
||||||
|
plugins: {
|
||||||
|
allow: ['whatsapp'], // built-in channel only
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const bundled = {
|
||||||
|
all: ['browser', 'openai'],
|
||||||
|
enabledByDefault: ['browser', 'openai'],
|
||||||
|
};
|
||||||
|
|
||||||
|
const modified = await sanitizeConfig(configPath, bundled);
|
||||||
|
expect(modified).toBe(true);
|
||||||
|
|
||||||
|
const result = await readConfig();
|
||||||
|
// plugins.allow should be removed (only built-in, no external plugins)
|
||||||
|
expect(result.plugins).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not modify config when no bundled plugins and no allowlist', async () => {
|
||||||
|
const original = {
|
||||||
|
gateway: { mode: 'local' },
|
||||||
|
};
|
||||||
|
await writeConfig(original);
|
||||||
|
|
||||||
|
const modified = await sanitizeConfig(configPath, { all: ['browser'], enabledByDefault: ['browser'] });
|
||||||
|
expect(modified).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user