T09 · Insecure Skill Coding Practices
Warning
- Location
- validators.md:38
- Finding
- Secret-scanning pre-commit validator can be bypassed with crafted filenames## Vulnerability Details **File Location**: `validators.md`, lines 38-49 **Vulnerability Type**: Incomplete and unsafe shell filename handling **Risk Level**: Medium ### Vulnerable Code ```bash # Check staged files for secrets STAGED=$(git diff --cached --name-only) for file in $STAGED; do if grep -qE '(api_key|password|secret)\s*=' "$file" 2>/dev/null; then echo "❌ BLOCKED: Potential secret in $file" echo "Rule: no-secrets-in-code (INC-002)" exit 1 fi done exit 0 ``` ### Technical Analysis The example processes the newline-delimited output of `git diff --cached --name-only` through unquoted shell word splitting: ```bash for file in $STAGED ``` Git filenames may legally contain spaces, tabs, newlines, wildcard characters, and leading hyphens. Consequently: - A filename containing whitespace is split into multiple nonexistent paths. - A filename containing a newline can alter the apparent list of files. - Shell wildcard characters can undergo pathname expansion. - A leading-hyphen filename may be interpreted by `grep` as an option because no `--` option terminator is used. - Errors are hidden by `2>/dev/null`, and failure to inspect a file does not block the commit. The validator therefore fails open: unreadable or incorrectly parsed staged files are treated as safe. Although presented as an example, the document explicitly directs users to install such validators as Git hooks, making this an insecure implementation pattern rather than merely illustrative pseudocode. ### Attack Path 1. An attacker who can contribute repository content creates a file whose name contains whitespace, a newline, or another shell-sensitive character. 2. The attacker places a value matching the secret pattern in that file, such as `api_key = ...`. 3. The crafted file is staged for commit. 4. `git diff --cached --name-only` returns the filename, but command substitution and `for file in $STAGED ...[truncated 594 chars]
- Remediation
- ## Remediation Suggestions Use Git's NUL-delimited output and consume it without command substitution or word splitting: ```bash while IFS= read -r -d '' file; do if [[ ! -f "$file" ]]; then echo "❌ BLOCKED: Cannot safely inspect staged path: $file" exit 1 fi if grep -qE -- '(api_key|password|secret)[[:space:]]*=' "$file"; then echo "❌ BLOCKED: Potential secret in $file" exit 1 fi done < <(git diff --cached --name-only -z --diff-filter=ACMR) ``` Additional hardening should include: - Scan staged blob contents rather than working-tree files, so the validator checks exactly what will be committed. - Use `--` before all externally derived path arguments. - Treat inspection errors as blocking failures. - Test filenames containing spaces, tabs, newlines, glob characters, Unicode, and leading hyphens. - Supplement pattern matching with a maintained secret-scanning tool where strong assurance is required.