From ce156eb8ede85b77a2b64030691d29063ccbbc80 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 11:26:28 -0700 Subject: [PATCH] fix(sync): unquote git C-style-quoted paths in buildSyncManifest (#3897) (#3899) Wave-assembled from PR #3899 by @SergeyShol. Co-Authored-By: Sergey Sholom --- src/core/sync.ts | 61 +++++++++++++++++++++++++++--- test/sync.test.ts | 96 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 6 deletions(-) diff --git a/src/core/sync.ts b/src/core/sync.ts index 9519c7660..f147ec09c 100644 --- a/src/core/sync.ts +++ b/src/core/sync.ts @@ -126,6 +126,54 @@ const CODE_EXTENSIONS = new Set([ * a real, accepted CLI flag on every command that imports this file. Pinned by * `test/cli-flag-validation.test.ts`. */ +/** + * Undo git's C-style path quoting. + * + * git quotes any path containing `"`, `\` or a control character, and it does + * so unconditionally: `core.quotepath=false` (buildGitInvocation in + * commands/sync.ts, #119) only suppresses octal-escaping of NON-ASCII bytes. + * A path like `people/Jason "Jay" Strand.md` therefore reaches + * buildSyncManifest as `"people/Jason \"Jay\" Strand.md"` — surrounding quotes + * included — so it ends `.md"`, fails isMarkdownFilePath(), and is dropped from + * the manifest with no error and no counter. + * + * Octal escapes are decoded as BYTES and utf-8 decoded once at the end: a + * single codepoint can span several \NNN escapes, so per-escape decoding + * produces mojibake. + * + * An unquoted path is returned unchanged, so this is a no-op for the common + * case. + */ +export function unquoteGitPath(raw: string): string { + if (raw.length < 2 || !raw.startsWith('"') || !raw.endsWith('"')) return raw; + const body = raw.slice(1, -1); + const simple: Record = { + n: 10, t: 9, r: 13, '"': 34, '\\': 92, a: 7, b: 8, f: 12, v: 11, + }; + const bytes: number[] = []; + let i = 0; + while (i < body.length) { + const c = body[i]; + if (c === '\\' && i + 1 < body.length) { + const nxt = body[i + 1]; + if (nxt in simple) { + bytes.push(simple[nxt]); + i += 2; + continue; + } + const trio = body.slice(i + 1, i + 4); + if (trio.length === 3 && /^[0-7]{3}$/.test(trio)) { + bytes.push(parseInt(trio, 8)); + i += 4; + continue; + } + } + for (const b of new TextEncoder().encode(c)) bytes.push(b); + i += 1; + } + return new TextDecoder('utf-8').decode(new Uint8Array(bytes)); +} + export function buildSyncManifest(gitDiffOutput: string): SyncManifest { const manifest: SyncManifest = { added: [], @@ -145,19 +193,22 @@ export function buildSyncManifest(gitDiffOutput: string): SyncManifest { const action = parts[0]; + // Unquote at the single point every path enters the manifest, so the + // extension filter and every downstream consumer (slug resolution, file + // reads) all see the real name. (#3897) if (action === 'A') { - manifest.added.push(parts[1]); + manifest.added.push(unquoteGitPath(parts[1])); } else if (action === 'M' || action === 'T' || action === 'U') { // T (typechange) and U (unmerged) both mean "this path exists and its // content is not what we imported" — the same remedy as M. - manifest.modified.push(parts[1]); + manifest.modified.push(unquoteGitPath(parts[1])); } else if (action === 'D') { - manifest.deleted.push(parts[1]); + manifest.deleted.push(unquoteGitPath(parts[1])); } else if (action.startsWith('R') || action.startsWith('C')) { // Rename/copy: R100\told-path\tnew-path. Copy is unreachable without -C, // but if the flags ever change, the destination must still be imported. - const oldPath = parts[1]; - const newPath = parts[2]; + const oldPath = unquoteGitPath(parts[1]); + const newPath = unquoteGitPath(parts[2]); if (oldPath && newPath) { if (action.startsWith('C')) { // A copy leaves the source in place — only the destination is new. diff --git a/test/sync.test.ts b/test/sync.test.ts index d48fa208d..33735abd1 100644 --- a/test/sync.test.ts +++ b/test/sync.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test'; -import { buildSyncManifest, isSyncable, pathToSlug, pruneDir, isCodeFilePath } from '../src/core/sync.ts'; +import { buildSyncManifest, isSyncable, pathToSlug, pruneDir, isCodeFilePath, unquoteGitPath } from '../src/core/sync.ts'; import { buildAutoEmbedArgs, buildGitInvocation } from '../src/commands/sync.ts'; import { mkdtempSync, writeFileSync, rmSync, mkdirSync } from 'fs'; import { join } from 'path'; @@ -307,6 +307,100 @@ describe('buildSyncManifest edge cases', () => { }); }); +// ──────────────────────────────────────────────────────────────── +// C-style-quoted paths. git quotes any path containing `"`, `\` or a +// control character, unconditionally — core.quotepath=false (#119) only +// governs octal-escaping of NON-ASCII bytes. Before unquoteGitPath, such +// an entry ended `.md"`, failed isSyncable(), and was dropped from the +// manifest silently: no error, no warning, no counter. +// ──────────────────────────────────────────────────────────────── + +describe('unquoteGitPath', () => { + test('leaves an unquoted path untouched', () => { + expect(unquoteGitPath('people/plain-name.md')).toBe('people/plain-name.md'); + expect(unquoteGitPath('people/Ольга Петрова.md')).toBe('people/Ольга Петрова.md'); + expect(unquoteGitPath('')).toBe(''); + }); + + test('strips the wrapping quotes and unescapes embedded ones', () => { + expect(unquoteGitPath('"people/Jason \\"Jay\\" Strand.md"')) + .toBe('people/Jason "Jay" Strand.md'); + }); + + test('unescapes a literal backslash', () => { + expect(unquoteGitPath('"people/a\\\\b.md"')).toBe('people/a\\b.md'); + }); + + test('decodes single-character escapes', () => { + expect(unquoteGitPath('"people/a\\tb\\nc.md"')).toBe('people/a\tb\nc.md'); + }); + + test('decodes octal escapes as bytes, utf-8 decoding once at the end', () => { + // "ы" is U+044B = 0xD1 0x8B — one codepoint, two \NNN escapes. Decoding + // per-escape instead of per-byte yields mojibake here. + expect(unquoteGitPath('"people/\\321\\213.md"')).toBe('people/ы.md'); + }); + + test('resulting path passes the extension filter', () => { + expect(isSyncable(unquoteGitPath('"people/Jason \\"Jay\\" Strand.md"'))).toBe(true); + }); +}); + +describe('buildSyncManifest — C-style-quoted paths', () => { + test('unquotes add, modify and delete entries', () => { + const output = [ + 'A\t"people/Alice \\"Ace\\" Example.md"', + 'M\t"companies/ПАО \\"Ростелеком\\".md"', + 'D\t"people/Jason \\"Jay\\" Strand.md"', + ].join('\n'); + const manifest = buildSyncManifest(output); + expect(manifest.added).toEqual(['people/Alice "Ace" Example.md']); + expect(manifest.modified).toEqual(['companies/ПАО "Ростелеком".md']); + expect(manifest.deleted).toEqual(['people/Jason "Jay" Strand.md']); + }); + + test('unquotes both sides of a rename', () => { + const output = 'R100\t"people/old \\"nick\\".md"\t"people/new \\"nick\\".md"'; + const manifest = buildSyncManifest(output); + expect(manifest.renamed).toEqual([ + { from: 'people/old "nick".md', to: 'people/new "nick".md' }, + ]); + }); + + test('quoted entries survive the syncable filter', () => { + const output = 'M\t"people/Christian \\"Raz\\" Kippelt.md"'; + const manifest = buildSyncManifest(output); + expect(manifest.modified.filter(p => isSyncable(p))).toHaveLength(1); + }); + + test('real git output for a quoted filename reaches the manifest', () => { + const repo = mkdtempSync(join(tmpdir(), 'gbrain-quoted-path-')); + try { + const name = 'people/Alice "Ace" Example.md'; + execSync('git init -q .', { cwd: repo }); + execSync('git config user.email t@t.t && git config user.name t', { cwd: repo, shell: '/bin/bash' }); + mkdirSync(join(repo, 'people')); + writeFileSync(join(repo, name), 'x\n'); + execSync('git add -A && git commit -q -m one', { cwd: repo, shell: '/bin/bash' }); + writeFileSync(join(repo, name), 'x\ny\n'); + execSync('git add -A && git commit -q -m two', { cwd: repo, shell: '/bin/bash' }); + + // gbrain's own invocation, quotepath and all. + const argv = buildGitInvocation(repo, ['diff', '--name-status', '-M', 'HEAD~1..HEAD']); + const out = execSync(`git ${argv.map(a => JSON.stringify(a)).join(' ')}`, { encoding: 'utf-8' }); + + // git really does quote it, whatever core.quotepath says. + expect(out.trim()).toBe('M\t"people/Alice \\"Ace\\" Example.md"'); + + const manifest = buildSyncManifest(out); + expect(manifest.modified).toEqual([name]); + expect(manifest.modified.filter(p => isSyncable(p))).toHaveLength(1); + } finally { + rmSync(repo, { recursive: true, force: true }); + } + }); +}); + // ──────────────────────────────────────────────────────────────── // performSync dry-run (v0.17 regression guard for full-sync silent writes) // ────────────────────────────────────────────────────────────────