fix: deny built-in skill workshop tool (#1131)

This commit is contained in:
Felix
2026-06-23 13:30:10 +08:00
committed by GitHub
parent d223af5747
commit ffe50ff8a5
3 changed files with 129 additions and 18 deletions
+15
View File
@@ -2916,6 +2916,21 @@ export async function sanitizeOpenClawConfig(): Promise<void> {
toolsModified = true;
}
// OpenClaw 6.5 moved Skill Workshop into the core skills surface.
// ClawX does not expose that durable-skill proposal flow yet, so keep the
// built-in tool denied even under tools.profile="full".
const deny = Array.isArray(toolsConfig.deny)
? toolsConfig.deny.filter((value): value is string => typeof value === 'string')
: [];
if (!deny.includes('skill_workshop')) {
toolsConfig.deny = [...deny, 'skill_workshop'];
toolsModified = true;
console.log('[sanitize] Added "skill_workshop" to tools.deny for ClawX desktop');
} else if (!Array.isArray(toolsConfig.deny) || toolsConfig.deny.length !== deny.length) {
toolsConfig.deny = deny;
toolsModified = true;
}
// ── tools.exec approvals (OpenClaw 3.28+) ──────────────────────
// ClawX is a local desktop app where the user is the trusted operator.
// Exec approval prompts add unnecessary friction in this context, so we
+17
View File
@@ -377,6 +377,7 @@ describe('sanitizeOpenClawConfig', () => {
// Fresh install should get tools settings enforced
const tools = result.tools as Record<string, unknown>;
expect(tools.profile).toBe('full');
expect(tools.deny).toEqual(['skill_workshop']);
logSpy.mockRestore();
});
@@ -405,10 +406,26 @@ describe('sanitizeOpenClawConfig', () => {
// tools settings should now be enforced
const tools = result.tools as Record<string, unknown>;
expect(tools.profile).toBe('full');
expect(tools.deny).toEqual(['skill_workshop']);
logSpy.mockRestore();
});
it('preserves existing denied tools while adding skill_workshop to the deny list', async () => {
await writeOpenClawJson({
tools: {
deny: ['browser'],
},
});
const { sanitizeOpenClawConfig } = await import('@electron/utils/openclaw-auth');
await sanitizeOpenClawConfig();
const result = await readOpenClawJson();
const tools = result.tools as Record<string, unknown>;
expect(tools.deny).toEqual(['browser', 'skill_workshop']);
});
it('migrates legacy tools.web.search.kimi into moonshot plugin config', async () => {
await writeOpenClawJson({
models: {
+97 -18
View File
@@ -26,6 +26,34 @@ async function readConfig(): Promise<Record<string, unknown>> {
return JSON.parse(raw);
}
function withClawXToolDefaults<T extends Record<string, unknown>>(config: T): T & { tools: Record<string, unknown> } {
const tools = (config.tools && typeof config.tools === 'object' && !Array.isArray(config.tools))
? { ...(config.tools as Record<string, unknown>) }
: {};
const sessions = (tools.sessions && typeof tools.sessions === 'object' && !Array.isArray(tools.sessions))
? { ...(tools.sessions as Record<string, unknown>) }
: {};
const exec = (tools.exec && typeof tools.exec === 'object' && !Array.isArray(tools.exec))
? { ...(tools.exec as Record<string, unknown>) }
: {};
const deny = Array.isArray(tools.deny)
? (tools.deny as unknown[]).filter((value): value is string => typeof value === 'string')
: [];
sessions.visibility = 'all';
exec.security = 'full';
exec.ask = 'off';
tools.profile = 'full';
tools.sessions = sessions;
tools.exec = exec;
tools.deny = deny.includes('skill_workshop') ? deny : [...deny, 'skill_workshop'];
return {
...config,
tools,
};
}
/**
* Standalone mirror of the sanitization logic in openclaw-auth.ts.
* Uses the same blocklist approach as the production code.
@@ -307,6 +335,47 @@ async function sanitizeConfig(
}
}
// Mirror: ClawX keeps Skill Workshop disabled even when OpenClaw exposes it
// as a built-in tool under permissive tool profiles.
const toolsConfig = (config.tools as Record<string, unknown> | undefined) || {};
let toolsModified = false;
if (toolsConfig.profile !== 'full') {
toolsConfig.profile = 'full';
toolsModified = true;
}
const sessions = (toolsConfig.sessions as Record<string, unknown> | undefined) || {};
if (sessions.visibility !== 'all') {
sessions.visibility = 'all';
toolsConfig.sessions = sessions;
toolsModified = true;
}
const deny = Array.isArray(toolsConfig.deny)
? toolsConfig.deny.filter((value): value is string => typeof value === 'string')
: [];
if (!deny.includes('skill_workshop')) {
toolsConfig.deny = [...deny, 'skill_workshop'];
toolsModified = true;
} else if (!Array.isArray(toolsConfig.deny) || toolsConfig.deny.length !== deny.length) {
toolsConfig.deny = deny;
toolsModified = true;
}
const execConfig = (toolsConfig.exec as Record<string, unknown> | undefined) || {};
if (execConfig.security !== 'full' || execConfig.ask !== 'off') {
execConfig.security = 'full';
execConfig.ask = 'off';
toolsConfig.exec = execConfig;
toolsModified = true;
}
if (toolsModified) {
config.tools = toolsConfig;
modified = true;
}
// Mirror: remove stale tools.web.search.kimi.apiKey when moonshot provider exists.
const providers = ((config.models as Record<string, unknown> | undefined)?.providers as Record<string, unknown> | undefined) || {};
if (providers.moonshot) {
@@ -431,12 +500,12 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('does nothing when config is already valid', async () => {
const original = {
const original = withClawXToolDefaults({
skills: {
entries: { 'my-skill': { enabled: true } },
allowBundled: ['web-search'],
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -449,7 +518,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
it('preserves unknown valid keys (forward-compatible)', async () => {
// If OpenClaw adds new valid keys to skills in the future,
// the blocklist approach should NOT strip them.
const original = {
const original = withClawXToolDefaults({
skills: {
entries: { 'x': { enabled: true } },
allowBundled: ['web-search'],
@@ -458,7 +527,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
limits: { maxSkillsInPrompt: 5 },
futureNewKey: { some: 'value' }, // hypothetical future key
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -473,14 +542,20 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
expect(modified).toBe(false);
expect(modified).toBe(true);
const result = await readConfig();
expect(result).toEqual(withClawXToolDefaults(original));
});
it('handles empty config', async () => {
await writeConfig({});
const modified = await sanitizeConfig(configPath);
expect(modified).toBe(false);
expect(modified).toBe(true);
const result = await readConfig();
expect(result).toEqual(withClawXToolDefaults({}));
});
it('returns false for missing config file', async () => {
@@ -490,10 +565,14 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
it('handles skills being an array (no-op, no crash)', async () => {
// Edge case: skills is not an object
await writeConfig({ skills: ['something'] });
const original = { skills: ['something'] };
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
expect(modified).toBe(false);
expect(modified).toBe(true);
const result = await readConfig();
expect(result).toEqual(withClawXToolDefaults(original));
});
it('preserves all other top-level config sections', async () => {
@@ -583,7 +662,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('keeps tools.web.search.kimi.apiKey when moonshot provider is absent', async () => {
const original = {
const original = withClawXToolDefaults({
models: {
providers: {
openrouter: { baseUrl: 'https://openrouter.ai/api/v1', api: 'openai-completions' },
@@ -598,7 +677,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
},
},
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -776,7 +855,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('does nothing when plugins.load.paths contains only valid paths', async () => {
const original = {
const original = withClawXToolDefaults({
plugins: {
load: {
paths: [tempDir],
@@ -784,7 +863,7 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
},
entries: { test: { enabled: true } },
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -818,11 +897,11 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('handles plugins.load as empty object (no paths key)', async () => {
const original = {
const original = withClawXToolDefaults({
plugins: {
load: {},
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -830,11 +909,11 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('handles plugins.load.paths as empty array', async () => {
const original = {
const original = withClawXToolDefaults({
plugins: {
load: { paths: [] },
},
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath);
@@ -951,9 +1030,9 @@ describe('sanitizeOpenClawConfig (blocklist approach)', () => {
});
it('does not modify config when no bundled plugins and no allowlist', async () => {
const original = {
const original = withClawXToolDefaults({
gateway: { mode: 'local' },
};
});
await writeConfig(original);
const modified = await sanitizeConfig(configPath, { all: ['browser'], enabledByDefault: ['browser'] });