[vulnhunter] VulnHunter findings in github/gh-aw
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
### Overview
Single-agent VulnHunter scan (Phase 2: Injection classes) over a 40-file, risk-ranked candidate scope. **1 confirmed finding** (CWE-88 argument injection, High severity) surfaced across 2 independently reachable CLI entry points, plus one systemic root cause. All other reviewed candidates were falsified as false positives or showed adequate existing defenses.
### Key issue
**CWE-88 Argument Injection via unvalidated git branch/ref names passed to `git checkout`/`git fetch`/`git pull`/`git push`** — `pkg/cli/git.go`'s `switchBranch`, `createAndSwitchBranch`, and `pushBranch` accept a caller-supplied branch name and pass it directly as a positional `exec.Command` argument, with **no validation** that it doesn't begin with `-`. The same file already contains and correctly applies a validator (`isSafeGitRevisionArg`) in a sibling function (`populateUnpushedWorkflowStatus`) — the defense exists in the codebase, it's just not applied consistently at these three call sites.
This was **empirically verified**, not just theorized: in an isolated throwaway local repo (no live remote, no credentials), running
```
git fetch origin '--upload-pack=touch /tmp/PWNED_TEST; true'
```
caused git to execute the injected command — proving the CLI's option parser reinterprets a value in the ref/positional slot as `--upload-pack=` and runs it. (This affects ssh/git/local transports; `--upload-pack` is not honored over `https`.)
Full finding detail, reachable call sites, and falsified candidates
#### Root cause — `pkg/cli/git.go`
```go
func createAndSwitchBranch(branchName string, verbose bool) error {
cmd := exec.Command("git", "checkout", "-b", branchName) // unvalidated
}
func switchBranch(branchName string, verbose bool) error {
cmd := exec.Command("git", "checkout", branchName) // unvalidated
}
func pushBranch(branchName string, verbose bool) error {
cmd := exec.Command("git", "push", "-u", "origin", branchName) // unvalidated
}
```
Compare with the file's own existing pattern that *is* safe:
```go
if !isSafeGitRevisionArg(upstream) {
return status, fmt.Errorf("unexpected upstream ref %q", upstream)
}
cmd = exec.Command("git", "-C", gitRoot, "log", "--oneline", "HEAD", "--not", upstream, "--", relPath)
```
#### Reachable call sites (attacker-controlled value → sink)
1. **`pkg/cli/add_interactive_git.go:updateLocalBranch()`** (~lines 248-312), reached via `gh aw add --repo `.
- `defaultBranch` is sourced from `gh repo view --repo c.RepoOverride --json defaultBranchRef` (or a `git ls-remote --symref origin HEAD` fallback) for whatever repository `--repo` targets — which can be **any** repository, not necessarily one the operator administers.
- Flows unvalidated into `exec.Command("git","fetch","origin",defaultBranch)`, `exec.Command("git","pull","origin",defaultBranch)`, and `switchBranch(defaultBranch)` → `git checkout `.
- Attacker model: anyone who controls the default-branch name of a repository that a victim points `gh aw add --repo` at (e.g. a public "workflow template" repo) can achieve local command execution on the victim's machine.
2. **`pkg/cli/pr_command.go:checkoutUpdatedDefaultBranch()`** (~lines 273-296), reached via `gh aw pr transfer --repo `.
- `defaultBranch` is sourced from `gh api /repos/{targetOwner}/{targetRepo} --jq .default_branch`.
- Flows unvalidated into `exec.Command("git","checkout",defaultBranch)` and `exec.Command("git","pull","origin",defaultBranch)`.
- Weaker attacker model than (1): this tool is documented as a trial→production repo transfer utility, so the target repo is typically org-controlled — still a cross-trust gap if a less-trusted collaborator or compromised account can rename that repo's default branch.
#### Why this is credible after falsification
- The exploit mechanism was proven locally (file created via `--upload-pack=`), not assumed.
- The codebase demonstrably knows this exact injection class matters: `isSafeGitRevisionArg` (git.go), `gitutil.ValidateGitRef` (`pkg/parser/remote_resolve_sha.go`, guarding an equivalent `git ls-remote` sink), and `buildSafeGitShowObjectArg`/`isSafeGitRevisionArg`/`isSafeGitTreePath` (`pkg/cli/experiments_git_safety.go`, correctly guarding `git show` sinks in `experiments_fetch.go`/`experiments_state.go` — confirmed safe, false positive). The gap is inconsistent application, not lack of awareness.
- `pkg/workflow/name_validation.go` separately guards npm/pip package names against the same leading-`-` class but explicitly notes that context is low-risk because it only affects the *developer's own* local invocation choices. The git-remote-controlled default-branch-name case is materially different: the value crosses a trust boundary (a remote repository's owner, not the operator, controls it).
#### Remediation
Apply the existing `isSafeGitRevisionArg` (or `gitutil.ValidateGitRef`) check to `branchName`/`defaultBranch` before use in `switchBranch`, `createAndSwitchBranch`, `pushBranch` (git.go), and directly in `checkoutUpdatedDefaultBranch` (pr_command.go) and `updateLocalBranch` (add_interactive_git.go) — reject any value starting with `-`, matching the pattern already used in `populateUnpushedWorkflowStatus`.
#### Other candidates reviewed — no finding
- `actions/setup/js/validate_secrets.cjs`: `exec()` call uses a static string literal, no interpolation — false positive.
- `actions/setup/js/apply_samples.cjs`, `artifact_client.cjs`, `send_otlp_span.cjs` (partial read): array-arg `spawn`/`spawnSync` throughout; path traversal guarded via `path.basename()` / explicit rejection of `..`/absolute paths before zip creation.
- `pkg/cli/grype.go`, `runner_guard.go`, `poutine.go`, `yamllint.go`: docker/exec calls have explicit allow-list validation comments and array-arg invocation.
- `pkg/cli/dispatch.go`: `workflowPaths()` explicitly checks `filepath.Rel(...)` and refuses writes outside the target directory; the `.yml` fallback path reuses the same pre-validated name.
#### Scan coverage note
Budget did not allow full reads of every one of the 40 ranked candidates; the remainder were grep-triaged for the same dangerous-sink patterns (`exec.Command`, `filepath.Join`, `http.NewRequest`, template execution, SQL string-building) with no additional high-severity leads surfacing. Full notes saved to the scan's `out/findings.md`.
### Next actions
- Add the `isSafeGitRevisionArg` guard to `switchBranch`, `createAndSwitchBranch`, `pushBranch` in `pkg/cli/git.go`.
- Add the same guard to `defaultBranch` in `pkg/cli/add_interactive_git.go:updateLocalBranch()` and `pkg/cli/pr_command.go:checkoutUpdatedDefaultBranch()` before it reaches any `exec.Command`.
- Consider a lint/test rule (the repo already has an eslint rule pattern for JS exec safety — `eslint-factory/src/rules/require-sync-exec-timeout.ts`) to catch future `exec.Command("git", ..., unvalidatedVar)` call sites lacking this guard.
**References:**
- Workflow run: [§34191703077](https://github.com/github/gh-aw/actions/runs/34191703077)
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🛡️ Daily VulnHunter Scan](https://github.com/github/gh-aw/actions/runs/34191703077) · claude · sonnet50 · 320.8 AIC · ⌖ 39.4 AIC · ⊞ 5.9K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fdaily-vulnhunter-scan%22&type=issues)
Contributor guide
Research direction
Start in pkg/cli/git.go with isSafeGitRevisionArg and the switchBranch, createAndSwitchBranch, and pushBranch call sites. Then trace defaultBranch through updateLocalBranch in pkg/cli/add_interactive_git.go and checkoutUpdatedDefaultBranch in pkg/cli/pr_command.go. Done means all listed remote-derived branch values are validated consistently before git commands run, with tests added or updated for the affected entry points.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, github, go
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100