Skip to content

Fix org role permission string, remove the now-unneeded diagnostic - #178

Merged
eliorerz merged 1 commit into
mainfrom
fix-runner-permission-string
Aug 7, 2026
Merged

Fix org role permission string, remove the now-unneeded diagnostic#178
eliorerz merged 1 commit into
mainfrom
fix-runner-permission-string

Conversation

@eliorerz

@eliorerz eliorerz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What

Fixes the root cause of #174/#176: manage_organization_runners (from #173) was rejected by the live API with 422 Invalid permission. A one-time diagnostic dispatch of #175/#177 confirmed the real identifier directly from GET /orgs/osac-project/organization-fine-grained-permissions (run 31149779707):

write_organization_runners_and_runner_groups: Manage organization runners and runner groups

Updates organization.tf's permissions list accordingly.

Also removes the diagnostic step and its list_org_fine_grained_permissions input from apply.yaml entirely -- it did its one job. Unlike exclude_addresses/state_rm_addresses/import_address, which are reusable escape hatches worth keeping, this had no future utility once the string was known. apply.yaml is now byte-identical to its pre-#175 state (verified via git diff against that commit).

Validation

  • tofu init -backend=false && tofu validate: passes (pre-existing unrelated deprecation warnings only)
  • tofu fmt: clean
  • yamllint --strict + pre-commit: clean

Once merged

I'll dispatch a normal apply (or wait for the next scheduled run) to confirm the role and team binding actually get created live, then close #174/#176.

NO-ISSUE

Summary by CodeRabbit

  • New Features

    • Updated organization runner management permissions to support writing organization runners and runner groups.
  • Changes

    • Simplified the manual apply workflow by removing the fine-grained permissions lookup option and related conditional behavior.

The live organization-fine-grained-permissions API rejected
manage_organization_runners (422); a one-time diagnostic dispatch
(github-config#175, #177) confirmed the real identifier is
write_organization_runners_and_runner_groups ("Manage organization
runners and runner groups"). Fix the role's permissions list, and
remove the diagnostic step and its input entirely -- it was genuinely
one-time (unlike exclude_addresses/state_rm_addresses/import_address,
which are reusable escape hatches), so apply.yaml is restored to its
pre-#175 shape.

NO-ISSUE
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0a7f8c55-7b17-47cd-b51a-cb05367b12a4

📥 Commits

Reviewing files that changed from the base of the PR and between b7a784a and 16e61fa.

📒 Files selected for processing (2)
  • .github/workflows/apply.yaml
  • organization.tf

Walkthrough

The workflow removes the fine-grained permission listing input and related API lookup. Import, state removal, and apply steps use their normal execution conditions. The runner manager role now uses write_organization_runners_and_runner_groups.

Changes

Apply workflow and runner permissions

Layer / File(s) Summary
Workflow execution conditions
.github/workflows/apply.yaml
Removes the permission-listing mode and its API lookup. Import and state removal depend only on their respective inputs. Apply runs normally.
Runner manager permission
organization.tf
Replaces manage_organization_runners with write_organization_runners_and_runner_groups.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes both primary changes: fixing the organization role permission string and removing the obsolete diagnostic.
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.
No-Hardcoded-Secrets ✅ Passed The patch only adds workflow conditions and a permission name; it adds no hardcoded secrets, credential URLs, private-key material, or encoded secret blobs.
No-Weak-Crypto ✅ Passed The PR only changes workflow conditions and an organization permission identifier; added lines contain no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons.
No-Injection-Vectors ✅ Passed The PR adds only workflow conditions and a static Terraform permission; it adds no SQL concatenation, shell/eval/exec, unsafe deserialization, os.system, or HTML injection.
Container-Privileges ✅ Passed The PR changes only a GitHub Actions workflow and Terraform. No container/Kubernetes manifests or prohibited privilege settings appear in the tracked files or diff.
No-Sensitive-Data-In-Logs ✅ Passed The change only removes the diagnostic API-to-step-summary logger and updates a permission string; no added code logs secrets, tokens, credentials, PII, or customer data.
Ai-Attribution ✅ Passed The PR description and sole PR commit do not mention AI tools; the current commit has no AI co-author or attribution trailer, so the attribution requirement is not triggered.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-runner-permission-string

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

@eliorerz
eliorerz merged commit 7078239 into main Aug 7, 2026
3 checks passed
@eliorerz
eliorerz deleted the fix-runner-permission-string branch August 7, 2026 05:15
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.

Apply configuration is failing

1 participant