From 28a172d45007bbdb6b72f3c4c91e119fc7fa73b1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 1 Jun 2026 09:46:18 -0400 Subject: [PATCH] fix(#570): scope Codex leak scanner to manifest + replace bare ~/.claude refs * fix(#570): scope Codex leak scanner to manifest, replace bare ~/.claude refs Two root causes: - scanForLeakedPaths walked entire ~/.codex tree, flagging pre-existing unrelated files; now reads gsd-file-manifest.json to scope scan to GSD-owned artifacts only - convertClaudeToCodexMarkdown replaced ~/\.claude/ (slash form) but not bare ~/\.claude\b; gsd-debugger.toml and gsd-surface/SKILL.md examples slipped through; bare word-boundary replacement now added - writeManifest tracked agents/gsd-*.md but Codex installs .toml files; manifest now also records .toml agent files so the scoped scanner covers them Closes #570 Co-Authored-By: Claude Sonnet 4.6 * chore: add changeset fragment for #570 Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/sunny-lynx-rally.md | 5 + bin/install.js | 71 ++++++----- tests/bug-570-codex-leak-scanner.test.cjs | 141 ++++++++++++++++++++++ 3 files changed, 188 insertions(+), 29 deletions(-) create mode 100644 .changeset/sunny-lynx-rally.md create mode 100644 tests/bug-570-codex-leak-scanner.test.cjs diff --git a/.changeset/sunny-lynx-rally.md b/.changeset/sunny-lynx-rally.md new file mode 100644 index 000000000..35c54a909 --- /dev/null +++ b/.changeset/sunny-lynx-rally.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 570 +--- +Codex leak scanner now reads gsd-file-manifest.json to scope path checks to GSD-owned files only; bare ~/.claude (no trailing slash) is now replaced in converted Codex markdown; writeManifest now records agents/gsd-*.toml so the manifest-scoped scanner covers them diff --git a/bin/install.js b/bin/install.js index 4be6edd57..8f75f813f 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2812,6 +2812,9 @@ function convertClaudeToCodexMarkdown(content) { converted = converted.replace(/\$HOME\/\.claude\//g, '$HOME/.codex/'); converted = converted.replace(/~\/\.claude\//g, '~/.codex/'); converted = converted.replace(/\.\/\.claude\//g, './.codex/'); + // Bare ~/.claude without trailing slash (e.g. configDir = ~/.claude) + converted = converted.replace(/\$HOME\/\.claude\b/g, '$HOME/.codex'); + converted = converted.replace(/~\/\.claude\b/g, '~/.codex'); // Bare/project-relative .claude/... references (#2639). Covers strings like // "check `.claude/skills/`" where there is no ~/, $HOME/, or ./ anchor. // Negative lookbehind prevents double-replacing already-anchored forms and @@ -7853,7 +7856,7 @@ function writeManifest(configDir, runtime = 'claude', options = {}) { } if (fs.existsSync(agentsDir)) { for (const file of fs.readdirSync(agentsDir)) { - if (file.startsWith('gsd-') && file.endsWith('.md')) { + if (file.startsWith('gsd-') && (file.endsWith('.md') || file.endsWith('.toml'))) { manifest.files['agents/' + file] = fileHash(path.join(agentsDir, file)); } } @@ -9104,42 +9107,48 @@ function install(isGlobal, runtime = 'claude', options = {}) { // Report any backed-up local patches reportLocalPatches(targetDir, runtime); - // Verify no leaked .claude paths in non-Claude runtimes + // Verify no leaked .claude paths in non-Claude runtimes (manifest-scoped) if (runtime !== 'claude') { const leakedPaths = []; - function scanForLeakedPaths(dir) { - if (!fs.existsSync(dir)) return; - let entries; - try { - entries = fs.readdirSync(dir, { withFileTypes: true }); - } catch (err) { - if (err.code === 'EPERM' || err.code === 'EACCES') { - return; // skip inaccessible directories + // Only scan files that were written by this install (manifest-tracked). + // Scanning the entire targetDir can match user-authored content that + // legitimately references ~/.claude (e.g. personal notes), producing + // false-positive warnings. Restricting to the manifest avoids that. + let manifestFiles = null; + try { + const manifestPath = path.join(targetDir, MANIFEST_NAME); + if (fs.existsSync(manifestPath)) { + const manifestData = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + if (manifestData && typeof manifestData.files === 'object') { + manifestFiles = Object.keys(manifestData.files); } - throw err; } - for (const entry of entries) { - const fullPath = path.join(dir, entry.name); - if (entry.isDirectory()) { - scanForLeakedPaths(fullPath); - } else if ((entry.name.endsWith('.md') || entry.name.endsWith('.toml')) && entry.name !== 'CHANGELOG.md') { - let content; - try { - content = fs.readFileSync(fullPath, 'utf8'); - } catch (err) { - if (err.code === 'EPERM' || err.code === 'EACCES') { - continue; // skip inaccessible files - } - throw err; - } - const matches = content.match(/(?:~|\$HOME)\/\.claude\b/g); - if (matches) { - leakedPaths.push({ file: fullPath.replace(targetDir + '/', ''), count: matches.length }); + } catch (_manifestParseErr) { + // If we cannot read/parse the manifest, skip the scan entirely to + // avoid false positives rather than falling back to a full directory walk. + manifestFiles = null; + } + if (manifestFiles !== null) { + for (const relPath of manifestFiles) { + const fileName = path.basename(relPath); + if (!(fileName.endsWith('.md') || fileName.endsWith('.toml'))) continue; + if (fileName === 'CHANGELOG.md') continue; + const fullPath = path.join(targetDir, relPath); + let content; + try { + content = fs.readFileSync(fullPath, 'utf8'); + } catch (err) { + if (err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'ENOENT') { + continue; // skip inaccessible or missing files } + throw err; + } + const matches = content.match(/(?:~|\$HOME)\/\.claude\b/g); + if (matches) { + leakedPaths.push({ file: relPath, count: matches.length }); } } } - scanForLeakedPaths(targetDir); if (leakedPaths.length > 0) { const totalLeaks = leakedPaths.reduce((sum, l) => sum + l.count, 0); console.warn(`\n ${yellow}⚠${reset} Found ${totalLeaks} unreplaced .claude path reference(s) in ${leakedPaths.length} file(s):`); @@ -9343,6 +9352,10 @@ function install(isGlobal, runtime = 'claude', options = {}) { } console.log(` ${green}✓${reset} Generated config.toml with ${agentCount} agent roles`); console.log(` ${green}✓${reset} Generated ${agentCount} agent .toml config files`); + // Re-write the manifest now that .toml agent files exist on disk. + // The initial writeManifest call (before Codex config generation) could + // not include agents/gsd-*.toml because those files did not yet exist. + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode }); } else { console.log(` ${dim}↳${reset} Skipping Codex agent config generation (minimal install)`); } diff --git a/tests/bug-570-codex-leak-scanner.test.cjs b/tests/bug-570-codex-leak-scanner.test.cjs new file mode 100644 index 000000000..127035a25 --- /dev/null +++ b/tests/bug-570-codex-leak-scanner.test.cjs @@ -0,0 +1,141 @@ +// allow-test-rule: source-text-is-the-product +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Regression tests for issue #570 — three related sub-bugs in the Codex leak + * scanner and supporting infrastructure. + * + * SUB-BUG A: scanForLeakedPaths recursively scans the entire targetDir, + * including pre-existing unrelated files that contain ~/.claude references. + * Fix: scan only files listed in gsd-file-manifest.json. + * + * SUB-BUG B: convertClaudeToCodexMarkdown replaces "~/.claude/" (with trailing + * slash) but NOT bare "~/.claude" (no slash). The scanner regex + * /(?:~|\$HOME)\/\.claude\b/ matches without trailing slash. + * Fix: add bare word-boundary replacement. + * + * SUB-BUG C: writeManifest checks file.endsWith('.md') for the agents/ + * directory. Codex installs .toml agent files, so they are invisible to the + * manifest and thus to any manifest-based scan fix. + * Fix: also check .toml. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const { + install, + writeManifest, + convertClaudeCommandToCodexSkill, +} = require('../bin/install.js'); +const { createTempDir, cleanup, captureConsole } = require('./helpers.cjs'); + +const HOOKS_DIST = path.join(__dirname, '..', 'hooks', 'dist'); +const BUILD_HOOKS_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); + +function withCodexHome(codexHome, fn) { + const prev = process.env.CODEX_HOME; + process.env.CODEX_HOME = codexHome; + try { + return fn(); + } finally { + if (prev == null) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prev; + } +} + +describe('#570 — Codex leak scanner sub-bugs', { concurrency: false }, () => { + let tmpRoot; + let codexHome; + + beforeEach(() => { + if (!fs.existsSync(HOOKS_DIST) || fs.readdirSync(HOOKS_DIST).length === 0) { + execFileSync(process.execPath, [BUILD_HOOKS_SCRIPT], { stdio: 'pipe' }); + } + tmpRoot = createTempDir('gsd-570-'); + codexHome = path.join(tmpRoot, '.codex'); + fs.mkdirSync(codexHome, { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpRoot); + }); + + // SUB-BUG B + test('convertClaudeToCodexMarkdown replaces bare ~/.claude (no trailing slash)', () => { + // convertClaudeToCodexMarkdown is not exported directly; exercise it via + // convertClaudeCommandToCodexSkill which calls it internally. + const input = 'configDir = ~/.claude\npath = ~/.claude/hooks/\ndir = $HOME/.claude'; + const out = convertClaudeCommandToCodexSkill(input, 'gsd-test'); + + assert.ok( + !/(?:~|\$HOME)\/\.claude\b/.test(out), + `Expected no leaked ~/.claude reference after conversion, got:\n${out}`, + ); + }); + + // SUB-BUG C + test('writeManifest includes .toml agent files for Codex', () => { + withCodexHome(codexHome, () => install(true, 'codex')); + + const agentsDir = path.join(codexHome, 'agents'); + // Confirm that Codex actually wrote .toml agent files — if none exist the + // test is vacuous and we should fail loudly. + const tomlFiles = fs.existsSync(agentsDir) + ? fs.readdirSync(agentsDir).filter((f) => f.startsWith('gsd-') && f.endsWith('.toml')) + : []; + assert.ok( + tomlFiles.length > 0, + `Precondition: Codex install must write at least one gsd-*.toml in agents/; found none in ${agentsDir}`, + ); + + const manifestPath = path.join(codexHome, 'gsd-file-manifest.json'); + assert.ok( + fs.existsSync(manifestPath), + `gsd-file-manifest.json must exist after install; not found at ${manifestPath}`, + ); + + const manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + const manifestKeys = Object.keys(manifest.files || {}); + + const tomlManifestKeys = manifestKeys.filter( + (k) => k.startsWith('agents/gsd-') && k.endsWith('.toml'), + ); + assert.ok( + tomlManifestKeys.length > 0, + `Expected at least one 'agents/gsd-*.toml' key in manifest.files, but found none.\n` + + `agents/ toml files on disk: ${tomlFiles.join(', ')}\n` + + `All manifest keys (agents/): ${manifestKeys.filter((k) => k.startsWith('agents/')).join(', ')}`, + ); + }); + + // SUB-BUG A + test('scanForLeakedPaths does not warn for pre-existing unrelated files in ~/.codex', () => { + // Write a pre-existing file with ~/.claude references BEFORE install. + const memoriesDir = path.join(codexHome, 'memories'); + fs.mkdirSync(memoriesDir, { recursive: true }); + const preExistingFile = path.join(memoriesDir, 'raw_memories.md'); + fs.writeFileSync( + preExistingFile, + '# Old memories\nI used to work in ~/.claude and $HOME/.claude regularly.\n', + ); + + let captured; + withCodexHome(codexHome, () => { + captured = captureConsole(() => install(true, 'codex')); + }); + + const combinedOutput = captured.stderr; + + assert.ok( + !combinedOutput.includes('memories/raw_memories.md'), + `scanForLeakedPaths must not warn about pre-existing unrelated file memories/raw_memories.md.\n` + + `Actual warnings:\n${combinedOutput}`, + ); + }); +});