T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/publish-to-clawhub.sh:4
- Finding
- Failed Directory Validation Can Publish an Unintended Directory## Vulnerability Details **File Location**: `scripts/publish-to-clawhub.sh`, lines 4–16 **Vulnerability Type**: Improper input validation, incorrect shell quoting, and unchecked directory change **Risk Level**: High ```bash SKILL_DIR=$1 SLUG=${2:-$(basename $SKILL_DIR)} NAME=${3:-$SLUG} VERSION=${4:-1.0.0} CHANGELOG=${5:-\"Quick publish\"} if [ -z \"$SKILL_DIR\" ]; then echo \"Usage: $0 <skill-dir> [slug] [name] [version] [changelog]\" exit 1 fi cd $SKILL_DIR clawhub publish . --slug $SLUG --name \"$NAME\" --version $VERSION --changelog \"$CHANGELOG\" ``` ### Technical Analysis The backslashes before the double quotes cause the quote characters to be treated as literal data rather than shell syntax. Consequently, the condition: ```bash [ -z \"$SKILL_DIR\" ] ``` does not safely test whether `SKILL_DIR` is empty. When the variable is empty, the tested value still contains literal quote characters, so the script can continue instead of displaying the usage message and terminating. The subsequent command uses an unquoted path: ```bash cd $SKILL_DIR ``` If `SKILL_DIR` is empty, `cd` can change to the invoking user's home directory. If the path is invalid, contains whitespace, or expands unexpectedly, `cd` can fail or target the wrong directory. Because the script neither checks the exit status nor enables fail-fast behavior, it proceeds to execute: ```bash clawhub publish . ``` This publishes whichever directory is current at that point rather than necessarily publishing the directory requested by the user. ### Attack Path 1. A user invokes the advertised publication script without an argument, with a malformed argument, or with a nonexistent directory. 2. The incorrectly quoted empty-value test fails to terminate the script. 3. `cd` changes to the user's home directory or fails while leaving the process in its original working directory. 4. The script does not inspect the ...[truncated 678 chars]
- Remediation
- ## Remediation Suggestions - Enable strict shell behavior with `set -euo pipefail`. - Test the first argument through `"${1:-}"` before assigning or using it. - Quote all path expansions and use `--` to terminate options. - Verify that the target exists and is a directory. - Abort explicitly if the directory change fails. - Consider resolving the target to a canonical path and presenting it for confirmation before publication. Example hardened implementation: ```bash #!/usr/bin/env bash set -euo pipefail if [[ -z "${1:-}" ]]; then printf 'Usage: %s <skill-dir> [slug] [name] [version] [changelog]\n' "$0" >&2 exit 1 fi SKILL_DIR=$1 if [[ ! -d "$SKILL_DIR" ]]; then printf 'Error: not a directory: %s\n' "$SKILL_DIR" >&2 exit 1 fi cd -- "$SKILL_DIR" || exit 1 ``` The script should also confirm the resolved publication directory immediately before invoking `clawhub publish`.