Fix: Add bash support#31
Conversation
Adds bash as a supported shell type across agent and web packages.
Sessions exited immediately on systems without zsh because the shell was hardcoded. The server now detects available shells at startup (zsh > bash) and can be overridden with CLSH_SHELL env var. The client now defers shell selection to the server. Closes my-claude-utils#29
| } | ||
|
|
||
| const createSession = useCallback((): void => { | ||
| wsClient?.send({ type: 'session_create', shell: 'zsh' }); |
There was a problem hiding this comment.
Not sure if there is a plan to allow the user to select a shell, but this still functions as before.
my-claude-utils
left a comment
my-claude-utils
left a comment
There was a problem hiding this comment.
Nice contribution — clean, well-scoped, and well-documented. Adding bash support + auto-detection is a great improvement for Linux users.
Two things to address before merging (one security, one nit):
-
Command injection risk in
shellExists()— see inline comment. Currently safe because callers validate first, but the function itself is unsafe by design. Since this is an open-source project, future contributors might call it without upstream validation. -
Missing trailing newline in
.env.example— minor nit.
Everything else looks solid — types are consistent across frontend and backend, the DefaultableShell vs ShellType distinction is a good pattern, and making shell optional in session_create is the right call.
| } | ||
|
|
||
| function shellExists(shell: string): boolean { | ||
| try { |
There was a problem hiding this comment.
Security: command injection risk
execSync(\command -v ${shell}`)interpolatesshelldirectly into a shell command string. While the current callers validate againstDEFAULTABLE_SHELLSbefore calling this, the function itself accepts anystring, so a future contributor calling shellExists(userInput)` without validation would introduce command injection.
Defense in depth fix: pass shell as a positional argument instead of interpolating:
import { execFileSync } from 'node:child_process';
function shellExists(shell: string): boolean {
try {
execFileSync('sh', ['-c', 'command -v -- "$1"', '--', shell], { stdio: 'ignore' });
return true;
} catch {
return false;
}
}This way the value of shell never gets interpreted by the shell, regardless of what's passed in. execFileSync doesn't spawn a shell for the outer call, and $1 passes the argument safely.
.env.example
Outdated
|
|
||
| # Optional: Default shell for new terminal sessions (auto-detected if not set) | ||
| # Valid values: bash, zsh | ||
| CLSH_SHELL= No newline at end of file |
There was a problem hiding this comment.
Nit: missing trailing newline. POSIX expects text files to end with \n.
|
Thanks so much for this contribution @Biohazord! 🎉 Great work on the shell auto-detection and bash support. This makes clsh way more accessible for Linux users who don't have zsh installed. The Merged and closing #29. Welcome aboard as a contributor! |
2 participants
Description
Proposed fix for issue #29
Type of Change
Testing Checklist
npx clsh-dev(ornpm run devfor development)npm run lint)npm run typecheck)npm run build)