From a2e59255ee2d0f58e3e28dfff7498c82e22c2f53 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 9 Sep 2026 03:24:28 +0000 Subject: [PATCH] test: limit native skill discovery to runtime assets --- test/helpers/hermetic-env.ts | 70 +++++++++++++++-- test/hermetic-skill-runtime.test.ts | 3 + test/hermetic-skills-seeding.test.ts | 112 ++++++++++++++++++++++++++- test/hermetic-wiring.test.ts | 14 +++- 4 files changed, 188 insertions(+), 11 deletions(-) diff --git a/test/helpers/hermetic-env.ts b/test/helpers/hermetic-env.ts index 012c22077..5658a7650 100644 --- a/test/helpers/hermetic-env.ts +++ b/test/helpers/hermetic-env.ts @@ -249,13 +249,72 @@ export function getHermeticDirs(): HermeticDirs { let cachedSkillsConfigDir: string | null = null; +/** + * Canonical paths without exposing the checkout to Claude's recursive markdown + * file index. Directory links would also expose .context, dependencies, and + * generated host registries. Expand owned runtime directories and link files + * individually, preserving their source realpaths and executable bits. + */ +export function seedHermeticRuntimeView(root: string, destination: string): void { + const sourceRoot = fs.realpathSync(root); + const excluded = new Set(['node_modules', 'test', 'tests']); + const roots = new Set([ + 'SKILL.md', 'ETHOS.md', 'VERSION', 'package.json', 'bin', 'lib', 'scripts', + 'browse', 'browser-skills', 'design', 'extension', 'model-overlays', 'agents', + // On-demand protocol/reference files explicitly read by shipped skills. + 'docs/askuserquestion-split.md', 'docs/askuserquestion-cjk.md', + 'docs/designs/PLAN_TUNING_V0.md', 'docs/designs/PLAN_TUNING_V1.md', + ...skillCensus(root).physicalSkillFiles.filter(file => file !== 'SKILL.md').map(file => path.dirname(file)), + ]); + const allowed = [...roots].filter(name => fs.existsSync(path.join(root, name))).map(name => ({ + path: fs.realpathSync(path.join(root, name)), directory: fs.statSync(path.join(root, name)).isDirectory(), + })); + function link(source: string, target: string, ancestors: Set): void { + const real = fs.realpathSync(source); + if (real !== sourceRoot && !real.startsWith(sourceRoot + path.sep)) + throw new Error(`Runtime asset leaves the owned checkout: ${source}`); + if (path.relative(sourceRoot, real).split(path.sep).some(name => + name.startsWith('.') || excluded.has(name) || name.endsWith('.tmpl'))) + throw new Error(`Runtime asset resolves into an excluded tree: ${source}`); + if (real !== sourceRoot && !allowed.some(asset => real === asset.path || + asset.directory && real.startsWith(asset.path + path.sep))) + throw new Error(`Runtime asset resolves into an excluded tree: ${source}`); + const stat = fs.statSync(source); + if (stat.isDirectory()) { + if (ancestors.has(real)) throw new Error(`Circular runtime asset: ${source}`); + fs.mkdirSync(target); + const next = new Set([...ancestors, real]); + for (const name of fs.readdirSync(source)) { + if (name.startsWith('.') || excluded.has(name) || name.endsWith('.tmpl')) continue; + link(path.join(source, name), path.join(target, name), next); + } + } else if (stat.isFile()) { + fs.symlinkSync(source, target, 'file'); + } + } + fs.mkdirSync(destination); + try { + for (const name of roots) { + const source = path.join(root, name); + if (fs.existsSync(source)) { + const target = path.join(destination, name); + fs.mkdirSync(path.dirname(target), { recursive: true }); + link(source, target, new Set([sourceRoot])); + } + } + } catch (error) { + fs.rmSync(destination, { recursive: true, force: true }); + throw error; + } +} + /** * A hermetic CLAUDE_CONFIG_DIR with the repo's shipped skills REGISTERED in * user scope, mirroring ./setup's registration exactly: each discovered skill * gets a REAL directory `/skills//` containing a * SYMLINK to that skill's SKILL.md (absolute path), plus symlinks to its - * runtime assets using setup's exclusions. The gstack checkout itself also - * lives at `/skills/gstack`, as in an installed setup, so canonical + * runtime assets using setup's exclusions. An owned runtime view also + * lives at `/skills/gstack`, so canonical * lazy-section paths work alongside flattened discovery. registryName is the frontmatter `name:` * (dir-name fallback), NO gstack- prefix; the root SKILL.md router registers * as `_gstack-command`. skillCensus().registryEntries is the authoritative @@ -318,10 +377,9 @@ export function hermeticSkillsConfigDir(): string { } } } - // Claude can resolve canonical skill paths from CLAUDE_CONFIG_DIR rather - // than HOME. Expose the same runtime checkout here as well as the flattened - // entries above; never send a missing lazy-section lookup to operator state. - fs.symlinkSync(root, path.join(skillsDir, 'gstack'), 'dir'); + // Canonical lazy paths remain available without letting native file-index + // discovery recursively read the source checkout's historical artifacts. + seedHermeticRuntimeView(root, path.join(skillsDir, 'gstack')); cachedSkillsConfigDir = configDir; return configDir; } diff --git a/test/hermetic-skill-runtime.test.ts b/test/hermetic-skill-runtime.test.ts index c708b104a..3d824bc30 100644 --- a/test/hermetic-skill-runtime.test.ts +++ b/test/hermetic-skill-runtime.test.ts @@ -49,11 +49,13 @@ const args = process.argv.slice(2); const addDir = args.includes('--add-dir') ? args[args.indexOf('--add-dir')+1] : null; const phase = spawnSync('bash',['-c','cat ~/.claude/skills/gstack/autoplan/sections/ceo-phase.md'],{timeout:5000}); const config = spawnSync('bash',['-c','"$HOME/.claude/skills/gstack/bin/gstack-config" get codex_reviews'],{timeout:5000}); +const canonicalConfig = process.env.CLAUDE_CONFIG_DIR && spawnSync('bash',['-c','"$CLAUDE_CONFIG_DIR/skills/gstack/bin/gstack-config" get codex_reviews'],{timeout:5000}); const codexHome = process.env.CODEX_HOME || path.join(process.env.HOME,'.codex'); const record = { pid:process.pid, home:process.env.HOME, root:fs.realpathSync(root), args, phaseExit:phase.status, phaseHash:createHash('sha256').update(phase.stdout).digest('hex'), configExit:config.status, configValue:config.stdout.toString().trim(), + canonicalConfigExit:canonicalConfig?.status, canonicalConfigValue:canonicalConfig?.stdout.toString().trim(), configDir:process.env.CLAUDE_CONFIG_DIR, state:process.env.GSTACK_HOME, runtimeAllowed:addDir !== null && fs.realpathSync(addDir) === fs.realpathSync(root), claudeAuthPreserved:process.env.ANTHROPIC_API_KEY === 'fixture-api-key', @@ -128,6 +130,7 @@ try { expect(result.home).not.toBe(operatorHome); expect(result.root).toBe(fs.realpathSync(ROOT)); expect(result.configValue).toBe('disabled'); expect(result.runtimeAllowed).toBe(true); + expect(result.canonicalConfigExit).toBe(0); expect(result.canonicalConfigValue).toBe('disabled'); for (const asset of result.discovery) { expect(asset.home, item.name + ': HOME discovery ' + asset.relative).toBe(fs.realpathSync(path.join(ROOT,asset.relative))); expect(asset.config, item.name + ': config discovery ' + asset.relative).toBe(asset.home); diff --git a/test/hermetic-skills-seeding.test.ts b/test/hermetic-skills-seeding.test.ts index b57befaba..9036baebf 100644 --- a/test/hermetic-skills-seeding.test.ts +++ b/test/hermetic-skills-seeding.test.ts @@ -19,10 +19,12 @@ import { describe, test, expect } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; import { hermeticSkillsConfigDir, getHermeticDirs, buildSeedConfig, + seedHermeticRuntimeView, } from './helpers/hermetic-env'; import { skillCensus } from './helpers/skill-census'; @@ -37,14 +39,118 @@ describe('hermeticSkillsConfigDir', () => { expect(configDir.startsWith(getHermeticDirs().runRoot + path.sep)).toBe(true); }); - test('exact skill registry plus the canonical runtime checkout', () => { + test('exact skill registry plus the canonical runtime view', () => { const seeded = fs.readdirSync(skillsDir).sort(); expect(seeded).toEqual([...skillCensus(ROOT).registryEntries, 'gstack'].sort()); const runtime = path.join(skillsDir, 'gstack'); - expect(fs.lstatSync(runtime).isSymbolicLink()).toBe(true); - expect(fs.realpathSync(runtime)).toBe(fs.realpathSync(ROOT)); + expect(fs.lstatSync(runtime).isDirectory()).toBe(true); expect(fs.readFileSync(path.join(runtime, 'plan-design-review/sections/review-sections.md'), 'utf8')) .toBe(fs.readFileSync(path.join(ROOT, 'plan-design-review/sections/review-sections.md'), 'utf8')); + for (const rel of ['bin/gstack-config', 'lib/claude-bin.ts', 'SKILL.md', 'ETHOS.md', 'extension/manifest.json', + 'browser-skills/hackernews-frontpage/script.ts', 'browser-skills/hackernews-frontpage/script.test.ts', + 'docs/askuserquestion-split.md', 'docs/askuserquestion-cjk.md', + 'docs/designs/PLAN_TUNING_V0.md', 'docs/designs/PLAN_TUNING_V1.md']) + expect(fs.realpathSync(path.join(runtime, rel))).toBe(fs.realpathSync(path.join(ROOT, rel))); + expect(fs.readdirSync(path.join(runtime, 'docs')).sort()).toEqual(['askuserquestion-cjk.md', 'askuserquestion-split.md', 'designs']); + expect(fs.readdirSync(path.join(runtime, 'docs/designs')).sort()).toEqual(['PLAN_TUNING_V0.md', 'PLAN_TUNING_V1.md']); + for (const rel of ['.context', '.git', 'node_modules', '.agents', 'hosts', 'test']) + expect(fs.existsSync(path.join(runtime, rel)), rel).toBe(false); + }); + + test('recursive installed discovery cannot read historical or dependency markdown', () => { + const fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-runtime-view-')); + const source = path.join(fixture, 'source'); + const view = path.join(fixture, 'view'); + const files = { + 'SKILL.md': '---\nname: router\n---\nCurrent router', + 'review/SKILL.md': '---\nname: review\n---\nCurrent review', + 'review/sections/method.md': 'CURRENT METHOD', + 'review/references/example.md': 'CURRENT REFERENCE', + 'bin/tool': '#!/bin/sh\nexit 0\n', + 'lib/helper.ts': 'export const current = true;', + 'browse/dist/browse': 'CURRENT BUILT RUNTIME', + '.context/old-review.md': 'HISTORICAL SENTINEL', + '.git/history.md': 'GIT SENTINEL', + 'node_modules/pkg/README.md': 'DEPENDENCY SENTINEL', + '.agents/skills/review/SKILL.md': 'GENERATED HOST SENTINEL', + 'hosts/review.md': 'HOST SOURCE SENTINEL', + 'review/sections/node_modules/pkg/README.md': 'NESTED DEPENDENCY SENTINEL', + 'review/test/fixture.md': 'TEST SENTINEL', + 'review/sections/method.md.tmpl': 'TEMPLATE SENTINEL', + }; + try { + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(source, rel)), { recursive: true }); + fs.writeFileSync(path.join(source, rel), content, { mode: 0o755 }); + } + fs.symlinkSync(path.join(source, 'review'), path.join(source, 'review-alias'), 'dir'); + seedHermeticRuntimeView(source, view); + const markdown: string[] = []; + const scan = (dir: string) => { + for (const name of fs.readdirSync(dir)) { + const file = path.join(dir, name); + if (fs.statSync(file).isDirectory()) { + expect(fs.lstatSync(file).isSymbolicLink()).toBe(false); + scan(file); + } else { + expect(fs.realpathSync(file)).toBe(fs.realpathSync(path.join(source, path.relative(view, file)))); + if (name.endsWith('.md')) markdown.push(fs.readFileSync(file, 'utf8')); + } + } + }; + scan(view); + expect(markdown).toContain('CURRENT METHOD'); + expect(markdown).toContain('CURRENT REFERENCE'); + expect(markdown.some(content => content.includes('SENTINEL'))).toBe(false); + expect(fs.readFileSync(path.join(view, 'browse/dist/browse'), 'utf8')).toBe('CURRENT BUILT RUNTIME'); + expect(fs.statSync(path.join(view, 'bin/tool')).mode & 0o111).toBe(0o111); + expect(fs.existsSync(path.join(view, 'review/sections/method.md.tmpl'))).toBe(false); + } finally { fs.rmSync(fixture, { recursive: true, force: true }); } + }); + + test('rejects escaped or circular assets and removes only its partial view', () => { + const fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-runtime-view-owned-')); + try { + const source = path.join(fixture, 'source'); + fs.mkdirSync(path.join(source, 'bin'), { recursive: true }); + const external = path.join(fixture, 'outside.md'); + fs.writeFileSync(external, 'OUTSIDE'); + fs.symlinkSync(external, path.join(source, 'bin/escape.md')); + const view = path.join(fixture, 'view'); + expect(() => seedHermeticRuntimeView(source, view)).toThrow('owned checkout'); + expect(fs.existsSync(view)).toBe(false); + expect(fs.readFileSync(external, 'utf8')).toBe('OUTSIDE'); + fs.unlinkSync(path.join(source, 'bin/escape.md')); + fs.symlinkSync(source, path.join(source, 'bin/cycle'), 'dir'); + expect(() => seedHermeticRuntimeView(source, view)).toThrow('Circular'); + expect(fs.existsSync(view)).toBe(false); + fs.mkdirSync(view); fs.writeFileSync(path.join(view, 'owner.md'), 'OWNED ELSEWHERE'); + expect(() => seedHermeticRuntimeView(source, view)).toThrow(); + expect(fs.readFileSync(path.join(view, 'owner.md'), 'utf8')).toBe('OWNED ELSEWHERE'); + } finally { fs.rmSync(fixture, { recursive: true, force: true }); } + }); + + test('ordinary aliases cannot reopen excluded directories or files', () => { + const fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-runtime-view-alias-')); + const source = path.join(fixture, 'source'); + const view = path.join(fixture, 'view'); + try { + fs.mkdirSync(path.join(source, 'review/sections'), { recursive: true }); + fs.writeFileSync(path.join(source, 'review/SKILL.md'), '---\nname: review\n---\nCURRENT'); + for (const forbidden of ['.context', 'node_modules', 'test', '.agents/skills', 'hosts', 'docs/history']) { + const hidden = path.join(source, forbidden); + fs.mkdirSync(hidden, { recursive: true }); + fs.writeFileSync(path.join(hidden, 'sentinel.md'), 'FORBIDDEN HISTORY'); + for (const directory of [true, false]) { + const alias = path.join(source, 'review/sections', directory ? 'archive' : 'ordinary.md'); + fs.symlinkSync(directory ? hidden : path.join(hidden, 'sentinel.md'), alias, directory ? 'dir' : 'file'); + expect(() => seedHermeticRuntimeView(source, view)).toThrow('excluded tree'); + expect(fs.existsSync(view)).toBe(false); + expect(fs.readFileSync(path.join(hidden, 'sentinel.md'), 'utf8')).toBe('FORBIDDEN HISTORY'); + fs.unlinkSync(alias); + } + } + } finally { fs.rmSync(fixture, { recursive: true, force: true }); } }); test('every SKILL.md is a symlink resolving to a real file', () => { diff --git a/test/hermetic-wiring.test.ts b/test/hermetic-wiring.test.ts index 944f741fe..019f7d0d4 100644 --- a/test/hermetic-wiring.test.ts +++ b/test/hermetic-wiring.test.ts @@ -156,8 +156,18 @@ describe('hermetic wiring tripwire', () => { const repoRootReal = fs.realpathSync(ROOT) + path.sep; for (const entry of fs.readdirSync(skillsDir)) { if (entry === 'gstack') { - expect(fs.lstatSync(path.join(skillsDir, entry)).isSymbolicLink()).toBe(true); - expect(fs.realpathSync(path.join(skillsDir, entry)) + path.sep).toBe(repoRootReal); + const verifyRuntime = (directory: string) => { + expect(fs.lstatSync(directory).isDirectory()).toBe(true); + for (const name of fs.readdirSync(directory)) { + const file = path.join(directory, name); + if (fs.statSync(file).isDirectory()) verifyRuntime(file); + else { + expect(fs.lstatSync(file).isSymbolicLink()).toBe(true); + expect(fs.realpathSync(file).startsWith(repoRootReal), file).toBe(true); + } + } + }; + verifyRuntime(path.join(skillsDir, entry)); continue; } const target = fs.readlinkSync(path.join(skillsDir, entry, 'SKILL.md'));