Skip to content

chore(ci): give new workspace comment workflow correct perms#2606

Open
hopehadfield wants to merge 1 commit intoredhat-developer:mainfrom
hopehadfield:update-workspace-workflow
Open

chore(ci): give new workspace comment workflow correct perms#2606
hopehadfield wants to merge 1 commit intoredhat-developer:mainfrom
hopehadfield:update-workspace-workflow

Conversation

@hopehadfield
Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

This should provide the correct permissions for the new workspace PR comment workflow. The test run failed because workflows triggered by pull_request receive a read-only GITHUB_TOKEN.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
@hopehadfield hopehadfield requested review from a team as code owners March 25, 2026 14:43
@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Update workspace PR comment workflow with elevated permissions

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Changed trigger from pull_request to pull_request_target for elevated permissions
• Added persist-credentials: false to checkout step for security
• Enables workflow to write comments on PRs from external contributors
Diagram
flowchart LR
  A["pull_request trigger"] -->|"Change to"| B["pull_request_target trigger"]
  B -->|"Enables"| C["Write permissions for PR comments"]
  D["Checkout step"] -->|"Add security flag"| E["persist-credentials: false"]
Loading

Grey Divider

File Changes

1. .github/workflows/new-workspace-pr-comment.yml ⚙️ Configuration changes +2/-1

Update workflow trigger and add security credentials flag

• Changed workflow trigger from pull_request to pull_request_target to enable write permissions
• Added persist-credentials: false to the checkout action for enhanced security
• Allows workflow to post comments on PRs while maintaining security best practices

.github/workflows/new-workspace-pr-comment.yml


Grey Divider

Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Mar 25, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Privileged checkout of PR 🐞 Bug ⛨ Security
Description
The workflow now runs on pull_request_target with write permissions, but still checks out the PR
head SHA, making attacker-controlled repository content available in a privileged job context. This
creates a critical token/permission abuse vector if any step (now or in the future) executes or
sources code from the checked-out workspace.
Code

.github/workflows/new-workspace-pr-comment.yml[4]

+  pull_request_target:
Evidence
In the updated workflow file, the trigger is pull_request_target and the workflow requests
issues: write and pull-requests: write, meaning the job runs with a write-capable token on
untrusted PR events. The job also checks out ref: ${{ github.event.pull_request.head.sha }}, which
is PR-controlled for fork PRs, placing untrusted content in the workspace. The repo contains a safer
pull_request_target pattern in validate-codeowners.yml: it checks out the base repo and then
selectively checks out only the needed file from the PR head, reducing exposure to untrusted
content.

.github/workflows/new-workspace-pr-comment.yml[1-89]
.github/workflows/validate-codeowners.yml[28-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pull_request_target` runs with base-repo context and a write-capable token. Checking out the PR head SHA in that job makes attacker-controlled code/content available, which is a known GitHub Actions security anti-pattern.

### Issue Context
The workflow only needs to detect whether `workspaces/` gained new top-level directories and then post a comment.

### Fix Focus Areas
- .github/workflows/new-workspace-pr-comment.yml[1-40]

### Suggested fix
- Do **not** check out `github.event.pull_request.head.sha` in a `pull_request_target` job.
- Prefer one of:
 1) Checkout the base (`ref: ${{ github.event.pull_request.base.sha }}`) and then `git fetch` the PR head into a separate ref and use `git ls-tree <sha>:workspaces` without checking out the PR content into the working tree; or
 2) Avoid checkout entirely and use GitHub API to compare tree contents (read-only operations) while keeping write permissions only for the comment step.
- If you must access PR content, only check out the minimal paths needed (similar to `validate-codeowners.yml`’s `git checkout pr-head -- <path>` pattern).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unneeded pull-request write scope 🐞 Bug ⛨ Security
Description
The workflow grants pull-requests: write even though it only lists and creates issue comments,
unnecessarily expanding the impact of any token misuse under pull_request_target. This permission
should be removed to maintain least-privilege on an untrusted trigger.
Code

↗ .github/workflows/new-workspace-pr-comment.yml

        with:
Evidence
In new-workspace-pr-comment.yml, permissions include issues: write and pull-requests: write.
The only API calls in the workflow are github.rest.issues.listComments and
github.rest.issues.createComment via actions/github-script, which are covered by `issues:
write`; no pull request write operations are performed.

.github/workflows/new-workspace-pr-comment.yml[8-11]
.github/workflows/new-workspace-pr-comment.yml[62-89]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow requests `pull-requests: write` but does not perform any pull-request write operations.

### Issue Context
The workflow posts a PR comment using the Issues API.

### Fix Focus Areas
- .github/workflows/new-workspace-pr-comment.yml[8-12]

### Suggested fix
- Remove `pull-requests: write` from the workflow `permissions` block.
- Keep `issues: write` (required for creating comments) and `contents: read` if still needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Credential removal may break fetch 🐞 Bug ⛯ Reliability
Description
Adding persist-credentials: false prevents actions/checkout from leaving auth in git config, so
the subsequent git fetch runs unauthenticated and can fail depending on repo visibility/policies.
This can make the workflow brittle if it ever runs in a context where unauthenticated fetch is
disallowed.
Code

.github/workflows/new-workspace-pr-comment.yml[R33-36]

+          persist-credentials: false

      - name: Fetch base commit
        run: git fetch --no-tags origin ${{ github.event.pull_request.base.sha }}
Evidence
The workflow now sets persist-credentials: false and then runs `git fetch --no-tags origin ${{
github.event.pull_request.base.sha }}`. With credentials not persisted, that fetch uses whatever
unauthenticated access is available; if unauthenticated fetch is restricted, this step can fail.
This is a direct consequence of the new configuration plus the later fetch step.

.github/workflows/new-workspace-pr-comment.yml[28-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`persist-credentials: false` removes git auth, and the workflow later runs `git fetch`, which may require authentication in some environments.

### Issue Context
This may be an intentional hardening change; if so, ensure the workflow still fetches required commits reliably.

### Fix Focus Areas
- .github/workflows/new-workspace-pr-comment.yml[28-37]

### Suggested fix
- If you need `persist-credentials: false`, then fetch commits without relying on git remote auth (e.g., use `actions/checkout` to fetch both SHAs via `fetch-depth: 0` + proper ref strategy, or use GitHub API), or explicitly provide a token only where required.
- Otherwise, remove `persist-credentials: false` if it causes fetch failures in your environment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud


on:
pull_request:
pull_request_target:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Privileged checkout of pr 🐞 Bug ⛨ Security

The workflow now runs on pull_request_target with write permissions, but still checks out the PR
head SHA, making attacker-controlled repository content available in a privileged job context. This
creates a critical token/permission abuse vector if any step (now or in the future) executes or
sources code from the checked-out workspace.
Agent Prompt
### Issue description
`pull_request_target` runs with base-repo context and a write-capable token. Checking out the PR head SHA in that job makes attacker-controlled code/content available, which is a known GitHub Actions security anti-pattern.

### Issue Context
The workflow only needs to detect whether `workspaces/` gained new top-level directories and then post a comment.

### Fix Focus Areas
- .github/workflows/new-workspace-pr-comment.yml[1-40]

### Suggested fix
- Do **not** check out `github.event.pull_request.head.sha` in a `pull_request_target` job.
- Prefer one of:
  1) Checkout the base (`ref: ${{ github.event.pull_request.base.sha }}`) and then `git fetch` the PR head into a separate ref and use `git ls-tree <sha>:workspaces` without checking out the PR content into the working tree; or
  2) Avoid checkout entirely and use GitHub API to compare tree contents (read-only operations) while keeping write permissions only for the comment step.
- If you must access PR content, only check out the minimal paths needed (similar to `validate-codeowners.yml`’s `git checkout pr-head -- <path>` pattern).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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