Repository navigation
feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml - #485
Conversation
…iage-runner.yml
|
Warning Review limit reached
More reviews will be available in 38 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR addresses a compliance finding by adding a top-level ChangesWorkflow Permissions Compliance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test script, test_workflow_toplevel_permissions.py, to verify that the issue-triage-runner.yml workflow includes a top-level permissions: declaration in accordance with the organization's least-privilege policy. The reviewer pointed out a minor issue where double curly braces {{}} are used in a non-f-string literal, which will cause them to be printed literally instead of as {}.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50a28214ab
ℹ️ 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 "@codex address that feedback".
Dev-Lead — rate-limited (intent: review-changes)PR: #485 |
|
Note @don-petry I received your request but all AI engines are currently rate-limited. I'll retry automatically once the rate limit clears. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00b7892dc6
ℹ️ 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 "@codex address that feedback".
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #485 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test-dev-lead.yml:
- Line 103: The checkout step currently using "uses:
actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10" should explicitly
disable credential persistence; modify that step to include the key
"persist-credentials: false" so the action does not keep the workflow token in
the workspace and aligns with least-privilege hardening.
- Around line 98-107: The validation job "toplevel-permissions" isn't triggered
for changes to .github/workflows/issue-triage-runner.yml because the workflow's
on.push.paths and on.pull_request.paths exclude that file; update the workflow
trigger filters so the guard runs when the validated workflow changes — either
add ".github/workflows/issue-triage-runner.yml" to the on.push.paths and
on.pull_request.paths lists or remove/relax the path filters entirely so the
"toplevel-permissions" validation step executes for commits/PRs that modify that
workflow file.
In `@tests/dev-lead/integration/test_workflow_toplevel_permissions.py`:
- Around line 36-43: The current check only ensures the top-level "permissions"
key exists but doesn't validate its value; change the guard so that in addition
to checking "permissions" in doc it verifies the value is the least-privilege
form (e.g., an empty mapping or explicit none/read-only entries as your policy
requires) instead of permissive forms like "write-all". Update the conditional
around doc and WORKFLOW to reject non-empty or broad permission values and log
the same failure message when doc["permissions"] is not the approved minimal
structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0751a38-2892-40a3-b7d7-75b1fb1b6a5f
📒 Files selected for processing (3)
.github/workflows/issue-triage-runner.yml.github/workflows/test-dev-lead.ymltests/dev-lead/integration/test_workflow_toplevel_permissions.py
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #485 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 288b86383333f758945fdaa7aea9df93e5411e04
Review mode: triage-approved (single reviewer)
Summary
This PR brings issue-triage-runner.yml into compliance with the org least-privilege policy by adding a top-level permissions: {} reset, and adds a regression-guard integration test (test_workflow_toplevel_permissions.py) wired into the test-dev-lead.yml CI job. The change is small (3 files, +74/-0), purely a security/compliance hardening, and does not alter runtime behavior of the triage workflow — each job already declares its own scoped permissions: block.
Linked issue analysis
Closes #393 (Compliance: missing-permissions-issue-triage-runner.yml). The remediation requested by the weekly compliance audit — adding a top-level permissions: declaration so GitHub's default write-all grant is not silently inherited — is implemented exactly as the standard prescribes (permissions: {} before jobs:). The added test additionally enforces the exact empty-mapping value, so future regressions to write-all/read-all would also fail CI.
Findings
All prior review feedback on this PR has been addressed on the head commit:
- gemini-code-assist (
{{}}vs{}in a non-f-string error message): fixed. - chatgpt-codex / CodeRabbit (validation job not triggered by changes to the workflow it validates):
.github/workflows/issue-triage-runner.ymlis now included in bothon.pull_request.pathsandon.push.pathsfilters oftest-dev-lead.yml. - CodeRabbit (
persist-credentials: falsemissing on new checkout): added to thetoplevel-permissionsjob checkout step. - CodeRabbit (key-presence check accepts
write-all/other broad values): now also assertsdoc.get("permissions") == {}, rejecting any non-empty mapping.
No new issues identified. No secrets, credentials, migrations, or risky surface touched. New Python test is small, self-contained, and uses yaml.safe_load.
CI status
All required checks green: Lint, ShellCheck, bats, unit-tests, validate-agent-profiles, Compile agentic workflows, gh-aw-compile, Agent Security Scan, Secret scan (gitleaks), CodeQL (actions + python), SonarCloud, AgentShield, dependency-audit, CodeRabbit. Dependabot and unrelated audits correctly SKIPPED. mergeStateStatus: BLOCKED reflects only the outstanding human review-team request, not a failing check.
Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.
…iage-runner.yml (#485) * feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
…iage-runner.yml (#485) * feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
…iage-runner.yml (#485) * feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
…iage-runner.yml (#485) * feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
…iage-runner.yml (#485) * feat: implement issue #393 — Compliance: missing-permissions-issue-triage-runner.yml * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Closes #393
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
Tests
Chores