diff --git a/.github/scripts/check-root-directory-entries.mjs b/.github/scripts/check-root-directory-entries.mjs new file mode 100644 index 000000000..5e7dcf3f0 --- /dev/null +++ b/.github/scripts/check-root-directory-entries.mjs @@ -0,0 +1,64 @@ +import { execFileSync } from 'node:child_process' +import { randomUUID } from 'node:crypto' + +function readRootEntries(sha) { + // Why: a git pathname is arbitrary bytes, and 'utf8' folds every invalid + // sequence to U+FFFD — that mangles the reported name and makes two different + // entries compare equal, so a genuinely new one can slip past the Set below. + // latin1 maps each byte to one code unit, so the bytes survive the round trip. + const stdout = execFileSync('git', ['ls-tree', '-z', '--name-only', sha], { + encoding: 'latin1', + stdio: ['ignore', 'pipe', 'inherit'] + }) + return stdout.split('\0').filter(Boolean) +} + +function checkRootDirectoryEntries(argv) { + if (argv.length !== 2) { + console.error(`Usage: ${process.argv[1]} `) + return 2 + } + + const [baseSha, headSha] = argv + const baseEntries = new Set(readRootEntries(baseSha)) + const blockedEntries = readRootEntries(headSha).filter((entry) => !baseEntries.has(entry)) + + if (blockedEntries.length === 0) { + console.log('Root directory guard passed: no new root-level files or folders.') + return 0 + } + + console.log( + '::error title=Root-level additions blocked::New root-level files or folders bloat the GitHub landing page.' + ) + console.log('Root directory guard failed.') + console.log( + 'New root-level files or folders are not allowed because they bloat the GitHub landing page.' + ) + console.log('Move each new entry under an existing top-level directory.') + console.log('Blocked entries:') + // Why: an entry name is attacker-controlled and may start with '::' (the runner + // trims leading spaces before matching) or embed a newline, so printing it bare + // lets a PR forge annotations. Fence the untrusted list with an unguessable + // stop-commands token, and write the raw bytes rather than a re-encoded string. + const resumeToken = randomUUID() + console.log(`::stop-commands::${resumeToken}`) + for (const entry of blockedEntries) { + process.stdout.write(Buffer.from(` ${entry}\n`, 'latin1')) + } + console.log(`::${resumeToken}::`) + return 1 +} + +try { + // Why: process.exit truncates a piped write part-way through on macOS, so set + // exitCode and let node flush the blocked-entry list before it exits. + process.exitCode = checkRootDirectoryEntries(process.argv.slice(2)) +} catch (error) { + // Why: git already reported the failure on the inherited stderr, so surface its + // status rather than a node stack trace. Anything else is a real bug — rethrow. + if (typeof error.status !== 'number') { + throw error + } + process.exitCode = error.status +} diff --git a/.github/scripts/check-root-directory-entries.sh b/.github/scripts/check-root-directory-entries.sh deleted file mode 100755 index c07d7d401..000000000 --- a/.github/scripts/check-root-directory-entries.sh +++ /dev/null @@ -1,37 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -if [[ $# -ne 2 ]]; then - echo "Usage: $0 " >&2 - exit 2 -fi - -base_sha=$1 -head_sha=$2 -git rev-parse --verify "${base_sha}^{tree}" >/dev/null -git rev-parse --verify "${head_sha}^{tree}" >/dev/null - -declare -A base_entries=() -while IFS= read -r -d '' entry; do - base_entries["$entry"]=1 -done < <(git ls-tree -z --name-only "$base_sha") - -blocked_entries=() -while IFS= read -r -d '' entry; do - if [[ -z "${base_entries[$entry]+present}" ]]; then - blocked_entries+=("$entry") - fi -done < <(git ls-tree -z --name-only "$head_sha") - -if (( ${#blocked_entries[@]} == 0 )); then - echo "Root directory guard passed: no new root-level files or folders." - exit 0 -fi - -echo "::error title=Root-level additions blocked::New root-level files or folders bloat the GitHub landing page." -echo "Root directory guard failed." -echo "New root-level files or folders are not allowed because they bloat the GitHub landing page." -echo "Move each new entry under an existing top-level directory." -printf 'Blocked entries:\n' -printf ' %s\n' "${blocked_entries[@]}" -exit 1 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 8d44cabf0..0789f1958 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -108,7 +108,7 @@ jobs: env: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: bash .github/scripts/check-root-directory-entries.sh "$BASE_SHA" "$HEAD_SHA" + run: node .github/scripts/check-root-directory-entries.mjs "$BASE_SHA" "$HEAD_SHA" typecheck: runs-on: ubuntu-latest diff --git a/config/scripts/check-root-directory-entries.test.mjs b/config/scripts/check-root-directory-entries.test.mjs index e8d3bb9bc..15276e8aa 100644 --- a/config/scripts/check-root-directory-entries.test.mjs +++ b/config/scripts/check-root-directory-entries.test.mjs @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it } from 'vitest' import { parse } from 'yaml' const projectDir = resolve(import.meta.dirname, '../..') -const guardScript = join(projectDir, '.github/scripts/check-root-directory-entries.sh') +const guardScript = join(projectDir, '.github/scripts/check-root-directory-entries.mjs') const tempDirs = [] function git(cwd, args) { @@ -37,13 +37,45 @@ function commitFiles(root, files) { return git(root, ['rev-parse', 'HEAD']) } +// Why: a root entry name can be bytes no filesystem here accepts (APFS rejects +// invalid UTF-8), so build the tree in the object database instead of on disk. +// git ls-tree -z emits exactly the record format git mktree -z reads back. +function commitRawEntries(root, parent, entries) { + const parentTree = execFileSync('git', ['ls-tree', '-z', parent], { cwd: root }) + const records = entries.map((name) => { + const blob = execFileSync('git', ['hash-object', '-w', '--stdin'], { + cwd: root, + encoding: 'utf8', + input: 'too prominent\n' + }).trim() + return Buffer.concat([Buffer.from(`100644 blob ${blob}\t`), name, Buffer.from([0])]) + }) + const tree = execFileSync('git', ['mktree', '-z'], { + cwd: root, + encoding: 'utf8', + input: Buffer.concat([parentTree, ...records]) + }).trim() + return execFileSync('git', ['commit-tree', tree, '-p', parent, '-m', 'head'], { + cwd: root, + encoding: 'utf8' + }).trim() +} + function runGuard({ root, base, head }) { - return spawnSync('bash', [guardScript, base, head], { + return runGuardArgs(root, [base, head]) +} + +function runGuardArgs(root, args) { + return spawnSync(process.execPath, [guardScript, ...args], { cwd: root, encoding: 'utf8' }) } +function runGuardBytes({ root, base, head }) { + return spawnSync(process.execPath, [guardScript, base, head], { cwd: root }) +} + afterEach(() => { while (tempDirs.length > 0) { rmSync(tempDirs.pop(), { force: true, recursive: true }) @@ -84,6 +116,100 @@ describe('root directory guard', () => { expect(output).toContain('new-folder') }) + // Why: git escapes odd paths unless it is read NUL-delimited, so dropping -z + // (or decoding the bytes wrong) reports a mangled name nobody can act on. + it.skipIf(process.platform === 'win32')('reports a blocked entry byte-for-byte', () => { + const awkwardName = '日本 root file\nwith newline.txt' + const fixture = makeFixture() + const head = commitFiles(fixture.root, [[awkwardName, 'too prominent\n']]) + + const result = runGuard({ ...fixture, head }) + + expect(result.status).toBe(1) + expect(result.stdout).toContain(awkwardName) + }) + + // Why: decoding git's output as UTF-8 rewrites every invalid byte to U+FFFD, so + // the name the guard prints is not the name anyone has to rename. + it('reports an entry whose name is not valid UTF-8 byte-for-byte', () => { + const rawName = Buffer.concat([Buffer.from([0xff, 0xfe]), Buffer.from('-raw.txt')]) + const fixture = makeFixture() + const head = commitRawEntries(fixture.root, fixture.base, [rawName]) + + const result = runGuardBytes({ ...fixture, head }) + + expect(result.status).toBe(1) + expect(result.stdout.includes(rawName)).toBe(true) + }) + + // Why: U+FFFD is not injective, so two different invalid names decode to the + // same string and a new root entry gets waved through as pre-existing. + it('does not confuse two different invalid UTF-8 names for the same entry', () => { + const fixture = makeFixture() + const base = commitRawEntries(fixture.root, fixture.base, [ + Buffer.concat([Buffer.from([0xc0, 0x80]), Buffer.from('.txt')]) + ]) + const head = commitRawEntries(fixture.root, fixture.base, [ + Buffer.concat([Buffer.from([0xc0, 0x81]), Buffer.from('.txt')]) + ]) + + const result = runGuardBytes({ root: fixture.root, base, head }) + + expect(result.status).toBe(1) + expect(result.stdout.toString('latin1')).not.toContain('guard passed') + }) + + // Why: the runner trims leading spaces before matching '::', so an indented + // entry name still reaches the workflow-command parser and can forge output. + it('prints blocked entries with workflow-command parsing disabled', () => { + const fixture = makeFixture() + const forgedName = '::error title=forged::injected\n::warning::second line.txt' + const head = commitRawEntries(fixture.root, fixture.base, [Buffer.from(forgedName)]) + + const result = runGuard({ ...fixture, head }) + const lines = result.stdout.split('\n') + const stopIndex = lines.findIndex((line) => line.startsWith('::stop-commands::')) + const resumeToken = lines[stopIndex]?.slice('::stop-commands::'.length) + const resumeIndex = lines.indexOf(`::${resumeToken}::`) + const escaped = lines.filter( + (line, index) => + (index < stopIndex || index > resumeIndex) && line.trimStart().startsWith('::') + ) + + expect(result.status).toBe(1) + expect(resumeToken).toMatch(/^[\da-f-]{36}$/) + expect(stopIndex).toBeLessThan(resumeIndex) + // Why: the guard's own annotation is the only line the runner may act on. + expect(escaped).toHaveLength(1) + expect(escaped[0]).toContain('Root-level additions blocked') + expect(lines.slice(stopIndex, resumeIndex).join('\n')).toContain(forgedName) + }) + + it('exits 2 with usage when the two shas are not both supplied', () => { + const fixture = makeFixture() + + const result = runGuardArgs(fixture.root, [fixture.base]) + + expect(result.status).toBe(2) + expect(result.stderr).toContain(' ') + }) + + it('fails loudly instead of passing when a sha does not resolve', () => { + const fixture = makeFixture() + + const result = runGuardArgs(fixture.root, [ + fixture.base, + 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' + ]) + + // Why: git's own exit status, not node's. An unhandled throw is also non-zero, + // so assert the status and the absent stack trace or the guard's error + // handling can be deleted without a test noticing. + expect(result.status).toBe(128) + expect(result.stderr).not.toContain('node:internal') + expect(result.stdout).not.toContain('guard passed') + }) + it('is wired into the PR verify gate', () => { const workflow = parse(readFileSync(join(projectDir, '.github/workflows/pr.yml'), 'utf8')) const guardJob = workflow.jobs.root_directory_guard @@ -93,7 +219,7 @@ describe('root directory guard', () => { expect(guardJob.name).toBe('root directory guard') expect(guardJob.steps[0].with['fetch-depth']).toBe(0) - expect(guardStep.run).toContain('.github/scripts/check-root-directory-entries.sh') + expect(guardStep.run).toContain('node .github/scripts/check-root-directory-entries.mjs') expect(workflow.jobs.verify.needs).toContain('root_directory_guard') }) })