Skip to content

Commit 8614b40

Browse files
claudeclaude
committed
Fix symlink copy failing when source and dest symlinks point to same target
When copying a directory containing symlinks, if the destination already contains symlinks pointing to the same target as the source symlinks, the operation would incorrectly fail with "Cannot copy X to a subdirectory of itself" error. This happened because the isSrcSubdir() check returns true when two paths are identical. However, two different symlinks that happen to point to the same target are distinct filesystem entries that should be copyable. The fix adds a check to ensure the resolved paths are actually different before applying the subdirectory constraints. When both symlinks resolve to the same target (resolvedSrc === resolvedDest), we skip the subdirectory checks and allow the copy to proceed, which correctly replaces the destination symlink with the source symlink. Fixes issue where: - Source: ./src/link → symlink to /tmp/target - Dest: ./dest/link → symlink to /tmp/target - copySync('./src', './dest') would fail Added comprehensive test cases for: - Copy symlink when dest has no existing symlink - Overwrite symlink when dest has symlink pointing to SAME target - Overwrite symlink when dest has symlink pointing to DIFFERENT target - File symlinks with same target - Edge case: copying symlink to itself still fails - Edge case: copying symlink into subdirectory of its target works
1 parent 403e8aa commit 8614b40

File tree

4 files changed

+368
-17
lines changed

4 files changed

+368
-17
lines changed

‎lib/copy/__tests__/copy-symlink-same-target.test.js‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
'use strict'
2+
3+
const os = require('os')
4+
const fs = require('../..')
5+
const path = require('path')
6+
const assert = require('assert')
7+
8+
/* global afterEach, beforeEach, describe, it */
9+
10+
describe('copy() / symlink same target', () => {
11+
const TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-symlink-same-target')
12+
13+
beforeEach(done => {
14+
fs.emptyDir(TEST_DIR, done)
15+
})
16+
17+
afterEach(done => {
18+
fs.remove(TEST_DIR, done)
19+
})
20+
21+
describe('> when copying directory containing symlink', () => {
22+
it('should copy symlink when dest has no existing symlink', done => {
23+
const target = path.join(TEST_DIR, 'target')
24+
const src = path.join(TEST_DIR, 'src')
25+
const dest = path.join(TEST_DIR, 'dest')
26+
27+
// Setup
28+
fs.mkdirpSync(target)
29+
fs.writeFileSync(path.join(target, 'file.txt'), 'content')
30+
fs.mkdirpSync(src)
31+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
32+
33+
// Copy
34+
fs.copy(src, dest, err => {
35+
assert.ifError(err)
36+
37+
// Verify symlink was copied
38+
const destLink = path.join(dest, 'link')
39+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
40+
assert.strictEqual(fs.readlinkSync(destLink), target)
41+
done()
42+
})
43+
})
44+
45+
it('should overwrite symlink when dest has existing symlink pointing to SAME target', done => {
46+
const target = path.join(TEST_DIR, 'target')
47+
const src = path.join(TEST_DIR, 'src')
48+
const dest = path.join(TEST_DIR, 'dest')
49+
50+
// Setup
51+
fs.mkdirpSync(target)
52+
fs.writeFileSync(path.join(target, 'file.txt'), 'content')
53+
fs.mkdirpSync(src)
54+
fs.mkdirpSync(dest)
55+
// Create symlinks pointing to the same target
56+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
57+
fs.symlinkSync(target, path.join(dest, 'link'), 'dir')
58+
59+
// Copy should work - two different symlinks pointing to same target are NOT the same file
60+
fs.copy(src, dest)
61+
.then(() => {
62+
// Verify symlink was copied/overwritten
63+
const destLink = path.join(dest, 'link')
64+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
65+
assert.strictEqual(fs.readlinkSync(destLink), target)
66+
done()
67+
})
68+
.catch(done)
69+
})
70+
71+
it('should overwrite symlink when dest has existing symlink pointing to DIFFERENT target', done => {
72+
const target1 = path.join(TEST_DIR, 'target1')
73+
const target2 = path.join(TEST_DIR, 'target2')
74+
const src = path.join(TEST_DIR, 'src')
75+
const dest = path.join(TEST_DIR, 'dest')
76+
77+
// Setup
78+
fs.mkdirpSync(target1)
79+
fs.mkdirpSync(target2)
80+
fs.writeFileSync(path.join(target1, 'file.txt'), 'content1')
81+
fs.writeFileSync(path.join(target2, 'file.txt'), 'content2')
82+
fs.mkdirpSync(src)
83+
fs.mkdirpSync(dest)
84+
// Create symlinks pointing to different targets
85+
fs.symlinkSync(target1, path.join(src, 'link'), 'dir')
86+
fs.symlinkSync(target2, path.join(dest, 'link'), 'dir')
87+
88+
// Copy should work
89+
fs.copy(src, dest, err => {
90+
assert.ifError(err)
91+
92+
// Verify symlink was copied/overwritten to point to target1
93+
const destLink = path.join(dest, 'link')
94+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
95+
assert.strictEqual(fs.readlinkSync(destLink), target1)
96+
done()
97+
})
98+
})
99+
})
100+
101+
describe('> when copying file symlinks', () => {
102+
it('should overwrite file symlink when dest has existing symlink pointing to SAME target', done => {
103+
const target = path.join(TEST_DIR, 'target.txt')
104+
const src = path.join(TEST_DIR, 'src')
105+
const dest = path.join(TEST_DIR, 'dest')
106+
107+
// Setup
108+
fs.writeFileSync(target, 'content')
109+
fs.mkdirpSync(src)
110+
fs.mkdirpSync(dest)
111+
// Create file symlinks pointing to the same target
112+
fs.symlinkSync(target, path.join(src, 'link'), 'file')
113+
fs.symlinkSync(target, path.join(dest, 'link'), 'file')
114+
115+
// Copy should work
116+
fs.copy(src, dest)
117+
.then(() => {
118+
// Verify symlink was copied/overwritten
119+
const destLink = path.join(dest, 'link')
120+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
121+
assert.strictEqual(fs.readlinkSync(destLink), target)
122+
done()
123+
})
124+
.catch(done)
125+
})
126+
})
127+
128+
describe('> edge cases', () => {
129+
it('should still prevent copying a symlink to itself', done => {
130+
const target = path.join(TEST_DIR, 'target')
131+
const link = path.join(TEST_DIR, 'link')
132+
133+
// Setup
134+
fs.mkdirpSync(target)
135+
fs.symlinkSync(target, link, 'dir')
136+
137+
// Copying a symlink to itself should fail
138+
fs.copy(link, link)
139+
.then(() => {
140+
done(new Error('Expected error to be thrown'))
141+
})
142+
.catch(err => {
143+
assert.strictEqual(err.message, 'Source and destination must not be the same.')
144+
done()
145+
})
146+
})
147+
148+
it('should allow copying symlink to subdirectory of its target (symlink copy, not recursive)', done => {
149+
// When copying a symlink (not dereferencing), we just copy the link itself
150+
// This should be allowed because we're not recursively copying the target's contents
151+
const target = path.join(TEST_DIR, 'target')
152+
const src = path.join(TEST_DIR, 'src')
153+
154+
// Setup
155+
fs.mkdirpSync(target)
156+
fs.mkdirpSync(path.join(target, 'subdir'))
157+
fs.mkdirpSync(src)
158+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
159+
160+
// Dest is inside src's resolved target
161+
const dest = path.join(target, 'subdir', 'dest')
162+
163+
// This should work - we're just copying the symlink, not following it
164+
fs.copy(src, dest)
165+
.then(() => {
166+
// Verify symlink was copied
167+
const destLink = path.join(dest, 'link')
168+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
169+
assert.strictEqual(fs.readlinkSync(destLink), target)
170+
done()
171+
})
172+
.catch(done)
173+
})
174+
})
175+
})

‎lib/copy/__tests__/copy-sync-symlink-same-target.test.js‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
'use strict'
2+
3+
const os = require('os')
4+
const fs = require('../..')
5+
const path = require('path')
6+
const assert = require('assert')
7+
const copySync = require('../copy-sync')
8+
9+
/* global afterEach, beforeEach, describe, it */
10+
11+
describe('copy-sync / symlink same target', () => {
12+
const TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-symlink-same-target')
13+
14+
beforeEach(done => {
15+
fs.emptyDir(TEST_DIR, done)
16+
})
17+
18+
afterEach(done => {
19+
fs.remove(TEST_DIR, done)
20+
})
21+
22+
describe('> when copying directory containing symlink', () => {
23+
it('should copy symlink when dest has no existing symlink', () => {
24+
const target = path.join(TEST_DIR, 'target')
25+
const src = path.join(TEST_DIR, 'src')
26+
const dest = path.join(TEST_DIR, 'dest')
27+
28+
// Setup
29+
fs.mkdirpSync(target)
30+
fs.writeFileSync(path.join(target, 'file.txt'), 'content')
31+
fs.mkdirpSync(src)
32+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
33+
34+
// Copy
35+
assert.doesNotThrow(() => {
36+
copySync(src, dest)
37+
})
38+
39+
// Verify symlink was copied
40+
const destLink = path.join(dest, 'link')
41+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
42+
assert.strictEqual(fs.readlinkSync(destLink), target)
43+
})
44+
45+
it('should overwrite symlink when dest has existing symlink pointing to SAME target', () => {
46+
const target = path.join(TEST_DIR, 'target')
47+
const src = path.join(TEST_DIR, 'src')
48+
const dest = path.join(TEST_DIR, 'dest')
49+
50+
// Setup
51+
fs.mkdirpSync(target)
52+
fs.writeFileSync(path.join(target, 'file.txt'), 'content')
53+
fs.mkdirpSync(src)
54+
fs.mkdirpSync(dest)
55+
// Create symlinks pointing to the same target
56+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
57+
fs.symlinkSync(target, path.join(dest, 'link'), 'dir')
58+
59+
// Copy should work - two different symlinks pointing to same target are NOT the same file
60+
assert.doesNotThrow(() => {
61+
copySync(src, dest)
62+
})
63+
64+
// Verify symlink was copied/overwritten
65+
const destLink = path.join(dest, 'link')
66+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
67+
assert.strictEqual(fs.readlinkSync(destLink), target)
68+
})
69+
70+
it('should overwrite symlink when dest has existing symlink pointing to DIFFERENT target', () => {
71+
const target1 = path.join(TEST_DIR, 'target1')
72+
const target2 = path.join(TEST_DIR, 'target2')
73+
const src = path.join(TEST_DIR, 'src')
74+
const dest = path.join(TEST_DIR, 'dest')
75+
76+
// Setup
77+
fs.mkdirpSync(target1)
78+
fs.mkdirpSync(target2)
79+
fs.writeFileSync(path.join(target1, 'file.txt'), 'content1')
80+
fs.writeFileSync(path.join(target2, 'file.txt'), 'content2')
81+
fs.mkdirpSync(src)
82+
fs.mkdirpSync(dest)
83+
// Create symlinks pointing to different targets
84+
fs.symlinkSync(target1, path.join(src, 'link'), 'dir')
85+
fs.symlinkSync(target2, path.join(dest, 'link'), 'dir')
86+
87+
// Copy should work
88+
assert.doesNotThrow(() => {
89+
copySync(src, dest)
90+
})
91+
92+
// Verify symlink was copied/overwritten to point to target1
93+
const destLink = path.join(dest, 'link')
94+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
95+
assert.strictEqual(fs.readlinkSync(destLink), target1)
96+
})
97+
})
98+
99+
describe('> when copying file symlinks', () => {
100+
it('should overwrite file symlink when dest has existing symlink pointing to SAME target', () => {
101+
const target = path.join(TEST_DIR, 'target.txt')
102+
const src = path.join(TEST_DIR, 'src')
103+
const dest = path.join(TEST_DIR, 'dest')
104+
105+
// Setup
106+
fs.writeFileSync(target, 'content')
107+
fs.mkdirpSync(src)
108+
fs.mkdirpSync(dest)
109+
// Create file symlinks pointing to the same target
110+
fs.symlinkSync(target, path.join(src, 'link'), 'file')
111+
fs.symlinkSync(target, path.join(dest, 'link'), 'file')
112+
113+
// Copy should work
114+
assert.doesNotThrow(() => {
115+
copySync(src, dest)
116+
})
117+
118+
// Verify symlink was copied/overwritten
119+
const destLink = path.join(dest, 'link')
120+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
121+
assert.strictEqual(fs.readlinkSync(destLink), target)
122+
})
123+
})
124+
125+
describe('> edge cases', () => {
126+
it('should still prevent copying a symlink to itself', () => {
127+
const target = path.join(TEST_DIR, 'target')
128+
const link = path.join(TEST_DIR, 'link')
129+
130+
// Setup
131+
fs.mkdirpSync(target)
132+
fs.symlinkSync(target, link, 'dir')
133+
134+
// Copying a symlink to itself should fail
135+
assert.throws(() => {
136+
copySync(link, link)
137+
}, /Source and destination must not be the same/)
138+
})
139+
140+
it('should allow copying symlink to subdirectory of its target (symlink copy, not recursive)', () => {
141+
// When copying a symlink (not dereferencing), we just copy the link itself
142+
// This should be allowed because we're not recursively copying the target's contents
143+
const target = path.join(TEST_DIR, 'target')
144+
const src = path.join(TEST_DIR, 'src')
145+
146+
// Setup
147+
fs.mkdirpSync(target)
148+
fs.mkdirpSync(path.join(target, 'subdir'))
149+
fs.mkdirpSync(src)
150+
fs.symlinkSync(target, path.join(src, 'link'), 'dir')
151+
152+
// Dest is inside src's resolved target
153+
const dest = path.join(target, 'subdir', 'dest')
154+
155+
// This should work - we're just copying the symlink, not following it
156+
assert.doesNotThrow(() => {
157+
copySync(src, dest)
158+
})
159+
160+
// Verify symlink was copied
161+
const destLink = path.join(dest, 'link')
162+
assert.ok(fs.lstatSync(destLink).isSymbolicLink())
163+
assert.strictEqual(fs.readlinkSync(destLink), target)
164+
})
165+
})
166+
})

‎lib/copy/copy-sync.js‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,20 @@ function onLink (destStat, src, dest, opts) {
149149
if (opts.dereference) {
150150
resolvedDest = path.resolve(process.cwd(), resolvedDest)
151151
}
152-
if (stat.isSrcSubdir(resolvedSrc, resolvedDest)) {
153-
throw new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`)
154-
}
155-
156-
// prevent copy if src is a subdir of dest since unlinking
157-
// dest in this case would result in removing src contents
158-
// and therefore a broken symlink would be created.
159-
if (stat.isSrcSubdir(resolvedDest, resolvedSrc)) {
160-
throw new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`)
152+
// If both symlinks resolve to the same target, they are still distinct symlinks
153+
// that can be copied/overwritten. Only check subdirectory constraints when
154+
// the resolved paths are different.
155+
if (resolvedSrc !== resolvedDest) {
156+
if (stat.isSrcSubdir(resolvedSrc, resolvedDest)) {
157+
throw new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`)
158+
}
159+
160+
// prevent copy if src is a subdir of dest since unlinking
161+
// dest in this case would result in removing src contents
162+
// and therefore a broken symlink would be created.
163+
if (stat.isSrcSubdir(resolvedDest, resolvedSrc)) {
164+
throw new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`)
165+
}
161166
}
162167
return copyLink(resolvedSrc, dest)
163168
}

0 commit comments

Comments
 (0)