fix: node fs.readlinkSync patch should maintain relative links (#2661)

jbedard · web-flow · commit dd02fdef5aef · 2026-01-19T13:14:56.000-08:00
Fix #2340 ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - New test cases added
diff --git a/js/private/devserver/js_run_devserver.mjs b/js/private/devserver/js_run_devserver.mjs
diff --git a/js/private/devserver/src/js_run_devserver.mjs b/js/private/devserver/src/js_run_devserver.mjs
@@ -143,9 +143,8 @@ async function syncSymlink(file, src, dst, sandbox, exists) {
     let symlinkMeta = ''
     if (isNodeModulePath(file)) {
         let linkPath = await fs.promises.readlink(src)
-        if (path.isAbsolute(linkPath)) {
-            linkPath = path.relative(src, linkPath)
-        }
+        const linkAbs = path.resolve(path.dirname(src), linkPath)
+        linkPath = path.relative(src, linkAbs) || '.'
         // Special case for 1p node_modules symlinks
         const maybe1pSync = path.join(sandbox, file, linkPath)
         if (fs.existsSync(maybe1pSync)) {
diff --git a/js/private/node-patches/fs.cjs b/js/private/node-patches/fs.cjs
diff --git a/js/private/node-patches/src/fs.cts b/js/private/node-patches/src/fs.cts
@@ -301,11 +301,12 @@ export function patcher(roots: string[]): () => void {
         ) {
             if (err) return cb(err)
             const resolved = resolvePathLike(args[0])
-            const str = path.resolve(path.dirname(resolved), p!)
-            const escapedRoot: string | false = isEscape(resolved, str)
+            const linkTarget = p!
+            const targetAbs = path.resolve(path.dirname(resolved), linkTarget)
+            const escapedRoot: string | false = isEscape(resolved, targetAbs)
             if (escapedRoot) {
                 const escapedRoots = [escapedRoot]
-                return nextHop(str, readlinkNextHopCb)
+                return nextHop(targetAbs, readlinkNextHopCb)
 
                 function readlinkNextHopCb(next: string | undefined | false) {
                     if (!next) {
@@ -319,16 +320,20 @@ export function patcher(roots: string[]): () => void {
                     }
                     const r = path.resolve(
                         path.dirname(resolved),
-                        path.relative(path.dirname(str), next)
+                        path.relative(path.dirname(targetAbs), next)
                     )
                     if (r != resolved && !isEscape(resolved, r, escapedRoots)) {
-                        return cb(null, r)
+                        if (path.isAbsolute(linkTarget)) {
+                            return cb(null, r)
+                        }
+                        const rel = path.relative(path.dirname(resolved), r)
+                        return cb(null, rel || '.')
                     }
                     // The escape from the root is not mappable back into the root; throw EINVAL
                     return cb(einval('readlink', args[0]))
                 }
             } else {
-                return cb(null, str)
+                return cb(null, linkTarget)
             }
         }
 
@@ -339,15 +344,12 @@ export function patcher(roots: string[]): () => void {
         ...args: Parameters<typeof origReadlinkSync>
     ) {
         const resolved = resolvePathLike(args[0])
+        const linkTarget = origReadlinkSync(...args)
+        const targetAbs = path.resolve(path.dirname(resolved), linkTarget)
 
-        const str = path.resolve(
-            path.dirname(resolved),
-            origReadlinkSync(...args)
-        )
-
-        const escapedRoot: string | false = isEscape(resolved, str)
+        const escapedRoot: string | false = isEscape(resolved, targetAbs)
         if (escapedRoot) {
-            const next = nextHopSync(str)
+            const next = nextHopSync(targetAbs)
             if (!next) {
                 if (next == undefined) {
                     // The escape from the root is not mappable back into the root; throw EINVAL
@@ -359,15 +361,19 @@ export function patcher(roots: string[]): () => void {
             }
             const r = path.resolve(
                 path.dirname(resolved),
-                path.relative(path.dirname(str), next)
+                path.relative(path.dirname(targetAbs), next)
             )
             if (r != resolved && !isEscape(resolved, r, [escapedRoot])) {
-                return r
+                if (path.isAbsolute(linkTarget)) {
+                    return r
+                }
+                const rel = path.relative(path.dirname(resolved), r)
+                return rel || '.'
             }
             // The escape from the root is not mappable back into the root; throw EINVAL
             throw einval('readlink', args[0])
         }
-        return str
+        return linkTarget
     }
 
     // =========================================================================
diff --git a/js/private/test/image/checksum_test.expected b/js/private/test/image/checksum_test.expected
@@ -1,4 +1,4 @@
-8fc69600f1f334af341785b642217395b09edd6b4f0e8032fa09693b03d21439  js/private/test/image/cksum_node.tar
+c95723d23b1d038ea1f7319f4c68fde5f81812dff389b988cd403e2e20f164a8  js/private/test/image/cksum_node.tar
 c6c899aac772f0dd98117f2a469af76d8fb6ad27ccc7bfbc68f5e4ed274bf0ea  js/private/test/image/cksum_package_store_3p.tar
 bcd3826edb788a083e88ae3dbfb7fbd86bb1de8d8ef1db881b6488d63d715245  js/private/test/image/cksum_package_store_1p.tar
 febf95a6d554c9bda3f0515bfd5ef273ac67d31c231d8162beaef8c4b7bc72f3  js/private/test/image/cksum_node_modules.tar
diff --git a/js/private/test/image/custom_layers_nomatch_test_node.listing b/js/private/test/image/custom_layers_nomatch_test_node.listing
@@ -8,7 +8,7 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       34729 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0       35271 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
 -r-xr-xr-x  0 0      0        1460 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
diff --git a/js/private/test/image/custom_owner_test_node.listing b/js/private/test/image/custom_owner_test_node.listing
@@ -7,7 +7,7 @@ drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runf
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 100    0       34729 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 100    0       35271 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
 -r-xr-xr-x  0 100    0        1460 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 100    0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
diff --git a/js/private/test/image/default_test_node.listing b/js/private/test/image/default_test_node.listing
@@ -7,7 +7,7 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runf
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       34729 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0       35271 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
 -r-xr-xr-x  0 0      0        1460 Jan  1  1970 ./js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
diff --git a/js/private/test/image/non_ascii/custom_layer_groups_test_just_the_fs_patch.listing b/js/private/test/image/non_ascii/custom_layer_groups_test_just_the_fs_patch.listing
@@ -9,4 +9,4 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       34729 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0       35271 Jan  1  1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/node-patches/fs.cjs
diff --git a/js/private/test/image/regex_edge_cases_test_node.listing b/js/private/test/image/regex_edge_cases_test_node.listing
@@ -8,7 +8,7 @@ drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/
--r-xr-xr-x  0 0      0       34729 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
+-r-xr-xr-x  0 0      0       35271 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/fs.cjs
 -r-xr-xr-x  0 0      0        1460 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/node-patches/register.cjs
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/
 drwxr-xr-x  0 0      0           0 Jan  1  1970 ./app/js/private/test/image/bin.runfiles/rules_nodejs~~node~nodejs_linux_amd64/bin/
diff --git a/js/private/test/node-patches/readlink.mjs b/js/private/test/node-patches/readlink.mjs
@@ -86,6 +86,44 @@ describe('testing readlink', async () => {
         )
     })
 
+    await it('preserves relative symlink targets in root', async () => {
+        await withFixtures(
+            {
+                a: {},
+                b: { file: 'contents' },
+            },
+            async (fixturesDir) => {
+                fixturesDir = fs.realpathSync(fixturesDir)
+                const linkPath = path.join(fixturesDir, 'a', 'link')
+                const relativeTarget = path.join('..', 'b', 'file')
+
+                fs.symlinkSync(relativeTarget, linkPath)
+
+                const revertPatches = patcher([path.join(fixturesDir)])
+
+                assert.deepStrictEqual(
+                    fs.readlinkSync(linkPath),
+                    relativeTarget,
+                    'SYNC: should preserve relative symlink target'
+                )
+
+                assert.deepStrictEqual(
+                    await util.promisify(fs.readlink)(linkPath),
+                    relativeTarget,
+                    'CB: should preserve relative symlink target'
+                )
+
+                assert.deepStrictEqual(
+                    await fs.promises.readlink(linkPath),
+                    relativeTarget,
+                    'Promise: should preserve relative symlink target'
+                )
+
+                revertPatches()
+            }
+        )
+    })
+
     await it("doesn't resolve as symlink outside of root", async () => {
         await withFixtures(
             {
