Skip to content

Add approval-monitoring alarms + least-privilege emit role#4

Merged
sudsali merged 1 commit into
mainfrom
appsec/approval-monitoring
Jul 23, 2026
Merged

Add approval-monitoring alarms + least-privilege emit role#4
sudsali merged 1 commit into
mainfrom
appsec/approval-monitoring

Conversation

@sudsali

@sudsali sudsali commented Jul 22, 2026

Copy link
Copy Markdown
Owner

What

AppSec asked for approval-specific monitoring beyond the invocation-spike proxy. This adds it, plus the plumbing to emit approval metrics without breaking the two-job security split.

  • Two CloudWatch alarms (ShadowApprovalSpikeAlarm, ShadowHighRiskApprovalAlarm) aggregating the Shadow namespace via Metrics Insights, matching the existing escalation/invocation alarms. High-risk = first-time contributor or large diff.
  • Canonical examples/auto-approve.yml (previously not in the engine repo) with a two-job split: approve (pull-requests, no AWS) and emit (id-token only) — no job holds both GitHub-write and AWS.
  • ShadowMetricsRole: PutMetricData-only, trust pinned to auto-approve.yml, so the approval workflow never assumes the engine's Bedrock/Secrets/S3 role. New MetricsRoleArn output → AWS_METRICS_ROLE_ARN secret.

Also fixes (pre-existing, surfaced while canonicalizing the workflow)

  • Unpaginated listReviews missed the latest review on PRs with >30 reviews.
  • Null review body crashed the gate instead of skipping.
  • Condition-0 didn't check previous_filename, so renaming a guarded file out of .github/ bypassed human review.

Tests

Contract tests pin the two new params' defaults, assert the metrics role is least-privilege (PutMetricData-only, no Bedrock/Secrets/S3, trust pinned to auto-approve.yml), and cross-check workflow-emitted metric names against the alarm SQL. 544 tests pass.

@sudsali
sudsali force-pushed the appsec/approval-monitoring branch from 2ca09d6 to 63061c7 Compare July 22, 2026 20:48
Adds AppSec-requested monitoring for auto-approvals:
- Two CloudWatch alarms (approval-spike, high-risk-approval) aggregating
  the Shadow namespace via Metrics Insights, matching the existing
  escalation/invocation alarm pattern.
- Canonical examples/auto-approve.yml with a two-job split: approve
  (pull-requests, no AWS) and emit (id-token only) so no job holds both
  GitHub-write and AWS. High-risk = first-time contributor or large diff.
  Bot identity (marker/login) is configurable via SHADOW_BOT_NAME /
  SHADOW_BOT_LOGIN vars; a dismissed approval on a SHA is never re-issued;
  approval metrics emit independently so one throttle can't drop the other.
- ShadowMetricsRole: PutMetricData-only, trust pinned to auto-approve.yml,
  so the approval workflow never assumes the engine's Bedrock/Secrets/S3
  role (MetricsRoleArn output -> AWS_METRICS_ROLE_ARN secret).

Also fixes pre-existing bugs surfaced while canonicalizing the workflow:
unpaginated listReviews, null review-body/user crashes, and a Condition-0
rename bypass (previous_filename was not checked).

Trims verbose comments across the engine to WHY-only; no logic change.

Contract tests pin the two new params, the alarm operator/missing-data
semantics, the metrics role's least-privilege, and the emitted metric names.
@sudsali
sudsali force-pushed the appsec/approval-monitoring branch from 63061c7 to b17660c Compare July 23, 2026 18:18
@sudsali
sudsali merged commit af2b2a7 into main Jul 23, 2026
3 checks passed
@sudsali
sudsali deleted the appsec/approval-monitoring branch July 23, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant