Roles for our pathogen-repo-build GitHub Actions workflow #6
Conversation
|
@joverlee521 Not finished, but I'd appreciate some earlier-than-later 👀 on this from you. |
There was a problem hiding this comment.
Left a couple of questions, but we'll probably want to revisit the whole idea again based on your commment.
| @@ -21,7 +21,7 @@ resource "aws_iam_role" "GitHubActionsRoleNextstrainBatchJobs" { | |||
| "Condition": { | |||
| "StringLike": { | |||
| "token.actions.githubusercontent.com:aud": "sts.amazonaws.com", | |||
| "token.actions.githubusercontent.com:sub": "repo:nextstrain/.github:*" | |||
| "token.actions.githubusercontent.com:sub": "repo:nextstrain/*:*:job_workflow_ref:nextstrain/.github/.github/workflows/pathogen-repo-build.yaml@refs/heads/*", | |||
There was a problem hiding this comment.
Would @refs/heads/* prevent the use of specific commit hashes, e.g. pathogen-repo-build.yaml@e48956bb5eb0b1db1ba3d614ede9d1a6275b72ec?
There was a problem hiding this comment.
Yes, it would… and that was kinda intentional. I was worried that forks could get a specific commit id accessible in workflow calls. Reason is that GitHub (roughly speaking) stores all forks in the same Git storage backend, which means you can do things like view commits in forks via the forked repo, e.g. nextstrain/test-github-actions@0d1fee0 which really comes from https://github.com/tsibley/test-github-actions.
However, GitHub's protected against such attacks:
(In hindsight, of course they have, but they've also had other oversights in Actions before, so I don't think I was wrong to be wary.)
| # Builds inside the AWS Batch runtime need access to the jobs bucket. | ||
| aws_iam_policy.NextstrainJobsAccessToBucket.arn, |
There was a problem hiding this comment.
I'm confused by this. Why does the build require access to the jobs bucket? I thought that was handled by the Nextstrain CLI?
Edit: OH duh, it's the workdir for the Batch job so the build needs to be able to write to bucket.
There was a problem hiding this comment.
Yup. That permission is provided by default by our Batch job role, but that role doesn't apply if you pass in your own credentials. In my ideal world we'd be able to take the union of the default job role + any additional creds passed in, but that's non-trivial in my understanding.
| } | ||
|
|
||
| resource "github_actions_repository_oidc_subject_claim_customization_template" "nextstrain" { | ||
| for_each = toset(data.github_repositories.nextstrain.names) |
There was a problem hiding this comment.
Am I understanding this correctly that this is for existing Nextstrain repos? For any new repos, we'd have to re-deploy the Terraform plan?
There was a problem hiding this comment.
Correct. No code changes required, but a new plan and apply cycle would be needed. (We could run that periodically on automation…)
Will let us manage (and self-document) our GitHub organization and repo settings via this Terraform configuration.
tsibley
force-pushed
the
trs/pathogen-repo-build-workflow
branch
3 times, most recently
from
ffe1302
to
8d33863
Compare
Static role permissions policies which are expected to be narrowed/scoped-down by an inline session policy set by the pathogen-repo-build workflow. Problems with this approach noted in review commentary: - <nextstrain/.github#81 (comment)> - <nextstrain/.github#81 (comment)> Resolves: <#4> Related-to: <nextstrain/.github#81>
tsibley
force-pushed
the
trs/pathogen-repo-build-workflow
branch
from
8d33863
to
3225f90
Compare
|
The approach here was static role permissions policies which are expected to be narrowed/scoped-down by an inline session policy set by the pathogen-repo-build workflow. But problems with this approach noted in review commentary: |
TKTK Resolves: <#4> Related-to: <nextstrain/.github#81> Supersedes: <#6>, <#7>
TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>
TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>
TKTK Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>
A collection of templated repo-specific roles for inside a Nextstrain runtime and one role for the GitHub Actions job itself, i.e. outside the runtime. The inside-the-runtime roles are given pathogen-specific permissions necessary for the ingest and phylogenetic workflows of a pathogen repo. The outside-the-runtime role is only necessary/used at the moment for access to AWS Batch. The roles can only be assumed by specific repos when performed by our centralized pathogen-repo-build workflow. While this doesn't completely prevent off-label use by other GitHub Actions workflows launched from a pathogen repo, it does make it more involved to do so, hopefully to the point of discouragement. The associated GitHub repository configuration is managed by Terraform now as well since the customization of the "sub" claim in GitHub Action's OIDC token is tightly coupled to our AWS role trust policies. Resolves: <#4> Related-to: <nextstrain/private#96>, <nextstrain/.github#81> Supersedes: <#6>, <#7>
See commits.
Resolves: #4
Related-to: nextstrain/.github#81
Checklist