Skip to content

Commit b93dcc6

Browse files
authored
fix(integ): gate integ CI on the existing deploy environment (#535)
Switch the integ deploy→assert→destroy job from `environment: integ` to the already-configured `deploy` environment (reused from deploy.yml). The `deploy` environment already has required reviewers (coding-agents-admin / coding-agents-maintainers) and the `GitHub` deploy role's OIDC trust already accepts `environment:deploy`, so integ CI can go live with no account-side IAM change — where the separate `integ` environment needed a new role + trust-policy edit that was never wired up. Tradeoff: integ (including approved fork code) runs with the full deploy role rather than a dedicated least-privilege role. The fork `safe-to-test` label gate, the deploy-environment approval, and forced teardown are the compensating controls; a scoped role remains a later option. ADR-013's residual-risk section is updated to reflect this; comments reconciled. Follow-up to #295 (merged). Refs #236.
1 parent 5b91595 commit b93dcc6

3 files changed

Lines changed: 25 additions & 22 deletions

File tree

.github/workflows/integ.yml

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ name: integ
88
# Trigger model mirrors deploy.yml: build.yml completes -> workflow_run picks it
99
# up in the trusted base-repo context (secrets/OIDC available even for fork PRs)
1010
# -> we resolve whether the PR touches cdk/** or agent/** -> an admin approves
11-
# the `integ` environment gate -> deploy/assert/destroy runs against the shared
12-
# account -> a commit status `integ-smoke` is posted back to the PR head so it
13-
# shows up as a (required) check that blocks merge.
11+
# the `deploy` environment gate (reused — same gate and role as deploy.yml) ->
12+
# deploy/assert/destroy runs against the shared account -> a commit status
13+
# `integ-smoke` is posted back to the PR head so it shows up as a (required)
14+
# check that blocks merge.
1415
#
1516
# Local dev path is unchanged: run `mise //cdk:integ` with your own AWS creds.
1617
#
@@ -19,9 +20,10 @@ name: integ
1920
on:
2021
# zizmor: ignore[dangerous-triggers] — intentional; workflow_run is required so
2122
# fork PRs can run against the shared account (a fork `pull_request` job gets no
22-
# secrets/OIDC). Mitigations: build-success guard, path-filter, `integ`
23-
# environment approval gate (admin reviews fork test code before it runs with
24-
# the privileged role), least-privilege role, status-only tokens per job.
23+
# secrets/OIDC). Mitigations: build-success guard, path-filter, fork
24+
# `safe-to-test` label gate, `deploy` environment approval gate (admin reviews
25+
# fork test code before it runs with the deploy role), status-only tokens per
26+
# job.
2527
workflow_run:
2628
workflows: [build]
2729
types: [completed]
@@ -41,7 +43,7 @@ jobs:
4143
# docs/cli-only PRs get an immediate green (skipped) status and never deadlock
4244
# the required check.
4345
resolve:
44-
# Manual dispatch is restricted to main (defence in depth — the `integ`
46+
# Manual dispatch is restricted to main (defence in depth — the `deploy`
4547
# environment approval is the primary gate). PR runs come via workflow_run.
4648
if: >-
4749
(github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') ||
@@ -131,7 +133,7 @@ jobs:
131133
132134
# Fork-PR safety: only run fork-authored code after a maintainer has
133135
# applied the `safe-to-test` label (defence in depth on top of the
134-
# `integ` environment approval). If it's absent, leave the status
136+
# `deploy` environment approval). If it's absent, leave the status
135137
# pending and don't run — re-trigger once the label is added.
136138
if [[ "$WF_HEAD_REPO" != "$REPO" ]]; then
137139
if ! LABELS=$(gh api "repos/$REPO/issues/$PR_NUMBER/labels" --jq '.[].name'); then
@@ -166,15 +168,16 @@ jobs:
166168
echo "PR #$PR_NUMBER has no cdk/** or agent/** changes — integ skipped (green)."
167169
fi
168170
169-
# The admin-gated deploy -> assert -> destroy. The `integ` environment's
170-
# required reviewer is the approval gate; while it waits, the integ-smoke
171-
# status stays pending and merge stays blocked.
171+
# The admin-gated deploy -> assert -> destroy. The `deploy` environment's
172+
# required reviewers (coding-agents-admin / coding-agents-maintainers) are the
173+
# approval gate; while it waits, the integ-smoke status stays pending and merge
174+
# stays blocked. Reuses deploy.yml's environment/role — no separate integ role.
172175
integ:
173176
needs: resolve
174177
if: needs.resolve.outputs.applicable == 'true'
175178
name: CDK integ smoke (Task API)
176179
runs-on: ubuntu-latest
177-
environment: integ
180+
environment: deploy
178181
timeout-minutes: 45
179182
permissions:
180183
id-token: write
@@ -186,8 +189,8 @@ jobs:
186189
- name: Checkout PR head (incl. forks)
187190
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
188191
with:
189-
# Approving the `integ` environment authorizes this fork-authored test
190-
# code to run with the privileged role — the approver MUST review
192+
# Approving the `deploy` environment authorizes this fork-authored test
193+
# code to run with the deploy role — the approver MUST review
191194
# cdk/test/integ/** changes before approving.
192195
repository: ${{ needs.resolve.outputs.head_repo }}
193196
ref: ${{ needs.resolve.outputs.head_sha }}

docs/decisions/ADR-013-tiered-validation-pyramid.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,16 @@ Progressive build-out:
129129

130130
Status: **Implemented** (GitHub Actions). This remains the authoritative gate for merge.
131131

132-
> **Deployed runtime E2E — Phase 0 landed (issue #236).** `@aws-cdk/integ-tests-alpha` + `integ-runner` provide deploy-then-verify coverage: `mise //cdk:integ` deploys a trimmed Task API stack to a real account, asserts the create-and-persist happy path (task persists at `SUBMITTED`), then tears it down. In CI (`.github/workflows/integ.yml`) it is triggered per-PR via `workflow_run` only when the diff touches `cdk/**` or `agent/**` (docs/cli-only PRs get an immediate green skip), runs behind the `integ` environment's admin-approval gate so it never assumes the privileged role unattended, and posts a required `integ-smoke` status that blocks merge. Because it is path-filtered and admin-gated it does not slow non-infra PRs. `workflow_dispatch` is retained for manual runs against `main`. Phase 1 (full lifecycle / real agent runs) and Phase 2 (channels) are follow-ups.
132+
> **Deployed runtime E2E — Phase 0 landed (issue #236).** `@aws-cdk/integ-tests-alpha` + `integ-runner` provide deploy-then-verify coverage: `mise //cdk:integ` deploys a trimmed Task API stack to a real account, asserts the create-and-persist happy path (task persists at `SUBMITTED`), then tears it down. In CI (`.github/workflows/integ.yml`) it is triggered per-PR via `workflow_run` only when the diff touches `cdk/**` or `agent/**` (docs/cli-only PRs get an immediate green skip), runs behind the reused `deploy` environment's admin-approval gate so it never assumes the deploy role unattended, and posts a required `integ-smoke` status that blocks merge. Because it is path-filtered and admin-gated it does not slow non-infra PRs. `workflow_dispatch` is retained for manual runs against `main`. Phase 1 (full lifecycle / real agent runs) and Phase 2 (channels) are follow-ups.
133133
134134
#### Residual risk acceptance — fork PR integ execution
135135

136-
The deployed E2E gate runs **fork-authored test code** (`cdk/test/integ/**`) against the shared AWS account. This is intentional: a fork `pull_request` job gets no secrets/OIDC, so the `workflow_run` trigger is the only way to give fork PRs the same deploy-verify coverage that branch PRs get. The accepted risk is that fork code executes with a role capable of CloudFormation operations in the shared account.
136+
The deployed E2E gate runs **fork-authored test code** (`cdk/test/integ/**`) against the shared AWS account. This is intentional: a fork `pull_request` job gets no secrets/OIDC, so the `workflow_run` trigger is the only way to give fork PRs the same deploy-verify coverage that branch PRs get. The accepted risk is that fork code executes with the shared `GitHub` deploy role (reused from `deploy.yml`), which is capable of CloudFormation operations in the shared account.
137137

138138
Mitigations layered against that risk:
139139

140-
- **Two human gates before fork code runs.** The `resolve` job requires a maintainer-applied `safe-to-test` label on fork PRs, and the `integ` job requires explicit approval of the `integ` GitHub environment. The approver MUST review `cdk/test/integ/**` changes before approving.
141-
- **Least-privilege role is a hard go-live prerequisite.** The privileged role (`AWS_ROLE_TO_ASSUME`) MUST be the dedicated `GitHubIntegE2E` role scoped per `docs/integ-e2e-role.policy.json`it can only assume the `cdk-hnb659fds-*` bootstrap roles and drive CloudFormation on the `backgroundagent-integ` (+ `TaskApiSmokeDefaultTestDeployAssert*`) stacks. A broad/admin role here would void this acceptance. Provisioning the role in the account is an account-side (Isengard) task tracked outside this repo.
140+
- **Two human gates before fork code runs.** The `resolve` job requires a maintainer-applied `safe-to-test` label on fork PRs, and the `integ` job requires explicit approval of the `deploy` GitHub environment (required reviewers: `coding-agents-admin` / `coding-agents-maintainers`, self-review prevented). The approver MUST review `cdk/test/integ/**` changes before approving.
141+
- **Environment reuse over a dedicated role.** Phase 0 reuses `deploy.yml`'s existing `deploy` environment and `GitHub` deploy role rather than provisioning a separate least-privilege role — the environment and its OIDC trust are already configured, so no account-side IAM change is needed to go live. The tradeoff is that integ (including approved fork code) runs with the full deploy role; the human gates above and forced teardown below are the compensating controls. Scoping a dedicated least-privilege integ role remains an option a maintainer can adopt later if the blast radius is judged too wide.
142142
- **Status-only tokens and forced teardown.** Each job gets minimal `permissions`; the report path only posts commit statuses, and teardown runs `if: always()` with a fail-loud `wait` so a stranded stack surfaces rather than leaking billable resources.
143143

144144
Residual risk after these mitigations (a malicious fork PR that passes maintainer review and operates strictly within the scoped stacks) is **accepted** for Phase 0.

docs/src/content/docs/decisions/Adr-013-tiered-validation-pyramid.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,16 @@ Progressive build-out:
133133

134134
Status: **Implemented** (GitHub Actions). This remains the authoritative gate for merge.
135135

136-
> **Deployed runtime E2E — Phase 0 landed (issue #236).** `@aws-cdk/integ-tests-alpha` + `integ-runner` provide deploy-then-verify coverage: `mise //cdk:integ` deploys a trimmed Task API stack to a real account, asserts the create-and-persist happy path (task persists at `SUBMITTED`), then tears it down. In CI (`.github/workflows/integ.yml`) it is triggered per-PR via `workflow_run` only when the diff touches `cdk/**` or `agent/**` (docs/cli-only PRs get an immediate green skip), runs behind the `integ` environment's admin-approval gate so it never assumes the privileged role unattended, and posts a required `integ-smoke` status that blocks merge. Because it is path-filtered and admin-gated it does not slow non-infra PRs. `workflow_dispatch` is retained for manual runs against `main`. Phase 1 (full lifecycle / real agent runs) and Phase 2 (channels) are follow-ups.
136+
> **Deployed runtime E2E — Phase 0 landed (issue #236).** `@aws-cdk/integ-tests-alpha` + `integ-runner` provide deploy-then-verify coverage: `mise //cdk:integ` deploys a trimmed Task API stack to a real account, asserts the create-and-persist happy path (task persists at `SUBMITTED`), then tears it down. In CI (`.github/workflows/integ.yml`) it is triggered per-PR via `workflow_run` only when the diff touches `cdk/**` or `agent/**` (docs/cli-only PRs get an immediate green skip), runs behind the reused `deploy` environment's admin-approval gate so it never assumes the deploy role unattended, and posts a required `integ-smoke` status that blocks merge. Because it is path-filtered and admin-gated it does not slow non-infra PRs. `workflow_dispatch` is retained for manual runs against `main`. Phase 1 (full lifecycle / real agent runs) and Phase 2 (channels) are follow-ups.
137137
138138
#### Residual risk acceptance — fork PR integ execution
139139

140-
The deployed E2E gate runs **fork-authored test code** (`cdk/test/integ/**`) against the shared AWS account. This is intentional: a fork `pull_request` job gets no secrets/OIDC, so the `workflow_run` trigger is the only way to give fork PRs the same deploy-verify coverage that branch PRs get. The accepted risk is that fork code executes with a role capable of CloudFormation operations in the shared account.
140+
The deployed E2E gate runs **fork-authored test code** (`cdk/test/integ/**`) against the shared AWS account. This is intentional: a fork `pull_request` job gets no secrets/OIDC, so the `workflow_run` trigger is the only way to give fork PRs the same deploy-verify coverage that branch PRs get. The accepted risk is that fork code executes with the shared `GitHub` deploy role (reused from `deploy.yml`), which is capable of CloudFormation operations in the shared account.
141141

142142
Mitigations layered against that risk:
143143

144-
- **Two human gates before fork code runs.** The `resolve` job requires a maintainer-applied `safe-to-test` label on fork PRs, and the `integ` job requires explicit approval of the `integ` GitHub environment. The approver MUST review `cdk/test/integ/**` changes before approving.
145-
- **Least-privilege role is a hard go-live prerequisite.** The privileged role (`AWS_ROLE_TO_ASSUME`) MUST be the dedicated `GitHubIntegE2E` role scoped per `docs/integ-e2e-role.policy.json`it can only assume the `cdk-hnb659fds-*` bootstrap roles and drive CloudFormation on the `backgroundagent-integ` (+ `TaskApiSmokeDefaultTestDeployAssert*`) stacks. A broad/admin role here would void this acceptance. Provisioning the role in the account is an account-side (Isengard) task tracked outside this repo.
144+
- **Two human gates before fork code runs.** The `resolve` job requires a maintainer-applied `safe-to-test` label on fork PRs, and the `integ` job requires explicit approval of the `deploy` GitHub environment (required reviewers: `coding-agents-admin` / `coding-agents-maintainers`, self-review prevented). The approver MUST review `cdk/test/integ/**` changes before approving.
145+
- **Environment reuse over a dedicated role.** Phase 0 reuses `deploy.yml`'s existing `deploy` environment and `GitHub` deploy role rather than provisioning a separate least-privilege role — the environment and its OIDC trust are already configured, so no account-side IAM change is needed to go live. The tradeoff is that integ (including approved fork code) runs with the full deploy role; the human gates above and forced teardown below are the compensating controls. Scoping a dedicated least-privilege integ role remains an option a maintainer can adopt later if the blast radius is judged too wide.
146146
- **Status-only tokens and forced teardown.** Each job gets minimal `permissions`; the report path only posts commit statuses, and teardown runs `if: always()` with a fail-loud `wait` so a stranded stack surfaces rather than leaking billable resources.
147147

148148
Residual risk after these mitigations (a malicious fork PR that passes maintainer review and operates strictly within the scoped stacks) is **accepted** for Phase 0.

0 commit comments

Comments
 (0)