Skip to content

Commit 2ee159f

Browse files
committed
fix: harden skills import guards
1 parent 7316e39 commit 2ee159f

4 files changed

Lines changed: 234 additions & 13 deletions

File tree

cli.js

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,13 +1810,12 @@ function importSkills(params = {}) {
18101810
if (!target) {
18111811
return { error: '目标宿主不支持' };
18121812
}
1813+
const targetRoot = path.resolve(target.dir);
18131814
const rawItems = Array.isArray(params.items) ? params.items : [];
18141815
if (!rawItems.length) {
18151816
return { error: '请先选择要导入的 skill' };
18161817
}
18171818

1818-
ensureDir(target.dir);
1819-
18201819
const imported = [];
18211820
const failed = [];
18221821
const dedup = new Set();
@@ -1874,8 +1873,8 @@ function importSkills(params = {}) {
18741873
continue;
18751874
}
18761875

1877-
const targetPath = path.join(target.dir, normalizedName.name);
1878-
const targetRelative = path.relative(target.dir, targetPath);
1876+
const targetPath = path.join(targetRoot, normalizedName.name);
1877+
const targetRelative = path.relative(targetRoot, targetPath);
18791878
if (targetRelative.startsWith('..') || path.isAbsolute(targetRelative)) {
18801879
failed.push({
18811880
name: normalizedName.name,
@@ -1914,6 +1913,15 @@ function importSkills(params = {}) {
19141913
});
19151914
continue;
19161915
}
1916+
if (isPathInside(targetRoot, sourceDirForCopy)) {
1917+
failed.push({
1918+
name: normalizedName.name,
1919+
sourceApp: source.app,
1920+
error: '目标路径不能位于来源 skill 目录内'
1921+
});
1922+
continue;
1923+
}
1924+
ensureDir(targetRoot);
19171925
const visitedRealPaths = new Set([sourceDirForCopy]);
19181926
copyDirRecursive(sourceDirForCopy, targetPath, {
19191927
dereferenceSymlinks: true,
@@ -2011,18 +2019,21 @@ function resolveSkillNameFromImportedDirectory(skillDir, extractionRoot, fallbac
20112019
}
20122020

20132021
async function importSkillsFromZipFile(zipPath, options = {}) {
2014-
const target = resolveSkillTarget(options, 'codex');
2015-
if (!target) {
2016-
return { error: '目标宿主不支持' };
2017-
}
20182022
const fallbackName = typeof options.fallbackName === 'string' ? options.fallbackName : '';
20192023
const tempDir = typeof options.tempDir === 'string' ? options.tempDir : '';
20202024
const imported = [];
20212025
const failed = [];
20222026
const dedupNames = new Set();
20232027
const extractionRoot = path.join(tempDir || path.dirname(zipPath), 'extract');
2028+
let target = null;
2029+
let targetRoot = '';
20242030

20252031
try {
2032+
target = resolveSkillTarget(options, 'codex');
2033+
if (!target) {
2034+
return { error: '目标宿主不支持' };
2035+
}
2036+
targetRoot = path.resolve(target.dir);
20262037
await inspectZipArchiveLimits(zipPath, {
20272038
maxEntryCount: MAX_SKILLS_ZIP_ENTRY_COUNT,
20282039
maxUncompressedBytes: MAX_SKILLS_ZIP_UNCOMPRESSED_BYTES
@@ -2038,7 +2049,6 @@ async function importSkillsFromZipFile(zipPath, options = {}) {
20382049
return { error: '压缩包中的技能目录数量超出导入上限' };
20392050
}
20402051

2041-
ensureDir(target.dir);
20422052
for (const skillDir of discoveredDirs) {
20432053
const normalizedName = resolveSkillNameFromImportedDirectory(skillDir, extractionRoot, fallbackName);
20442054
if (normalizedName.error) {
@@ -2054,8 +2064,8 @@ async function importSkillsFromZipFile(zipPath, options = {}) {
20542064
}
20552065
dedupNames.add(dedupKey);
20562066

2057-
const targetPath = path.join(target.dir, normalizedName.name);
2058-
const targetRelative = path.relative(target.dir, targetPath);
2067+
const targetPath = path.join(targetRoot, normalizedName.name);
2068+
const targetRelative = path.relative(targetRoot, targetPath);
20592069
if (targetRelative.startsWith('..') || path.isAbsolute(targetRelative)) {
20602070
failed.push({
20612071
name: normalizedName.name,
@@ -2082,6 +2092,14 @@ async function importSkillsFromZipFile(zipPath, options = {}) {
20822092
});
20832093
continue;
20842094
}
2095+
if (isPathInside(targetRoot, sourceRealPath)) {
2096+
failed.push({
2097+
name: normalizedName.name,
2098+
error: '目标路径不能位于来源 skill 目录内'
2099+
});
2100+
continue;
2101+
}
2102+
ensureDir(targetRoot);
20852103
const visitedRealPaths = new Set([sourceRealPath]);
20862104
copyDirRecursive(sourceRealPath, targetPath, {
20872105
dereferenceSymlinks: true,

tests/unit/skills-market-runtime.test.mjs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,3 +285,27 @@ test('skills import and export entrypoints return early while delete is in progr
285285
assert.strictEqual(uploadCalls, 0);
286286
assert.deepStrictEqual(vm.messageLog, []);
287287
});
288+
289+
test('skills import entrypoints return early while scan is in progress', async () => {
290+
let apiCalls = 0;
291+
let uploadCalls = 0;
292+
const vm = buildVm(async () => {
293+
apiCalls += 1;
294+
return {};
295+
}, {
296+
skillsScanningImports: true,
297+
skillsImportList: [{ name: 'beta', sourceApp: 'claude' }],
298+
skillsImportSelectedKeys: ['claude:beta']
299+
});
300+
vm.uploadSkillsZipStream = async () => {
301+
uploadCalls += 1;
302+
return {};
303+
};
304+
305+
await vm.importSelectedSkills();
306+
await vm.importSkillsFromZipFile({ name: 'skills.zip', size: 1024 });
307+
308+
assert.strictEqual(apiCalls, 0);
309+
assert.strictEqual(uploadCalls, 0);
310+
assert.deepStrictEqual(vm.messageLog, []);
311+
});

tests/unit/web-run-host.test.mjs

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,16 @@ const resolveSkillTargetAppFromRequest = instantiateFunction(
286286
normalizeSkillTargetApp
287287
}
288288
);
289+
const importSkillsSource = extractFunctionBySignature(
290+
cliContent,
291+
'function importSkills(params = {}) {',
292+
'importSkills'
293+
);
294+
const importSkillsFromZipFileSource = extractFunctionBySignature(
295+
cliContent,
296+
'async function importSkillsFromZipFile(zipPath, options = {}) {',
297+
'importSkillsFromZipFile'
298+
);
289299
const handleImportSkillsZipUploadSource = extractFunctionBySignature(
290300
cliContent,
291301
'async function handleImportSkillsZipUpload(req, res, options = {}) {',
@@ -402,6 +412,175 @@ test('handleImportSkillsZipUpload derives fallback zip name from the resolved ta
402412
assert.strictEqual(res.statusCode, 200);
403413
});
404414

415+
test('importSkills rejects target roots nested inside the source skill before ensuring the destination root', () => {
416+
let copyCalls = 0;
417+
const ensureDirCalls = [];
418+
const importSkills = instantiateFunction(importSkillsSource, 'importSkills', {
419+
resolveSkillTarget() {
420+
return { app: 'codex', label: 'Codex', dir: '/tmp/source/nested' };
421+
},
422+
normalizeCodexSkillName(name) {
423+
return { name: String(name || '') };
424+
},
425+
getSkillImportSourceByApp() {
426+
return { app: 'claude', label: 'Claude', dir: '/tmp' };
427+
},
428+
ensureDir(dir) {
429+
ensureDirCalls.push(dir);
430+
},
431+
path,
432+
fs: {
433+
existsSync(targetPath) {
434+
return targetPath === '/tmp/source';
435+
},
436+
lstatSync() {
437+
return {
438+
isDirectory: () => true,
439+
isSymbolicLink: () => false
440+
};
441+
},
442+
statSync() {
443+
return {
444+
isDirectory: () => true
445+
};
446+
}
447+
},
448+
copyDirRecursive() {
449+
copyCalls += 1;
450+
},
451+
removeDirectoryRecursive() {},
452+
isPathInside(targetPath, rootPath) {
453+
return targetPath === '/tmp/source/nested' && rootPath === '/tmp/source';
454+
}
455+
});
456+
457+
const result = importSkills({
458+
targetApp: 'codex',
459+
items: [{ name: 'source', sourceApp: 'claude' }]
460+
});
461+
462+
assert.strictEqual(copyCalls, 0);
463+
assert.deepStrictEqual(ensureDirCalls, []);
464+
assert.deepStrictEqual(result.imported, []);
465+
assert.strictEqual(result.failed.length, 1);
466+
assert.strictEqual(result.failed[0].error, '目标路径不能位于来源 skill 目录内');
467+
});
468+
469+
test('importSkillsFromZipFile rejects target roots nested inside extracted skills before ensuring the destination root', async () => {
470+
let copyCalls = 0;
471+
const ensureDirCalls = [];
472+
const cleanupCalls = [];
473+
const importSkillsFromZipFile = instantiateFunction(importSkillsFromZipFileSource, 'importSkillsFromZipFile', {
474+
resolveSkillTarget() {
475+
return { app: 'codex', label: 'Codex', dir: '/tmp/upload/extract/source/nested' };
476+
},
477+
path,
478+
fs: {
479+
realpathSync(targetPath) {
480+
return targetPath;
481+
},
482+
statSync() {
483+
return {
484+
isDirectory: () => true
485+
};
486+
},
487+
existsSync() {
488+
return false;
489+
},
490+
rmSync(targetPath, options) {
491+
cleanupCalls.push({ targetPath, options });
492+
}
493+
},
494+
inspectZipArchiveLimits: async () => {},
495+
extractUploadZip: async () => {},
496+
collectSkillDirectoriesFromRoot() {
497+
return {
498+
results: ['/tmp/upload/extract/source'],
499+
truncated: false
500+
};
501+
},
502+
resolveSkillNameFromImportedDirectory() {
503+
return { name: 'source' };
504+
},
505+
ensureDir(dir) {
506+
ensureDirCalls.push(dir);
507+
},
508+
copyDirRecursive() {
509+
copyCalls += 1;
510+
},
511+
removeDirectoryRecursive() {},
512+
isPathInside(targetPath, rootPath) {
513+
return targetPath === '/tmp/upload/extract/source/nested' && rootPath === '/tmp/upload/extract/source';
514+
},
515+
MAX_SKILLS_ZIP_ENTRY_COUNT: 100,
516+
MAX_SKILLS_ZIP_UNCOMPRESSED_BYTES: 1024
517+
});
518+
519+
const result = await importSkillsFromZipFile('/tmp/upload/archive.zip', {
520+
tempDir: '/tmp/upload',
521+
targetApp: 'codex'
522+
});
523+
524+
assert.strictEqual(copyCalls, 0);
525+
assert.deepStrictEqual(ensureDirCalls, []);
526+
assert.deepStrictEqual(result.imported, []);
527+
assert.strictEqual(result.failed.length, 1);
528+
assert.strictEqual(result.failed[0].error, '目标路径不能位于来源 skill 目录内');
529+
assert.deepStrictEqual(cleanupCalls, [{
530+
targetPath: '/tmp/upload',
531+
options: { recursive: true, force: true }
532+
}]);
533+
});
534+
535+
test('importSkillsFromZipFile still cleans tempDir when target app is unsupported', async () => {
536+
const cleanupCalls = [];
537+
const importSkillsFromZipFile = instantiateFunction(importSkillsFromZipFileSource, 'importSkillsFromZipFile', {
538+
resolveSkillTarget() {
539+
return null;
540+
},
541+
path,
542+
fs: {
543+
existsSync() {
544+
return false;
545+
},
546+
rmSync(targetPath, options) {
547+
cleanupCalls.push({ targetPath, options });
548+
}
549+
},
550+
inspectZipArchiveLimits: async () => {
551+
throw new Error('inspectZipArchiveLimits should not run');
552+
},
553+
extractUploadZip: async () => {
554+
throw new Error('extractUploadZip should not run');
555+
},
556+
collectSkillDirectoriesFromRoot() {
557+
return { results: [], truncated: false };
558+
},
559+
resolveSkillNameFromImportedDirectory() {
560+
return { name: 'demo' };
561+
},
562+
ensureDir() {},
563+
copyDirRecursive() {},
564+
removeDirectoryRecursive() {},
565+
isPathInside() {
566+
return false;
567+
},
568+
MAX_SKILLS_ZIP_ENTRY_COUNT: 100,
569+
MAX_SKILLS_ZIP_UNCOMPRESSED_BYTES: 1024
570+
});
571+
572+
const result = await importSkillsFromZipFile('/tmp/upload/archive.zip', {
573+
tempDir: '/tmp/upload',
574+
targetApp: 'invalid'
575+
});
576+
577+
assert.deepStrictEqual(result, { error: '目标宿主不支持' });
578+
assert.deepStrictEqual(cleanupCalls, [{
579+
targetPath: '/tmp/upload',
580+
options: { recursive: true, force: true }
581+
}]);
582+
});
583+
405584
test('codex-only zip upload route pins target app before request fallback resolution', () => {
406585
assert.match(
407586
cliContent,

web-ui/modules/skills.methods.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ export function createSkillsMethods({ api }) {
239239
},
240240

241241
async importSelectedSkills() {
242-
if (this.skillsDeleting || this.skillsImporting || this.skillsZipImporting || this.skillsExporting) return;
242+
if (this.skillsDeleting || this.skillsScanningImports || this.skillsImporting || this.skillsZipImporting || this.skillsExporting) return;
243243
const selectedSet = new Set(Array.isArray(this.skillsImportSelectedKeys) ? this.skillsImportSelectedKeys : []);
244244
const selectedItems = (Array.isArray(this.skillsImportList) ? this.skillsImportList : [])
245245
.filter((item) => selectedSet.has(this.buildSkillImportKey(item)))
@@ -331,7 +331,7 @@ export function createSkillsMethods({ api }) {
331331
},
332332

333333
async importSkillsFromZipFile(file) {
334-
if (this.skillsDeleting || this.skillsZipImporting || this.skillsImporting || this.skillsExporting) return;
334+
if (this.skillsDeleting || this.skillsScanningImports || this.skillsZipImporting || this.skillsImporting || this.skillsExporting) return;
335335
const maxSize = 20 * 1024 * 1024;
336336
if (file.size > maxSize) {
337337
this.showMessage('ZIP 文件过大,限制 20MB', 'error');

0 commit comments

Comments
 (0)