diff --git a/.github/workflows/guardian.yml b/.github/workflows/guardian.yml index 8a8e4e82..3c7a8ac6 100644 --- a/.github/workflows/guardian.yml +++ b/.github/workflows/guardian.yml @@ -13,6 +13,9 @@ jobs: github.event.issue.pull_request != null && github.event.comment.body == '/guardian review' runs-on: ubuntu-latest + # Below the app token's one-hour lifetime (#734): a review cannot outlive + # the token it posts with. + timeout-minutes: 45 permissions: contents: read pull-requests: write @@ -98,6 +101,25 @@ jobs: git show origin/data/guardian-metrics:guardian_metrics.jsonl \ > guardian_metrics.jsonl 2>/dev/null || true + # Post as the codegraph-brain GitHub App when it is configured (#734): + # set vars.GUARDIAN_APP_CLIENT_ID and secrets.GUARDIAN_APP_PRIVATE_KEY. + # With the variable unset this step is skipped and posting uses + # GITHUB_TOKEN, as before. With it set, a missing key, a wrong key or an + # app not installed here fails the job before any LLM call: no silent + # fallback to another identity. The app's key reaches every repository + # the app is installed on, so the token is narrowed to this one and to + # pull-requests: write, and only the two posting steps read it. It lives + # one hour; the job's timeout-minutes keeps the review inside that. + - name: Mint GitHub App token + id: app-token + if: vars.GUARDIAN_APP_CLIENT_ID != '' + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.GUARDIAN_APP_CLIENT_ID }} + private-key: ${{ secrets.GUARDIAN_APP_PRIVATE_KEY }} + repositories: ${{ github.event.repository.name }} + permission-pull-requests: write + - name: Run Guardian Review id: guardian env: @@ -109,7 +131,7 @@ jobs: # Thinking depth for an anthropic finder or skeptic; unset = the # provider's default ("low"). #246 J measured the finder at "medium". GUARDIAN_ANTHROPIC_EFFORT: ${{ vars.GUARDIAN_ANTHROPIC_EFFORT }} - GH_TOKEN: ${{ github.token }} + GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} GITHUB_REPOSITORY: ${{ github.repository }} GUARDIAN_FEATURES: ${{ vars.GUARDIAN_FEATURES }} GUARDIAN_SKEPTIC: ${{ vars.GUARDIAN_SKEPTIC }} @@ -147,6 +169,7 @@ jobs: if: steps.guardian.outputs.posted_inline != 'true' uses: peter-evans/create-or-update-comment@a111a2c3bacd7be7898ee22d0d71d9aec2bb972c # v4 with: + token: ${{ steps.app-token.outputs.token || github.token }} issue-number: ${{ env.PR_NUMBER }} body-path: guardian_report.md diff --git a/tests/unit/test_guardian_workflow.py b/tests/unit/test_guardian_workflow.py index 468a56c1..bb07654e 100644 --- a/tests/unit/test_guardian_workflow.py +++ b/tests/unit/test_guardian_workflow.py @@ -131,3 +131,69 @@ def test_the_evidence_flag_reaches_no_level_of_the_review_job( levels = [workflow.get("env") or {}, workflow["jobs"]["review"].get("env") or {}] levels += [s.get("env") or {} for s in steps] assert not any("GUARDIAN_EVIDENCE" in env for env in levels) + + +# Reviews post as the codegraph-brain GitHub App when it is configured (#734). +# Its private key is stronger than GITHUB_TOKEN — it reaches every repository +# the app is installed on — so the token minted from it is narrowed to this +# repository and to the one permission posting needs. +APP_TOKEN_ACTION = "actions/create-github-app-token@" +APP_TOKEN = "${{ steps.app-token.outputs.token || github.token }}" + + +def _app_token_step(steps: list[dict[str, Any]]) -> dict[str, Any]: + minted = [s for s in steps if str(s.get("uses", "")).startswith(APP_TOKEN_ACTION)] + assert len(minted) == 1 + return minted[0] + + +def test_the_app_token_is_minted_only_when_configured(steps: list[dict[str, Any]]) -> None: + """Without the app's client id the step is skipped and posting falls back to GITHUB_TOKEN.""" + step = _app_token_step(steps) + assert step.get("id") == "app-token" + assert step.get("if") == "vars.GUARDIAN_APP_CLIENT_ID != ''" + assert re.fullmatch(r"actions/create-github-app-token@[0-9a-f]{40}", step["uses"]) + assert _with(step)["client-id"] == "${{ vars.GUARDIAN_APP_CLIENT_ID }}" + assert _with(step)["private-key"] == "${{ secrets.GUARDIAN_APP_PRIVATE_KEY }}" + + +def test_the_app_token_reaches_this_repository_and_pull_requests_only( + steps: list[dict[str, Any]], +) -> None: + """Scope and permissions are named, not inherited from whatever the app was granted.""" + granted = _with(_app_token_step(steps)) + assert granted["repositories"] == "${{ github.event.repository.name }}" + permissions = {k: v for k, v in granted.items() if k.startswith("permission-")} + assert permissions == {"permission-pull-requests": "write"} + + +def test_the_review_posts_with_the_app_token_or_falls_back(steps: list[dict[str, Any]]) -> None: + """Both posting paths — inline review and fallback comment — use the same token.""" + review = next(s for s in steps if s.get("name") == "Run Guardian Review") + assert review["env"]["GH_TOKEN"] == APP_TOKEN + comment = next(s for s in steps if s.get("name") == "Post review as PR comment") + assert _with(comment).get("token") == APP_TOKEN + + +def test_only_the_posting_steps_reference_the_app_token(steps: list[dict[str, Any]]) -> None: + """Where the token is minted matters less than who can read it: the two posting steps only.""" + holders = [s.get("name") for s in steps if "steps.app-token" in yaml.safe_dump(s)] + assert holders == ["Run Guardian Review", "Post review as PR comment"] + + +def test_the_app_token_is_minted_after_every_pr_step(steps: list[dict[str, Any]]) -> None: + """Nothing that checks out or runs in the PR checkout comes after the token exists.""" + minted = steps.index(_app_token_step(steps)) + last_pr = max( + i + for i, s in enumerate(steps) + if _in_pr(s.get("working-directory")) + or "refs/pull/" in str(_with(s).get("ref", "")) + or f"{PR_DIR}/" in str(s.get("run", "")) + ) + assert minted > last_pr + + +def test_the_review_job_ends_before_the_app_token_expires(workflow: dict[Any, Any]) -> None: + """An installation token lives 60 minutes; a longer job would post with an expired one.""" + assert workflow["jobs"]["review"]["timeout-minutes"] < 60