T09 · Insecure Skill Coding Practices
Warning
- Location
SKILL.md:34- Finding
Insufficient Validation of User-Controlled Git Revision Arguments
- Content
View full analysis
=$(git log -1 --format=%ci $PREV_TAG | cut -d' ' -f1)" --json number,title,author,labels ``` ### Technical Analysis The Skill accepts `$ARGUMENTS` as `PREV_TAG`, but it does not establish that this value is one exact, literal Git tag before using it in subsequent commands. The purported validation command is insufficient: ```bash git tag -l "$PREV_TAG" ``` `git tag -l` interprets its argument as a pattern and also supports option-like inputs. Merely obtaining nonempty output does not prove that exactly one tag exists whose name is identical to `PREV_TAG`. Although the surrounding instructions state that the output must contain exactly one matching line, this requirement is not programmatically enforced. The input is then expanded without quoting in revision expressions and a nested `git log` command: ```bash git log ${PREV_TAG}..HEAD git diff ${PREV_TAG}..HEAD git log -1 --format=%ci $PREV_TAG ``` Shell word splitting can therefore convert whitespace-containing input into multiple Git arguments. Option-like, pattern-based, malformed, or multiword values may change how Git parses the command. This can cause the workflow to inspect an unintende ...[truncated 1961 chars]- Remediation
View remediation
/dev/null 2>&1 || { echo "Invalid tag format: $PREV_TAG" exit 1 } # Verify that the exact tag reference exists. git show-ref --verify --quiet "refs/tags/$PREV_TAG" || { echo "Tag not found: $PREV_TAG" exit 1 } # Resolve and verify that the tag ultimately names a commit. PREV_COMMIT=$(git rev-parse --verify "refs/tags/$PREV_TAG^{commit}") || { echo "Tag does not resolve to a commit: $PREV_TAG" exit 1 } # Use the verified commit as one quoted revision argument. git log "$PREV_COMMIT..HEAD" --pretty=format:"%h %s" --no-merges git diff "$PREV_COMMIT..HEAD" --stat git diff "$PREV_COMMIT..HEAD" --name-only | sort | uniq MERGED_SINCE=$( git log -1 --format=%ci "$PREV_COMMIT" | cut -d' ' -f1 ) || exit 1 gh pr list \ --state merged \ --search "merged:>=$MERGED_SINCE" \ --json number,title,author,labels ``` Additional hardening measures: 1. Require exactly one Skill argument rather than assigning an unrestricted argument string to `PREV_TAG`. 2. Avoid `git tag -l` as an existence check; use exact reference verification with `git show-ref --verify`. 3. Quote every variable expansion passed to Git. 4. Resolve the validated tag once and reuse the resulting immutable commit ID. 5. Check the exit status of every Git and GitHub CLI command before using its output. 6. Preserve the documented fallback behavior when `gh pr list` fails or returns invalid JSON. ]]>