Add docker agent exposure rules and reduce false positives#39
Add docker agent exposure rules and reduce false positives#39harekrishnarai merged 7 commits intomainfrom
Conversation
New rules: - DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE (Critical): detect reusable workflows passing secrets into containers running on untrusted fork code - AI_AGENT_ON_UNTRUSTED_CODE (High): detect AI/LLM agent steps operating on PR code without network isolation False positive reductions: - SELF_HOSTED_RUNNER_SECURITY: recognize managed runner providers (Namespace.so, BuildJet, ubuntu-slim) and dynamic expressions - UNTRUSTED_ACTION_SOURCE: skip local actions (./) and SHA-pinned third-party actions - DANGEROUS_WRITE_OPERATION: remove patterns matching safe shell variables - EXTERNAL_TRIGGER_DEBUG: only flag workflow_run/repository_dispatch with write permissions - ARTIPACKED: suppress for trusted workflows without untrusted input Infrastructure fixes: - ConcurrentProcessor now routes through RuleEngine.ExecuteRules for context-aware severity adjustment and suppression - convertToLegacyWorkflow parses YAML so context analyzer has data - Add RepositoryOwner detection from git remote origin - Add Uses/With/Secrets fields to parser.Job for reusable workflows Tested on large monorepo: 287 -> 160 findings, 12 -> 3 critical, 30 findings context-adjusted.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
There was a problem hiding this comment.
Pull request overview
This PR adds new detection rules for Docker/agent secret exposure in GitHub Actions pull_request_target workflows, and refines multiple existing rules and infrastructure paths to reduce false positives and enable context-aware suppression during scans.
Changes:
- Added new rules:
DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODEandAI_AGENT_ON_UNTRUSTED_CODE, plus tests. - Reduced false positives in supply-chain and runner-detection logic (e.g., skip local actions, downgrade/skip SHA-pinned third-party actions, recognize managed runner labels).
- Routed concurrent scanning through
RuleEngine.ExecuteRules()(context-aware suppression/severity adjustment), added YAML parsing in HybridEngine conversion, and added repository owner detection from git remotes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/rules/supply_chain.go | Refines action-source checks (skip local actions; adjust handling of SHA-pinned/untrusted actions). |
| pkg/rules/shell_injection.go | Updates self-hosted runner detection to recognize additional GitHub-hosted and managed runner labels. |
| pkg/rules/rules.go | Registers new rules and reduces false positives for dangerous write ops + external-trigger findings. |
| pkg/rules/docker_agent_exposure.go | Implements new Docker/agent exposure detection logic (direct docker runs + reusable workflow patterns + AI agent detection). |
| pkg/rules/docker_agent_exposure_test.go | Adds test coverage for the new Docker/agent exposure rules. |
| pkg/platform/platform.go | Extends platform workflow model with RepositoryOwner. |
| pkg/parser/parser.go | Extends parser.Job to support reusable workflow fields (uses/with/secrets). |
| pkg/engine/hybrid.go | Parses YAML into legacy workflow struct, and infers repository owner from .git/config. |
| pkg/concurrent/concurrent.go | Uses RuleEngine.ExecuteRules() to enable context-aware suppression in concurrent scans. |
| pkg/analysis/context/analyzer.go | Adds suppression logic for ARTIPACKED_VULNERABILITY in trusted/no-untrusted-input contexts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| version := actionParts[1] | ||
| if !strings.HasPrefix(version, "v") && len(version) != 40 { | ||
| // Not a semantic version or SHA - might be a branch name | ||
| isSuspicious = true | ||
| suspiciousReason = "uses branch name instead of pinned version" | ||
| if len(version) == 40 { | ||
| isPinnedToSHA = true | ||
| } else if !strings.HasPrefix(version, "v") { | ||
| isBranchRef = true |
There was a problem hiding this comment.
Version pinning detection treats any 40-character ref as a commit SHA. This can misclassify non-SHA refs (e.g., long tag/branch names) and incorrectly skip UNTRUSTED_ACTION_SOURCE findings. Consider reusing the existing SHA validation helper (e.g., isHexString / analyzeVersionPinning in advanced_supply_chain.go) to confirm the ref is actually a 40-hex commit hash before treating it as SHA-pinned.
| go func() { | ||
| defer wg.Done() | ||
|
|
||
| var standardFindings []rules.Finding | ||
| for _, rule := range job.StandardRules { | ||
| select { | ||
| case <-ctx.Done(): | ||
| errChan <- ctx.Err() | ||
| return | ||
| default: | ||
| } | ||
|
|
||
| findings := rule.Check(job.Workflow) | ||
| standardFindings = append(standardFindings, findings...) | ||
| } | ||
| // Use the RuleEngine for context-aware analysis (severity adjustment, suppression) | ||
| engine := rules.NewRuleEngine(nil) | ||
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) | ||
|
|
There was a problem hiding this comment.
This goroutine no longer checks ctx.Done() while running standard rules. If a workflow-level timeout/cancel happens, engine.ExecuteRules will keep running until completion, which defeats the cancellation behavior that existed before the refactor. Consider adding a cancellation check before/after execution, or extending RuleEngine.ExecuteRules to accept a context and short-circuit per rule.
| // checkDockerExecWithSecrets detects direct docker run commands that forward | ||
| // secrets via -e/--env while operating on fork code without network isolation. | ||
| func checkDockerExecWithSecrets(workflow parser.WorkflowFile) []Finding { | ||
| var findings []Finding | ||
| lineMapper := linenum.NewLineMapper(workflow.Content) | ||
|
|
||
| dockerRunWithEnv := []*regexp.Regexp{ | ||
| regexp.MustCompile(`docker\s+run\b[^;|&]*\s-e\s`), | ||
| regexp.MustCompile(`docker\s+run\b[^;|&]*\s--env\s`), | ||
| regexp.MustCompile(`docker\s+run\b[^;|&]*\s--env-file\s`), | ||
| } | ||
|
|
||
| secretEnvPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`-e\s+\w*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)\w*`), | ||
| regexp.MustCompile(`--env\s+\w*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)\w*`), | ||
| regexp.MustCompile(`-e\s+\$\{\{\s*secrets\.\w+\s*\}\}`), | ||
| regexp.MustCompile(`--env\s+\w+=\$\{\{\s*secrets\.\w+\s*\}\}`), | ||
| } | ||
|
|
||
| networkNone := regexp.MustCompile(`--network[=\s]+none`) | ||
|
|
||
| for jobName, job := range workflow.Workflow.Jobs { | ||
| for stepIdx, step := range job.Steps { | ||
| if step.Run == "" { | ||
| continue | ||
| } | ||
|
|
||
| hasDockerEnv := false | ||
| for _, pattern := range dockerRunWithEnv { | ||
| if pattern.MatchString(step.Run) { | ||
| hasDockerEnv = true | ||
| break | ||
| } | ||
| } | ||
| if !hasDockerEnv { | ||
| continue | ||
| } | ||
|
|
||
| if networkNone.MatchString(step.Run) { | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
checkDockerExecWithSecrets currently treats any pull_request_target workflow that runs docker run -e/--env as “operating on fork code”, but pull_request_target is safe if the workflow never checks out / uses PR head content. This will create significant false positives for PRT workflows that only act on base-repo metadata. Consider requiring evidence of untrusted-code checkout/usage (e.g., actions/checkout with ref: ${{ github.event.pull_request.head.sha }} / head_ref) similar to existing pull_request_target risk checks before emitting this finding.
| if !hasSecrets { | ||
| contentStr := string(workflow.Content) | ||
| if strings.Contains(contentStr, "secrets:") { | ||
| hasSecrets = true | ||
| } | ||
| } |
There was a problem hiding this comment.
The fallback strings.Contains(contentStr, "secrets:") is workflow-global and will mark hasSecrets true even when the specific reusable-workflow job does not pass any secrets. This is likely to reintroduce false positives. Prefer relying on the parsed job.Secrets / job.With fields (now in parser.Job), and if parsing is unavailable, scope any text search to the specific job block rather than the entire file.
| Steps: []parser.Step{ | ||
| { | ||
| Uses: "org/reusable-workflows/.github/workflows/review-pull-request.yml@main", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
This test models a reusable workflow call as a step Uses: org/.../.github/workflows/..., but reusable workflows are invoked at the job level (job.Uses + job.Secrets). As a result, the test can pass via the step-level fallback logic and may not cover the real-world path you added in checkReusableWorkflowAgentExposure. Consider setting parser.Job{Uses: ..., Secrets: ...} (and omitting steps) to validate the intended behavior.
| Steps: []parser.Step{ | |
| { | |
| Uses: "org/reusable-workflows/.github/workflows/review-pull-request.yml@main", | |
| }, | |
| }, | |
| Uses: "org/reusable-workflows/.github/workflows/review-pull-request.yml@main", | |
| Secrets: "inherit", |
| isManaged := func(label string) bool { | ||
| if githubHostedRunners[label] { | ||
| return true | ||
| } | ||
| lower := strings.ToLower(label) | ||
| for _, prefix := range managedRunnerPrefixes { | ||
| if strings.HasPrefix(lower, prefix) { | ||
| return true | ||
| } | ||
| } | ||
| // GitHub variant suffixes: ubuntu-latest-xl, macos-13-large, etc. | ||
| if strings.HasPrefix(lower, "ubuntu-") || strings.HasPrefix(lower, "macos-") || strings.HasPrefix(lower, "windows-") { | ||
| return true |
There was a problem hiding this comment.
isManaged returns true for any runner label starting with ubuntu-/macos-/windows-. Self-hosted runners commonly include OS-like custom labels (e.g., ubuntu-self-hosted), so this change can suppress SELF_HOSTED_RUNNER_SECURITY findings (false negatives). Consider limiting this heuristic to known GitHub-hosted variants (e.g., ubuntu-latest-*, macos-*-large) and the explicit managed-provider prefixes, rather than any ubuntu- prefix.
| isManaged := func(label string) bool { | |
| if githubHostedRunners[label] { | |
| return true | |
| } | |
| lower := strings.ToLower(label) | |
| for _, prefix := range managedRunnerPrefixes { | |
| if strings.HasPrefix(lower, prefix) { | |
| return true | |
| } | |
| } | |
| // GitHub variant suffixes: ubuntu-latest-xl, macos-13-large, etc. | |
| if strings.HasPrefix(lower, "ubuntu-") || strings.HasPrefix(lower, "macos-") || strings.HasPrefix(lower, "windows-") { | |
| return true | |
| // Narrowly scoped managed runner patterns for known GitHub-hosted variants. | |
| // Avoid treating arbitrary OS-prefixed labels such as "ubuntu-self-hosted" | |
| // as managed runners. | |
| managedRunnerPatterns := []*regexp.Regexp{ | |
| regexp.MustCompile(`^ubuntu-latest-[0-9]+-cores$`), | |
| regexp.MustCompile(`^macos-(latest|[0-9]+)-(large|xlarge)$`), | |
| } | |
| isManaged := func(label string) bool { | |
| lower := strings.ToLower(label) | |
| if githubHostedRunners[lower] { | |
| return true | |
| } | |
| for _, prefix := range managedRunnerPrefixes { | |
| if strings.HasPrefix(lower, prefix) { | |
| return true | |
| } | |
| } | |
| for _, pattern := range managedRunnerPatterns { | |
| if pattern.MatchString(lower) { | |
| return true | |
| } |
| // Check for dangerous external triggers | ||
| // Only flag triggers that combine external accessibility with write permissions, | ||
| // since read-only workflows triggered externally pose minimal risk. | ||
| dangerousTriggers := map[string]string{ | ||
| "issue_comment": "Can be triggered by anyone who can comment on issues", | ||
| "pull_request_target": "Can be triggered by external pull requests with elevated permissions", | ||
| "workflow_run": "Can be triggered by completion of other workflows", | ||
| "repository_dispatch": "Can be triggered via API by repository collaborators", | ||
| "issue_comment": "Can be triggered by anyone who can comment on issues", | ||
| } |
There was a problem hiding this comment.
The comment says we “Only flag triggers that combine external accessibility with write permissions”, but issue_comment is included in dangerousTriggers and is always flagged (High) even when permissions are read-only. Either move issue_comment into the write-permission-gated map or update the comment/logic so the behavior matches the stated intent.
| // checkIndirectDockerWithSecrets detects step-level reusable workflow/action | ||
| // calls that pass secrets to agent/review workflows under pull_request_target. | ||
| func checkIndirectDockerWithSecrets(workflow parser.WorkflowFile, lineMapper *linenum.LineMapper) []Finding { | ||
| var findings []Finding | ||
|
|
||
| agentWorkflowPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)review.*pull.*request`), | ||
| regexp.MustCompile(`(?i)agent`), | ||
| regexp.MustCompile(`(?i)bot.*respond`), | ||
| regexp.MustCompile(`(?i)ai.*review`), | ||
| regexp.MustCompile(`(?i)code.*review`), | ||
| regexp.MustCompile(`(?i)auto.*review`), | ||
| } | ||
|
|
||
| for jobName, job := range workflow.Workflow.Jobs { | ||
| for stepIdx, step := range job.Steps { | ||
| if step.Uses == "" { | ||
| continue | ||
| } | ||
|
|
||
| if !strings.Contains(step.Uses, ".github/workflows/") && | ||
| !strings.Contains(step.Uses, ".github/actions/") { | ||
| continue | ||
| } |
There was a problem hiding this comment.
This step-level check for .github/workflows/ in step.Uses won’t match valid GitHub Actions syntax (reusable workflows are invoked via job.uses, not step uses). As written, real reusable-workflow calls may be missed while tests can pass using invalid structures. Consider moving reusable-workflow detection to job.Uses/job.Secrets (new parser.Job fields) and limiting step-level detection to composite actions (.github/actions/).
| aiAgentPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)\b(oz|claude|copilot|gpt|openai|anthropic)\s+(agent|run|review)`), | ||
| regexp.MustCompile(`(?i)\bagent\s+run\b`), | ||
| regexp.MustCompile(`(?i)\bclaude\s+(-p|--prompt|code)\b`), | ||
| regexp.MustCompile(`(?i)docker\s+run\b[^;|&]*(agent|review|bot|ai)[^;|&]*`), | ||
| regexp.MustCompile(`(?i)\b(aider|cursor|continue|cody|devin|sweep)\b.*\b(run|start|review)\b`), | ||
| } |
There was a problem hiding this comment.
The AI-agent rule claims to detect lack of network isolation, but the implementation does not check for --network=none (or other isolation) at all, so even mitigated docker-based agent steps can still be flagged. Also, line numbers are only resolved for run: steps; uses:-based detections will report LineNumber: 1. Consider adding an isolation check when the evidence is a docker run, and use the line mapper to resolve uses: lines too.
| // detectRepositoryOwner attempts to determine the repository owner from | ||
| // the git remote URL or falls back to the directory name. | ||
| func detectRepositoryOwner(repoPath string) string { | ||
| // Try reading .git/config for remote origin URL | ||
| gitConfigPath := filepath.Join(repoPath, ".git", "config") | ||
| data, err := os.ReadFile(gitConfigPath) | ||
| if err != nil { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
The comment says this “falls back to the directory name”, but on read/parse failure the function currently returns an empty string. Either implement the documented fallback (e.g., derive from filepath.Base(repoPath)), or update the comment so callers don’t assume an owner will be present. Also consider handling common ssh://git@host/owner/repo.git remote URL formats, which won’t be parsed correctly by the current SSH heuristic.
Review fixes: - SHA detection: verify hex chars, not just 40-char length - Restore ctx.Done() check in concurrent goroutine - Require untrusted checkout evidence for docker+secrets finding - Scope secrets detection to parsed job fields, not entire file - Fix test: reusable workflow at job-level, not step-level - Remove invalid step-level .github/workflows/ detection - Narrow isManaged runner to exact GitHub-hosted patterns - Gate issue_comment trigger on write permissions - Add --network=none mitigation check for AI agent rule - Resolve line numbers for uses:-based AI agent detections - Implement directory name fallback in detectRepositoryOwner - Handle ssh:// URL format in ownerFromRemoteURL New rule: AI_AGENT_COMMENT_TRIGGERED - Detects AI agents triggered by issue_comment/PR review comment - Flags prompt injection risk (High/Critical) when secrets present - Flags denial-of-wallet (Medium) even without secrets - Suppresses when author_association gate is present - Covers Claude, Gemini, Copilot, CodeRabbit, Aider, Devin, etc.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| triggerName := "" | ||
| switch on := workflow.Workflow.On.(type) { | ||
| case map[string]interface{}: | ||
| if _, ok := on["issue_comment"]; ok { | ||
| hasCommentTrigger = true | ||
| triggerName = "issue_comment" | ||
| } | ||
| if _, ok := on["pull_request_review_comment"]; ok { | ||
| hasCommentTrigger = true | ||
| if triggerName != "" { | ||
| triggerName += " + pull_request_review_comment" | ||
| } else { | ||
| triggerName = "pull_request_review_comment" | ||
| } |
There was a problem hiding this comment.
triggerName is assigned but never used in CheckAIAgentCommentTriggered, which will cause a Go compile error (unused variable). Remove it or incorporate it into the finding evidence/description (e.g., include which comment trigger(s) were detected).
| triggerName := "" | |
| switch on := workflow.Workflow.On.(type) { | |
| case map[string]interface{}: | |
| if _, ok := on["issue_comment"]; ok { | |
| hasCommentTrigger = true | |
| triggerName = "issue_comment" | |
| } | |
| if _, ok := on["pull_request_review_comment"]; ok { | |
| hasCommentTrigger = true | |
| if triggerName != "" { | |
| triggerName += " + pull_request_review_comment" | |
| } else { | |
| triggerName = "pull_request_review_comment" | |
| } | |
| switch on := workflow.Workflow.On.(type) { | |
| case map[string]interface{}: | |
| if _, ok := on["issue_comment"]; ok { | |
| hasCommentTrigger = true | |
| } | |
| if _, ok := on["pull_request_review_comment"]; ok { | |
| hasCommentTrigger = true |
| if !hasSecrets { | ||
| contentStr := string(workflow.Content) | ||
| if strings.Contains(contentStr, "secrets: inherit") { | ||
| hasSecrets = true | ||
| } | ||
| } |
There was a problem hiding this comment.
checkIndirectDockerWithSecrets falls back to strings.Contains(contentStr, "secrets: inherit") to decide hasSecrets. This can produce false positives because it is file-global (could match a different job) and secrets: inherit is job-level reusable-workflow syntax, not step-level composite action syntax. Use the parsed job.Secrets field (and/or the specific job’s with:/env:) instead of scanning the entire file content.
| for jobName, job := range workflow.Workflow.Jobs { | ||
| for stepIdx, step := range job.Steps { | ||
| isAIStep := false | ||
| evidence := "" | ||
|
|
||
| if step.Run != "" { | ||
| for _, pattern := range aiAgentPatterns { | ||
| if pattern.MatchString(step.Run) { | ||
| isAIStep = true | ||
| evidence = step.Run | ||
| break | ||
| } | ||
| } | ||
| // If this is a docker run with --network=none, it's mitigated | ||
| if isAIStep && networkNonePattern.MatchString(step.Run) { | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| if !isAIStep && step.Name != "" { | ||
| for _, pattern := range aiStepNamePatterns { | ||
| if pattern.MatchString(step.Name) { | ||
| isAIStep = true | ||
| evidence = "step name: " + step.Name | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !isAIStep && step.Uses != "" { | ||
| for _, pattern := range aiActionPatterns { | ||
| if pattern.MatchString(step.Uses) { | ||
| isAIStep = true | ||
| evidence = "uses: " + step.Uses | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !isAIStep { | ||
| continue | ||
| } | ||
|
|
||
| hasSecrets := hasSecretsAccess(step) | ||
| if !hasSecrets && job.Env != nil { | ||
| for _, value := range job.Env { |
There was a problem hiding this comment.
checkAIAgentOnUntrustedCode flags any detected AI agent step in pull_request_target workflows, but it does not require evidence that the job is actually operating on fork-controlled code (e.g., checkout of PR head ref). This will reintroduce false positives for pull_request_target workflows that only run on trusted base branch code. Consider gating on jobCheckoutsUntrustedCode(job) (similar to checkDockerExecWithSecrets) before emitting findings.
| // Check if author_association is already gated | ||
| hasActorGate := false | ||
| if job.If != "" { | ||
| if strings.Contains(job.If, "author_association") { | ||
| hasActorGate = true | ||
| } | ||
| } | ||
| if step.If != "" { | ||
| if strings.Contains(step.If, "author_association") { | ||
| hasActorGate = true | ||
| } | ||
| } | ||
| // Also check the job-level if from raw content for complex expressions | ||
| contentStr := string(workflow.Content) | ||
| if strings.Contains(contentStr, "author_association") && | ||
| (strings.Contains(contentStr, "'MEMBER'") || strings.Contains(contentStr, "'OWNER'") || strings.Contains(contentStr, "'COLLABORATOR'")) { | ||
| hasActorGate = true | ||
| } |
There was a problem hiding this comment.
Actor-gating suppression is currently too permissive: any if containing author_association suppresses the finding, even if it’s not restricting to trusted actors (e.g., != 'MEMBER' or checks for 'NONE'). Additionally, the raw file scan can suppress findings if those strings appear in unrelated contexts. Tighten this to only suppress when the relevant job/step if: expression positively restricts to MEMBER/OWNER/COLLABORATOR (e.g., via a regex for equality/in-list checks).
| case []interface{}: | ||
| for _, runner := range runsOn { | ||
| if runnerStr, ok := runner.(string); ok { | ||
| if strings.Contains(runnerStr, "${{") && strings.Contains(runnerStr, "matrix") { | ||
| return false // Conservative approach for matrix runners | ||
| if strings.Contains(runnerStr, "${{") { | ||
| return false | ||
| } | ||
| if !githubHostedRunners[runnerStr] { | ||
| if !isManaged(runnerStr) { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
In the runs-on array case, the presence of any dynamic expression (${{ ... }}) causes an immediate return false, which can hide an explicit self-hosted label in the same list (e.g., runs-on: [self-hosted, ${{ matrix.os }}]). Instead, treat dynamic entries as “unknown” and continue scanning other labels; only return false when you can confidently classify the job as managed/GitHub-hosted.
| // Determine version pinning status | ||
| isPinnedToSHA := false | ||
| isBranchRef := false | ||
| if len(actionParts) > 1 { | ||
| version := actionParts[1] | ||
| if !strings.HasPrefix(version, "v") && len(version) != 40 { | ||
| // Not a semantic version or SHA - might be a branch name | ||
| isSuspicious = true | ||
| suspiciousReason = "uses branch name instead of pinned version" | ||
| if len(version) == 40 && isHexString(version) { | ||
| isPinnedToSHA = true | ||
| } else if !strings.HasPrefix(version, "v") { | ||
| isBranchRef = true | ||
| } | ||
| } |
There was a problem hiding this comment.
isBranchRef is set for any version that doesn’t start with v and isn’t a SHA. This will classify common semver tags like 1.2.3 (no v prefix) as a “branch name” and raise severity to High. Consider treating numeric semver tags without v as pinned versions (e.g., via parseSemanticVersion or a semver regex) and reserving the “branch ref” classification for clearly mutable refs like main/master or non-semver strings.
| func checkIndirectDockerWithSecrets(workflow parser.WorkflowFile, lineMapper *linenum.LineMapper) []Finding { | ||
| var findings []Finding | ||
|
|
There was a problem hiding this comment.
checkIndirectDockerWithSecrets declares a lineMapper parameter but never uses it, which will cause a Go compile error (unused parameter). Either use it to compute LineNumber for the finding(s) (e.g., locate the uses: line), or remove the parameter and adjust the caller accordingly.
| // If the job has write permissions, severity is Critical | ||
| if job.Permissions != nil && permsImplyWrite(job.Permissions) { |
There was a problem hiding this comment.
Severity escalation to Critical is based on job.Permissions != nil && permsImplyWrite(job.Permissions), but permsImplyWrite(nil) returns true (GitHub default is write-all). As written, jobs that inherit workflow permissions (nil) will never be marked Critical even when they effectively have write access. Consider using permsImplyWrite(job.Permissions) directly and also factoring in workflow.Workflow.Permissions when job.Permissions is nil/inherited.
| // If the job has write permissions, severity is Critical | |
| if job.Permissions != nil && permsImplyWrite(job.Permissions) { | |
| // If the job effectively has write permissions, severity is Critical. | |
| // Job-level permissions inherit from workflow-level permissions when nil, | |
| // and permsImplyWrite(nil) reflects GitHub's default permissions. | |
| effectivePermissions := job.Permissions | |
| if effectivePermissions == nil { | |
| effectivePermissions = workflow.Workflow.Permissions | |
| } | |
| if permsImplyWrite(effectivePermissions) { |
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) | ||
|
|
There was a problem hiding this comment.
This goroutine now checks ctx.Done() only once before running all standard rules. Since RuleEngine.ExecuteRules doesn’t take a context, long rule execution won’t be cancellable until completion. Consider reinstating a per-rule (or per-finding batch) ctx.Done() check around rule execution, or updating the rule engine to support context-aware cancellation.
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) | |
| for i := range job.StandardRules { | |
| select { | |
| case <-ctx.Done(): | |
| errChan <- ctx.Err() | |
| return | |
| default: | |
| } | |
| ruleFindings := engine.ExecuteRules(job.Workflow, job.StandardRules[i:i+1]) | |
| standardFindings = append(standardFindings, ruleFindings...) | |
| } |
|
|
||
| // Parse action name and version | ||
| // Skip local actions — these are in the same repo and already covered by LOCAL_ACTION_USAGE | ||
| if strings.HasPrefix(step.Uses, "./") { |
There was a problem hiding this comment.
Local actions can also be referenced with ../ prefixes (and other parts of the codebase already treat both ./ and ../ as local). To avoid reintroducing local-action false positives here, consider skipping ../ as well (and any other local forms you support consistently elsewhere).
| if strings.HasPrefix(step.Uses, "./") { | |
| if strings.HasPrefix(step.Uses, "./") || strings.HasPrefix(step.Uses, "../") { |
- Remove unused triggerName variable (CodeQL finding) - Use lineMapper in checkIndirectDockerWithSecrets for line numbers - Replace file-global secrets:inherit fallback with parsed job.Secrets - Gate checkAIAgentOnUntrustedCode on jobCheckoutsUntrustedCode - Use effective permissions (job || workflow) for severity escalation - Tighten author_association gate to positive equality regex only - Fix isBranchRef: recognize semver tags without v prefix (e.g. 1.2.3) - Fix runs-on array: skip dynamic expressions instead of returning false - Skip ../ prefixed local actions in supply chain checks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Skip local actions — these are in the same repo and already covered by LOCAL_ACTION_USAGE | ||
| if strings.HasPrefix(step.Uses, "./") || strings.HasPrefix(step.Uses, "../") { | ||
| continue |
| // checkReusableWorkflowAgentExposure detects job-level reusable workflow calls | ||
| // that pass secrets to agent/review/bot workflows under pull_request_target. | ||
| func checkReusableWorkflowAgentExposure(workflow parser.WorkflowFile) []Finding { | ||
| var findings []Finding | ||
|
|
||
| agentWorkflowPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)review.*pull.*request`), | ||
| regexp.MustCompile(`(?i)review-pr`), | ||
| regexp.MustCompile(`(?i)agent`), | ||
| regexp.MustCompile(`(?i)bot.*respond`), | ||
| regexp.MustCompile(`(?i)ai.*review`), | ||
| regexp.MustCompile(`(?i)code.*review`), | ||
| regexp.MustCompile(`(?i)auto.*review`), | ||
| regexp.MustCompile(`(?i)respond.*comment`), | ||
| regexp.MustCompile(`(?i)triage.*issue`), | ||
| } | ||
|
|
||
| for jobName, job := range workflow.Workflow.Jobs { | ||
| if job.Uses == "" { | ||
| continue | ||
| } | ||
|
|
||
| if !strings.Contains(job.Uses, ".github/workflows/") { | ||
| continue | ||
| } | ||
|
|
||
| isAgentWorkflow := false | ||
| for _, pattern := range agentWorkflowPatterns { | ||
| if pattern.MatchString(job.Uses) { | ||
| isAgentWorkflow = true | ||
| break | ||
| } | ||
| } | ||
| if !isAgentWorkflow { | ||
| continue | ||
| } | ||
|
|
||
| hasSecrets := false | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| hasSecrets = true | ||
| } | ||
| case map[string]interface{}: | ||
| if len(s) > 0 { | ||
| hasSecrets = true | ||
| } | ||
| case map[interface{}]interface{}: | ||
| if len(s) > 0 { | ||
| hasSecrets = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !hasSecrets { | ||
| continue | ||
| } | ||
|
|
||
| severity := High | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| severity = Critical | ||
| } | ||
| } | ||
| } | ||
|
|
||
| finding := Finding{ | ||
| RuleID: "DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE", | ||
| RuleName: "Reusable Agent Workflow Called With Secrets on pull_request_target", | ||
| Description: "A pull_request_target workflow calls a reusable workflow that runs an AI agent or automated review, passing secrets including API keys or private keys. The downstream workflow checks out fork code and processes it in a container with these secrets available, enabling secret exfiltration by external attackers.", | ||
| Severity: severity, | ||
| Category: SecretExposure, | ||
| FilePath: workflow.Path, | ||
| JobName: jobName, | ||
| StepName: "", | ||
| Evidence: "uses: " + job.Uses, | ||
| Remediation: "Ensure downstream Docker containers use --network=none. Do not forward secrets into containers processing fork code. Require a maintainer label before running agents on fork PRs.", | ||
| LineNumber: 1, | ||
| } | ||
| findings = append(findings, finding) | ||
| } | ||
|
|
||
| return findings |
| if !hasSecretForward && job.Env != nil { | ||
| for key, value := range job.Env { | ||
| if strings.Contains(value, "secrets.") { | ||
| keyUpper := strings.ToUpper(key) | ||
| if strings.Contains(keyUpper, "KEY") || strings.Contains(keyUpper, "TOKEN") || | ||
| strings.Contains(keyUpper, "SECRET") || strings.Contains(keyUpper, "PASSWORD") { | ||
| hasSecretForward = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } |
| // Dynamic expressions — can't determine at static analysis time | ||
| if strings.Contains(runsOn, "${{") { | ||
| return false | ||
| } | ||
| return !githubHostedRunners[runsOn] | ||
| return !isManaged(runsOn) |
|
|
||
| var standardFindings []rules.Finding | ||
| // Use the RuleEngine for context-aware analysis (severity adjustment, suppression) | ||
| engine := rules.NewRuleEngine(nil) |
| if err != nil { | ||
| return filepath.Base(filepath.Dir(repoPath)) | ||
| } |
| hasCommentTrigger := false | ||
| switch on := workflow.Workflow.On.(type) { | ||
| case map[string]interface{}: | ||
| if _, ok := on["issue_comment"]; ok { | ||
| hasCommentTrigger = true | ||
| } | ||
| if _, ok := on["pull_request_review_comment"]; ok { | ||
| hasCommentTrigger = true | ||
| } | ||
| } |
…n fixes - Skip local actions (./ ../) in UNTRUSTED_ACTION_SOURCE, TYPOSQUATTING, and UNPINNABLE rules - Lower reusable workflow agent finding to Medium; note checkout verification needed - Only flag job.Env secrets when actually forwarded via docker -e/--env - Treat non-matrix dynamic runner expressions as potentially self-hosted - Pass job.Config to concurrent RuleEngine (honors disabled rules/ignore patterns) - Fix detectRepositoryOwner fallback: use repo dir name, not parent dir - Handle string and list forms of on: for comment trigger detection
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fallback: use parent directory name as a best-effort owner guess | ||
| return filepath.Base(filepath.Dir(repoPath)) |
| // Phase 4: Docker agent exposure (AI agents on fork code with secrets) | ||
| findings = append(findings, CheckDockerAgentExposure(workflow)...) |
| // Use the RuleEngine for context-aware analysis (severity adjustment, suppression) | ||
| engine := rules.NewRuleEngine(job.Config) | ||
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) | ||
|
|
| if !hasSecrets { | ||
| continue | ||
| } | ||
|
|
||
| severity := Medium | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| severity = High | ||
| } | ||
| } | ||
| } | ||
|
|
||
| finding := Finding{ | ||
| RuleID: "DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE", | ||
| RuleName: "Reusable Agent Workflow Called With Secrets on pull_request_target", | ||
| Description: "A pull_request_target workflow calls a reusable workflow that appears to run an AI agent or automated review, passing secrets. Verify whether the called workflow checks out the PR head ref — if it does, fork code runs in a container with secrets available, enabling exfiltration.", | ||
| Severity: severity, | ||
| Category: SecretExposure, | ||
| FilePath: workflow.Path, | ||
| JobName: jobName, | ||
| StepName: "", | ||
| Evidence: "uses: " + job.Uses, | ||
| Remediation: "Ensure downstream Docker containers use --network=none. Do not forward secrets into containers processing fork code. Require a maintainer label before running agents on fork PRs.", | ||
| LineNumber: 1, | ||
| } | ||
| findings = append(findings, finding) |
| if !hasSecrets && step.Env != nil { | ||
| for _, value := range step.Env { | ||
| if strings.Contains(strings.ToUpper(value), "KEY") || | ||
| strings.Contains(strings.ToUpper(value), "TOKEN") { | ||
| hasSecrets = true | ||
| break | ||
| } | ||
| } |
| actorGatePattern := regexp.MustCompile(`author_association\s*==\s*'(MEMBER|OWNER|COLLABORATOR)'`) | ||
| if job.If != "" && actorGatePattern.MatchString(job.If) { | ||
| hasActorGate = true | ||
| } | ||
| if step.If != "" && actorGatePattern.MatchString(step.If) { | ||
| hasActorGate = true |
… CheckAllRules - Fix the parsed-but-unparseable-URL fallback to also use filepath.Base(repoPath) - Add checkAIAgentOnUntrustedCode and CheckAIAgentCommentTriggered to the legacy CheckAllRules entrypoint for consistency (primary path via StandardRules already had them registered)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var standardFindings []rules.Finding | ||
| // Use the RuleEngine for context-aware analysis (severity adjustment, suppression) | ||
| engine := rules.NewRuleEngine(job.Config) | ||
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) | ||
|
|
||
| mu.Lock() | ||
| allFindings = append(allFindings, standardFindings...) | ||
| mu.Unlock() |
| @@ -167,6 +172,7 @@ func (he *HybridEngine) AnalyzeRepository(repoPath string) (*AnalysisResult, err | |||
| if err != nil { | |||
| continue // Skip invalid workflows | |||
| } | |||
| workflow.RepositoryOwner = repoOwner | |||
| func checkIndirectDockerWithSecrets(workflow parser.WorkflowFile, lineMapper *linenum.LineMapper) []Finding { | ||
| var findings []Finding | ||
|
|
||
| agentWorkflowPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)review.*pull.*request`), | ||
| regexp.MustCompile(`(?i)agent`), | ||
| regexp.MustCompile(`(?i)bot.*respond`), | ||
| regexp.MustCompile(`(?i)ai.*review`), | ||
| regexp.MustCompile(`(?i)code.*review`), | ||
| regexp.MustCompile(`(?i)auto.*review`), | ||
| } | ||
|
|
||
| for jobName, job := range workflow.Workflow.Jobs { | ||
| for stepIdx, step := range job.Steps { | ||
| if step.Uses == "" { | ||
| continue | ||
| } | ||
|
|
||
| // Step-level uses: only composite actions are valid here | ||
| if !strings.Contains(step.Uses, ".github/actions/") { | ||
| continue | ||
| } | ||
|
|
||
| isAgentWorkflow := false | ||
| for _, pattern := range agentWorkflowPatterns { | ||
| if pattern.MatchString(step.Uses) { | ||
| isAgentWorkflow = true | ||
| break | ||
| } | ||
| } | ||
| if !isAgentWorkflow { | ||
| continue | ||
| } | ||
|
|
||
| hasSecrets := false | ||
| if step.With != nil { | ||
| for key, value := range step.With { | ||
| if valueStr, ok := value.(string); ok { | ||
| if strings.Contains(valueStr, "secrets.") || | ||
| strings.Contains(strings.ToLower(key), "token") || | ||
| strings.Contains(strings.ToLower(key), "key") { | ||
| hasSecrets = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !hasSecrets { | ||
| // Check job-level secrets field (for reusable workflow calls in same job) | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| hasSecrets = true | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !hasSecrets { | ||
| continue | ||
| } | ||
|
|
||
| stepName := step.Name | ||
| if stepName == "" { | ||
| stepName = "Step " + string(rune('1'+stepIdx)) | ||
| } | ||
|
|
||
| lineNumber := 1 | ||
| if step.Uses != "" { | ||
| linePattern := linenum.FindPattern{ | ||
| Key: "uses", | ||
| Value: step.Uses, | ||
| } | ||
| lineResult := lineMapper.FindLineNumber(linePattern) | ||
| if lineResult != nil { | ||
| lineNumber = lineResult.LineNumber | ||
| } | ||
| } | ||
|
|
||
| finding := Finding{ | ||
| RuleID: "DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE", | ||
| RuleName: "Reusable Workflow Runs Agent on Fork Code With Secrets", | ||
| Description: "A pull_request_target workflow passes secrets to a reusable workflow that runs an AI agent or automated review on fork code. If the downstream container lacks network isolation, an external attacker can exfiltrate secrets via malicious PR files.", | ||
| Severity: High, | ||
| Category: SecretExposure, | ||
| FilePath: workflow.Path, | ||
| JobName: jobName, | ||
| StepName: stepName, | ||
| Evidence: "uses: " + step.Uses, | ||
| Remediation: "Audit the reusable workflow to ensure Docker containers use --network=none and secrets are not forwarded into containers processing fork code.", | ||
| LineNumber: lineNumber, | ||
| } | ||
| findings = append(findings, finding) | ||
| } | ||
| } | ||
|
|
||
| return findings |
| // checkReusableWorkflowAgentExposure detects job-level reusable workflow calls | ||
| // that pass secrets to agent/review/bot workflows under pull_request_target. | ||
| func checkReusableWorkflowAgentExposure(workflow parser.WorkflowFile) []Finding { | ||
| var findings []Finding | ||
|
|
||
| agentWorkflowPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`(?i)review.*pull.*request`), | ||
| regexp.MustCompile(`(?i)review-pr`), | ||
| regexp.MustCompile(`(?i)agent`), | ||
| regexp.MustCompile(`(?i)bot.*respond`), | ||
| regexp.MustCompile(`(?i)ai.*review`), | ||
| regexp.MustCompile(`(?i)code.*review`), | ||
| regexp.MustCompile(`(?i)auto.*review`), | ||
| regexp.MustCompile(`(?i)respond.*comment`), | ||
| regexp.MustCompile(`(?i)triage.*issue`), | ||
| } | ||
|
|
||
| for jobName, job := range workflow.Workflow.Jobs { | ||
| if job.Uses == "" { | ||
| continue | ||
| } | ||
|
|
||
| if !strings.Contains(job.Uses, ".github/workflows/") { | ||
| continue | ||
| } | ||
|
|
||
| isAgentWorkflow := false | ||
| for _, pattern := range agentWorkflowPatterns { | ||
| if pattern.MatchString(job.Uses) { | ||
| isAgentWorkflow = true | ||
| break | ||
| } | ||
| } | ||
| if !isAgentWorkflow { | ||
| continue | ||
| } | ||
|
|
||
| hasSecrets := false | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| hasSecrets = true | ||
| } | ||
| case map[string]interface{}: | ||
| if len(s) > 0 { | ||
| hasSecrets = true | ||
| } | ||
| case map[interface{}]interface{}: | ||
| if len(s) > 0 { | ||
| hasSecrets = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !hasSecrets { | ||
| continue | ||
| } | ||
|
|
||
| severity := Medium | ||
| if job.Secrets != nil { | ||
| switch s := job.Secrets.(type) { | ||
| case string: | ||
| if s == "inherit" { | ||
| severity = High | ||
| } | ||
| } | ||
| } | ||
|
|
||
| finding := Finding{ | ||
| RuleID: "DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE", | ||
| RuleName: "Reusable Agent Workflow Called With Secrets on pull_request_target", | ||
| Description: "A pull_request_target workflow calls a reusable workflow that appears to run an AI agent or automated review, passing secrets. Verify whether the called workflow checks out the PR head ref — if it does, fork code runs in a container with secrets available, enabling exfiltration.", | ||
| Severity: severity, | ||
| Category: SecretExposure, | ||
| FilePath: workflow.Path, | ||
| JobName: jobName, | ||
| StepName: "", | ||
| Evidence: "uses: " + job.Uses, | ||
| Remediation: "Ensure downstream Docker containers use --network=none. Do not forward secrets into containers processing fork code. Require a maintainer label before running agents on fork PRs.", | ||
| LineNumber: 1, | ||
| } | ||
| findings = append(findings, finding) | ||
| } |
| secretEnvPatterns := []*regexp.Regexp{ | ||
| regexp.MustCompile(`-e\s+\w*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)\w*`), | ||
| regexp.MustCompile(`--env\s+\w*(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)\w*`), | ||
| regexp.MustCompile(`-e\s+\$\{\{\s*secrets\.\w+\s*\}\}`), |
| if !hasSecretForward && job.Env != nil { | ||
| for key, value := range job.Env { | ||
| if strings.Contains(value, "secrets.") { | ||
| keyUpper := strings.ToUpper(key) | ||
| if strings.Contains(keyUpper, "KEY") || strings.Contains(keyUpper, "TOKEN") || | ||
| strings.Contains(keyUpper, "SECRET") || strings.Contains(keyUpper, "PASSWORD") { | ||
| // Only flag if the docker run command actually forwards this env var | ||
| if strings.Contains(step.Run, "-e "+key) || strings.Contains(step.Run, "--env "+key) || | ||
| strings.Contains(step.Run, "-e $"+key) || strings.Contains(step.Run, "--env $"+key) || | ||
| strings.Contains(step.Run, "-e ${"+key+"}") || strings.Contains(step.Run, "--env ${"+key+"}") { | ||
| hasSecretForward = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| // Matrix expansions can't be resolved statically — skip | ||
| if strings.Contains(runsOn, "${{ matrix.") { | ||
| return false | ||
| } | ||
| return !githubHostedRunners[runsOn] | ||
| // Other dynamic expressions (e.g. inputs.runner) may resolve to self-hosted | ||
| if strings.Contains(runsOn, "${{") { | ||
| return true | ||
| } | ||
| return !isManaged(runsOn) | ||
| case []interface{}: | ||
| for _, runner := range runsOn { | ||
| if runnerStr, ok := runner.(string); ok { | ||
| if strings.Contains(runnerStr, "${{") && strings.Contains(runnerStr, "matrix") { | ||
| return false // Conservative approach for matrix runners | ||
| // Matrix expansions can't be resolved statically — skip this label | ||
| if strings.Contains(runnerStr, "${{ matrix.") { | ||
| continue | ||
| } | ||
| // Other dynamic expressions may resolve to self-hosted | ||
| if strings.Contains(runnerStr, "${{") { |
| finding := Finding{ | ||
| RuleID: "DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODE", | ||
| RuleName: "Reusable Agent Workflow Called With Secrets on pull_request_target", | ||
| Description: "A pull_request_target workflow calls a reusable workflow that appears to run an AI agent or automated review, passing secrets. Verify whether the called workflow checks out the PR head ref — if it does, fork code runs in a container with secrets available, enabling exfiltration.", | ||
| Severity: severity, | ||
| Category: SecretExposure, | ||
| FilePath: workflow.Path, | ||
| JobName: jobName, | ||
| StepName: "", | ||
| Evidence: "uses: " + job.Uses, | ||
| Remediation: "Ensure downstream Docker containers use --network=none. Do not forward secrets into containers processing fork code. Require a maintainer label before running agents on fork PRs.", | ||
| LineNumber: 1, | ||
| } |
Fixes applied:
- AnalyzeWorkflow now infers RepositoryOwner by walking up to .git root
- checkIndirectDockerWithSecrets gated on jobCheckoutsUntrustedCode
- Added -e NAME=${{ secrets.FOO }} regex pattern (was only --env form)
- Check step.Env in addition to job.Env for docker secret forwarding
- Handle ${{matrix. (no space) in runs-on dynamic expression check
- Reusable workflow finding now uses lineMapper for accurate line numbers
Rejected (with reasoning):
- ctx cancellation mid-ExecuteRules: rules complete in ms, threading ctx
through every rule is massive scope creep for no practical benefit
- Step-level agent without checkout gate: wrong — checkAIAgentOnUntrustedCode
already has jobCheckoutsUntrustedCode gate at line 385
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var standardFindings []rules.Finding | ||
| // Use the RuleEngine for context-aware analysis (severity adjustment, suppression) | ||
| engine := rules.NewRuleEngine(job.Config) | ||
| standardFindings = engine.ExecuteRules(job.Workflow, job.StandardRules) |
| // Fallback: use repository directory name as a best-effort owner guess | ||
| return filepath.Base(repoPath) |
| for _, pattern := range dockerRunWithEnv { | ||
| if pattern.MatchString(step.Run) { | ||
| hasDockerEnv = true | ||
| break | ||
| } | ||
| } | ||
| if !hasDockerEnv { | ||
| continue | ||
| } | ||
|
|
||
| if networkNone.MatchString(step.Run) { | ||
| continue | ||
| } | ||
|
|
||
| hasSecretForward := false | ||
| for _, pattern := range secretEnvPatterns { | ||
| if pattern.MatchString(step.Run) { | ||
| hasSecretForward = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if !hasSecretForward && job.Env != nil { | ||
| for key, value := range job.Env { | ||
| if strings.Contains(value, "secrets.") { | ||
| keyUpper := strings.ToUpper(key) | ||
| if strings.Contains(keyUpper, "KEY") || strings.Contains(keyUpper, "TOKEN") || | ||
| strings.Contains(keyUpper, "SECRET") || strings.Contains(keyUpper, "PASSWORD") { | ||
| // Only flag if the docker run command actually forwards this env var | ||
| if strings.Contains(step.Run, "-e "+key) || strings.Contains(step.Run, "--env "+key) || | ||
| strings.Contains(step.Run, "-e $"+key) || strings.Contains(step.Run, "--env $"+key) || | ||
| strings.Contains(step.Run, "-e ${"+key+"}") || strings.Contains(step.Run, "--env ${"+key+"}") { | ||
| hasSecretForward = true | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
| // If this is a docker run with --network=none, it's mitigated | ||
| if isAIStep && networkNonePattern.MatchString(step.Run) { | ||
| continue |
| if strings.Contains(strings.ToUpper(value), "KEY") || | ||
| strings.Contains(strings.ToUpper(value), "TOKEN") { |
…secrets. check - Both fallback paths in detectRepositoryOwner now return "" instead of a directory name, so same-org checks are skipped rather than producing false matches against unrelated directory names - step.Env secret detection now checks for 'secrets.' reference instead of substring-matching KEY/TOKEN which caused false positives on values like 'tokenizer' or 'api-key-mode'
Summary
New Rules
DOCKER_EXEC_WITH_SECRETS_ON_FORK_CODEDetects
pull_request_targetworkflows that run Docker containers with secrets forwarded via-e/--envwhile operating on fork code (verified via untrusted checkout evidence). Only flags when:pull_request_targettrigger--network=noneAI_AGENT_ON_UNTRUSTED_CODEDetects AI agent invocations (Claude, Copilot, GPT, Aider, etc.) processing fork-controlled code with secrets available in
pull_request_targetworkflows. Now checks for--network=nonemitigation and resolves line numbers foruses:-based detections.AI_AGENT_COMMENT_TRIGGERED(NEW)Detects AI agents triggered by
issue_commentorpull_request_review_commentwithoutauthor_associationgating. Two attack tiers:Covers: Claude Code Action, Gemini, Copilot, CodeRabbit, Sourcery, Sweep, Aider, Devin, Qodo, Cursor + CLI variants.
Suppressed when workflow already gates on
author_association == 'MEMBER'/'OWNER'/'COLLABORATOR'.False Positive Reductions
isManagedto exact GitHub-hosted patterns (no longer matchesubuntu-self-hosted)issue_commentnow gated on write permissions (read-only is benign)pull_request_targetworkflows.github/workflows/is invalid syntax)job.Secretsfield instead of file-globalstrings.Contains("secrets:")Infrastructure Fixes
ctx.Done()cancellation check in concurrent scan goroutinedetectRepositoryOwner: falls back to directory name, handlesssh://URL formatrun:anduses:stepsTest Coverage
TestExternalTriggerPermissionsfor newissue_commentbehavior