Studio CLI sites command - #1958
Conversation
![github-advanced-security[bot]](https://cdn.statically.io/img/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 9 months 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 9 months 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; |
|
We are nearing the launch of the CLI i2 project (see the |
4 participants
Related issues
Proposed Changes
Testing Instructions
Pre-merge Checklist