Skip to content

Resolve zizmor security warnings.#478

Merged
Stormheg merged 2 commits intomainfrom
zizmor-fixes
Apr 21, 2026
Merged

Resolve zizmor security warnings.#478
Stormheg merged 2 commits intomainfrom
zizmor-fixes

Conversation

@tim-schilling
Copy link
Copy Markdown
Member

permissions: {} added at the workflow level to six workflows. Without this, GitHub falls back to the default token permissions (typically read/write on contents), which is broader than needed. Job-level permissions already existed and are unaffected.

Explanatory comments added to all job-level permission entries. Zizmor's undocumented-permissions audit requires each permission to explain why it's needed, so a future reader knows if it's still necessary.

concurrency: blocks added to add_member.yml, member-verification.yml, and zizmor.yml. Without concurrency limits, if two issues are opened in quick succession, two runs of add_member.yml could execute simultaneously. Both would check out the same branch state, both would try to create a branch named add-user/ (or race to push to main), and both would try to create a pull request — likely causing one to fail with a git conflict or a "PR already exists" error.

For member-verification.yml, two PRs opened at the same time would just run independently and correctly — there's no shared state between them. The concurrency key is ${{ github.event.pull_request.number }}, so only runs for the same PR would be deduplicated. In practice the trigger is types: [opened] so there can only ever be one opened event per PR, making the concurrency block essentially a no-op for this workflow. It's still correct to have it as a safeguard.

For zizmor.yml, a push to main followed immediately by another push could queue two runs scanning the same or nearly-identical code. The concurrency block with cancel-in-progress: true cancels the older run in favour of the newer one, avoiding redundant work. No correctness risk, just efficiency.

name: added to the unnamed verify-new-member job in member-verification.yml.

environment: production added to jobs that reference TERRAFORM_MANAGEMENT_GITHUB_TOKEN. Zizmor's secrets-outside-env audit flags secrets used without a GitHub Environment, as environments enable access controls and audit trails around sensitive credentials. The environment was created with no protection rules so the apply workflows remain fully automated.

# zizmor: ignore[artipacked] added to the checkout step in add_member.yml. persist-credentials: true is intentional there — the job performs git push after checking out, so credentials must persist. The finding is suppressed rather than worked around.

This was generated with the help of Claude Code (I think 4.7?). It should resolve the security and quality warnings on the repo.

permissions: {} added at the workflow level to six workflows. Without this, GitHub falls back to the default token permissions (typically read/write on contents), which is
broader than needed. Job-level permissions already existed and are unaffected.

Explanatory comments added to all job-level permission entries. Zizmor's undocumented-permissions audit requires each permission to explain why it's needed, so a future reader
knows if it's still necessary.

concurrency: blocks added to add_member.yml, member-verification.yml, and zizmor.yml. Without concurrency limits, if two issues are opened in quick succession, two runs of add_member.yml could execute simultaneously. Both would check out the same branch state, both would try to create a branch named add-user/<issue-number> (or race to push to main), and both would try to create a pull request — likely causing one to fail with a git
conflict or a "PR already exists" error.

For member-verification.yml, two PRs opened at the same time would just run independently and correctly — there's no shared state between them. The concurrency key is ${{
github.event.pull_request.number }}, so only runs for the same PR would be deduplicated. In practice the trigger is types: [opened] so there can only ever be one opened event
per PR, making the concurrency block essentially a no-op for this workflow. It's still correct to have it as a safeguard.

For zizmor.yml, a push to main followed immediately by another push could queue two runs scanning the same or nearly-identical code. The concurrency block with
cancel-in-progress: true cancels the older run in favour of the newer one, avoiding redundant work. No correctness risk, just efficiency.

name: added to the unnamed verify-new-member job in member-verification.yml.

environment: production added to jobs that reference TERRAFORM_MANAGEMENT_GITHUB_TOKEN. Zizmor's secrets-outside-env audit flags secrets used without a GitHub Environment, as
environments enable access controls and audit trails around sensitive credentials. The environment was created with no protection rules so the apply workflows remain fully
automated.

\# zizmor: ignore[artipacked] added to the checkout step in add_member.yml. persist-credentials: true is intentional there — the job performs git push after checking out, so
credentials must persist. The finding is suppressed rather than worked around.
@cunla
Copy link
Copy Markdown
Member

cunla commented Apr 18, 2026

Terraform plan for members

Plan: 0 to add, 2 to change, 0 to destroy.
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
!~  update in-place

Terraform will perform the following actions:

  # github_team_members.org_team_members["Admins"] will be updated in-place
!~  resource "github_team_members" "org_team_members" {
        id      = "9763562"
#        (1 unchanged attribute hidden)

-       members {
-           role     = "maintainer" -> null
-           username = "Stormheg" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "cunla" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "ryancheley" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "tim-schilling" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "williln" -> null
        }
+       members {
+           role     = "member"
+           username = "Stormheg"
        }
+       members {
+           role     = "member"
+           username = "cunla"
        }
+       members {
+           role     = "member"
+           username = "ryancheley"
        }
+       members {
+           role     = "member"
+           username = "tim-schilling"
        }
+       members {
+           role     = "member"
+           username = "williln"
        }

#        (3 unchanged blocks hidden)
    }

  # github_team_members.org_team_members["super-admins"] will be updated in-place
!~  resource "github_team_members" "org_team_members" {
        id      = "15586545"
#        (1 unchanged attribute hidden)

-       members {
-           role     = "maintainer" -> null
-           username = "Stormheg" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "cunla" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "ryancheley" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "tim-schilling" -> null
        }
-       members {
-           role     = "maintainer" -> null
-           username = "williln" -> null
        }
+       members {
+           role     = "member"
+           username = "Stormheg"
        }
+       members {
+           role     = "member"
+           username = "cunla"
        }
+       members {
+           role     = "member"
+           username = "ryancheley"
        }
+       members {
+           role     = "member"
+           username = "tim-schilling"
        }
+       members {
+           role     = "member"
+           username = "williln"
        }
    }

Plan: 0 to add, 2 to change, 0 to destroy.

✅ Plan applied in Apply org membership changes #33

Outputs
invalid_users = []

@cunla
Copy link
Copy Markdown
Member

cunla commented Apr 18, 2026

Terraform plan for repos

No changes. Your infrastructure matches the configuration.
No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

📝 Plan generated in Plan org repositories changes and list them in a PR #53

@tim-schilling
Copy link
Copy Markdown
Member Author

That's what I get for changing the repo's outside of the terraform file 😅

Copy link
Copy Markdown
Member

@Stormheg Stormheg left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

@Stormheg Stormheg merged commit 7d6de52 into main Apr 21, 2026
6 checks passed
@Stormheg Stormheg deleted the zizmor-fixes branch April 21, 2026 10:24
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