diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ddd768e6..f151ab6f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -220,6 +220,10 @@ jobs: run: node scripts/ci/check-unicode-safety.js continue-on-error: false + - name: Validate no personal paths + run: node scripts/ci/validate-no-personal-paths.js + continue-on-error: false + security: name: Security Scan runs-on: ubuntu-latest diff --git a/.github/workflows/reusable-validate.yml b/.github/workflows/reusable-validate.yml index a606404b..ff57dd8e 100644 --- a/.github/workflows/reusable-validate.yml +++ b/.github/workflows/reusable-validate.yml @@ -50,3 +50,6 @@ jobs: - name: Check unicode safety run: node scripts/ci/check-unicode-safety.js + + - name: Validate no personal paths + run: node scripts/ci/validate-no-personal-paths.js diff --git a/scripts/ci/validate-no-personal-paths.js b/scripts/ci/validate-no-personal-paths.js index fee859db..9427b6f9 100755 --- a/scripts/ci/validate-no-personal-paths.js +++ b/scripts/ci/validate-no-personal-paths.js @@ -1,6 +1,11 @@ #!/usr/bin/env node /** * Prevent shipping user-specific absolute paths in public docs/skills/commands. + * + * Catches generic `/Users/` (macOS) and `C:\Users\` (Windows) paths, + * while allowing obvious placeholder usernames used in templates/examples. + * Forensic incident reports under `docs/fixes/` are exempt because they may + * legitimately document a reporter's local machine path. */ 'use strict'; @@ -18,11 +23,50 @@ const TARGETS = [ '.opencode/commands', ]; -const BLOCK_PATTERNS = [ - /\/Users\/affoon\b/g, - /C:\\Users\\affoon\b/gi, +const EXEMPT_PREFIXES = [ + 'docs/fixes/', ]; +const PLACEHOLDER_USERNAMES = new Set([ + 'example', + 'me', + 'user', + 'username', + 'you', + 'yourname', + 'yourusername', + 'your-username', +]); + +const POSIX_USER_RE = /\/Users\/([a-zA-Z][a-zA-Z0-9._-]*)/g; +const WIN_USER_RE = /C:\\Users\\([a-zA-Z][a-zA-Z0-9._-]*)/gi; + +function repoRelative(file) { + return path.relative(ROOT, file).split(path.sep).join('/'); +} + +function isExempt(file) { + const rel = repoRelative(file); + return EXEMPT_PREFIXES.some(prefix => rel.startsWith(prefix)); +} + +function findLeaks(content) { + const leaks = []; + + for (const pattern of [POSIX_USER_RE, WIN_USER_RE]) { + pattern.lastIndex = 0; + let match; + + while ((match = pattern.exec(content)) !== null) { + if (!PLACEHOLDER_USERNAMES.has(match[1].toLowerCase())) { + leaks.push(match[0]); + } + } + } + + return leaks; +} + function collectFiles(targetPath, out) { if (!fs.existsSync(targetPath)) return; const stat = fs.statSync(targetPath); @@ -45,14 +89,14 @@ for (const target of TARGETS) { let failures = 0; for (const file of files) { if (!/\.(md|json|js|ts|sh|toml|yml|yaml)$/i.test(file)) continue; + if (isExempt(file)) continue; + const content = fs.readFileSync(file, 'utf8'); - for (const pattern of BLOCK_PATTERNS) { - const match = content.match(pattern); - if (match) { - console.error(`ERROR: personal path detected in ${path.relative(ROOT, file)}`); - failures += match.length; - break; - } + const leaks = findLeaks(content); + + for (const leak of leaks) { + console.error(`ERROR: personal path "${leak}" detected in ${repoRelative(file)}`); + failures += 1; } } diff --git a/scripts/ci/validate-workflow-security.js b/scripts/ci/validate-workflow-security.js index 7f6183ba..03936136 100644 --- a/scripts/ci/validate-workflow-security.js +++ b/scripts/ci/validate-workflow-security.js @@ -75,7 +75,7 @@ function extractCheckoutSteps(source) { startLine: block.startLine, text: block.lines.join('\n'), })) - .filter(block => /uses:\s*actions\/checkout@/m.test(block.text)); + .filter(block => /uses:\s*['"]?actions\/checkout@/m.test(block.text)); } function findViolations(filePath, source) { diff --git a/tests/ci/no-personal-paths.test.js b/tests/ci/no-personal-paths.test.js new file mode 100644 index 00000000..188f1928 --- /dev/null +++ b/tests/ci/no-personal-paths.test.js @@ -0,0 +1,211 @@ +/** + * Tests for scripts/ci/validate-no-personal-paths.js. + * + * Run with: node tests/ci/no-personal-paths.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const repoRoot = path.join(__dirname, '..', '..'); +const validatorPath = path.join(repoRoot, 'scripts', 'ci', 'validate-no-personal-paths.js'); + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + return true; + } catch (err) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function createTestDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'no-personal-paths-test-')); +} + +function cleanupTestDir(testDir) { + fs.rmSync(testDir, { recursive: true, force: true }); +} + +function writeFile(filePath, content) { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); +} + +function stripShebang(source) { + let result = source; + if (result.charCodeAt(0) === 0xFEFF) result = result.slice(1); + if (result.startsWith('#!')) { + const newline = result.indexOf('\n'); + result = newline === -1 ? '' : result.slice(newline + 1); + } + return result; +} + +function runValidatorAgainst(testDir) { + let source = fs.readFileSync(validatorPath, 'utf8'); + source = stripShebang(source); + source = source.replace( + /const ROOT = .*?;/, + `const ROOT = ${JSON.stringify(testDir)};`, + ); + + const tmpFile = path.join( + os.tmpdir(), + `no-personal-paths-${Date.now()}-${Math.random().toString(36).slice(2)}.js`, + ); + + try { + fs.writeFileSync(tmpFile, source, 'utf8'); + const stdout = execFileSync('node', [tmpFile], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 10000, + cwd: repoRoot, + }); + return { code: 0, stdout, stderr: '' }; + } catch (err) { + return { + code: err.status || 1, + stdout: err.stdout || '', + stderr: err.stderr || '', + }; + } finally { + try { fs.unlinkSync(tmpFile); } catch (_) { /* ignore cleanup errors */ } + } +} + +function runValidatorAgainstRealRepo() { + try { + const stdout = execFileSync('node', [validatorPath], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 10000, + cwd: repoRoot, + }); + return { code: 0, stdout, stderr: '' }; + } catch (err) { + return { + code: err.status || 1, + stdout: err.stdout || '', + stderr: err.stderr || '', + }; + } +} + +console.log('\n=== Testing validate-no-personal-paths.js ===\n'); + +let passed = 0; +let failed = 0; + +function record(ok) { + if (ok) passed += 1; + else failed += 1; +} + +record(test('passes against the real repository', () => { + const result = runValidatorAgainstRealRepo(); + assert.strictEqual(result.code, 0, `expected exit 0; stderr: ${result.stderr}`); + assert.ok(result.stdout.includes('Validated:'), 'expected success line in stdout'); +})); + +record(test('flags a leaked /Users/ path', () => { + const testDir = createTestDir(); + try { + writeFile(path.join(testDir, 'skills', 'leaky', 'SKILL.md'), 'See /Users/sugig/.claude/settings.json\n'); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 1, 'expected non-zero exit on leak'); + assert.ok(result.stderr.includes('/Users/sugig'), `expected stderr to mention leaked path; got: ${result.stderr}`); + assert.ok(result.stderr.includes('skills/leaky/SKILL.md'), `expected normalized file path; got: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +record(test('flags a leaked C:\\Users\\ path case-insensitively', () => { + const testDir = createTestDir(); + try { + writeFile(path.join(testDir, 'docs', 'guide.md'), 'See C:\\Users\\Affaan\\projects\\thing\n'); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 1, 'expected non-zero exit on leak'); + assert.ok(result.stderr.includes('C:\\Users\\Affaan'), `expected stderr to mention leaked path; got: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +record(test('allows /Users/ templates', () => { + const testDir = createTestDir(); + try { + writeFile(path.join(testDir, 'commands', 'demo.md'), [ + '/Users/you/.claude/session.json', + '/Users/example/.claude/rules/foo.md', + '/Users/yourname/projects/app', + '/Users/your-username/.claude/settings.json', + 'C:\\Users\\USER\\.claude\\settings.json', + ].join('\n')); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 0, `expected exit 0 for placeholders; stderr: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +record(test('exempts docs/fixes forensic reports', () => { + const testDir = createTestDir(); + try { + writeFile( + path.join(testDir, 'docs', 'fixes', 'HOOK-FIX-EXAMPLE.md'), + 'Reporter ran: C:\\Users\\sugig\\.claude\\settings.local.json\n', + ); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 0, `expected exit 0 for docs/fixes; stderr: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +record(test('only scans configured file extensions', () => { + const testDir = createTestDir(); + try { + writeFile(path.join(testDir, 'skills', 'demo', 'image.png'), 'binary /Users/sugig/secret'); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 0, `expected non-text extensions to be skipped; stderr: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +record(test('reports every leak on a single offending file', () => { + const testDir = createTestDir(); + try { + writeFile(path.join(testDir, 'skills', 'multi', 'SKILL.md'), [ + '/Users/sugig/.claude/a.json', + '/Users/sugig/.claude/b.json', + 'C:\\Users\\foo\\bar', + ].join('\n')); + const result = runValidatorAgainst(testDir); + assert.strictEqual(result.code, 1, 'expected non-zero exit on leak'); + const sugigCount = (result.stderr.match(/\/Users\/sugig/g) || []).length; + const fooCount = (result.stderr.match(/C:\\Users\\foo/g) || []).length; + assert.strictEqual(sugigCount, 2, `expected both /Users/sugig occurrences reported; got: ${result.stderr}`); + assert.strictEqual(fooCount, 1, `expected C:\\Users\\foo reported once; got: ${result.stderr}`); + } finally { + cleanupTestDir(testDir); + } +})); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}\n`); + +if (failed > 0) { + process.exit(1); +} diff --git a/tests/ci/validate-workflow-security.test.js b/tests/ci/validate-workflow-security.test.js index b1e69a39..9d58c6f6 100644 --- a/tests/ci/validate-workflow-security.test.js +++ b/tests/ci/validate-workflow-security.test.js @@ -81,6 +81,24 @@ function run() { assert.match(result.stderr, /pull_request\.head\.sha/); })) passed++; else failed++; + // Quoted action names are valid YAML. The checkout-step filter must still + // inspect their `with.ref` values in privileged workflows. + if (test('rejects pull_request_target checkout when uses is double-quoted', () => { + const result = runValidator({ + 'unsafe-double-quoted.yml': `name: Unsafe\non:\n pull_request_target:\n branches: [main]\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: "actions/checkout@v4"\n with:\n ref: \${{ github.event.pull_request.head.sha }}\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail on double-quoted uses:'); + assert.match(result.stderr, /pull_request\.head\.sha/); + })) passed++; else failed++; + + if (test('rejects pull_request_target checkout when uses is single-quoted', () => { + const result = runValidator({ + 'unsafe-single-quoted.yml': `name: Unsafe\non:\n pull_request_target:\n branches: [main]\njobs:\n inspect:\n runs-on: ubuntu-latest\n steps:\n - uses: 'actions/checkout@v4'\n with:\n ref: \${{ github.event.pull_request.head.sha }}\n`, + }); + assert.notStrictEqual(result.status, 0, 'Expected validator to fail on single-quoted uses:'); + assert.match(result.stderr, /pull_request\.head\.sha/); + })) passed++; else failed++; + console.log(`\nPassed: ${passed}`); console.log(`Failed: ${failed}`);