`createSymlink` fails when ensuring a symbolic link with a relative path if the link already exists

[jandockx]

• Operating System: macOS 14.2.1
• Node.js version: v20.11.1
• fs-extra version: 11.2.0

Issue

• create a target directory with a file in it
• create a symbolic link from somewhere else to the target directory
• do this a second time

This works if the reference to the target is an absolute path, but fails if the reference is a relative path.

Example

This is a mocha test that shows the issue. The first test ensures the symbolic link a second time with an absolute path, which succeeds as expected. The second test ensures the symbolic link a second time with a relative path, which is rejected with ENOENT, but should succeed too.

/* eslint-env mocha */

require('should')
const { resolve, join, relative, dirname } = require('node:path')
const { ensureFile, remove, pathExists, ensureSymlink } = require('fs-extra')

const testBaseDirectory = resolve('fs-extra-test-base-directory')

describe('fs-extra ensureSymlink fails when ensuring a symbolic link with a relative path if it already exists', function () {
  beforeEach(async function () {
    // a directory with a file, as `destination` or `target`
    this.targetDirectory = join(testBaseDirectory, 'target-directory')
    const targetFileName = 'target-file'
    this.targetDirectoryFile = join(this.targetDirectory, targetFileName)
    await ensureFile(this.targetDirectoryFile)
    // a directory to put the symbolic link in (the `source`)
    this.linkDirectory = join(testBaseDirectory, 'link-directory')
    this.symbolicLinkPath = join(this.linkDirectory, 'link')
    this.targetFileViaSymbolicLink = join(this.symbolicLinkPath, targetFileName)
    this.relativeSymbolicLinkReference = relative(dirname(this.symbolicLinkPath), this.targetDirectory)
  })

  afterEach(async function () {
    return remove(testBaseDirectory)
  })

  it('can ensure a symbolic link a second time with an absolute path', async function () {
    await pathExists(this.targetDirectoryFile).should.be.resolvedWith(true)

    // first time, setting up with a relative reference
    await ensureSymlink(this.relativeSymbolicLinkReference, this.symbolicLinkPath, 'dir').should.be.resolved()
    await pathExists(this.symbolicLinkPath).should.be.resolvedWith(true)
    await pathExists(this.targetFileViaSymbolicLink).should.be.resolvedWith(true)

    // second time, setting up with an absolute reference
    await ensureSymlink(this.targetDirectory, this.symbolicLinkPath, 'dir').should.be.resolved()
    await pathExists(this.symbolicLinkPath).should.be.resolvedWith(true)
    await pathExists(this.targetFileViaSymbolicLink).should.be.resolvedWith(true)
  })

  it('can ensure a symbolic link a second time with a relative path', async function () {
    await pathExists(this.targetDirectoryFile).should.be.resolvedWith(true)

    // first time, setting up with a relative reference
    await ensureSymlink(this.relativeSymbolicLinkReference, this.symbolicLinkPath, 'dir').should.be.resolved()
    await pathExists(this.symbolicLinkPath).should.be.resolvedWith(true)
    await pathExists(this.targetFileViaSymbolicLink).should.be.resolvedWith(true)

    // second time, setting up with a relative reference SHOULD ALSO RESOLVE, BUT REJECTS
    const error = await ensureSymlink(
      this.relativeSymbolicLinkReference,
      this.symbolicLinkPath,
      'dir'
    ).should.be.rejected()
    error.code.should.equal('ENOENT')
    // YET THE TARGET FILE EXISTS VIA THE ABSOLUTE PATH
    await pathExists(this.targetDirectory).should.be.resolvedWith(true)
    // AND THE RELATIVE PATH RESOLVES TO THE ABSOLUTE PATH
    join(dirname(this.symbolicLinkPath), this.relativeSymbolicLinkReference).should.equal(this.targetDirectory)
  })
})

Analysis

The issue is clear in fs-extra/lib/ensure/symlink.js, line 24, versus line 32.

When there is no symbolic link yet at dstpath, the if of line 22 is skipped

  let stats
  try {
    stats = await fs.lstat(dstpath)
  } catch { }

  if (stats && stats.isSymbolicLink()) {
  …
  }

and we arrive at line 31—32 where work is done to deal with relative srcpaths:

  const relative = await symlinkPaths(srcpath, dstpath)
  srcpath = relative.toDst

When there is a symbolic link at dstpath, the if–branch at line 22 is executed. Here, the status of the srcpath is requested as is:

fs.stat(srcpath),

This evaluates a relative srcpath relative to the cwd, not to the dstpath. At that location the source does not exist, which results in ENOENT.

[jandockx]

In a project: https://bitbucket.org/toryt/git-backup/src/3c9d47dfd169b00613568ec829b827e95ab070c7/test/30.Remote/fs-extra-createSymlink-bug.test.js

[RyanZim]

Good catch, that's definitely a bug.

BTW, I appreciate your clearly-written issue; it has all the information I need, and none of the information I don't. It's a breath of fresh air; if everyone filed issues like this, my job as an open-source maintainer would be much easier. Just thought it's worthy of a shout-out.

Looks like createSymlinkSync has the same bug as well: https://github.com/jprichardson/node-fs-extra/blob/426bb46067a57deacb93259d5af8b20d8ab528c0/lib/ensure/symlink.js#L58-L67

It seems like the simplest fix would be to stat both the srcpath & the srcpath relative to the dst, with proper error handling (as we expect at least one of the calls to fail), then check which one actually returned stats, and run it through the areIdentical check.

If you want to take a stab at a PR, that'd be welcome; otherwise, I'll try to get to this myself in the coming week or two.

[jandockx]

Glad to be of service.

I will not be taking a stab at a PR at this time. I'm trying to make progress on my own thing (https://bitbucket.org/toryt/git-backup) during my holiday, where fs-extra has been a great help so far just getting some things done. Thanks. This was just the first time I started playing with symbolic links.

I've worked around the issue for now, so, as far as I’m concerned, take your time. fs-extra has been a tremendous help in personal and professional projects for years already, so thank you again.

Just be sure not to include any backdoors in here :-).