fix: enforce skill discovery depth for dotdot names (#3725)

This commit is contained in:
Neil
2026-05-30 11:50:30 -07:00
committed by GitHub
parent 5304ae118d
commit cc028d36e7
3 changed files with 106 additions and 78 deletions
+15
View File
@@ -94,4 +94,19 @@ describe('skill discovery', () => {
expect(result.skills).toEqual([])
})
it('enforces depth limits for valid child directories whose names start with dot-dot', async () => {
const root = await mkdtemp(join(tmpdir(), 'orca-skills-'))
const home = join(root, 'home')
const deepSkill = join(home, '.agents', 'skills', '..deep', 'a', 'b', 'c', 'd', 'too-deep')
await mkdir(deepSkill, { recursive: true })
await writeFile(join(deepSkill, 'SKILL.md'), '# Too Deep\n\nShould not be discovered.')
const result = await discoverSkills({
homeDir: home,
cwd: join(root, 'missing-cwd')
})
expect(result.skills.map((skill) => skill.name)).not.toContain('Too Deep')
})
})
+14 -78
View File
@@ -1,19 +1,21 @@
import { createHash } from 'node:crypto'
import type { Dirent } from 'node:fs'
import { open, readdir, realpath, stat } from 'node:fs/promises'
import { homedir } from 'node:os'
import { basename, dirname, join, relative, sep } from 'node:path'
import type { Repo } from '../../shared/types'
import { basename, dirname, isAbsolute, join, relative, sep } from 'node:path'
import { summarizeSkillMarkdown } from '../../shared/skill-metadata'
import type { Repo } from '../../shared/types'
import type {
DiscoveredSkill,
SkillDiscoveryResult,
SkillDiscoverySource,
SkillProvider,
SkillSourceKind
} from '../../shared/skills'
import {
buildSkillDiscoverySources,
stablePathId,
type SkillScanRoot
} from './skill-discovery-sources'
type SkillScanRoot = Omit<SkillDiscoverySource, 'exists' | 'skippedReason'>
export { buildSkillDiscoverySources } from './skill-discovery-sources'
const SKILL_FILE_NAME = 'SKILL.md'
const MAX_MARKDOWN_BYTES = 256 * 1024
@@ -28,10 +30,6 @@ async function pathExists(pathValue: string): Promise<boolean> {
}
}
function stableId(pathValue: string): string {
return createHash('sha1').update(pathValue).digest('hex').slice(0, 16)
}
function compareSkills(a: DiscoveredSkill, b: DiscoveredSkill): number {
return (
a.name.localeCompare(b.name, undefined, { sensitivity: 'base' }) ||
@@ -42,9 +40,13 @@ function compareSkills(a: DiscoveredSkill, b: DiscoveredSkill): number {
function isWithinDepth(rootPath: string, childPath: string, maxDepth: number): boolean {
const rel = relative(rootPath, childPath)
if (!rel || rel.startsWith('..')) {
if (!rel) {
return true
}
// Why: `..cache` is a valid child name; only a real parent traversal escapes.
if (rel === '..' || rel.startsWith(`..${sep}`) || isAbsolute(rel)) {
return false
}
return rel.split(sep).length <= maxDepth
}
@@ -210,7 +212,7 @@ async function scanRoot(root: SkillScanRoot): Promise<DiscoveredSkill[]> {
const summary = await readSkillSummary(skillFilePath)
const sourceKind = sourceKindForSkill(root, skillFilePath)
return {
id: stableId(skillFilePath),
id: stablePathId(skillFilePath),
name: summary.name ?? basename(directoryPath),
description: summary.description,
providers: root.providers,
@@ -228,72 +230,6 @@ async function scanRoot(root: SkillScanRoot): Promise<DiscoveredSkill[]> {
return skills
}
function source(
id: string,
label: string,
path: string,
sourceKind: SkillSourceKind,
providers: SkillProvider[]
): SkillScanRoot {
return { id, label, path, sourceKind, providers }
}
export function buildSkillDiscoverySources(
args: {
homeDir?: string
cwd?: string
repos?: Repo[]
} = {}
): SkillScanRoot[] {
const home = args.homeDir ?? homedir()
const cwd = args.cwd ?? process.cwd()
const roots: SkillScanRoot[] = [
source('home-codex', 'Codex home', join(home, '.codex', 'skills'), 'home', ['codex']),
source('home-agents', 'Agent skills home', join(home, '.agents', 'skills'), 'home', [
'agent-skills'
]),
source('home-claude', 'Claude home', join(home, '.claude', 'skills'), 'home', ['claude']),
source(
'codex-plugin-cache',
'Codex plugin cache',
join(home, '.codex', 'plugins', 'cache'),
'plugin',
['codex', 'agent-skills']
)
]
const projectPaths = new Set<string>()
for (const repo of args.repos ?? []) {
if (repo.connectionId) {
continue
}
projectPaths.add(repo.path)
}
projectPaths.add(cwd)
for (const repoPath of projectPaths) {
const label = `Repo ${basename(repoPath)}`
roots.push(
source(
`repo-agents-${stableId(repoPath)}`,
`${label} .agents`,
join(repoPath, '.agents', 'skills'),
'repo',
['agent-skills']
),
source(
`repo-claude-${stableId(repoPath)}`,
`${label} .claude`,
join(repoPath, '.claude', 'skills'),
'repo',
['claude']
)
)
}
return roots
}
export async function discoverSkills(args: {
repos?: Repo[]
homeDir?: string
@@ -0,0 +1,77 @@
import { createHash } from 'node:crypto'
import { homedir } from 'node:os'
import { basename, join } from 'node:path'
import type { SkillDiscoverySource, SkillProvider, SkillSourceKind } from '../../shared/skills'
import type { Repo } from '../../shared/types'
export type SkillScanRoot = Omit<SkillDiscoverySource, 'exists' | 'skippedReason'>
export function stablePathId(pathValue: string): string {
return createHash('sha1').update(pathValue).digest('hex').slice(0, 16)
}
function source(
id: string,
label: string,
path: string,
sourceKind: SkillSourceKind,
providers: SkillProvider[]
): SkillScanRoot {
return { id, label, path, sourceKind, providers }
}
export function buildSkillDiscoverySources(
args: {
homeDir?: string
cwd?: string
repos?: Repo[]
} = {}
): SkillScanRoot[] {
const home = args.homeDir ?? homedir()
const cwd = args.cwd ?? process.cwd()
const roots: SkillScanRoot[] = [
source('home-codex', 'Codex home', join(home, '.codex', 'skills'), 'home', ['codex']),
source('home-agents', 'Agent skills home', join(home, '.agents', 'skills'), 'home', [
'agent-skills'
]),
source('home-claude', 'Claude home', join(home, '.claude', 'skills'), 'home', ['claude']),
source(
'codex-plugin-cache',
'Codex plugin cache',
join(home, '.codex', 'plugins', 'cache'),
'plugin',
['codex', 'agent-skills']
)
]
const projectPaths = new Set<string>()
for (const repo of args.repos ?? []) {
if (repo.connectionId) {
continue
}
projectPaths.add(repo.path)
}
projectPaths.add(cwd)
for (const repoPath of projectPaths) {
const label = `Repo ${basename(repoPath)}`
roots.push(
source(
`repo-agents-${stablePathId(repoPath)}`,
`${label} .agents`,
join(repoPath, '.agents', 'skills'),
'repo',
['agent-skills']
),
source(
`repo-claude-${stablePathId(repoPath)}`,
`${label} .claude`,
join(repoPath, '.claude', 'skills'),
'repo',
['claude']
)
)
}
return roots
}