T09 · Insecure Skill Coding Practices
- Location
scripts/coverage-check.sh:61- Finding
Coverage Gate Can Falsely Certify Unreviewed Files
- Content
View full analysis
Vulnerability Details
File Location:
scripts/coverage-check.sh:61-80
Vulnerability Type: Inexact file identity validation
Risk Level: Mediumbash 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 -qas 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
javais 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
- A target repository contains security-relevant files with duplicate basenames in separate modules, such as
module-a/src/UserService.javaandmodule-b/src/UserService.java. - Only one occurrence is included in the reviewed-file manifest.
- The script processes each actual file but converts its path to `Use ...[truncated 795 chars]
- Remediation
View 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.
