Skip to content

Commit ba6c287

Browse files
committed
fix(cp): address bugbot review comments
- Add WebSocket readyState check before ws.send() to prevent uncaught exceptions when connection closes unexpectedly during file streaming - Add circular symlink protection using inode-based cycle detection - Skip directory symlinks to prevent infinite recursion - Follow file symlinks (copy target content) for consistent behavior
1 parent 38c9dbd commit ba6c287

File tree

1 file changed

+64
-12
lines changed

1 file changed

+64
-12
lines changed

src/lib/cp.ts

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,52 @@ function buildWsURL(baseURL: string, instanceId: string): string {
151151
* ```
152152
*/
153153
export async function cpToInstance(cfg: CpConfig, opts: CpToInstanceOptions): Promise<void> {
154-
// Get file stats
155-
const stats = fs.statSync(opts.srcPath);
156-
const isDir = stats.isDirectory();
154+
// Use internal implementation with cycle detection
155+
const visited = new Set<string>();
156+
await cpToInstanceInternal(cfg, opts, visited);
157+
}
158+
159+
/**
160+
* Internal recursive implementation with cycle detection.
161+
*/
162+
async function cpToInstanceInternal(
163+
cfg: CpConfig,
164+
opts: CpToInstanceOptions,
165+
visited: Set<string>,
166+
): Promise<void> {
167+
// Use lstatSync to not follow symlinks - this lets us detect symlinks
168+
const lstats = fs.lstatSync(opts.srcPath);
169+
170+
if (lstats.isSymbolicLink()) {
171+
// For symlinks, check what they point to
172+
try {
173+
const targetStats = fs.statSync(opts.srcPath);
174+
if (targetStats.isDirectory()) {
175+
// Skip directory symlinks to prevent potential cycles
176+
// The directory could point to an ancestor, causing infinite recursion
177+
return;
178+
}
179+
// For file symlinks, copy the target file contents
180+
await cpSingleFileToInstance(cfg, {
181+
...opts,
182+
isDir: false,
183+
});
184+
} catch {
185+
// Broken symlink - skip it
186+
return;
187+
}
188+
return;
189+
}
190+
191+
if (lstats.isDirectory()) {
192+
// Check for cycles using device + inode
193+
const dirId = `${lstats.dev}:${lstats.ino}`;
194+
if (visited.has(dirId)) {
195+
// Already visited this directory - skip to prevent infinite recursion
196+
return;
197+
}
198+
visited.add(dirId);
157199

158-
if (isDir) {
159200
// For directories, first create the directory, then recursively copy contents
160201
await cpSingleFileToInstance(cfg, {
161202
...opts,
@@ -169,12 +210,16 @@ export async function cpToInstance(cfg: CpConfig, opts: CpToInstanceOptions): Pr
169210
// Use path.posix.join for guest paths to ensure forward slashes
170211
const dstEntryPath = path.posix.join(opts.dstPath, entry.name);
171212

172-
await cpToInstance(cfg, {
173-
instanceId: opts.instanceId,
174-
srcPath: srcEntryPath,
175-
dstPath: dstEntryPath,
176-
...(opts.mode !== undefined && { mode: opts.mode }),
177-
});
213+
await cpToInstanceInternal(
214+
cfg,
215+
{
216+
instanceId: opts.instanceId,
217+
srcPath: srcEntryPath,
218+
dstPath: dstEntryPath,
219+
...(opts.mode !== undefined && { mode: opts.mode }),
220+
},
221+
visited,
222+
);
178223
}
179224
} else {
180225
// For files, copy directly
@@ -251,11 +296,18 @@ async function cpSingleFileToInstance(
251296
});
252297

253298
fileStream.on('data', (chunk: any) => {
254-
ws.send(chunk as Buffer);
299+
// Check WebSocket is still open before sending
300+
// Queued events can fire after close is initiated
301+
if (ws.readyState === ws.OPEN) {
302+
ws.send(chunk as Buffer);
303+
}
255304
});
256305

257306
fileStream.on('end', () => {
258-
ws.send(JSON.stringify({ type: 'end' }));
307+
// Check WebSocket is still open before sending
308+
if (ws.readyState === ws.OPEN) {
309+
ws.send(JSON.stringify({ type: 'end' }));
310+
}
259311
});
260312

261313
fileStream.on('error', (err) => {

0 commit comments

Comments
 (0)