Run the backport bot in CI - #3416
Conversation
91df775 to
db872ad
Compare
f1eca9f to
caf1347
Compare
db872ad to
5230da1
Compare
caf1347 to
e9becee
Compare
5230da1 to
3dc664f
Compare
e9becee to
7c1ee95
Compare
3dc664f to
b0af9b0
Compare
7c1ee95 to
c9dc173
Compare
b0af9b0 to
46a366f
Compare
c9dc173 to
af8be4e
Compare
46a366f to
58aa3b4
Compare
af8be4e to
db6b557
Compare
a98ee5d to
8f25b64
Compare
db6b557 to
dc7c482
Compare
8f25b64 to
d678198
Compare
dc7c482 to
4396667
Compare
| # The backport bot gets its own OIDC role, pinned to its own workflow file the | ||
| # same way autofix's role is above. Without a role of its own, any workflow in | ||
| # the repository could assume it and reach the shared Bedrock role below | ||
| backport_oidc_role_name = "AwsLcGitHubActionsBackportOidcRole" | ||
| self.backport_oidc_role = iam.Role(self, id=backport_oidc_role_name, role_name=backport_oidc_role_name, |
There was a problem hiding this comment.
Is it possible to create a shared role with autofix to use bedrock for both?
b0c0197 to
7ea55a0
Compare
| needs: analyze | ||
| permissions: | ||
| contents: write # to push the backport branches | ||
| pull-requests: write # to open and comment on the backport pull requests |
There was a problem hiding this comment.
For this to work, we need to turn on Allow GitHub Actions to create and approve pull requests under Settings → Actions → General -> Workflow Permissions -> Allow GitHub Actions to create and approve pull requests. is this on?
This poses a security risk due to a GHA having perms to open a PR. However, since this is for backports, which are already established fixes, it might not be a big risk, but for autofix, had a discussion with @justsmth and he recommended against not turning this on as it could open a can of worms. If this was discussed beforehand then ignore.
| oidcRole: AwsLcGitHubActionsBackportOidcRole | ||
| roleName: AwsLcGitHubActionsBedrockRole | ||
| - name: Install the AI client | ||
| run: pip3 install --user anthropic boto3 |
There was a problem hiding this comment.
Pin to a major version release so any vulnerabilities can be tracked
c7fe3e4 to
c896521
Compare
4396667 to
c552a42
Compare
c896521 to
faf3235
Compare
c552a42 to
a720764
Compare
faf3235 to
10eff92
Compare
a720764 to
8ee3798
Compare
10eff92 to
9ae7108
Compare
8ee3798 to
fe5b4ea
Compare
9ae7108 to
aa04cc7
Compare
fe5b4ea to
90e3318
Compare
aa04cc7 to
fb4c4cb
Compare
90e3318 to
5d6728c
Compare
fb4c4cb to
777d18c
Compare
5d6728c to
b41143d
Compare
777d18c to
d7d13b4
Compare
b41143d to
8a94c2f
Compare
### Issues: Addresses `P425131803` ### Description of changes: Currently security fixes (or features) must be manually backported to supported LTS and FIPS branches by manually locating the affected branches from the change in code through either `git blame` or running tests against each branch. For a single fix this can tedious but still manageable, however when multiple fixes arrive, the coordination overhead complicates this process. This pull request adds a tool that **identifies** which currently supported branches are affected by a given commit or collection of commits. ### Call-outs: - The tool only reports its findings from its analysis of the collection of commits and **does not perform any cherrypicks automatically**. - Anything that git history cannot settle is flagged for AI to review, **never immediately flagged as not affected**, avoiding false negatives. - The model **answers through a schema, not prose**. It's given one tool, `record_verdict`, forced with `tool_choice`, whose `input_schema` constrains the verdict to `yes | no | uncertain` and confidence to `high | medium | low`. Bedrock returns the arguments as a dict already in that shape, so there is no reply text to parse. That removes a whole class of bug rather than guarding it: earlier versions read the verdict out of Markdown, and a reasoning sentence starting with "No" could clear a branch. Two Bedrock limits are worth knowing, both confirmed against the live model: `"strict": true` on a tool is rejected (`400 tools.0.custom.strict: Extra inputs are not permitted`), and so is `output_config` with a `json_schema` (`400 output_config.format: Extra inputs are not permitted`), which is the response-format style the Bedrock guide shows. So the enums are a strong steer rather than a hard guarantee, and `read_verdict` still validates: an off-enum value, a missing field, a wrong type, a reply with no `record_verdict` call, or a reply cut short by the token limit all read as no answer, and **no answer leaves the branch flagged**. - A fix that reaches inside `crypto/fipsmodule/` gets a **FIPS boundary warning** after the table. The module is validated as a build of exactly that source, so a backport there has certification consequences this tool cannot judge; all it can do is make sure nobody finds out later. The file list goes into the saved run so `publish` (#3415) can carry the same warning into every pull request it opens. Tests and generated files under that path are excluded, since neither is compiled into the module and a warning that fires on those stops meaning anything. - Locally, the AI pass runs on local Amazon Bedrock credentials, can refer to the `README.md` for setup inquiry and troubleshooting. - Currently this is the **analyze command only**. Applying cherrypicks (#3414), opening the pull requests (#3415), CI integration (#3416) and merge conflict resolution (#3417) follow on top of this one, in that order. Each is reviewable on its own diff, and each leaves the tool working. ### Testing: Tested with two layers run from 'util/backport' from an AWS-LC checkout. **Unit tests** - 137 cases, no checkout or credentials required: ``` python3 -m unittest testing.test_engine ``` They cover the pure helpers (whitespace normalization, C-file detection, comment and boilerplate line filters, source-file selection, test/generated path check, branch ordering), the verdict the model records and every way it can be unreadable, the FIPS boundary check including the paths that must not match it, and the engine itself against a stubbed git: patch-id fingerprinting, cherry-pick trailers, already-patched detection, rename following, every verdict `classify_branch` can reach, the prompt budget, and the support-window dates. **Replay bench** - 39 real AWS-LC fixes across 7 release branches with 157 fix branch cells: ``` python3 testing/replay_fixes.py --no-ai # git history only, ~5 min python3 testing/replay_fixes.py # with the AI pass, ~20 min ``` Each fix is replayed in a throwaway sandbox with 'origin/main' pinned to the fix and every branch that received a backport wound back to the commit before it (making the tool unable to see the answer). The sandbox borrows objects from the checkout, so **nothing is cloned**. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
Pushes the branches apply built and opens one pull request per affected branch. apply --open-pr offers it as soon as the cherry-picks are done. A branch is publishable once its cherry-pick is finished, which is read from git, so resolving a conflict by hand is enough for the next publish to pick it up. A branch carrying no pick of its own is measured against the release ref apply cut it from, so a stale fork cannot make it look ready. Branches go to a fork and the pull requests are opened against aws/aws-lc, both worked out from the checkout instead of assuming origin and upstream. --base-repo points the pull requests elsewhere, for a staging repo. Pushing to aws/aws-lc is refused. A branch that already has an open pull request is left alone, so re-running is safe. Everything that talks to GitHub lives in util/github.py. --dry-run prints the summary comment instead of posting it, and says how many pull requests it would have opened. A fix inside crypto/fipsmodule carries the FIPS boundary warning into every pull request body and the summary comment, from the file list analyze saved. Nothing is ever a draft and nothing is auto-merged.
A pull request labelled needs-backport now opens its own backport pull requests when it merges. Two jobs. 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, so repository content is never handled by a job holding a token that could change the repository. The verdict moves between them as an artifact. publish writes with a GitHub App token, not GITHUB_TOKEN. GITHUB_TOKEN cannot open a pull request unless the repository lets every workflow do so, and pull requests it opens do not start CI, so the backports would arrive untested. The app carries the write scope instead of the repository, and the job itself is contents: read. The workflow assumes its own AwsLcGitHubActionsBackportOidcRole, pinned by job_workflow_ref to this one file, and that role chains into the shared AwsLcGitHubActionsBedrockRole that aws#3376 renamed for this. The general AwsLcGitHubActionsOidcRole now excludes this workflow file too, so the bot cannot skip its own role and assume the general one to reach the rest of CI. In CI the checkout already is aws/aws-lc, so the branches have nowhere else to go. --push-to-aws-lc is refused anywhere else, and push_branch will only ever push a branch named backport-, so the escape has two independent limits. Both run blocks pass event values through the environment instead of interpolating them into the shell, which is GitHub's script injection guidance. The actions are pinned to major versions, matching the rest of the repository. The checkout is the merge commit, never the pull request head, so no untrusted code runs.
d7d13b4 to
7b410bb
Compare
Issues:
Addresses
P425131803Description of changes:
analyze,applyandpublishstill need someone to run them after a fix merges.This pull request adds
.github/workflows/backport-bot.yml, which runs all three automatically when a pull request labelledneeds-backportmerges. The decision to backport stays with the reviewers who apply the label, not with the tool.It also adds the bot's own OIDC role to the CDK stack, chaining into the shared
AwsLcGitHubActionsBedrockRolethat #3376 renamed for exactly this.Stacked on #3415. The base here is
backport-stack/publish, a scaffolding branch holding the commits below it, so this diff is only the 7 files this change touches. I will retarget it tomainas the stack lands. Please don't merge it into the scaffolding branch.Call-outs:
Two jobs, and the split is the point.
analyzeis the only job that reaches the model and it hascontents: read.publishis the only job that can write and it never reaches the model. So repository content, which the model reads, is never handled by a job holding a token that could change the repository. The verdict moves between them as an artifact.The checkout is the merge commit, never the pull request head, so no untrusted code runs.
The role is pinned by
job_workflow_refto this one workflow file. The bot can't borrow autofix's role and general CI can't borrow either. Renaming the file breaks the trust policy until the stack is redeployed.The pinning goes both ways, same as autofix: the general
AwsLcGitHubActionsOidcRolenow excludes this workflow file too, so the bot can't skip its own role and use the general one to reach the rest of CI.In CI the checkout already is
aws/aws-lc, so the branches have nowhere else to go.--push-to-aws-lcsays so out loud, is refused anywhere else, andpush_branchonly ever pushes a branch namedbackport-, so the escape has two independent limits.Two things this needs that merging it doesn't provide, and I can't do either: the CDK stack has to be deployed before the workflow can assume the new role, and a pull request opened with
GITHUB_TOKENstarts no workflow, so the backport pull requests would arrive with no CI of their own. Getting them tested needs a token that isn'tGITHUB_TOKEN.Uses Share Bedrock model settings between autofix and util/backport #3395 (merged) for
.github/workflows/ai-config.json, which the workflow reads for the region.Testing:
Unit tests - 6 new, 173 total:
They cover the
--push-to-aws-lcescape, that the refusal still stands without it, and that only abackport-branch can be pushed.CDK - synthesized the stack (no deploy) and read the rendered trust policies. The backport role is pinned to
backport-bot.yml@*, the general role excludes both AI workflows, and the Bedrock role has one statement per OIDC role. Staging account path rendersaws-lc-stagingtoo.Workflow - parsed it to confirm the two jobs hold the permissions above,
analyzehas no write access,publishhas noid-token, the settings step runs beforeconfigure-aws-credentials, and the CDK pin matches this filename. I can't run the bot end to end until the role is deployed.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.