@@ -2557,6 +2557,114 @@ def test_notice_caps_output_at_10_files(
25572557 assert "and 15 more" in err
25582558
25592559
2560+ class TestWorkflowForkSkip :
2561+ """The AI review workflow must skip PRs from forks to avoid the
2562+ untrusted-checkout pattern that CodeQL flagged as alerts #11 and #12.
2563+ Two-layer skip:
2564+ 1. Workflow-level `if:` gates `pull_request` events on
2565+ `head.repo.full_name == github.repository`
2566+ 2. The resolve-pr step sets `is_fork` output (via API fetch);
2567+ all 7 post-resolve steps gate on `is_fork == 'false'`.
2568+
2569+ These contract tests pin both layers — without them, a future workflow
2570+ refactor could drop the gate and re-introduce the CodeQL alerts."""
2571+
2572+ @pytest .fixture
2573+ def workflow_text (self ):
2574+ assert _SCRIPT_PATH is not None
2575+ repo_root = _SCRIPT_PATH .parent .parent .parent
2576+ wf = repo_root / ".github" / "workflows" / "ai_pr_review.yml"
2577+ if not wf .exists ():
2578+ pytest .skip ("workflow not found" )
2579+ return wf .read_text ()
2580+
2581+ def test_workflow_pull_request_if_block_excludes_fork_prs (self , workflow_text ):
2582+ """Layer 1: the workflow `if:` block for `pull_request` events must
2583+ require head.repo.full_name == github.repository so fork PRs never
2584+ start a workflow run."""
2585+ assert (
2586+ "github.event.pull_request.head.repo.full_name == github.repository"
2587+ in workflow_text
2588+ ), (
2589+ "workflow `if:` for pull_request events must check that the PR "
2590+ "head is from the same repo (not a fork) — required to clear "
2591+ "CodeQL alerts #11/#12 (untrusted checkout)."
2592+ )
2593+
2594+ def test_workflow_resolve_pr_step_sets_is_fork_output (self , workflow_text ):
2595+ """Layer 2: the resolve-pr github-script step must set the `is_fork`
2596+ output that subsequent steps gate on. Comment-triggered events
2597+ (`issue_comment`, `pull_request_review_comment`) can't be gated at
2598+ the workflow `if:` level (event payload doesn't include head-repo
2599+ info), so the gate happens at the step level via this output."""
2600+ assert 'core.setOutput("is_fork"' in workflow_text , (
2601+ "resolve-pr step must set `is_fork` output so post-resolve steps "
2602+ "can gate on `steps.pr.outputs.is_fork == 'false'`."
2603+ )
2604+
2605+ def test_workflow_post_resolve_steps_gated_on_is_fork (self , workflow_text ):
2606+ """Every step in the `review` job that runs AFTER the `resolve-pr`
2607+ step must include `steps.pr.outputs.is_fork == 'false'` in its
2608+ `if:` clause.
2609+
2610+ Per CodeQL alerts #11/#12, no step that could touch untrusted PR
2611+ contents (or run while OPENAI_API_KEY is in scope) may execute
2612+ on a fork PR. The resolve-pr step itself only API-fetches PR
2613+ metadata via GITHUB_TOKEN — safe to run before the gate is
2614+ computed. Every step after must be gated.
2615+
2616+ The earlier (PR #427 R0) version of this test counted the string
2617+ `is_fork == 'false'` globally with `>= 7`, which had two false-
2618+ negative modes:
2619+ (a) a real gate could be removed — string count drops 8→7,
2620+ still passes
2621+ (b) a new ungated post-resolve step could be added — gate
2622+ count stays at 7, total step count grows, passes
2623+
2624+ This rewrite (R1, addressing the reviewer's P3) anchors on:
2625+ - `^ if:` at 8-space indent (the step-property indent
2626+ level for the review job's nested `if:` keys), excluding
2627+ the JS doc comment inside the resolve-pr step's `script: |`
2628+ block which would not match this anchor
2629+ - `^ - (name|uses):` at 6-space indent (step-list-item
2630+ indent), counting every step in the job
2631+
2632+ Then asserts `gated_steps == total_steps - 1` (resolve-pr is the
2633+ only legitimately ungated step). Catches both failure modes
2634+ above."""
2635+ import re
2636+
2637+ # `if:` lines at step-property indent (8 spaces) containing the
2638+ # gate. Allows combined conditions like
2639+ # `if: steps.pr.outputs.state == 'open' && steps.pr.outputs.is_fork == 'false'`.
2640+ gate_re = re .compile (
2641+ r"^ if:.*is_fork == 'false'" , re .MULTILINE
2642+ )
2643+ gates = gate_re .findall (workflow_text )
2644+
2645+ # All step starts in the review job (` - name:` or
2646+ # ` - uses:` at 6-space indent).
2647+ step_start_re = re .compile (
2648+ r"^ - (?:name|uses):" , re .MULTILINE
2649+ )
2650+ steps = step_start_re .findall (workflow_text )
2651+
2652+ # The resolve-pr step is the only ungated step (it sets the
2653+ # output that all subsequent steps gate on).
2654+ expected_gates = len (steps ) - 1
2655+ assert len (gates ) == expected_gates , (
2656+ f"Fork-skip gate invariant violated: found { len (gates )}
2657+ f"gated step(s) but { len (steps )}
2658+ f"`review` job — expected exactly { expected_gates }
2659+ f"(every step except resolve-pr must include "
2660+ f"`is_fork == 'false'` in its `if:`). Either a gate was "
2661+ f"removed or a new post-resolve step was added without one. "
2662+ f"Per CodeQL alerts #11/#12, every post-resolve step must "
2663+ f"be gated to prevent untrusted-checkout execution on fork "
2664+ f"PRs."
2665+ )
2666+
2667+
25602668class TestExtractResponseText :
25612669 def test_prefers_output_text_field (self , review_mod ):
25622670 result = {"output_text" : "Direct text." , "output" : []}
0 commit comments