Skip to content

fix(iam): pin javabin-ci-infra to platform-ci.yml on main - #161

Open
Alexanderamiri wants to merge 2 commits into
mainfrom
fix/sec-ci-infra-pin
Open

Alexanderamiri wants to merge 2 commits into
mainfrom
fix/sec-ci-infra-pin

Conversation

@Alexanderamiri

Copy link
Copy Markdown
Member

Why

javabin-ci-infra (AllowAll inside the org boundary) trusts the OIDC sub repo:javaBin/platform:environment:production. That sub has no branch in it. The production environment currently has no branch policy and no protection rules (gh api repos/javaBin/platform/environments). So a workflow pushed to any branch that declares environment: production can assume the apply role. The org default repo permission is write, which makes that 38 people. This is finding C1 in the security audit.

What

  • iam/main.tf: the ci_infra trust now also requires token.actions.githubusercontent.com:job_workflow_ref = javaBin/platform/.github/workflows/platform-ci.yml@refs/heads/main. Both existing sub values stay. Only platform-ci.yml as committed on main can assume the role now, whatever the environment settings say.
  • platform-ci.yml: the weekly drift job uses javabin-ci-infra-plan instead of javabin-ci-infra. Drift only plans. The plan role already has ReadOnlyAccess, the infra lock table, and ssm:GetParameter on /javabin/slack/* for the webhook. After this change the apply job is the only user of javabin-ci-infra.
  • Docs: ci-workflow.md and platform-modules.md describe the new trust.

This is defence in depth. The GitHub-side fix is to limit production to main. That is a manual settings change, listed in the audit report.

How verified

  • terraform fmt -recursive -check and terraform validate (aws provider 5.100.0) pass.
  • python -m pytest terraform/lambda-src/tests scripts/tests passes (107 tests).
  • The job_workflow_ref condition key already works in this account. javabin-ci-app-broker, javabin-ci-apply-gate and javabin-ci-override-approver use it, and all three were used today.
  • The apply job runs on push, workflow_dispatch and repository_dispatch, all gated to refs/heads/main. Each of these produces job_workflow_ref = ...platform-ci.yml@refs/heads/main.
  • Expected plan: 1 in-place update, module.iam.aws_iam_role.ci_infra (trust policy only).

Rollback

The apply that changes the trust runs with credentials it assumed before the change. If a later apply is rejected, an admin SSO session can revert this one resource locally with terraform apply. The other option is to remove the job_workflow_ref condition from the role trust in the IAM console.

Follow-ups

  • GitHub settings: limit the production environment to main. The commands are in the audit report.

🤖 Generated with Claude Code

The production-environment OIDC sub carries no branch, so any branch that
declared the environment could assume the apply role. Require
job_workflow_ref = platform-ci.yml@refs/heads/main, and move the drift job
to the read-only plan role so apply is the only ci-infra user.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@Alexanderamiri
Alexanderamiri requested a review from a team as a code owner October 24, 2026 13:37
@github-actions

Copy link
Copy Markdown

Terraform Plan

🚧 Changes detected — Plan: 0 to add, 1 to change, 0 to destroy.

Plan output
aws_ssm_parameter.atlassian_org_id: Refreshing state... [id=/javabin/platform/atlassian-org-id]
aws_ssm_parameter.atlassian_api_key: Refreshing state... [id=/javabin/platform/atlassian-api-key]

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:

  # module.iam.aws_iam_role.ci_infra will be updated in-place
  ~ resource "aws_iam_role" "ci_infra" {
      ~ assume_role_policy    = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                      ~ Condition = {
                          ~ StringLike   = {
                              + "token.actions.githubusercontent.com:job_workflow_ref" = "javaBin/platform/.github/workflows/platform-ci.yml@refs/heads/main"
                                # (1 unchanged attribute hidden)
                            }
                            # (1 unchanged attribute hidden)
                        }
                        # (3 unchanged attributes hidden)
                    },
                ]
                # (1 unchanged attribute hidden)
            }
        )
        id                    = "javabin-ci-infra"
        name                  = "javabin-ci-infra"
        tags                  = {
            "Name" = "javabin-ci-infra"
        }
        # (9 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

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

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

LLM Review

Risk: 🟡 MEDIUM

Plan updates one IAM role (javabin-ci-infra) in place by adding a job_workflow_ref condition to its GitHub OIDC trust policy; no creates or destroys.

  • 🔒 [security] The assume_role_policy of the CI infrastructure role gains a new StringLike condition, token.actions.githubusercontent.com:job_workflow_ref = javaBin/platform/.github/workflows/platform-ci.yml@refs/heads/main. This narrows which GitHub workflow can assume the role, which is a tightening rather than an expansion. Confirm the Effect of the statement is Allow and that the condition matches the intended workflow path and branch.
  • 🔒 [security] Because the trust policy is changing, any workflow other than platform-ci.yml on main (including other branches, forks, or other workflows) will lose the ability to assume this role after apply. Verify no other pipelines depend on javabin-ci-infra before applying.
  • 💥 [destruction] Update is in place. No resources are created or destroyed (Plan: 0 to add, 1 to change, 0 to destroy). The role name, ID and tags are unchanged.
  • 💰 [cost] No billable resources are added or modified by this plan.
  • ✅ [routine] Single-attribute trust policy change. Not a high-risk change on its own, so rated MEDIUM because it alters authentication for a CI role that can affect infrastructure.

@github-actions

Copy link
Copy Markdown

Terraform Plan

🚧 Changes detected — Plan: 0 to add, 1 to change, 0 to destroy.

Plan output
Acquiring state lock. This may take a few moments...
aws_ssm_parameter.atlassian_org_id: Refreshing state... [id=/javabin/platform/atlassian-org-id]
aws_ssm_parameter.atlassian_api_key: Refreshing state... [id=/javabin/platform/atlassian-api-key]

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:

  # module.iam.aws_iam_role.ci_infra will be updated in-place
  ~ resource "aws_iam_role" "ci_infra" {
      ~ assume_role_policy    = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                      ~ Condition = {
                          ~ StringLike   = {
                              + "token.actions.githubusercontent.com:job_workflow_ref" = "javaBin/platform/.github/workflows/platform-ci.yml@refs/heads/main"
                                # (1 unchanged attribute hidden)
                            }
                            # (1 unchanged attribute hidden)
                        }
                        # (3 unchanged attributes hidden)
                    },
                ]
                # (1 unchanged attribute hidden)
            }
        )
        id                    = "javabin-ci-infra"
        name                  = "javabin-ci-infra"
        tags                  = {
            "Name" = "javabin-ci-infra"
        }
        # (9 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

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

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

LLM Review

Risk: 🟡 MEDIUM

Plan updates one IAM role (javabin-ci-infra) in place by adding a StringLike trust-policy condition restricting GitHub OIDC job_workflow_ref to javaBin/platform's platform-ci.yml on main; no creates or destroys.

  • 🔒 [security] The assume_role_policy of the CI infrastructure role changes: a new StringLike condition on token.actions.githubusercontent.com:job_workflow_ref is added, limiting which GitHub Actions workflow can assume the role to javaBin/platform/.github/workflows/platform-ci.yml on refs/heads/main. This tightens trust, so it is a reduction in exposure rather than an expansion. Confirm the value matches the actual workflow path and branch, since a mismatch will cause CI to fail to assume the role.
  • 🔒 [security] The diff shows only the trust policy Condition change. The Statement's Effect, Principal and Action are hidden as unchanged, so the statement is not being widened. No new Allow grants, security group rules, or public exposure appear in this plan.
  • 💥 [destruction] Plan is 0 to add, 1 to change, 0 to destroy. The role is updated in place, not replaced, so there is no risk of losing the role or its ARN.
  • ✅ [routine] Behavioural risk: existing CI runs from other workflows, other branches, or other repositories that assume this role will be denied after apply. Verify that platform-ci.yml on main is the only intended caller.
  • 💰 [cost] No billable resources are created or modified. No cost impact.

@Alexanderamiri

Copy link
Copy Markdown
Member Author

Review fix: corrected the role table in docs/platform-modules.md. javabin-ci-infra-plan trusts any platform branch / PR, and the app rows now describe the broker model (ci-app-broker, ci-team-{team}, ci-deploy-{team}).

This branch has not been deployed

No deployments
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