T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:747
- Finding
- Unrestricted Execution of Repository-Defined Post-Sync Hooks<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 747-752 **Vulnerability Type**: Arbitrary command execution through unvalidated configuration **Risk Level**: High ### Vulnerable Code Snippet ```markdown 3. **`post-sync hooks`** *(optional, repo-specific)* - Run custom post-sync actions - Skip if `OLD_SHA == NEW_SHA` (no upstream changes) - Hooks are defined per-repo in `config.json` under `"postSyncHooks"` (array of shell commands or descriptions) - Example: detect CHANGELOG changes, update downstream skills, trigger CI - If no hooks configured: skip this step entirely ``` ### Technical Analysis The `full-sync` workflow directs the agent to run commands obtained from the repository-specific `postSyncHooks` configuration. These hooks are described as an array of either shell commands or descriptions, but the Skill does not define: - A schema that distinguishes executable commands from descriptive text. - An allowlist of permitted commands or arguments. - Validation for shell metacharacters, command substitution, redirects, or pipelines. - A repository or configuration trust check. - A mandatory command preview and explicit user approval. - Filesystem, credential, environment-variable, or network isolation for hook execution. As a result, configuration data crosses directly into an execution channel. If an attacker can create or modify the selected `config.json`, the next `full-sync` operation can cause the agent to execute attacker-controlled commands with the same operating-system privileges and tool credentials as the agent process. Although the versioned example configurations do not contain malicious hooks, the documented execution model itself is unsafe because local configurations are mutable inputs and may be copied from untrusted sources. ### Attack Path 1. An attacker supplies a repository configuration or modifies an existing local `repos/<name>/config.json`. 2. The attacker adds a malicious command to `post ...[truncated 1303 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove support for arbitrary shell strings in `postSyncHooks`. 2. Replace hooks with structured, allowlisted actions, for example: ```json { "postSyncHooks": [ { "action": "trigger-ci", "workflow": "downstream-check.yml" } ] } ``` 3. Implement each supported action with fixed executables and argument arrays rather than a shell interpreter. 4. Validate every configuration field against a strict schema and reject unknown actions or properties. 5. Require an explicit per-run user confirmation showing the exact executable, arguments, target repository, and expected effects. 6. Treat configurations outside the Skill's trusted local directory, configurations stored in managed repositories, and newly modified configurations as untrusted. 7. If custom commands must remain supported: - Disable them in cron and unattended modes. - Never invoke them using `sh -c`, `bash -c`, `eval`, or equivalent shell parsing. - Reject redirections, pipelines, command substitutions, control operators, and environment assignments. - Run them in a restricted subprocess with a minimal environment, bounded working directory, timeout, and no unnecessary credentials. - Block network access unless the specific approved action requires it. 8. Log the approved hook definition, executable, arguments, exit status, and affected files without recording secrets. ]]>