Filter Runner-Guard RGS-012 false positives for local Copilot allow-tools

docs/adr/54341-filter-runner-guard-rgs012-false-positives-copilot-allow-tools.md

@@ -0,0 +1,46 @@
+# ADR-54341: Filter Runner-Guard RGS-012 False Positives for Copilot Local Allow-Tool Declarations
+
+**Date**: 2026-08-20
+**Status**: Draft
+**Deciders**: pelikhan (via Copilot SWE agent)
+
+---
+
+### Context
+
+Runner-Guard enforces rule RGS-012, which flags workflow lines that reference `curl` with a local-host URL as potential secret exfiltration attempts. The Copilot CLI workflow compiler generates YAML comment headers (e.g., `# --allow-tool shell(curl http://localhost:*)`) and `--allow-tool` argument lines documenting the tools the agent is permitted to use. These declarations are never executed by the shell — they are documentation artifacts — but Runner-Guard cannot distinguish them from executable `curl` commands. This causes systematic false-positive RGS-012 findings that block legitimate Copilot-enabled workflows from passing the Runner-Guard gate. The fix must suppress only the false positives while preserving all genuine exfiltration signals.
+
+### Decision
+
+We will add a targeted post-processing filter, `filterCopilotLocalAllowToolFindings`, in the Runner-Guard output pipeline. The filter suppresses RGS-012 findings that point at lines within a Copilot CLI execution step whose `--allow-tool` declarations reference only loopback hosts (`localhost`, `127.0.0.1`, `::1`, `host.docker.internal`). It preserves findings on all other lines, on steps that mix local and non-local targets, and on any other rule.
+
+Suppression is additionally refused whenever `curl` appears outside of a `shell(...)` allow-tool value — either on the finding's own line or anywhere in the executable body of the Copilot step — so a real outbound request can never be hidden by a neighbouring allow-tool declaration. Host extraction handles bracketed and bare IPv6 literals (`http://[::1]`, `http://[::1]:4321`, `::1`) so loopback targets are recognized with or without an explicit port.
+
+### Alternatives Considered
+
+#### Alternative 1: Patch the Runner-Guard RGS-012 rule to understand Copilot allow-tool syntax
+
+Runner-Guard is a separate tool maintained outside this repository. Modifying its rule logic would require upstream changes and cross-team coordination. A local post-processing filter can be shipped independently without blocking on upstream changes, making it the lower-friction path to resolution.
+
+#### Alternative 2: Add a blanket suppression for all RGS-012 findings on local-curl targets
+
+Suppressing every local-curl RGS-012 finding would hide genuine exfiltration attempts such as `curl http://localhost:4321/collect -d "secret=$SECRET_TOKEN"` inside executable `run:` blocks. The PR's safety requirement explicitly preserves those findings; a blanket suppression is therefore ruled out.
+
+### Consequences
+
+#### Positive
+- Eliminates systematic false-positive noise for Copilot CLI workflows that include local `--allow-tool` declarations.
+- Preserves security signal: executable `curl` commands to local endpoints with payloads remain flagged by RGS-012.
+- Steps that mix local and non-local allow-tool targets are intentionally not suppressed, keeping the safety boundary strict.
+
+#### Negative
+- The filter depends on the Copilot CLI's generated comment format (`# Copilot CLI tool arguments`) and step name (`Execute GitHub Copilot CLI`). If either changes, the filter silently stops suppressing false positives — it will not introduce new false negatives, but the benefit is lost until the filter is updated.
+- Any new local-endpoint alias beyond the four currently recognized (`localhost`, `127.0.0.1`, `::1`, `host.docker.internal`) must be explicitly added to `isLocalCurlAllowToolHost`.
+
+#### Neutral
+- This is the third targeted filter added to the Runner-Guard post-processing pipeline, following the pattern of `filterRunnerGuardIgnoredFindings` and `filterGvisorInstallFindings`.
+- The implementation is isolated in a new file (`runner_guard_copilot_allow_tool.go`), keeping `runner_guard.go` uncluttered.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/runner_guard.go

@@ -43,10 +43,10 @@ func buildRunnerGuardContainerScanPath(scanPath string) (string, error) {
 	}
 	cleanPath := filepath.Clean(scanPath)
 	if !filepath.IsLocal(cleanPath) {
-		return "", fmt.Errorf("runner-guard scan path must stay local to the repository. Got: %q", scanPath)
+		return "", fmt.Errorf("runner-guard scan path must stay local to the repository. Expected a relative path inside the repository. Example: .github/workflows. Got: %q", scanPath)
 	}
 	if containsControlCharacters(cleanPath) {
-		return "", fmt.Errorf("runner-guard scan path contains invalid control characters. Got: %q", scanPath)
+		return "", fmt.Errorf("runner-guard scan path contains invalid control characters. Expected a plain relative path. Example: .github/workflows. Got: %q", scanPath)
 	}
 	return "./" + filepath.ToSlash(cleanPath), nil
 }
@@ -64,7 +64,7 @@ func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) er
 
 	gitRoot, err = fileutil.ValidateAbsolutePath(gitRoot)
 	if err != nil {
-		return fmt.Errorf("invalid git root %q: %w", gitRoot, err)
+		return fmt.Errorf("git root %q is not a valid absolute path; runner-guard requires an absolute repository root. Example: run gh aw from inside a git checkout: %w", gitRoot, err)
 	}
 
 	// Determine the scan path: use workflowDir relative to gitRoot when possible,
@@ -73,14 +73,14 @@ func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) er
 	if workflowDir != "" {
 		absWorkflowDir, err := filepath.Abs(workflowDir)
 		if err != nil {
-			return fmt.Errorf("failed to resolve workflow directory %q: %w", workflowDir, err)
+			return fmt.Errorf("workflow directory %q could not be resolved to an absolute path; expected an existing directory. Example: .github/workflows: %w", workflowDir, err)
 		}
 		if err := fileutil.ValidatePathWithinBase(gitRoot, absWorkflowDir); err != nil {
-			return fmt.Errorf("workflow directory %q must stay within git root %q: %w", workflowDir, gitRoot, err)
+			return fmt.Errorf("workflow directory %q must stay within git root %q; expected a directory inside the repository. Example: .github/workflows: %w", workflowDir, gitRoot, err)
 		}
 		relDir, relErr := filepath.Rel(gitRoot, absWorkflowDir)
 		if relErr != nil {
-			return fmt.Errorf("failed to compute relative path for workflow directory %q: %w", workflowDir, relErr)
+			return fmt.Errorf("workflow directory %q could not be expressed relative to git root %q; expected a directory inside the repository. Example: .github/workflows: %w", workflowDir, gitRoot, relErr)
 		}
 		if !filepath.IsLocal(relDir) {
 			return fmt.Errorf("workflow directory %q resolved to non-local relative path %q", workflowDir, relDir)
@@ -93,7 +93,7 @@ func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) er
 	// produce a scanPath beginning with "-", which runner-guard could interpret as a flag.
 	containerScanPath, err := buildRunnerGuardContainerScanPath(scanPath)
 	if err != nil {
-		return fmt.Errorf("invalid runner-guard scan path: %w", err)
+		return fmt.Errorf("runner-guard scan path is invalid; expected a relative path inside the repository. Example: .github/workflows: %w", err)
 	}
 
 	// Build the Docker command
@@ -104,11 +104,11 @@ func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) er
 	}
 	volumeMount, err := buildDockerVolumeMount(gitRoot, "/workdir")
 	if err != nil {
-		return fmt.Errorf("invalid docker mount path: %w", err)
+		return fmt.Errorf("docker mount path for git root %q is invalid; expected an absolute host path. Example: /home/user/repo: %w", gitRoot, err)
 	}
 	runnerGuardImageRef, err := validateDockerImageRef(RunnerGuardImage)
 	if err != nil {
-		return fmt.Errorf("invalid runner-guard scanner image reference %q: %w", RunnerGuardImage, err)
+		return fmt.Errorf("runner-guard scanner image reference %q is invalid; expected a registry reference. Example: ghcr.io/owner/image:tag: %w", RunnerGuardImage, err)
 	}
 	// #nosec G204 -- gitRoot is validated as an absolute path above (from git rev-parse, a trusted
 	// source). containerScanPath is derived from filepath.Rel(gitRoot, workflowDir), cleaned with
@@ -175,10 +175,10 @@ func runRunnerGuardOnDirectory(workflowDir string, verbose bool, strict bool) er
 				return nil
 			}
 			// Other exit codes are actual errors
-			return fmt.Errorf("runner-guard failed with exit code %d", exitCode)
+			return fmt.Errorf("runner-guard failed with exit code %d; expected 0 (clean) or 1 (findings reported). Example: rerun with gh aw --verbose to see the scanner output", exitCode)
 		}
 		// Non-ExitError errors (e.g., command not found)
-		return fmt.Errorf("runner-guard failed: %w", err)
+		return fmt.Errorf("runner-guard failed to start; a working docker installation is required. Example: run docker info to check the daemon: %w", err)
 	}
 
 	return nil
@@ -214,7 +214,7 @@ func parseAndDisplayRunnerGuardOutput(stdout string, verbose bool, gitRoot strin
 
 	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)
+		return 0, fmt.Errorf("runner-guard JSON output could not be parsed; expected a JSON object. Example: {\"findings\":[]}: %w", err)
 	}
 
 	totalFindings := len(output.Findings)
@@ -230,6 +230,10 @@ func parseAndDisplayRunnerGuardOutput(stdout string, verbose bool, gitRoot strin
 	// location in the compiled workflow.
 	output.Findings = filterRunnerGuardIgnoredFindings(output.Findings, gitRoot)
 
+	// Drop RGS-012 findings for Copilot allow-tool declarations that only document local curl
+	// permissions. The declarations are not executable curl calls and cannot exfiltrate secrets.
+	output.Findings = filterCopilotLocalAllowToolFindings(output.Findings, gitRoot)
+
 	// Drop RGS-012 findings for the compiler-generated gVisor install step, which downloads a
 	// pinned, SHA-512-verified artifact and never exfiltrates secrets.
 	output.Findings = filterGvisorInstallFindings(output.Findings, gitRoot)

pkg/cli/runner_guard_copilot_allow_tool.go

@@ -0,0 +1,246 @@
+package cli
+
+import "strings"
+
+const copilotExecutionStepNameMarker = "Execute GitHub Copilot CLI"
+
+// filterCopilotLocalAllowToolFindings drops RGS-012 findings that point at Copilot CLI
+// allow-tool declarations for loopback curl targets. These declarations document the agent's
+// permitted tools; they are not executable curl calls and cannot exfiltrate secrets.
+func filterCopilotLocalAllowToolFindings(findings []runnerGuardFinding, gitRoot string) []runnerGuardFinding {
+	filtered := make([]runnerGuardFinding, 0, len(findings))
+	fileLinesByPath := make(map[string][]string)
+
+	for _, finding := range findings {
+		if finding.RuleID != runnerGuardSecretExfiltrationRule {
+			filtered = append(filtered, finding)
+			continue
+		}
+
+		resolvedPath := resolveRunnerGuardFilePath(gitRoot, finding.File)
+		lines, ok := fileLinesByPath[resolvedPath]
+		if !ok {
+			lines = readWorkflowLines(resolvedPath)
+			fileLinesByPath[resolvedPath] = lines
+		}
+
+		if findingInCopilotLocalCurlAllowTool(lines, finding.Line) {
+			runnerGuardLog.Printf("Suppressing %s finding for Copilot local curl allow-tool declaration in %s", finding.RuleID, finding.File)
+			continue
+		}
+		filtered = append(filtered, finding)
+	}
+
+	return filtered
+}
+
+func findingInCopilotLocalCurlAllowTool(lines []string, lineNum int) bool {
+	if len(lines) == 0 || lineNum <= 0 || lineNum > len(lines) {
+		return false
+	}
+
+	lineIndex := lineNum - 1
+	if isLocalCurlAllowToolComment(lines[lineIndex]) || isLocalCurlAllowToolArgumentLine(lines[lineIndex]) {
+		return true
+	}
+
+	stepStart := -1
+	for i := lineIndex; i >= 0; i-- {
+		if isStepBoundaryLine(lines[i]) {
+			stepStart = i
+			break
+		}
+	}
+	if stepStart == -1 || !isStepNameLine(lines[stepStart], copilotExecutionStepNameMarker) {
+		return false
+	}
+
+	stepEnd := len(lines)
+	for i := stepStart + 1; i < len(lines); i++ {
+		if isStepBoundaryLine(lines[i]) {
+			stepEnd = i
+			break
+		}
+	}
+
+	runIndex := -1
+	for i := stepStart + 1; i < stepEnd; i++ {
+		if isRunLine(lines[i]) {
+			runIndex = i
+			break
+		}
+	}
+	if runIndex == -1 || lineIndex >= runIndex {
+		return false
+	}
+
+	// The executable body of the step must not contain any curl invocation outside of
+	// allow-tool values, otherwise a real outbound request would be hidden.
+	for i := runIndex; i < stepEnd; i++ {
+		if containsCurlOutsideAllowTool(lines[i]) {
+			return false
+		}
+	}
+
+	hasToolCommentHeader := false
+	hasLocalCurlAllowTool := false
+	hasNonLocalCurlAllowTool := false
+	for i := stepStart; i < runIndex; i++ {
+		line := lines[i]
+		if strings.Contains(line, "# Copilot CLI tool arguments") {
+			hasToolCommentHeader = true
+		}
+		host, ok := curlAllowToolCommentHost(line)
+		if !ok {
+			continue
+		}
+		if isLocalCurlAllowToolHost(host) {
+			hasLocalCurlAllowTool = true
+		} else {
+			hasNonLocalCurlAllowTool = true
+		}
+	}
+
+	return hasToolCommentHeader && hasLocalCurlAllowTool && !hasNonLocalCurlAllowTool
+}
+
+func isRunLine(line string) bool {
+	trimmed := strings.TrimSpace(line)
+	return trimmed == "run:" || strings.HasPrefix(trimmed, "run: ")
+}
+
+func isLocalCurlAllowToolComment(line string) bool {
+	host, ok := curlAllowToolCommentHost(line)
+	return ok && isLocalCurlAllowToolHost(host)
+}
+
+func isLocalCurlAllowToolArgumentLine(line string) bool {
+	if !strings.Contains(line, "--allow-tool") || !strings.Contains(line, "copilot") {
+		return false
+	}
+	if containsCurlOutsideAllowTool(line) {
+		return false
+	}
+
+	hosts := curlAllowToolHosts(line)
+	if len(hosts) == 0 {
+		return false
+	}
+	for _, host := range hosts {
+		if !isLocalCurlAllowToolHost(host) {
+			return false
+		}
+	}
+	return true
+}
+
+func curlAllowToolCommentHost(line string) (string, bool) {
+	trimmed := strings.TrimSpace(line)
+	trimmed = strings.TrimPrefix(trimmed, "#")
+	trimmed = strings.TrimSpace(trimmed)
+	const prefix = "--allow-tool shell(curl "
+	if !strings.HasPrefix(trimmed, prefix) || !strings.HasSuffix(trimmed, ")") {
+		return "", false
+	}
+
+	return curlTargetHost(strings.TrimSuffix(strings.TrimPrefix(trimmed, prefix), ")"))
+}
+
+func curlAllowToolHosts(line string) []string {
+	const prefix = "shell(curl "
+	var hosts []string
+	remaining := line
+	for {
+		index := strings.Index(remaining, prefix)
+		if index < 0 {
+			return hosts
+		}
+		remaining = remaining[index+len(prefix):]
+		end := strings.Index(remaining, ")")
+		if end < 0 {
+			return hosts
+		}
+		if host, ok := curlTargetHost(remaining[:end]); ok {
+			hosts = append(hosts, host)
+		}
+		remaining = remaining[end+1:]
+	}
+}
+
+// containsCurlOutsideAllowTool reports whether a line invokes curl outside of a
+// shell(...) allow-tool value, which indicates a real executable request.
+func containsCurlOutsideAllowTool(line string) bool {
+	var remainder strings.Builder
+	remaining := line
+	for {
+		index := strings.Index(remaining, "shell(")
+		if index < 0 {
+			remainder.WriteString(remaining)
+			break
+		}
+		remainder.WriteString(remaining[:index])
+		remaining = remaining[index+len("shell("):]
+		end := strings.Index(remaining, ")")
+		if end < 0 {
+			break
+		}
+		remaining = remaining[end+1:]
+	}
+	return strings.Contains(remainder.String(), "curl")
+}
+
+func curlTargetHost(target string) (string, bool) {
+	target = strings.TrimSpace(target)
+	if fields := strings.Fields(target); len(fields) > 0 {
+		target = fields[0]
+	}
+	target = strings.Trim(target, `"'`)
+
+	if schemeEnd := strings.Index(target, "://"); schemeEnd >= 0 {
+		target = target[schemeEnd+len("://"):]
+	}
+	if slash := strings.Index(target, "/"); slash >= 0 {
+		target = target[:slash]
+	}
+
+	if strings.HasPrefix(target, "[") {
+		// Bracketed IPv6 literal: the host is inside the brackets and any trailing
+		// ":port" suffix is outside them.
+		if end := strings.Index(target, "]"); end >= 0 {
+			target = target[1:end]
+		} else {
+			target = target[1:]
+		}
+	} else if colon := strings.LastIndex(target, ":"); colon >= 0 && strings.Count(target, ":") == 1 {
+		port := target[colon+1:]
+		if port == "*" || allDigits(port) {
+			target = target[:colon]
+		}
+	}
+
+	if target == "" {
+		return "", false
+	}
+	return strings.ToLower(target), true
+}
+
+func allDigits(value string) bool {
+	if value == "" {
+		return false
+	}
+	for _, r := range value {
+		if r < '0' || r > '9' {
+			return false
+		}
+	}
+	return true
+}
+
+func isLocalCurlAllowToolHost(host string) bool {
+	switch strings.ToLower(strings.Trim(host, "[]")) {
+	case "localhost", "127.0.0.1", "::1", "host.docker.internal":
+		return true
+	default:
+		return false
+	}
+}