Skip to content

Commit 8f25b64

Browse files
committed
Run the backport bot in CI
A pull request labelled needs-backport now opens its own backport pull requests when it merges, so the seven branch case needs nobody at a keyboard Two jobs, and the split is the point. analyze is the only job that reaches the model and it has contents: read. publish is the only job that can write and it never reaches the model. Repository content, which the model reads, is therefore never handled by a job holding a token that could change the repository. The verdict moves between them as an artifact The workflow assumes its own AwsLcGitHubActionsBackportOidcRole, pinned by job_workflow_ref to this one file, and that role is what chains into the shared AwsLcGitHubActionsBedrockRole that #3376 renamed for exactly this. The bot cannot borrow autofix's role and general CI cannot borrow either In CI the checkout already is aws/aws-lc, so the branches have nowhere else to go. --push-to-aws-lc says so out loud, is refused anywhere else, and push_branch will only ever push a branch named backport-, so the escape has two independent limits rather than one The checkout is the merge commit, never the pull request head, so no untrusted code runs
1 parent 2bd6434 commit 8f25b64

7 files changed

Lines changed: 259 additions & 12 deletions

File tree

.github/workflows/backport-bot.yml

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
# SPDX-License-Identifier: Apache-2.0 OR ISC
3+
#
4+
# Backport bot. When a pull request labelled needs-backport merges, work out which
5+
# supported release branches still need the fix and open one pull request each.
6+
#
7+
# Split into two jobs on purpose. `analyze` is the only job that talks to the model,
8+
# and it has no write access. `publish` is the only job that can write, and it never
9+
# talks to the model. So repository content, which the model reads, is never handled
10+
# by a job holding a token that could change the repository.
11+
#
12+
# The OIDC role is pinned to this file by job_workflow_ref in
13+
# tests/ci/cdk/cdk/aws_lc_github_oidc_stack.py, so renaming this file breaks the
14+
# trust policy until the CDK stack is redeployed.
15+
name: backport-bot
16+
17+
on:
18+
pull_request_target:
19+
types: [closed]
20+
21+
permissions: {}
22+
23+
jobs:
24+
analyze:
25+
name: analyze
26+
# Only merged pull requests, and only when somebody asked for a backport. Reading
27+
# the label here rather than in the tool keeps the decision with the reviewers
28+
if: >-
29+
github.event.pull_request.merged == true &&
30+
contains(github.event.pull_request.labels.*.name, 'needs-backport')
31+
permissions:
32+
id-token: write # to assume the Bedrock role
33+
contents: read
34+
runs-on:
35+
- codebuild-aws-lc-ci-github-actions-${{ github.run_id }}-${{ github.run_attempt }}
36+
image:linux-5.0
37+
instance-size:small
38+
steps:
39+
- uses: actions/checkout@v6
40+
with:
41+
# The merge commit, not the pull request head, so no untrusted code runs
42+
ref: ${{ github.event.pull_request.merge_commit_sha }}
43+
# Brings every branch, which is how the tool sees the release branches, and
44+
# the whole history, which it needs to compare them
45+
fetch-depth: 0
46+
persist-credentials: false
47+
- name: Load shared Bedrock settings
48+
# For configure-aws-credentials, which takes no region input and reads
49+
# AWS_REGION from the environment. No model id is set here: the tool reads
50+
# the same shared file itself
51+
run: |
52+
jq -r '"AWS_REGION=\(.aws_region)"' \
53+
.github/workflows/ai-config.json >> "$GITHUB_ENV"
54+
- uses: ./.github/actions/configure-aws-credentials
55+
with:
56+
oidcRole: AwsLcGitHubActionsBackportOidcRole
57+
roleName: AwsLcGitHubActionsBedrockRole
58+
- name: Install the AI client
59+
run: pip3 install --user anthropic boto3
60+
- name: Work out which branches need the fix
61+
run: |
62+
util/backport/backport analyze \
63+
--commit "${{ github.event.pull_request.merge_commit_sha }}" --skip
64+
- name: Keep the verdict for the publish job
65+
uses: actions/upload-artifact@v4
66+
with:
67+
name: backport-run
68+
path: util/backport/.backport-runs/last-run.json
69+
if-no-files-found: error
70+
71+
publish:
72+
name: publish
73+
needs: analyze
74+
permissions:
75+
contents: write # to push the backport branches
76+
pull-requests: write # to open and comment on the backport pull requests
77+
runs-on:
78+
- codebuild-aws-lc-ci-github-actions-${{ github.run_id }}-${{ github.run_attempt }}
79+
image:linux-5.0
80+
instance-size:small
81+
steps:
82+
- uses: actions/checkout@v6
83+
with:
84+
ref: ${{ github.event.pull_request.merge_commit_sha }}
85+
fetch-depth: 0
86+
# persist-credentials is left alone here, unlike in analyze: this job pushes
87+
# the backport branches, and it is the checkout's credentials that do it
88+
- name: Take the verdict from the analyze job
89+
uses: actions/download-artifact@v4
90+
with:
91+
name: backport-run
92+
path: util/backport/.backport-runs
93+
- name: Cherry-pick and open the pull requests
94+
env:
95+
GH_TOKEN: ${{ github.token }}
96+
run: |
97+
# cherry-pick needs an author, and a runner has no git identity
98+
git config user.name "aws-lc-backport-bot"
99+
git config user.email "backport-bot@users.noreply.github.com"
100+
# --push-to-aws-lc because in CI the checkout is aws/aws-lc itself,
101+
# so the backport branches have nowhere else to go
102+
util/backport/backport apply --yes
103+
util/backport/backport publish \
104+
--pr "${{ github.event.pull_request.number }}" \
105+
--push-to-aws-lc --yes

tests/ci/cdk/cdk/aws_lc_github_oidc_stack.py

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,34 @@ def __init__(
9393
},
9494
}))
9595

96+
# The backport bot gets its own OIDC role, pinned to its own workflow file the
97+
# same way autofix's role is above. Without a role of its own, any workflow in
98+
# the repository could assume it and reach the shared Bedrock role below
99+
backport_oidc_role_name = "AwsLcGitHubActionsBackportOidcRole"
100+
self.backport_oidc_role = iam.Role(self, id=backport_oidc_role_name, role_name=backport_oidc_role_name,
101+
assumed_by=iam.WebIdentityPrincipal(self.oidc_provider.attr_arn, {
102+
"StringEquals": {
103+
"token.actions.githubusercontent.com:aud": "sts.amazonaws.com",
104+
},
105+
"StringLike": {
106+
"token.actions.githubusercontent.com:sub": "repo:{}/{}:*".format(
107+
GITHUB_REPO_OWNER, (
108+
STAGING_GITHUB_REPO_NAME
109+
if (env.account == PRE_PROD_ACCOUNT)
110+
else GITHUB_REPO_NAME
111+
)
112+
),
113+
"token.actions.githubusercontent.com:job_workflow_ref":
114+
"{}/{}/.github/workflows/backport-bot.yml@*".format(
115+
GITHUB_REPO_OWNER, (
116+
STAGING_GITHUB_REPO_NAME
117+
if (env.account == PRE_PROD_ACCOUNT)
118+
else GITHUB_REPO_NAME
119+
)
120+
),
121+
},
122+
}))
123+
96124
ecr_repos = [ecr.Repository.from_repository_name(self, x.replace('/', '-'), repository_name=x)
97125
for x in ECR_REPOS]
98126

@@ -116,9 +144,13 @@ def __init__(
116144
block_public_access=s3.BlockPublicAccess.BLOCK_ALL,
117145
)
118146

147+
# The shared Bedrock role. Autofix and the backport bot both assume it, each
148+
# through its own pinned OIDC role, so neither can borrow the other's
119149
self.bedrock_role = create_bedrock_role(
120-
self, "AwsLcGitHubActionsBedrockRole", env, self.autofix_oidc_role)
150+
self, "AwsLcGitHubActionsBedrockRole", env,
151+
[self.autofix_oidc_role, self.backport_oidc_role])
121152
self.bedrock_role.grant_assume_role(self.autofix_oidc_role)
153+
self.bedrock_role.grant_assume_role(self.backport_oidc_role)
122154

123155
self.autofix_upload_role = create_autofix_upload_role(
124156
self, "AwsLcGitHubActionAutofixUploadRole", self.autofix_oidc_role,
@@ -334,9 +366,10 @@ def create_standard_github_actions_role(scope: Construct, id: str,
334366

335367
def create_bedrock_role(scope: Construct, id: str,
336368
env: typing.Union[Environment, typing.Dict[str, typing.Any]],
337-
principal: iam.IPrincipal) -> iam.Role:
369+
principals: typing.List[iam.IPrincipal]) -> iam.Role:
338370
return iam.Role(scope, id, role_name=id,
339-
assumed_by=iam.SessionTagsPrincipal(principal),
371+
assumed_by=iam.CompositePrincipal(
372+
*[iam.SessionTagsPrincipal(p) for p in principals]),
340373
inline_policies={
341374
"bedrock_policy": iam.PolicyDocument(
342375
statements=[

util/backport/README.md

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,12 @@ util/backport/backport apply --open-pr
204204
| `--pr` | source pull request number, linked in each body and given a summary comment |
205205
| `--remote` | fork remote the branches are pushed to, `origin` by default |
206206
| `--dry-run` | print what would be pushed and opened, touch nothing |
207+
| `--push-to-aws-lc` | let branches be pushed to `aws/aws-lc`. For CI only |
207208
| `--yes` | skip the confirm, for scripts and CI |
208209

209210
Branches go to your fork; the pull requests are opened against `aws/aws-lc`. Pushing
210-
to `aws/aws-lc` is refused outright, so a stray `--remote` cannot put half-reviewed
211-
work on the real repository.
211+
branches to `aws/aws-lc` is refused unless `--push-to-aws-lc` asks for it, which only
212+
CI does, so a stray `--remote` cannot put half-reviewed work on the real repository.
212213

213214
**Example Output:**
214215

@@ -239,6 +240,44 @@ Nothing is ever a draft and nothing is auto-merged. Re-running is safe: a branch
239240
already has a pull request is left alone, and finishing a conflict by hand is enough
240241
to let the next run pick it up, with no need to run `apply` again.
241242

243+
## Running In CI
244+
245+
`.github/workflows/backport-bot.yml` does the same three steps automatically. It fires
246+
when a pull request labelled `needs-backport` merges, so the decision to backport stays
247+
with the reviewers rather than with the tool.
248+
249+
It runs as two jobs, and the split is the point:
250+
251+
| Job | Can reach the model | Can write to the repo |
252+
| --- | --- | --- |
253+
| `analyze` | yes, `id-token: write` for Bedrock | no, `contents: read` |
254+
| `publish` | no | yes, `contents: write` and `pull-requests: write` |
255+
256+
The model reads repository content, so the job that reads it is never the job holding a
257+
token that could change it. The verdict travels between them as an artifact.
258+
259+
Branches are pushed to `aws/aws-lc` itself, because in CI the checkout already is
260+
`aws/aws-lc`. That needs `--push-to-aws-lc`, which is refused everywhere else,
261+
and `push_branch` will only ever push a `backport-` branch, so the escape cannot reach
262+
anything else.
263+
264+
The workflow assumes `AwsLcGitHubActionsBackportOidcRole`, which the OIDC stack pins to
265+
this exact workflow file by `job_workflow_ref`. Renaming the file breaks the trust
266+
policy until the CDK stack is redeployed. That role is the only thing allowed to chain
267+
into the shared `AwsLcGitHubActionsBedrockRole`, alongside autofix's own OIDC role.
268+
269+
### Before the bot can run
270+
271+
Two things have to be true first, and neither is settled by merging this code.
272+
273+
The role has to exist. CDK takes the role name from the construct id, so
274+
`tests/ci/cdk` has to be deployed before the workflow can assume it. Until then the
275+
`analyze` job fails at the credentials step.
276+
277+
A pull request opened with `GITHUB_TOKEN` does not start any workflow, so the backport
278+
pull requests arrive with no CI of their own. Getting them tested needs a token that is
279+
not `GITHUB_TOKEN`, which is a decision about secrets rather than about this tool.
280+
242281
## Configuration
243282

244283
### Model settings
@@ -333,6 +372,9 @@ util/backport/
333372
.backport-worktrees/ # where a conflicted pick waits, not checked in
334373
```
335374

375+
The CI half of the tool lives outside this folder, in
376+
`.github/workflows/backport-bot.yml` and `tests/ci/cdk/cdk/aws_lc_github_oidc_stack.py`.
377+
336378
## Testing
337379

338380
### Unit tests
@@ -489,6 +531,9 @@ Backport branches belong on a fork; only the pull requests go to `aws/aws-lc`. P
489531
util/backport/backport publish --remote origin
490532
```
491533

534+
CI is the one exception: it runs inside `aws/aws-lc` already, and passes
535+
`--push-to-aws-lc` to say so.
536+
492537
### Wrong or empty results from a subdirectory
493538

494539
Should not happen. The tool pins itself to the checkout it lives in rather than

util/backport/src/commands/publish.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ def cmd_publish(args) -> int:
184184

185185
# Checked before anything is printed, so nobody confirms a run that cannot work
186186
require_gh()
187-
push_slug = require_push_remote(args.remote)
187+
push_slug = require_push_remote(args.remote, args.push_to_aws_lc)
188188

189189
repo = base_repo("upstream")
190190
print(f"Fix {run['fix'][:10]}")

util/backport/src/main.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ def add_publish(subparsers) -> None:
8484
action="store_true",
8585
help="say what would be pushed and opened, without doing it",
8686
)
87+
p.add_argument(
88+
"--push-to-aws-lc",
89+
action="store_true",
90+
help="let branches be pushed to aws/aws-lc. For CI, which runs there already",
91+
)
8792
p.add_argument(
8893
"--yes",
8994
action="store_true",

util/backport/src/util/github.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
Kept in one place so publish and resolve cannot grow two different PR openers
77
"""
88

9-
from util.config import BackportError
9+
from util.config import BACKPORT_BRANCH_PREFIX, BackportError
1010
from util.git import git, run
1111

1212
import re
1313
import shutil
1414
from typing import List, Optional, Tuple
1515

16-
# The repo backports are reviewed in. Branches are never pushed here, only PRs opened
16+
# Where the pull requests are opened. Branches only go here when CI says so
1717
AWS_LC_REPO = "aws/aws-lc"
1818

1919
_SLUG = re.compile(r"github\.com[:/]+([^/]+)/([^/]+?)(?:\.git)?/?$")
@@ -60,16 +60,18 @@ def remote_slug(remote: str) -> Optional[str]:
6060
return f"{found.group(1)}/{found.group(2)}" if found else None
6161

6262

63-
def require_push_remote(remote: str) -> str:
63+
def require_push_remote(remote: str, allow_aws_lc: bool = False) -> str:
6464
"""
6565
Checks a remote is somewhere we are allowed to push branches
66-
Returns its owner/repo. aws/aws-lc is refused: backport branches belong on a fork,
67-
and pushing them there would put half-reviewed work on the real repository
66+
Returns its owner/repo. aws/aws-lc is refused unless allow_aws_lc is set, which
67+
only CI does: running there, the checkout already is aws/aws-lc, so the branches
68+
have nowhere else to go. Locally the refusal stands, so a stray --remote cannot
69+
put half-reviewed work on the real repository
6870
"""
6971
slug = remote_slug(remote)
7072
if slug is None:
7173
raise BackportError(f"no remote called '{remote}' in this checkout.")
72-
if slug.lower() == AWS_LC_REPO:
74+
if slug.lower() == AWS_LC_REPO and not allow_aws_lc:
7375
raise BackportError(
7476
f"remote '{remote}' is {slug}, which is where the pull requests go, not\n"
7577
" where the branches go. Push to your fork instead, with --remote."
@@ -104,7 +106,11 @@ def push_branch(remote: str, branch: str) -> Optional[str]:
104106
"""
105107
Pushes one local branch to the remote
106108
Returns None on success or the git error, so one bad branch cannot stop the rest
109+
Anything not named backport- is refused, so even the CI run that is allowed to
110+
push to aws/aws-lc can only push branches this tool built
107111
"""
112+
if not branch.startswith(BACKPORT_BRANCH_PREFIX):
113+
raise BackportError(f"refusing to push '{branch}': not a backport branch.")
108114
pushed = git(
109115
"push", "--force-with-lease", remote, f"{branch}:{branch}", check=False
110116
)

util/backport/testing/test_engine.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,5 +1531,58 @@ def test_a_good_run_returns_the_url(self):
15311531
self.assertEqual(detail, "http://pr/9")
15321532

15331533

1534+
class PushToAwsLcEscape(unittest.TestCase):
1535+
# CI runs inside aws/aws-lc, so the branches have nowhere else to go. That escape
1536+
# has to be explicit, and it must not weaken the local refusal
1537+
1538+
def test_ci_may_push_to_aws_lc(self):
1539+
with mock.patch.object(github, "remote_slug", lambda r: "aws/aws-lc"):
1540+
got = github.require_push_remote("origin", allow_aws_lc=True)
1541+
self.assertEqual(got, "aws/aws-lc")
1542+
1543+
def test_the_refusal_still_stands_by_default(self):
1544+
with mock.patch.object(
1545+
github, "remote_slug", lambda r: "aws/aws-lc"
1546+
), self.assertRaises(config.BackportError):
1547+
github.require_push_remote("origin")
1548+
1549+
def test_a_missing_remote_is_still_an_error_even_for_ci(self):
1550+
with mock.patch.object(
1551+
github, "remote_slug", lambda r: None
1552+
), self.assertRaises(config.BackportError):
1553+
github.require_push_remote("nope", allow_aws_lc=True)
1554+
1555+
1556+
class PushOnlyBackportBranches(unittest.TestCase):
1557+
# The escape above means a bug here could push anything to the real repository, so
1558+
# the branch name is checked too. Two independent limits, not one
1559+
1560+
def test_a_backport_branch_is_pushed(self):
1561+
pushed = []
1562+
1563+
def record(*args, **kwargs):
1564+
# Records the call, so a silent no-push cannot pass as a success
1565+
pushed.append(args)
1566+
return completed()
1567+
1568+
with mock.patch.object(github, "git", record):
1569+
error = github.push_branch("origin", "backport-fips-2024-09-27-abc")
1570+
self.assertIsNone(error)
1571+
self.assertEqual(len(pushed), 1)
1572+
1573+
def test_anything_else_is_refused_before_git_runs(self):
1574+
with mock.patch.object(github, "git", never_called):
1575+
for branch in ("main", "fips-2024-09-27", "my-feature"):
1576+
with self.assertRaises(config.BackportError):
1577+
github.push_branch("origin", branch)
1578+
1579+
def test_a_failed_push_comes_back_as_text(self):
1580+
with mock.patch.object(
1581+
github, "git", lambda *a, **k: completed(returncode=1, stdout="denied")
1582+
):
1583+
got = github.push_branch("origin", "backport-x")
1584+
self.assertIn("denied", got)
1585+
1586+
15341587
if __name__ == "__main__":
15351588
unittest.main()

0 commit comments

Comments
 (0)