Skip to content
147 changes: 147 additions & 0 deletions scopes/toolbox/fs/hard-link-directory/hard-link-directory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,150 @@ describe('hardLinkDirectory()', () => {
expect(fs.readdirSync(dest2Dir)).to.deep.equal([]);
});
});

function findQuarantined(parentDir: string, originalName: string): string | undefined {
const quarantineDir = fs.readdirSync(parentDir).find((entry) => entry.startsWith(`${originalName}.bit-stray-`));
if (!quarantineDir) return undefined;
const quarantined = path.join(parentDir, quarantineDir, originalName);
try {
fs.lstatSync(quarantined);
return quarantined;
} catch {
return undefined;
}
}

it('recover when an ancestor of the destination subdirectory is a regular file', async () => {
const tempDir = globalBitTempDir();
const srcDir = path.join(tempDir, 'source');
const destDir = path.join(tempDir, 'dest');

fs.mkdirpSync(srcDir);
fs.mkdirpSync(path.join(srcDir, '@scope', 'pkg'));
fs.writeFileSync(path.join(srcDir, '@scope/pkg/file.txt'), 'Hello World');

// Simulate a corrupted node_modules layout: '@scope' exists as a regular file
// where a directory is expected. This is the shape of the ENOTDIR mkdir failure
// seen during 'bit install' post-install linking into '.bit_roots'.
fs.mkdirpSync(destDir);
fs.writeFileSync(path.join(destDir, '@scope'), 'stray file');

await hardLinkDirectory(srcDir, [destDir]);

expect(fs.readFileSync(path.join(destDir, '@scope/pkg/file.txt'), 'utf8')).to.equal('Hello World');
// The stray entry must be preserved (renamed, not deleted) so the user can recover it.
const quarantined = findQuarantined(destDir, '@scope');
expect(quarantined).to.not.equal(undefined);
expect(fs.readFileSync(quarantined!, 'utf8')).to.equal('stray file');
});

it('recover when the exact destination subdirectory exists as a regular file', async () => {
const tempDir = globalBitTempDir();
const srcDir = path.join(tempDir, 'source');
const destDir = path.join(tempDir, 'dest');

fs.mkdirpSync(srcDir);
fs.mkdirpSync(path.join(srcDir, 'subdir'));
fs.writeFileSync(path.join(srcDir, 'subdir/file.txt'), 'Hello World');

fs.mkdirpSync(destDir);
fs.writeFileSync(path.join(destDir, 'subdir'), 'stray file');

await hardLinkDirectory(srcDir, [destDir]);

expect(fs.readFileSync(path.join(destDir, 'subdir/file.txt'), 'utf8')).to.equal('Hello World');
const quarantined = findQuarantined(destDir, 'subdir');
expect(quarantined).to.not.equal(undefined);
expect(fs.readFileSync(quarantined!, 'utf8')).to.equal('stray file');
});

it('recover when an ancestor of the destination subdirectory is a dangling symlink', async () => {
const tempDir = globalBitTempDir();
const srcDir = path.join(tempDir, 'source');
const destDir = path.join(tempDir, 'dest');

fs.mkdirpSync(srcDir);
fs.mkdirpSync(path.join(srcDir, '@scope', 'pkg'));
fs.writeFileSync(path.join(srcDir, '@scope/pkg/file.txt'), 'Hello World');

fs.mkdirpSync(destDir);
// Dangling symlink at '@scope' — points to a non-existent target. lstat reports it
// as a symlink (not a directory), so mkdir(@scope/pkg) fails with ENOENT through it.
const offender = path.join(destDir, '@scope');
fs.symlinkSync(path.join(tempDir, 'does-not-exist'), offender, 'junction');
const originalTarget = fs.readlinkSync(offender);

await hardLinkDirectory(srcDir, [destDir]);

expect(fs.readFileSync(path.join(destDir, '@scope/pkg/file.txt'), 'utf8')).to.equal('Hello World');
// The dangling symlink itself must be preserved as a symlink at the quarantined name.
const quarantined = findQuarantined(destDir, '@scope');
expect(quarantined).to.not.equal(undefined);
expect(fs.lstatSync(quarantined!).isSymbolicLink()).to.equal(true);
expect(fs.readlinkSync(quarantined!)).to.equal(originalTarget);
});

it('does not overwrite an existing quarantine file', async () => {
const tempDir = globalBitTempDir();
const srcDir = path.join(tempDir, 'source');
const destDir = path.join(tempDir, 'dest');
const offender = path.join(destDir, 'subdir');
const timestamp = 123456789;
const existingQuarantine = `${offender}.bit-stray-${timestamp}`;

fs.mkdirpSync(path.join(srcDir, 'subdir'));
fs.writeFileSync(path.join(srcDir, 'subdir/file.txt'), 'Hello World');
fs.mkdirpSync(destDir);
fs.writeFileSync(offender, 'new stray file');
fs.writeFileSync(existingQuarantine, 'previous stray file');

const originalDateNow = Date.now;
Date.now = () => timestamp;
try {
await hardLinkDirectory(srcDir, [destDir]);
} finally {
Date.now = originalDateNow;
}

expect(fs.readFileSync(existingQuarantine, 'utf8')).to.equal('previous stray file');
expect(fs.readFileSync(path.join(`${existingQuarantine}-1`, 'subdir'), 'utf8')).to.equal('new stray file');
expect(fs.readFileSync(path.join(destDir, 'subdir/file.txt'), 'utf8')).to.equal('Hello World');
});

it('recovers when concurrent calls encounter the same non-directory entry', async () => {
const tempDir = globalBitTempDir();
const srcDir = path.join(tempDir, 'source');
const destDir = path.join(tempDir, 'dest');
const offender = path.join(destDir, 'subdir');

fs.mkdirpSync(path.join(srcDir, 'subdir'));
fs.writeFileSync(path.join(srcDir, 'subdir/file.txt'), 'Hello World');
fs.mkdirpSync(destDir);
fs.writeFileSync(offender, 'stray file');

const originalRename = fs.rename;
let offenderRenames = 0;
let releaseRenames!: () => void;
const bothRenamesStarted = new Promise<void>((resolve) => {
releaseRenames = resolve;
});
(fs as any).rename = async (src: string, dest: string) => {
if (src !== offender) return originalRename(src, dest);
offenderRenames += 1;
if (offenderRenames === 2) releaseRenames();
await bothRenamesStarted;
return originalRename(src, dest);
};

try {
await Promise.all([hardLinkDirectory(srcDir, [destDir]), hardLinkDirectory(srcDir, [destDir])]);
} finally {
(fs as any).rename = originalRename;
}

expect(offenderRenames).to.equal(2);
expect(fs.readFileSync(path.join(destDir, 'subdir/file.txt'), 'utf8')).to.equal('Hello World');
const quarantineDirs = fs.readdirSync(destDir).filter((entry) => entry.startsWith('subdir.bit-stray-'));
expect(quarantineDirs).to.have.lengthOf(1);
expect(fs.readFileSync(path.join(destDir, quarantineDirs[0], 'subdir'), 'utf8')).to.equal('stray file');
});
143 changes: 126 additions & 17 deletions scopes/toolbox/fs/hard-link-directory/hard-link-directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import path from 'path';
import fs from 'fs-extra';
import symlinkDir from 'symlink-dir';
import resolveLinkTarget from 'resolve-link-target';
import { formatWarningSummary } from '@teambit/cli';
import { getConfig } from '@teambit/config-store';
import { CFG_NO_WARNINGS } from '@teambit/legacy.constants';
import { logger } from '@teambit/legacy.logger';
Comment thread
zkochan marked this conversation as resolved.

/**
* Hard link all files from a directory to several target directories.
Expand All @@ -20,11 +24,7 @@ export async function hardLinkDirectory(src: string, destDirs: string[]) {
const destSubdirs = await Promise.all(
destDirs.map(async (destDir) => {
const destSubdir = path.join(destDir, file.name);
try {
await fs.mkdir(destSubdir, { recursive: true });
} catch (err: any) {
if (err.code !== 'EEXIST') throw err;
}
await ensureDir(destSubdir);
return destSubdir;
})
);
Expand All @@ -36,9 +36,9 @@ export async function hardLinkDirectory(src: string, destDirs: string[]) {
let srcStats: fs.Stats;
try {
srcStats = await fs.stat(srcFile);
} catch (err: any) {
} catch (err) {
// if the link is broken, ignore it
if (err.code === 'ENOENT') return;
if (errnoCode(err) === 'ENOENT') return;
throw err;
}
if (srcStats.isDirectory()) {
Expand All @@ -56,8 +56,8 @@ export async function hardLinkDirectory(src: string, destDirs: string[]) {
const destFile = path.join(destDir, file.name);
try {
await linkFile(srcFile, destFile);
} catch (err: any) {
if (err.code === 'ENOENT') {
} catch (err) {
if (errnoCode(err) === 'ENOENT') {
// broken symlinks are skipped
return;
}
Expand All @@ -72,18 +72,19 @@ export async function hardLinkDirectory(src: string, destDirs: string[]) {
async function linkFile(srcFile: string, destFile: string) {
try {
await fs.link(srcFile, destFile);
} catch (err: any) {
if (err.code === 'ENOENT') {
await fs.mkdir(path.dirname(destFile), { recursive: true });
} catch (err) {
const code = errnoCode(err);
if (code === 'ENOENT' || code === 'ENOTDIR') {
await ensureDir(path.dirname(destFile));
await linkFileIfNotExists(srcFile, destFile);
return;
}
if (err.code === 'EXDEV') {
if (code === 'EXDEV') {
// hard links can't cross devices (e.g. bind mounts or overlayfs on CI), fall back to copying
await fs.copyFile(srcFile, destFile);
return;
}
if (err.code !== 'EEXIST') {
if (code !== 'EEXIST') {
throw err;
}
}
Expand All @@ -92,13 +93,121 @@ async function linkFile(srcFile: string, destFile: string) {
async function linkFileIfNotExists(srcFile: string, destFile: string) {
try {
await fs.link(srcFile, destFile);
} catch (err: any) {
if (err.code === 'EXDEV') {
} catch (err) {
const code = errnoCode(err);
if (code === 'EXDEV') {
await fs.copyFile(srcFile, destFile);
return;
}
if (err.code !== 'EEXIST') {
if (code !== 'EEXIST') {
throw err;
}
}
}

/**
* Like `fs.mkdir(dir, { recursive: true })`, but recovers from a corrupted node_modules
* tree where some ancestor of `dir` exists as a regular file or a non-directory symlink
* (which causes `mkdir` to throw `ENOTDIR` or `ENOENT` through a broken symlink). The
* blocking entry is moved aside (not deleted — the offender could be high up the tree
* and we don't want to discard the user's data) and `mkdir` is retried.
*/
async function ensureDir(dir: string) {
let mkdirError: unknown;
for (let attempt = 0; attempt < 10; attempt++) {
try {
await fs.mkdir(dir, { recursive: true });
return;
} catch (err) {
mkdirError = err;
}

// ENOTDIR: a regular file blocks the path. EEXIST: leaf already exists as a non-directory
// (rare with recursive: true). ENOENT: a dangling symlink in the path can't be traversed.
const code = errnoCode(mkdirError);
if (code !== 'ENOTDIR' && code !== 'EEXIST' && code !== 'ENOENT') throw mkdirError;
const offender = await findNonDirectoryAncestor(dir);
if (offender == null) {
// EEXIST with a directory already at `dir` is benign — recursive mkdir normally
// swallows it, but be defensive against races.
if (code === 'EEXIST') return;
// Another worker may have already moved the offender. Retry mkdir against the new state.
continue;
}
const quarantined = await quarantineStrayEntry(offender);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Recovery moves arbitrary path entries 🐞 Bug ⛨ Security

ensureDir quarantines blockers without checking that they belong to a generated Bit directory,
while injected destinations can be arbitrary absolute paths read from package-manager metadata. A
stale or malformed injected location can therefore cause an unrelated file or symlink outside the
workspace to be moved during compilation.
Agent Prompt
## Issue description
The new recovery path can rename a blocking entry at any absolute destination supplied to `hardLinkDirectory`. Package-manager metadata can supply absolute injected locations without containment validation, allowing unrelated filesystem entries to be moved.

## Issue Context
The compiler preserves absolute injected paths, and Yarn forwards locations from `.yarn-state.yml`. Recovery should only mutate generated directories explicitly owned by Bit.

## Fix Focus Areas
- scopes/toolbox/fs/hard-link-directory/hard-link-directory.ts[115-145]
- scopes/compilation/compiler/compiler.task.ts[64-83]
- scopes/dependencies/yarn/yarn.package-manager.ts[492-510]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

// Another worker already quarantined this entry. Retry mkdir against the new state.
if (quarantined == null) continue;
const msg =
`non-directory entry at ${offender} blocked link target ${dir}; ` +
`moved aside to ${quarantined} so the install could continue. inspect or delete it manually if it isn't expected.`;
logger.warn(msg);
printRecoveryWarning(msg);
}
throw mkdirError;
}

/**
* Reserve a unique sibling directory, then atomically move `offender` into it. The empty,
* exclusively created directory makes the rename no-clobber without recreating files or
* symlinks, so Windows junction types and all other entry metadata remain intact.
* On the rare chance the directory name already exists (e.g. a previous recovery in the
* same millisecond, or a leftover from a prior failed run), keep bumping a counter.
* Returns undefined when another worker already moved the offender.
*/
async function quarantineStrayEntry(offender: string): Promise<string | undefined> {
const base = `${offender}.bit-stray-${Date.now()}`;
let quarantineDir = base;
for (let i = 1; ; i++) {
try {
await fs.mkdir(quarantineDir, { mode: 0o700 });
} catch (err) {
if (errnoCode(err) !== 'EEXIST') throw err;
quarantineDir = `${base}-${i}`;
continue;
}

const quarantined = path.join(quarantineDir, path.basename(offender));
try {
await fs.rename(offender, quarantined);
return quarantined;
Comment on lines +169 to +172

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Quarantine retargets relative symlinks 🐞 Bug ≡ Correctness

Quarantine moves an offending symlink from its original parent into a nested directory, so any
relative link target is subsequently resolved from a different directory. The entry is retained, but
it no longer references the same path and cannot be restored later with its original behavior.
Agent Prompt
## Issue description
Moving a relative symlink into `<offender>.bit-stray-*/<basename>` changes the base directory used to resolve its target. Quarantining should preserve the symlink's effective target as well as its link text and type.

## Issue Context
The existing symlink test uses an absolute target, so it does not detect this behavior. Add coverage for a relative dangling symlink and a relative symlink targeting a non-directory entry.

## Fix Focus Areas
- scopes/toolbox/fs/hard-link-directory/hard-link-directory.ts[157-179]
- scopes/toolbox/fs/hard-link-directory/hard-link-directory.spec.ts[190-214]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

} catch (err) {
// Only remove the directory if it is still empty. Never recursively remove it: after a
// successful rename it contains user data, even if an unusual filesystem reports an error.
await fs.rmdir(quarantineDir).catch(() => undefined);
if (errnoCode(err) === 'ENOENT') return undefined;
throw err;
}
}
}

function printRecoveryWarning(msg: string) {
if (getConfig(CFG_NO_WARNINGS) === 'true' || !logger.shouldWriteToConsole) return;
logger.console(formatWarningSummary(msg), 'warn');
}

/**
* Walk up from `dir` until we find an existing path component. If that component is not
* a directory, return it (it's the entry blocking `mkdir`). Otherwise return null.
*/
async function findNonDirectoryAncestor(dir: string): Promise<string | null> {
let current = dir;
while (current && path.dirname(current) !== current) {
let stat: fs.Stats;
try {
stat = await fs.lstat(current);
} catch (err) {
const code = errnoCode(err);
if (code === 'ENOENT' || code === 'ENOTDIR') {
current = path.dirname(current);
continue;
}
throw err;
}
return stat.isDirectory() ? null : current;
}
return null;
}

function errnoCode(err: unknown): string | undefined {
return (err as NodeJS.ErrnoException | undefined)?.code;
}
Loading