Skip to content

Commit e3b5980

Browse files
committed
fix(cli): validate skill path
1 parent c27402f commit e3b5980

4 files changed

Lines changed: 32 additions & 3 deletions

File tree

packages/cli/src/__tests__/lib/SkillManager.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ describe("SkillManager", () => {
9696

9797
mockedSkillUtil.validateRegistryId.mockImplementation(() => { });
9898
mockedSkillUtil.validateSkillName.mockImplementation(() => { });
99+
mockedSkillUtil.isValidSkillName.mockImplementation((name: string) => /^[a-z0-9]+(-[a-z0-9]+)*$/.test(name));
99100
mockedGitUtil.ensureGitInstalled.mockResolvedValue(undefined);
100101
mockConfigManager.addSkill.mockResolvedValue({} as any);
101102
});

packages/cli/src/__tests__/util/skill.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { validateRegistryId, validateSkillName } from '../../util/skill';
1+
import { validateRegistryId, validateSkillName, isValidSkillName } from '../../util/skill';
22

33
describe('Skill Validation Utilities', () => {
44
describe('validateRegistryId', () => {
@@ -180,4 +180,25 @@ describe('Skill Validation Utilities', () => {
180180
});
181181
});
182182
});
183+
184+
describe('isValidSkillName', () => {
185+
it('should return true for valid skill names', () => {
186+
expect(isValidSkillName('my-skill')).toBe(true);
187+
expect(isValidSkillName('skill123')).toBe(true);
188+
expect(isValidSkillName('a')).toBe(true);
189+
});
190+
191+
it('should return false for names with path traversal characters', () => {
192+
expect(isValidSkillName('../etc')).toBe(false);
193+
expect(isValidSkillName('skill/name')).toBe(false);
194+
expect(isValidSkillName('.hidden')).toBe(false);
195+
});
196+
197+
it('should return false for names that fail skill name rules', () => {
198+
expect(isValidSkillName('-leading')).toBe(false);
199+
expect(isValidSkillName('trailing-')).toBe(false);
200+
expect(isValidSkillName('double--hyphen')).toBe(false);
201+
expect(isValidSkillName('UPPERCASE')).toBe(false);
202+
});
203+
});
183204
});

packages/cli/src/lib/SkillManager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { GlobalConfigManager } from './GlobalConfig';
77
import { EnvironmentSelector } from './EnvironmentSelector';
88
import { getGlobalSkillPath, getSkillPath, validateEnvironmentCodes } from '../util/env';
99
import { ensureGitInstalled, cloneRepository, isGitRepository, pullRepository, fetchGitHead } from '../util/git';
10-
import { validateRegistryId, validateSkillName, extractSkillDescription } from '../util/skill';
10+
import { validateRegistryId, validateSkillName, extractSkillDescription, isValidSkillName } from '../util/skill';
1111
import { fetchGitHubSkillPaths, fetchRawGitHubFile } from '../util/github';
1212
import { isInteractiveTerminal } from '../util/terminal';
1313
import { ui } from '../util/terminal-ui';
@@ -555,7 +555,7 @@ export class SkillManager {
555555
const skills: RegistrySkillChoice[] = [];
556556

557557
for (const entry of entries) {
558-
if (!entry.isDirectory()) {
558+
if (!entry.isDirectory() || !isValidSkillName(entry.name)) {
559559
continue;
560560
}
561561

packages/cli/src/util/skill.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export function validateSkillName(skillName: string): void {
3636
}
3737
}
3838

39+
/**
40+
* Checks if a skill name is valid without throwing
41+
*/
42+
export function isValidSkillName(name: string): boolean {
43+
return /^[a-z0-9]+(-[a-z0-9]+)*$/.test(name);
44+
}
45+
3946
/**
4047
* Extract skill description from SKILL.md frontmatter
4148
* @param content - Content of SKILL.md file

0 commit comments

Comments
 (0)