← Back to PRs

#8767: fix(signal): validate cliPath before spawning signal-cli daemon

by yubrew open 2026-02-04 11:00 View on GitHub →
channel: signal stale
## Summary Validates the signal-cli executable path before spawning to prevent arbitrary command execution via malicious configuration. ## The Problem The `spawnSignalDaemon()` function accepts a user-configurable `cliPath` parameter that is passed directly to `child_process.spawn()` without validation. An attacker who can modify the configuration (via config file write access or the config.patch API) could specify a malicious executable path, leading to arbitrary code execution with the privileges of the OpenClaw process. ## Changes - `src/signal/daemon.ts`: Add `isSafeExecutableValue()` validation before `spawn()` - `src/signal/daemon.test.ts`: Add tests for path validation (null bytes, shell metacharacters, newlines, flag injection) ## Test Plan - [x] `pnpm build && pnpm check && pnpm test` passes - [x] New tests verify malicious paths are rejected - [x] Existing signal functionality unaffected ## Related - [CWE-78: OS Command Injection](https://cwe.mitre.org/data/definitions/78.html) - Internal audit ref: VULN-028 --- *Built with [bitsec-ai](https://github.com/bitsec-ai). AI-assisted: Yes. Testing: fully tested (test written before fix). Code reviewed and understood.* <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR adds a defensive validation check for `SignalDaemonOpts.cliPath` in `src/signal/daemon.ts` before calling `child_process.spawn()`, using the existing `isSafeExecutableValue()` helper. It also extends `src/signal/daemon.test.ts` with unit tests that assert various malicious `cliPath` inputs (null bytes, shell metacharacters, newlines, leading `-`) are rejected. The change aligns Signal’s daemon spawning with other parts of the codebase that already validate executable tokens (e.g. config schema and onboarding helpers) to reduce command-execution risk from untrusted configuration. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with low risk; it adds a straightforward validation guard plus targeted tests. - Changes are localized to Signal daemon spawn path and reuse an existing shared validation helper that’s already used in config/onboarding. The main remaining concern is error-message hygiene (reflecting raw untrusted values) and test brittleness around matching error text. - src/signal/daemon.ts (error message content), src/signal/daemon.test.ts (assertions depending on error text) <!-- greptile_other_comments_section --> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=0d0c8278-ef8e-4d6c-ab21-f5527e322f13)) <!-- /greptile_comment -->

Most Similar PRs