From 9f85ef528ce1c993657fcc102135373095961bf6 Mon Sep 17 00:00:00 2001 From: Sarath Francis Date: Mon, 29 Jun 2026 22:57:56 -0400 Subject: [PATCH 1/2] Throw EEXIST when ensureSymlink dest is a broken symlink ensureSymlink/ensureSymlinkSync stat() the destination to check whether an existing symlink already points at srcpath. When that symlink is broken, following it fails and the raw ENOENT (stat '') surfaced to the caller, which is confusing: a symlink that points at a *different* existing target already throws EEXIST. Skip the idempotency check when the destination can't be followed so the existing creation path reports EEXIST consistently, broken or not. Closes #925 --- lib/ensure/__tests__/symlink.test.js | 23 +++++++++++++++++++++++ lib/ensure/symlink.js | 18 ++++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/lib/ensure/__tests__/symlink.test.js b/lib/ensure/__tests__/symlink.test.js index cec7e9dd..e0850065 100644 --- a/lib/ensure/__tests__/symlink.test.js +++ b/lib/ensure/__tests__/symlink.test.js @@ -399,4 +399,27 @@ describe('fse-ensure-symlink', () => { assert.strictEqual(content, 'content') }) }) + + // https://github.com/jprichardson/node-fs-extra/issues/925 + describe('ensureSymlink() when dest is an existing broken symlink (issue #925)', () => { + it('should fail with EEXIST (not ENOENT) when dest is a broken symlink', async () => { + const srcpath = path.join(TEST_DIR, 'src-925.txt') + const dstpath = path.join(TEST_DIR, 'broken-link-925') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-925.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + await assert.rejects(ensureSymlink(srcpath, dstpath), { code: 'EEXIST' }) + }) + + it('should fail with EEXIST (not ENOENT) when dest is a broken symlink (sync)', () => { + const srcpath = path.join(TEST_DIR, 'src-925-sync.txt') + const dstpath = path.join(TEST_DIR, 'broken-link-925-sync') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-925-sync.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + assert.throws(() => ensureSymlinkSync(srcpath, dstpath), { code: 'EEXIST' }) + }) + }) }) diff --git a/lib/ensure/symlink.js b/lib/ensure/symlink.js index a7de8cad..f283672e 100644 --- a/lib/ensure/symlink.js +++ b/lib/ensure/symlink.js @@ -35,8 +35,13 @@ async function createSymlink (srcpath, dstpath, type) { } } - const dstStat = await fs.stat(dstpath, { bigint: true }) - if (areIdentical(srcStat, dstStat)) return + // Following the existing symlink fails if it's broken; in that case fall + // through so fs.symlink reports EEXIST, rather than leaking the stat ENOENT. + let dstStat + try { + dstStat = await fs.stat(dstpath, { bigint: true }) + } catch { } + if (dstStat && areIdentical(srcStat, dstStat)) return } const relative = await symlinkPaths(srcpath, dstpath) @@ -72,8 +77,13 @@ function createSymlinkSync (srcpath, dstpath, type) { } } - const dstStat = fs.statSync(dstpath, { bigint: true }) - if (areIdentical(srcStat, dstStat)) return + // Following the existing symlink fails if it's broken; in that case fall + // through so fs.symlinkSync reports EEXIST, rather than leaking the stat ENOENT. + let dstStat + try { + dstStat = fs.statSync(dstpath, { bigint: true }) + } catch { } + if (dstStat && areIdentical(srcStat, dstStat)) return } const relative = symlinkPathsSync(srcpath, dstpath) From 92eb03eac8b0a2bf65064630b381fc9132b55665 Mon Sep 17 00:00:00 2001 From: Sarath Francis Date: Mon, 13 Jul 2026 14:52:14 -0400 Subject: [PATCH 2/2] Only swallow ENOENT when checking symlink dest, rethrow otherwise When an existing dest is a symlink, the stat that checks whether it already points at src is expected to fail with ENOENT for a broken link. Narrow the catch so that only ENOENT is swallowed (falling through to the existing EEXIST behavior) and any other error propagates instead of being hidden. --- lib/ensure/__tests__/symlink.test.js | 50 ++++++++++++++++++++++++++++ lib/ensure/symlink.js | 18 ++++++---- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/lib/ensure/__tests__/symlink.test.js b/lib/ensure/__tests__/symlink.test.js index e0850065..92222738 100644 --- a/lib/ensure/__tests__/symlink.test.js +++ b/lib/ensure/__tests__/symlink.test.js @@ -7,6 +7,8 @@ const os = require('os') const fse = require('../..') const path = require('path') const assert = require('assert') +const proxyquire = require('proxyquire') +const realFs = require('../../fs') const _symlinkPaths = require('../symlink-paths') const symlinkPathsSync = _symlinkPaths.symlinkPathsSync const ensureSymlink = fse.ensureSymlink @@ -422,4 +424,52 @@ describe('fse-ensure-symlink', () => { assert.throws(() => ensureSymlinkSync(srcpath, dstpath), { code: 'EEXIST' }) }) }) + + // When checking whether an existing dest symlink points at src, only an + // ENOENT (broken link) should be swallowed. Any other error must propagate. + describe('ensureSymlink() when statting the existing dest fails for a non-ENOENT reason', () => { + it('should rethrow the error instead of swallowing it', async () => { + const srcpath = path.join(TEST_DIR, 'src-eacces.txt') + const dstpath = path.join(TEST_DIR, 'dst-symlink-eacces') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-eacces.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + const fsStub = { + stat (p, ...args) { + if (p === dstpath) { + const err = new Error('EACCES: permission denied, stat') + err.code = 'EACCES' + return Promise.reject(err) + } + return realFs.stat(p, ...args) + } + } + const { createSymlink } = proxyquire('../symlink', { '../fs': fsStub }) + + await assert.rejects(createSymlink(srcpath, dstpath), { code: 'EACCES' }) + }) + + it('should rethrow the error instead of swallowing it (sync)', () => { + const srcpath = path.join(TEST_DIR, 'src-eacces-sync.txt') + const dstpath = path.join(TEST_DIR, 'dst-symlink-eacces-sync') + fs.writeFileSync(srcpath, 'src\n') + fs.symlinkSync(path.join(TEST_DIR, 'missing-eacces-sync.txt'), dstpath) + assert.strictEqual(fs.lstatSync(dstpath).isSymbolicLink(), true) + + const fsStub = { + statSync (p, ...args) { + if (p === dstpath) { + const err = new Error('EACCES: permission denied, stat') + err.code = 'EACCES' + throw err + } + return realFs.statSync(p, ...args) + } + } + const { createSymlinkSync } = proxyquire('../symlink', { '../fs': fsStub }) + + assert.throws(() => createSymlinkSync(srcpath, dstpath), { code: 'EACCES' }) + }) + }) }) diff --git a/lib/ensure/symlink.js b/lib/ensure/symlink.js index f283672e..295b236f 100644 --- a/lib/ensure/symlink.js +++ b/lib/ensure/symlink.js @@ -35,12 +35,15 @@ async function createSymlink (srcpath, dstpath, type) { } } - // Following the existing symlink fails if it's broken; in that case fall - // through so fs.symlink reports EEXIST, rather than leaking the stat ENOENT. + // Following the existing symlink fails with ENOENT if it's broken; in that + // case fall through so fs.symlink reports EEXIST, rather than leaking the + // stat ENOENT. Any other error is unexpected, so rethrow it. let dstStat try { dstStat = await fs.stat(dstpath, { bigint: true }) - } catch { } + } catch (err) { + if (err.code !== 'ENOENT') throw err + } if (dstStat && areIdentical(srcStat, dstStat)) return } @@ -77,12 +80,15 @@ function createSymlinkSync (srcpath, dstpath, type) { } } - // Following the existing symlink fails if it's broken; in that case fall - // through so fs.symlinkSync reports EEXIST, rather than leaking the stat ENOENT. + // Following the existing symlink fails with ENOENT if it's broken; in that + // case fall through so fs.symlinkSync reports EEXIST, rather than leaking the + // stat ENOENT. Any other error is unexpected, so rethrow it. let dstStat try { dstStat = fs.statSync(dstpath, { bigint: true }) - } catch { } + } catch (err) { + if (err.code !== 'ENOENT') throw err + } if (dstStat && areIdentical(srcStat, dstStat)) return }