mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-16 13:15:36 +02:00
fix(qwen): reject unrepresentable empty tool authority before writes
This commit is contained in:
@@ -61,3 +61,15 @@ Canonical agent files stay unchanged. The standalone `scripts/qwen-adapt-agents.
|
||||
can adapt an explicitly selected, user-owned copy; use the managed installer for ECC-owned files
|
||||
to keep repair and uninstall receipts consistent. Native subagent dispatch still requires
|
||||
verification in the user's configured Qwen runtime.
|
||||
|
||||
Qwen Code 0.25.0 treats omitted, null, and empty `tools` fields in named-agent
|
||||
files as inheriting the parent tool registry. An empty definition list differs
|
||||
from the internal runtime `ToolConfig` and `fork_tools` deny-all contracts.
|
||||
See the [official named-agent conversion](https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/subagent-manager.ts#L1661-L1685)
|
||||
and [empty-list validation](https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/validation.ts#L216-L226).
|
||||
ECC rejects an explicitly empty or null source tools field before managed
|
||||
installation writes or standalone batch rewrites, because this adapter cannot
|
||||
represent a deny-all agent through that native field. Use a nonempty explicit
|
||||
allowlist, or omit the field only when parent-tool inheritance is intended.
|
||||
These checks verify adaptation and write prevention; they do not certify native
|
||||
provider execution or sandbox containment.
|
||||
|
||||
@@ -676,7 +676,20 @@ function prepareHookConsentMigration(plan, migration) {
|
||||
};
|
||||
}
|
||||
|
||||
function preflightQwenAgentOperations(plan) {
|
||||
for (const operation of plan.operations || []) {
|
||||
if (operation.kind === 'copy-file' && operation.contentTransform === 'qwen-agent-frontmatter') {
|
||||
assertSafeInstallOperation(plan, operation);
|
||||
if (!fs.lstatSync(operation.sourcePath).isFile()) {
|
||||
throw new Error('Refusing Qwen agent adaptation from a non-regular source file');
|
||||
}
|
||||
transformInstallContent(operation, fs.readFileSync(operation.sourcePath, 'utf8'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function previewInstallPlan(plan) {
|
||||
preflightQwenAgentOperations(plan);
|
||||
assertOpenCodeHookDeactivationReady(plan);
|
||||
const migration = prepareStaleOperationsReconciliation(
|
||||
plan,
|
||||
@@ -736,6 +749,7 @@ function applyInstallPlan(plan, dependencies = {}) {
|
||||
}
|
||||
|
||||
function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) {
|
||||
preflightQwenAgentOperations(plan);
|
||||
const persistInstallState = dependencies.writeInstallState || writeInstallState;
|
||||
const beforeInstallStateRead = dependencies.beforeInstallStateRead;
|
||||
const beforeOperationWrite = dependencies.beforeOperationWrite;
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const yaml = require('js-yaml');
|
||||
const { normalizeAgentTools } = require('./lib/agent-tools');
|
||||
|
||||
// Qwen Code is a Gemini CLI descendant, so most tool ids match Gemini's. Three
|
||||
@@ -90,15 +91,6 @@ function ensureDirectory(dirPath) {
|
||||
}
|
||||
}
|
||||
|
||||
function parseToolList(line) {
|
||||
const match = line.match(/^\s*tools\s*:\s*(.*)$/);
|
||||
if (!match) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return normalizeAgentTools(match[1]);
|
||||
}
|
||||
|
||||
function adaptToolName(toolName) {
|
||||
const mapped = TOOL_NAME_MAP.get(toolName);
|
||||
if (mapped) {
|
||||
@@ -113,9 +105,7 @@ function formatToolLine(tools) {
|
||||
// Comma-separated scalar rather than the JSON flow sequence
|
||||
// gemini-adapt-agents.js emits: Qwen Code parses both, and the scalar form is
|
||||
// what its own agent examples use.
|
||||
// YAML null can discard an optional allowlist and inherit parent tools.
|
||||
// Keep an explicitly empty source list as an empty native list.
|
||||
return tools.length ? `tools: ${tools.join(', ')}` : 'tools: []';
|
||||
return `tools: ${tools.join(', ')}`;
|
||||
}
|
||||
|
||||
function adaptColor(line) {
|
||||
@@ -149,10 +139,31 @@ function adaptFrontmatter(text) {
|
||||
return { text, changed: false };
|
||||
}
|
||||
|
||||
const firstMappingLine = match[1].split('\n').find(line => line.trim() && !line.trimStart().startsWith('#'));
|
||||
if (firstMappingLine && /^\s/.test(firstMappingLine)) {
|
||||
throw new Error('Unsupported indented Qwen agent frontmatter: use unindented root mapping keys');
|
||||
}
|
||||
|
||||
const frontmatter = yaml.load(match[1]);
|
||||
const hasTools = frontmatter && Object.hasOwn(frontmatter, 'tools');
|
||||
const sourceTools = hasTools ? normalizeAgentTools(frontmatter.tools) : null;
|
||||
// Qwen 0.25.0 definition files interpret [] as inherit-all, unlike its
|
||||
// internal runtime ToolConfig. Reject rather than broaden source authority.
|
||||
// https://github.com/QwenLM/qwen-code/blob/v0.25.0/packages/core/src/subagents/subagent-manager.ts#L1661-L1685
|
||||
if (hasTools && sourceTools.length === 0) {
|
||||
throw new Error('Unsupported empty tools allowlist: Qwen Code inherits all tools for an empty agent list; omit tools only when inheritance is intended');
|
||||
}
|
||||
|
||||
if (hasTools && !/^tools\s*:/m.test(match[1])) {
|
||||
throw new Error('Unsupported Qwen tools key format: use an unquoted root tools key');
|
||||
}
|
||||
|
||||
let changed = false;
|
||||
const updatedLines = [];
|
||||
|
||||
for (const line of match[1].split('\n')) {
|
||||
const lines = match[1].split('\n');
|
||||
for (let index = 0; index < lines.length; index += 1) {
|
||||
const line = lines[index];
|
||||
// Claude tiers are not Qwen provider IDs; inherit the configured session.
|
||||
if (/^model:\s*["']?(haiku|sonnet|opus)["']?\s*$/.test(line)) {
|
||||
updatedLines.push('model: inherit');
|
||||
@@ -170,12 +181,16 @@ function adaptFrontmatter(text) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const tools = parseToolList(line);
|
||||
if (tools) {
|
||||
if (/^tools\s*:/.test(line) && hasTools) {
|
||||
// Consume indented YAML continuations before writing the native scalar.
|
||||
while (index + 1 < lines.length && /^(?:\s|#|$)/.test(lines[index + 1])) {
|
||||
index += 1;
|
||||
changed = true;
|
||||
}
|
||||
const adaptedTools = [];
|
||||
const seen = new Set();
|
||||
|
||||
for (const tool of tools.map(adaptToolName)) {
|
||||
for (const tool of sourceTools.map(adaptToolName)) {
|
||||
if (!tool || seen.has(tool)) {
|
||||
continue;
|
||||
}
|
||||
@@ -212,15 +227,25 @@ function adaptAgents(dirPath) {
|
||||
let updated = 0;
|
||||
let unchanged = 0;
|
||||
|
||||
for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) {
|
||||
const prepared = [];
|
||||
// Validate the entire batch before rewriting any installed agent.
|
||||
for (const entry of fs.readdirSync(dirPath, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) {
|
||||
if (!entry.isFile() || !entry.name.endsWith('.md')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const filePath = path.join(dirPath, entry.name);
|
||||
const original = fs.readFileSync(filePath, 'utf8');
|
||||
const adapted = adaptFrontmatter(original);
|
||||
let adapted;
|
||||
try {
|
||||
adapted = adaptFrontmatter(original);
|
||||
} catch (error) {
|
||||
throw new Error(`${entry.name}: ${error.message}`);
|
||||
}
|
||||
prepared.push({ filePath, adapted });
|
||||
}
|
||||
|
||||
for (const { filePath, adapted } of prepared) {
|
||||
if (adapted.changed) {
|
||||
fs.writeFileSync(filePath, adapted.text);
|
||||
updated += 1;
|
||||
|
||||
@@ -366,28 +366,88 @@ function runTests() {
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('preserves an explicit empty allowlist through actual CLI adaptation and rerun', () => {
|
||||
if (test('rejects an empty allowlist before writing any agent in the CLI batch', () => {
|
||||
const tempDir = createTempDir();
|
||||
const agentsDir = path.join(tempDir, '.qwen', 'agents');
|
||||
const yaml = require('js-yaml');
|
||||
const frontmatter = text => yaml.load(text.match(/^---\n([\s\S]*?)\n---/)[1]);
|
||||
try {
|
||||
writeAgent(agentsDir, 'no-tools.md', [
|
||||
'---', 'name: no-tools', 'description: No tools permitted',
|
||||
'tools: []', 'model: sonnet', '---', '', 'BODY_SENTINEL'
|
||||
].join('\n'));
|
||||
const first = run([agentsDir]);
|
||||
assert.strictEqual(first.code, 0, first.stderr);
|
||||
assert.ok(first.stdout.includes('Updated 1 agent file(s)'));
|
||||
const adapted = readAgent(agentsDir, 'no-tools.md');
|
||||
assert.deepStrictEqual(frontmatter(adapted).tools, []);
|
||||
assert.strictEqual(frontmatter(adapted).model, 'inherit');
|
||||
assert.ok(adapted.endsWith('BODY_SENTINEL'));
|
||||
const second = run([agentsDir]);
|
||||
assert.strictEqual(second.code, 0, second.stderr);
|
||||
assert.ok(second.stdout.includes('Updated 0 agent file(s)'));
|
||||
assert.strictEqual(readAgent(agentsDir, 'no-tools.md'), adapted);
|
||||
assert.deepStrictEqual(frontmatter(readAgent(agentsDir, 'no-tools.md')).tools, []);
|
||||
for (const emptyTools of ['[]', '[] # no tools', '', 'null', '""']) {
|
||||
writeAgent(agentsDir, 'a-valid.md', ['---', 'name: valid', 'tools: Read', 'model: sonnet', '---', 'VALID_BODY'].join('\n'));
|
||||
writeAgent(agentsDir, 'z-no-tools.md', ['---', 'name: no-tools', `tools: ${emptyTools}`, 'model: sonnet', '---', 'NO_TOOLS_BODY'].join('\n'));
|
||||
writeAgent(agentsDir, 'ignored.txt', 'UNCHANGED');
|
||||
const snapshot = () => fs.readdirSync(agentsDir).sort().map(name => [name, fs.readFileSync(path.join(agentsDir, name)).toString('base64')]);
|
||||
const before = snapshot();
|
||||
for (let attempt = 0; attempt < 2; attempt++) {
|
||||
const result = run([agentsDir]);
|
||||
assert.strictEqual(result.code, 1, result.stdout);
|
||||
assert.match(result.stderr, /z-no-tools.md: Unsupported empty tools allowlist/);
|
||||
assert.match(result.stderr, /inherits all tools/);
|
||||
assert.deepStrictEqual(snapshot(), before);
|
||||
}
|
||||
}
|
||||
} finally { cleanupTempDir(tempDir); }
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('adapts a nonempty YAML block list without leftover tool entries', () => {
|
||||
const { adaptFrontmatter } = require(SCRIPT);
|
||||
const yaml = require('js-yaml');
|
||||
const result = adaptFrontmatter(['---', 'name: block-tools', 'description: Neighbor metadata', 'tools:', ' - Read', '', '# Tool list comment', ' - Grep', 'model: sonnet', 'metadata:', ' owner: operator', '---', 'BODY'].join('\n'));
|
||||
const parsed = yaml.load(result.text.match(/^---\n([\s\S]*?)\n---/)[1]);
|
||||
assert.strictEqual(parsed.tools, 'read_file, grep_search');
|
||||
assert.strictEqual(parsed.model, 'inherit');
|
||||
assert.strictEqual(parsed.description, 'Neighbor metadata');
|
||||
assert.deepStrictEqual(parsed.metadata, { owner: 'operator' });
|
||||
assert.strictEqual(adaptFrontmatter(result.text).changed, false);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('rejects uniformly indented mappings without changing neighboring metadata', () => {
|
||||
const tempDir = createTempDir();
|
||||
try {
|
||||
const source = ['---', ' name: indented', ' description: Preserve me', ' tools:', ' - Read', ' - Grep', ' model: sonnet', '---', 'BODY'].join('\n');
|
||||
writeAgent(tempDir, 'indented.md', source);
|
||||
for (let attempt = 0; attempt < 2; attempt++) {
|
||||
const result = run([tempDir]);
|
||||
assert.strictEqual(result.code, 1);
|
||||
assert.match(result.stderr, /Unsupported indented Qwen agent frontmatter/);
|
||||
assert.strictEqual(readAgent(tempDir, 'indented.md'), source);
|
||||
}
|
||||
} finally { cleanupTempDir(tempDir); }
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('preflights every managed Qwen transform before file or receipt writes', () => {
|
||||
const { applyInstallPlan, previewInstallPlan } = require('../../scripts/lib/install/apply');
|
||||
const tempDir = createTempDir();
|
||||
const targetRoot = path.join(tempDir, 'installed');
|
||||
const sourceRoot = path.join(tempDir, 'source');
|
||||
try {
|
||||
fs.mkdirSync(targetRoot);
|
||||
fs.mkdirSync(sourceRoot);
|
||||
const valid = ['---', 'name: valid', 'tools: Read', '---', 'BODY'].join('\n');
|
||||
const invalid = ['---', 'name: invalid', 'tools: []', '---', 'BODY'].join('\n');
|
||||
writeAgent(sourceRoot, 'a-valid.md', valid);
|
||||
writeAgent(sourceRoot, 'z-invalid.md', invalid);
|
||||
writeAgent(targetRoot, 'z-invalid.md', 'OPERATOR_OWNED_BYTES');
|
||||
writeAgent(targetRoot, 'ecc-install-state.json', '{"preserve":"receipt"}');
|
||||
const plan = {
|
||||
adapter: { id: 'qwen-home', target: 'qwen' },
|
||||
targetRoot,
|
||||
installStatePath: path.join(targetRoot, 'ecc-install-state.json'),
|
||||
operations: ['a-valid.md', 'z-invalid.md'].map(name => ({
|
||||
kind: 'copy-file', sourcePath: path.join(sourceRoot, name),
|
||||
sourceRelativePath: `agents/${name}`,
|
||||
destinationPath: path.join(targetRoot, name),
|
||||
contentTransform: 'qwen-agent-frontmatter',
|
||||
})),
|
||||
};
|
||||
const snapshot = () => fs.readdirSync(targetRoot).sort().map(name => [name, fs.readFileSync(path.join(targetRoot, name)).toString('base64')]);
|
||||
const before = snapshot();
|
||||
for (const operation of [previewInstallPlan, applyInstallPlan]) {
|
||||
assert.throws(() => operation(plan), /Unsupported empty tools allowlist/);
|
||||
assert.deepStrictEqual(snapshot(), before);
|
||||
}
|
||||
fs.unlinkSync(path.join(sourceRoot, 'z-invalid.md'));
|
||||
fs.mkdirSync(path.join(sourceRoot, 'z-invalid.md'));
|
||||
assert.throws(() => applyInstallPlan(plan), /non-regular source file/);
|
||||
assert.deepStrictEqual(snapshot(), before);
|
||||
} finally { cleanupTempDir(tempDir); }
|
||||
})) passed++; else failed++;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user