Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions .github/workflows/integ.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ name: integ
# Trigger model mirrors deploy.yml: build.yml completes -> workflow_run picks it
# up in the trusted base-repo context (secrets/OIDC available even for fork PRs)
# -> we resolve whether the PR touches cdk/** or agent/** -> an admin approves
# the `integ` environment gate -> deploy/assert/destroy runs against the shared
# account -> a commit status `integ-smoke` is posted back to the PR head so it
# shows up as a (required) check that blocks merge.
# the `deploy` environment gate (reused — same gate and role as deploy.yml) ->
# deploy/assert/destroy runs against the shared account -> a commit status
# `integ-smoke` is posted back to the PR head so it shows up as a (required)
# check that blocks merge.
#
# Local dev path is unchanged: run `mise //cdk:integ` with your own AWS creds.
#
Expand All @@ -19,9 +20,10 @@ name: integ
on:
# zizmor: ignore[dangerous-triggers] — intentional; workflow_run is required so
# fork PRs can run against the shared account (a fork `pull_request` job gets no
# secrets/OIDC). Mitigations: build-success guard, path-filter, `integ`
# environment approval gate (admin reviews fork test code before it runs with
# the privileged role), least-privilege role, status-only tokens per job.
# secrets/OIDC). Mitigations: build-success guard, path-filter, fork
# `safe-to-test` label gate, `deploy` environment approval gate (admin reviews
# fork test code before it runs with the deploy role), status-only tokens per
# job.
workflow_run:
workflows: [build]
types: [completed]
Expand All @@ -41,7 +43,7 @@ jobs:
# docs/cli-only PRs get an immediate green (skipped) status and never deadlock
# the required check.
resolve:
# Manual dispatch is restricted to main (defence in depth — the `integ`
# Manual dispatch is restricted to main (defence in depth — the `deploy`
# environment approval is the primary gate). PR runs come via workflow_run.
if: >-
(github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') ||
Expand Down Expand Up @@ -131,7 +133,7 @@ jobs:
# Fork-PR safety: only run fork-authored code after a maintainer has
# applied the `safe-to-test` label (defence in depth on top of the
# `integ` environment approval). If it's absent, leave the status
# `deploy` environment approval). If it's absent, leave the status
# pending and don't run — re-trigger once the label is added.
if [[ "$WF_HEAD_REPO" != "$REPO" ]]; then
if ! LABELS=$(gh api "repos/$REPO/issues/$PR_NUMBER/labels" --jq '.[].name'); then
Expand Down Expand Up @@ -166,15 +168,16 @@ jobs:
echo "PR #$PR_NUMBER has no cdk/** or agent/** changes — integ skipped (green)."
fi
# The admin-gated deploy -> assert -> destroy. The `integ` environment's
# required reviewer is the approval gate; while it waits, the integ-smoke
# status stays pending and merge stays blocked.
# The admin-gated deploy -> assert -> destroy. The `deploy` environment's
# required reviewers (coding-agents-admin / coding-agents-maintainers) are the
# approval gate; while it waits, the integ-smoke status stays pending and merge
# stays blocked. Reuses deploy.yml's environment/role — no separate integ role.
integ:
needs: resolve
if: needs.resolve.outputs.applicable == 'true'
name: CDK integ smoke (Task API)
runs-on: ubuntu-latest
environment: integ
environment: deploy
timeout-minutes: 45
permissions:
id-token: write
Expand All @@ -186,8 +189,8 @@ jobs:
- name: Checkout PR head (incl. forks)
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
with:
# Approving the `integ` environment authorizes this fork-authored test
# code to run with the privileged role — the approver MUST review
# Approving the `deploy` environment authorizes this fork-authored test
# code to run with the deploy role — the approver MUST review
# cdk/test/integ/** changes before approving.
repository: ${{ needs.resolve.outputs.head_repo }}
ref: ${{ needs.resolve.outputs.head_sha }}
Expand Down
8 changes: 4 additions & 4 deletions docs/decisions/ADR-013-tiered-validation-pyramid.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,16 @@ Progressive build-out:

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

> **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.
> **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.

#### Residual risk acceptance — fork PR integ execution

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.
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.

Mitigations layered against that risk:

- **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.
- **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.
- **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.
- **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.
- **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.

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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,16 @@ Progressive build-out:

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

> **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.
> **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.

#### Residual risk acceptance — fork PR integ execution

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.
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.

Mitigations layered against that risk:

- **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.
- **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.
- **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.
- **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.
- **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.

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.
Expand Down
Loading