Relative paths aren't evaluated properly in symlinked files

[rovo89]

I'm trying to include the devcontainer command in my Docker image. I extend a Next.js a standalone build by setting outputFileTracingIncludes to the results of nodeFileTrace() for the additional binaries I want to bundle.

Anyway, the issue is easily reproducable by installing @devcontainers/cli locally and then:

$ node_modules/.bin/nft print node_modules/.bin/devcontainer 
FILELIST:
node_modules/.bin/devcontainer
node_modules/@devcontainers/cli/devcontainer.js


WARNINGS:
Error: Failed to resolve dependency "./dist/spec-node/devContainersSpecCLI":
Cannot find module '/home/dev/vibe-web/node_modules/.bin/dist/spec-node/devContainersSpecCLI' loaded from /home/dev/vibe-web/node_modules/.bin/devcontainer

As usual, node_modules/.bin/devcontainer is just a symlink to ../@devcontainers/cli/devcontainer.js, which is really just:

#!/usr/bin/env node
/*---------------------------------------------------------------------------------------------
 *  Copyright (c) Microsoft Corporation. All rights reserved.
 *  Licensed under the MIT License. See License.txt in the project root for license information.
 *--------------------------------------------------------------------------------------------*/

require('./dist/spec-node/devContainersSpecCLI');

Seemingly, this relative path should be evaluated based on the actual file location / symlink target (../@devcontainers/cli/). Here's the test that it would work (despite some strange warnings that I assume to be caused by some settings in the nft CLI):

$ node_modules/.bin/nft print node_modules/.bin/../@devcontainers/cli/devcontainer.js
Skipping asset emission of /home/dev/vibe-web/*/* for /home/dev/vibe-web/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js as it is outside the package base /home/dev/vibe-web/node_modules/@devcontainers/cli
Skipping asset emission of /home/dev/vibe-web/*/* for /home/dev/vibe-web/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js as it is outside the package base /home/dev/vibe-web/node_modules/@devcontainers/cli
Skipping asset emission of /home/dev/vibe-web/*/* for /home/dev/vibe-web/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js as it is outside the package base /home/dev/vibe-web/node_modules/@devcontainers/cli
Skipping asset emission of /tmp for /home/dev/vibe-web/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js as it is outside the package base /home/dev/vibe-web/node_modules/@devcontainers/cli
FILELIST:
node_modules/@devcontainers/cli/devcontainer.js
node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js
node_modules/@devcontainers/cli/package.json

[rovo89]

My workaround is to run my list of files to keep (e.g. node_modules/.bin/devcontainer) through this function:

function resolveSymlinks(files: string[]) {
  return files.flatMap((file) => {
    try {
      const stats = fs.lstatSync(file);
      if (stats.isSymbolicLink()) {
        const target = fs.readlinkSync(file);
        const resolvedTarget = path.normalize(path.join(path.dirname(file), target));
        return [file, resolvedTarget];
      }
      return [file];
    } catch {
      return [file];
    }
  });
}

[styfle]

The default behavior is to use fs.readFile().

If you want to override the default behavior, you can here:

https://github.com/vercel/nft/blob/main/readme.md#hooks

[rovo89]

The default behavior is to use fs.readFile().

If you want to override the default behavior, you can here:

https://github.com/vercel/nft/blob/main/readme.md#hooks

Hm. Shouldn't this work out of the box though? I mean, it's not like I have a virtual file system or generated files or anything. fs.readFile() can be used, it just needs the correct path.

And even if I wanted to use the hook - how would I know which file contained the require()? From the docs, it seems I would only get the (relative?) path, but that isn't enough to solve the problem at hand, which is calculating the correct path.

[rovo89]

Just had a look at the hooks again. The resolve hook looks more appropriate, assuming that parent would be the path to the file which contains the require(). Whether that's easier than my workaround... I don't think so. If you say that this package doesn't want to handle symlinks itself, then I guess I'll keep my workaround. I'm a bit surprised though, because all the files in node_modules/.bin are symlinks, and I wouldn't be surprised if many of the packages used relative includes in their binaries.

[styfle]

There is symlink detection here:

nft/src/node-file-trace.ts

Line 258 in 34f1ec7

const symlink = await this.readlink(path);

I confirmed this is a bug.

Feel free to submit a PR to fix it (along with a test), thanks!

[rovo89]

Feel free to submit a PR to fix it (along with a test), thanks!

Done: #564

[rovo89]

@styfle Could I bother you to review the PR please? The fix is almost trivial, most of the changed lines are due to the different variable name and the added test.

[styfle]

I'll have @vercel/ci-cd team review to confirm this will work correctly with consumers like @vercel/node

[github-actions]

🎉 This issue has been resolved in version 1.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀