basant307/AI_Governance_Project

1/**2 * @license3 * Copyright 2026 Qwen Team4 * SPDX-License-Identifier: Apache-2.05 */6 7/**8 * Tests for the #4437 fix:9 *  - `write_file` to an existing path inside the project skills root is10 *    denied (was 'allow' before — silently clobbered the prior SKILL.md).11 *  - `edit` semantics for existing auto-skills are preserved.12 *  - `buildTaskPrompt` enumerates existing skill directory names so the13 *    agent picks a fresh name on the first attempt.14 */15 16import * as fs from 'node:fs/promises';17import * as os from 'node:os';18import * as path from 'node:path';19import { afterEach, beforeEach, describe, expect, it } from 'vitest';20import type { Config } from '../config/config.js';21import {22  AUTO_SKILL_DIR_PREFIX,23  buildTaskPrompt,24  createSkillScopedAgentConfig,25  listExistingSkillDirNames,26  SKILL_REVIEW_SYSTEM_PROMPT,27} from './skillReviewAgentPlanner.js';28import { ToolNames } from '../tools/tool-names.js';29 30function makeMinimalConfig(projectRoot: string): Config {31  return {32    getProjectRoot: () => projectRoot,33    getPermissionManager: () => undefined,34  } as unknown as Config;35}36 37/**38 * Build the scoped Config and return its non-null PermissionManager.39 * `createSkillScopedAgentConfig` always installs one, but Config's40 * declared `getPermissionManager(): PermissionManager | null` forces41 * tests to launder the null at the call site — this helper does it42 * once with an assertion that fires loudly if the contract ever breaks.43 */44function scopedPm(projectRoot: string) {45  const scoped = createSkillScopedAgentConfig(46    makeMinimalConfig(projectRoot),47    projectRoot,48  );49  const pm = scoped.getPermissionManager();50  if (!pm) {51    throw new Error(52      'createSkillScopedAgentConfig must install a PermissionManager',53    );54  }55  return pm;56}57 58async function writeSkillFile(59  projectRoot: string,60  skillName: string,61  content: string,62): Promise<string> {63  const dir = path.join(projectRoot, '.qwen', 'skills', skillName);64  await fs.mkdir(dir, { recursive: true });65  const filePath = path.join(dir, 'SKILL.md');66  await fs.writeFile(filePath, content, 'utf-8');67  return filePath;68}69 70const AUTO_SKILL = `---71name: my-skill72source: auto-skill73---74 75body76`;77 78const USER_SKILL = `---79name: my-skill80description: hand-authored81---82 83human body84`;85 86describe('skillReviewAgentPlanner — write_file collision deny (#4437)', () => {87  let tempDir: string;88  let projectRoot: string;89 90  beforeEach(async () => {91    tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-review-v2-'));92    projectRoot = path.join(tempDir, 'project');93    await fs.mkdir(projectRoot, { recursive: true });94  });95 96  afterEach(async () => {97    await fs.rm(tempDir, { recursive: true, force: true });98  });99 100  it("denies write_file to an existing AUTO-skill path (the #4437 bug — was 'allow')", async () => {101    const filePath = await writeSkillFile(projectRoot, 'my-skill', AUTO_SKILL);102    const pm = scopedPm(projectRoot);103 104    const decision = await pm.evaluate({105      toolName: ToolNames.WRITE_FILE,106      filePath,107    });108    expect(decision).toBe('deny');109  });110 111  it('denies write_file to an existing USER-skill path (already worked — kept as regression guard)', async () => {112    const filePath = await writeSkillFile(projectRoot, 'my-skill', USER_SKILL);113    const pm = scopedPm(projectRoot);114 115    const decision = await pm.evaluate({116      toolName: ToolNames.WRITE_FILE,117      filePath,118    });119    expect(decision).toBe('deny');120  });121 122  it('allows write_file to a fresh path that does not yet exist', async () => {123    const fresh = path.join(124      projectRoot,125      '.qwen',126      'skills',127      'brand-new',128      'SKILL.md',129    );130    const pm = scopedPm(projectRoot);131 132    const decision = await pm.evaluate({133      toolName: ToolNames.WRITE_FILE,134      filePath: fresh,135    });136    expect(decision).toBe('allow');137  });138 139  it('still allows edit on an existing auto-skill (update path preserved)', async () => {140    const filePath = await writeSkillFile(projectRoot, 'my-skill', AUTO_SKILL);141    const pm = scopedPm(projectRoot);142 143    const decision = await pm.evaluate({144      toolName: ToolNames.EDIT,145      filePath,146    });147    expect(decision).toBe('allow');148  });149 150  it('still denies edit on a user skill (update path safety preserved)', async () => {151    const filePath = await writeSkillFile(projectRoot, 'my-skill', USER_SKILL);152    const pm = scopedPm(projectRoot);153 154    const decision = await pm.evaluate({155      toolName: ToolNames.EDIT,156      filePath,157    });158    expect(decision).toBe('deny');159  });160 161  it('write_file deny rule message points the agent at a fresh name', async () => {162    const filePath = await writeSkillFile(projectRoot, 'my-skill', AUTO_SKILL);163    const pm = scopedPm(projectRoot);164 165    const rule = pm.findMatchingDenyRule({166      toolName: ToolNames.WRITE_FILE,167      filePath,168    });169    expect(rule).toMatch(/<name>-2/);170    expect(rule).toMatch(/edit/);171  });172 173  it('denies write_file to a path outside the project skills root', async () => {174    // Security-boundary regression guard for the `isProjectSkillPath`175    // false branch — without it the agent could escape to anywhere176    // reachable from CWD.177    const escape = path.join(projectRoot, 'NOT-SKILLS', 'evil.md');178    const pm = scopedPm(projectRoot);179    expect(180      await pm.evaluate({181        toolName: ToolNames.WRITE_FILE,182        filePath: escape,183      }),184    ).toBe('deny');185  });186 187  it('denies write_file to a non-SKILL.md path inside the skills root', async () => {188    // Auxiliary files (NOTES.md, attachments) must not land in the189    // skills dir — SkillManager would ignore them but they'd still190    // pollute the layout. Tightening the basename invariant is the191    // hard guard for that.192    const aux = path.join(193      projectRoot,194      '.qwen',195      'skills',196      'my-skill',197      'NOTES.md',198    );199    await fs.mkdir(path.dirname(aux), { recursive: true });200    const pm = scopedPm(projectRoot);201    expect(202      await pm.evaluate({203        toolName: ToolNames.WRITE_FILE,204        filePath: aux,205      }),206    ).toBe('deny');207  });208 209  it('denies write_file when the target traverses a symlink outside the skills root', async () => {210    // Symlink-escape regression guard for the `assertRealProjectSkillPath`211    // catch. A skill dir that's actually a symlink to /tmp would let the212    // agent write outside the project; the realpath check stops it.213    const outside = path.join(tempDir, 'outside');214    await fs.mkdir(outside, { recursive: true });215    const skillsRoot = path.join(projectRoot, '.qwen', 'skills');216    await fs.mkdir(skillsRoot, { recursive: true });217    await fs.symlink(outside, path.join(skillsRoot, 'escape'));218    const target = path.join(skillsRoot, 'escape', 'SKILL.md');219    const pm = scopedPm(projectRoot);220    expect(221      await pm.evaluate({222        toolName: ToolNames.WRITE_FILE,223        filePath: target,224      }),225    ).toBe('deny');226  });227 228  it('denies write_file when the target path is a directory, not a file', async () => {229    // `fs.stat` on a directory SUCCEEDS (returning stats with230    // `isDirectory: true`); it does not throw EISDIR. So this exercise231    // path A in evaluateScopedDecision — `try { await fs.stat(); return232    // 'deny'; }` — i.e. "target exists" rather than the non-ENOENT233    // catch. WriteFileTool would later fail with EISDIR on the actual234    // write, but the permission layer catches it earlier here.235    const dirAsFile = path.join(236      projectRoot,237      '.qwen',238      'skills',239      'is-a-directory',240      'SKILL.md',241    );242    await fs.mkdir(dirAsFile, { recursive: true });243    const pm = scopedPm(projectRoot);244    expect(245      await pm.evaluate({246        toolName: ToolNames.WRITE_FILE,247        filePath: dirAsFile,248      }),249    ).toBe('deny');250  });251 252  // Note on coverage of the `fs.stat` catch branch in253  // evaluateScopedDecision:254  // The branch is defense-in-depth — anything that would make `fs.stat`255  // throw a non-ENOENT error (EACCES, ELOOP, ENAMETOOLONG, EIO) also256  // throws from `assertRealProjectSkillPath`'s `realpath`/`lstat` one257  // step earlier, which is exercised by the symlink-traversal test258  // above. Spying on `fs.stat` from ESM tests is blocked259  // (https://vitest.dev/guide/browser/#limitations), and chmod-based260  // reproductions of EACCES are non-portable to Windows CI. The deny261  // contract is straightforward enough that the structural duplication262  // here is acceptable.263});264 265describe('listExistingSkillDirNames', () => {266  let tempDir: string;267  let projectRoot: string;268 269  beforeEach(async () => {270    tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-list-'));271    projectRoot = path.join(tempDir, 'project');272    await fs.mkdir(projectRoot, { recursive: true });273  });274 275  afterEach(async () => {276    await fs.rm(tempDir, { recursive: true, force: true });277  });278 279  it('returns sorted directory names that contain a SKILL.md', async () => {280    await writeSkillFile(projectRoot, 'zebra', AUTO_SKILL);281    await writeSkillFile(projectRoot, 'apple', AUTO_SKILL);282    expect(await listExistingSkillDirNames(projectRoot)).toEqual([283      'apple',284      'zebra',285    ]);286  });287 288  it('skips directories without SKILL.md so half-built dirs do not reserve names', async () => {289    await writeSkillFile(projectRoot, 'real', AUTO_SKILL);290    await fs.mkdir(path.join(projectRoot, '.qwen', 'skills', 'empty'), {291      recursive: true,292    });293    expect(await listExistingSkillDirNames(projectRoot)).toEqual(['real']);294  });295 296  it('returns [] when the skills directory does not exist', async () => {297    expect(await listExistingSkillDirNames(projectRoot)).toEqual([]);298  });299 300  it('includes skills whose directory is a symlink (matches skill-load.ts convention)', async () => {301    // Build a real skill outside the skills root, then symlink it in.302    // `skill-load.ts:31-34` and `skill-manager.ts:994-997` both treat303    // `isDirectory() || isSymbolicLink()` as a skill candidate; the304    // enumeration here mirrors that.305    const external = path.join(tempDir, 'external-skills', 'linked');306    await fs.mkdir(external, { recursive: true });307    await fs.writeFile(path.join(external, 'SKILL.md'), AUTO_SKILL, 'utf-8');308    const skillsRoot = path.join(projectRoot, '.qwen', 'skills');309    await fs.mkdir(skillsRoot, { recursive: true });310    await fs.symlink(external, path.join(skillsRoot, 'linked'));311    await writeSkillFile(projectRoot, 'regular', AUTO_SKILL);312    expect(await listExistingSkillDirNames(projectRoot)).toEqual([313      'linked',314      'regular',315    ]);316  });317});318 319describe('buildTaskPrompt', () => {320  let tempDir: string;321  let projectRoot: string;322 323  beforeEach(async () => {324    tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-prompt-'));325    projectRoot = path.join(tempDir, 'project');326    await fs.mkdir(projectRoot, { recursive: true });327  });328 329  afterEach(async () => {330    await fs.rm(tempDir, { recursive: true, force: true });331  });332 333  it('lists existing skill names so the agent picks a non-colliding name', async () => {334    await writeSkillFile(projectRoot, 'alpha', AUTO_SKILL);335    await writeSkillFile(projectRoot, 'beta', AUTO_SKILL);336    const prompt = await buildTaskPrompt(projectRoot);337    expect(prompt).toContain('alpha');338    expect(prompt).toContain('beta');339    expect(prompt).toMatch(/do NOT reuse/i);340  });341 342  it('falls back to a placeholder line when no skills exist yet', async () => {343    const prompt = await buildTaskPrompt(projectRoot);344    expect(prompt).toMatch(/no skills exist yet/i);345  });346 347  it('displays the project skills root derived from the same projectRoot used for enumeration', async () => {348    // Regression guard for the param collapse — the displayed root and349    // the enumerated names always come from the same source.350    await writeSkillFile(projectRoot, 'real', AUTO_SKILL);351    const prompt = await buildTaskPrompt(projectRoot);352    expect(prompt).toContain(path.join(projectRoot, '.qwen', 'skills'));353    expect(prompt).toContain('real');354  });355 356  it('instructs the agent to use the auto-skill- directory prefix (#4837)', async () => {357    // The `.gitignore` re-ignores `.qwen/skills/auto-skill-*/`, so new358    // auto-generated skills must land under an `auto-skill-`-prefixed359    // directory to stay out of version control. The prompt is the soft360    // guard that steers the agent there.361    const prompt = await buildTaskPrompt(projectRoot);362    expect(prompt).toContain(AUTO_SKILL_DIR_PREFIX);363    expect(prompt).toContain(`.qwen/skills/${AUTO_SKILL_DIR_PREFIX}<name>/`);364    expect(prompt).toMatch(/mandatory/i);365  });366});367 368describe('SKILL_REVIEW_SYSTEM_PROMPT', () => {369  it('requires the auto-skill- directory prefix for new skills (#4837)', () => {370    // The system prompt and buildTaskPrompt carry the prefix instruction on371    // two independent string arrays. buildTaskPrompt is asserted above; this372    // guards the parallel system-prompt line so an edit to one can't silently373    // drop the prefix mandate from the other.374    expect(SKILL_REVIEW_SYSTEM_PROMPT).toContain(AUTO_SKILL_DIR_PREFIX);375    expect(SKILL_REVIEW_SYSTEM_PROMPT).toContain(376      `.qwen/skills/${AUTO_SKILL_DIR_PREFIX}<name>/`,377    );378    expect(SKILL_REVIEW_SYSTEM_PROMPT).toMatch(/MUST use/i);379  });380});381