Add --runner-guard to compile; wire into static-analysis-report workflow#25281
Add --runner-guard to compile; wire into static-analysis-report workflow#25281
Conversation
…port workflow - Add RunnerGuardImage constant (ghcr.io/vigilant-llc/runner-guard:v3.0.1) - Add RunnerGuard bool to CompileConfig - Create runner_guard.go with Docker-based implementation (mirrors poutine.go) - Update CheckAndPrepareDockerImages to accept useRunnerGuard param - Add runBatchRunnerGuard/RunRunnerGuardOnDirectory to compile_batch_operations - Wire runner-guard into compile_pipeline.go (both compileSpecificFiles and compileAllFilesInDirectory) - Fix lockFilesForDirTools to allow --poutine/--runner-guard without --zizmor - Add --runner-guard flag to cmd/gh-aw/main.go - Add runner-guard field to MCP compile tool schema - Update static-analysis-report.md: use Docker for runner_guard job and add --runner-guard to compile step Agent-Logs-Url: https://github.com/github/gh-aw/sessions/40d94ea5-12ef-45f4-8336-71b9904b8ff4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds runner-guard as an additional static analysis tool in the gh aw compile pipeline and wires it into the repository’s static analysis reporting workflow.
Changes:
- Introduces
--runner-guardflag /CompileConfig.RunnerGuardand runs runner-guard via Docker during compile. - Updates Docker image preparation logic to include the runner-guard image.
- Updates the
static-analysis-reportworkflow to run runner-guard via Docker and include it in the compile step.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/runner_guard.go | New runner-guard Docker runner + JSON parsing and console reporting of findings |
| pkg/cli/mcp_tools_readonly.go | Adds runner-guard arg to MCP compile tool schema and execution |
| pkg/cli/docker_images.go | Adds RunnerGuardImage and extends image preparation checks |
| pkg/cli/docker_images_test.go | Updates tests for new signature + constant assertion |
| pkg/cli/compile_pipeline.go | Fixes dir-tool lockfile collection + runs runner-guard in batch mode |
| pkg/cli/compile_config.go | Adds RunnerGuard to compile configuration |
| pkg/cli/compile_batch_operations.go | Adds batch runner-guard directory operation |
| cmd/gh-aw/main.go | Wires --runner-guard flag into CLI compile command |
| .github/workflows/static-analysis-report.md | Switches runner-guard job to Docker image and includes runner-guard in compile invocation |
| .github/workflows/static-analysis-report.lock.yml | Regenerated lockfile reflecting workflow changes |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 5
| if errors.As(err, &exitErr) { | ||
| exitCode := exitErr.ExitCode() | ||
| runnerGuardLog.Printf("runner-guard exited with code %d (findings=%d)", exitCode, totalFindings) | ||
| // Exit code 1 typically indicates findings in the repository | ||
| if exitCode == 1 { | ||
| if strict && totalFindings > 0 { | ||
| return fmt.Errorf("strict mode: runner-guard found %d security findings - workflows must have no runner-guard findings in strict mode", totalFindings) | ||
| } | ||
| // In non-strict mode, findings are logged but not treated as errors | ||
| return nil | ||
| } |
There was a problem hiding this comment.
In strict mode, an exit code of 1 is treated as “findings present”, but the strict failure condition currently depends on totalFindings from JSON parsing. If parsing fails (parseErr != nil), totalFindings will be 0 and strict mode will incorrectly pass even though runner-guard indicated findings via exit code. Consider treating exit code 1 as a strict-mode failure whenever strict is enabled (optionally include parseErr/stdout context in the error), regardless of whether JSON parsing succeeded.
pkg/cli/runner_guard.go
Outdated
|
|
||
| // Check if the resolved path is within gitRoot to prevent path traversal | ||
| relPath, err := filepath.Rel(absGitRoot, absPath) | ||
| if err != nil || strings.HasPrefix(relPath, "..") { |
There was a problem hiding this comment.
The path traversal guard uses strings.HasPrefix(relPath, ".."), which can incorrectly reject legitimate paths like "..foo" that are still within the repo. Prefer checking relPath == ".." or strings.HasPrefix(relPath, ".."+string(filepath.Separator)) (and the equivalent for the OS separator) to distinguish real parent traversal from filenames starting with dots.
| if err != nil || strings.HasPrefix(relPath, "..") { | |
| if err != nil || relPath == ".." || strings.HasPrefix(relPath, ".."+string(filepath.Separator)) { |
| // runRunnerGuardOnDirectory runs the runner-guard taint analysis scanner on a directory | ||
| // containing workflows using the Docker image. | ||
| func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) error { | ||
| runnerGuardLog.Printf("Running runner-guard taint analysis on directory: %s", workflowDir) | ||
|
|
There was a problem hiding this comment.
runRunnerGuardOnDirectory takes workflowDir and the comment/log message imply the scan is scoped to that directory, but the Docker invocation always runs scan . from the repo root (/workdir). If the intent is to scan only the compiled workflows directory, pass workflowDir (relative to /workdir) to runner-guard (or set the container working directory to workflowDir) so the behavior matches the function contract.
| // parseAndDisplayRunnerGuardOutput parses runner-guard JSON output and displays findings. | ||
| // Returns the total number of findings found. | ||
| func parseAndDisplayRunnerGuardOutput(stdout string, verbose bool, gitRoot string) (int, error) { | ||
| if stdout == "" { | ||
| return 0, nil // No output means no findings | ||
| } | ||
|
|
||
| trimmed := strings.TrimSpace(stdout) | ||
| if !strings.HasPrefix(trimmed, "{") && !strings.HasPrefix(trimmed, "[") { | ||
| if len(trimmed) > 0 { | ||
| return 0, fmt.Errorf("unexpected runner-guard output format: %s", trimmed) | ||
| } | ||
| return 0, nil | ||
| } | ||
|
|
||
| var output runnerGuardOutput | ||
| if err := json.Unmarshal([]byte(stdout), &output); err != nil { | ||
| return 0, fmt.Errorf("failed to parse runner-guard JSON output: %w", err) | ||
| } |
There was a problem hiding this comment.
New runner-guard parsing/formatting logic is introduced here, but there are no unit tests covering JSON parsing, severity→errorType mapping, and path sanitization behavior. There are existing tests for similar logic in pkg/cli/poutine_test.go; adding analogous tests for runner-guard would help prevent regressions and ensure strict/non-strict behaviors remain correct.
| SetMockImageAvailable(ActionlintImage, false) | ||
|
|
||
| // Simulate multiple images already downloading | ||
| SetDockerImageDownloading(ZizmorImage, true) | ||
| SetDockerImageDownloading(PoutineImage, true) | ||
|
|
||
| // Request all tools | ||
| err := CheckAndPrepareDockerImages(context.Background(), true, true, true) | ||
| err := CheckAndPrepareDockerImages(context.Background(), true, true, true, false) | ||
| if err == nil { |
There was a problem hiding this comment.
CheckAndPrepareDockerImages gained a new useRunnerGuard parameter, but the tests currently only exercise it as false. Add at least one test case where useRunnerGuard is true (e.g., image downloading/available, Docker unavailable) to ensure runner-guard is included in the missing/downloading lists and error messages as intended.
See below for a potential fix:
SetMockImageAvailable(RunnerGuardImage, false)
// Simulate multiple images already downloading
SetDockerImageDownloading(ZizmorImage, true)
SetDockerImageDownloading(PoutineImage, true)
SetDockerImageDownloading(RunnerGuardImage, true)
// Request all tools, including runner-guard
err := CheckAndPrepareDockerImages(context.Background(), true, true, true, true)
if err == nil {
t.Error("Expected error when images are downloading, got nil")
}
// Error should mention downloading images and runner-guard
if err != nil {
errMsg := err.Error()
if !strings.Contains(errMsg, "downloading") && !strings.Contains(errMsg, "retry") {
t.Errorf("Expected error to mention downloading and retry, got: %s", errMsg)
}
if !strings.Contains(errMsg, RunnerGuardImage) {
t.Errorf("Expected error to mention runner-guard image %q, got: %s", RunnerGuardImage, errMsg)
}
|
@copilot merge main and recompile |
|
@copilot review comments |
- Resolve conflict in mcp_tools_readonly.go: use DockerUnavailableError from main while preserving RunnerGuard support from this branch - Update docker_images_test.go for new test from main (5-param CheckAndPrepareDockerImages) - Recompile all 182 workflow lock files Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2051d7b9-f534-443a-8db5-11384d63688a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 7d7cf8a. Merged main, resolved the conflict in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main, fix conflicts and recompile |
- Fix strict mode: exit code 1 fails even when JSON parsing fails
- Fix path traversal guard: use == ".." and HasPrefix("../") to avoid
rejecting legitimate "..foo" paths within the repo
- Fix Docker scan scope: pass workflowDir relative to gitRoot as scan
target instead of always scanning "." (the full repo root)
- Add runner_guard_test.go: JSON parsing, severity→errorType mapping,
path sanitization behavior
- Add docker_images_test.go test for useRunnerGuard=true (image downloading)
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7ced1697-6262-4a36-8d0d-c66db06a819a
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All review comments addressed in af745ca:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 7a02008. Resolved conflicts in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
@copilot remove this job and integrate runner guard with other security tools test
There was a problem hiding this comment.
Done in 97f9a6e. Removed the separate runner_guard job and the artifact download step. Runner-guard now runs via --runner-guard in the existing compile step alongside zizmor, poutine, and actionlint, with findings in /tmp/gh-aw/compile-output.txt. Updated Phase 6 to reference that file instead of the removed artifact.
…e step Agent-Logs-Url: https://github.com/github/gh-aw/sessions/1d8fbd2e-38f8-4fd4-8038-9dfd1918ddd0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot validate docker image name https://github.com/github/gh-aw/actions/runs/24136991460/job/70427760523#step:13:1 |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2edba045-be10-410f-b268-53f477f5e640 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 5d375e4. The CI was failing with Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Hey The contribution looks well-structured and ready for maintainer review. Here's a quick summary of what looks great:
This PR looks aligned with the project's contribution guidelines and quality standards. 🎉
|
…se-older-issues Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a46b04a-8d4d-42ef-a7e4-5b8314a7ed36 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in c9d2942. Changes in
|
Adds runner-guard (
ghcr.io/vigilant-llc/runner-guard:latestDocker image) as a fourth static analysis tool alongside zizmor, poutine, and actionlint.CLI
--runner-guardflag added togh aw compile— runsghcr.io/vigilant-llc/runner-guard:latestvia Docker on generated.lock.ymlfilesRunnerGuard booladded toCompileConfigand the MCP compile tool schemaCheckAndPrepareDockerImagesupdated to handle runner-guard image pre-pull/availability checks (4th parameter)Compile pipeline
runner_guard.go— new file mirroringpoutine.go; runs Docker, parses runner-guard JSON (RGS-XXX findings), displays viaconsole.CompilerErrorrunBatchRunnerGuard/RunRunnerGuardOnDirectoryadded tocompile_batch_operations.gocompileSpecificFilesandcompileAllFilesInDirectory--poutineor--runner-guardwithout--zizmorpreviously silently skipped (both relied onlockFilesForZizmorbeing non-empty). Replaced withlockFilesForDirTools, populated whenever any directory-scoped tool is requested...foowhile still rejecting real parent traversalStatic analysis workflow
static-analysis-report.md: removed the separaterunner_guardjob — runner-guard now runs via--runner-guardin the existing compile step alongside zizmor, poutine, and actionlint/tmp/gh-aw/compile-output.txtlatesttag)[static-analysis]) instead of a discussion, withclose-older-issues: trueso each daily run replaces the previous report issue[static-analysis]title prefixTests
runner_guard_test.go: covers JSON parsing, severity→errorType mappings, score/grade display, line-0 defaulting, multiple findings, empty/invalid JSON, and path sanitizationdocker_images_test.go: addedTestCheckAndPrepareDockerImages_RunnerGuardImageDownloadingwithuseRunnerGuard=true