gemini-cli icon indicating copy to clipboard operation
gemini-cli copied to clipboard

Hooks - Security, PII, and Telemetry Review

Open Edilmo opened this issue 3 weeks ago • 0 comments

Comprehensive security review needed covering two main areas:

  1. 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.

  2. PII in Telemetry: Hook inputs/outputs may contain sensitive data (user prompts, file contents, API responses). The telemetry system respects telemetry.logPrompts setting, 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
  • Telemetry/PII:
    • packages/core/src/hooks/hookEventHandler.ts:155-182 - recordHookCall() method
    • PII-sensitive fields: hook_input, hook_output, stdout, stderr (conditionally logged based on telemetry.logPrompts)

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=false properly excludes sensitive data
  • [ ] Document best practices for hook developers handling sensitive data

Edilmo avatar Dec 08 '25 17:12 Edilmo