Skip to content

ci: harden CI workflow permissions/concurrency + add Dependabot#1036

Open
SalemOurabi wants to merge 2 commits into
4gray:masterfrom
SalemOurabi:hardening/ci-pipeline
Open

ci: harden CI workflow permissions/concurrency + add Dependabot#1036
SalemOurabi wants to merge 2 commits into
4gray:masterfrom
SalemOurabi:hardening/ci-pipeline

Conversation

@SalemOurabi

Copy link
Copy Markdown
Contributor

CI hardening, rebased onto the latest master:

  • ci.yml: least-privilege permissions: contents: read (was the broad default token) + a concurrency group that cancels superseded runs on the same ref.
  • New .github/dependabot.yml: weekly github-actions (also enables SHA-pinning), npm (dev/tooling grouped), and docker updates.

No job logic changed.

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens CI supply-chain security by adding least-privilege permissions blocks to three workflow files and a concurrency group that cancels superseded runs, plus a new Dependabot config for weekly updates across GitHub Actions, npm, and Docker.

  • Permissions: All three workflows now explicitly declare contents: read (and security-events: write for CodeQL), which is correct and an improvement over the broad implicit default token.
  • Dependabot: The new config is well-structured — dev npm deps are grouped to reduce PR noise, runtime deps are raised individually, and docker updates target the /docker directory.
  • Concurrency: The cancel-in-progress: true is applied unconditionally across all refs. For codeql-analysis.yml this is particularly impactful because the schedule trigger shares the same concurrency group key as push runs (both resolve to refs/heads/master), meaning a weekly scheduled security scan can be silently cancelled by a master push — or vice versa — with no failure status recorded.

Confidence Score: 4/5

Safe to merge after addressing the CodeQL concurrency group — the scheduled weekly security scan can be silently cancelled by a master push.

The CodeQL workflow now has a schedule trigger sharing a concurrency group with push runs. Because both resolve github.ref to refs/heads/master, a weekly scan in flight will be cancelled the moment a push to master fires — or vice versa — with no failed status, just a missing result. The permissions additions and Dependabot config are correct.

.github/workflows/codeql-analysis.yml — the concurrency group needs the event name included to prevent the scheduled scan from being preempted by push triggers.

Important Files Changed

Filename Overview
.github/dependabot.yml New Dependabot config enabling weekly updates for github-actions, npm (dev deps grouped), and docker; configuration is well-structured and correct.
.github/workflows/ci.yml Adds permissions: contents: read (correct) and unconditional cancel-in-progress: true concurrency (master-branch cancellation already flagged in a prior review thread).
.github/workflows/codeql-analysis.yml Adds correct least-privilege permissions (actions: read, contents: read, security-events: write), but the unconditional concurrency group collapses the weekly schedule trigger with push/PR runs, allowing one to silently cancel the other.
.github/workflows/e2e-tests.yaml Adds permissions: contents: read (correct) and cancel-in-progress: true concurrency; same unconditional master-branch cancellation pattern as ci.yml.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[push to master] -->|github.ref = refs/heads/master| G1
    B[weekly schedule] -->|github.ref = refs/heads/master| G1
    C[pull_request] -->|github.ref = refs/pull/N/merge| G2

    G1["concurrency group: codeql-refs/heads/master\n⚠️ SHARED KEY"]
    G2["concurrency group: codeql-refs/pull/N/merge"]

    G1 -->|cancel-in-progress: true| X["❌ One of push or schedule\nsilently cancelled"]
    G2 -->|cancel-in-progress: true| Y["✅ Old PR run cancelled\n(desired)"]

    style G1 fill:#ffcccc,stroke:#cc0000
    style X fill:#ffcccc,stroke:#cc0000
    style G2 fill:#ccffcc,stroke:#009900
    style Y fill:#ccffcc,stroke:#009900
Loading

Reviews (2): Last reviewed commit: "ci: add least-privilege permissions and ..." | Re-trigger Greptile

Comment thread .github/workflows/ci.yml
Comment on lines +17 to +19
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The current cancel-in-progress: true is unconditional, so rapid pushes to master (e.g. two PRs merged back-to-back) will cancel each other's CI runs. That means a commit that breaks master could silently slip past — the CI for the first merge gets killed before it finishes, and neither run produces a green or red result for that SHA. Cancelling superseded runs is valuable for PR branches but should be disabled for the default branch.

Suggested change
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: true
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: ${{ github.ref != 'refs/heads/master' }}
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 17-19

Comment:
The current `cancel-in-progress: true` is unconditional, so rapid pushes to `master` (e.g. two PRs merged back-to-back) will cancel each other's CI runs. That means a commit that breaks `master` could silently slip past — the CI for the first merge gets killed before it finishes, and neither run produces a green or red result for that SHA. Cancelling superseded runs is valuable for PR branches but should be disabled for the default branch.

```suggestion
concurrency:
    group: ci-${{ github.ref }}
    cancel-in-progress: ${{ github.ref != 'refs/heads/master' }}
```

How can I resolve this? If you propose a fix, please make it concise.

ci.yml now declares permissions (contents: read) instead of the broad default
token, and a concurrency group that cancels superseded runs on the same ref.
Adds .github/dependabot.yml for weekly github-actions, npm (dev-grouped), and
docker updates — which also lets Dependabot pin actions to commit SHAs.
e2e-tests.yaml: permissions contents:read + concurrency cancelling superseded
multi-OS runs. codeql-analysis.yml: permissions actions:read / contents:read /
security-events:write (SARIF upload) + concurrency. Release/deploy workflows are
intentionally left out — cancel-in-progress is unsafe mid-release/deploy.
@SalemOurabi SalemOurabi force-pushed the hardening/ci-pipeline branch from 3331fcc to b0863b3 Compare June 11, 2026 09:46

Copy link
Copy Markdown
Contributor Author

@greptileai

Please review the current PR head b0863b33. The previous Greptile review covered 692484cc, so please re-evaluate the full current diff, including the concurrency behavior on master, immutable action pinning, and the Dependabot configuration.

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