Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/common/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ export namespace VenvManagerStrings {

export const venvRemoving = l10n.t('Removing virtual environment');
export const venvRemoveFailed = l10n.t('Failed to remove virtual environment');
export const venvRemoveInvalidPath = l10n.t(
'Cannot remove: path does not appear to be a valid virtual environment',
);
export const venvRemoveUnsafePath = l10n.t('Cannot remove: path appears to be a system or root directory');

export const installEditable = l10n.t('Install project as editable');
export const searchingDependencies = l10n.t('Searching for dependencies');
Expand Down
57 changes: 54 additions & 3 deletions src/managers/builtin/venvUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,12 +422,63 @@ export async function createPythonVenv(
return createStepBasedVenvFlow(nativeFinder, api, log, manager, basePythons, venvRoot, options);
}

function isDriveRoot(fsPath: string): boolean {
const normalized = path.normalize(fsPath);
if (os.platform() === 'win32') {
return /^[a-zA-Z]:[\\/]?$/.test(normalized);
}
return normalized === '/';
}

function hasMinimumPathDepth(fsPath: string, minDepth: number = 2): boolean {
const normalized = path.normalize(fsPath);
const parts = normalized.split(path.sep).filter((p) => p.length > 0 && p !== '.');

if (os.platform() === 'win32') {
return parts.length >= minDepth;
}
return parts.length >= minDepth;
}

async function isValidVenvPath(fsPath: string): Promise<boolean> {
try {
const pyvenvCfgPath = path.join(fsPath, 'pyvenv.cfg');
return await fsapi.pathExists(pyvenvCfgPath);
} catch {
return false;
}
}

async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel): Promise<string | undefined> {
if (isDriveRoot(envPath)) {
log.error(`Refusing to remove drive root: ${envPath}`);
return VenvManagerStrings.venvRemoveUnsafePath;
}

if (!hasMinimumPathDepth(envPath, 2)) {
log.error(`Refusing to remove path with insufficient depth: ${envPath}`);
return VenvManagerStrings.venvRemoveUnsafePath;
}

if (!(await isValidVenvPath(envPath))) {
log.error(`Path does not appear to be a valid venv (no pyvenv.cfg): ${envPath}`);
return VenvManagerStrings.venvRemoveInvalidPath;
}

return undefined;
}

export async function removeVenv(environment: PythonEnvironment, log: LogOutputChannel): Promise<boolean> {
const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';

const envPath = environment.environmentPath.fsPath.endsWith(pythonPath)
? path.dirname(path.dirname(environment.environmentPath.fsPath))
: environment.environmentPath.fsPath;
const envFsPath = path.normalize(environment.environmentPath.fsPath);
const envPath = envFsPath.endsWith(pythonPath) ? path.dirname(path.dirname(envFsPath)) : envFsPath;

const validationError = await validateVenvRemovalPath(envPath, log);
if (validationError) {
showErrorMessage(validationError);
return false;
}

// Normalize path for UI display - ensure forward slashes on Windows
const displayPath = normalizePath(envPath);
Expand Down
149 changes: 149 additions & 0 deletions src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import * as assert from 'assert';
import * as os from 'os';
import * as path from 'path';

suite('venvUtils Path Validation', () => {
suite('isDriveRoot behavior', () => {
test('should identify Windows drive roots correctly', function () {
if (os.platform() !== 'win32') {
this.skip();
return;
}

const driveRoots = ['C:\\', 'D:\\', 'c:\\', 'C:/'];

for (const root of driveRoots) {
const normalized = path.normalize(root);
const isDrive = /^[a-zA-Z]:[\\/]?$/.test(normalized);
assert.strictEqual(
isDrive,
true,
`${root} (normalized: ${normalized}) should be identified as drive root`,
);
}
});

test('should not identify non-root Windows paths as drive roots', function () {
if (os.platform() !== 'win32') {
this.skip();
return;
}

const nonRoots = ['C:\\Users', 'C:\\Program Files', 'D:\\python\\venv', 'C:\\Users\\test\\.venv'];

for (const nonRoot of nonRoots) {
const normalized = path.normalize(nonRoot);
const isDrive = /^[a-zA-Z]:[\\/]?$/.test(normalized);
assert.strictEqual(isDrive, false, `${nonRoot} should not be identified as drive root`);
}
});

test('should identify Unix root correctly', function () {
if (os.platform() === 'win32') {
this.skip();
return;
}

const normalized = path.normalize('/');
assert.strictEqual(normalized, '/', 'Unix root should be /');
});
});

suite('hasMinimumPathDepth behavior', () => {
test('should correctly count path components on Windows', function () {
if (os.platform() !== 'win32') {
this.skip();
return;
}

const testCases: [string, number][] = [
['C:\\', 1],
['C:\\Users', 2],
['C:\\Users\\test', 3],
['C:\\Users\\test\\.venv', 4],
];

for (const [testPath, expectedDepth] of testCases) {
const normalized = path.normalize(testPath);
const parts = normalized.split(path.sep).filter((p) => p.length > 0 && p !== '.');
assert.strictEqual(parts.length, expectedDepth, `${testPath} should have ${expectedDepth} components`);
}
});

test('should correctly count path components on Unix', function () {
if (os.platform() === 'win32') {
this.skip();
return;
}

const testCases: [string, number][] = [
['/', 0],
['/home', 1],
['/home/user', 2],
['/home/user/.venv', 3],
];

for (const [testPath, expectedDepth] of testCases) {
const normalized = path.normalize(testPath);
const parts = normalized.split(path.sep).filter((p) => p.length > 0 && p !== '.');
assert.strictEqual(parts.length, expectedDepth, `${testPath} should have ${expectedDepth} components`);
}
});
});

suite('Path normalization in removeVenv', () => {
test('should normalize path separators before checking python.exe suffix', () => {
const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';

const mixedPath =
os.platform() === 'win32' ? 'C:/Users/test/.venv/Scripts/python.exe' : '/home/user/.venv/bin/python';

const normalizedPath = path.normalize(mixedPath);
const endsWithPython = normalizedPath.endsWith(pythonPath);

assert.strictEqual(endsWithPython, true, 'Normalized path should end with python executable');

const envPath = path.dirname(path.dirname(normalizedPath));
const expectedEnvPath =
os.platform() === 'win32' ? path.normalize('C:/Users/test/.venv') : '/home/user/.venv';

assert.strictEqual(envPath, expectedEnvPath, 'Environment path should be the venv root');
});

test('should correctly derive venv path from python executable path', () => {
const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python';

const testPaths =
os.platform() === 'win32'
? [
{ input: 'C:\\project\\.venv\\Scripts\\python.exe', expected: 'C:\\project\\.venv' },
{ input: 'D:\\envs\\myenv\\Scripts\\python.exe', expected: 'D:\\envs\\myenv' },
]
: [
{ input: '/home/user/project/.venv/bin/python', expected: '/home/user/project/.venv' },
{ input: '/opt/envs/myenv/bin/python', expected: '/opt/envs/myenv' },
];

for (const { input, expected } of testPaths) {
const normalized = path.normalize(input);
const envPath = normalized.endsWith(pythonPath) ? path.dirname(path.dirname(normalized)) : normalized;

assert.strictEqual(envPath, expected, `${input} should derive to ${expected}`);
}
});
});
});

suite('venvUtils removeVenv validation integration', () => {
test('pyvenv.cfg detection should use correct path', async () => {
const testEnvPath = os.platform() === 'win32' ? 'C:\\Users\\test\\.venv' : '/home/user/.venv';

const expectedCfgPath = path.join(testEnvPath, 'pyvenv.cfg');

assert.strictEqual(
expectedCfgPath,
os.platform() === 'win32' ? 'C:\\Users\\test\\.venv\\pyvenv.cfg' : '/home/user/.venv/pyvenv.cfg',
'Should check for pyvenv.cfg in the environment root',
);
});
});