Skip to content

feat(codeowners): add hardening rules - #58

Merged
gmcculloug merged 1 commit into
RedHatInsights:masterfrom
karelhala:hardening
Jun 4, 2026
Merged

feat(codeowners): add hardening rules#58
gmcculloug merged 1 commit into
RedHatInsights:masterfrom
karelhala:hardening

Conversation

@karelhala

Copy link
Copy Markdown
Contributor

What

  • Adds a CODEOWNERS file to protect GitHub configuration files from unauthorized modifications
  • Requires code owner review for all changes to .github/** (workflows, custom actions, PR templates, etc.)
  • Includes placeholder for future .tekton/** protection

Why

Changes to GitHub Actions workflows and custom actions can:

  • Execute arbitrary code in CI/CD pipelines
  • Access repository secrets
  • Modify release processes
  • Affect all contributors
  • Requiring code owner approval adds a critical security layer to prevent unauthorized or malicious modifications.

Manual Steps Required After Merge

To fully enable this protection, repository admins need to:

  1. Enable Code Owner Review Enforcement:
  2. Enable Fork Workflow Approval:

Test Plan

  • CODEOWNERS file syntax is valid
  • After merge, verify PR touching .github/ requires code owner review

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Chores
    • Tightened CI workflow permissions: narrowed top-level token scope and granted elevated write permissions only for the release job; release now runs only on pushes to the master branch.
    • Added repository ownership rules establishing default owners and explicit admin-only ownership for repository configuration paths.

Walkthrough

Top-level workflow permissions were tightened; the release job now runs only on pushes to refs/heads/master, uses job-scoped permissions for publish steps, the checkout step no longer uses an SSH key, and release auth/identity/env were switched to built-in github-actions[bot] and GITHUB_TOKEN. A CODEOWNERS file was added to assign default owners and require admin ownership for .github/** and the CODEOWNERS file.

Changes

CI and repository governance

Layer / File(s) Summary
Release job conditional guard and permissions
.github/workflows/release.yml
Workflow-level permissions.contents changed from write to read; removed top-level id-token; release job now has if: github.event_name == 'push' && github.ref == 'refs/heads/master' and a job-level permissions block granting contents: write and id-token: write. Checkout step no longer supplies ssh-key; release step uses github-actions[bot] identity and secrets.GITHUB_TOKEN for npx nx release environment.
Code ownership rules
CODEOWNERS
Adds a CODEOWNERS file assigning default repo ownership to @RedHatInsights/experience-services-committers, marks .github/** and CODEOWNERS itself as admin-only, and includes a commented .tekton/** admin rule and precedence guidance.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(codeowners): add hardening rules' accurately reflects the main change—adding a CODEOWNERS file with security protections for critical GitHub configuration files.
Description check ✅ Passed The PR description covers the main requirements but deviates from the template structure, lacking explicit sections for 'Blast radius', 'Rollback plan', and the formatted 'Checklist' section specified in the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/release.yml (1)

3-18: ⚡ Quick win

Clarify intent: workflow_dispatch trigger remains but release job is gated.

The workflow accepts workflow_dispatch as a trigger but the release job will never execute when manually dispatched due to the if guard. This could be intentional (e.g., to allow testing the setup steps without releasing), or the workflow_dispatch trigger may no longer be needed.

♻️ Consider removing workflow_dispatch if not needed

If manual dispatch is not required, remove it from triggers:

 on:
   push:
     branches:
       - master
-  workflow_dispatch:

Otherwise, add a comment explaining why workflow_dispatch is kept despite the release job being skipped.

🤖 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/release.yml around lines 3 - 18, The workflow currently
declares the workflow_dispatch trigger but the release job is gated by the
condition if: github.event_name == 'push' && github.ref == 'refs/heads/master'
so manual dispatch will never run the release job; either remove
workflow_dispatch from the top-level on: block if manual runs are not needed, or
keep workflow_dispatch and add a clear comment near the triggers explaining that
manual dispatch is intentional and that the release job is purposely restricted
by the if guard (reference the workflow_dispatch trigger and the if condition on
the release job).
🤖 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 `@CODEOWNERS`:
- Around line 1-13: The CODEOWNERS file references non-existent GitHub team
slugs `@RedHatInsights/experience-services-committers` and
`@RedHatInsights/experience-services-admins` which return 404 and therefore owners
aren't applied; update the CODEOWNERS entries (the wildcard rule "*" and the
specific rules for ".github/**", ".tekton/**", and "CODEOWNERS") to use the
actual team slugs that exist in the RedHatInsights org (or create those teams),
e.g., replace the two invalid slugs with the correct team names used by the org
so that gh api orgs/RedHatInsights/teams/<slug> returns 200 and ownership rules
take effect.

---

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 3-18: The workflow currently declares the workflow_dispatch
trigger but the release job is gated by the condition if: github.event_name ==
'push' && github.ref == 'refs/heads/master' so manual dispatch will never run
the release job; either remove workflow_dispatch from the top-level on: block if
manual runs are not needed, or keep workflow_dispatch and add a clear comment
near the triggers explaining that manual dispatch is intentional and that the
release job is purposely restricted by the if guard (reference the
workflow_dispatch trigger and the if condition on the release job).
🪄 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: CHILL

Plan: Enterprise

Run ID: 34612f3e-4edf-439f-8ac8-e5ff557c4bce

📥 Commits

Reviewing files that changed from the base of the PR and between 9fdd12a and 8a7e2296a81f7e12fd15c0c7d05348cef46949d3.

📒 Files selected for processing (2)
  • .github/workflows/release.yml
  • CODEOWNERS

Comment thread CODEOWNERS
Comment on lines +1 to +13
# Default owners for everything in the repo
* @RedHatInsights/experience-services-committers

# All GitHub configuration requires review from admins
# This prevents unauthorized modifications to CI/CD pipelines, custom actions, and repo processes
.github/** @RedHatInsights/experience-services-admins

# All Konflux tekton configuration changes review from admins
#.tekton/** @RedHatInsights/experience-services-admins

# CODEOWNERS file itself requires admin review to prevent bypass attacks
# Must come after wildcard so this rule takes precedence (last match wins)
CODEOWNERS @RedHatInsights/experience-services-admins

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that the GitHub teams referenced in CODEOWNERS exist in the organization.

# Check if teams exist
gh api orgs/RedHatInsights/teams/experience-services-committers --silent && echo "✓ experience-services-committers exists" || echo "✗ experience-services-committers NOT FOUND"
gh api orgs/RedHatInsights/teams/experience-services-admins --silent && echo "✓ experience-services-admins exists" || echo "✗ experience-services-admins NOT FOUND"

Repository: RedHatInsights/platform-frontend-ai-toolkit

Length of output: 237


Fix CODEOWNERS team slugs: referenced teams don’t exist in RedHatInsights (404)
In CODEOWNERS (lines 1-13), @RedHatInsights/experience-services-committers and @RedHatInsights/experience-services-admins both return HTTP 404 Not Found for gh api orgs/RedHatInsights/teams/<slug>, so these CODEOWNERS owners won’t be applied. Update the team names/slugs to the existing org teams (or create them) so the rules take effect.

🤖 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 `@CODEOWNERS` around lines 1 - 13, The CODEOWNERS file references non-existent
GitHub team slugs `@RedHatInsights/experience-services-committers` and
`@RedHatInsights/experience-services-admins` which return 404 and therefore owners
aren't applied; update the CODEOWNERS entries (the wildcard rule "*" and the
specific rules for ".github/**", ".tekton/**", and "CODEOWNERS") to use the
actual team slugs that exist in the RedHatInsights org (or create those teams),
e.g., replace the two invalid slugs with the correct team names used by the org
so that gh api orgs/RedHatInsights/teams/<slug> returns 200 and ownership rules
take effect.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/release.yml:
- Line 10: Move the broad `id-token: write` permission out of the workflow-level
permissions and add it only under `jobs.release.permissions` to scope OIDC token
minting to the release job; update the workflow YAML so the top-level
`permissions` block no longer includes `id-token: write` and ensure
`jobs.release.permissions` contains `id-token: write` (and any other necessary
permissions) to follow least-privilege principle.
🪄 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: CHILL

Plan: Enterprise

Run ID: 314d030c-cf59-485a-90f7-4687414db085

📥 Commits

Reviewing files that changed from the base of the PR and between 8a7e2296a81f7e12fd15c0c7d05348cef46949d3 and df0b1a084185406d18b2d597d95de767d9ae8464.

📒 Files selected for processing (2)
  • .github/workflows/release.yml
  • CODEOWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
  • CODEOWNERS

Comment thread .github/workflows/release.yml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/release.yml:
- Around line 18-20: Remove the unnecessary job-level write permission by
deleting or replacing the permissions key "contents: write" in the workflow
permissions block; keep "id-token: write" (or set "contents: read" only if any
steps require repo read access) and ensure git pushes remain authenticated via
actions/checkout with ssh-key and npm publish uses the provided RELEASE_TOKEN so
no job-level contents write is required.
🪄 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: CHILL

Plan: Enterprise

Run ID: 408773fc-0c5c-4fb3-a672-6aa952813e39

📥 Commits

Reviewing files that changed from the base of the PR and between df0b1a084185406d18b2d597d95de767d9ae8464 and 44b2bcb950543afbee4e5e4858b8a489493a8f54.

📒 Files selected for processing (2)
  • .github/workflows/release.yml
  • CODEOWNERS
✅ Files skipped from review due to trivial changes (1)
  • CODEOWNERS

Comment thread .github/workflows/release.yml Outdated
Comment on lines +18 to +20
permissions:
contents: write # Needed for git push
id-token: write # Needed for npm publish

@coderabbitai coderabbitai Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/release.yml"

echo "== Job permissions =="
sed -n '18,20p' "$file"

echo
echo "== Write credentials actually used by this workflow =="
sed -n '22,27p;54,60p' "$file"

# Expected:
# - checkout/push auth comes from BOT_SSH_KEY
# - GitHub API auth for nx release comes from RELEASE_TOKEN
# If both are true, the default GITHUB_TOKEN should not need contents: write.

Repository: RedHatInsights/platform-frontend-ai-toolkit

Length of output: 687


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/release.yml"
echo "== Full workflow file =="
cat -n "$file"

echo
echo "== References to default GitHub token =="
rg -n --hidden --no-ignore -S "github\.token|GITHUB_TOKEN:\s*\$\{\{\s*github\.token|secrets\.GITHUB_TOKEN|token:" "$file" || true

echo
echo "== References to RELEASE_TOKEN / BOT_SSH_KEY =="
rg -n --hidden --no-ignore -S "RELEASE_TOKEN|BOT_SSH_KEY|nx release|npm publish|id-token" "$file" || true

Repository: RedHatInsights/platform-frontend-ai-toolkit

Length of output: 2457


Drop contents: write from job-level permissions

.github/workflows/release.yml grants job-level permissions.contents: write, but this workflow authenticates git operations via actions/checkout using ssh-key: ${{ secrets.BOT_SSH_KEY }} and runs nx release with env.GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }}—so the default GITHUB_TOKEN doesn’t need write access here.

Suggested change
     permissions:
-      contents: write  # Needed for git push
+      contents: read   # Default token only needs read access here
       id-token: write  # Needed for npm publish
🤖 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/release.yml around lines 18 - 20, Remove the unnecessary
job-level write permission by deleting or replacing the permissions key
"contents: write" in the workflow permissions block; keep "id-token: write" (or
set "contents: read" only if any steps require repo read access) and ensure git
pushes remain authenticated via actions/checkout with ssh-key and npm publish
uses the provided RELEASE_TOKEN so no job-level contents write is required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've migrated to OIDC and removed the SSH key, I DO need contents: write because:

  • nx release uses GITHUB_TOKEN to push version tags
  • nx release uses GITHUB_TOKEN to create GitHub releases
  • Without contents: write, these operations will fail with permission errors

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@karelhala, you're absolutely right — my comment was based on stale script output that still showed BOT_SSH_KEY and RELEASE_TOKEN from the old workflow. Now that you've migrated to OIDC and GITHUB_TOKEN, nx release uses that token directly to push version tags and create GitHub releases, both of which require contents: write. The job-level permission is correct and necessary. Apologies for the noise!

🐇 (˶˃ ᵕ ˂˶)


📝 Learnings were identified and not saved (knowledge base disabled). Enable

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/release.yml (1)

16-17: 💤 Low value

workflow_dispatch is effectively disabled by this condition.

The if: condition requires both push event and refs/heads/master, which means manual workflow_dispatch triggers will always be skipped. The comment indicates this is intentional, but it removes the ability to manually re-trigger a release if one fails partway through. If manual release triggers are never needed, this is fine; otherwise, consider allowing workflow_dispatch when the ref is master.

🤖 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/release.yml around lines 16 - 17, The current if:
condition "github.event_name == 'push' && github.ref == 'refs/heads/master'"
prevents manual workflow_dispatch runs; update the condition in
.github/workflows/release.yml to also allow workflow_dispatch when the ref is
master (e.g., permit github.event_name == 'workflow_dispatch' OR combine push
with ref check) so manual triggers targeting refs/heads/master are not skipped;
locate the line with the existing if: and adjust the boolean expression
accordingly.
🤖 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.

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 16-17: The current if: condition "github.event_name == 'push' &&
github.ref == 'refs/heads/master'" prevents manual workflow_dispatch runs;
update the condition in .github/workflows/release.yml to also allow
workflow_dispatch when the ref is master (e.g., permit github.event_name ==
'workflow_dispatch' OR combine push with ref check) so manual triggers targeting
refs/heads/master are not skipped; locate the line with the existing if: and
adjust the boolean expression accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: defc118d-0e39-4cd2-af07-9a0a1038cb1d

📥 Commits

Reviewing files that changed from the base of the PR and between 44b2bcb950543afbee4e5e4858b8a489493a8f54 and e9b2da1.

📒 Files selected for processing (2)
  • .github/workflows/release.yml
  • CODEOWNERS
🚧 Files skipped from review as they are similar to previous changes (1)
  • CODEOWNERS

@gmcculloug
gmcculloug merged commit 2baba51 into RedHatInsights:master Jun 4, 2026
2 checks passed
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.

3 participants