From 9a78073a2742d77aa361d588184e5c075c518b69 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Fri, 4 Sep 2026 19:25:44 -0700 Subject: [PATCH 1/4] fix: contain discovered metadata and validate generated command arguments --- .changeset/safe-discovery-boundaries.md | 5 + docs/cli/intent-list.md | 2 + .../intent/src/commands/install/guidance.ts | 8 +- packages/intent/src/commands/list.ts | 6 +- packages/intent/src/discovery/scanner.ts | 37 ++- packages/intent/src/shared/command-runner.ts | 21 +- packages/intent/src/skills/paths.ts | 9 +- .../intent/tests/discovery-safety.test.ts | 245 ++++++++++++++++++ 8 files changed, 319 insertions(+), 14 deletions(-) create mode 100644 .changeset/safe-discovery-boundaries.md create mode 100644 packages/intent/tests/discovery-safety.test.ts diff --git a/.changeset/safe-discovery-boundaries.md b/.changeset/safe-discovery-boundaries.md new file mode 100644 index 00000000..a6318f98 --- /dev/null +++ b/.changeset/safe-discovery-boundaries.md @@ -0,0 +1,5 @@ +--- +'@tanstack/intent': patch +--- + +Reject unsafe dynamic identifiers in runnable list commands, install mappings, and runtime lookup hints. Skip skill metadata that resolves outside its package root while preserving valid package-manager symlinks and direct-load containment errors. diff --git a/docs/cli/intent-list.md b/docs/cli/intent-list.md index b3c05f73..0db2c728 100644 --- a/docs/cli/intent-list.md +++ b/docs/cli/intent-list.md @@ -150,3 +150,5 @@ An excluded package never triggers the unlisted-source notice, because an exclud - Scanner failures are printed as errors - Deno projects without `node_modules` are unsupported +- Runnable commands require identifiers containing only ASCII letters, numbers, `_`, `.`, `/`, `@`, `#`, and `-`. Identifiers cannot start with `#`. Other characters cause an error before a command is emitted; rename the package or skill to generate runnable guidance. `--json` still exposes permitted identifiers as data, not shell commands. +- Discovery skips skill files whose real path cannot be resolved or lies outside their package root, with a warning. Symlinks to files inside the resolved package root remain supported. External frontmatter is not read or included in catalogs or mappings. diff --git a/packages/intent/src/commands/install/guidance.ts b/packages/intent/src/commands/install/guidance.ts index 50945e40..9ddc89a7 100644 --- a/packages/intent/src/commands/install/guidance.ts +++ b/packages/intent/src/commands/install/guidance.ts @@ -304,10 +304,10 @@ export function buildIntentSkillsBlock( ) lines.push( ` run: ${quoteYamlString( - formatIntentCommand( - scanResult.packageManager, - `load ${formatSkillUse(pkg.name, skill.name)}`, - ), + formatIntentCommand(scanResult.packageManager, [ + 'load', + formatSkillUse(pkg.name, skill.name), + ]), )}`, ) lines.push(` for: ${quoteYamlString(formatWhen(pkg.name, skill))}`) diff --git a/packages/intent/src/commands/list.ts b/packages/intent/src/commands/list.ts index 0fc45c5e..7b02fd77 100644 --- a/packages/intent/src/commands/list.ts +++ b/packages/intent/src/commands/list.ts @@ -86,7 +86,11 @@ function formatLoadCommand( packageManager: ScanResult['packageManager'], scopeFlag: string, ): string { - return formatIntentCommand(packageManager, `load ${skill.use}${scopeFlag}`) + return formatIntentCommand(packageManager, [ + 'load', + skill.use, + ...(scopeFlag ? [scopeFlag.trim()] : []), + ]) } function printHiddenSources(result: IntentSkillList, audience: string): void { diff --git a/packages/intent/src/discovery/scanner.ts b/packages/intent/src/discovery/scanner.ts index 622113ce..728fe9ae 100644 --- a/packages/intent/src/discovery/scanner.ts +++ b/packages/intent/src/discovery/scanner.ts @@ -265,8 +265,16 @@ function readSkillEntry( childDir: string, skillFile: string, readFs: ReadFs = nodeReadFs, -): SkillEntry { - const fm = parseFrontmatter(skillFile, readFs) +): SkillEntry | null { + let realSkillFile: string + try { + realSkillFile = readFs.realpathSync.native(skillFile) + const realPackageRoot = readFs.realpathSync.native(dirname(skillsDir)) + if (!isWithinOrEqual(realSkillFile, realPackageRoot)) return null + } catch { + return null + } + const fm = parseFrontmatter(realSkillFile, readFs) const relName = toPosixPath(relative(skillsDir, childDir)) const desc = typeof fm?.description === 'string' @@ -299,7 +307,13 @@ function discoverSkillByNameHint( const { childDir, skillFile } = resolvedHint if (!readFs.existsSync(skillFile)) continue - const skill = readSkillEntry(skillsDir, childDir, skillFile, readFs) + // Keep the hinted identity so loading can report its existing path error, + // without reading metadata from an unreadable or escaping target. + const skill = readSkillEntry(skillsDir, childDir, skillFile, readFs) ?? { + name: hint, + path: skillFile, + description: '', + } if (skill.name !== hint || seen.has(skill.name)) continue seen.add(skill.name) @@ -311,7 +325,9 @@ function discoverSkillByNameHint( function discoverSkills( skillsDir: string, + packageName: string, fsCache: IntentFsCache, + warnings: Array, ): Array { const readFs = fsCache.getReadFs() return fsCache @@ -319,7 +335,14 @@ function discoverSkills( .flatMap((skillFile): Array => { const childDir = dirname(skillFile) if (childDir === skillsDir) return [] - return [readSkillEntry(skillsDir, childDir, skillFile, readFs)] + const skill = readSkillEntry(skillsDir, childDir, skillFile, readFs) + if (!skill) { + warnings.push( + `Skipped unreadable or out-of-package skill metadata for "${packageName}".`, + ) + return [] + } + return [skill] }) } @@ -580,7 +603,8 @@ export function scanForIntents( createPackageRegistrar({ comparePackageVersions, deriveIntentConfig, - discoverSkills: (skillsDir) => discoverSkills(skillsDir, fsCache), + discoverSkills: (skillsDir, packageName) => + discoverSkills(skillsDir, packageName, fsCache, warnings), getPackageDepth, getPackageKind, getFsIdentity: fsCache.getFsIdentity, @@ -787,7 +811,8 @@ export function scanIntentPackageAtRoot( options.skillNameHint!, fsCache.getReadFs(), ) - : (skillsDir) => discoverSkills(skillsDir, fsCache), + : (skillsDir, packageName) => + discoverSkills(skillsDir, packageName, fsCache, warnings), getPackageDepth, getPackageKind, getFsIdentity: fsCache.getFsIdentity, diff --git a/packages/intent/src/shared/command-runner.ts b/packages/intent/src/shared/command-runner.ts index 0bd681a9..a908a610 100644 --- a/packages/intent/src/shared/command-runner.ts +++ b/packages/intent/src/shared/command-runner.ts @@ -11,11 +11,28 @@ const runnerByPackageManager: Record = { yarn: 'yarn dlx @tanstack/intent@latest', } +/** Use argument arrays for discovered identifiers; strings are trusted templates. */ export function formatIntentCommand( packageManager: PackageManager, - args: string, + args: string | ReadonlyArray, ): string { const command = runnerByPackageManager[packageManager] - const trimmedArgs = args.trim() + const trimmedArgs = + typeof args === 'string' + ? args.trim() + : args + .map((arg) => { + if ( + arg === '' || + arg.startsWith('#') || + /[^a-zA-Z0-9_./@#-]/.test(arg) + ) { + throw new Error( + 'Cannot generate an Intent command: identifiers must contain only letters, numbers, underscores, dots, slashes, @, #, and hyphens, and cannot start with #.', + ) + } + return arg + }) + .join(' ') return trimmedArgs ? `${command} ${trimmedArgs}` : command } diff --git a/packages/intent/src/skills/paths.ts b/packages/intent/src/skills/paths.ts index cc236722..c4c72238 100644 --- a/packages/intent/src/skills/paths.ts +++ b/packages/intent/src/skills/paths.ts @@ -1,6 +1,8 @@ import { existsSync } from 'node:fs' import { join, relative } from 'node:path' import { toPosixPath } from '../shared/utils.js' +import { formatIntentCommand } from '../shared/command-runner.js' +import { formatSkillUse } from './use.js' import type { SkillUse } from './use.js' import type { SkillEntry } from '../shared/types.js' @@ -64,7 +66,12 @@ export function rewriteSkillLoadPaths({ } export function formatRuntimeSkillLookupComment(target: SkillUse): string { - return `Runtime lookup only: run \`npx @tanstack/intent@latest load ${target.packageName}#${target.skillName} --path\`, and load its reported path for this session. Do not copy the resolved path into this file.` + const command = formatIntentCommand('npm', [ + 'load', + formatSkillUse(target.packageName, target.skillName), + '--path', + ]) + return `Runtime lookup only: run \`${command}\`, and load its reported path for this session. Do not copy the resolved path into this file.` } export function isRuntimeSkillLookupComment(value: string): boolean { diff --git a/packages/intent/tests/discovery-safety.test.ts b/packages/intent/tests/discovery-safety.test.ts new file mode 100644 index 00000000..f781756e --- /dev/null +++ b/packages/intent/tests/discovery-safety.test.ts @@ -0,0 +1,245 @@ +import { + existsSync, + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { execFileSync } from 'node:child_process' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { parse as parseYaml } from 'yaml' +import { main } from '../src/cli.js' +import { listIntentSkills, loadIntentSkill } from '../src/core/index.js' +import { scanForIntents } from '../src/discovery/scanner.js' +import { buildIntentSkillsBlock } from '../src/commands/install/guidance.js' +import { formatRuntimeSkillLookupHint } from '../src/skills/paths.js' +import { nodeReadFs } from '../src/shared/utils.js' +import { formatIntentCommand } from '../src/shared/command-runner.js' + +const packageName = '@scope/library' +let root: string +let originalCwd: string + +function write(file: string, content: string): void { + mkdirSync(dirname(file), { recursive: true }) + writeFileSync(file, content) +} + +function skillFile(description: string): string { + return `---\nname: core\ndescription: ${description}\n---\n\nSkill body.\n` +} + +function packageRoot(): string { + return join(root, 'node_modules', '@scope', 'library') +} + +beforeEach(() => { + root = realpathSync(mkdtempSync(join(tmpdir(), 'intent-discovery-safety-'))) + originalCwd = process.cwd() + write( + join(root, 'package.json'), + JSON.stringify({ intent: { skills: [packageName] } }), + ) + write( + join(packageRoot(), 'package.json'), + JSON.stringify({ + name: packageName, + version: '1.0.0', + intent: { version: 1, repo: 'scope/library', docs: 'docs/' }, + }), + ) +}) + +afterEach(() => { + process.chdir(originalCwd) + vi.restoreAllMocks() + rmSync(root, { recursive: true, force: true }) +}) + +describe('discovered command arguments', () => { + it.skipIf(process.platform === 'win32')( + 'round-trips scoped and nested identifiers through installed POSIX shells', + async () => { + const skillName = 'guide/nested-entry_v2.0' + const use = `${packageName}#${skillName}` + write( + join(packageRoot(), 'skills', skillName, 'SKILL.md'), + skillFile('Nested skill'), + ) + const scan = scanForIntents(root) + const block = buildIntentSkillsBlock(scan).block + const yaml = block.replace(//g, '') + const mappings = parseYaml(yaml) as { + tanstackIntent: Array<{ run: string }> + } + const command = mappings.tanstackIntent[0]!.run + expect(loadIntentSkill(use, { cwd: root }).skillName).toBe(skillName) + + process.chdir(root) + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + expect(await main(['list'])).toBe(0) + expect(log.mock.calls.flat().join('\n')).toContain(command) + + const hint = formatRuntimeSkillLookupHint({ packageName, skillName }) + const hintCommand = hint.split('`')[1]! + const runners = ['npm', 'pnpm', 'yarn', 'bun', 'unknown'] as const + const stubs = + 'npx() { printf "%s\\n" "$@"; }; pnpm() { printf "%s\\n" "$@"; }; yarn() { printf "%s\\n" "$@"; }; bunx() { printf "%s\\n" "$@"; }; ' + for (const shell of ['/bin/sh', '/bin/bash', '/bin/zsh'].filter( + existsSync, + )) { + for (const runner of runners) { + const generated = formatIntentCommand(runner, [ + 'load', + use, + '--global', + ]) + const actual = execFileSync(shell, ['-c', stubs + generated], { + encoding: 'utf8', + }) + .trim() + .split('\n') + expect(actual).toEqual([ + ...(runner === 'pnpm' || runner === 'yarn' ? ['dlx'] : []), + '@tanstack/intent@latest', + 'load', + use, + '--global', + ]) + } + expect( + execFileSync(shell, ['-c', stubs + command], { encoding: 'utf8' }) + .trim() + .split('\n'), + ).toEqual(['@tanstack/intent@latest', 'load', use]) + expect( + execFileSync(shell, ['-c', stubs + hintCommand], { encoding: 'utf8' }) + .trim() + .split('\n'), + ).toEqual(['@tanstack/intent@latest', 'load', use, '--path']) + } + }, + ) + + it.each([ + 'core;echo injected', + 'core$(echo injected)', + 'core`echo injected`', + 'core"quoted', + "core'quoted", + 'core with spaces', + 'core\nnext', + 'core\rnext', + 'core\tnext', + 'core%PATH%', + 'core!PATH!', + 'core&echo', + 'core|echo', + ])('refuses runnable commands for unsafe skill names: %j', (skillName) => { + // A scan result can contain names that are not valid filenames on this host. + write( + join(packageRoot(), 'skills', 'core', 'SKILL.md'), + skillFile('Safe description'), + ) + const scan = scanForIntents(root) + scan.packages[0]!.skills[0]!.name = skillName + + expect(() => buildIntentSkillsBlock(scan)).toThrow( + 'Cannot generate an Intent command', + ) + expect(() => + formatRuntimeSkillLookupHint({ packageName, skillName }), + ).toThrow('Cannot generate an Intent command') + }) + + it('refuses an unsafe identifier in runnable list output', async () => { + write( + join(packageRoot(), 'skills', 'core;echo injected', 'SKILL.md'), + skillFile('Safe description'), + ) + process.chdir(root) + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + + expect(await main(['list'])).toBe(1) + expect(error.mock.calls.flat().join('\n')).toContain( + 'Cannot generate an Intent command', + ) + expect(log.mock.calls.flat().join('\n')).not.toContain( + 'npx @tanstack/intent@latest load @scope/library#core;echo', + ) + }) +}) + +describe('discovered metadata containment', () => { + it('does not read escaping frontmatter through discovery or direct loading', () => { + const outside = join(root, 'outside', 'SKILL.md') + const link = join(packageRoot(), 'skills', 'core', 'SKILL.md') + write(outside, skillFile('EXTERNAL_METADATA_SENTINEL')) + mkdirSync(dirname(link), { recursive: true }) + symlinkSync(outside, link) + const read = vi.spyOn(nodeReadFs, 'readFileSync') + + const listed = listIntentSkills({ cwd: root, audience: 'human' }) + expect(listed.skills).toEqual([]) + expect(JSON.stringify(listed)).not.toContain('EXTERNAL_METADATA_SENTINEL') + expect(buildIntentSkillsBlock(scanForIntents(root)).mappingCount).toBe(0) + expect(() => loadIntentSkill(`${packageName}#core`, { cwd: root })).toThrow( + 'outside package root', + ) + expect( + read.mock.calls.some(([path]) => path === outside || path === link), + ).toBe(false) + }) + + it('keeps in-package skill symlinks loadable', () => { + const target = join(packageRoot(), 'references', 'core.md') + const link = join(packageRoot(), 'skills', 'core', 'SKILL.md') + write(target, skillFile('Internal metadata')) + mkdirSync(dirname(link), { recursive: true }) + symlinkSync(target, link) + + expect(listIntentSkills({ cwd: root }).skills[0]?.description).toBe( + 'Internal metadata', + ) + expect( + loadIntentSkill(`${packageName}#core`, { cwd: root }).content, + ).toContain('Skill body.') + }) + + it.each([packageName, '@scope/hidden"source'])( + 'redacts rejected metadata diagnostics for unlisted agent source %s', + (hiddenName) => { + const outside = join(root, 'outside', 'SKILL.md') + const link = join(packageRoot(), 'skills', 'core', 'SKILL.md') + write(outside, skillFile('EXTERNAL_METADATA_SENTINEL')) + write( + join(packageRoot(), 'package.json'), + JSON.stringify({ + name: hiddenName, + version: '1.0.0', + intent: { version: 1, repo: 'scope/library', docs: 'docs/' }, + }), + ) + mkdirSync(dirname(link), { recursive: true }) + symlinkSync(outside, link) + write( + join(root, 'package.json'), + JSON.stringify({ intent: { skills: [] } }), + ) + + const listed = listIntentSkills({ cwd: root, audience: 'agent' }) + expect(listed.warnings).toEqual([]) + expect(JSON.stringify(listed)).not.toContain(packageName) + expect(JSON.stringify(listed)).not.toContain( + JSON.stringify(hiddenName).slice(1, -1), + ) + expect(JSON.stringify(listed)).not.toContain(outside) + expect(JSON.stringify(listed)).not.toContain('EXTERNAL_METADATA_SENTINEL') + }, + ) +}) From bf84d3710e05df4b3e49dba5b179c995e278bd22 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Fri, 4 Sep 2026 19:35:33 -0700 Subject: [PATCH 2/4] fix: align benchmark fixtures with skill frontmatter schema --- benchmarks/intent/helpers.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/benchmarks/intent/helpers.ts b/benchmarks/intent/helpers.ts index 011fd52b..859e74f4 100644 --- a/benchmarks/intent/helpers.ts +++ b/benchmarks/intent/helpers.ts @@ -1,6 +1,6 @@ import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { dirname, join } from 'node:path' +import { basename, dirname, join } from 'node:path' let builtCliMainPromise: Promise< (argv?: Array) => Promise @@ -168,12 +168,20 @@ export function writeSkill( options: SkillOptions, ): void { const frontmatter = [ - `name: ${JSON.stringify(skillName)}`, + `name: ${JSON.stringify(basename(skillName))}`, `description: ${JSON.stringify(options.description)}`, ] - if (options.type) { - frontmatter.push(`type: ${JSON.stringify(options.type)}`) + if (options.type || options.libraryVersion) { + frontmatter.push('metadata:') + if (options.type) { + frontmatter.push(` type: ${JSON.stringify(options.type)}`) + } + if (options.libraryVersion) { + frontmatter.push( + ` library_version: ${JSON.stringify(options.libraryVersion)}`, + ) + } } if (options.requires) { @@ -183,12 +191,6 @@ export function writeSkill( } } - if (options.libraryVersion) { - frontmatter.push( - `library_version: ${JSON.stringify(options.libraryVersion)}`, - ) - } - if (options.sources) { frontmatter.push('sources:') for (const source of options.sources) { From 98f23d273bc7e38b6ee4ee79aead950d746f1a99 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Fri, 4 Sep 2026 19:43:25 -0700 Subject: [PATCH 3/4] docs: align list reference with install documentation --- docs/cli/intent-list.md | 197 +++++++++++++++++++++------------------- 1 file changed, 102 insertions(+), 95 deletions(-) diff --git a/docs/cli/intent-list.md b/docs/cli/intent-list.md index 0db2c728..7e478471 100644 --- a/docs/cli/intent-list.md +++ b/docs/cli/intent-list.md @@ -3,7 +3,7 @@ title: intent list id: intent-list --- -`intent list` discovers skill-enabled packages and prints available skills. +`intent list` discovers skill-enabled packages and shows the skills available under the project's permissions and exclusions. It does not change permissions or write guidance. ```bash npx @tanstack/intent@latest list [--json] [--debug] [--global] [--global-only] [--show-hidden] [--no-notices] @@ -13,40 +13,81 @@ npx @tanstack/intent@latest list [--json] [--debug] [--global] [--global-only] [ ### Output -- `--json`: print JSON instead of text output -- `--debug`: print discovery debug details to stderr -- `--no-notices`: suppress non-critical notices on stderr; the acknowledged-risk notice for `intent.skills: ["*"]` remains visible +- `--json`: print structured skills, packages, and diagnostics instead of text output +- `--debug`: print discovery details to stderr, including scan counts and package.json reads +- `--show-hidden`: include a hidden-source summary in text output when run outside an agent session +- `--no-notices`: suppress non-critical notices in text mode; the notice for `intent.skills: ["*"]` remains visible ### Scan scope - `--global`: include global packages after project packages - `--global-only`: list global packages only -- `--show-hidden`: show unlisted hidden skill sources when run outside an agent session -## What you get +## Behavior -### Selection +### Default list -- Scans project and workspace dependencies for intent-enabled packages and skills -- Surfaces packages permitted by `package.json#intent.skills` (see [Allowlist](#allowlist)) -- Includes global packages only when `--global` or `--global-only` is passed -- Excludes packages and skills matched by package.json `intent.exclude` +Intent scans project and workspace dependencies, applies `package.json#intent.skills`, then removes packages and skills matched by `intent.exclude`. It uses project `node_modules` when available and Yarn's PnP API in PnP projects without usable `node_modules`. -When both local and global packages are scanned, local packages take precedence. `SOURCE` shows whether the selected package came from local discovery or explicit global scanning. +Global packages are scanned only with `--global` or `--global-only`. When both local and global copies of a package are found, the local copy takes precedence. Version conflicts show the chosen package and other discovered versions and paths. -### Text output +Run [intent install](./intent-install) to configure permissions on first use. Listing skills does not open the install picker. -- Summary line with package count and skill count -- Package table columns: `PACKAGE`, `SOURCE`, `VERSION`, `SKILLS` -- Skill tree grouped by package -- Discovery warnings (`⚠ ...`) on stdout -- `No intent-enabled packages found.` when no packages are discovered +### Which skills appear -Policy notices (`ℹ ...`) are written to stderr. +The nearest configured `intent.skills` list applies, including inherited workspace permissions. Each entry enables a package, a package pattern, or one exact skill: + +| Saved rule | Skills included | Includes future additions? | +| --- | --- | --- | +| `"*"` | All discovered npm and workspace sources. | All packages and skills. | +| `"@tanstack/query"` | All skills in that npm package. | New skills in the package. | +| `"@tanstack/*"` | All skills in matching npm packages. | New matching packages and skills. | +| `"@tanstack/query#fetching"` | The `fetching` skill in that package. | Only that skill name. | +| `"workspace:@scope/internal"` | All skills in that workspace package. | New skills in the package. | + +Workspace patterns and individual skills also use the `workspace:` prefix, such as `workspace:@scope/*` and `workspace:@scope/internal#testing`. Package patterns support `*`; individual-skill entries require an exact package and skill name. Git sources are not supported. + +- **No configured list:** all discovered sources appear, with a migration notice. This is the existing-project upgrade path; a future version will require explicit permissions. +- **An empty list (`[]`):** no sources are permitted, with an informational notice. +- **All sources (`["*"]`):** all discovered sources appear, with a notice that unvetted skills may enter agent guidance. + +Permissions select sources and skill names. They do not freeze skill content when dependencies update. See [Configuration](../concepts/configuration) and [Trust model](../concepts/trust-model). + +### Exclusions + +`intent.exclude` takes precedence over permissions. Intent combines exclusions from package.json files between the workspace or project root and the current directory. + +| Exclusion | Effect | +| --- | --- | +| `@tanstack/*devtools*` | Excludes matching packages. | +| `@tanstack/query#experimental-*` | Excludes matching skills in that package. | +| `*#experimental-*` | Excludes matching skills across packages. | +| `@tanstack/query#*` | Excludes the whole package. | + +Only exact names and `*` wildcards are supported. Excluded packages do not trigger unlisted-source notices. Manage exclusions with [intent exclude](./intent-exclude). + +### Hidden sources + +Packages outside an explicit allowlist are omitted from the available catalog. In a human session, a policy notice names them; `--show-hidden` adds their names and skill counts to the text output. This does not enable them. + +In agent sessions, hidden sources are reported by count only. `--show-hidden` cannot reveal their identities there; run it outside the agent session to review candidates. A configured package or package pattern that was not discovered also produces a notice. + +## Default output + +Text output includes: + +- A summary with package and skill counts. +- A package table with `PACKAGE`, `SOURCE`, `VERSION`, and `SKILLS` columns. +- A skill tree grouped by package, with descriptions and commands to load each skill. +- Version conflicts and discovery warnings, when present. + +Load commands use the detected package manager and preserve the selected global scan scope. `SOURCE` distinguishes local discovery from explicit global scanning. + +Text output and discovery warnings go to stdout. Policy notices and `--debug` details go to stderr. ## JSON output -`--json` prints an adapter-friendly skill list: +`--json` prints a structured catalog to stdout. This example shows one available skill with no hidden sources or diagnostics; paths and package metadata vary by project: ```json { @@ -59,8 +100,8 @@ Policy notices (`ℹ ...`) are written to stderr. "packageSource": "local", "skillName": "fetching", "description": "Query data fetching patterns", - "type": "skill (optional)", - "framework": "react (optional)" + "type": "core", + "framework": "react" } ], "packages": [ @@ -72,83 +113,49 @@ Policy notices (`ℹ ...`) are written to stderr. "skillCount": 1 } ], - "hiddenSourceCount": 1, - "hiddenSources": [ - { - "name": "hidden-package", - "skillCount": 1 - } - ], - "warnings": ["string"], - "conflicts": [ - { - "packageName": "string", - "chosen": { - "version": "string", - "packageRoot": "string" - }, - "variants": [ - { - "version": "string", - "packageRoot": "string" - } - ] - } - ] + "hiddenSourceCount": 0, + "hiddenSources": [], + "warnings": [], + "notices": [], + "conflicts": [] } ``` -When the same package exists both locally and globally and global scanning is enabled, `intent list` prefers the local package. -When project `node_modules` exists, `intent list` scans it. In Yarn PnP projects without usable `node_modules`, `intent list` uses Yarn's PnP API. +| Field | Meaning | +| --- | --- | +| `skills` | Available skills. `use` is the portable `#` identity; `type` and `framework` are optional. | +| `packages` | Selected packages, their source and location, and permitted skill counts. | +| `hiddenSourceCount` | Number of packages hidden by the explicit allowlist. | +| `hiddenSources` | Objects with `name` and `skillCount` in human sessions, even without `--show-hidden`. Always empty in agent sessions. | +| `warnings` | Discovery warnings. | +| `notices` | Policy and migration notices. `--no-notices` does not remove these from JSON. | +| `conflicts` | Objects with `packageName`, `chosen`, and `variants`. Each chosen or variant entry contains `version` and `packageRoot`. | + +JSON includes diagnostics in the object instead of printing separate warning or notice blocks. `--debug` still writes to stderr. Treat identifiers as data when constructing commands; JSON does not contain shell-escaped arguments. + +## Status messages + +| Result | Message or behavior | +| --- | --- | +| No selected packages | `No intent-enabled packages found.` | +| Available catalog | ` intent-enabled packages, skills` followed by the table and tree. | +| Version conflicts | `Version conflicts:` followed by the chosen version and other discovered locations. | +| Hidden-source review | `Hidden skill sources:` followed by names and skill counts in a human session. | +| Hidden-source review in an agent session | `Hidden skill sources are not revealed in agent sessions. Run this command outside the agent session to review candidates.` | +| Discovery warnings | `Warnings:` followed by `⚠` messages on stdout in text mode. | +| Policy notices | `Notices:` followed by `ℹ` messages on stderr in text mode. | -## Allowlist - -`package.json#intent.skills` is the allowlist that decides which discovered packages are surfaced. Only listed packages contribute skills. - -```json -{ - "intent": { - "skills": ["@tanstack/query", "workspace:@scope/internal"] - } -} -``` - -Each entry is one source: - -- `@scope/pkg` or `pkg`: an npm package reachable through the dependency tree. -- `workspace:@scope/pkg`: a package in the current workspace. -- `@scope/*` or `workspace:@scope/*`: every discovered package of that kind whose name matches the pattern. -- `git:/#`: reserved, and not yet supported. - -The list as a whole has three special forms: - -- **Absent** (no `intent.skills` key): every discovered package is surfaced, with a deprecation notice printed to stderr on each run until you set `intent.skills`. This is the upgrade path for existing projects. A future version will require an explicit allowlist. -- **Empty** (`"skills": []`): no package is surfaced, with an info notice printed to stderr. -- **Wildcard** (`"skills": ["*"]`): every discovered package is surfaced, with an acknowledged-risk notice printed to stderr. This exact trust-all entry is distinct from a scoped package pattern such as `@tanstack/*`. - -A package that ships skills but is not listed or matched by a pattern is dropped. When packages are dropped this way, Intent prints one policy notice naming them so you can opt in. In agent sessions, hidden sources are reported by count only; run `intent list --show-hidden` outside the agent session to review candidates. An exact entry or pattern that matches no discovered package is reported as well. Package patterns support `*` wildcards. Matching uses both package name and source kind. See [Configuration](../concepts/configuration) and [Trust model](../concepts/trust-model). - -## Excludes - -Package excludes are hard filters for packages that should not be used in a repo, applied after the allowlist. -Intent reads `intent.exclude` arrays from package.json files while walking from the workspace or project root to the current working directory. -Manage persistent excludes with `intent exclude add|remove|list`. - -```json -{ - "intent": { - "exclude": ["@tanstack/*devtools*", "@tanstack/router#experimental-*"] - } -} -``` - -A pattern without `#` excludes a whole package. A pattern with `#` excludes a single skill (`@scope/pkg#search-params`), and the skill segment may itself be a glob (`@scope/pkg#experimental-*`). A pattern may cross package boundaries at skill granularity (`*#experimental-*`). The `#*` shortcut (`@scope/pkg#*`) excludes the whole package. Only exact names and `*` wildcards are supported on each segment. Bare package-name patterns keep working unchanged. +## Common errors -An excluded package never triggers the unlisted-source notice, because an exclude is an explicit decision rather than an oversight. +- **Invalid permissions or unreadable policy files:** Intent stops and reports the problem. Fix the reported package.json or `intent.skills` entry before retrying. +- **Unsupported runnable identifier:** generated commands accept only ASCII letters, numbers, `_`, `.`, `/`, `@`, `#`, and `-`. Identifiers cannot start with `#`. Rename the package or skill to generate runnable guidance. `--json` can still expose permitted identifiers as data. +- **Unreadable or out-of-package skill metadata:** discovery skips skill files whose real path cannot be resolved or lies outside the package root, with a warning. Symlinks within the resolved package root remain supported. External frontmatter is not read into the catalog. +- **Deno without `node_modules`:** this discovery mode is unsupported. -## Common errors +## Related -- Scanner failures are printed as errors -- Deno projects without `node_modules` are unsupported -- Runnable commands require identifiers containing only ASCII letters, numbers, `_`, `.`, `/`, `@`, `#`, and `-`. Identifiers cannot start with `#`. Other characters cause an error before a command is emitted; rename the package or skill to generate runnable guidance. `--json` still exposes permitted identifiers as data, not shell commands. -- Discovery skips skill files whose real path cannot be resolved or lies outside their package root, with a warning. Symlinks to files inside the resolved package root remain supported. External frontmatter is not read or included in catalogs or mappings. +- [intent install](./intent-install) +- [intent load](./intent-load) +- [intent exclude](./intent-exclude) +- [Configuration](../concepts/configuration) +- [Trust model](../concepts/trust-model) From f54ca1934beeaf1c01f7d4fe2a73d0bfa326803c Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Fri, 4 Sep 2026 19:59:02 -0700 Subject: [PATCH 4/4] fix: bind skill metadata reads to checked file descriptors --- .changeset/safe-discovery-boundaries.md | 2 +- docs/cli/intent-list.md | 4 +- .../intent/src/commands/install/guidance.ts | 2 +- packages/intent/src/commands/list.ts | 2 +- packages/intent/src/discovery/scanner.ts | 29 +++- packages/intent/src/shared/utils.ts | 29 ++-- packages/intent/src/skills/paths.ts | 3 +- .../intent/tests/discovery-safety.test.ts | 141 ++++++++++++++++++ 8 files changed, 191 insertions(+), 21 deletions(-) diff --git a/.changeset/safe-discovery-boundaries.md b/.changeset/safe-discovery-boundaries.md index a6318f98..fa3884ef 100644 --- a/.changeset/safe-discovery-boundaries.md +++ b/.changeset/safe-discovery-boundaries.md @@ -2,4 +2,4 @@ '@tanstack/intent': patch --- -Reject unsafe dynamic identifiers in runnable list commands, install mappings, and runtime lookup hints. Skip skill metadata that resolves outside its package root while preserving valid package-manager symlinks and direct-load containment errors. +Reject unsafe dynamic identifiers before normalization in runnable list commands, install mappings, and runtime lookup hints. Skip skill metadata that resolves outside its package root, verify the opened file's identity before reading, and keep large-frontmatter reads on the same descriptor. Preserve valid package-manager symlinks and direct-load containment errors. diff --git a/docs/cli/intent-list.md b/docs/cli/intent-list.md index 7e478471..7fe6438e 100644 --- a/docs/cli/intent-list.md +++ b/docs/cli/intent-list.md @@ -148,8 +148,8 @@ JSON includes diagnostics in the object instead of printing separate warning or ## Common errors - **Invalid permissions or unreadable policy files:** Intent stops and reports the problem. Fix the reported package.json or `intent.skills` entry before retrying. -- **Unsupported runnable identifier:** generated commands accept only ASCII letters, numbers, `_`, `.`, `/`, `@`, `#`, and `-`. Identifiers cannot start with `#`. Rename the package or skill to generate runnable guidance. `--json` can still expose permitted identifiers as data. -- **Unreadable or out-of-package skill metadata:** discovery skips skill files whose real path cannot be resolved or lies outside the package root, with a warning. Symlinks within the resolved package root remain supported. External frontmatter is not read into the catalog. +- **Unsupported runnable identifier:** generated commands accept only ASCII letters, numbers, `_`, `.`, `/`, `@`, `#`, and `-`. Identifiers cannot start with `#`; leading and trailing whitespace is rejected rather than trimmed. Rename the package or skill to generate runnable guidance. `--json` can still expose permitted identifiers as data. +- **Unreadable or out-of-package skill metadata:** discovery skips skill files whose real path cannot be resolved or lies outside the package root, with a warning. It checks the opened file's identity before reading and uses that descriptor for the full metadata read, including large frontmatter, so later pathname replacement cannot redirect the read. Symlinks within the resolved package root remain supported. - **Deno without `node_modules`:** this discovery mode is unsupported. ## Related diff --git a/packages/intent/src/commands/install/guidance.ts b/packages/intent/src/commands/install/guidance.ts index 9ddc89a7..f76827d3 100644 --- a/packages/intent/src/commands/install/guidance.ts +++ b/packages/intent/src/commands/install/guidance.ts @@ -306,7 +306,7 @@ export function buildIntentSkillsBlock( ` run: ${quoteYamlString( formatIntentCommand(scanResult.packageManager, [ 'load', - formatSkillUse(pkg.name, skill.name), + `${pkg.name}#${skill.name}`, ]), )}`, ) diff --git a/packages/intent/src/commands/list.ts b/packages/intent/src/commands/list.ts index 7b02fd77..f5d4201a 100644 --- a/packages/intent/src/commands/list.ts +++ b/packages/intent/src/commands/list.ts @@ -88,7 +88,7 @@ function formatLoadCommand( ): string { return formatIntentCommand(packageManager, [ 'load', - skill.use, + `${skill.packageName}#${skill.skillName}`, ...(scopeFlag ? [scopeFlag.trim()] : []), ]) } diff --git a/packages/intent/src/discovery/scanner.ts b/packages/intent/src/discovery/scanner.ts index 728fe9ae..2215a7b1 100644 --- a/packages/intent/src/discovery/scanner.ts +++ b/packages/intent/src/discovery/scanner.ts @@ -2,7 +2,7 @@ // executes discovered package code. The only sanctioned dynamic load is Yarn's // PnP runtime (.pnp.cjs / pnpapi), used solely to map identities to readable // roots. Enforced by the `intent/static-discovery` ESLint rule. -import { existsSync } from 'node:fs' +import { constants, existsSync } from 'node:fs' import { createRequire } from 'node:module' import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path' import semver from 'semver' @@ -266,15 +266,36 @@ function readSkillEntry( skillFile: string, readFs: ReadFs = nodeReadFs, ): SkillEntry | null { - let realSkillFile: string + if (!readFs.openSync || !readFs.fstatSync || !readFs.closeSync) return null + let fd: number | undefined + let fm: Record | null try { - realSkillFile = readFs.realpathSync.native(skillFile) + const realSkillFile = readFs.realpathSync.native(skillFile) const realPackageRoot = readFs.realpathSync.native(dirname(skillsDir)) if (!isWithinOrEqual(realSkillFile, realPackageRoot)) return null + const expected = readFs.lstatSync(realSkillFile) + if (!expected.isFile()) return null + if (readFs.realpathSync.native(realSkillFile) !== realSkillFile) return null + + fd = readFs.openSync( + realSkillFile, + constants.O_RDONLY | constants.O_NOFOLLOW, + ) + const opened = readFs.fstatSync(fd) + // Compare the opened file with the checked entry before reading any bytes. + // The descriptor then survives pathname replacement, including parent swaps. + if ( + !opened.isFile() || + opened.dev !== expected.dev || + opened.ino !== expected.ino + ) + return null + fm = parseFrontmatter(fd, readFs) } catch { return null + } finally { + if (fd !== undefined) readFs.closeSync(fd) } - const fm = parseFrontmatter(realSkillFile, readFs) const relName = toPosixPath(relative(skillsDir, childDir)) const desc = typeof fm?.description === 'string' diff --git a/packages/intent/src/shared/utils.ts b/packages/intent/src/shared/utils.ts index 2fc1679c..1d7ac0d7 100644 --- a/packages/intent/src/shared/utils.ts +++ b/packages/intent/src/shared/utils.ts @@ -2,6 +2,7 @@ import { execFileSync } from 'node:child_process' import { closeSync, existsSync, + fstatSync, lstatSync, openSync, readFileSync, @@ -35,6 +36,7 @@ export interface ReadFs { openSync?: typeof openSync readSync?: typeof readSync closeSync?: typeof closeSync + fstatSync?: typeof fstatSync } export const nodeReadFs: ReadFs = { @@ -46,6 +48,7 @@ export const nodeReadFs: ReadFs = { openSync, readSync, closeSync, + fstatSync, } /** @@ -391,10 +394,11 @@ export function readScalarField( } /** - * Parse YAML frontmatter from a file. Returns null if no frontmatter or on error. + * Parse YAML frontmatter from a path or caller-owned descriptor at offset zero. + * Returns null if no frontmatter or on error; caller-owned descriptors stay open. */ export function parseFrontmatter( - filePath: string, + filePath: string | number, fs: ReadFs = nodeReadFs, ): Record | null { const content = readFrontmatterRegion(filePath, fs) @@ -418,13 +422,14 @@ const frontmatterBuffer = Buffer.allocUnsafe(FRONTMATTER_READ_LIMIT) * instead of its whole body. Falls back to a full read when the bounded read * primitives are unavailable or the frontmatter exceeds the probe limit. */ -function readFrontmatterRegion(filePath: string, fs: ReadFs): string | null { +function readFrontmatterRegion( + filePath: string | number, + fs: ReadFs, +): string | null { if (fs.openSync && fs.readSync && fs.closeSync) { - let region: string | null = null - let truncated = false let fd: number try { - fd = fs.openSync(filePath, 'r') + fd = typeof filePath === 'number' ? filePath : fs.openSync(filePath, 'r') } catch { return null } @@ -436,16 +441,20 @@ function readFrontmatterRegion(filePath: string, fs: ReadFs): string | null { FRONTMATTER_READ_LIMIT, 0, ) - region = frontmatterBuffer.toString('utf8', 0, bytesRead) + const region = frontmatterBuffer.toString('utf8', 0, bytesRead) // A full buffer means the file may extend past the probe window; only // trust the bounded read when it captured the closing fence. - truncated = + const truncated = bytesRead === FRONTMATTER_READ_LIMIT && !/\r?\n---/.test(region.slice(3)) + // The positional probe leaves the descriptor at offset zero. Keep the + // full-read fallback on that descriptor so replacement cannot redirect it. + return truncated ? fs.readFileSync(fd, 'utf8') : region + } catch { + return null } finally { - fs.closeSync(fd) + if (typeof filePath !== 'number') fs.closeSync(fd) } - if (!truncated) return region } try { diff --git a/packages/intent/src/skills/paths.ts b/packages/intent/src/skills/paths.ts index c4c72238..9dc68e61 100644 --- a/packages/intent/src/skills/paths.ts +++ b/packages/intent/src/skills/paths.ts @@ -2,7 +2,6 @@ import { existsSync } from 'node:fs' import { join, relative } from 'node:path' import { toPosixPath } from '../shared/utils.js' import { formatIntentCommand } from '../shared/command-runner.js' -import { formatSkillUse } from './use.js' import type { SkillUse } from './use.js' import type { SkillEntry } from '../shared/types.js' @@ -68,7 +67,7 @@ export function rewriteSkillLoadPaths({ export function formatRuntimeSkillLookupComment(target: SkillUse): string { const command = formatIntentCommand('npm', [ 'load', - formatSkillUse(target.packageName, target.skillName), + `${target.packageName}#${target.skillName}`, '--path', ]) return `Runtime lookup only: run \`${command}\`, and load its reported path for this session. Do not copy the resolved path into this file.` diff --git a/packages/intent/tests/discovery-safety.test.ts b/packages/intent/tests/discovery-safety.test.ts index f781756e..6eddf249 100644 --- a/packages/intent/tests/discovery-safety.test.ts +++ b/packages/intent/tests/discovery-safety.test.ts @@ -3,6 +3,7 @@ import { mkdirSync, mkdtempSync, realpathSync, + renameSync, rmSync, symlinkSync, writeFileSync, @@ -139,6 +140,9 @@ describe('discovered command arguments', () => { 'core!PATH!', 'core&echo', 'core|echo', + ' core', + 'core ', + '\tcore\n', ])('refuses runnable commands for unsafe skill names: %j', (skillName) => { // A scan result can contain names that are not valid filenames on this host. write( @@ -156,6 +160,42 @@ describe('discovered command arguments', () => { ).toThrow('Cannot generate an Intent command') }) + it.each([' @scope/library', '@scope/library ', '\t@scope/library\n'])( + 'refuses whitespace-wrapped package names: %j', + (unsafePackage) => { + write( + join(packageRoot(), 'skills', 'core', 'SKILL.md'), + skillFile('Safe'), + ) + const scan = scanForIntents(root) + scan.packages[0]!.name = unsafePackage + expect(() => buildIntentSkillsBlock(scan)).toThrow( + 'Cannot generate an Intent command', + ) + expect(() => + formatRuntimeSkillLookupHint({ + packageName: unsafePackage, + skillName: 'core', + }), + ).toThrow('Cannot generate an Intent command') + }, + ) + + it('does not emit a trimmed identifier for a different existing skill', async () => { + write( + join(packageRoot(), 'skills', 'core', 'SKILL.md'), + skillFile('Other skill'), + ) + write( + join(packageRoot(), 'skills', ' core ', 'SKILL.md'), + skillFile('Whitespace skill'), + ) + process.chdir(root) + vi.spyOn(console, 'log').mockImplementation(() => {}) + vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(await main(['list'])).toBe(1) + }) + it('refuses an unsafe identifier in runnable list output', async () => { write( join(packageRoot(), 'skills', 'core;echo injected', 'SKILL.md'), @@ -176,6 +216,107 @@ describe('discovered command arguments', () => { }) describe('discovered metadata containment', () => { + it('rejects a parent-directory replacement during the identity check', () => { + const file = join(packageRoot(), 'skills', 'core', 'SKILL.md') + const outside = join(root, 'outside', 'SKILL.md') + write(file, skillFile('Internal')) + write(outside, skillFile('EXTERNAL_METADATA_SENTINEL')) + const lstat = nodeReadFs.lstatSync + let replaced = false + vi.spyOn(nodeReadFs, 'lstatSync').mockImplementation( + (...args: Parameters) => { + if (args[0] === file && !replaced) { + replaced = true + renameSync(dirname(file), `${dirname(file)}.original`) + symlinkSync(dirname(outside), dirname(file), 'junction') + } + return lstat(...args) + }, + ) + const read = vi.spyOn(nodeReadFs, 'readSync') + const result = listIntentSkills({ cwd: root }) + expect(replaced).toBe(true) + expect(result.skills).toEqual([]) + expect(read).not.toHaveBeenCalled() + }) + + it.each(['file', 'directory'] as const)( + 'rejects a %s replaced with an escaping symlink before open', + (kind) => { + const file = join(packageRoot(), 'skills', 'core', 'SKILL.md') + const outside = join(root, 'outside', 'SKILL.md') + write(file, skillFile('Internal')) + write(outside, skillFile('EXTERNAL_METADATA_SENTINEL')) + const open = nodeReadFs.openSync! + const read = vi.spyOn(nodeReadFs, 'readSync') + const close = vi.spyOn(nodeReadFs, 'closeSync') + let replaced = false + vi.spyOn(nodeReadFs, 'openSync').mockImplementation( + (path, flags, mode) => { + if (path === file && !replaced) { + replaced = true + const target = kind === 'file' ? file : dirname(file) + renameSync(target, `${target}.original`) + symlinkSync( + kind === 'file' ? outside : dirname(outside), + target, + kind === 'file' ? 'file' : 'junction', + ) + } + return open(path, flags, mode) + }, + ) + const result = listIntentSkills({ cwd: root }) + expect(replaced).toBe(true) + expect(result.skills).toEqual([]) + expect(read).not.toHaveBeenCalled() + if (kind === 'directory') expect(close).toHaveBeenCalledTimes(1) + expect(JSON.stringify(result)).not.toContain('EXTERNAL_METADATA_SENTINEL') + }, + ) + + it.each([false, true])( + 'reads the validated descriptor after replacement (large frontmatter: %s)', + (large) => { + const file = join(packageRoot(), 'skills', 'core', 'SKILL.md') + const outside = join(root, 'outside', 'SKILL.md') + const internal = large + ? `---\nname: core\npadding: ${'x'.repeat(20_000)}\ndescription: Internal\n---\n` + : skillFile('Internal') + write(file, internal) + write(outside, skillFile('EXTERNAL_METADATA_SENTINEL')) + const read = nodeReadFs.readSync! + let replaced = false + vi.spyOn(nodeReadFs, 'readSync').mockImplementation( + (...args: Parameters) => { + if (!replaced) { + replaced = true + renameSync(file, `${file}.original`) + symlinkSync(outside, file) + } + return read(...args) + }, + ) + const result = listIntentSkills({ cwd: root }) + expect(replaced).toBe(true) + expect(result.skills[0]?.description).toBe('Internal') + expect(JSON.stringify(result)).not.toContain('EXTERNAL_METADATA_SENTINEL') + }, + ) + + it('closes the descriptor when reading frontmatter fails', () => { + write( + join(packageRoot(), 'skills', 'core', 'SKILL.md'), + skillFile('Internal'), + ) + vi.spyOn(nodeReadFs, 'readSync').mockImplementation(() => { + throw new Error('read failed') + }) + const close = vi.spyOn(nodeReadFs, 'closeSync') + expect(listIntentSkills({ cwd: root }).skills[0]?.description).toBe('') + expect(close).toHaveBeenCalledTimes(1) + }) + it('does not read escaping frontmatter through discovery or direct loading', () => { const outside = join(root, 'outside', 'SKILL.md') const link = join(packageRoot(), 'skills', 'core', 'SKILL.md')