fix(security): prevent command injection in test-hook.sh (#148)

## Description

Fix command injection vulnerability in `test-hook.sh` by using proper
argument passing instead of string interpolation in bash -c.

The previous implementation was vulnerable:

```bash
# Vulnerable (before)
output=$(timeout "$TIMEOUT" bash -c "cat '$TEST_INPUT' | $HOOK_SCRIPT" 2>&1)
```

This has been fixed with:

1. **Path validation**: Reject hook script paths containing dangerous
shell characters (`;|&`$(){}<>`)

2. **Safe argument passing**: Use bash -c's positional parameters via
`--` separator:

```bash
# Safe (after)
if [ "$HOOK_IS_EXECUTABLE" = true ]; then
  output=$(timeout "$TIMEOUT" bash -c 'cat "$1" | "$2"' -- "$TEST_INPUT" "$HOOK_SCRIPT" 2>&1)
else
  output=$(timeout "$TIMEOUT" bash -c 'cat "$1" | bash "$2"' -- "$TEST_INPUT" "$HOOK_SCRIPT" 2>&1)
fi
```

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Documentation update (improvements to README, CLAUDE.md, or
component docs)
- [ ] Refactoring (code change that neither fixes a bug nor adds a
feature)
- [ ] Configuration change (changes to .markdownlint.json, plugin.json,
etc.)

## Component(s) Affected

- [ ] Commands (`/plugin-dev:*`)
- [x] Skills (methodology and best practices)
- [ ] Agents (requirements-assistant)
- [ ] Hooks (UserPromptSubmit)
- [ ] Documentation (README.md, CLAUDE.md, SECURITY.md)
- [ ] Configuration (.markdownlint.json, plugin.json, marketplace.json)
- [ ] Issue/PR templates
- [ ] Other (please specify):

## Motivation and Context

The `test-hook.sh` script used string interpolation when passing
arguments to `bash -c`, which could allow command injection through
maliciously crafted file paths. This security fix ensures that file
paths are properly validated and passed as positional parameters.

## How Has This Been Tested?

**Test Configuration**:
- Claude Code version: Latest
- GitHub CLI version: 2.x
- OS: macOS
- Testing repository: N/A

**Test Steps**:
1. Load plugin with `cc --plugin-dir plugins/plugin-dev`
2. Run `./skills/hook-development/scripts/test-hook.sh` with a normal
hook script
3. Verify normal execution still works
4. Verify paths with spaces work correctly
5. Verify paths with dangerous characters (`;|&`) are rejected with
error message

## Checklist

### General

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
(if applicable)
- [x] My changes generate no new warnings or errors

### Documentation

- [ ] I have updated the documentation accordingly (README.md,
CLAUDE.md, or component docs)
- [ ] I have updated YAML frontmatter (if applicable)
- [ ] I have verified all links work correctly

### Markdown

- [x] I have run `markdownlint` and fixed all issues
- [x] My markdown follows the repository style (ATX headers, dash lists,
fenced code blocks)
- [ ] I have verified special HTML elements are properly closed
(`<example>`, `<commentary>`, etc.)

### Component-Specific Checks

N/A - This is a shell script security fix, not a command/skill/agent
change.

### Testing

- [ ] I have tested the plugin locally with `cc --plugin-dir
plugins/plugin-dev`
- [ ] I have tested the full workflow (if applicable)
- [ ] I have verified GitHub CLI integration works (if applicable)
- [ ] I have tested in a clean repository (not my development repo)

### Version Management (if applicable)

- [ ] I have updated version numbers in both `plugin.json` and
`marketplace.json` (if this is a release)
- [ ] I have updated CHANGELOG.md with relevant changes

## Screenshots (if applicable)

N/A

## Additional Notes

This is a security fix identified during code review. The vulnerability
could theoretically be exploited if an attacker could control the hook
script path passed to `test-hook.sh`.

## Reviewer Notes

**Areas that need special attention**:
- Verify the path validation regex correctly catches all dangerous shell
characters
- Verify the positional parameter approach works correctly for both
executable and non-executable scripts

**Known limitations or trade-offs**:
- The path validation is conservative and may reject some
technically-safe paths that contain these characters

---

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: sjnims

plugins/plugin-dev/skills/hook-development/scripts/test-hook.sh

@@ -141,16 +141,34 @@ if [ ! -f "$HOOK_SCRIPT" ]; then
   exit 1
 fi
 
+# Security: Validate hook script path doesn't contain dangerous characters
+# This prevents potential command injection through maliciously crafted paths
+if [[ "$HOOK_SCRIPT" =~ [\;\|\&\`\$\(\)\{\}\<\>] ]]; then
+  echo "❌ Error: Hook script path contains invalid characters"
+  echo "   Path must not contain: ; | & \` \$ ( ) { } < >"
+  exit 1
+fi
+
+# Track if we need to invoke with bash explicitly
+HOOK_IS_EXECUTABLE=true
 if [ ! -x "$HOOK_SCRIPT" ]; then
   echo "⚠️  Warning: Hook script is not executable. Attempting to run with bash..."
-  HOOK_SCRIPT="bash $HOOK_SCRIPT"
+  HOOK_IS_EXECUTABLE=false
 fi
 
 if [ ! -f "$TEST_INPUT" ]; then
   echo "❌ Error: Test input not found: $TEST_INPUT"
   exit 1
 fi
 
+# Security: Validate test input path doesn't contain dangerous characters
+# This mirrors the HOOK_SCRIPT validation for defense-in-depth
+if [[ "$TEST_INPUT" =~ [\;\|\&\`\$\(\)\{\}\<\>] ]]; then
+  echo "❌ Error: Test input path contains invalid characters"
+  echo "   Path must not contain: ; | & \` \$ ( ) { } < >"
+  exit 1
+fi
+
 # Validate test input JSON
 if ! jq empty "$TEST_INPUT" 2>/dev/null; then
   echo "❌ Error: Test input is not valid JSON"
@@ -187,7 +205,13 @@ echo ""
 start_time=$(date +%s)
 
 set +e
-output=$(timeout "$TIMEOUT" bash -c "cat '$TEST_INPUT' | $HOOK_SCRIPT" 2>&1)
+# Use proper argument passing to prevent command injection
+# Arguments are passed safely via bash -c's positional parameters
+if [ "$HOOK_IS_EXECUTABLE" = true ]; then
+  output=$(timeout "$TIMEOUT" bash -c 'cat "$1" | "$2"' -- "$TEST_INPUT" "$HOOK_SCRIPT" 2>&1)
+else
+  output=$(timeout "$TIMEOUT" bash -c 'cat "$1" | bash "$2"' -- "$TEST_INPUT" "$HOOK_SCRIPT" 2>&1)
+fi
 exit_code=$?
 set -e