Studio CLI sites command #1958
Studio CLI sites command #1958
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| await fs.promises.mkdir( appDir, { recursive: true } ); | ||
|
|
||
| const escapedCliCommand = cliCommand.replace( /"/g, '\\"' ); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
To fix the problem, we need to ensure that both backslashes and double-quote characters are properly escaped in the cliCommand before it is interpolated into the AppleScript template string. This should be done on line 278 by replacing not only all " with \", but also all \ with \\ (with the correct order: backslashes first). The best way to do this is with two chained .replace operations or, preferably, by using a sanitizer library if available. Since we are restricted to working only within the provided code region, we will use regular expressions: first .replace(/\\/g, '\\\\') (escape any backslashes), then .replace(/"/g, '\\"') (escape quotes). No additional imports are needed.
This change should be made directly to the block where escapedCliCommand is set. No other changes or method definitions are needed.
| @@ -275,7 +275,7 @@ | ||
|
|
||
| await fs.promises.mkdir( appDir, { recursive: true } ); | ||
|
|
||
| const escapedCliCommand = cliCommand.replace( /"/g, '\\"' ); | ||
| const escapedCliCommand = cliCommand.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); | ||
|
|
||
| const appleScript = `on open location this_URL | ||
| set command to "${ escapedCliCommand } '" & this_URL & "'" |
| const unzipStream = unzipper.Parse(); | ||
|
|
||
| unzipStream.on( 'entry', ( entry ) => { | ||
| const entryPath = entry.path; |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 days ago
To fix this issue, sanitize and validate each archive entry's path before using it for extraction. The best approach is:
- Use
path.jointo construct the output path, - Use
path.resolveon both the constructed output path and the intended destination directory, - Ensure that the output path starts with the resolved extraction destination: in other words, that the output file is inside the intended extraction directory.
- If the path contains any
..component or the constructed absolute path does not start withdestinationFolder, skip extraction and log an error.
You should implement this by adding a validation step before writing the file. Make changes only within the provided code in common/lib/wordpress-version-manager.ts, namely the relevant lines within the downloadFileAndUnzip function.
You do not need to add external dependencies; Node.js's built-in path module suffices.
| @@ -84,6 +84,18 @@ | ||
| const entryPath = entry.path; | ||
| const fullPath = path.join( destinationFolder, entryPath ); | ||
|
|
||
| // Sanitize: ensure that fullPath is inside destinationFolder, and entryPath is not malicious | ||
| const resolvedDestFolder = path.resolve( destinationFolder ); | ||
| const resolvedFullPath = path.resolve( fullPath ); | ||
| if ( | ||
| !resolvedFullPath.startsWith(resolvedDestFolder + path.sep) && | ||
| resolvedFullPath !== resolvedDestFolder // allow extraction directly into base folder | ||
| ) { | ||
| console.error( `Skipping potentially unsafe archive entry path: ${ entryPath }` ); | ||
| entry.autodrain(); | ||
| return; | ||
| } | ||
|
|
||
| if ( entry.type === 'Directory' ) { | ||
| entry.autodrain(); | ||
| return; |
Related issues
Proposed Changes
Testing Instructions
Pre-merge Checklist