Repository navigation
ci(pullfrog): pin the action and make the reviewer review-only - #306
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 31 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: fabiodalez-dev/FAZ-Cookie-Manager/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: fabiodalez-dev/FAZ-Cookie-Manager/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. WalkthroughIl workflow fissa a SHA le revisioni di ChangesWorkflow Pullfrog
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The workflow’s read-only and credential-isolation settings match the pinned actions’ contracts, with no established PR-introduced issue blocking merge. Architecture SummaryArchitecture risk: 🔵 Low · up to The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency. Changed systems: None identified. Architecture concerns Review detailsBefore / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two must-address items before this can merge: the file trips the repo's own diff-hygiene gate, and the branch currently conflicts with main because main already contains the generated version of this same path. Details below.
Reviewed changes
The Pullfrog reviewer workflow was reviewed end to end: the action refs, the review-only inputs, the job permissions, and the CI interaction.
- Commit-SHA pinning —
pullfrog/pullfrogandactions/checkoutare pinned to full 40-char SHAs. The action SHA resolves to npmpullfrog@0.1.90;actions/checkout@34e11487… # v4matches the sibling workflows. - Review-only inputs —
push: disabledandshell: restrictedare both declared in the action'saction.ymlat the pinned SHA.shell: restrictedscrubs*_TOKEN/*_KEY/*_SECRET/*_PASSWORD/*_CREDENTIAL, which also coversACTIONS_ID_TOKEN_REQUEST_TOKEN. - Permissions — workflow-level
contents: read; the job addsid-token: write(the action authenticates to Pullfrog over OIDC) pluscontents: read. Norun:steps and no untrusted event data, so there is no shell interpolation surface.
⚠️ The branch conflicts with main, which already contains a pullfrog.yml
main HEAD (1532818, "Add pullfrog.yml workflow") already adds .github/workflows/pullfrog.yml — the same path this PR adds, forked from 4f44d467. GitHub reports mergeable: false / mergeable_state: dirty, so no merge commit can be built and pull_request-triggered workflows (Quality, Plugin Check, CodeQL) are not running on this PR at all — only the external apps (CodeRabbit, Semgrep, Pullfrog) appear. Resolve the conflict by rebasing onto current main and keeping this hardened version, so the repo's own gates actually run before merge.
Technical details
# Branch conflicts with main on an add/add of the same path
## Affected sites
- `.github/workflows/pullfrog.yml` — added by this PR; already present on `main` at `1532818` (in the unpinned, generated form).
- Merge base: `4f44d4677ec165525c1ad252c94a93723b7c355d`; `main` tip: `1532818`.
## Required outcome
- Rebase/merge `main` into the branch, resolving the add/add conflict in favor of this PR's hardened file.
- Confirm the PR becomes mergeable so `Quality`, `Plugin Check`, and `CodeQL` run on it.
## Open question for the human
- Since the generated (unpinned) workflow is already live on `main`, is this PR intended to replace it, or should the two coexist?Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
Three changes to the workflow the Pullfrog console generated, each for a reason that applies to this repository specifically. `push: disabled`. The input defaults to `enabled`, which lets the agent push branches and open pull requests of its own. Every commit and pull request here is authored by its maintainer, so the agent gets no write path at all: it reads the diff and comments. The input is declared in the action's own action.yml and is not mentioned in the getting-started guide, so the generated file leaves it at the permissive default. Pinned to a commit SHA instead of `@v0`. That tag moves — it has already advanced through ninety v0.1.x releases — and this is the one action here that runs an agent with repository access, so a mutable ref is the last place to accept one. Pullfrog's versioning page supports pinning and notes nothing is lost: the action fetches its steps from npm either way, so the agent still tracks patch releases without this file changing. `shell: restricted`, which scrubs *_TOKEN / *_KEY / *_SECRET / *_PASSWORD / *_CREDENTIAL from the environment the agent's shell sees. Already the default for a public repository; stated explicitly so that making this repository private some day cannot silently widen it, since private repos default to `enabled`. The provider `env:` block is left exactly as generated. Unset secrets resolve to empty strings, and the action's sanitizeSecret() leaves the variable untouched in that case rather than treating it as a key, so the block is inert until a key is actually added and it cannot shadow keys held in Pullfrog's own store or the Router. Nothing else is touched: no `run:` steps, so no shell interpolation surface, and no workflow expression reads untrusted event data.
218afc7 to
d20c1e0
Compare
fabiodalez-dev
changed the title
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (218afc7): the branch was rebased onto current main, resolving both must-address items from that review. The workflow file's content is otherwise unchanged.
- Rebased onto current
main(1532818) — the branch now sits directly on the commit that already carried the generatedpullfrog.yml, so the previous add/add conflict is gone. GitHub reports the PRmergeable: trueand thepull_requestgates (Quality,Plugin Check,CodeQL) run against it. - Diff-hygiene failure resolved — the trailing-whitespace lines that previously tripped
git diff --check HEAD^are now inherited frommainand are no longer part of this PR's diff; a localgit diff --check HEAD^on the branch is clean. - Pinned refs and review-only inputs unchanged —
push: disabled,shell: restricted, and the full-SHA pins forpullfrog/pullfrogandactions/checkoutare identical to the previously reviewed and verified version.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
actions/checkout defaults to persist-credentials: true, which writes the job's GITHUB_TOKEN into .git/config. That leaves a working push credential inside the very checkout the agent then operates on, so it contradicts the `push: disabled` set alongside it: the input closes the action's own push path and the token reopens it from underneath. Caught by Pinakes' CI policy check — "pullfrog.yml has an actions/checkout step that persists GitHub credentials" — which refuses any checkout that persists credentials. It was right, and the same omission was in all of these repositories, so the fix goes everywhere rather than only where a check happened to be watching. Nothing needs the credential: the agent reaches GitHub through the App's installation token, not the local git remote, and these repositories are public, so an unauthenticated fetch works.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (d20c1e0): one new commit that hardens the checkout step.
- Checkout no longer persists credentials —
persist-credentials: falsewas added to theactions/checkoutstep, with a comment explaining that the default (true) leaves the job'sGITHUB_TOKENin.git/config, which reopens a push path thatpush: disabledis meant to close.
The added input is valid for actions/checkout@v4 and matches the sibling workflows' convention. Nothing in this workflow needs the credential: push: disabled means the agent never pushes, and the agent reaches GitHub through the App's installation token rather than the local remote. The commit is whitespace-clean (git diff --check) and the PR reports mergeable: true. All prior Pullfrog threads are already resolved.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
The reviewer workflow was the only file in .github failing the Workflow, YAML and shell security gate. Four separate problems, all real: - zizmor (pedantic) wants every write permission documented, and it reads a trailing comment on the permission line, not a block above it. id-token: write now says on its own line what it is for: an OIDC identity for Pullfrog token service, which grants nothing on the repository. - The job had no name, so it showed up anonymously in the checks list. - The workflow had no concurrency group. It was left out on purpose, because every run is dispatched against the default branch and one shared group would make two reviews cancel each other. The group is keyed on the dispatched run name instead, so a fresh review of the same pull request supersedes a stale one while different pull requests never collide, and it falls back to the unique run id when no name is passed. - Two lines carried trailing whitespace, inherited from the generated template, which yamllint rejects. The gate had never reached yamllint because zizmor failed first. Also pins actions/checkout by commit SHA where it was still a floating v6 tag. Verified locally with the same three tools the gate runs, over the whole .github tree: zizmor --persona pedantic, actionlint and yamllint all exit 0.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (1fe93cb): one commit, d60ce44, that brings the reviewer workflow into compliance with the repo's workflow/YAML lint gates.
- Concurrency group added — a stale run is now cancelled when a fresh one is dispatched for the same PR, via
group: ${{ github.workflow }}-${{ inputs.name || github.run_id }}withcancel-in-progress: true. Because every run is dispatched against the default branch, keying ongithub.ref(the sibling workflows' pattern) would collide across PRs, so it keys on the dispatched run name instead, falling back to the unique run id when no name is passed. - Job named —
name: Pullfrog agent, so the check is no longer anonymous in the checks list. - Write permission documented —
id-token: writecarries its own trailing comment (zizmor pedantic reads the permission line, not the block above it), alongside acontents: readrationale. - Inherited template whitespace removed — two blank lines in the
env:block that yamllint rejected are now clean.
The inputs context in concurrency is valid for workflow_dispatch per GitHub's context-availability table, and the commit is whitespace-clean.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
deepseek-v4.1-flash (model usage covered by Pullfrog) ｜ 𝕏
1 participant
The Pullfrog console generated this workflow when I enabled the repository. It works, but it leaves three things at defaults that are wrong for this project, so this tightens them before the reviewer starts running.
push: disabled. This is the one that matters. The input defaults toenabled, which lets the agent push branches and open pull requests of its own. Every commit and pull request in my repositories is authored by me, so the agent gets no write path at all: it reads the diff and comments. The input is declared in the action's ownaction.ymland is not mentioned in the getting-started guide, which is why the generated file leaves it permissive.Pinned to a commit SHA instead of
@v0. That tag moves — it has already advanced through ninetyv0.1.xreleases — and this is the only action here that runs an agent with access to the repository, so a mutable ref is the last place I want one. Pullfrog's own versioning page supports pinning and says nothing is lost: the action fetches its steps from npm either way, so the agent keeps tracking patch releases without this file changing. Bump the SHA to update.shell: restricted, which scrubs*_TOKEN/*_KEY/*_SECRET/*_PASSWORD/*_CREDENTIALout of the environment the agent's shell sees. It is already the default for a public repository; I set it explicitly so that making this repository private some day cannot widen it silently, since private repos default toenabled.I left the provider
env:block exactly as generated. I checked what an unset secret does: it resolves to an empty string, and the action'ssanitizeSecret()leaves the variable untouched in that case rather than treating it as a key, so the block is inert until a key is actually added and it cannot shadow keys held in Pullfrog's own store or in the Router.Nothing else changes. There are no
run:steps, so there is no shell interpolation surface, and no workflow expression reads untrusted event data.One caveat worth recording: the file carries Pullfrog's own
DO NOT EDIT EXCEPT WHERE INDICATEDheader, so if the console regenerates it these three changes are lost. The durable place for the push tier is the console's own repository setting; this file is the belt to that braces.Summary by CodeRabbit