fix(skills): XML-escape description/whenToUse; symlinks in skill-load.ts; dot:true in ConditionalRulesRegistry

Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/a56d83ce-cbdf-4213-a90a-888a9f05ee4f

packages/core/src/skills/skill-load.test.ts

@@ -212,8 +212,18 @@ Some content without frontmatter.
 
     it('should load skills from directory', async () => {
       vi.mocked(fs.readdir).mockResolvedValue([
-        { name: 'skill1', isDirectory: () => true, isFile: () => false },
-        { name: 'not-a-dir.txt', isDirectory: () => false, isFile: () => true },
+        {
+          name: 'skill1',
+          isDirectory: () => true,
+          isFile: () => false,
+          isSymbolicLink: () => false,
+        },
+        {
+          name: 'not-a-dir.txt',
+          isDirectory: () => false,
+          isFile: () => true,
+          isSymbolicLink: () => false,
+        },
       ] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
 
       vi.mocked(fs.access).mockResolvedValue(undefined);
@@ -241,8 +251,18 @@ Skill body.
 
     it('should skip skills with invalid YAML and continue loading others', async () => {
       vi.mocked(fs.readdir).mockResolvedValue([
-        { name: 'valid-skill', isDirectory: () => true, isFile: () => false },
-        { name: 'invalid-skill', isDirectory: () => true, isFile: () => false },
+        {
+          name: 'valid-skill',
+          isDirectory: () => true,
+          isFile: () => false,
+          isSymbolicLink: () => false,
+        },
+        {
+          name: 'invalid-skill',
+          isDirectory: () => true,
+          isFile: () => false,
+          isSymbolicLink: () => false,
+        },
       ] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
 
       vi.mocked(fs.access).mockResolvedValue(undefined);
@@ -265,6 +285,76 @@ Valid skill.
       expect(skills).toHaveLength(1);
       expect(skills[0]?.name).toBe('test-skill');
     });
+
+    it('should load skills from symlinked directories', async () => {
+      vi.mocked(fs.readdir).mockResolvedValue([
+        {
+          name: 'symlinked-skill',
+          isDirectory: () => false,
+          isFile: () => false,
+          isSymbolicLink: () => true,
+        },
+      ] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
+
+      // stat resolves to a directory (symlink target)
+      vi.mocked(fs.stat).mockResolvedValue({
+        isDirectory: () => true,
+      } as unknown as Awaited<ReturnType<typeof fs.stat>>);
+
+      vi.mocked(fs.access).mockResolvedValue(undefined);
+      vi.mocked(fs.readFile).mockResolvedValue(`---
+name: test-skill
+description: A test skill
+---
+
+Symlinked skill body.
+`);
+
+      const skills = await loadSkillsFromDir(testBaseDir);
+
+      // Skill is loaded from the symlinked directory.
+      expect(skills).toHaveLength(1);
+    });
+
+    it('should skip symlinks that do not point to a directory', async () => {
+      vi.mocked(fs.readdir).mockResolvedValue([
+        {
+          name: 'file-symlink',
+          isDirectory: () => false,
+          isFile: () => false,
+          isSymbolicLink: () => true,
+        },
+      ] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
+
+      // stat resolves to a file (not a directory)
+      vi.mocked(fs.stat).mockResolvedValue({
+        isDirectory: () => false,
+      } as unknown as Awaited<ReturnType<typeof fs.stat>>);
+
+      const skills = await loadSkillsFromDir(testBaseDir);
+
+      expect(skills).toHaveLength(0);
+    });
+
+    it('should skip broken symlinks gracefully', async () => {
+      vi.mocked(fs.readdir).mockResolvedValue([
+        {
+          name: 'broken-symlink',
+          isDirectory: () => false,
+          isFile: () => false,
+          isSymbolicLink: () => true,
+        },
+      ] as unknown as Awaited<ReturnType<typeof fs.readdir>>);
+
+      // stat throws because the symlink target doesn't exist
+      vi.mocked(fs.stat).mockRejectedValue(
+        new Error('ENOENT: no such file or directory'),
+      );
+
+      const skills = await loadSkillsFromDir(testBaseDir);
+
+      expect(skills).toHaveLength(0);
+    });
   });
 
   describe('validateConfig', () => {

packages/core/src/skills/skill-load.ts

@@ -25,13 +25,35 @@ export async function loadSkillsFromDir(
     debugLogger.debug(`Found ${entries.length} entries in ${baseDir}`);
 
     for (const entry of entries) {
-      // Only process directories (each skill is a directory)
-      if (!entry.isDirectory()) {
+      // Process directories and symlinks that resolve to directories.
+      // Plain files are silently skipped (each skill must be a directory).
+      const isDirectory = entry.isDirectory();
+      const isSymlink = entry.isSymbolicLink();
+
+      if (!isDirectory && !isSymlink) {
         debugLogger.warn(`Skipping non-directory entry: ${entry.name}`);
         continue;
       }
 
       const skillDir = path.join(baseDir, entry.name);
+
+      // For symlinks, verify the target resolves to a directory.
+      if (isSymlink) {
+        try {
+          const targetStat = await fs.stat(skillDir);
+          if (!targetStat.isDirectory()) {
+            debugLogger.warn(
+              `Skipping symlink ${entry.name} that does not point to a directory`,
+            );
+            continue;
+          }
+        } catch (error) {
+          debugLogger.warn(
+            `Skipping invalid symlink ${entry.name}: ${error instanceof Error ? error.message : 'Unknown error'}`,
+          );
+          continue;
+        }
+      }
       const skillManifest = path.join(skillDir, SKILL_MANIFEST_FILE);
 
       try {

packages/core/src/tools/skill.test.ts

@@ -146,6 +146,33 @@ describe('SkillTool', () => {
       );
     });
 
+    it('should XML-escape description and whenToUse fields', async () => {
+      // A crafted description containing XML-special characters must not
+      // inject raw tags into the <available_skills> block.
+      vi.mocked(mockSkillManager.listSkills).mockResolvedValue([
+        {
+          name: 'xss-skill',
+          description: 'Skill <b>bold</b> & more',
+          whenToUse: 'When <script> tags > nothing',
+          level: 'project',
+          filePath: '/project/.qwen/skills/xss-skill/SKILL.md',
+          body: 'Body text.',
+        },
+      ]);
+      const tool = new SkillTool(config);
+      await vi.runAllTimersAsync();
+
+      expect(tool.description).toContain(
+        'Skill &lt;b&gt;bold&lt;/b&gt; &amp; more',
+      );
+      expect(tool.description).toContain(
+        'When &lt;script&gt; tags &gt; nothing',
+      );
+      // Raw tags must not appear
+      expect(tool.description).not.toContain('<b>');
+      expect(tool.description).not.toContain('<script>');
+    });
+
     it('should handle empty skills list gracefully', async () => {
       vi.mocked(mockSkillManager.listSkills).mockResolvedValue([]);
 

packages/core/src/tools/skill.ts

@@ -17,6 +17,18 @@ import { registerSkillHooks } from '../hooks/registerSkillHooks.js';
 
 const debugLogger = createDebugLogger('SKILL');
 
+/**
+ * Escape characters that have special meaning in XML so that user-supplied
+ * skill fields (description, whenToUse) cannot inject tags into the
+ * <available_skills> block that is sent to the model.
+ */
+function escapeXml(text: string): string {
+  return text
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;');
+}
+
 export interface SkillParams {
   skill: string;
 }
@@ -169,12 +181,13 @@ export class SkillTool extends BaseDeclarativeTool<SkillParams, ToolResult> {
     const allSkillEntries: string[] = [];
 
     for (const skill of this.availableSkills) {
+      const descText = `${escapeXml(skill.description)}${skill.whenToUse ? ` — ${escapeXml(skill.whenToUse)}` : ''} (${skill.level})`;
       allSkillEntries.push(`<skill>
 <name>
 ${skill.name}
 </name>
 <description>
-${skill.description}${skill.whenToUse ? ` — ${skill.whenToUse}` : ''} (${skill.level})
+${descText}
 </description>
 <location>
 ${skill.level}
@@ -188,7 +201,7 @@ ${skill.level}
 ${cmd.name}
 </name>
 <description>
-${cmd.description}
+${escapeXml(cmd.description)}
 </description>
 </skill>`);
     }

packages/core/src/utils/rulesDiscovery.test.ts

@@ -423,5 +423,18 @@ Use hooks.`,
       const result = reg.matchAndConsume('src/foo.ts');
       expect(result).toContain('Strict.');
     });
+
+    it('activates on dotfiles when glob covers them (dot: true semantics)', () => {
+      // **/*.yml must match .github/workflows/ci.yml, .prettierrc.yml, etc.
+      // Regression: previously picomatch used { dot: false } so hidden paths
+      // were silently excluded.
+      const reg = new ConditionalRulesRegistry(
+        [rule('/r/yml.md', ['**/*.yml'], 'YAML rule.')],
+        '/project',
+      );
+      expect(
+        reg.matchAndConsume('/project/.github/workflows/ci.yml'),
+      ).toContain('YAML rule.');
+    });
   });
 });

packages/core/src/utils/rulesDiscovery.ts

@@ -238,7 +238,7 @@ export class ConditionalRulesRegistry {
     this.projectRoot = projectRoot;
     this.compiledRules = rules.map((rule) => ({
       rule,
-      matchers: (rule.paths ?? []).map((p) => picomatch(p, { dot: false })),
+      matchers: (rule.paths ?? []).map((p) => picomatch(p, { dot: true })),
     }));
     logger.debug(
       `ConditionalRulesRegistry created with ${rules.length} rule(s)`,