Harden runner-guard Docker command rendering

[Copilot]

Sighthound identified dynamic Docker command arguments as potential command-injection sinks. The execution path already validates mount and scan-path inputs; this change makes the argument boundary explicit and secures verbose command rendering.

• Fixed argument construction

  • Centralize runner-guard Docker arguments in runnerGuardDockerArgs.
  • Reuse the exact argument slice for exec.Command.

• Safe verbose output

  • Render the copyable Docker command with shellJoinArgs rather than interpolating mount paths into a quoted string.

dockerArgs := runnerGuardDockerArgs(volumeMount, containerScanPath)
cmd := exec.Command(dockerPath, dockerArgs...)

dockerCmd := shellJoinArgs(append([]string{"docker"}, dockerArgs...))

• Coverage
  • Add a focused test confirming dynamic mount and scan-path values remain discrete arguments and are shell-escaped for display.

• Fixes [sighthound] Security findings in github/gh-aw #51695

---

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.76 AIC · ⌖ 5.55 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[Copilot]

Pull request overview

Hardens runner-guard Docker invocation and verbose command rendering.

Changes:

• Centralizes Docker arguments in runnerGuardDockerArgs.
• Reuses arguments for execution and shell-rendered output.
• Adds focused argument-boundary coverage.

Show a summary per file

File	Description
pkg/cli/runner_guard.go	Centralizes and safely renders Docker arguments.
pkg/cli/runner_guard_test.go	Tests dynamic argument preservation and quoting.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

✅ Ponytail Reviewer completed successfully!

Reviewed diff for over-engineering: extracting runnerGuardDockerArgs and using shellJoinArgs is a straightforward, minimal refactor with no speculative abstraction or unneeded complexity. Lean already. Ship.

Generated by Ponytail Reviewer for #51753

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.individual.githubcopilot.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.individual.githubcopilot.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #51753 does not have the 'implementation' label and has only 37 new lines of code in business logic directories (threshold: 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

The refactor cleanly extracts runnerGuardDockerArgs for testability and switches the verbose display to use shellJoinArgs for proper quoting. exec.Command still passes args directly to the OS (no shell), so no security regression. The new test covers spaces and flag-injection in paths. LGTM.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 16.7 AIC · ⌖ 7 AIC · ⊞ 5.4K

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — changes look well-targeted.

📋 Key Themes & Highlights

Key Themes

• Argument centralisation: runnerGuardDockerArgs cleanly unifies the exec.Command build and the verbose display path, eliminating drift.
• Test coverage: The new test verifies both argument discreteness (defence against option injection) and shell-quoting for display — good dual coverage.

One open concern

• The pre-existing shellEscapeArg implementation omits ! from its special-character set. Single-quoted strings are safe in POSIX sh, but interactive Bash with histexpand enabled will still expand ! in history-expansion contexts. The generated command is display-only (never evald), so this is low-severity, but worth noting for paths that may contain !.

Positive Highlights

• ✅ runnerGuardDockerArgs is a pure function — easy to test and reason about.
• ✅ #nosec G204 comment retained with the original rationale, keeping the security context local to the call site.
• ✅ Test uses adversarial inputs (./--help, spaces in path) that cover real injection vectors.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 28.4 AIC · ⌖ 7.45 AIC · ⊞ 7.1K
_{Comment /matt to run again}

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 100/100 — Excellent

Analyzed 1 test: 1 design, 0 implementation, 0 violation(s).

📊 Metrics (1 test)

Metric	Value
Analyzed	1 (Go: 1, JS: 0)
✅ Design	1 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	1 (100%)
Duplicate clusters	0
Inflation	No
🚨 Violations	0

Test	File	Classification	Issues
TestRunnerGuardDockerArgsPreservesDynamicValuesAsArguments	pkg/cli/runner_guard_test.go:387	design_test, behavioral_contract	None

Quality Analysis

TestRunnerGuardDockerArgsPreservesDynamicValuesAsArguments (pkg/cli/runner_guard_test.go:387) — This test verifies the security invariant that dynamic values (volume mount paths, container scan paths) are preserved as individual arguments to the Docker command, and that shell escaping works correctly for paths with spaces. The test uses require.Equal() to verify exact argument structure and require.Contains() to confirm proper shell quoting. High-value design test with edge-case coverage (spaces, special characters like --help). No violations.

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). No violations.

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 21.5 AIC · ⌖ 6.32 AIC · ⊞ 7.6K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: 30%).

[gh-aw-bot]

@copilot PR sous-chef triage: please refresh the current maintainer-facing state on the latest head, resolve any addressed unresolved review threads, refresh the branch if GitHub can update it cleanly, and run the pr-finisher skill before handing this back to maintainers.

No failed checks were listed in the compact candidate snapshot.
Run details: https://github.com/github/gh-aw/actions/runs/31384398611

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.76 AIC · ⌖ 5.55 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot PR sous-chef triage: please refresh the current maintainer-facing state on the latest head, resolve any addressed unresolved review threads, refresh the branch if GitHub can update it cleanly...

Added ! to the metacharacter set in shellEscapeArg and a dedicated test (TestRunnerGuardDockerArgsShellEscapesBangInPaths) covering ! in both host mount paths and scan paths. Fixed in the latest commit.

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.86.2