T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/coverage-check.sh:61
- Finding
- Coverage Gate Can Falsely Certify Unreviewed Files## Vulnerability Details **File Location**: `scripts/coverage-check.sh:61-80` **Vulnerability Type**: Inexact file identity validation **Risk Level**: Medium ```bash REVIEWED_FILES=$(grep -oE "[a-zA-Z0-9_/-]+\.java" "$REVIEWED_FILE" | sort -u) REVIEWED_COUNT=$(echo "$REVIEWED_FILES" | grep -v "^$" | wc -l) MISSED_FILES="" MISSED_COUNT=0 while IFS= read -r actual_file; do if [[ -z "$actual_file" ]]; then continue fi filename=$(basename "$actual_file") if ! echo "$REVIEWED_FILES" | grep -q "$filename"; then MISSED_FILES="$MISSED_FILES$actual_file\n" ((MISSED_COUNT++)) fi done <<< "$ACTUAL_FILES" ``` ### Technical Analysis The coverage gate extracts reviewed paths but reduces every actual project file to its basename before testing membership. It then passes that basename to `grep -q` as an unanchored regular expression. This creates several integrity problems: - If different modules contain files with the same basename, reviewing one file can cause every same-named file to be treated as reviewed. - A basename may match a longer reviewed path or filename because the comparison is not anchored. - Regex metacharacters in filenames are not escaped. In particular, the period before `java` is interpreted as a wildcard during the membership check. - The manifest extraction expression excludes some valid path characters, which can further distort file identities. This violates the Skill's declared requirement to compare the review manifest against the exact file list and to prevent progression until coverage reaches 100%. ### Attack Path 1. A target repository contains security-relevant files with duplicate basenames in separate modules, such as `module-a/src/UserService.java` and `module-b/src/UserService.java`. 2. Only one occurrence is included in the reviewed-file manifest. 3. The script processes each actual file but converts its path to `Use ...[truncated 795 chars]
- Remediation
- ## Remediation Suggestions - Compare canonical project-relative paths rather than basenames. - Use fixed-string, exact-line comparison instead of regular-expression matching, such as `grep -Fqx`. - Prefer generating two sorted files of normalized relative paths and comparing them with `comm`. - Use NUL-delimited traversal and storage where possible to support spaces, newlines, and other valid filename characters. - Reject reviewed-manifest paths that resolve outside the target project. - Add regression tests for duplicate basenames, regex metacharacters, spaces, nested modules, and prefix or suffix collisions. - Require the exact set difference between actual and reviewed relative paths to be empty before reporting 100% coverage.