Support remote workspace & support auth on electron and capacitor #89
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // List all folders in a path | ||
| async function handleListProjects(uri: string) { | ||
| const rootPath = uri; | ||
| const files = await fs.promises.readdir(rootPath, { withFileTypes: true }); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
| .map((file) => file.name) | ||
| .map((projectName) => ({ | ||
| name: projectName, | ||
| ctime: fs.statSync(path.join(rootPath, projectName)).ctime, |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
General fix approach:
Implement a safe root directory for all file-system operations. User-provided URIs/paths should be resolved relative to that root directory and then normalized to remove any traversal (..) components. After normalization, verify that the resolved file path is still within the intended root directory—otherwise, reject the operation.
Detailed fix for current code:
- Define a constant
PROJECTS_ROOTat the top of the file to specify a safe base directory (e.g.,/pulse-editor/projects). - In the
handleListProjectsfunction, for any incominguri, resolve it relative toPROJECTS_ROOTusingpath.resolve(PROJECTS_ROOT, uri). - After resolving, ensure the final path starts with
PROJECTS_ROOT(usingpath.resolvefor both). - If the validation fails, throw an error or return an error response.
- All uses of
rootPathin this function should refer to the sanitized path, not the raw user input.
Changes required:
- Define and use a safe
PROJECTS_ROOTdirectory. - Amend
handleListProjectsto securely resolve and validate the path. - Ensure all
rootPathusages in this method are with the checked, normalized value. - Add required error handling (e.g., throwing an error or returning an appropriate message if the check fails).
| @@ -4,6 +4,7 @@ | ||
|
|
||
| // Define a safe root directory for projects. Can be overridden by env or configured as needed. | ||
|
|
||
| const PROJECTS_ROOT = "/pulse-editor/projects"; // <-- Set this directory as your allowed root | ||
| const settingsPath = path.join("/pulse-editor", "settings.json"); | ||
|
|
||
| export async function handlePlatformAPIRequest( | ||
| @@ -113,14 +114,19 @@ | ||
|
|
||
| // List all folders in a path | ||
| async function handleListProjects(uri: string) { | ||
| const rootPath = uri; | ||
| const files = await fs.promises.readdir(rootPath, { withFileTypes: true }); | ||
| // Sanitize and secure the rootPath by resolving it under PROJECTS_ROOT | ||
| const requestedPath = path.resolve(PROJECTS_ROOT, "." + path.sep + uri); // enforce relative | ||
| // Check that the requested path is still within PROJECTS_ROOT | ||
| if (!requestedPath.startsWith(path.resolve(PROJECTS_ROOT))) { | ||
| throw new Error("Access to the requested path is not allowed."); | ||
| } | ||
| const files = await fs.promises.readdir(requestedPath, { withFileTypes: true }); | ||
| const folders = files | ||
| .filter((file) => file.isDirectory()) | ||
| .map((file) => file.name) | ||
| .map((projectName) => ({ | ||
| name: projectName, | ||
| ctime: fs.statSync(path.join(rootPath, projectName)).ctime, | ||
| ctime: fs.statSync(path.join(requestedPath, projectName)).ctime, | ||
| })); | ||
|
|
||
| return folders; |
| baseUri: string | undefined = undefined, | ||
| ) { | ||
| const rootPath = uri; | ||
| const files = await fs.promises.readdir(rootPath, { withFileTypes: true }); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The best way to fix the issue is to restrict all user-supplied paths involved in filesystem operations to a safe root directory (e.g., /pulse-editor). This involves:
- Defining a constant, such as
PROJECTS_ROOT, that points to the root folder under which all filesystem operations must take place. - Whenever a path (such as
uri) is received from the user, normalize it by resolving it againstPROJECTS_ROOTusingpath.resolve. - After resolving, use
fs.realpathSync(or an async equivalent) to collapse symbolic links and further normalize the path. - After normalization, validate that the resolved path starts with the safe root directory (
PROJECTS_ROOT) before proceeding. If not, reject the operation. - Apply these checks in every handler that receives and uses user path input:
handleListProjects,listPathContent,handleListPathContent, and similar handlers.
This approach ensures that even if a user tries to supply "../../etc" or similar, the final path will never escape the designated project root.
|
|
||
| async function handleCreateProject(uri: string) { | ||
| // Create a folder at the validated path | ||
| await fs.promises.mkdir(uri); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The appropriate fix is to validate and restrict all user-supplied paths before using them in file operations. The best approach is:
- Define a safe root directory for all operations (for example,
/pulse-editor/projects). - For any API accepting a path from the user, first normalize (resolve) the path to get rid of things like
../, and then verify that the resulting absolute path is still under the intended root. - If the resulting path lies outside the root, reject the operation.
This logic should be implemented once in a utility function (e.g., getSafePath(uri: string): string), used anywhere a user-supplied path is accessed. This function should:
- Use
path.resolve(ROOT_DIR, uri)to normalize against the project root. - Optionally canonicalize further via
fs.realpathSync()if necessary. - Ensure the resulting path starts with the project root absolute path.
Apply this function to all uses of the user-supplied uri in file operations (handleCreateProject, handleDeleteProject, etc.).
You'll need:
- A constant for the project root.
- The helper function.
- Modification of all functions to use the safe path before all filesystem access.
|
|
||
| async function handleDeleteProject(uri: string) { | ||
| // Delete the folder at the validated path | ||
| await fs.promises.rm(uri, { recursive: true, force: true }); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The best way to fix the problem is to validate and restrict all user-supplied file/folder paths before performing any filesystem operations (create, delete, list, etc.). A robust approach is to define a safe root directory for all project operations (for example /pulse-editor), and ensure that all operations happen only within subdirectories of this root. To enforce this, before executing any operation like fs.promises.rm(uri, ...), normalize the requested uri (using path.resolve or similar), and confirm that it starts with the configured root path. If not, throw an error or deny the request.
The required changes are:
- Define a root directory constant (reuse or clarify
/pulse-editoras appropriate). - For each function that accepts a
uriparameter and operates on the filesystem (handleDeleteProject,handleCreateProject,handleUpdateProject, etc.), add code to resolve and validate the path:- Use
path.resolve(ROOT, uri)or similar. - (Optionally): Use
fs.realpathSyncfor symlink safety. - Ensure the result begins with the root path.
- If validation fails, throw an error or return a forbidden response.
- Use
- Change all further references to the original
urito use the validated/normalized path. - The change should be done in
remote-workspace/src/servers/api-server/platform-api/handler.ts, in each handler that uses user-supplied URIs.
No external libraries are needed beyond path and fs, which are already imported.
| @@ -4,8 +4,24 @@ | ||
|
|
||
| // Define a safe root directory for projects. Can be overridden by env or configured as needed. | ||
|
|
||
| const settingsPath = path.join("/pulse-editor", "settings.json"); | ||
| const ROOT = "/pulse-editor"; // Define safe project root | ||
| const settingsPath = path.join(ROOT, "settings.json"); | ||
|
|
||
| // Validate path is under root directory | ||
| function validatePathUnderRoot(uri: string): string { | ||
| const resolvedPath = path.resolve(ROOT, uri); | ||
| // Optionally use realpathSync for symlink safety. Try/catch, fallback to resolvedPath if it fails. | ||
| let realPath; | ||
| try { | ||
| realPath = fs.realpathSync(resolvedPath); | ||
| } catch { | ||
| realPath = resolvedPath; | ||
| } | ||
| if (!realPath.startsWith(ROOT)) { | ||
| throw new Error("Forbidden path detected"); | ||
| } | ||
| return realPath; | ||
| } | ||
| export async function handlePlatformAPIRequest( | ||
| data: { | ||
| operation: string; | ||
| @@ -187,13 +202,15 @@ | ||
| } | ||
|
|
||
| async function handleCreateProject(uri: string) { | ||
| // Create a folder at the validated path | ||
| await fs.promises.mkdir(uri); | ||
| // Validate and create folder | ||
| const safePath = validatePathUnderRoot(uri); | ||
| await fs.promises.mkdir(safePath); | ||
| } | ||
|
|
||
| async function handleDeleteProject(uri: string) { | ||
| // Delete the folder at the validated path | ||
| await fs.promises.rm(uri, { recursive: true, force: true }); | ||
| // Validate and delete folder | ||
| const safePath = validatePathUnderRoot(uri); | ||
| await fs.promises.rm(safePath, { recursive: true, force: true }); | ||
| } | ||
|
|
||
| async function handleUpdateProject( | ||
| @@ -203,13 +217,16 @@ | ||
| ctime?: Date; | ||
| }, | ||
| ) { | ||
| const newUri = path.join(path.dirname(uri), updatedInfo.name); | ||
| await fs.promises.rename(uri, newUri); | ||
| const safeOldPath = validatePathUnderRoot(uri); | ||
| const newUri = path.join(path.dirname(safeOldPath), updatedInfo.name); | ||
| const safeNewPath = validatePathUnderRoot(path.relative(ROOT, newUri)); | ||
| await fs.promises.rename(safeOldPath, safeNewPath); | ||
| } | ||
|
|
||
| async function handleCreateFolder(uri: string) { | ||
| // Create a folder at the validated path | ||
| await fs.promises.mkdir(uri); | ||
| // Validate and create folder | ||
| const safePath = validatePathUnderRoot(uri); | ||
| await fs.promises.mkdir(safePath); | ||
| } | ||
|
|
||
| async function handleCreateFile(uri: string) { |
| const safePath = uri; | ||
| // create parent directory if it doesn't exist | ||
| const dir = path.dirname(safePath); | ||
| if (!fs.existsSync(dir)) { |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
| // create parent directory if it doesn't exist | ||
| const dir = path.dirname(safePath); | ||
| if (!fs.existsSync(dir)) { | ||
| fs.mkdirSync(dir, { recursive: true }); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The best fix is to enforce that any filesystem path affected by user input cannot escape a designated safe root directory (such as /pulse-editor). This should be done by resolving the provided path relative to the root, normalizing and de-referencing it, and checking that the result begins with the root directory.
For every handler where a user-supplied path is being used as a filesystem path (e.g., the uri parameter in read/write/delete/has-path/copy-files etc.), wrap the path resolution as follows:
- Compute the absolute resolved path using
path.resolve(ROOT_DIR, userInputPath). - Optionally, call
fs.realpathSyncfor sym-link safety. - Check that the result starts with the root directory's absolute path (
resolvedPath.startsWith(ROOT_DIR)). - If it does not, throw an error or return an appropriate response.
You will need to:
- Define ROOT_DIR (at the top of the module).
- For each handler, wrap the critical path(s) with normalization and boundary check.
- For functions accessing parent directories (such as
write-file, which creates parent directory), ensure that the computed parent is also checked under the same boundary.
In terms of code regions:
- Insert a
const ROOT_DIR = ...(e.g.,/pulse-editor). - In all relevant handler functions (
handleWriteFile,handleReadFile,handleDelete,handleCopyFiles,handleHasPath), replace direct use of the URIs/paths with the safe, normalized, boundary-checked path.
| fs.mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| fs.writeFileSync(safePath, data); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
To fix the uncontrolled path usage, all user-supplied paths must be validated and restricted to a safe location. The best approach is as follows:
- Define a constant (e.g.,
PROJECTS_ROOT) as the root directory where all permitted file operations take place (e.g.,/pulse-editor/projects). - Replace all raw usage of the
uriargument in file access functions (handleWriteFile,handleReadFile,handleDelete,handleHasPath,handleCopyFiles) by resolving the final path usingpath.resolve(PROJECTS_ROOT, uri)to eliminate any..segments and to convert to an absolute path. - After resolution, ensure the path is strictly contained in
PROJECTS_ROOTusingstartsWithor similar. - Deny access (throw error or return a clear response) if the resolved path is not a subdirectory or file within the designated root.
- Optionally, use filenames sanitization (
sanitize-filename) if you want to restrict to simple filenames, but using the "root containment" technique works for (almost) all use-cases if you intend to allow directories/files underneath a given root.
Changes will affect remote-workspace/src/servers/api-server/platform-api/handler.ts in the relevant handler functions. The only necessary import is path (already present), so no third-party libraries are strictly required for root containment. There is no need to edit remote-workspace/src/servers/api-server/index.ts, except to ensure the handler is still called correctly.
| async function handleCopyFiles(from: string, to: string) { | ||
| // Copy the files from the validated from path to the validated to path | ||
| await fs.promises.cp( | ||
| from, |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
The best fix is to restrict file/directory operations to a safe root directory (e.g., /pulse-editor/projects or similar). For each handler function accepting a path from untrusted input (not just copy-files, but also delete, write-file, etc.), normalize the input path relative to the root and check that the resolved absolute path indeed lives within (is a descendant of) that root. This means:
- Normalize using
path.resolveand/orfs.realpathSync(for symlink attacks protection). - For each relevant function, change the usage from directly trusting
from,to,uri, etc. to mapping them to validated, normalized safe paths. - Add a reusable helper (e.g.,
getSafePath(subPath: string)), which throws/rejects if the resolved path is not within the root. - Use this helper in all affected functions.
- Update
handleCopyFilesto use safe paths for bothfromandto.
The edits should be in remote-workspace/src/servers/api-server/platform-api/handler.ts only, and must not alter functional logic, just add validation.
You only need to add imports from well-known external libraries. However, for normalization and checking, standard Node.js path and fs suffice.
| // Copy the files from the validated from path to the validated to path | ||
| await fs.promises.cp( | ||
| from, | ||
| to, |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
To prevent path traversal vulnerabilities, we should restrict file operations to a "safe root directory." Every user-provided file path (including both from and to in copy operations) must be resolved and normalized relative to this root, then checked to ensure the result stays within the root directory. This typically involves using path.resolve to join the root with the user-provided path, then confirming the resulting path starts with the root directory. If this check fails, the operation should throw an error.
We'll make the following changes:
- Choose a secure, absolute root directory (e.g.,
/pulse-editoras already used elsewhere in the file). - Update
handleCopyFilesto:- Resolve both
fromandtoagainst the root directory usingpath.resolve. - (Optional, but good practice) Ensure the actual realpath exists within the root using
fs.realpathSync. - Check both resolved paths start with the root directory.
- If either path does not, throw an error or return a 403/permission denied.
- Resolve both
- Optionally, for consistency/safety, wrap this path-validation logic in a helper method.
Code changes needed:
- In
handler.ts:- Define a constant for the safe root directory if not already present.
- Add a helper function (e.g.,
resolveSafePath) that validates paths against this root. - Update
handleCopyFilesto call this helper for bothfromandto. - Raise an error (e.g.,
throw new Error("Access denied")) if the check fails.
| @@ -4,8 +4,11 @@ | ||
|
|
||
| // Define a safe root directory for projects. Can be overridden by env or configured as needed. | ||
|
|
||
| const settingsPath = path.join("/pulse-editor", "settings.json"); | ||
| // Define a safe root directory. Should be an absolute path. Consider making this configurable via env. | ||
| const SAFE_ROOT = "/pulse-editor"; | ||
|
|
||
| const settingsPath = path.join(SAFE_ROOT, "settings.json"); | ||
|
|
||
| export async function handlePlatformAPIRequest( | ||
| data: { | ||
| operation: string; | ||
| @@ -257,11 +259,23 @@ | ||
| fs.writeFileSync(safePath, data); | ||
| } | ||
|
|
||
| function resolveSafePath(userPath: string): string { | ||
| // Resolve the path relative to the root | ||
| const absPath = path.resolve(SAFE_ROOT, userPath); | ||
| // Ensure the resulting path is within the SAFE_ROOT | ||
| if (!absPath.startsWith(SAFE_ROOT + path.sep)) { | ||
| throw new Error("Access denied: Path outside of safe root directory"); | ||
| } | ||
| return absPath; | ||
| } | ||
|
|
||
| async function handleCopyFiles(from: string, to: string) { | ||
| // Copy the files from the validated from path to the validated to path | ||
| // Validate and resolve the supplied paths | ||
| const safeFrom = resolveSafePath(from); | ||
| const safeTo = resolveSafePath(to); | ||
| await fs.promises.cp( | ||
| from, | ||
| to, | ||
| safeFrom, | ||
| safeTo, | ||
| { | ||
| recursive: true, | ||
| }, |
No description provided.