Skip to content

[vulnhunter] VulnHunter findings in github/gh-aw #48310

Description

@github-actions

Summary

A VulnHunter methodology scan (injection + secrets phases) of the github/gh-aw Go CLI found the codebase to be well-hardened overall: command/argument-injection sinks are consistently guarded (ValidateGitRef/ValidateGitPath reject leading - and .., -- end-of-options separators, no sh -c string interpolation), tar extraction enforces filepath.IsLocal, and SSRF is prevented because attacker-controlled owner/repo reach only the URL path, never the host.

Two findings survived falsification. The higher-confidence one is a local secret-exposure via subprocess argv; the second is a defense-in-depth path-traversal gap on @include writes.

Finding 1 — Secret material passed on subprocess argv (CWE-214, High)

Freshly-entered credentials (fine-grained PATs, Anthropic/OpenAI API keys, git PATs) are passed as the --body argument to gh secret set, making them visible in the process table for the lifetime of the child process.

  • pkg/cli/engine_secrets.go:574RunGHCombined("Setting secret...", "secret", "set", secretName, "--repo", repoSlug, "--body", secretValue)
  • pkg/cli/add_interactive_secrets.go:53 — same "secret", "set", name, "--repo", ..., "--body", value pattern

Attacker path / preconditions: On a shared CI runner or multi-user developer host, any local user or unprivileged process that can read /proc/<pid>/cmdline (or run ps auxww) during the gh secret set invocation captures the plaintext secret. gh does not redact its own argv.

Why credible after falsification: The value is genuine credential material — it flows from the interactive prompts promptForCopilotPATUnified / promptForSystemTokenUnified / promptForGenericAPIKeyUnified (engine_secrets.go:394/472/524) and from os.Getenv(req.Name) via ensureSecretAvailable, straight into the argv slot with no indirection. Unlike other secret paths in the repo (which emit only ${{ secrets.NAME }} references), this one carries the resolved value.

Remediation: Pass the value over stdin instead of argv. The repo already has RunGHInputContext (pkg/workflow/github_cli.go:179, which sets cmd.Stdin); invoke gh secret set NAME --repo R --body - (or omit --body) so gh reads the secret from stdin. Alternatively use the existing NaCl-encrypted REST path (setRepoSecret). Both eliminate argv exposure.

Falsified / not reported (secrets phase)
  • Debug loggers (secretsLog.Printf) print secret names/expressions (secrets.FOO), never resolved values; SecretInfo.Value is stored but not logged.
  • uploadSecretToRepo error wraps command output, but gh secret set does not echo the body, and SanitizeErrorMessage redaction applies — no confirmed leak.
  • Generated YAML emits secrets only as ${{ secrets.NAME }} refs or ${VAR} bash indirection (ReplaceSecretsWithBashVars), never inlined literals.
  • stdio log files are pre-created with umask 177 (0600); no 0644/0666 secret writes found.

Finding 2 — Missing path-boundary guard on @include writes (CWE-22, Medium, PLAUSIBLE)

fetchAndSaveRemoteIncludes writes a file whose path is derived from an attacker-controlled @include <path> directive in a remote markdown body, without the traversal guard its sibling writers apply.

  • Parse: pkg/cli/includes.go:449/455 — the @include regex captures an arbitrary path.
  • Sink: pkg/cli/includes.go:494targetPath = filepath.Join(targetDir, filePath), written at :515 via os.WriteFile.

Why credible: The two analogous writers both enforce a boundary — frontmatter imports reject ../ at includes.go:312 and re-verify with filepath.Rel at :357; resources do the same at resources.go:113/:149. fetchAndSaveRemoteIncludes performs neither check and never calls fileutil.ValidatePathWithinBase. When a developer runs gh aw add <malicious-repo>, a crafted @include can direct the write outside targetDir.

Attacker path / preconditions: Requires the malicious include path to first fetch successfully. A fully-escaping path (../../../../tmp/evil.md) is likely rejected by GitHub's Contents API (404 → no write), which is why this is rated PLAUSIBLE rather than CONFIRMED. However, partial traversal that stays inside the source repo but resolves outside the local targetDir (e.g. @include ../secrets/x.md when the source repo has .github/secrets/x.md) can write into sibling directories such as .github/. The raw.githubusercontent.com fallback (remote_download_file.go:351) does not pre-Clean the path, so CDN normalization is the residual risk worth closing.

Remediation: After computing targetPath, call fileutil.ValidatePathWithinBase(targetDir, targetPath) (or replicate the imports ../ + filepath.Rel check) before os.MkdirAll/os.WriteFile, matching the sibling writers.

Hardening note (not a finding)

The git ls-tree pathspecs at pkg/parser/remote_list_files.go:240/358/497/579 lack a ValidateGitPath call, so a malicious upstream manifest dir beginning with - reaches git as e.g. -x/. Impact is bounded to a failed listing (DoS) — git ls-tree has no argument that writes files or executes code — so this is a consistency/defense-in-depth improvement, not a shippable vulnerability. Adding ValidateGitPath there and ValidateGitRef in resolveListRepoCloneConfig (remote_list_files.go:47) is recommended for uniformity.

Scope

Injection and secrets phases applied to the highest-risk entry points: remote workflow/import download & git fallbacks (pkg/parser/remote_*.go, pkg/gitutil), secret prompting/upload (pkg/cli/engine_secrets.go, add_interactive_secrets.go), and local write paths (pkg/cli/includes.go, resources.go, fileutil). Test code, testdata, and vendored code were excluded per methodology.

Generated by 🛡️ Daily VulnHunter Scan · sonnet46 · 881.4 AIC · ⌖ 41.3 AIC · ⊞ 4.1K ·

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions