fix: make script template creation reliable

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: StellaHuang95

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
 

files/templates/copilot-instructions-text/script-copilot-instructions.md

@@ -1,7 +1,7 @@
 # Copilot Instructions for <script_name>
 
--   The script `<script_name>` 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 `<script_name>` 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 `<script_name>` path to get the Python executable details.
 -   Substitute the Python executable you get from the `Get Python Environment Information` tool anywhere you see `<python>` in these instructions.

files/templates/new723ScriptTemplate/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.

src/features/creators/newScriptProject.ts

@@ -1,33 +1,127 @@
 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 { normalizePath } from '../../common/utils/pathUtils';
+import { isWindows } from '../../common/utils/platformUtils';
+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$/, '');
+    const deviceBaseName = baseName.split('.')[0];
+    if (isWindows() && /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(deviceBaseName)) {
+        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 isSameOrDescendantPath(parentPath: string, candidatePath: string): boolean {
+    const relativePath = path.relative(
+        normalizePath(path.resolve(parentPath)),
+        normalizePath(path.resolve(candidatePath)),
+    );
+    return (
+        relativePath === '' ||
+        (relativePath !== '..' && !relativePath.startsWith(`..${path.sep}`) && !path.isAbsolute(relativePath))
+    );
+}
+
+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),
+    });
+}
+
+async function lstatIfExists(candidatePath: string) {
+    try {
+        return await fs.lstat(candidatePath);
+    } catch (error) {
+        if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
+            return undefined;
+        }
+        throw error;
+    }
+}
+
+async function isCopilotInstructionsDestinationContained(
+    destinationRoot: string,
+    physicalDestinationRoot: string,
+    physicalWorkspaceRoot: string,
+): Promise<boolean> {
+    try {
+        const githubFolder = path.join(destinationRoot, '.github');
+        const githubEntry = await lstatIfExists(githubFolder);
+        if (!githubEntry) {
+            return isSameOrDescendantPath(physicalWorkspaceRoot, physicalDestinationRoot);
+        }
+
+        const physicalGithubFolder = await fs.realpath(githubFolder);
+        if (!isSameOrDescendantPath(physicalWorkspaceRoot, physicalGithubFolder)) {
+            return false;
+        }
+        if (!(await fs.stat(githubFolder)).isDirectory()) {
+            return false;
+        }
+
+        const instructionsFile = path.join(githubFolder, 'copilot-instructions.md');
+        const instructionsEntry = await lstatIfExists(instructionsFile);
+        if (!instructionsEntry) {
+            return true;
+        }
+        if (instructionsEntry.isSymbolicLink() || !instructionsEntry.isFile()) {
+            return false;
+        }
+
+        const physicalInstructionsFile = await fs.realpath(instructionsFile);
+        return isSameOrDescendantPath(physicalWorkspaceRoot, physicalInstructionsFile);
+    } catch (error) {
+        traceError('Failed to validate the Copilot instructions destination:', error);
+        return false;
+    }
+}
+
 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<PythonProject | Uri | undefined> {
-        // 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) {
             // If quickCreate is true, we should not prompt for any input
             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 +131,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 +145,152 @@ 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, '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}`);
+            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) => isSameOrDescendantPath(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;
+        }
+
+        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 (!isSameOrDescendantPath(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);
             }
+            window.showErrorMessage(containmentError);
+            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;
+        if (
+            createCopilotInstructions &&
+            !(await isCopilotInstructionsDestinationContained(
+                resolvedDestRoot,
+                physicalDestRoot,
+                physicalWorkspaceRoot,
+            ))
+        ) {
+            showErrorMessage(
+                l10n.t('Copilot instructions must be stored inside the open workspace, aborting creation.'),
+            );
+            return undefined;
+        }
+
+        // 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;
+        }
+        await fs.copy(newScriptTemplateFile, scriptDestination);
+
+        // 2. Replace 'script_name' in the file using a helper (just script name remove .py)
+        await replaceInFilesAndNames(scriptDestination, 'script_name', scriptFileName.replace(/\.py$/, ''));
+
+        // Add the created script to the project manager
+        const createdScript: PythonProject = {
+            name: scriptFileName,
+            uri: identityRootUri.with({
+                path: path.posix.join(identityRootUri.path, scriptFileName),
+            }),
+        };
+        try {
+            await this.projectManager.add(createdScript);
+        } catch (registrationError) {
+            try {
+                this.projectManager.remove(createdScript);
+            } catch (rollbackError) {
+                traceError('Failed to remove the new script project after registration failed:', rollbackError);
             }
-            await fs.copy(newScriptTemplateFile, scriptDestination);
-
-            // 2. Replace 'script_name' in the file using a helper (just script name remove .py)
-            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: '<script_name>', replaceValue: scriptFileName },
-                ]);
+            try {
+                await fs.remove(scriptDestination);
+            } catch (rollbackError) {
+                traceError('Failed to delete the new script after registration failed:', rollbackError);
             }
+            throw registrationError;
+        }
 
-            // 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: '<script_name>', replaceValue: scriptFileName },
+            ]);
+        }
 
-            await showTextDocument(createdScript.uri);
+        await showTextDocument(createdScript.uri);
 
-            return createdScript;
-        }
-        return undefined;
+        return createdScript;
     }
 }