diff --git a/package.json b/package.json index 1d3396fd49..7d1528c169 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "check:fix": "biome check --write", "check-types": "turbo run check-types", "test": "turbo run test", - "skills:validate": "bun scripts/validate-skills.ts && bun test scripts/clawhub-ignore-policy.test.ts scripts/claude-mcp-config-policy.test.ts scripts/openai-tool-annotation-policy.test.ts scripts/public-skill-discovery-policy.test.ts", + "skills:validate": "bun scripts/validate-skills.ts && bun test scripts/clawhub-ignore-policy.test.ts scripts/claude-mcp-config-policy.test.ts scripts/openai-tool-annotation-policy.test.ts scripts/path-containment.test.ts scripts/public-skill-discovery-policy.test.ts", "kill": "lsof -ti:3002 | xargs kill -9 2>/dev/null || echo 'No processes found on port 3002'", "clean:cache": "rm -rf apps/*/.next apps/*/.swc apps/*/.turbo packages/*/.turbo tooling/*/.turbo .turbo node_modules/.cache", "restart": "bun kill && bun clean:cache && bun dev", diff --git a/scripts/path-containment.test.ts b/scripts/path-containment.test.ts new file mode 100644 index 0000000000..a4b26354a7 --- /dev/null +++ b/scripts/path-containment.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, test } from 'bun:test' +import { isPathInside } from './path-containment' + +describe('isPathInside', () => { + test.each([ + ['windows separators', 'C:\\repo\\skills\\pascal-3d', 'C:\\repo\\skills\\pascal-3d\\a.md'], + ['posix separators', '/repo/skills/pascal-3d', '/repo/skills/pascal-3d/a.md'], + ['mixed separators', 'C:\\repo\\skills\\pascal-3d', 'C:/repo/skills/pascal-3d/a.md'], + ['nested directory', '/repo/skills/pascal-3d', '/repo/skills/pascal-3d/examples/a.md'], + ['drive root', 'C:\\', 'C:\\a.md'], + ['trailing separator on the parent', '/repo/skills/', '/repo/skills/a.md'], + ])('accepts a target inside the parent with %s', (_label, parent, target) => { + expect(isPathInside(parent, target)).toBe(true) + }) + + test.each([ + ['a sibling sharing the name prefix', '/repo/skills', '/repo/skills-extra/a.md'], + ['a parent directory', '/repo/skills/pascal-3d', '/repo/skills/a.md'], + ['an unrelated directory', '/repo/skills', '/repo/other/a.md'], + ['the parent itself', '/repo/skills', '/repo/skills'], + ['a traversal out of the parent', '/repo/skills', '/repo/other/../skills-evil/a.md'], + ['a different drive', 'C:\\repo\\skills', 'D:\\repo\\skills\\a.md'], + ])('rejects %s', (_label, parent, target) => { + expect(isPathInside(parent, target)).toBe(false) + }) +}) diff --git a/scripts/path-containment.ts b/scripts/path-containment.ts new file mode 100644 index 0000000000..f2afe9c8cb --- /dev/null +++ b/scripts/path-containment.ts @@ -0,0 +1,16 @@ +/** + * Containment check for paths produced by `resolve()` / `join()`. + * + * A `${parentDir}/` string prefix is not portable: `resolve()` returns + * backslash-separated paths on Windows, so the prefix never matches there and + * every file that is genuinely inside the directory is reported as outside. + * Comparing on a normalized separator keeps the check identical on every + * platform. + */ +function normalizeSeparators(path: string): string { + return path.replace(/\\/g, '/').replace(/\/+$/, '') +} + +export function isPathInside(parentDir: string, target: string): boolean { + return normalizeSeparators(target).startsWith(`${normalizeSeparators(parentDir)}/`) +} diff --git a/scripts/validate-skills.ts b/scripts/validate-skills.ts index 88b8516dba..aafef47fd5 100644 --- a/scripts/validate-skills.ts +++ b/scripts/validate-skills.ts @@ -5,6 +5,7 @@ import { XMLParser, XMLValidator } from 'fast-xml-parser' import { validateClaudeMcpPolicy } from './claude-mcp-config-policy' import { validateClawHubIgnorePolicy } from './clawhub-ignore-policy' import { validateOpenAiToolAnnotationPacket } from './openai-tool-annotation-policy' +import { isPathInside } from './path-containment' import { collectSkillDiscoveryEntries, validatePublicSkillDiscoverySurface, @@ -384,7 +385,7 @@ for (const skillName of skillNames) { const target = match[1]! if (/^(?:https?:|mailto:|#)/.test(target)) continue const resolvedTarget = resolve(dirname(path), target.split('#')[0]!) - if (!resolvedTarget.startsWith(`${skillRoot}/`)) { + if (!isPathInside(skillRoot, resolvedTarget)) { fail(`${relative(root, path)} links outside its standalone skill bundle: ${target}`) } } @@ -1079,7 +1080,7 @@ for (const field of ['composerIcon', 'logo']) { continue } const asset = resolve(root, value) - if (!asset.startsWith(`${root}/`) || !existsSync(asset) || !lstatSync(asset).isFile()) { + if (!isPathInside(root, asset) || !existsSync(asset) || !lstatSync(asset).isFile()) { fail(`OpenAI ${field} must reference an existing file inside the plugin`) continue }