Comfy-Org / Comfy-Org/Comfy-Desktop

Console terminal: shell-escape user-controlled paths in init commands

Open
#1,109 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
458
Forks
59
Avg merge
22h 18m
Merged PRs (30d)
45

Description

## Summary

The interactive Console (`src/main/lib/terminal.ts`) sets up each install's shell by interpolating filesystem paths directly into quoted shell command strings (PowerShell on Windows, bash on macOS/Linux). For example:

```ts
`$env:VIRTUAL_ENV = "${venvDir}"`
`function pip { & "${env.pip.exe}" ${env.pip.args.join(' ')} $args }`
`source "${env.venvDir}/bin/activate"`
`alias pip='"${env.pip.exe}" ...'`
```

The interpolated values (`venvDir`, `pathPrepends`, `pip.exe`, `promptName`) are derived from user-controlled install paths. They are not shell-escaped, so paths containing shell metacharacters can break activation or, in the worst case, execute unintended shell syntax:

- PowerShell double-quoted strings expand `$...`, so a path like `C:\my$stuff\.venv` is mangled.
- A literal `"` or backtick (`` ` ``) can terminate the quote early.
- On bash, `$`, backticks, `$( )`, and `'` inside a single-quoted alias break similarly.

Spaces (the common case) are already handled by the existing quoting.

## Scope / risk

- **Pre-existing**: this string-building predates #1107 (which only added the git/portable/desktop env cases while preserving the existing pattern). Filed as a follow-up from that PR's review.
- **Severity: low / local-only**: only triggers when an install path contains unusual characters; impact is scoped to the local user.

## Proposed fix

- Add per-shell quoting helpers, e.g.:
```ts
function psQuote(v: string) { return `'${v.replace(/'/g, "''")}'` } // PowerShell single-quote
function shQuote(v: string) { return `'${v.replace(/'/g, `'\\''`)}'` } // POSIX single-quote
```
- Use them for every interpolated path/arg in `initCommands()`; prefer single-quoted PowerShell strings (or concatenation) so `$` is not expanded, and a bash `pip()` function over an alias for cleaner arg forwarding.
- Add unit tests covering hostile paths (containing `$`, `"`, `` ` ``, `'`, spaces) for both platforms.

## References

- PR that introduced the source-aware terminal env: https://github.com/Comfy-Org/Comfy-Desktop/pull/1107

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.