Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 26 additions & 0 deletions scripts/path-containment.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
16 changes: 16 additions & 0 deletions scripts/path-containment.ts
Original file line number Diff line number Diff line change
@@ -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)}/`)
}
5 changes: 3 additions & 2 deletions scripts/validate-skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}`)
}
}
Expand Down Expand Up @@ -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
}
Expand Down