From 1a4a7cceb402da7b7551bf0f7ba8ad82c8a54ad5 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Mon, 21 Sep 2026 13:30:48 +0530 Subject: [PATCH] fix(patch): reject binary file changes in npm patch commit --- docs/lib/content/commands/npm-patch.md | 5 +++ lib/utils/patch-diff.js | 23 ++++++++++++-- test/lib/commands/patch.js | 25 +++++++++++++++ test/lib/utils/patch-diff.js | 43 ++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/docs/lib/content/commands/npm-patch.md b/docs/lib/content/commands/npm-patch.md index 030c414f4d6ac..b3acdc60d1cfc 100644 --- a/docs/lib/content/commands/npm-patch.md +++ b/docs/lib/content/commands/npm-patch.md @@ -39,6 +39,11 @@ package literally named like a subcommand must use the explicit form, e.g. writes the unified diff to `/@.patch`, adds the entry to `patchedDependencies`, and updates `package-lock.json`. + Patches are text diffs, so binary files (images, fonts, wasm, native + addons) cannot be patched. If a binary file in the edit directory differs + from the original, the commit fails with `EPATCHBINARY` and lists the + files to revert. + * `npm patch ls` Lists registered patches and how many installed nodes each one matches. diff --git a/lib/utils/patch-diff.js b/lib/utils/patch-diff.js index b0ab9a3aa0bff..4f4455830adf5 100644 --- a/lib/utils/patch-diff.js +++ b/lib/utils/patch-diff.js @@ -2,6 +2,7 @@ // Used by `npm patch commit` to capture edits against a clean tarball. // The output is consumed by Arborist's apply step (jsdiff parsePatch). const { createTwoFilesPatch } = require('diff') +const { isUtf8 } = require('node:buffer') const { readdir, readFile } = require('node:fs/promises') const { join, sep } = require('node:path') @@ -27,9 +28,12 @@ const listFiles = async dir => { return out } +// A NUL byte or invalid UTF-8 means the file cannot round-trip through a text diff. +const isBinary = buf => buf !== null && (buf.includes(0) || !isUtf8(buf)) + const readMaybe = async file => { try { - return await readFile(file, 'utf8') + return await readFile(file) } catch { return null } @@ -39,6 +43,7 @@ const readMaybe = async file => { // Added files use `--- /dev/null`, deleted files use `+++ /dev/null`. // The root package.json is excluded: Arborist resolves the pre-patch manifest, so a patched manifest would apply to disk without being honored. // ignore holds extra root-relative filenames the caller keeps out of the diff, e.g. the patch-update marker. +// A binary file that differs throws EPATCHBINARY because a text diff cannot represent it. const diffDirs = async (originalDir, editedDir, ignore = new Set()) => { const [origFiles, editFiles] = await Promise.all([ listFiles(originalDir), @@ -48,13 +53,14 @@ const diffDirs = async (originalDir, editedDir, ignore = new Set()) => { let result = '' let packageJsonChanged = false + const binaryFiles = [] for (const file of all) { const native = file.split('/').join(sep) const [a, b] = await Promise.all([ readMaybe(join(originalDir, native)), readMaybe(join(editedDir, native)), ]) - if (a === b) { + if (a === b || (a && b && a.equals(b))) { continue } @@ -69,8 +75,13 @@ const diffDirs = async (originalDir, editedDir, ignore = new Set()) => { continue } + if (isBinary(a) || isBinary(b)) { + binaryFiles.push(file) + continue + } + let patch = createTwoFilesPatch( - `a/${file}`, `b/${file}`, a || '', b || '', '', '' + `a/${file}`, `b/${file}`, a ? a.toString() : '', b ? b.toString() : '', '', '' ).replace('===================================================================\n', '') // mark adds and deletes with /dev/null so the apply step creates/removes files @@ -82,6 +93,12 @@ const diffDirs = async (originalDir, editedDir, ignore = new Set()) => { } result += patch } + if (binaryFiles.length) { + throw Object.assign( + new Error(`binary files cannot be patched: ${binaryFiles.join(', ')}. Revert them in the edit directory and commit again.`), + { code: 'EPATCHBINARY', files: binaryFiles } + ) + } return { diff: result, packageJsonChanged } } diff --git a/test/lib/commands/patch.js b/test/lib/commands/patch.js index ac409bf2d175a..234148640c202 100644 --- a/test/lib/commands/patch.js +++ b/test/lib/commands/patch.js @@ -494,6 +494,31 @@ t.test('commit: only package.json changed warns and writes no patch', async t => t.match(logs.warn.join('\n'), /only package.json changed/, 'warns package.json is not patchable') }) +t.test('commit: a changed binary file rejects with EPATCHBINARY and writes no patch', async t => { + const { npm, registry } = await loadMockNpm(t, { + config: { 'ignore-scripts': true, audit: false }, + strictRegistryNock: false, + prefixDir: basePrefix(), + }) + await setupDep(npm, registry) + await npm.exec('install', []) + + const editDir = path.join(npm.prefix, 'clean-edit') + await pacote.extract(`${DEP_NAME}@${DEP_VERSION}`, editDir, npm.flatOptions) + fs.writeFileSync(path.join(editDir, 'index.js'), 'module.exports = () => "patched"\n') + fs.writeFileSync(path.join(editDir, 'logo.png'), Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00])) + + await t.rejects( + npm.exec('patch', ['commit', editDir]), + { code: 'EPATCHBINARY', files: ['logo.png'] } + ) + t.notOk( + fs.existsSync(path.join(npm.prefix, 'patches', `${DEP_NAME}@${DEP_VERSION}.patch`)), + 'no patch file written' + ) + t.notOk(readJson(path.join(npm.prefix, 'package.json')).patchedDependencies, 'no patchedDependencies added') +}) + t.test('commit: package.json change alongside code is dropped with a warning', async t => { const { npm, logs, registry } = await loadMockNpm(t, { config: { 'ignore-scripts': true, audit: false }, diff --git a/test/lib/utils/patch-diff.js b/test/lib/utils/patch-diff.js index dd571651be8ec..aa5d3c07026ed 100644 --- a/test/lib/utils/patch-diff.js +++ b/test/lib/utils/patch-diff.js @@ -147,3 +147,46 @@ t.test('round-trip: applying the diff reproduces the edited tree', async t => { t.equal(read(orig, 'lib', 'deep', 'x.js'), 'after\n', 'nested file matches edit') t.notOk(existsSync(resolve(orig, 'del.js')), 'deleted file was removed') }) + +t.test('changed binary files are rejected with EPATCHBINARY', async t => { + const dir = t.testdir({ + orig: { + 'mod.bin': Buffer.from([0x00, 0x01, 0x02]), + 'latin1.txt': Buffer.from([0x63, 0x61, 0x66, 0xe9]), + 'same.bin': Buffer.from([0x00, 0xff]), + 'text.js': 'a\n', + }, + edit: { + 'mod.bin': Buffer.from([0x00, 0x01, 0x03]), + 'latin1.txt': Buffer.from([0x63, 0x61, 0x66, 0xe8]), + 'add.wasm': Buffer.from([0x00, 0x61, 0x73, 0x6d]), + 'same.bin': Buffer.from([0x00, 0xff]), + 'text.js': 'b\n', + }, + }) + await t.rejects( + diffDirs(resolve(dir, 'orig'), resolve(dir, 'edit')), + { code: 'EPATCHBINARY', files: ['add.wasm', 'latin1.txt', 'mod.bin'] } + ) +}) + +t.test('deleting a binary file is rejected with EPATCHBINARY', async t => { + const dir = t.testdir({ + orig: { 'gone.png': Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00]) }, + edit: {}, + }) + await t.rejects( + diffDirs(resolve(dir, 'orig'), resolve(dir, 'edit')), + { code: 'EPATCHBINARY', files: ['gone.png'], message: /gone\.png/ } + ) +}) + +t.test('unchanged binary files do not block a text diff', async t => { + const dir = t.testdir({ + orig: { 'img.png': Buffer.from([0x89, 0x00]), 'index.js': 'a\n' }, + edit: { 'img.png': Buffer.from([0x89, 0x00]), 'index.js': 'b\n' }, + }) + const { diff } = await diffDirs(resolve(dir, 'orig'), resolve(dir, 'edit')) + t.match(diff, 'a/index.js', 'text change is captured') + t.notMatch(diff, 'img.png', 'unchanged binary is not in the diff') +})