T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/atomgit-batch.ps1.txt:14
- Finding
- Dot-Sourcing the Batch Script Automatically Approves Hard-Coded Pull Requests<![CDATA[ ## Vulnerability Details **File Location**: `scripts/atomgit-batch.ps1.txt:14-16` and `scripts/atomgit-batch.ps1.txt:366-368` **Vulnerability Type**: Unsafe top-level execution with destructive defaults **Risk Level**: High ### Vulnerable Code ```powershell [Parameter(Mandatory=$false)] [int[]]$PRs = @(2557, 2558, 2560), ``` ```powershell # If the script is run directly, execute the default operation if ($PRs.Count -gt 0) { Invoke-BatchApprove -Owner $Owner -Repo $Repo -PRs $PRs -Parallel:$Parallel.IsPresent -MaxConcurrency $MaxConcurrency } ``` The documented loading instructions explicitly tell users to dot-source the script: ```powershell . ~/.openclaw/workspace/skills/atomgit-powershell/scripts/atomgit-batch.ps1 ``` Relevant documentation locations include `SKILL.md:163-166`, `README.md:76-79`, and `commands.md:120-123`. ### Technical Analysis Dot-sourcing a PowerShell script executes all top-level statements in addition to importing its functions. The script assigns a non-empty default value to `$PRs` and unconditionally calls `Invoke-BatchApprove` whenever `$PRs.Count` is greater than zero. Consequently, the condition described by the comment as distinguishing direct execution does not actually distinguish direct execution from dot-sourcing. Loading the script according to its documentation immediately initiates authenticated write operations against the default repository and PR numbers. The invoked function posts `/lgtm` and `/approve` comments using the user's AtomGit bearer token. These are security-sensitive repository actions, not harmless initialization behavior. ### Attack Path 1. A user configures an `ATOMGIT_TOKEN` with permission to comment on or approve pull requests. 2. The user follows the documented instruction and dot-sources `atomgit-batch.ps1`. 3. PowerShell executes the script's top-level statements. 4. The default `$PRs` value contains PRs `2557`, `2558`, and `2560`. 5. The final condition evaluates to true and ...[truncated 761 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Change the default PR list to an empty array: ```powershell [int[]]$PRs = @() ``` 2. Remove all authenticated or state-changing operations from top-level script execution. Dot-sourcing should only define functions and constants. 3. If direct execution must be supported, implement a reliable direct-execution entry point rather than relying on `$PRs.Count`. 4. Require explicit PR identifiers for every invocation. 5. Add confirmation for approval operations using PowerShell's `SupportsShouldProcess` and `ShouldProcess`, with an explicit `-Confirm:$false` or `-Force` option only for intentional automation. 6. Add a dry-run mode that lists the repository, PRs, and operations before making requests. 7. Add tests verifying that dot-sourcing the script produces no network requests and no repository changes. ]]>