Skip to content

ci: name the code-review config instead of dumping every secret - #659

Open
titouanmathis wants to merge 1 commit into
2.xfrom
fix/code-review-workflow-blocked
Open

titouanmathis wants to merge 1 commit into
2.xfrom
fix/code-review-workflow-blocked

Conversation

@titouanmathis

Copy link
Copy Markdown
Contributor

The code-review workflow has not run on a pull request since 2026-09-03. Every run since is created and then completed instantly with conclusion action_required and zero jobs, so no check ever appears on the pull request.

Cause

GitHub's workflow scanning flagged the file. The run page carries this banner:

GitHub detected that this workflow file may be malicious. It will not run until someone with write access approves it. Review the workflow file carefully before approving.

The pattern it matches is the config-forwarding step, which serialised the whole vars and secrets contexts with toJSON() and appended the result to $GITHUB_ENV. Dumping every secret into the environment is the shape of an exfiltration attempt, so the heuristic cannot tell it apart from one.

The file itself has not changed since 2026-07-23, and runs succeeded through 2026-09-02. GitHub's detection changed, not the workflow.

This is not a fork-approval gate, and the REST endpoint refuses it:

This run is not from a fork pull request or queued by the Actions bot (HTTP 403)

Ruled out along the way: the workflow is active, the repo and the org allow all actions, weareikko/code-review@0.8.2 exists and is public, the repo is public so billing does not gate it, and the three other workflows created in the same second on the same pull requests all ran.

Change

Each value is now named, and declared at the job rather than the step, because a composite action's own steps inherit the job environment. The CLI still reads CODE_REVIEW_* from ambient environment and de-prefixes CODE_REVIEW_<PROVIDER>_* credentials, so weareikko/code-review needs no change.

The 12 entries cover every CODE_REVIEW_* org secret and variable that exists today. CODE_REVIEW_MODEL stays the model input, which the action uses to set that variable itself.

Trade-off

The old step existed so a new option could be configured purely at the org level. That property is gone: adding an option now costs one line here. That is the price of not matching the exfiltration pattern, and naming what a workflow reads is the safer default regardless.

Verification

Merging this is the test. If the fix works, this pull request's own code-review run is held (it is built from the base branch's flagged file) but the next pull request opened after the merge runs the review normally.

Note

main carries a byte-identical copy of this workflow and needs the same change for pull requests targeting the v1 line.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LMSCm41fm3g7chxD728vAu

The workflow serialised the whole `vars` and `secrets` contexts with
`toJSON()` and appended the result to $GITHUB_ENV, so that a new review
option could be configured at the org level with no change here.

GitHub's workflow scanning reads that as an exfiltration attempt. It
flags the file and holds every run as `action_required` with no job ever
created, until someone with write access approves that run by hand. No
review has run on a pull request since 2026-09-03 for that reason, while
every other workflow on the same pull requests ran normally.

Declare each value instead, at the job rather than the step, because a
composite action's own steps inherit the job environment. The CLI still
reads CODE_REVIEW_* from ambient environment, so the action needs no
change. The cost is one line here when the org gains an option.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LMSCm41fm3g7chxD728vAu
@github-actions

Copy link
Copy Markdown

Code Review

Risk: Low — The workflow now names each required variable and secret explicitly, removing the broad context serialization that triggered GitHub's workflow scanning. The composite action still receives the configuration through the job environment, and the model remains supplied through the action input.

The change is safe to merge for this workflow; no blocking issues were found.

Notes:
The described byte-identical workflow copy on main for the v1 line is not included in this diff and will require the same update separately.


Review usage: 5,713 in (3,677 cached) / 477 out tokens — $0.0029 (openrouter/openai/gpt-5.6-luna, thinking: low)

Reviewed by @weareikko/code-review v0.9.5 for commit bda9039.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.32%. Comparing base (2d5253b) to head (bda9039).
⚠️ Report is 8 commits behind head on 2.x.

Additional details and impacted files
@@            Coverage Diff            @@
##                2.x     #659   +/-   ##
=========================================
  Coverage     86.32%   86.32%           
  Complexity      145      145           
=========================================
  Files            20       20           
  Lines           746      746           
  Branches         88       88           
=========================================
  Hits            644      644           
  Misses           95       95           
  Partials          7        7           
Flag Coverage Δ
unittests 86.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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