diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index 9f64f773d..21bfcd971 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -75,9 +75,9 @@ Use this to scaffold a brand new Python project with the correct structure and f - **Package**: A structured Python package with `pyproject.toml`, tests folder, and package directory - **Script**: A simple standalone Python file using PEP 723 inline metadata 4. Enter a name for your project. -5. Choose whether to create a virtual environment. +5. If you're creating a package, choose whether to create a virtual environment. -The extension creates the project structure, adds it to your workspace, and optionally creates a virtual environment. +The extension creates the project structure and adds it to your workspace. For packages, it can also create a virtual environment. #### Package template structure diff --git a/files/templates/copilot-instructions-text/script-copilot-instructions.md b/files/templates/copilot-instructions-text/script-copilot-instructions.md index 01bd826a6..130afb400 100644 --- a/files/templates/copilot-instructions-text/script-copilot-instructions.md +++ b/files/templates/copilot-instructions-text/script-copilot-instructions.md @@ -1,7 +1,7 @@ # Copilot Instructions for -- The script `` is a Python python project within the workspace. -- It has inline script metadata (as proposed by PEP 723) that defines the script name, required python version, and dependencies. +- The script `` is a Python project within the workspace. +- It has inline script metadata (as proposed by PEP 723) that defines the required Python version and dependencies. - If imports which require a specific Python version or dependencies are added, keep the inline script metadata up to date. - You need to call the `Get Python Environment Information` tool on the `` path to get the Python executable details. - Substitute the Python executable you get from the `Get Python Environment Information` tool anywhere you see `` in these instructions. diff --git a/files/templates/new723ScriptTemplate/script.py b/files/templates/newInlineScriptTemplate/script.py similarity index 51% rename from files/templates/new723ScriptTemplate/script.py rename to files/templates/newInlineScriptTemplate/script.py index 7511eea1f..74abfb6e3 100644 --- a/files/templates/new723ScriptTemplate/script.py +++ b/files/templates/newInlineScriptTemplate/script.py @@ -1,8 +1,7 @@ # /// script -# requires-python = ">=X.XX" TODO: Update this to the minimum Python version you want to support -# dependencies = [ -# TODO: Add any dependencies your script requires -# ] +# requires-python = ">=3.9" +# dependencies = [] +# # Add dependency requirement strings to the array above as needed. # /// # TODO: Update the main function to your needs or remove it. diff --git a/src/common/utils/pathUtils.ts b/src/common/utils/pathUtils.ts index d398828a1..df796e5f7 100644 --- a/src/common/utils/pathUtils.ts +++ b/src/common/utils/pathUtils.ts @@ -65,6 +65,44 @@ export function normalizePath(fsPath: string): string { return path1; } +/** + * Determines whether `candidatePath` is the same as, or nested inside, `parentPath`. + * + * Both paths are resolved to absolute form and normalized (case-insensitive on + * Windows) before comparison, so differences in separators or drive-letter case + * do not affect the result. + * + * @param parentPath The candidate parent (or ancestor) directory. + * @param candidatePath The path being tested for containment. + * @returns `true` when `candidatePath` equals `parentPath` or is a descendant of it. + */ +export function isSameOrParentPath(parentPath: string, candidatePath: string): boolean { + const relative = path.relative( + normalizePath(path.resolve(parentPath)), + normalizePath(path.resolve(candidatePath)), + ); + return ( + relative === '' || + (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)) + ); +} + +/** + * Determines whether `value` maps to a reserved Windows device name (e.g. `CON`, + * `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`). + * + * The check inspects the portion of the name before the first dot, since Windows + * disallows these names regardless of extension, and only reports `true` on + * Windows, where the restriction applies. + * + * @param value The base file name (without directory) to test. + * @returns `true` on Windows when `value` resolves to a reserved device name. + */ +export function isWindowsReservedDeviceName(value: string): boolean { + const deviceBaseName = value.split('.')[0]; + return isWindows() && /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(deviceBaseName); +} + export function getResourceUri(resourcePath: string, root?: string): Uri | undefined { try { if (!resourcePath) { diff --git a/src/features/creators/newScriptProject.ts b/src/features/creators/newScriptProject.ts index 5a62b6cb4..93196145a 100644 --- a/src/features/creators/newScriptProject.ts +++ b/src/features/creators/newScriptProject.ts @@ -1,26 +1,56 @@ import * as fs from 'fs-extra'; import * as path from 'path'; -import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, workspace } from 'vscode'; +import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, WorkspaceFolder } from 'vscode'; import { PythonProject, PythonProjectCreator, PythonProjectCreatorOptions } from '../../api'; import { NEW_PROJECT_TEMPLATES_FOLDER } from '../../common/constants'; import { traceError } from '../../common/logging'; -import { showInputBoxWithButtons, showTextDocument } from '../../common/window.apis'; +import { isSameOrParentPath, isWindowsReservedDeviceName } from '../../common/utils/pathUtils'; +import { showErrorMessage, showInputBoxWithButtons, showTextDocument } from '../../common/window.apis'; +import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/workspace.apis'; import { PythonProjectManager } from '../../internal.api'; import { isCopilotInstalled, manageCopilotInstructionsFile, replaceInFilesAndNames } from './creationHelpers'; +function validateScriptFileName(value: string): string | null { + const pathSegments = value.split(/[\\/]/); + if (pathSegments.length !== 1 || pathSegments.includes('..')) { + return l10n.t('Script name must be a file name without path separators or traversal.'); + } + if (!value.endsWith('.py')) { + return l10n.t('Script name must end with ".py".'); + } + const baseName = value.replace(/\.py$/, ''); + if (isWindowsReservedDeviceName(baseName)) { + return l10n.t('Script name uses a reserved Windows device name.'); + } + // following PyPI (PEP 508) rules for package names + if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) { + return l10n.t( + 'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.', + ); + } + if (/^[-._0-9]$/i.test(baseName)) { + return l10n.t('Single-character script names cannot be a number, hyphen, or period.'); + } + return null; +} + +function uriForFileRootInWorkspace(rootPath: string, workspaceFolder: WorkspaceFolder): Uri { + const relativeRootPath = path.relative(path.resolve(workspaceFolder.uri.fsPath), path.resolve(rootPath)); + const pathSegments = relativeRootPath.split(/[\\/]/).filter((segment) => segment.length > 0); + return workspaceFolder.uri.with({ + path: path.posix.join(workspaceFolder.uri.path, ...pathSegments), + }); +} + export class NewScriptProject implements PythonProjectCreator { public readonly name = l10n.t('newScript'); public readonly displayName = l10n.t('Script'); - public readonly description = l10n.t('Creates a new script folder in your current workspace'); + public readonly description = l10n.t('Creates a new script in your current workspace'); public readonly tooltip = new MarkdownString(l10n.t('Create a new Python script')); constructor(private readonly projectManager: PythonProjectManager) {} async create(options?: PythonProjectCreatorOptions): Promise { - // quick create (needs name, will always create venv and copilot instructions) - // not quick create - // ask for script file name - // ask if they want venv let scriptFileName = options?.name; let createCopilotInstructions: boolean | undefined; if (options?.quickCreate === true) { @@ -28,6 +58,9 @@ export class NewScriptProject implements PythonProjectCreator { if (!scriptFileName) { throw new Error('Script file name is required in quickCreate mode.'); } + if (path.extname(scriptFileName) === '') { + scriptFileName = `${scriptFileName}.py`; + } createCopilotInstructions = true; } else { //Prompt as quickCreate is false @@ -37,23 +70,7 @@ export class NewScriptProject implements PythonProjectCreator { prompt: l10n.t('What is the name of the script? (e.g. my_script.py)'), ignoreFocusOut: true, showBackButton: true, - validateInput: (value) => { - // Ensure the filename ends with .py and follows valid naming conventions - if (!value.endsWith('.py')) { - return l10n.t('Script name must end with ".py".'); - } - const baseName = value.replace(/\.py$/, ''); - // following PyPI (PEP 508) rules for package names - if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) { - return l10n.t( - 'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.', - ); - } - if (/^[-._0-9]$/i.test(baseName)) { - return l10n.t('Single-character script names cannot be a number, hyphen, or period.'); - } - return null; - }, + validateInput: validateScriptFileName, }); } catch (ex) { if (ex === QuickInputButtons.Back) { @@ -67,64 +84,143 @@ export class NewScriptProject implements PythonProjectCreator { createCopilotInstructions = true; } } + } + const validationError = validateScriptFileName(scriptFileName); + if (validationError) { + if (options?.quickCreate === true) { + throw new Error(validationError); + } + window.showErrorMessage(validationError); + return undefined; + } - // 1. Copy template folder - const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'new723ScriptTemplate', 'script.py'); - if (!(await fs.pathExists(newScriptTemplateFile))) { - window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.')); - traceError(`Template file not found at: ${newScriptTemplateFile}`); + // 1. Copy template file + const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'newInlineScriptTemplate', 'script.py'); + if (!(await fs.pathExists(newScriptTemplateFile))) { + window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.')); + traceError(`Template file not found at: ${newScriptTemplateFile}`); + return undefined; + } + + // Check if the destination folder is provided, otherwise use the first workspace folder. + let destinationRootUri = options?.rootUri; + let workspaceFolders: readonly WorkspaceFolder[] | undefined; + if (!destinationRootUri) { + workspaceFolders = getWorkspaceFolders(); + if (!workspaceFolders || workspaceFolders.length === 0) { + window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.')); return undefined; } + destinationRootUri = workspaceFolders[0].uri; + } - // Check if the destination folder is provided, otherwise use the first workspace folder - let destRoot = options?.rootUri.fsPath; - if (!destRoot) { - const workspaceFolders = workspace.workspaceFolders; - if (!workspaceFolders || workspaceFolders.length === 0) { - window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.')); - return undefined; - } - destRoot = workspaceFolders[0].uri.fsPath; - } + const destRoot = destinationRootUri.fsPath; + const resolvedDestRoot = path.resolve(destRoot); + let workspaceFolder = getWorkspaceFolder(destinationRootUri); + if (!workspaceFolder && destinationRootUri.scheme === 'file') { + workspaceFolders ??= getWorkspaceFolders(); + workspaceFolder = workspaceFolders + ?.filter((folder) => isSameOrParentPath(folder.uri.fsPath, resolvedDestRoot)) + .sort((first, second) => second.uri.fsPath.length - first.uri.fsPath.length)[0]; + } + if (!workspaceFolder) { + showErrorMessage(l10n.t('Destination folder must be inside an open workspace, aborting creation.')); + return undefined; + } - // Check if the destination folder already exists - const scriptDestination = path.join(destRoot, scriptFileName); - if (await fs.pathExists(scriptDestination)) { - window.showErrorMessage( - l10n.t( - 'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.', - ), - ); - return undefined; + let physicalDestRoot: string; + let physicalWorkspaceRoot: string; + try { + [physicalDestRoot, physicalWorkspaceRoot] = await Promise.all([ + fs.realpath(resolvedDestRoot), + fs.realpath(workspaceFolder.uri.fsPath), + ]); + } catch (error) { + traceError('Failed to resolve the destination or workspace folder:', error); + showErrorMessage(l10n.t('Unable to resolve the destination folder inside the open workspace.')); + return undefined; + } + if (!isSameOrParentPath(physicalWorkspaceRoot, physicalDestRoot)) { + showErrorMessage(l10n.t('Destination folder must resolve inside the open workspace, aborting creation.')); + return undefined; + } + + const identityRootUri = + destinationRootUri.scheme === 'file' && workspaceFolder.uri.scheme !== 'file' + ? uriForFileRootInWorkspace(resolvedDestRoot, workspaceFolder) + : destinationRootUri; + const scriptDestination = path.resolve(resolvedDestRoot, scriptFileName); + const relativeScriptPath = path.relative(resolvedDestRoot, scriptDestination); + if ( + relativeScriptPath === '' || + relativeScriptPath === '..' || + relativeScriptPath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativeScriptPath) + ) { + const containmentError = l10n.t('Script name must resolve to a file inside the destination folder.'); + if (options?.quickCreate === true) { + throw new Error(containmentError); } - await fs.copy(newScriptTemplateFile, scriptDestination); + window.showErrorMessage(containmentError); + return undefined; + } - // 2. Replace 'script_name' in the file using a helper (just script name remove .py) + // Check if the destination file already exists + if (await fs.pathExists(scriptDestination)) { + window.showErrorMessage( + l10n.t( + 'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.', + ), + ); + return undefined; + } + // Build the project entry up front so copying the template, substituting + // the script name, and registering the project share one cleanup boundary: + // if any step fails, the partially created script is removed so a retry + // starts from a clean state. + const createdScript: PythonProject = { + name: scriptFileName, + uri: identityRootUri.with({ + path: path.posix.join(identityRootUri.path, scriptFileName), + }), + }; + let projectRegistrationAttempted = false; + try { + await fs.copy(newScriptTemplateFile, scriptDestination); + // Replace 'script_name' in the file (script name without the .py suffix). await replaceInFilesAndNames(scriptDestination, 'script_name', scriptFileName.replace(/\.py$/, '')); - - // 3. add custom github copilot instructions - if (createCopilotInstructions) { - const packageInstructionsPath = path.join( - NEW_PROJECT_TEMPLATES_FOLDER, - 'copilot-instructions-text', - 'script-copilot-instructions.md', - ); - await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [ - { searchValue: '', replaceValue: scriptFileName }, - ]); + projectRegistrationAttempted = true; + await this.projectManager.add(createdScript); + } catch (creationError) { + if (projectRegistrationAttempted) { + try { + this.projectManager.remove(createdScript); + } catch (rollbackError) { + traceError('Failed to remove the new script project after creation failed:', rollbackError); + } } + try { + await fs.remove(scriptDestination); + } catch (rollbackError) { + traceError('Failed to delete the new script after creation failed:', rollbackError); + } + throw creationError; + } - // Add the created script to the project manager - const createdScript: PythonProject | undefined = { - name: scriptFileName, - uri: Uri.file(scriptDestination), - }; - this.projectManager.add(createdScript); + // 3. add custom github copilot instructions + if (createCopilotInstructions) { + const packageInstructionsPath = path.join( + NEW_PROJECT_TEMPLATES_FOLDER, + 'copilot-instructions-text', + 'script-copilot-instructions.md', + ); + await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [ + { searchValue: '', replaceValue: scriptFileName }, + ]); + } - await showTextDocument(createdScript.uri); + await showTextDocument(createdScript.uri); - return createdScript; - } - return undefined; + return createdScript; } } diff --git a/src/features/terminal/utils.ts b/src/features/terminal/utils.ts index f4320d13c..866a74861 100644 --- a/src/features/terminal/utils.ts +++ b/src/features/terminal/utils.ts @@ -11,6 +11,7 @@ import { VENV_MANAGER_ID } from '../../common/constants'; import { traceError, traceVerbose } from '../../common/logging'; import { timeout } from '../../common/utils/asyncUtils'; import { createSimpleDebounce } from '../../common/utils/debounce'; +import { isSameOrParentPath } from '../../common/utils/pathUtils'; import { onDidChangeTerminalShellIntegration, onDidWriteTerminalData } from '../../common/window.apis'; import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis'; import { identifyTerminalShell } from '../common/shellDetector'; @@ -199,11 +200,6 @@ async function getDistinctProjectEnvs( return envs; } -function isSameOrParentPath(parentPath: string, candidatePath: string): boolean { - const relative = path.relative(path.resolve(parentPath), path.resolve(candidatePath)); - return relative === '' || (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)); -} - function getProjectForCwd(projects: readonly PythonProject[], cwd: string): PythonProject | undefined { return [...projects] .filter((project) => isSameOrParentPath(project.uri.fsPath, cwd)) diff --git a/src/test/common/pathUtils.unit.test.ts b/src/test/common/pathUtils.unit.test.ts index 1733e789f..c9bdd6150 100644 --- a/src/test/common/pathUtils.unit.test.ts +++ b/src/test/common/pathUtils.unit.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; -import { getResourceUri, normalizePath } from '../../common/utils/pathUtils'; +import { getResourceUri, isWindowsReservedDeviceName, normalizePath } from '../../common/utils/pathUtils'; import * as utils from '../../common/utils/platformUtils'; suite('Path Utilities', () => { @@ -128,4 +128,40 @@ suite('Path Utilities', () => { assert.strictEqual(result, 'C:/Path/To/File.txt'); }); }); + + suite('isWindowsReservedDeviceName', () => { + let isWindowsStub: sinon.SinonStub; + + setup(() => { + isWindowsStub = sinon.stub(utils, 'isWindows'); + }); + + teardown(() => { + sinon.restore(); + }); + + test('flags reserved device names on Windows regardless of extension', () => { + isWindowsStub.returns(true); + + for (const name of ['CON', 'prn', 'aux', 'nul', 'com1', 'LPT9', 'con.py', 'nul.foo']) { + assert.strictEqual(isWindowsReservedDeviceName(name), true, `${name} should be reserved`); + } + }); + + test('allows names that only resemble reserved device names on Windows', () => { + isWindowsStub.returns(true); + + for (const name of ['console', 'com10', 'lpt0', 'printer', 'aux_helper']) { + assert.strictEqual(isWindowsReservedDeviceName(name), false, `${name} should be allowed`); + } + }); + + test('never flags reserved device names off Windows', () => { + isWindowsStub.returns(false); + + for (const name of ['CON', 'nul', 'com1', 'lpt9']) { + assert.strictEqual(isWindowsReservedDeviceName(name), false, `${name} should be allowed off Windows`); + } + }); + }); }); diff --git a/src/test/features/creators/newScriptProject.unit.test.ts b/src/test/features/creators/newScriptProject.unit.test.ts index 897544f35..02f7ebb9e 100644 --- a/src/test/features/creators/newScriptProject.unit.test.ts +++ b/src/test/features/creators/newScriptProject.unit.test.ts @@ -1,7 +1,19 @@ import assert from 'assert'; +import fsExtra from 'fs-extra'; import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; +import * as sinon from 'sinon'; +import { TextEditor, Uri, WorkspaceFolder } from 'vscode'; +import { PythonProject } from '../../../api'; +import { NEW_PROJECT_TEMPLATES_FOLDER } from '../../../common/constants'; +import { readInlineScriptMetadata } from '../../../common/inlineScript/metadata'; +import * as platformUtils from '../../../common/utils/platformUtils'; +import * as windowApis from '../../../common/window.apis'; +import * as workspaceApis from '../../../common/workspace.apis'; +import * as creationHelpers from '../../../features/creators/creationHelpers'; +import { NewScriptProject } from '../../../features/creators/newScriptProject'; +import { PythonProjectManager } from '../../../internal.api'; // Path to the real script template, resolved from the compiled test location // (out/test/features/creators/ → workspaceRoot/files/templates/...). We do NOT @@ -16,29 +28,580 @@ const TEMPLATE_PATH = path.join( '..', 'files', 'templates', - 'new723ScriptTemplate', + 'newInlineScriptTemplate', 'script.py', ); -suite('new723ScriptTemplate / NewScriptProject', () => { +function asRemoteUri(fsPath: string, authority = 'ssh-remote+test-host'): Uri { + return Uri.from({ + scheme: 'vscode-remote', + authority, + path: Uri.file(fsPath).path, + }); +} + +suite('newInlineScriptTemplate / NewScriptProject', () => { let tmpDir: string; + let getWorkspaceFolderStub: sinon.SinonStub; + let getWorkspaceFoldersStub: sinon.SinonStub; + let workspaceFolder: WorkspaceFolder; setup(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'new-script-test-')); + workspaceFolder = { + index: 0, + name: 'test-workspace', + uri: Uri.file(tmpDir), + }; + getWorkspaceFolderStub = sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(workspaceFolder); + getWorkspaceFoldersStub = sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); }); teardown(async () => { + sinon.restore(); await fs.remove(tmpDir); }); - test('Template file starts with a valid PEP 723 header (# /// script)', async () => { + function stubSuccessfulFileCreation() { + const templateFile = path.resolve( + path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'newInlineScriptTemplate', 'script.py'), + ); + const showTextDocumentStub = sinon + .stub(windowApis, 'showTextDocument') + .resolves({} as TextEditor); + // Resolve existence by the requested path (the template exists, the new + // script does not) so the fixture does not depend on probe call order. + sinon.stub(fsExtra, 'pathExists').callsFake(async (checkedPath) => { + return path.resolve(String(checkedPath)) === templateFile; + }); + const copyStub = sinon.stub(fsExtra, 'copy').callsFake(async (_source, destination) => { + await fs.copyFile(TEMPLATE_PATH, destination); + }); + const replaceStub = sinon.stub(creationHelpers, 'replaceInFilesAndNames').resolves(); + const instructionsStub = sinon.stub(creationHelpers, 'manageCopilotInstructionsFile').resolves(); + return { copyStub, instructionsStub, replaceStub, showTextDocumentStub }; + } + + async function createDirectoryLink(target: string, link: string): Promise { + try { + await fs.symlink(target, link, platformUtils.isWindows() ? 'junction' : 'dir'); + return true; + } catch { + await fs.ensureDir(link); + return false; + } + } + + test('shipped template contains parseable PEP 723 metadata', async () => { const contents = await fs.readFile(TEMPLATE_PATH, 'utf8'); - const firstNonBlankLine = contents.split(/\r?\n/).find((l) => l.trim().length > 0); + const metadata = readInlineScriptMetadata(contents); + + assert.ok(metadata, 'Template must contain valid PEP 723 metadata'); + assert.strictEqual(metadata.range.start, 0, 'PEP 723 metadata should be at the start of the template'); + assert.strictEqual(metadata.requiresPython, '>=3.9'); + assert.notStrictEqual(metadata.dependencies, undefined, 'Template should declare dependencies'); + assert.deepStrictEqual(metadata.dependencies, []); + assert.strictEqual(metadata.tool, undefined); + }); + + test('interactive filename validation rejects unsafe names and accepts a valid name', async () => { + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons').callsFake(async (options) => { + const validateInput = options?.validateInput; + assert.ok(validateInput); + assert.strictEqual(typeof (await validateInput('../escape.py')), 'string'); + assert.strictEqual(typeof (await validateInput('nested/script.py')), 'string'); + assert.strictEqual(typeof (await validateInput('script')), 'string'); + assert.strictEqual(typeof (await validateInput('invalid name.py')), 'string'); + assert.strictEqual(await validateInput('valid_script.py'), null); + return undefined; + }); + + const result = await creator.create(); + + assert.strictEqual(result, undefined); + assert.ok(promptStub.calledOnce); + assert.strictEqual(addStub.called, false); + }); + + test('Windows filename validation rejects reserved device names and accepts similar names', async () => { + sinon.stub(platformUtils, 'isWindows').returns(true); + const creator = new NewScriptProject({ add: sinon.stub().resolves() } as unknown as PythonProjectManager); + sinon.stub(windowApis, 'showInputBoxWithButtons').callsFake(async (options) => { + const validateInput = options?.validateInput; + assert.ok(validateInput); + const reservedNames = [ + 'CON.py', + 'prn.py', + 'Aux.data.py', + 'nul.foo.py', + ...Array.from({ length: 9 }, (_, index) => `cOm${index + 1}.py`), + ...Array.from({ length: 9 }, (_, index) => `LpT${index + 1}.extra.py`), + ]; + for (const reservedName of reservedNames) { + assert.strictEqual( + typeof (await validateInput(reservedName)), + 'string', + `${reservedName} should be rejected on Windows`, + ); + } + for (const validName of ['console.py', 'com10.py', 'lpt10.py']) { + assert.strictEqual( + await validateInput(validName), + null, + `${validName} should remain valid on Windows`, + ); + } + return undefined; + }); + + assert.strictEqual(await creator.create(), undefined); + await assert.rejects( + creator.create({ + name: 'cOn', + quickCreate: true, + rootUri: Uri.file(tmpDir), + }), + /reserved Windows device name/, + ); + }); + + test('non-Windows filename validation allows Windows device names', async () => { + sinon.stub(platformUtils, 'isWindows').returns(false); + const creator = new NewScriptProject({ add: sinon.stub().resolves() } as unknown as PythonProjectManager); + sinon.stub(windowApis, 'showInputBoxWithButtons').callsFake(async (options) => { + const validateInput = options?.validateInput; + assert.ok(validateInput); + assert.strictEqual(await validateInput('CON.py'), null); + assert.strictEqual(await validateInput('con.foo.py'), null); + return undefined; + }); + + assert.strictEqual(await creator.create(), undefined); + }); + + test('quick create rejects a traversal name without prompting', async () => { + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const showTextDocumentStub = sinon.stub(windowApis, 'showTextDocument'); + + await assert.rejects( + creator.create({ + name: '../escape.py', + quickCreate: true, + rootUri: Uri.file(tmpDir), + }), + /path separators or traversal/, + ); + assert.strictEqual(promptStub.called, false); + assert.strictEqual(addStub.called, false); + assert.strictEqual(showTextDocumentStub.called, false); + }); + + test('quick create appends .py to a safe extensionless base name', async () => { + const rootUri = Uri.file(tmpDir); + const scriptDestination = path.resolve(rootUri.fsPath, 'hello_world.py'); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const { showTextDocumentStub } = stubSuccessfulFileCreation(); + + const result = await creator.create({ + name: 'hello_world', + quickCreate: true, + rootUri, + }); + + assert.ok(result); + const createdScript = result as PythonProject; + assert.strictEqual(createdScript.name, 'hello_world.py'); + assert.strictEqual(createdScript.uri.fsPath, Uri.file(scriptDestination).fsPath); + assert.strictEqual(promptStub.called, false); + assert.ok(addStub.calledOnceWithExactly(createdScript)); + assert.ok(showTextDocumentStub.calledOnceWithExactly(createdScript.uri)); + }); + + test('quick create rejects a non-Python extension without prompting', async () => { + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const showTextDocumentStub = sinon.stub(windowApis, 'showTextDocument'); + + await assert.rejects( + creator.create({ + name: 'hello_world.txt', + quickCreate: true, + rootUri: Uri.file(tmpDir), + }), + /must end with ".py"/, + ); + + assert.strictEqual(promptStub.called, false); + assert.strictEqual(addStub.called, false); + assert.strictEqual(showTextDocumentStub.called, false); + }); + + test('quick create accepts a valid name inside an open workspace without prompting', async () => { + const scriptFileName = 'quick_script.py'; + const rootUri = Uri.file(tmpDir); + const scriptDestination = path.resolve(rootUri.fsPath, scriptFileName); + const expectedTemplatePath = path.join( + NEW_PROJECT_TEMPLATES_FOLDER, + 'newInlineScriptTemplate', + 'script.py', + ); + const addStub = sinon.stub().resolves(); + const projectManager = { add: addStub } as unknown as PythonProjectManager; + const creator = new NewScriptProject(projectManager); + + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const { copyStub, instructionsStub, replaceStub, showTextDocumentStub } = stubSuccessfulFileCreation(); + + const result = await creator.create({ + name: scriptFileName, + quickCreate: true, + rootUri, + }); + + assert.ok(result); + const createdScript = result as PythonProject; + assert.strictEqual(createdScript.name, scriptFileName); + assert.strictEqual(createdScript.uri.fsPath, Uri.file(scriptDestination).fsPath); + assert.strictEqual(await fs.readFile(scriptDestination, 'utf8'), await fs.readFile(TEMPLATE_PATH, 'utf8')); + assert.strictEqual(promptStub.called, false, 'quick create must not prompt for a script name'); + assert.ok(getWorkspaceFolderStub.calledOnce); + assert.strictEqual( + getWorkspaceFolderStub.firstCall.args[0].fsPath, + Uri.file(path.resolve(rootUri.fsPath)).fsPath, + ); + assert.ok(copyStub.calledOnce); + assert.strictEqual(copyStub.firstCall.args[0], expectedTemplatePath); + assert.strictEqual(copyStub.firstCall.args[1], scriptDestination); + assert.ok( + replaceStub.calledOnceWithExactly(scriptDestination, 'script_name', 'quick_script'), + 'template substitution should run', + ); + assert.ok(addStub.calledOnceWithExactly(createdScript), 'created script should be added as a project'); + assert.ok( + showTextDocumentStub.calledOnceWithExactly(createdScript.uri), + 'created script should be opened in the editor', + ); + assert.ok( + instructionsStub.calledOnceWithExactly( + rootUri.fsPath, + path.join( + NEW_PROJECT_TEMPLATES_FOLDER, + 'copilot-instructions-text', + 'script-copilot-instructions.md', + ), + [{ searchValue: '', replaceValue: scriptFileName }], + ), + 'quick create should retain Copilot-instruction handling', + ); + }); + + test('interactive fallback preserves a remote workspace URI', async () => { + const remoteWorkspaceUri = asRemoteUri(tmpDir); + workspaceFolder = { + index: 0, + name: 'remote-workspace', + uri: remoteWorkspaceUri, + }; + getWorkspaceFoldersStub.returns([workspaceFolder]); + getWorkspaceFolderStub.callsFake((uri: Uri) => + uri.toString() === remoteWorkspaceUri.toString() ? workspaceFolder : undefined, + ); + sinon.stub(windowApis, 'showInputBoxWithButtons').resolves('remote_script.py'); + const { showTextDocumentStub } = stubSuccessfulFileCreation(); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const result = await creator.create(); + + assert.ok(result); + const createdScript = result as PythonProject; + assert.strictEqual(getWorkspaceFolderStub.firstCall.args[0].toString(), remoteWorkspaceUri.toString()); + assert.strictEqual(createdScript.uri.scheme, remoteWorkspaceUri.scheme); + assert.strictEqual(createdScript.uri.authority, remoteWorkspaceUri.authority); + assert.strictEqual(createdScript.uri.fsPath, Uri.file(path.join(tmpDir, 'remote_script.py')).fsPath); + assert.ok(addStub.calledOnceWithExactly(createdScript)); + assert.ok(showTextDocumentStub.calledOnceWithExactly(createdScript.uri)); + }); + + test('quick file root matches by fsPath and retains the remote workspace URI identity', async () => { + const nestedRoot = path.join(tmpDir, 'nested'); + await fs.ensureDir(nestedRoot); + const remoteWorkspaceUri = asRemoteUri(tmpDir, 'dev-container+test'); + workspaceFolder = { + index: 0, + name: 'remote-workspace', + uri: remoteWorkspaceUri, + }; + getWorkspaceFolderStub.returns(undefined); + getWorkspaceFoldersStub.returns([workspaceFolder]); + const rootUri = Uri.file(nestedRoot); + const { showTextDocumentStub } = stubSuccessfulFileCreation(); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const result = await creator.create({ + name: 'remote_quick', + quickCreate: true, + rootUri, + }); + + assert.ok(result); + const createdScript = result as PythonProject; + assert.strictEqual(getWorkspaceFolderStub.firstCall.args[0].toString(), rootUri.toString()); + assert.strictEqual(createdScript.uri.scheme, remoteWorkspaceUri.scheme); + assert.strictEqual(createdScript.uri.authority, remoteWorkspaceUri.authority); + assert.strictEqual(createdScript.uri.fsPath, Uri.file(path.join(nestedRoot, 'remote_quick.py')).fsPath); + assert.ok(addStub.calledOnceWithExactly(createdScript)); + assert.ok(showTextDocumentStub.calledOnceWithExactly(createdScript.uri)); + }); + + test('quick and programmatic roots outside the workspace are rejected before file creation', async () => { + const outsideRoot = Uri.file(path.join(tmpDir, 'outside')); + getWorkspaceFolderStub.returns(undefined); + getWorkspaceFoldersStub.returns([]); + sinon.stub(fsExtra, 'pathExists').resolves(true); + const copyStub = sinon.stub(fsExtra, 'copy').resolves(); + const replaceStub = sinon.stub(creationHelpers, 'replaceInFilesAndNames').resolves(); + const instructionsStub = sinon.stub(creationHelpers, 'manageCopilotInstructionsFile').resolves(); + const showTextDocumentStub = sinon.stub(windowApis, 'showTextDocument'); + const showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage'); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const quickResult = await creator.create({ + name: 'quick_outside.py', + quickCreate: true, + rootUri: outsideRoot, + }); + const programmaticResult = await creator.create({ + name: 'programmatic_outside.py', + rootUri: outsideRoot, + }); + + assert.strictEqual(quickResult, undefined); + assert.strictEqual(programmaticResult, undefined); + assert.strictEqual(getWorkspaceFolderStub.callCount, 2); + assert.strictEqual(showErrorMessageStub.callCount, 2); + assert.strictEqual(copyStub.called, false); + assert.strictEqual(replaceStub.called, false); + assert.strictEqual(addStub.called, false); + assert.strictEqual(instructionsStub.called, false); + assert.strictEqual(showTextDocumentStub.called, false); + }); + + test('a symlinked workspace root remains a valid destination', async () => { + const physicalWorkspaceRoot = path.join(tmpDir, 'physical-workspace'); + const linkedWorkspaceRoot = path.join(tmpDir, 'linked-workspace'); + const nestedRoot = path.join(linkedWorkspaceRoot, 'nested'); + await fs.ensureDir(path.join(physicalWorkspaceRoot, 'nested')); + await createDirectoryLink(physicalWorkspaceRoot, linkedWorkspaceRoot); + await fs.ensureDir(nestedRoot); + workspaceFolder = { + index: 0, + name: 'linked-workspace', + uri: Uri.file(linkedWorkspaceRoot), + }; + getWorkspaceFolderStub.returns(workspaceFolder); + getWorkspaceFoldersStub.returns([workspaceFolder]); + const { copyStub, showTextDocumentStub } = stubSuccessfulFileCreation(); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const result = await creator.create({ + name: 'linked_workspace.py', + quickCreate: true, + rootUri: Uri.file(nestedRoot), + }); + + assert.ok(result); + const createdScript = result as PythonProject; + assert.strictEqual(createdScript.uri.fsPath, Uri.file(path.join(nestedRoot, 'linked_workspace.py')).fsPath); + assert.ok(copyStub.calledOnce); + assert.ok(addStub.calledOnceWithExactly(createdScript)); + assert.ok(showTextDocumentStub.calledOnceWithExactly(createdScript.uri)); + }); + + test('a destination that physically escapes the workspace is rejected before writing', async () => { + const workspaceRoot = path.join(tmpDir, 'workspace'); + const outsideRoot = path.join(tmpDir, 'outside'); + const linkedDestination = path.join(workspaceRoot, 'linked-outside'); + await fs.ensureDir(workspaceRoot); + await fs.ensureDir(outsideRoot); + + // Simulate a destination that is lexically inside the workspace but whose + // real path (via a symlink/junction) resolves outside of it. + const realpathStub = sinon.stub(fsExtra, 'realpath') as sinon.SinonStub; + realpathStub.callsFake(async (targetPath: string) => { + const resolved = path.resolve(String(targetPath)); + if (path.relative(resolved, path.resolve(linkedDestination)) === '') { + return path.resolve(outsideRoot); + } + if (path.relative(resolved, path.resolve(workspaceRoot)) === '') { + return path.resolve(workspaceRoot); + } + throw new Error(`Unexpected realpath: ${targetPath}`); + }); + + workspaceFolder = { + index: 0, + name: 'test-workspace', + uri: Uri.file(workspaceRoot), + }; + getWorkspaceFolderStub.returns(workspaceFolder); + getWorkspaceFoldersStub.returns([workspaceFolder]); + sinon.stub(fsExtra, 'pathExists').resolves(true); + const copyStub = sinon.stub(fsExtra, 'copy').resolves(); + const replaceStub = sinon.stub(creationHelpers, 'replaceInFilesAndNames').resolves(); + const instructionsStub = sinon.stub(creationHelpers, 'manageCopilotInstructionsFile').resolves(); + const showTextDocumentStub = sinon.stub(windowApis, 'showTextDocument'); + const showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage'); + const addStub = sinon.stub().resolves(); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const result = await creator.create({ + name: 'escaped.py', + quickCreate: true, + rootUri: Uri.file(linkedDestination), + }); + + assert.strictEqual(result, undefined); + assert.ok(showErrorMessageStub.calledOnce); + assert.strictEqual(copyStub.called, false); + assert.strictEqual(replaceStub.called, false); + assert.strictEqual(addStub.called, false); + assert.strictEqual(instructionsStub.called, false); + assert.strictEqual(showTextDocumentStub.called, false); + }); + + test('create waits for project registration before opening and returning', async () => { + const scriptFileName = 'wait_for_registration.py'; + const rootUri = Uri.file(tmpDir); + const { showTextDocumentStub } = stubSuccessfulFileCreation(); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + let notifyAddStarted!: () => void; + let releaseRegistration!: () => void; + const addStarted = new Promise((resolve) => { + notifyAddStarted = resolve; + }); + const registration = new Promise((resolve) => { + releaseRegistration = resolve; + }); + const addStub = sinon.stub().callsFake(async () => { + notifyAddStarted(); + await registration; + }); + const creator = new NewScriptProject({ add: addStub } as unknown as PythonProjectManager); + + const createPromise = creator.create({ + name: scriptFileName, + quickCreate: true, + rootUri, + }); + let createSettled = false; + const trackedCreate = createPromise.finally(() => { + createSettled = true; + }); + await addStarted; + await Promise.resolve(); + + assert.strictEqual(createSettled, false, 'create should remain pending while project registration is pending'); + assert.strictEqual(showTextDocumentStub.called, false, 'the script must not open before registration completes'); + assert.strictEqual(promptStub.called, false); + + releaseRegistration(); + const result = await trackedCreate; + + assert.ok(result); + assert.ok(addStub.calledOnce); + assert.ok(showTextDocumentStub.calledOnce); + }); + + test('an insert-then-reject registration rolls back creator side effects and preserves the error', async () => { + const rootUri = Uri.file(tmpDir); + const scriptDestination = path.resolve(rootUri.fsPath, 'registration_failure.py'); + const { instructionsStub, showTextDocumentStub } = stubSuccessfulFileCreation(); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const projects: PythonProject[] = []; + const registrationError = new Error('registration failed after insertion'); + const addStub = sinon.stub().callsFake(async (project: PythonProject) => { + projects.push(project); + throw registrationError; + }); + const removeStub = sinon.stub().callsFake((project: PythonProject) => { + const index = projects.indexOf(project); + if (index >= 0) { + projects.splice(index, 1); + } + }); + const creator = new NewScriptProject({ + add: addStub, + remove: removeStub, + } as unknown as PythonProjectManager); + + await assert.rejects( + creator.create({ + name: 'registration_failure.py', + quickCreate: true, + rootUri, + }), + (error: unknown) => error === registrationError, + ); + + assert.ok(addStub.calledOnce); + assert.ok(removeStub.calledOnceWithExactly(addStub.firstCall.args[0])); + assert.deepStrictEqual(projects, []); + await assert.rejects(fs.readFile(scriptDestination), (error: NodeJS.ErrnoException) => error.code === 'ENOENT'); + assert.strictEqual(showTextDocumentStub.called, false); + assert.strictEqual(instructionsStub.called, false); + assert.strictEqual(promptStub.called, false); + }); + + test('a substitution failure after copy removes the partially created script', async () => { + const rootUri = Uri.file(tmpDir); + const scriptDestination = path.resolve(rootUri.fsPath, 'substitution_failure.py'); + const { copyStub, instructionsStub, replaceStub, showTextDocumentStub } = stubSuccessfulFileCreation(); + const substitutionError = new Error('template substitution failed'); + replaceStub.rejects(substitutionError); + const promptStub = sinon.stub(windowApis, 'showInputBoxWithButtons'); + const addStub = sinon.stub().resolves(); + const removeStub = sinon.stub(); + const creator = new NewScriptProject({ + add: addStub, + remove: removeStub, + } as unknown as PythonProjectManager); + + await assert.rejects( + creator.create({ + name: 'substitution_failure.py', + quickCreate: true, + rootUri, + }), + (error: unknown) => error === substitutionError, + ); + + assert.ok(copyStub.calledOnce, 'the template must be copied before substitution runs'); + assert.ok(replaceStub.calledOnce); + await assert.rejects( + fs.readFile(scriptDestination), + (error: NodeJS.ErrnoException) => error.code === 'ENOENT', + 'the copied script must be removed when substitution fails', + ); + assert.strictEqual(addStub.called, false, 'registration must not run when substitution fails'); assert.strictEqual( - firstNonBlankLine, - '# /// script', - 'Template must start with a valid PEP 723 `script` header line', + removeStub.called, + false, + 'project rollback must not run when registration was never attempted', ); + assert.strictEqual(instructionsStub.called, false); + assert.strictEqual(showTextDocumentStub.called, false); + assert.strictEqual(promptStub.called, false); }); });