github / github/gh-aw

[vulnhunter] VulnHunter findings in github/gh-aw

Open
#59,381 116 comments 0 reactions 0 assignees View on GitHub
cookie security vulnhunter
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.