Skip to content

Commit 9e44ce1

Browse files
Make standalone script template creation reliable (#1736)
## Context The existing Script project template is presented as a PEP 723 standalone script, but its placeholder metadata is not valid TOML. The quick-create path also returns without creating a file, even though external callers provide a script project name and destination. This change fixes the existing user-facing template and file-creation workflow. It does not create, select, or expose an inline-script environment. ## Why this change is needed - The shipped metadata block cannot be parsed by the extension's own PEP 723 parser. - Quick create skips the copy/register/open path. - External quick callers provide a base project name such as `hello_world`, not necessarily a complete `.py` filename. - Filename/path handling needs to reject traversal, reserved Windows device names, remote-workspace identity loss, and physical symlink escapes. - Project-registration failure can otherwise leave a generated file or in-memory ghost project. - Copilot instruction paths can be redirected outside the workspace through existing symlinks or junctions. ## What changed - The template now contains valid `requires-python = ">=3.9"` metadata and an empty dependency list. - Quick base names are normalized to `.py`; interactive creation continues requiring an explicit `.py` filename. - Interactive and quick flows share validation for extension, characters, separators, traversal, containment, and Windows reserved names. - Workspace matching preserves remote URI scheme and authority. - Destination and Copilot-instruction paths are physically checked against the containing workspace before any side effect. - Quick and interactive creation share the file copy, substitution, project registration, instruction, and open flow. - Project registration is awaited. Insert-then-fail behavior removes the in-memory project and copied script while preserving the original error. - Out-of-workspace destinations are rejected because they cannot be durably registered. - Related documentation and Copilot instructions now describe the actual script behavior and metadata fields. ## Behavior and compatibility - Package project creation is unchanged. - Script creation still does not provision an environment. - The hidden inline manager remains undeclared, default-off, and unregistered. - Users already saw a PEP 723 block in generated scripts; this change makes that existing block valid. - Opening the now-valid generated file may emit the existing anonymized telemetry-only detection event. It does not create or select an environment. ## Reviewer guide 1. Review shared filename normalization and validation. 2. Review workspace URI matching and physical containment preflight. 3. Review side-effect ordering and registration rollback. 4. Review the actual template parser test and quick-create tests. ## Validation - `npm run compile-tests --silent` - Targeted metadata parser and script creator suites: 71 passing - ESLint on changed TypeScript files - `git diff --check` --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1487b95c-ac14-455f-9b7f-9770cf65e11e
1 parent 3d63516 commit 9e44ce1

8 files changed

Lines changed: 820 additions & 92 deletions

File tree

docs/managing-python-projects.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ Use this to scaffold a brand new Python project with the correct structure and f
7575
- **Package**: A structured Python package with `pyproject.toml`, tests folder, and package directory
7676
- **Script**: A simple standalone Python file using PEP 723 inline metadata
7777
4. Enter a name for your project.
78-
5. Choose whether to create a virtual environment.
78+
5. If you're creating a package, choose whether to create a virtual environment.
7979

80-
The extension creates the project structure, adds it to your workspace, and optionally creates a virtual environment.
80+
The extension creates the project structure and adds it to your workspace. For packages, it can also create a virtual environment.
8181

8282
#### Package template structure
8383

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Copilot Instructions for <script_name>
22

3-
- The script `<script_name>` is a Python python project within the workspace.
4-
- It has inline script metadata (as proposed by PEP 723) that defines the script name, required python version, and dependencies.
3+
- The script `<script_name>` is a Python project within the workspace.
4+
- It has inline script metadata (as proposed by PEP 723) that defines the required Python version and dependencies.
55
- If imports which require a specific Python version or dependencies are added, keep the inline script metadata up to date.
66
- You need to call the `Get Python Environment Information` tool on the `<script_name>` path to get the Python executable details.
77
- 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 renamed to files/templates/newInlineScriptTemplate/script.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
# /// script
2-
# requires-python = ">=X.XX" TODO: Update this to the minimum Python version you want to support
3-
# dependencies = [
4-
# TODO: Add any dependencies your script requires
5-
# ]
2+
# requires-python = ">=3.9"
3+
# dependencies = []
4+
# # Add dependency requirement strings to the array above as needed.
65
# ///
76

87
# TODO: Update the main function to your needs or remove it.

src/common/utils/pathUtils.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,44 @@ export function normalizePath(fsPath: string): string {
6565
return path1;
6666
}
6767

68+
/**
69+
* Determines whether `candidatePath` is the same as, or nested inside, `parentPath`.
70+
*
71+
* Both paths are resolved to absolute form and normalized (case-insensitive on
72+
* Windows) before comparison, so differences in separators or drive-letter case
73+
* do not affect the result.
74+
*
75+
* @param parentPath The candidate parent (or ancestor) directory.
76+
* @param candidatePath The path being tested for containment.
77+
* @returns `true` when `candidatePath` equals `parentPath` or is a descendant of it.
78+
*/
79+
export function isSameOrParentPath(parentPath: string, candidatePath: string): boolean {
80+
const relative = path.relative(
81+
normalizePath(path.resolve(parentPath)),
82+
normalizePath(path.resolve(candidatePath)),
83+
);
84+
return (
85+
relative === '' ||
86+
(relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))
87+
);
88+
}
89+
90+
/**
91+
* Determines whether `value` maps to a reserved Windows device name (e.g. `CON`,
92+
* `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`).
93+
*
94+
* The check inspects the portion of the name before the first dot, since Windows
95+
* disallows these names regardless of extension, and only reports `true` on
96+
* Windows, where the restriction applies.
97+
*
98+
* @param value The base file name (without directory) to test.
99+
* @returns `true` on Windows when `value` resolves to a reserved device name.
100+
*/
101+
export function isWindowsReservedDeviceName(value: string): boolean {
102+
const deviceBaseName = value.split('.')[0];
103+
return isWindows() && /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(deviceBaseName);
104+
}
105+
68106
export function getResourceUri(resourcePath: string, root?: string): Uri | undefined {
69107
try {
70108
if (!resourcePath) {
Lines changed: 167 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,66 @@
11
import * as fs from 'fs-extra';
22
import * as path from 'path';
3-
import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, workspace } from 'vscode';
3+
import { commands, l10n, MarkdownString, QuickInputButtons, Uri, window, WorkspaceFolder } from 'vscode';
44
import { PythonProject, PythonProjectCreator, PythonProjectCreatorOptions } from '../../api';
55
import { NEW_PROJECT_TEMPLATES_FOLDER } from '../../common/constants';
66
import { traceError } from '../../common/logging';
7-
import { showInputBoxWithButtons, showTextDocument } from '../../common/window.apis';
7+
import { isSameOrParentPath, isWindowsReservedDeviceName } from '../../common/utils/pathUtils';
8+
import { showErrorMessage, showInputBoxWithButtons, showTextDocument } from '../../common/window.apis';
9+
import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/workspace.apis';
810
import { PythonProjectManager } from '../../internal.api';
911
import { isCopilotInstalled, manageCopilotInstructionsFile, replaceInFilesAndNames } from './creationHelpers';
1012

13+
function validateScriptFileName(value: string): string | null {
14+
const pathSegments = value.split(/[\\/]/);
15+
if (pathSegments.length !== 1 || pathSegments.includes('..')) {
16+
return l10n.t('Script name must be a file name without path separators or traversal.');
17+
}
18+
if (!value.endsWith('.py')) {
19+
return l10n.t('Script name must end with ".py".');
20+
}
21+
const baseName = value.replace(/\.py$/, '');
22+
if (isWindowsReservedDeviceName(baseName)) {
23+
return l10n.t('Script name uses a reserved Windows device name.');
24+
}
25+
// following PyPI (PEP 508) rules for package names
26+
if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) {
27+
return l10n.t(
28+
'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.',
29+
);
30+
}
31+
if (/^[-._0-9]$/i.test(baseName)) {
32+
return l10n.t('Single-character script names cannot be a number, hyphen, or period.');
33+
}
34+
return null;
35+
}
36+
37+
function uriForFileRootInWorkspace(rootPath: string, workspaceFolder: WorkspaceFolder): Uri {
38+
const relativeRootPath = path.relative(path.resolve(workspaceFolder.uri.fsPath), path.resolve(rootPath));
39+
const pathSegments = relativeRootPath.split(/[\\/]/).filter((segment) => segment.length > 0);
40+
return workspaceFolder.uri.with({
41+
path: path.posix.join(workspaceFolder.uri.path, ...pathSegments),
42+
});
43+
}
44+
1145
export class NewScriptProject implements PythonProjectCreator {
1246
public readonly name = l10n.t('newScript');
1347
public readonly displayName = l10n.t('Script');
14-
public readonly description = l10n.t('Creates a new script folder in your current workspace');
48+
public readonly description = l10n.t('Creates a new script in your current workspace');
1549
public readonly tooltip = new MarkdownString(l10n.t('Create a new Python script'));
1650

1751
constructor(private readonly projectManager: PythonProjectManager) {}
1852

1953
async create(options?: PythonProjectCreatorOptions): Promise<PythonProject | Uri | undefined> {
20-
// quick create (needs name, will always create venv and copilot instructions)
21-
// not quick create
22-
// ask for script file name
23-
// ask if they want venv
2454
let scriptFileName = options?.name;
2555
let createCopilotInstructions: boolean | undefined;
2656
if (options?.quickCreate === true) {
2757
// If quickCreate is true, we should not prompt for any input
2858
if (!scriptFileName) {
2959
throw new Error('Script file name is required in quickCreate mode.');
3060
}
61+
if (path.extname(scriptFileName) === '') {
62+
scriptFileName = `${scriptFileName}.py`;
63+
}
3164
createCopilotInstructions = true;
3265
} else {
3366
//Prompt as quickCreate is false
@@ -37,23 +70,7 @@ export class NewScriptProject implements PythonProjectCreator {
3770
prompt: l10n.t('What is the name of the script? (e.g. my_script.py)'),
3871
ignoreFocusOut: true,
3972
showBackButton: true,
40-
validateInput: (value) => {
41-
// Ensure the filename ends with .py and follows valid naming conventions
42-
if (!value.endsWith('.py')) {
43-
return l10n.t('Script name must end with ".py".');
44-
}
45-
const baseName = value.replace(/\.py$/, '');
46-
// following PyPI (PEP 508) rules for package names
47-
if (!/^([a-z_]|[a-z0-9_][a-z0-9._-]*[a-z0-9_])$/i.test(baseName)) {
48-
return l10n.t(
49-
'Invalid script name. Use only letters, numbers, underscores, hyphens, or periods. Must start and end with a letter or number.',
50-
);
51-
}
52-
if (/^[-._0-9]$/i.test(baseName)) {
53-
return l10n.t('Single-character script names cannot be a number, hyphen, or period.');
54-
}
55-
return null;
56-
},
73+
validateInput: validateScriptFileName,
5774
});
5875
} catch (ex) {
5976
if (ex === QuickInputButtons.Back) {
@@ -67,64 +84,143 @@ export class NewScriptProject implements PythonProjectCreator {
6784
createCopilotInstructions = true;
6885
}
6986
}
87+
}
88+
const validationError = validateScriptFileName(scriptFileName);
89+
if (validationError) {
90+
if (options?.quickCreate === true) {
91+
throw new Error(validationError);
92+
}
93+
window.showErrorMessage(validationError);
94+
return undefined;
95+
}
7096

71-
// 1. Copy template folder
72-
const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'new723ScriptTemplate', 'script.py');
73-
if (!(await fs.pathExists(newScriptTemplateFile))) {
74-
window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.'));
75-
traceError(`Template file not found at: ${newScriptTemplateFile}`);
97+
// 1. Copy template file
98+
const newScriptTemplateFile = path.join(NEW_PROJECT_TEMPLATES_FOLDER, 'newInlineScriptTemplate', 'script.py');
99+
if (!(await fs.pathExists(newScriptTemplateFile))) {
100+
window.showErrorMessage(l10n.t('Template file does not exist, aborting creation.'));
101+
traceError(`Template file not found at: ${newScriptTemplateFile}`);
102+
return undefined;
103+
}
104+
105+
// Check if the destination folder is provided, otherwise use the first workspace folder.
106+
let destinationRootUri = options?.rootUri;
107+
let workspaceFolders: readonly WorkspaceFolder[] | undefined;
108+
if (!destinationRootUri) {
109+
workspaceFolders = getWorkspaceFolders();
110+
if (!workspaceFolders || workspaceFolders.length === 0) {
111+
window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.'));
76112
return undefined;
77113
}
114+
destinationRootUri = workspaceFolders[0].uri;
115+
}
78116

79-
// Check if the destination folder is provided, otherwise use the first workspace folder
80-
let destRoot = options?.rootUri.fsPath;
81-
if (!destRoot) {
82-
const workspaceFolders = workspace.workspaceFolders;
83-
if (!workspaceFolders || workspaceFolders.length === 0) {
84-
window.showErrorMessage(l10n.t('No workspace folder is open or provided, aborting creation.'));
85-
return undefined;
86-
}
87-
destRoot = workspaceFolders[0].uri.fsPath;
88-
}
117+
const destRoot = destinationRootUri.fsPath;
118+
const resolvedDestRoot = path.resolve(destRoot);
119+
let workspaceFolder = getWorkspaceFolder(destinationRootUri);
120+
if (!workspaceFolder && destinationRootUri.scheme === 'file') {
121+
workspaceFolders ??= getWorkspaceFolders();
122+
workspaceFolder = workspaceFolders
123+
?.filter((folder) => isSameOrParentPath(folder.uri.fsPath, resolvedDestRoot))
124+
.sort((first, second) => second.uri.fsPath.length - first.uri.fsPath.length)[0];
125+
}
126+
if (!workspaceFolder) {
127+
showErrorMessage(l10n.t('Destination folder must be inside an open workspace, aborting creation.'));
128+
return undefined;
129+
}
89130

90-
// Check if the destination folder already exists
91-
const scriptDestination = path.join(destRoot, scriptFileName);
92-
if (await fs.pathExists(scriptDestination)) {
93-
window.showErrorMessage(
94-
l10n.t(
95-
'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.',
96-
),
97-
);
98-
return undefined;
131+
let physicalDestRoot: string;
132+
let physicalWorkspaceRoot: string;
133+
try {
134+
[physicalDestRoot, physicalWorkspaceRoot] = await Promise.all([
135+
fs.realpath(resolvedDestRoot),
136+
fs.realpath(workspaceFolder.uri.fsPath),
137+
]);
138+
} catch (error) {
139+
traceError('Failed to resolve the destination or workspace folder:', error);
140+
showErrorMessage(l10n.t('Unable to resolve the destination folder inside the open workspace.'));
141+
return undefined;
142+
}
143+
if (!isSameOrParentPath(physicalWorkspaceRoot, physicalDestRoot)) {
144+
showErrorMessage(l10n.t('Destination folder must resolve inside the open workspace, aborting creation.'));
145+
return undefined;
146+
}
147+
148+
const identityRootUri =
149+
destinationRootUri.scheme === 'file' && workspaceFolder.uri.scheme !== 'file'
150+
? uriForFileRootInWorkspace(resolvedDestRoot, workspaceFolder)
151+
: destinationRootUri;
152+
const scriptDestination = path.resolve(resolvedDestRoot, scriptFileName);
153+
const relativeScriptPath = path.relative(resolvedDestRoot, scriptDestination);
154+
if (
155+
relativeScriptPath === '' ||
156+
relativeScriptPath === '..' ||
157+
relativeScriptPath.startsWith(`..${path.sep}`) ||
158+
path.isAbsolute(relativeScriptPath)
159+
) {
160+
const containmentError = l10n.t('Script name must resolve to a file inside the destination folder.');
161+
if (options?.quickCreate === true) {
162+
throw new Error(containmentError);
99163
}
100-
await fs.copy(newScriptTemplateFile, scriptDestination);
164+
window.showErrorMessage(containmentError);
165+
return undefined;
166+
}
101167

102-
// 2. Replace 'script_name' in the file using a helper (just script name remove .py)
168+
// Check if the destination file already exists
169+
if (await fs.pathExists(scriptDestination)) {
170+
window.showErrorMessage(
171+
l10n.t(
172+
'A script file by that name already exists, aborting creation. Please retry with a unique script name given your workspace.',
173+
),
174+
);
175+
return undefined;
176+
}
177+
// Build the project entry up front so copying the template, substituting
178+
// the script name, and registering the project share one cleanup boundary:
179+
// if any step fails, the partially created script is removed so a retry
180+
// starts from a clean state.
181+
const createdScript: PythonProject = {
182+
name: scriptFileName,
183+
uri: identityRootUri.with({
184+
path: path.posix.join(identityRootUri.path, scriptFileName),
185+
}),
186+
};
187+
let projectRegistrationAttempted = false;
188+
try {
189+
await fs.copy(newScriptTemplateFile, scriptDestination);
190+
// Replace 'script_name' in the file (script name without the .py suffix).
103191
await replaceInFilesAndNames(scriptDestination, 'script_name', scriptFileName.replace(/\.py$/, ''));
104-
105-
// 3. add custom github copilot instructions
106-
if (createCopilotInstructions) {
107-
const packageInstructionsPath = path.join(
108-
NEW_PROJECT_TEMPLATES_FOLDER,
109-
'copilot-instructions-text',
110-
'script-copilot-instructions.md',
111-
);
112-
await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [
113-
{ searchValue: '<script_name>', replaceValue: scriptFileName },
114-
]);
192+
projectRegistrationAttempted = true;
193+
await this.projectManager.add(createdScript);
194+
} catch (creationError) {
195+
if (projectRegistrationAttempted) {
196+
try {
197+
this.projectManager.remove(createdScript);
198+
} catch (rollbackError) {
199+
traceError('Failed to remove the new script project after creation failed:', rollbackError);
200+
}
115201
}
202+
try {
203+
await fs.remove(scriptDestination);
204+
} catch (rollbackError) {
205+
traceError('Failed to delete the new script after creation failed:', rollbackError);
206+
}
207+
throw creationError;
208+
}
116209

117-
// Add the created script to the project manager
118-
const createdScript: PythonProject | undefined = {
119-
name: scriptFileName,
120-
uri: Uri.file(scriptDestination),
121-
};
122-
this.projectManager.add(createdScript);
210+
// 3. add custom github copilot instructions
211+
if (createCopilotInstructions) {
212+
const packageInstructionsPath = path.join(
213+
NEW_PROJECT_TEMPLATES_FOLDER,
214+
'copilot-instructions-text',
215+
'script-copilot-instructions.md',
216+
);
217+
await manageCopilotInstructionsFile(destRoot, packageInstructionsPath, [
218+
{ searchValue: '<script_name>', replaceValue: scriptFileName },
219+
]);
220+
}
123221

124-
await showTextDocument(createdScript.uri);
222+
await showTextDocument(createdScript.uri);
125223

126-
return createdScript;
127-
}
128-
return undefined;
224+
return createdScript;
129225
}
130226
}

src/features/terminal/utils.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { VENV_MANAGER_ID } from '../../common/constants';
1111
import { traceError, traceVerbose } from '../../common/logging';
1212
import { timeout } from '../../common/utils/asyncUtils';
1313
import { createSimpleDebounce } from '../../common/utils/debounce';
14+
import { isSameOrParentPath } from '../../common/utils/pathUtils';
1415
import { onDidChangeTerminalShellIntegration, onDidWriteTerminalData } from '../../common/window.apis';
1516
import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis';
1617
import { identifyTerminalShell } from '../common/shellDetector';
@@ -199,11 +200,6 @@ async function getDistinctProjectEnvs(
199200
return envs;
200201
}
201202

202-
function isSameOrParentPath(parentPath: string, candidatePath: string): boolean {
203-
const relative = path.relative(path.resolve(parentPath), path.resolve(candidatePath));
204-
return relative === '' || (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative));
205-
}
206-
207203
function getProjectForCwd(projects: readonly PythonProject[], cwd: string): PythonProject | undefined {
208204
return [...projects]
209205
.filter((project) => isSameOrParentPath(project.uri.fsPath, cwd))

0 commit comments

Comments
 (0)