diff --git a/.gitattributes b/.gitattributes index b37363335a4..cbd3a039dc0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -12,8 +12,9 @@ /src/cli/bundled-skill-guides.ts text eol=lf # Bundled plugin trees are byte-hashed; CRLF checkout would break the pinned hash. /resources/plugins/** text eol=lf -# pnpm hashes this generated patch byte-for-byte, so a CRLF checkout breaks the install; -# its minified bundle lines also make a diff nobody can read. Review the hand-written -# source patch under xterm-src/ instead. -/config/patches/@xterm__xterm@*.patch -diff -text +# pnpm hashes every patch byte-for-byte, so a CRLF checkout breaks the install. +/config/patches/*.patch -text +# The xterm bundle hunks also make a diff nobody can read; review the hand-written +# source patch under xterm-src/ instead. The sibling patches stay diffable. +/config/patches/@xterm__xterm@*.patch -diff /config/patches/xterm-src/*.patch text eol=lf diff --git a/config/scripts/regenerate-xterm-patches.mjs b/config/scripts/regenerate-xterm-patches.mjs index 952fc01e6ea..160d075aee2 100644 --- a/config/scripts/regenerate-xterm-patches.mjs +++ b/config/scripts/regenerate-xterm-patches.mjs @@ -139,13 +139,6 @@ export function assertSourceDerivationsAgree(checkoutSource, patchText) { ) } -/** The derived half of a patch: build output, never edited by hand. */ -export function generatedHunks(patchText, generatedPaths) { - return selectPatchEntries(patchText, (file) => - generatedPaths.some((prefix) => file.startsWith(prefix)) - ) -} - export function stampVersionSource(source, version) { const stamped = source.replace( /export const XTERM_VERSION = '[^']+';/, @@ -196,9 +189,11 @@ export function assertBuildStepsAllowed(manifest) { } } -export const SOURCEMAP_POLICIES = new Set(['include', 'delete']) +// `delete` was dropped with the code that implemented it: accepting a policy +// nothing honours would silently ship maps that do not match the bundle. +export const SOURCEMAP_POLICIES = new Set(['include']) -/** A typo would fall through to `include` and re-inflate the patch by 5.8 MB. */ +/** An unrecognised policy is a manifest bug, not a default to fall through to. */ export function assertSourcemapPolicy(manifest) { const policy = manifest.sourcemaps?.policy if (!SOURCEMAP_POLICIES.has(policy)) { @@ -298,11 +293,17 @@ export function formatCheckFailure({ name, patchPath, committed, regenerated }) ].join('\n') } +// npm ships as `npm.cmd` on Windows, and execFile applies no PATHEXT and refuses +// a `.cmd` target without a shell (CVE-2024-27980). git and tar are real .exe. +const WINDOWS_SHIM_COMMANDS = new Set(['npm', 'npx', 'pnpm', 'yarn']) + function run(command, args, options = {}) { - return execFileSync(command, args, { + const shim = process.platform === 'win32' && WINDOWS_SHIM_COMMANDS.has(command) + return execFileSync(shim ? `${command}.cmd` : command, args, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'inherit'], + shell: shim, ...options }) } @@ -504,29 +505,6 @@ function diffFolders(folderA, folderB) { return normalizePnpmDiff(stdout, folderA, folderB) } -// Why: dropping only the map hunks would ship maps whose offsets no longer line -// up with the patched bundle. Deleting the maps outright is the honest form of -// the same size saving, and the diff carries it as a file-deletion stanza. -function deleteGeneratedSourcemaps(patchedDir, packageEntry) { - for (const relative of listFilesRelative(patchedDir)) { - const posix = toPosix(relative) - if (!packageEntry.generatedPaths.some((prefix) => posix.startsWith(prefix))) { - continue - } - const absolute = path.join(patchedDir, relative) - if (posix.endsWith('.map')) { - rmSync(absolute) - continue - } - // The reference outlives the file it points at, so it goes with it. - const text = readFileSync(absolute, 'utf8') - const stripped = text.replace(/\n\/\/# sourceMappingURL=[^\n]*\n?$/, '') - if (stripped !== text) { - writeFileSync(absolute, stripped) - } - } -} - /** The source of truth for the hand-written half: what the checkout itself holds. */ function diffCheckoutSource(packageRoot) { return run('git', [...CHECKOUT_DIFF_FLAGS, '--', 'src/'], { @@ -557,9 +535,6 @@ function regeneratePackage(packageEntry, manifest, context) { const patchedDir = path.join(workDir, 'patched', packageEntry.name.replace(/[@/]/g, '_')) overlayBuildOutput(pristineDir, upstreamRoot, packageEntry, patchedDir) - if (assertSourcemapPolicy(manifest) === 'delete') { - deleteGeneratedSourcemaps(patchedDir, packageEntry) - } // Leave the checkout diffable: the pinned commit plus the source patch, with // no publish-time version stamp mixed in, so `git diff` there is the source diff --git a/config/scripts/regenerate-xterm-patches.test.mjs b/config/scripts/regenerate-xterm-patches.test.mjs index f1231a87f52..50c309f21b0 100644 --- a/config/scripts/regenerate-xterm-patches.test.mjs +++ b/config/scripts/regenerate-xterm-patches.test.mjs @@ -13,7 +13,6 @@ import { assertSourcemapPolicy, firstDifferenceIndex, formatCheckFailure, - generatedHunks, lockfilePatchHashIsStale, normalizePnpmDiff, patchHash, @@ -26,6 +25,15 @@ import { updateLockfilePatchHash } from './regenerate-xterm-patches.mjs' +// Only the tests need to slice the generated half out of a patch; the generator +// reads `generatedPaths` directly where it compares against the pristine build. +function generatedHunks(patchText, generatedPaths) { + return splitPatchEntries(patchText) + .filter((entry) => generatedPaths.some((prefix) => entry.path.startsWith(prefix))) + .map((entry) => entry.text) + .join('') +} + const REPO_ROOT = path.resolve(import.meta.dirname, '..', '..') const MANIFEST_PATH = path.join(REPO_ROOT, 'config', 'patches', 'xterm-upstream.json') const temporaryDirectories = [] @@ -318,10 +326,14 @@ describe('manifest guards', () => { }) it('refuses a sourcemap policy it does not implement', () => { - expect(assertSourcemapPolicy({ sourcemaps: { policy: 'delete' } })).toBe('delete') expect(assertSourcemapPolicy({ sourcemaps: { policy: 'include' } })).toBe('include') + // `delete` named a code path that no longer exists; accepting it would ship + // maps that do not match the bundle. + expect(() => assertSourcemapPolicy({ sourcemaps: { policy: 'delete' } })).toThrow( + /must be one of include, got "delete"/ + ) expect(() => assertSourcemapPolicy({ sourcemaps: { policy: 'exclude' } })).toThrow( - /must be one of include, delete, got "exclude"/ + /must be one of include, got "exclude"/ ) expect(() => assertSourcemapPolicy({})).toThrow(/got undefined/) }) diff --git a/docs/reference/xterm-patch-regeneration.md b/docs/reference/xterm-patch-regeneration.md index c63f24e23d3..dead914ef74 100644 --- a/docs/reference/xterm-patch-regeneration.md +++ b/docs/reference/xterm-patch-regeneration.md @@ -41,13 +41,12 @@ and are tracked separately; see [Known Gaps](#known-gaps). silently patch the wrong tree. 5. Sourcemaps move with the bundle, and are never silently omitted. The patch moves the code, so dropping only the map hunks would ship offsets pointing at - the wrong lines. `sourcemaps.policy` in the manifest picks between patching - them (`include`, today) and deleting them (`delete`); omitting them is not an - option either setting offers. `include` costs about 5.8 MB of the emitted - patch and is required because + the wrong lines. `sourcemaps.policy` accepts `include` and nothing else: it + costs about 5.8 MB of the emitted patch and is required because `src/renderer/src/components/terminal-pane/terminal-ime-xterm-transaction-events.test.ts` reads `lib/*.map` and asserts the mapped `Version.ts` matches the runtime - version. `delete` becomes available again only once nothing reads them. + version. Deleting the maps was once an option; the code that did it was + removed as unreachable, so re-adding the policy means re-adding that code. 6. `--check` is the authority on the lockfile, not `pnpm install`. pnpm writes the patch hash in two places — `patchedDependencies` and every resolution key that depends on the patched package — and on a warm store it will leave the @@ -231,8 +230,10 @@ and which folding them into this manifest would also fix. Both addons do build from the pinned commit: the registry stamps `53a98a720ae4a973e384fa2440880d09537132f3` on `addon-webgl@0.20.0-beta.286` and `addon-serialize@0.15.0-beta.287` alike, despite the mismatched version numbers. -Their published `lib/*.mjs` and `lib/*.mjs.map` reproduce byte for byte from that -commit; this was measured, not assumed. +On 2026-08-17 their published `lib/*.mjs` and `lib/*.mjs.map` reproduced byte for +byte from that commit. That was a one-off measurement, not an invariant: no check +in this repo re-runs it, so treat it as a starting point to re-measure rather than +as something the harness holds true. What blocks folding them in is the other half of their published output. Both also ship a CJS `lib/addon-*.js`, and the root `package` script does not build