NO-ISSUE: Add tofu plan and validate workflows for pull requests - #132
NO-ISSUE: Add tofu plan and validate workflows for pull requests#132rccrdpccl wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
WalkthroughAdds OpenTofu validation and planning workflows for Terraform and CSV pull request changes, including detailed plan reporting through pull request comments. Existing workflows pin actions to commits, and three repositories remove obsolete required status checks. ChangesTerraform/OpenTofu CI and repository policy
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant OpenTofu
participant AWS
participant GitHubAPI
PullRequest->>GitHubActions: trigger plan workflow
GitHubActions->>OpenTofu: initialize, validate, and plan
OpenTofu->>AWS: access authenticated infrastructure data
AWS-->>OpenTofu: return infrastructure data
GitHubActions->>GitHubAPI: create or update plan comment
Possibly related PRs
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/plan.yaml:
- Line 29: Pin every third-party action to an immutable full commit SHA instead
of a mutable tag: update actions/checkout at .github/workflows/plan.yaml lines
29-29, actions/create-github-app-token at lines 35-35, opentofu/setup-opentofu
at lines 42-42, and actions/github-script at lines 67-67, preserving each
action’s existing version.
- Around line 3-4: Replace the credentialed pull_request_target workflow trigger
with a pull_request-based flow for untrusted PR execution, and ensure checkout
and OpenTofu planning use the repository’s trusted ref rather than the PR head.
Keep AWS credentials and the GitHub App token out of all jobs that execute
attacker-controlled code, including the referenced environment approval and
planning steps.
In @.github/workflows/validate.yaml:
- Around line 3-7: Add a workflow-level concurrency configuration to the
validation workflow, using a pull-request-scoped group and setting
cancel-in-progress to true so superseded runs are cancelled while separate pull
requests remain isolated.
- Around line 19-24: Replace the mutable version tags in the workflow steps
using actions/checkout and opentofu/setup-opentofu with full immutable commit
SHAs corresponding to the intended v7 and v2 releases, respectively. Preserve
the existing action configuration, including tofu_wrapper: false.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 830ec18b-26af-4a90-8fef-de61cf36174e
📒 Files selected for processing (2)
.github/workflows/plan.yaml.github/workflows/validate.yaml
rccrdpccl
changed the title
2cbb365 to
6d72c28
Compare
6d72c28 to
577851f
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/apply.yaml:
- Line 41: Replace the hardcoded client ID in the workflow’s client-id
configuration with the established environment-variable reference used by
workflows such as plan.yaml, preserving the existing authentication behavior and
variable naming convention.
In @.github/workflows/plan.yaml:
- Around line 84-93: Review the plan output handling in the workflow’s
plan-comment generation and ensure sensitive Terraform values cannot be
published verbatim in the PR comment. Audit the providers and resources used by
this repository’s Terraform configuration for correctly marked sensitive
attributes, and add or apply redaction before the `<pre>` content is constructed
for any values not protected by provider schemas.
- Around line 28-31: Update the “Checkout PR” actions/checkout step to disable
credential persistence by setting persist-credentials to false, ensuring the
checkout token is not retained before OpenTofu execution; leave the existing ref
unchanged.
- Around line 33-39: Update the “Generate a token” step using
actions/create-github-app-token so it scopes the installation token to the
specific repository required by this job via repositories, rather than relying
only on owner. Add the minimum required permission, including
permission-contents: read, and preserve the existing client-id and private-key
inputs.
- Around line 100-105: Update the comment lookup around listComments to use
github.paginate with the existing issues.listComments request, so all pages of
comments are retrieved before searching. Keep the existing marker-based
comments.find logic and ensure it operates on the complete paginated result.
In @.github/workflows/validate.yaml:
- Line 23: Set persist-credentials to false on each actions/checkout step in
.github/workflows/validate.yaml (23-23), .github/workflows/apply.yaml (36-36),
and .github/workflows/pre-commit.yaml (16-16); no other checkout behavior should
change.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9bedb393-5288-4282-bd35-0fff3c4e4ec3
📒 Files selected for processing (5)
.github/workflows/apply.yaml.github/workflows/plan.yaml.github/workflows/pre-commit.yaml.github/workflows/validate.yamlrepositories.tf
💤 Files with no reviewable changes (1)
- repositories.tf
577851f to
de9ff84
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/plan.yaml (1)
34-41: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winApp token still inherits all installation permissions.
repositories: github-confignarrows the token to this repo (fix applied from earlier review), but nopermission-*input is set, so the token still carries every permission the app installation has been granted for this repo (peractions/create-github-app-tokendocs: "By default, the token inherits all of the installation's permissions... We recommend to explicitly list the permissions that are required"). Since this step only needs the token fortofu plan, add e.g.permission-contents: readto cap the blast radius if the token is ever exposed.🔒 Proposed fix
- name: Generate a token id: generate-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 with: client-id: ${{ env.CONFIG_APP_CLIENT_ID }} private-key: ${{ secrets.CONFIG_APP_SECRET }} owner: ${{ github.repository_owner }} repositories: github-config + permission-contents: readAs per path instructions, "Least privilege: minimize GITHUB_TOKEN permissions." This is the still-open half of a previously flagged concern (the
repositoriesscoping was addressed, permission scoping wasn't).🤖 Prompt for 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. In @.github/workflows/plan.yaml around lines 34 - 41, Restrict the token created by the generate-token step to the permissions required by tofu plan by adding an explicit read-only contents permission input, such as permission-contents: read, alongside the existing repository scope. Do not broaden the token or change the client, owner, or repository settings.Sources: Path instructions, Linters/SAST tools
🤖 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/apply.yaml:
- Around line 40-46: Update the “Generate a token” step using
actions/create-github-app-token to set its repositories input to the current
repository, matching the least-privilege configuration in plan.yaml while
preserving the existing owner and credential inputs.
In @.github/workflows/plan.yaml:
- Around line 1-123: Add a Terraform-focused SAST/SCA validation step to the
plan job before TF init and any tofu plan execution, using an approved scanner
such as tfsec, checkov, or tflint. Configure it to analyze the checked-out
Terraform changes and fail the workflow on detected violations, ensuring
untrusted PR infrastructure is scanned before credentials are used.
---
Duplicate comments:
In @.github/workflows/plan.yaml:
- Around line 34-41: Restrict the token created by the generate-token step to
the permissions required by tofu plan by adding an explicit read-only contents
permission input, such as permission-contents: read, alongside the existing
repository scope. Do not broaden the token or change the client, owner, or
repository settings.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c833bd07-df59-44ca-a06c-6f5e6a8fbc4a
📒 Files selected for processing (5)
.github/workflows/apply.yaml.github/workflows/plan.yaml.github/workflows/pre-commit.yaml.github/workflows/validate.yamlrepositories.tf
💤 Files with no reviewable changes (1)
- repositories.tf
de9ff84 to
754e3a2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (4)
.github/workflows/apply.yaml (1)
40-46: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict GitHub App token scope to the current repository.
The
actions/create-github-app-tokenstep omits therepositoriesinput, causing the generated token to inherit access to all repositories the GitHub App is installed in. For least privilege, restrict the token to the current repository.🛡️ Proposed fix
- name: Generate a token id: generate-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 with: client-id: ${{ env.CONFIG_APP_CLIENT_ID }} private-key: ${{ secrets.CONFIG_APP_SECRET }} owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }}🤖 Prompt for 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. In @.github/workflows/apply.yaml around lines 40 - 46, Update the “Generate a token” step using actions/create-github-app-token to set its repositories input to the current repository, preserving the existing owner and credential configuration while limiting the token to this repository only..github/workflows/plan.yaml (3)
34-40: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winApp token still scoped to the whole owner installation.
repositoriesandpermission-*inputs are still missing;owneralone grants the generated token access to every repo the App installation can reach. This was flagged previously and not yet addressed in this diff.🔒 Proposed fix
- name: Generate a token id: generate-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 with: client-id: ${{ env.CONFIG_APP_CLIENT_ID }} private-key: ${{ secrets.CONFIG_APP_SECRET }} - owner: ${{ github.repository_owner }} + owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + permission-contents: read🤖 Prompt for 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. In @.github/workflows/plan.yaml around lines 34 - 40, Update the “Generate a token” step using the actions/create-github-app-token configuration to restrict the token to the intended repository and explicitly declare the required permission-* inputs, rather than relying on owner-only installation scope. Preserve the existing client-id, private-key, and owner settings while adding repository and least-privilege permission configuration.Sources: Path instructions, Linters/SAST tools
50-53: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winNo SAST/SCA scan before credentialed plan runs.
Still no
tfsec/checkov/tflint-style static analysis beforetofu planexecutes with AWS credentials and an App token against untrusted PR diffs. Previously flagged, still absent.As per path instructions, ".github/workflows/**/*" changes require "SAST/SCA steps in pipeline."
🤖 Prompt for 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. In @.github/workflows/plan.yaml around lines 50 - 53, Add a SAST/SCA static-analysis step to the workflow between the “TF Validate” and “TF Plan” steps, using an approved Terraform scanner such as tfsec, checkov, or tflint. Ensure the scan runs before the credentialed “TF Plan” step and analyzes the untrusted PR Terraform changes.Source: Path instructions
85-99: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winFull plan output still posted verbatim to a public-ish PR comment.
HTML-escaping guards rendering but doesn't redact attribute values — that depends entirely on every touched provider/resource marking sensitive fields correctly. Any unmarked attribute lands in plaintext in the PR comment. This was raised previously and is unchanged.
🤖 Prompt for 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. In @.github/workflows/plan.yaml around lines 85 - 99, Update the body construction around the `plan` interpolation so the PR comment no longer embeds raw OpenTofu plan output. Remove the `<pre>` plan section or replace it with a trusted redacted artifact/link, ensuring unmarked sensitive provider or resource attributes cannot appear in the public comment while preserving the summary and commit metadata.Source: Path instructions
🤖 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.
Duplicate comments:
In @.github/workflows/apply.yaml:
- Around line 40-46: Update the “Generate a token” step using
actions/create-github-app-token to set its repositories input to the current
repository, preserving the existing owner and credential configuration while
limiting the token to this repository only.
In @.github/workflows/plan.yaml:
- Around line 34-40: Update the “Generate a token” step using the
actions/create-github-app-token configuration to restrict the token to the
intended repository and explicitly declare the required permission-* inputs,
rather than relying on owner-only installation scope. Preserve the existing
client-id, private-key, and owner settings while adding repository and
least-privilege permission configuration.
- Around line 50-53: Add a SAST/SCA static-analysis step to the workflow between
the “TF Validate” and “TF Plan” steps, using an approved Terraform scanner such
as tfsec, checkov, or tflint. Ensure the scan runs before the credentialed “TF
Plan” step and analyzes the untrusted PR Terraform changes.
- Around line 85-99: Update the body construction around the `plan`
interpolation so the PR comment no longer embeds raw OpenTofu plan output.
Remove the `<pre>` plan section or replace it with a trusted redacted
artifact/link, ensuring unmarked sensitive provider or resource attributes
cannot appear in the public comment while preserving the summary and commit
metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 10513d7b-e79d-4d57-a5ba-1132c158e5c5
📒 Files selected for processing (5)
.github/workflows/apply.yaml.github/workflows/plan.yaml.github/workflows/pre-commit.yaml.github/workflows/validate.yamlrepositories.tf
💤 Files with no reviewable changes (1)
- repositories.tf
Split into two workflows: - validate.yaml: runs on pull_request (no secrets needed), does tofu init -backend=false + tofu validate for immediate feedback on syntax/config errors. - plan.yaml: runs on pull_request_target with a required "plan" environment gate, so a maintainer must approve before secrets (AWS creds, GitHub App token) are exposed to the PR's code. Posts the plan output as a PR comment. The environment gate mitigates the Pwn Request risk inherent in pull_request_target + checkout PR head: external data sources and provider binaries execute during plan, so untrusted code must not run without human review of the diff. Also: - Pin all third-party actions to immutable commit SHAs - Set persist-credentials: false on all checkout steps - Paginate listComments to handle PRs with 30+ comments - Extract CONFIG_APP_CLIENT_ID env var in apply.yaml - Remove stale Prow required status checks (ci/prow/temp, ci/prow/unit) from fulfillment-service, osac-operator, osac-aap Note: the GitHub App token is intentionally NOT scoped to a single repository — the Terraform GitHub provider manages all repos in the org, so both plan and apply need org-wide access. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Riccardo Piccoli <rpiccoli@redhat.com>
754e3a2 to
349824e
Compare
- Scope GitHub App tokens to current repository in both plan and apply workflows (least privilege) - Add permission-contents: read to plan workflow token - Add tflint SAST step before tofu plan Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Riccardo Piccoli <rpiccoli@redhat.com>
349824e to
3a4a59f
Compare
1 participant
Split into two workflows:
validate.yaml: runs on pull_request (no secrets needed), does tofu init -backend=false + tofu validate for immediate feedback on syntax/config errors. Here a potential attacker could technically download a new
tofubinary but with no access to the secrets, the risk should be minimalplan.yaml: runs on pull_request_target with a required "plan" environment gate, so a maintainer must approve before secrets (AWS creds, GitHub App token) are exposed to the PR's code. Posts the plan output as a PR comment.
The environment gate mitigates the Pwn Request risk inherent in pull_request_target + checkout PR head: external data sources and provider binaries execute during plan, so untrusted code must not run without human review of the diff.
Assisted-by: Claude Code noreply@anthropic.com
Summary by CodeRabbit