Tighten workflow permissions and add concurrency guards#9
Conversation
WalkthroughUpdated GitHub Actions workflows to set explicit top-level permissions and concurrency policies, adjusted triggers for CI to push-on-main, added deploy-job permissions in docs, and moved per-job concurrency in perf-light to a top-level concurrency block. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub (Workflow Dispatcher)
participant WF as Workflow Run
participant Q as Concurrency Group
participant Job as Jobs (build/test/deploy)
Dev->>GH: Push / PR event
GH->>Q: Check group = ${workflow}-${ref}
alt Another run in-progress
Q-->>WF: Cancel in-progress (if same group)
end
GH->>WF: Start workflow with scoped permissions
par CI/Docs/Perf-light
WF->>Job: Run with top-level permissions
note over Job: Docs deploy elevates to contents: write
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| name: perf-light | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: |
There was a problem hiding this comment.
Restore actions scope for perf-light artifacts
The new workflow-level permissions restrict the token to contents: read only, which implicitly sets the actions scope to none. The k6 job still calls actions/upload-artifact@v4 to publish the perf summary, and that action requires actions: write on the GITHUB_TOKEN. In its current state the workflow will fail on the upload step with "Resource not accessible by integration" whenever the perf-light job runs. Consider adding back actions: write (or removing the artifact upload) so the job can complete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/perf-light.yml (1)
40-86: Indent heredoc terminator to match run-block indentation (.github/workflows/perf-light.yml:84)
The closingPYmust be indented exactly as thepython - <<'PY'line underrun: |so the YAML block parses correctly..github/workflows/docs.yml (1)
10-45: Use recursive globs (**/*.ext) or a brace pattern for paths
The current'**.py','**.ts', etc. won’t match files in subdirectories. Under bothpush.pathsandpull_request.paths, replace those entries with either individual**/*.extglobs or collapse into:- '**/*.{py,ts,tsx,js,jsx,html,md,png,svg}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks).github/workflows/docs.yml(2 hunks).github/workflows/perf-light.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
.github/workflows/perf-light.yml
[error] 45-46: check-yaml: Could not parse simple key: missing ':' in perf-light.yml at lines 45-46.
[error] 1-1: check-yaml: YAML syntax error detected in workflow configuration.
.github/workflows/ci.yml
[error] 1-1: Process completed with exit code 1. The CI pipeline failed due to multiple lint, type, and YAML configuration errors.
🔇 Additional comments (4)
.github/workflows/ci.yml (1)
8-10: Concurrency group looks goodWorkflow+ref grouping with cancel-in-progress is a solid default for CI.
If you intend different behavior for PRs vs. pushes, consider
${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }}to avoid cross-cancellation between PR and push runs on the same ref..github/workflows/docs.yml (2)
4-8: Good: minimal top-level permissions and concurrency guardContents read at top-level and deploy-only write scoped to the job is correct. Concurrency setting is appropriate.
98-99: Deploy job permission scope is correctGranting
contents: writeonly for the deploy job aligns with least-privilege best practices..github/workflows/perf-light.yml (1)
3-9: Good: minimal permissions and workflow-level concurrencyContents read and a workflow+ref concurrency group with cancel-in-progress is appropriate.
| permissions: | ||
| contents: read | ||
| actions: read | ||
| checks: write |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Trim workflow-level permissions to least privilege
actions: read and checks: write are rarely needed at the workflow level. Prefer only contents: read and elevate per-job if a step truly requires more.
permissions:
contents: read
- actions: read
- checks: write📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| actions: read | |
| checks: write | |
| permissions: | |
| contents: read |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 3 to 6, the workflow-level permissions
are too broad (actions: read and checks: write); remove those two entries and
keep only contents: read at the workflow level, then add per-job permissions
blocks for any job that actually requires actions: read or checks: write so
privileges are least-privilege and scoped to only the jobs/steps that need them.
| group: perf-${{ github.ref }} | ||
| cancel-in-progress: true | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use latest checkout action for consistency
CI/docs use actions/checkout@v5. Align here too.
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v5 |
🤖 Prompt for AI Agents
In .github/workflows/perf-light.yml around line 18 the workflow pins
actions/checkout to v4; update the checkout action reference to
actions/checkout@v5 to match other CI/docs for consistency and ensure any v5
fixes/features are used by replacing the version string.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68eefbf06e188330b99091c6303b0e42
Summary by CodeRabbit