Skip to content

ci: enable supareview pull request reviews - #782

Merged
khoi merged 3 commits into
mainfrom
chore/enable-supareview
Aug 7, 2026
Merged

ci: enable supareview pull request reviews#782
khoi merged 3 commits into
mainfrom
chore/enable-supareview

Conversation

@khoi

@khoi khoi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the SupaReview reusable workflow to Supacode. It reviews non-draft pull requests when they open, update, reopen, or leave draft state, with the required read and write permissions and the repository's CODEX_BALANCER_API_KEY secret.

Type of change

  • Bug fix (the linked issue is a bug report)
  • Feature (the linked issue is a feature request marked ready)
  • Documentation
  • Other (CI configuration)

How was this tested?

The workflow matches the existing Supaterm SupaReview configuration. Local validation passed:

  • make check passes (format + lint)
  • make test passes
  • I built and ran the app to confirm the change works

make build-app also passed locally.

AI tool disclosure (optional)

  • Model(s): GPT-5
  • Harness / tools: Codex

Checklist

  • This pull request is linked to an issue with Closes # above. Repository write-access exemption applies.
  • For a feature, the linked issue is labeled ready.
  • I am the author of this work and accountable for it; no commit is authored or co-authored by an AI agent.
  • I have read the Contributing guide and the Code of Conduct.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

SupaReview

Findings

The workflow is syntactically valid and covers the intended non-draft lifecycle events, but it introduces a privileged mutable supply-chain dependency and invokes a secret/write-dependent workflow for PR sources that cannot receive those credentials or permissions.

Findings

  • [P1] Pin the privileged reusable workflow to an immutable revision
    uses: supabitapp/supareview/.github/workflows/supareview-review.yml@main

    This calls a mutable @main workflow while granting it issues: write, pull-requests: write, and the repository's CODEX_BALANCER_API_KEY. A force-push, compromise, or accidental upstream change can therefore execute new code in Supacode's privileged context, exfiltrate the API key, or modify pull-request data without any change being reviewed here. Pin the reusable workflow to a reviewed full commit SHA and update it deliberately.
  • [P2] Handle fork and Dependabot pull requests before invoking SupaReview
    if: github.event.pull_request.draft == false

    The only job guard checks whether the pull request is draft, so fork and Dependabot pull requests also invoke this reusable workflow. GitHub withholds repository secrets for those pull_request runs and downgrades their token to read-only; the called workflow requires CODEX_BALANCER_API_KEY and needs write permissions to publish its review. Those PRs consequently cannot authenticate or post a review and can produce a failing/no-op check. Add an explicit safe skip/filter for unsupported contributors, or provide a safe handling path.

Review type: Full review

Reviewers: thermo-nuclear-review-subagent, thermo-nuclear-code-quality-review-subagent

@khoi

khoi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Accepting both findings. I will pin the reusable workflow to reviewed commit 79badffe75c017e9f66178e720570277c35a2013 and skip SupaReview for fork and Dependabot pull requests, since pull_request runs cannot receive the secret or write token there. Same-repository human pull requests keep the review path unchanged.

Pin the reusable review workflow to a reviewed revision and skip pull requests that cannot provide the review secret or write token.
@khoi

khoi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Changes since last review:

  • Pinned the SupaReview reusable workflow to 79badffe75c017e9f66178e720570277c35a2013.
  • Skip fork and Dependabot pull requests that cannot receive the secret or write token.

Commits: 8be9f054
Tests: git diff --check; prior make check, make build-app, and make test passed.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

SupaReview

Findings

The new condition correctly skips fork and Dependabot pull requests, and the caller reference is now a full SHA. However, that pinned reusable workflow still invokes a mutable @main action in the privileged, secret-bearing execution path, so the prior supply-chain risk remains incompletely fixed.

Findings

  • [P1] Pin the nested SupaReview action as well
    uses: supabitapp/supareview/.github/workflows/supareview-review.yml@79badffe75c017e9f66178e720570277c35a2013

    The caller is now pinned, but the reusable workflow at 79badffe75c017e9f66178e720570277c35a2013 still executes supabitapp/supareview/actions/supareview@main in its final step. That mutable action is run with this workflow's CODEX_BALANCER_API_KEY and issues/pull-requests write permissions, so an upstream change can still exfiltrate the secret or modify PR data. Pin the nested action too, or select a reviewed reusable-workflow revision that pins it, before treating the prior supply-chain finding as fixed.

Review type: Incremental review since a972792...8be9f05

Reviewers: thermo-nuclear-review-subagent, thermo-nuclear-code-quality-review-subagent

@khoi

khoi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Accepting the remaining finding. SupaReview PR #4 is merged as f1ed7e0db53c98de048c9fab7439744d0000417d; its reusable workflow now pins the nested action to reviewed commit 79badffe75c017e9f66178e720570277c35a2013. I am updating Supacode to call that fixed reusable-workflow revision.

Use the merged SupaReview workflow revision that pins its nested privileged action.
@khoi

khoi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Changes since last review:

Commit: 79d169c2
Tests: git diff --check; prior make check, make build-app, and make test passed.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

SupaReview

LGTM

The only new change replaces the reusable-workflow reference with immutable SHA f1ed7e0d. That revision also pins the nested SupaReview action to immutable SHA 79badffe, resolving the remaining supply-chain concern. The existing fork and Dependabot guards remain intact, and no new issue is introduced by this delta.

Review type: Incremental review since 8be9f05...79d169c

Reviewers: thermo-nuclear-review-subagent, thermo-nuclear-code-quality-review-subagent

@khoi
khoi merged commit f1961ac into main Aug 7, 2026
4 checks passed
@khoi
khoi deleted the chore/enable-supareview branch August 7, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant