Repository navigation
feat: add runs-on input to the shareable workflows - #67
Conversation
Every reusable workflow accepts an optional runs-on input (default ubuntu-latest) and threads it into the workflows it calls, so a consuming repository can run the jobs on its own runner such as a CodeBuild-hosted GitHub Actions runner. Callers that do not set the input are unchanged.
This comment has been minimized.
This comment has been minimized.
Claude AI reviewReviewed the full diff (10 shareable workflow files + README) against the base revision. The change mechanically adds an optional Verified completeness and consistency:
No functional, typing, or lifecycle defects were found; this PR does not touch execution-history matching, placeholder/variable substitution, async/polling logic, or OpenTelemetry semantics. Residual test risk: this is a declarative GitHub Actions change with no in-repo harness to execute workflows; verification here is limited to YAML validity and the Reviewed commit |
This comment has been minimized.
This comment has been minimized.
The model workflows drop privileges with sudo and wrote the invoking account's name as the literal `runner`: in sudoers `Defaults:runner env_keep` rules, in `chown runner:<user>` / `install -o runner` calls, and in a traversal ACL on /home/runner. GitHub-hosted runners match all three; CodeBuild-hosted runners run as root with no `runner` account, no /home/runner, and a workspace under /codebuild, so the setup step failed before the model ran, and a sudoers rule that did not match the invoker silently dropped the AWS credentials across sudo. Resolve the account with `id -un` / `id -gn` at runtime, and replace the /home/runner ACL with a top-down walk over the workspace's ancestors that grants execute-only traversal exactly where the unprivileged user is blocked. On GitHub-hosted runners this reproduces the previous behaviour; on CodeBuild it is a no-op because /codebuild is world-traversable. Document the actual runner requirements and the invariant that AWS_CONTAINER_CREDENTIALS_RELATIVE_URI must never be added to env_keep, since sudo's environment reset is what keeps the CodeBuild project role out of the model's reach.
This comment has been minimized.
This comment has been minimized.
The model jobs preserved their Bedrock credentials across sudo with `Defaults:<user> env_keep += ...` rules. That only restricts the environment if the runner image's sudoers still has `env_reset` on and no broader `env_keep`, which is the image's choice, not ours; with the new `runs-on` input any image can be selected, and one that disables the reset would hand the whole job environment (OIDC request token, CodeBuild credential URI, GH_TOKEN) to the model process. Start every model process with `sudo -u <user> -- env -i` and an explicit allowlist instead, as codex-issue-worker already does. The allowlist is the same set the env_keep rules named plus the variables sudo's reset used to keep (HOME, PATH, LANG, USER, LOGNAME, SHELL), so the model sees the same environment on GitHub-hosted runners and is independent of sudo policy everywhere else. AWS_EC2_METADATA_DISABLED is set so the SDK never falls back to instance or container metadata. With that, the four sudoers.d files, their visudo checks, and the "never add X to env_keep" caveat are gone; the preflight that checked whether sudo preserved the credentials now checks the job environment directly. In run_claude_isolated.sh unset optional variables (AWS_BEARER_TOKEN_BEDROCK, CLAUDE_CODE_*) are skipped rather than passed as empty strings.
This comment has been minimized.
This comment has been minimized.
…nuation Two gaps surfaced by review of the runs-on input: - The caller templates in docs/ai-pr-review-address.md did not carry runs-on, so a repository copying them would leave the workflow_run continuation (and the intake/address jobs) on ubuntu-latest while the rest of its workflows moved to a hosted runner. All three calls now show the input, with a note that the value must be identical across them. - The prepare scripts create the unprivileged user and its home unconditionally and never clean up, which only works on a runner that discards its environment after each job. The README now states that requirement explicitly instead of implying any sudo-capable Linux runner will do.
Codex AI reviewNo actionable findings. Residual risk remains around custom-runner layouts because the new runner propagation and environment-isolation paths lack dedicated automated coverage. Reviewed commit |
|
The failing Claude review / Generate Claude review check (run 37400320408) is a false positive in What happened
Why
This PR's changes to Suggested fix (separate PR against
|
|
Tracking the |
## Summary The AI PR review, issue triage, Slack notification, and stale-issue closer workflows still run on the shared `ubuntu-latest` pool. Pass the JavaScript testing account's CodeBuild runner label to these four reusable workflow callers, completing the shared CI portion of #937. Pin all four callers to `8ea82196db65b5b0dd67f0b72bb03fa2fa9cf65b`, which is four commits after `288e4317948ca778ebf01acca9342d8b898a1dc2` (the merged commit from aws/aws-durable-execution-ci#67 that introduced the `runs-on` input). The pinned revision retains that input in all four reusable workflows and both nested AI review workflows, and includes subsequent dependency, model-default, and Claude turn-limit fixes. Merged `main` into the branch, preserving its updated stale-issue schedule and OTel workflow. OTel's CodeBuild migration, newer conformance orchestrator pin, and tested Node 22 setup helper are already on `main`; the resulting PR diff contains only the four CI workflow callers. ## Validation - Parsed the four callers at `6f6b7b86` and all six referenced shared workflows at `8ea82196` with duplicate-key detection; verified supplied/required inputs, explicit secrets, and CodeBuild runner propagation through both nested AI review workflows. - `actionlint` 1.7.12 passed for all four modified callers. - Parsed all 16 workflow YAML files and checked for duplicate keys. - Validated supplied inputs, required inputs, explicit secrets, and permissions against the exact pinned reusable workflow contracts, including nested AI review and OTel workflows. - Node 22 setup tests: all three offline fixtures passed. Shell syntax and `git diff --check` passed. - Previous CodeBuild runtime verification: [reusable review run](https://github.com/aws/aws-durable-execution-sdk-js/actions/runs/37542509461) and [model-user scripts running as root](https://github.com/aws/aws-durable-execution-sdk-js/actions/runs/37539805626). These four callers use `main`'s workflow definitions, so their first event after merge exercises the updated callers. No linked issue: follow-up to #937. --------- Co-authored-by: Alex Wang <[email protected]> Co-authored-by: Frank Chen <[email protected]> Co-authored-by: Harry Nguyen <[email protected]>
Summary
Adds an optional
runs-oninput to every shareable workflow so a consuming repository can run the jobs on its own runner, for example the CodeBuild-hosted GitHub Actions runner labelcodebuild-github-actions-runner-${{ github.run_id }}-${{ github.run_attempt }}.workflow_callworkflows (kebab-case, like the existing inputs), defaulting toubuntu-latest.runs-on: ubuntu-latestbecomesruns-on: ${{ inputs['runs-on'] || 'ubuntu-latest' }}. The||fallback matches how the other inputs are read and keeps the directly triggered forms of these workflows (issue_comment,pull_request_target, ...) working, where theinputscontext is empty.ai-pr-review->claude-review/codex-review;ai-pr-review-addressandai-issue-implementation->ai-work-item-resolver/codex-issue-worker) forward the input, so a single value from the caller reaches every job.ai-pr-review-address-continuation.ymlis untouched: it has noworkflow_callsurface and runs only in this repository.Behaviour on merge
No behaviour change for any existing caller. Every consumer (
aws-durable-execution-sdk-js,-python,-java,-rust,-conformance-tests,-docs) pins these workflows by commit SHA, so none of them sees this change until it bumps the pin, and a caller that bumps without settingruns-onstill getsubuntu-latest.Runner requirements for callers
Noted in the README so callers can check their runner before opting in:
claude-review,codex-review,codex-issue-worker,issue-triageandnotifycreate an unprivileged user and drop into it withsudo, so the runner must run jobs as an account with passwordlesssudoand provideadduserandsetfacl. The account's name does not matter (see below).timeout-minutes:codex-issue-worker140 (used byai-pr-review-addressandai-issue-implementation),codex-review45,claude-review30,issue-triage30. A runner whose own build timeout is lower than these will kill the job.Removing the
runneraccount assumptionThe Codex review on this PR pointed out that the model workflows wrote the invoking account's name as the literal
runner, which only holds on GitHub-hosted runners. Measured on the CodeBuild project default (EC2,standard:7.0) in aws/aws-durable-execution-sdk-js#968:runnerrootDefaults:runner env_keepnever matches, sosudo -u <user>drops the AWS credentials and the model cannot reach Bedrockrunneruser/group existschown runner:<user>andinstall -o runnerfail withinvalid user; the setup step exits before the model runs/home/runner/codebuild/output/.../_work/<repo>/<repo>, every ancestor755setfacl ... /home/runnerfails on a missing pathChanges:
env_keeprules are removed altogether (see the next section); nothing is written to/etc/sudoers.dany more.chown/installowner and group use$(id -un)/$(id -gn)(prepare_ai_review_user.sh,prepare_codex_implementation_user.sh,codex-review,codex-issue-worker,issue-triage,notify)./home/runnerexecute-only ACL becomes a top-down walk over the workspace's ancestors that grants--xexactly where the unprivileged user is blocked. On GitHub-hosted runners this reproduces the previous behaviour (/home/runneris the only blocking ancestor); on CodeBuild it is a no-op.tests/test_prepare_codex_implementation_user.pyandtests/test_codex_issue_implementation.pyupdated for the new strings; full suite passes (301 tests).The privilege separation itself is unchanged: the model still runs as a freshly created user that is not in sudoers.
Passing the model environment as an explicit allowlist
The follow-up Codex finding on the first fix was that
Defaults:<user> env_keep += ...only narrows the environment if the runner image's own sudoers still hasenv_resetenabled and no broaderenv_keep. That is the image's choice, not ours, and with a configurableruns-onan image that disables the reset would pass the whole job environment (OIDC request token, CodeBuild credential URI,GH_TOKEN) into the model process.Every model process is now started with
sudo -u <user> -- env -iand an explicit allowlist, the patterncodex-issue-workeralready used:codex-review,issue-triage,notify: the five AWS variables,AWS_EC2_METADATA_DISABLED=true,CODEX_HOME, and the variables sudo's reset used to keep (HOME,PATH,LANG,USER,LOGNAME,SHELL). The preflight that checked whether sudo had preserved the credentials now checks the job environment directly.claude-reviewviascripts/run_claude_isolated.sh: the same AWS set plus the 8ANTHROPIC_*/CLAUDE_CODE_*/DETAILED_PERMISSION_MESSAGESsettings the oldenv_keeprules named; unset optional variables are skipped rather than passed as empty strings./etc/sudoers.dwrites and theirvisudochecks are deleted, as is the sudoers block inprepare_ai_summary_user.sh.On GitHub-hosted runners the model sees the same environment as before (the allowlist is the old
env_keepset plus whatenv_resetkept). Everywhere else it is independent of sudo policy, which is also what keeps the CodeBuild project role out of reach: the aws/aws-durable-execution-sdk-js#968 probe confirmed the unprivileged user inherits noAWS_CONTAINER_*variables and cannot read the root process environment, and withenv -ithat no longer depends on the image.Context
Part of moving Durable Execution SDK CI onto CodeBuild-hosted runners (aws/aws-durable-execution-sdk-js#937, aws/aws-durable-execution-sdk-python#747, aws/aws-durable-execution-sdk-java#751). Those PRs migrate each repository's own workflows; the thin shells that call this repository were left on
ubuntu-latestbecause theirruns-onlives here. Follow-up PRs in the SDK repositories will bump the pin and pass the label. Companion change for the OpenTelemetry conformance orchestrator: aws/aws-durable-execution-conformance-tests#132.Verification
workflow_callworkflows exposeruns-onwith theubuntu-latestdefault.grep ubuntu-latestover the workflows now matches only the input defaults and the||fallbacks;grepforDefaults:runner,-o runner,runner:and/home/runner(outside comments) over scripts and workflows matches nothing.python3 -m unittest discover -s tests: 301 tests, OK (test_serve_aws_credentialsis intermittently flaky on a loopback-socket race, unrelated to this change).bash -non the three edited scripts. Ancestor enumeration checked against both workspace shapes (/home/runner/work/<repo>/<repo>and/codebuild/output/<id>/src/actions-runner/_work/<repo>/<repo>).prepare_ai_review_user.sh,run_claude_isolated.shandenv -ilaunch paths onubuntu-latest(whereid -unisrunnerand the ACL walk reduces to the previous/home/runnergrant). On5123354the Codex Generate and Post jobs passed; Claude started, authenticated to Bedrock and ran until the pre-existing 20-turn cap (the same failure occurs onmain-based runs), so credential delivery is confirmed for both.root, EC2standard:7.0): aws-durable-execution-sdk-js run 37539805626 checks out this branch atd901b2fand runsprepare_ai_review_user.sh(claude-review and codex-review),prepare_ai_summary_user.shandprepare_codex_implementation_user.shas the job account, then mirrors each workflow'senv -ilaunch with a stub binary. All four pass: users are created, the ACL walk adds nothing (every ancestor is755), the review users read the workspace and cannot read/root, the summary user cannot read the workspace, the implementation user can write the worktree but not.git, and the stub receives exactly the allowlisted variables (noGITHUB_*, noAWS_CONTAINER_*, no unlisted job variables).codex-review.ymlfrom this branch withruns-onset to the CodeBuild label. The Generate job ran oncodebuild-github-actions-runner-37542509461-1, createdcodex-review, assumed the Bedrock role through OIDC, launched Codex through theenv -iallowlist, reviewed the pull request on Amazon Bedrock and the Post job published the review. The caller ispush-triggered because theai-pr-review-runtimeenvironment's branch policy cannot match arefs/pull/N/mergeref.