Skip to content

Commit 3252213

Browse files
north-echoclaude
andcommitted
feat(rules): refine FG-001 with post-checkout execution analysis
Split FG-001 severity into critical (confirmed/likely code execution) and high (pattern present, no execution detected). Analyzes run: blocks and uses: steps after the fork checkout to determine whether attacker-controlled code actually executes. - Add confirmed/likely/pattern-only confidence levels - Detect build tools (npm, mvn, cargo, go, make, etc.) - Detect config-loading tools (eslint, jest, webpack, etc.) - Detect build actions (docker/build-push-action, etc.) - Add 4 new test fixtures covering each severity tier - Add confidence field to JSON output Motivated by early disclosure feedback: a workflow had the Pwn Request pattern but only ran read-only operations, not code execution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 286bd58 commit 3252213

File tree

7 files changed

+395
-26
lines changed

7 files changed

+395
-26
lines changed

internal/scanner/finding.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,22 @@ const (
1111
SeverityInfo = "info"
1212
)
1313

14+
// Confidence levels for findings.
15+
const (
16+
ConfidenceConfirmed = "confirmed"
17+
ConfidenceLikely = "likely"
18+
ConfidencePatternOnly = "pattern-only"
19+
)
20+
1421
// Finding represents a single security finding in a workflow file.
1522
type Finding struct {
16-
RuleID string `json:"rule_id"`
17-
Severity string `json:"severity"`
18-
File string `json:"file"`
19-
Line int `json:"line"`
20-
Message string `json:"message"`
21-
Details string `json:"details,omitempty"`
23+
RuleID string `json:"rule_id"`
24+
Severity string `json:"severity"`
25+
Confidence string `json:"confidence,omitempty"`
26+
File string `json:"file"`
27+
Line int `json:"line"`
28+
Message string `json:"message"`
29+
Details string `json:"details,omitempty"`
2230
}
2331

2432
func (f Finding) String() string {

internal/scanner/rules.go

Lines changed: 256 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,39 +29,276 @@ var RuleDescriptions = map[string]string{
2929
"FG-005": "Secrets in Logs",
3030
}
3131

32+
// ExecutionAnalysis captures the result of post-checkout step analysis.
33+
type ExecutionAnalysis struct {
34+
Confirmed bool
35+
Likely bool
36+
Detail string
37+
}
38+
39+
// Build tools that definitely execute code from the working directory.
40+
var confirmedBuildCommands = []string{
41+
"npm install", "npm ci", "npm run", "npm test", "npm start",
42+
"yarn install", "yarn run", "yarn test", "yarn build",
43+
"pnpm install", "pnpm run", "pnpm test",
44+
"pip install", "poetry install",
45+
"bundle install", "bundle exec",
46+
"cargo build", "cargo test", "cargo run",
47+
"go build", "go test", "go run",
48+
"make", "cmake",
49+
"mvn", "gradle", "ant",
50+
"dotnet build", "dotnet test", "dotnet run",
51+
"docker build",
52+
}
53+
54+
// Tools that load and execute config files from the repo.
55+
var configLoadingTools = []string{
56+
"eslint", "prettier", "jest", "vitest",
57+
"webpack", "rollup", "vite", "next", "nuxt",
58+
"pytest", "tox", "nox",
59+
"tsup", "esbuild",
60+
}
61+
62+
// Actions known to execute code from the working directory.
63+
var buildActions = []string{
64+
"docker/build-push-action",
65+
"gradle/gradle-build-action",
66+
"borales/actions-yarn",
67+
"bahmutov/npm-install",
68+
}
69+
70+
// Read-only commands that don't execute checked-out code.
71+
var readOnlyCommands = []string{
72+
"diff", "cmp", "cat", "grep", "head", "tail", "wc",
73+
"sha256sum", "md5sum", "jq", "yq",
74+
"gh pr comment", "gh pr review", "gh issue comment",
75+
"test -f", "[ -f", "stat", "file", "ls",
76+
"echo", "printf",
77+
}
78+
3279
// CheckPwnRequest detects pull_request_target workflows that checkout PR head
33-
// code and have elevated permissions or secret access (FG-001).
80+
// code with post-checkout execution analysis (FG-001).
3481
func CheckPwnRequest(wf *Workflow) []Finding {
3582
if !wf.On.PullRequestTarget {
3683
return nil
3784
}
3885

3986
var findings []Finding
4087
for _, job := range wf.Jobs {
41-
for _, step := range job.Steps {
42-
if !isCheckoutAction(step.Uses) {
43-
continue
88+
checkoutIdx := -1
89+
checkoutLine := 0
90+
checkoutRef := ""
91+
92+
for i, step := range job.Steps {
93+
if isCheckoutAction(step.Uses) && refPointsToPRHead(step.With["ref"]) {
94+
checkoutIdx = i
95+
checkoutLine = step.Line
96+
checkoutRef = step.With["ref"]
97+
break
98+
}
99+
}
100+
101+
if checkoutIdx == -1 {
102+
continue
103+
}
104+
105+
// Analyze all steps after the checkout
106+
postCheckoutSteps := job.Steps[checkoutIdx+1:]
107+
execResult := analyzePostCheckoutExecution(postCheckoutSteps)
108+
109+
severity := SeverityHigh
110+
confidence := ConfidencePatternOnly
111+
112+
if execResult.Confirmed {
113+
severity = SeverityCritical
114+
confidence = ConfidenceConfirmed
115+
} else if execResult.Likely {
116+
severity = SeverityCritical
117+
confidence = ConfidenceLikely
118+
}
119+
120+
msg := fmt.Sprintf("Pwn Request: pull_request_target with fork checkout [%s]", confidence)
121+
if execResult.Detail != "" {
122+
msg += " — " + execResult.Detail
123+
}
124+
125+
permDesc := describePermissions(wf.Permissions, job.Permissions)
126+
details := fmt.Sprintf(
127+
"Trigger: pull_request_target, Checkout ref: %s, Permissions: %s, Execution: %s",
128+
checkoutRef, permDesc, confidence,
129+
)
130+
131+
findings = append(findings, Finding{
132+
RuleID: "FG-001",
133+
Severity: severity,
134+
Confidence: confidence,
135+
File: wf.Path,
136+
Line: checkoutLine,
137+
Message: msg,
138+
Details: details,
139+
})
140+
}
141+
return findings
142+
}
143+
144+
// analyzePostCheckoutExecution checks whether steps after a fork checkout
145+
// execute code from the working directory.
146+
func analyzePostCheckoutExecution(steps []Step) ExecutionAnalysis {
147+
hasSetupAction := false
148+
149+
for _, step := range steps {
150+
// Check uses: for known build actions
151+
if step.Uses != "" {
152+
actionName := step.Uses
153+
if idx := strings.Index(actionName, "@"); idx != -1 {
154+
actionName = actionName[:idx]
155+
}
156+
157+
for _, ba := range buildActions {
158+
if actionName == ba {
159+
return ExecutionAnalysis{
160+
Confirmed: true,
161+
Detail: fmt.Sprintf("action '%s' executes repo code (line %d)", actionName, step.Line),
162+
}
163+
}
164+
}
165+
166+
if strings.HasPrefix(actionName, "actions/setup-") {
167+
hasSetupAction = true
168+
}
169+
}
170+
171+
// Check run: blocks
172+
if step.Run != "" {
173+
if cmd, found := matchesBuildCommand(step.Run); found {
174+
return ExecutionAnalysis{
175+
Confirmed: true,
176+
Detail: fmt.Sprintf("run block executes '%s' on checked-out code (line %d)", cmd, step.Line),
177+
}
178+
}
179+
if cmd, found := matchesConfigLoadingTool(step.Run); found {
180+
return ExecutionAnalysis{
181+
Likely: true,
182+
Detail: fmt.Sprintf("run block invokes '%s' which loads config from repo (line %d)", cmd, step.Line),
183+
}
184+
}
185+
// If there's a setup action and a run block, it likely executes repo code
186+
if hasSetupAction && !isReadOnlyRun(step.Run) {
187+
return ExecutionAnalysis{
188+
Likely: true,
189+
Detail: fmt.Sprintf("run block after setup action may execute repo code (line %d)", step.Line),
190+
}
191+
}
192+
}
193+
}
194+
195+
return ExecutionAnalysis{
196+
Confirmed: false,
197+
Likely: false,
198+
Detail: "no code execution detected in post-checkout steps",
199+
}
200+
}
201+
202+
// splitShellCommands splits a shell line on &&, ||, ;, and |.
203+
func splitShellCommands(line string) []string {
204+
var segments []string
205+
current := ""
206+
i := 0
207+
for i < len(line) {
208+
if i+1 < len(line) && (line[i:i+2] == "&&" || line[i:i+2] == "||") {
209+
segments = append(segments, current)
210+
current = ""
211+
i += 2
212+
continue
213+
}
214+
if line[i] == ';' || line[i] == '|' {
215+
segments = append(segments, current)
216+
current = ""
217+
i++
218+
continue
219+
}
220+
current += string(line[i])
221+
i++
222+
}
223+
if current != "" {
224+
segments = append(segments, current)
225+
}
226+
return segments
227+
}
228+
229+
// matchesBuildCommand checks if a run block contains a known build command.
230+
func matchesBuildCommand(run string) (string, bool) {
231+
lines := strings.Split(run, "\n")
232+
for _, line := range lines {
233+
line = strings.TrimSpace(line)
234+
if line == "" || strings.HasPrefix(line, "#") {
235+
continue
236+
}
237+
segments := splitShellCommands(line)
238+
for _, seg := range segments {
239+
seg = strings.TrimSpace(seg)
240+
for _, cmd := range confirmedBuildCommands {
241+
if strings.HasPrefix(seg, cmd+" ") || seg == cmd {
242+
return cmd, true
243+
}
244+
}
245+
// Check for relative path execution
246+
if strings.HasPrefix(seg, "./") {
247+
return seg, true
248+
}
249+
}
250+
}
251+
return "", false
252+
}
253+
254+
// matchesConfigLoadingTool checks if a run block invokes a config-loading tool.
255+
func matchesConfigLoadingTool(run string) (string, bool) {
256+
lines := strings.Split(run, "\n")
257+
for _, line := range lines {
258+
line = strings.TrimSpace(line)
259+
if line == "" || strings.HasPrefix(line, "#") {
260+
continue
261+
}
262+
segments := splitShellCommands(line)
263+
for _, seg := range segments {
264+
seg = strings.TrimSpace(seg)
265+
for _, tool := range configLoadingTools {
266+
if strings.HasPrefix(seg, tool+" ") || seg == tool {
267+
return tool, true
268+
}
44269
}
45-
ref := step.With["ref"]
46-
if !refPointsToPRHead(ref) {
270+
}
271+
}
272+
return "", false
273+
}
274+
275+
// isReadOnlyRun checks if a run block only contains read-only commands.
276+
func isReadOnlyRun(run string) bool {
277+
lines := strings.Split(run, "\n")
278+
for _, line := range lines {
279+
line = strings.TrimSpace(line)
280+
if line == "" || strings.HasPrefix(line, "#") {
281+
continue
282+
}
283+
segments := splitShellCommands(line)
284+
for _, seg := range segments {
285+
seg = strings.TrimSpace(seg)
286+
if seg == "" {
47287
continue
48288
}
49-
if HasElevatedPermissions(wf.Permissions, job.Permissions) || AccessesSecrets(job) {
50-
findings = append(findings, Finding{
51-
RuleID: "FG-001",
52-
Severity: SeverityCritical,
53-
File: wf.Path,
54-
Line: step.Line,
55-
Message: "Pwn Request: pull_request_target with fork checkout and secret access",
56-
Details: fmt.Sprintf(
57-
"Trigger: pull_request_target, Checkout ref: %s, Permissions: %s",
58-
ref, describePermissions(wf.Permissions, job.Permissions),
59-
),
60-
})
289+
isReadOnly := false
290+
for _, ro := range readOnlyCommands {
291+
if strings.HasPrefix(seg, ro+" ") || seg == ro {
292+
isReadOnly = true
293+
break
294+
}
295+
}
296+
if !isReadOnly {
297+
return false
61298
}
62299
}
63300
}
64-
return findings
301+
return true
65302
}
66303

67304
// CheckScriptInjection detects attacker-controllable expressions used in

0 commit comments

Comments
 (0)