gemini-cli
gemini-cli copied to clipboard
Hooks - Security, PII, and Telemetry Review
Comprehensive security review needed covering two main areas:
-
Command Injection Vulnerability: Shell command execution in hooks can be exploited if an attacker convinces a user to run the CLI on a directory containing malicious shell characters. Similar vulnerabilities have been reported before. Claude Code accepts bash expressions, so we may need to maintain compatibility while securing the implementation.
-
PII in Telemetry: Hook inputs/outputs may contain sensitive data (user prompts, file contents, API responses). The telemetry system respects
telemetry.logPromptssetting, but comprehensive review needed.
Code Location:
- Command execution (security risk):
- packages/core/src/hooks/hookRunner.ts:177-336 -
executeCommandHook()spawns shell commands - packages/core/src/hooks/hookRunner.ts:207-219 - Environment and command setup
- packages/core/src/hooks/hookRunner.ts:341-345 -
expandCommand()performs variable substitution
- packages/core/src/hooks/hookRunner.ts:177-336 -
- Telemetry/PII:
- packages/core/src/hooks/hookEventHandler.ts:155-182 -
recordHookCall()method - PII-sensitive fields:
hook_input,hook_output,stdout,stderr(conditionally logged based ontelemetry.logPrompts)
- packages/core/src/hooks/hookEventHandler.ts:155-182 -
Related Documentation:
- Design doc lines 928-955 describe privacy considerations
- docs/hooks/best-practices.md documents privacy settings
Follow-Up Action:
- [ ] Security Expert to conduct security review (deferred to post-v1):
- [ ] Review shell command execution for injection vulnerabilities
- [ ] Propose mitigations while maintaining Claude Code compatibility
- [ ] Greg will contribute fixes/improvements himself
- [ ] Known issue: shell exec with directory-based shell characters
- [ ] Core team member to coordinate additional reviews:
- [ ] Get telemetry expert to review telemetry logging patterns
- [ ] Get PII expert to validate data handling across all hook events
- [ ] Review all hook input/output types for PII exposure
- [ ] Validate
telemetry.logPrompts=falseproperly excludes sensitive data - [ ] Document best practices for hook developers handling sensitive data