From b5c8b8083915ecda9e5a7ca9d88ad28169b61b09 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:54:49 +0800 Subject: [PATCH] fix(dev-infra): reject non-UTF-8 scope paths --- ...-27-explicit-change-scope-report.i18n.yaml | 4 +- ...2026-07-27-explicit-change-scope-report.md | 6 +- ...6-07-27-explicit-change-scope-report.zh.md | 6 +- scripts/change-scope.spec.ts | 38 +++++++++++- scripts/change-scope.ts | 60 ++++++++++++++++--- 5 files changed, 100 insertions(+), 14 deletions(-) diff --git a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.i18n.yaml b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.i18n.yaml index 991099cd5b..05c5c3ab35 100644 --- a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.i18n.yaml +++ b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.md -2026-07-27-explicit-change-scope-report.md: 2be148d4fe5503cbdfb3bc15c32f3846dc7f413f -2026-07-27-explicit-change-scope-report.zh.md: 33ecddd966ec56759f13a603d95de35d3f3601ec +2026-07-27-explicit-change-scope-report.md: aa6398368d5f161ed852aa92c1d9d35dd2564a5e +2026-07-27-explicit-change-scope-report.zh.md: 042258122d43e5da81cd470e1cf5af09dfeb2fba diff --git a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.md b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.md index 2be148d4fe..aa6398368d 100644 --- a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.md +++ b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.md @@ -12,13 +12,13 @@ An incorrect range undermines evidence selection because it can omit affected pa ## Decision -The root `change-scope` command requires `--base `, accepts `--head ` with `HEAD` as the default, and offers a versioned `--json` form. It resolves both inputs to commits with ambiguity detection and requires one merge base before writing output. The report records the repository root without normalizing legal path whitespace, current branch, configured upstream, input refs, resolved base, head, and merge-base commit IDs, plus sorted committed, staged, unstaged, and untracked path sets. +The root `change-scope` command requires `--base `, accepts `--head ` with `HEAD` as the default, and offers a versioned `--json` form. It resolves both inputs to commits with ambiguity detection and requires one merge base before writing output. The report records the repository root without normalizing legal path whitespace, current branch, configured upstream, input refs, resolved base, head, and merge-base commit IDs, plus sorted committed, staged, unstaged, and untracked path sets. Path records are split at raw NUL bytes and decoded as strict UTF-8; an invalid record aborts the report before output instead of substituting characters or collapsing distinct paths. Committed paths compare the resolved merge base with the resolved head. Dirty path sets always describe the current worktree and index, even when `--head` names another commit. Diff configuration cannot hide submodules or invoke external diff or text-conversion drivers, and rename detection is disabled so both sides of a rename remain visible. The command never guesses or fetches a base, queries a hosting provider, or selects tests. Each calling workflow verifies current remote or stack state, supplies the base explicitly, and uses the factual report as input to semantic review or evidence selection. -Focused temporary-repository tests cover a fresh branch tracking `origin/master` without a same-name remote, its post-push upstream, a worktree path ending in legal whitespace, a stacked non-master base, every dirty layer, invalid, ambiguous, and non-commit refs, deterministic human/JSON parity, and unchanged refs, index, config, and status after reporting. +Focused temporary-repository tests cover a fresh branch tracking `origin/master` without a same-name remote, its post-push upstream, a worktree path ending in legal whitespace, a stacked non-master base, every dirty layer, distinct non-UTF-8 POSIX paths failing without partial output, invalid, ambiguous, and non-commit refs, deterministic human/JSON parity, and unchanged refs, index, config, and status after reporting. ## Alternatives considered @@ -34,4 +34,6 @@ Focused temporary-repository tests cover a fresh branch tracking `origin/master` The explicit input makes an incorrect base possible but visible: both input refs and all three resolved commit IDs appear in either output form. Callers pay the small cost of verifying and fetching the live base before running the command. +The string schema deliberately cannot represent non-UTF-8 path bytes. A repository containing them must rename those paths before it can produce a report, preserving exact scope instead of returning a lossy one. + The repository owns one Git-topology helper and focused tests. In return, pre-push selection, code review, and documentation audit share a deterministic, read-only account of committed and local changes without importing forge or policy concerns. diff --git a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.zh.md b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.zh.md index 33ecddd966..042258122d 100644 --- a/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.zh.md +++ b/.agents/notes/implemented/process/2026-07-27-explicit-change-scope-report.zh.md @@ -12,13 +12,13 @@ Status: implemented ## 决策 -根目录的 `change-scope` 命令要求提供 `--base `，接受可选的 `--head `（默认为 `HEAD`），并提供带版本号的 `--json` 输出格式。该命令会检测歧义，将两个输入解析为 commit，并要求二者恰好有一个合并基点，之后才会输出结果。报告记录仓库根目录（不对路径中的合法空白字符作规范化处理）、当前分支、配置的上游、输入引用、解析后的基准、头部与合并基点 commit ID，以及排序后的已提交、已暂存、未暂存和未跟踪路径集合。 +根目录的 `change-scope` 命令要求提供 `--base `，接受可选的 `--head `（默认为 `HEAD`），并提供带版本号的 `--json` 输出格式。该命令会检测歧义，将两个输入解析为 commit，并要求二者恰好有一个合并基点，之后才会输出结果。报告记录仓库根目录（不对路径中的合法空白字符作规范化处理）、当前分支、配置的上游、输入引用、解析后的基准、头部与合并基点 commit ID，以及排序后的已提交、已暂存、未暂存和未跟踪路径集合。路径记录先按原始 NUL 字节切分，再逐条以严格 UTF-8 解码；遇到无效记录时，命令会在写出任何结果前失败，不会用替换字符代替无效字节或把不同路径合并为一条。 已提交路径由解析后的合并基点与头部之间的比较得出。即使 `--head` 指定其他 commit，各类未提交路径集合仍始终描述当前 worktree 与索引。diff 配置不能隐藏子模块，也不能调用外部 diff 或文本转换驱动；系统禁用重命名检测，因此重命名前后的路径都会保留在结果中。 该命令从不猜测或获取基准，不查询代码托管提供方，也不选择测试。调用该命令的每个工作流都会验证当前远端或堆叠状态、显式提供基准，并将这份事实报告作为语义评审或证据选择的输入。 -聚焦的临时仓库测试覆盖以下情形：新分支跟踪 `origin/master` 但没有同名远端分支；同一分支推送后的上游配置；以合法空白字符结尾的 worktree 路径；堆叠分支以非 master 分支为基准；所有未提交改动层；无效、有歧义及不指向 commit 的引用；人类可读输出与 JSON 输出保持确定性一致。测试还确认生成报告前后，引用、索引、配置与状态均不发生变化。 +聚焦的临时仓库测试覆盖以下情形：新分支跟踪 `origin/master` 但没有同名远端分支；同一分支推送后的上游配置；以合法空白字符结尾的 worktree 路径；堆叠分支以非 master 分支为基准；所有未提交改动层；两个互异的非 UTF-8 POSIX 路径会使报告失败且不产生部分输出；无效、有歧义及不指向 commit 的引用；人类可读输出与 JSON 输出保持确定性一致。测试还确认生成报告前后，引用、索引、配置与状态均不发生变化。 ## 考虑过的替代方案 @@ -34,4 +34,6 @@ Status: implemented 显式输入仍可能指定错误的基准，但这种错误是可见的：两种输出格式都会显示输入引用与解析出的三个 commit ID。调用方需要付出少量成本，在运行该命令前验证实时基准并从远端获取它。 +字符串 schema 有意不表示非 UTF-8 路径字节。含有这类路径的仓库必须先重命名这些路径才能生成报告，以此保持范围精确，而非返回有损结果。 + 仓库需要维护一个 Git 拓扑辅助工具及相应的聚焦测试。由此，pre-push 证据选择、代码评审与文档审计可以共享一份确定且只读的已提交及本地变更说明，而不必混入代码托管平台或策略职责。 diff --git a/scripts/change-scope.spec.ts b/scripts/change-scope.spec.ts index 69a23715e7..01829c96dd 100644 --- a/scripts/change-scope.spec.ts +++ b/scripts/change-scope.spec.ts @@ -26,7 +26,7 @@ afterEach(() => { for (const root of fixtureRoots.splice(0)) rmSync(root, { recursive: true, force: true }) }) -function git(cwd: string, args: string[], input?: string): string { +function git(cwd: string, args: string[], input?: string | Buffer): string { return execFileSync('git', ['-C', cwd, ...args], { encoding: 'utf8', env: { ...process.env, LANG: 'C', LC_ALL: 'C' }, @@ -35,6 +35,14 @@ function git(cwd: string, args: string[], input?: string): string { }).trim() } +function gitBytes(cwd: string, args: string[], input?: Buffer): Buffer { + return execFileSync('git', ['-C', cwd, ...args], { + env: { ...process.env, LANG: 'C', LC_ALL: 'C' }, + input, + stdio: ['pipe', 'pipe', 'pipe'], + }) +} + function write(path: string, content: string): void { mkdirSync(dirname(path), { recursive: true }) writeFileSync(path, content) @@ -187,6 +195,34 @@ describe('change-scope', () => { expect(repositoryState(root)).toEqual(before) }) + it.skipIf(process.platform === 'win32')('rejects distinct non-UTF-8 Git paths without partial output', () => { + const { root } = fixture() + const blobSha = git(root, ['hash-object', '-w', '--stdin'], 'content') + const entry = Buffer.from(`100644 ${blobSha}\t`, 'ascii') + const firstPath = Buffer.from([0x80]) + const secondPath = Buffer.from([0x81]) + gitBytes(root, ['update-index', '-z', '--index-info'], Buffer.concat([ + entry, + firstPath, + Buffer.from([0]), + entry, + secondPath, + Buffer.from([0]), + ])) + expect(gitBytes(root, ['diff', '--cached', '--name-only', '-z', '--'])).toEqual(Buffer.concat([ + firstPath, + Buffer.from([0]), + secondPath, + Buffer.from([0]), + ])) + const output: string[] = [] + + expect(() => { + writeChangeScope(['--base', 'HEAD', '--json'], root, chunk => output.push(chunk)) + }).toThrow('cannot inspect staged paths: Git path 1 is not valid UTF-8') + expect(output).toEqual([]) + }) + it('rejects missing, ambiguous, and non-commit refs before writing output', () => { const { root } = fixture() git(root, ['branch', 'collision']) diff --git a/scripts/change-scope.ts b/scripts/change-scope.ts index cbb3f1e185..743346b99e 100644 --- a/scripts/change-scope.ts +++ b/scripts/change-scope.ts @@ -3,10 +3,11 @@ import { spawnSync } from 'node:child_process' import { fileURLToPath } from 'node:url' import { resolve } from 'node:path' -import { parseArgs } from 'node:util' +import { parseArgs, TextDecoder } from 'node:util' const FORMAT_VERSION = 1 const MAX_GIT_OUTPUT = 64 * 1024 * 1024 +const UTF8_DECODER = new TextDecoder('utf-8', { fatal: true }) interface ChangeScopeReport { formatVersion: typeof FORMAT_VERSION @@ -39,6 +40,13 @@ interface GitCommandResult { error: Error | undefined } +interface GitBytesCommandResult { + status: number | null + stdout: Buffer + stderr: Buffer + error: Error | undefined +} + interface ChangeScopeOptions { base: string head: string @@ -59,6 +67,19 @@ function executeGit(cwd: string, args: string[]): GitCommandResult { } } +function executeGitBytes(cwd: string, args: string[]): GitBytesCommandResult { + const result = spawnSync('git', ['-C', cwd, ...args], { + env: { ...process.env, LANG: 'C', LC_ALL: 'C' }, + maxBuffer: MAX_GIT_OUTPUT, + }) + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + error: result.error, + } +} + function failureDetail(result: GitCommandResult): string { return result.error?.message ?? (result.stderr.trim() || `Git exited with status ${String(result.status)}`) } @@ -69,6 +90,16 @@ function requireGit(cwd: string, args: string[], context: string): string { return result.stdout } +function requireGitBytes(cwd: string, args: string[], context: string): Buffer { + const result = executeGitBytes(cwd, args) + if (result.status !== 0) { + const detail = result.error?.message + ?? (result.stderr.toString('utf8').trim() || `Git exited with status ${String(result.status)}`) + throw new Error(`${context}: ${detail}`) + } + return result.stdout +} + function parseOptions(args: string[]): ChangeScopeOptions { const { values } = parseArgs({ args, @@ -141,12 +172,27 @@ function comparePaths(left: string, right: string): number { return 0 } -function parsePathSet(output: string): string[] { - return [...new Set(output.split('\0').filter(Boolean))].sort(comparePaths) +function parsePathSet(output: Buffer, context: string): string[] { + const paths: string[] = [] + let start = 0 + let record = 0 + for (let end = 0; end < output.length; end += 1) { + if (output[end] !== 0) continue + if (end > start) { + record += 1 + try { + paths.push(UTF8_DECODER.decode(output.subarray(start, end))) + } catch { + throw new Error(`${context}: Git path ${record} is not valid UTF-8`) + } + } + start = end + 1 + } + return [...new Set(paths)].sort(comparePaths) } function diffPaths(root: string, args: string[], context: string): string[] { - return parsePathSet(requireGit(root, [ + return parsePathSet(requireGitBytes(root, [ 'diff', '--no-ext-diff', '--no-textconv', @@ -156,7 +202,7 @@ function diffPaths(root: string, args: string[], context: string): string[] { '-z', ...args, '--', - ], context)) + ], context), context) } function stripGitLineTerminator(output: string): string { @@ -194,11 +240,11 @@ function collectReport(options: ChangeScopeOptions, cwd: string): ChangeScopeRep committed: diffPaths(root, [mergeBaseSha, headSha], 'cannot inspect committed paths'), staged: diffPaths(root, ['--cached'], 'cannot inspect staged paths'), unstaged: diffPaths(root, [], 'cannot inspect unstaged paths'), - untracked: parsePathSet(requireGit( + untracked: parsePathSet(requireGitBytes( root, ['ls-files', '--others', '--exclude-standard', '-z', '--'], 'cannot inspect untracked paths', - )), + ), 'cannot inspect untracked paths'), }, } }