diff --git a/CLAUDE.md b/CLAUDE.md index 405a1e1..6e8b747 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -194,7 +194,9 @@ terraform/state/ | `scripts/sync-members.py` | CI: sync approved heroes from Google Sheets into members.yaml | | `scripts/review-plan.py` | CI: LLM plan review via Bedrock (platform CI; also bundled into the plan-review Lambda) | | `scripts/invoke-plan-review.sh` | CI: app tf-plan → plan-review Lambda | -| `scripts/notify-slack.py` | CI: generic Slack webhook notification | +| `scripts/notify-slack.sh` | CI: generic Slack webhook notification (via `slack_post.py`) | +| `scripts/notify-high-risk.sh` | CI: blocked-apply alert (risk HIGH or review FAILED) with plan key + override link (via `slack_post.py`) | +| `scripts/slack_post.py` | CI: builds escaped, size-limited Block Kit, checks Slack's reply, falls back to plain text (tests: `scripts/tests/`) | | `scripts/invoke-apply-gate.sh` | CI: invoke gate Lambda for apply credentials | | `scripts/run-plan.sh` | CI: terraform plan with exit code handling | | `scripts/upload-plan.sh` | CI: upload plan artifact to S3 | diff --git a/docs/ci-workflow.md b/docs/ci-workflow.md index 8fe8220..9440894 100644 --- a/docs/ci-workflow.md +++ b/docs/ci-workflow.md @@ -23,7 +23,7 @@ Monday 08:00 ──► securityhub-summary Lambda ──► #javabin-infra-ale - The PR comment shows at most 60k chars of the plan (GitHub's comment limit) and says so when it cuts; this does not affect the review. - Structured output via Bedrock Converse tool use: `{risk: LOW|MEDIUM|HIGH, summary: ..., findings: [...]}` - Posts review as PR comment -- HIGH risk on main → sends Slack notification +- HIGH risk (or a FAILED review) on main → Slack alert to #platform-override-alerts with plan key, run ID and override button. `slack_post.py` checks Slack's reply; rejected blocks fall back to plain text, and an undelivered alert shows as a CI error annotation - Outputs: `has_changes`, `plan_key`, `plan_sha256`, `risk_level` ### apply diff --git a/scripts/notify-high-risk.sh b/scripts/notify-high-risk.sh index 2a1379d..41b5c5d 100644 --- a/scripts/notify-high-risk.sh +++ b/scripts/notify-high-risk.sh @@ -1,92 +1,25 @@ #!/bin/sh -# Notify Slack about a HIGH risk plan review with full findings and override link. +# Notify Slack that an apply is blocked (risk HIGH, or the LLM review FAILED), +# with the review findings, plan key and override link. # # Usage: notify-high-risk.sh # -# Reads review-result.json from current directory. -# Uses GitHub Actions env vars for repo context (GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_ACTOR). +# Reads review-result.json from the current directory and repo context from the +# GitHub Actions env (GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_ACTOR, GITHUB_RUN_ID). +# The payload is built and checked by slack_post.py: Slack's reply is verified +# and a rejected block message falls back to plain text. A failed notification +# is reported as a CI error annotation but never fails the job. set -e SSM_PARAM="$1" OVERRIDE_URL="$2" +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) -# Resolve webhook URL from SSM SLACK_WEBHOOK_URL=$(aws ssm get-parameter \ --name "$SSM_PARAM" \ --with-decryption --query Parameter.Value --output text 2>/dev/null || echo "") +export SLACK_WEBHOOK_URL -if [ -z "$SLACK_WEBHOOK_URL" ]; then - echo "No webhook URL available — skipping Slack notification." - exit 0 -fi - -# Extract review data -SUMMARY=$(jq -r '.summary // "Review unavailable"' review-result.json 2>/dev/null || echo "Review unavailable") -RISK=$(jq -r '.risk // "HIGH"' review-result.json 2>/dev/null || echo "HIGH") - -# Build findings blocks as JSON array entries -FINDINGS_TEXT="" -if [ -f review-result.json ]; then - FINDINGS_TEXT=$(python3 -c " -import json, sys -try: - data = json.load(open('review-result.json')) - cat_emoji = {'security': ':lock:', 'cost': ':moneybag:', 'destruction': ':boom:', 'routine': ':white_check_mark:'} - lines = [] - for f in data.get('findings', []): - emoji = cat_emoji.get(f.get('category', ''), ':black_small_square:') - lines.append(f\"{emoji} *[{f.get('category', 'other')}]* {f.get('detail', '')}\") - print('\n'.join(lines)) -except Exception: - pass -" 2>/dev/null || echo "") -fi - -# Repo context from GitHub Actions -REPO="${GITHUB_REPOSITORY:-unknown}" -SHA=$(echo "${GITHUB_SHA:-unknown}" | cut -c1-8) -ACTOR="${GITHUB_ACTOR:-unknown}" -RUN_ID="${GITHUB_RUN_ID:-}" -PLAN_KEY="plans/${REPO}/${RUN_ID}/tfplan" - -# Build source line -if [ -n "$RUN_ID" ] && [ "$REPO" != "unknown" ]; then - SOURCE_LINE=":robot_face: / / (triggered by ${ACTOR})" -else - SOURCE_LINE=":bust_in_silhouette: ${REPO} / \`${SHA}\` / ${ACTOR}" -fi - -# Build the Block Kit payload -FINDINGS_BLOCK="" -if [ -n "$FINDINGS_TEXT" ]; then - ESCAPED_FINDINGS=$(echo "$FINDINGS_TEXT" | python3 -c "import sys,json; print(json.dumps(sys.stdin.read())[1:-1])") - FINDINGS_BLOCK=",{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"*Findings*\n${ESCAPED_FINDINGS}\"}}" -fi - -ESCAPED_SUMMARY=$(echo "$SUMMARY" | python3 -c "import sys,json; print(json.dumps(sys.stdin.read().strip())[1:-1])") -ESCAPED_SOURCE=$(echo "$SOURCE_LINE" | python3 -c "import sys,json; print(json.dumps(sys.stdin.read().strip())[1:-1])") - -PAYLOAD=$(cat < /dev/null -echo "Slack notification sent: HIGH risk plan blocked" +python3 "$SCRIPT_DIR/slack_post.py" risk review-result.json "$OVERRIDE_URL" || + echo "::error::Blocked-apply Slack notification was not delivered (see above)." diff --git a/scripts/notify-slack.sh b/scripts/notify-slack.sh index a5254bf..ed9c7f4 100644 --- a/scripts/notify-slack.sh +++ b/scripts/notify-slack.sh @@ -7,7 +7,9 @@ # 1. SLACK_WEBHOOK_URL env var (if set) # 2. SSM_WEBHOOK_PARAM env var → fetched from AWS SSM # -# If neither is set, the notification is silently skipped. +# If neither is set, the notification is silently skipped. slack_post.py builds +# the payload (escaped, size-limited), checks Slack's reply and falls back to +# plain text if the blocks are rejected. set -e @@ -28,23 +30,7 @@ if [ -z "$SLACK_WEBHOOK_URL" ]; then exit 0 fi -# Build blocks JSON -URL_BLOCK="" -if [ -n "$URL" ]; then - URL_BLOCK=",{\"type\":\"section\",\"text\":{\"type\":\"mrkdwn\",\"text\":\"<${URL}|${URL_LABEL}>\"}}" -fi - -PAYLOAD=$(cat < /dev/null -echo "Slack notification sent: ${TITLE}" +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +export SLACK_WEBHOOK_URL +python3 "$SCRIPT_DIR/slack_post.py" message "$TITLE" "$MESSAGE" "$URL" "$URL_LABEL" || + echo "::error::Slack notification '${TITLE}' was not delivered (see above)." diff --git a/scripts/review-plan.py b/scripts/review-plan.py index c0098c6..bd7c56d 100755 --- a/scripts/review-plan.py +++ b/scripts/review-plan.py @@ -263,11 +263,46 @@ def review_plan(plan_text, converse=cli_converse): for content in response.get("output", {}).get("message", {}).get("content", []): if "toolUse" in content and content["toolUse"]["name"] == "submit_review": - return content["toolUse"]["input"] + return normalize_review(content["toolUse"]["input"]) return None +def _maybe_json(value): + """The model sometimes returns a nested array/object as a JSON string.""" + if isinstance(value, str): + try: + return json.loads(value) + except ValueError: + return value + return value + + +def normalize_review(review): + """Coerce a tool-call answer to the schema, or return None if it can't be. + + Bedrock occasionally serialises `findings` (or one finding) as a JSON string + instead of an array of objects; that used to crash the review and block the + apply with risk FAILED. + """ + review = _maybe_json(review) + if not isinstance(review, dict) or review.get("risk") not in RISK_EMOJI: + return None + findings = _maybe_json(review.get("findings", [])) + if isinstance(findings, dict): + findings = [findings] + if not isinstance(findings, list): + findings = [] + clean = [] + for f in findings: + f = _maybe_json(f) + if isinstance(f, dict) and f.get("detail"): + clean.append({"category": str(f.get("category", "routine")), "detail": str(f["detail"])}) + elif isinstance(f, str) and f.strip(): + clean.append({"category": "routine", "detail": f.strip()}) + return {"risk": review["risk"], "summary": str(review.get("summary", "")), "findings": clean} + + def format_review(review): """Format structured review as markdown for CI output / PR comment.""" risk = review["risk"] @@ -278,7 +313,7 @@ def format_review(review): lines = [f"**Risk: {emoji} {risk}**", "", summary, ""] for f in findings: - cat_emoji = CATEGORY_EMOJI.get(f["category"], "\u2022") + cat_emoji = CATEGORY_EMOJI.get(f.get("category"), "\u2022") lines.append(f"- {cat_emoji} **[{f['category']}]** {f['detail']}") return "\n".join(lines) diff --git a/scripts/slack_post.py b/scripts/slack_post.py new file mode 100644 index 0000000..6e55e4b --- /dev/null +++ b/scripts/slack_post.py @@ -0,0 +1,145 @@ +"""Build and post Slack webhook messages for CI alerts. + +Usage: + slack_post.py message <message> [url] [url_label] + slack_post.py risk <review_result.json> <override_url> + +The webhook URL comes from SLACK_WEBHOOK_URL. Text is JSON-escaped and cut to +Slack's block limits (header 150, section 3000). If Slack rejects the blocks +(e.g. invalid_blocks), the message is re-sent as plain text so the alert is +never lost; a non-"ok" reply on the fallback exits 1. +""" + +import json +import os +import sys +import urllib.request + +HEADER_MAX = 150 +SECTION_MAX = 3000 +TEXT_MAX = 3900 + +CATEGORY_EMOJI = {"security": ":lock:", "cost": ":moneybag:", "destruction": ":boom:", "routine": ":white_check_mark:"} +RISK_EMOJI = {"HIGH": ":red_circle:", "FAILED": ":warning:"} + + +def cut(text, limit): + text = str(text) + return text if len(text) <= limit else text[: limit - 1] + "…" + + +def header(text): + return {"type": "header", "text": {"type": "plain_text", "text": cut(text, HEADER_MAX), "emoji": True}} + + +def section(text): + return {"type": "section", "text": {"type": "mrkdwn", "text": cut(text, SECTION_MAX)}} + + +def message_payload(title, message, url="", url_label="View"): + blocks = [header(title), section(message.replace("\\n", "\n"))] + if url: + blocks.append(section(f"<{url}|{url_label}>")) + return {"text": cut(title, TEXT_MAX), "blocks": blocks} + + +def load_review(path): + try: + with open(path) as f: + data = json.load(f) + return data if isinstance(data, dict) else {} + except (OSError, ValueError): + return {} + + +def risk_payload(review, override_url, env): + risk = review.get("risk") or "FAILED" + if risk not in RISK_EMOJI: + risk = "HIGH" + repo = env.get("GITHUB_REPOSITORY", "unknown") + sha = env.get("GITHUB_SHA", "unknown") + run_id = env.get("GITHUB_RUN_ID", "") + actor = env.get("GITHUB_ACTOR", "unknown") + plan_key = f"plans/{repo}/{run_id}/tfplan" + + if risk == "FAILED": + title = ":rotating_light: Deploy Blocked — Plan Review Failed" + summary = review.get("summary") or "The LLM plan review did not return a result, so the plan is treated as HIGH risk." + footer = ":warning: Review FAILED | Auto-apply blocked — check the plan job log, then override if the plan is safe" + else: + title = ":rotating_light: Deploy Blocked — HIGH Risk Plan" + summary = review.get("summary") or "Review unavailable" + footer = ":red_circle: Risk HIGH | Auto-apply blocked — override required" + + if run_id and repo != "unknown": + source = (f":robot_face: <https://github.com/{repo}|{repo}> / <https://github.com/{repo}/commit/{sha}|{sha[:8]}>" + f" / <https://github.com/{repo}/actions/runs/{run_id}|run> (triggered by {actor})") + else: + source = f":bust_in_silhouette: {repo} / `{sha[:8]}` / {actor}" + + blocks = [header(title), section(source), section(f"*Summary*\n{summary}")] + findings = [f for f in review.get("findings") or [] if isinstance(f, dict)] + if findings: + lines = [f"{CATEGORY_EMOJI.get(f.get('category'), ':black_small_square:')} *[{f.get('category', 'other')}]* {f.get('detail', '')}" + for f in findings] + blocks.append(section("*Findings*\n" + "\n".join(lines))) + blocks += [ + {"type": "section", "fields": [ + {"type": "mrkdwn", "text": f"*Plan key*\n`{plan_key}`"}, + {"type": "mrkdwn", "text": f"*Repository*\n`{repo}`"}, + {"type": "mrkdwn", "text": f"*Run ID*\n`{run_id}`"}, + ]}, + {"type": "actions", "elements": [{"type": "button", "text": {"type": "plain_text", "text": ":unlock: Approve Override", "emoji": True}, + "url": override_url, "style": "danger"}]}, + {"type": "divider"}, + {"type": "context", "elements": [{"type": "mrkdwn", "text": footer}]}, + ] + text = (f"{title}: {repo} run {run_id}\nPlan key: {plan_key}\nRun ID: {run_id}\n" + f"Summary: {summary}\nOverride: {override_url}") + return {"text": cut(text, TEXT_MAX), "blocks": blocks} + + +def post(url, payload, opener=urllib.request.urlopen): + req = urllib.request.Request(url, data=json.dumps(payload).encode(), headers={"Content-Type": "application/json"}) + try: + with opener(req, timeout=15) as resp: + return resp.read().decode() + except urllib.error.HTTPError as e: + return e.read().decode() or f"http {e.code}" + except urllib.error.URLError as e: + return f"error {e.reason}" + + +def send(url, payload, opener=urllib.request.urlopen): + """Post blocks; on rejection fall back to plain text. Returns True if Slack said ok.""" + reply = post(url, payload, opener) + if reply == "ok": + return True + print(f"::warning::Slack rejected the block message ({reply}); sending plain text instead.") + reply = post(url, {"text": payload["text"]}, opener) + if reply == "ok": + return True + print(f"::error::Slack rejected the plain-text message too ({reply}).") + return False + + +def main(argv, env=os.environ): + url = env.get("SLACK_WEBHOOK_URL", "") + if not url: + print("No webhook URL available — skipping Slack notification.") + return 0 + if len(argv) >= 3 and argv[1] == "message": + payload = message_payload(*argv[2:6]) + elif len(argv) == 4 and argv[1] == "risk": + payload = risk_payload(load_review(argv[2]), argv[3], env) + else: + print(__doc__, file=sys.stderr) + return 2 + if not send(url, payload): + return 1 + print(f"Slack notification sent: {payload['text'].splitlines()[0]}") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/scripts/tests/test_review_plan.py b/scripts/tests/test_review_plan.py index aee3247..aae7699 100644 --- a/scripts/tests/test_review_plan.py +++ b/scripts/tests/test_review_plan.py @@ -194,3 +194,37 @@ def test_huge_change_list_is_grouped_and_stays_under_the_limit(self): if __name__ == "__main__": unittest.main() + + +def tool_input_response(tool_input): + return {"output": {"message": {"content": [{"toolUse": {"name": "submit_review", "input": tool_input}}]}}} + + +class NormalizeReviewTest(unittest.TestCase): + def review(self, tool_input): + return rp.review_plan(plan(1), converse=lambda m, t: tool_input_response(tool_input)) + + def test_findings_returned_as_json_string_are_parsed(self): + # The model sometimes serialises the array; this crashed format_review (risk FAILED). + findings = '[{"category": "security", "detail": "boundary change"}]' + review = self.review({"risk": "HIGH", "summary": "s", "findings": findings}) + self.assertEqual(review["findings"], [{"category": "security", "detail": "boundary change"}]) + self.assertIn("boundary change", rp.format_review(review)) + + def test_single_finding_as_json_string_is_parsed(self): + review = self.review({"risk": "LOW", "summary": "s", + "findings": ['{"category": "cost", "detail": "small"}', "plain text finding"]}) + self.assertEqual(review["findings"], [{"category": "cost", "detail": "small"}, + {"category": "routine", "detail": "plain text finding"}]) + + def test_unparseable_findings_are_dropped_not_fatal(self): + review = self.review({"risk": "MEDIUM", "summary": "s", "findings": "not json"}) + self.assertEqual(review["findings"], []) + rp.format_review(review) + + def test_whole_input_as_json_string(self): + review = self.review('{"risk": "LOW", "summary": "s", "findings": []}') + self.assertEqual(review["risk"], "LOW") + + def test_unknown_risk_is_not_trusted(self): + self.assertIsNone(self.review({"risk": "SAFE", "summary": "s", "findings": []})) diff --git a/scripts/tests/test_slack_post.py b/scripts/tests/test_slack_post.py new file mode 100644 index 0000000..f29f06f --- /dev/null +++ b/scripts/tests/test_slack_post.py @@ -0,0 +1,111 @@ +"""Tests for slack_post.py — run: python3 -m unittest discover -s scripts/tests""" + +import importlib.util +import json +import os +import tempfile +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +spec = importlib.util.spec_from_file_location("slack_post", os.path.join(HERE, "..", "slack_post.py")) +sp = importlib.util.module_from_spec(spec) +spec.loader.exec_module(sp) + +ENV = {"GITHUB_REPOSITORY": "javaBin/platform", "GITHUB_SHA": "abcdef1234", "GITHUB_RUN_ID": "42", "GITHUB_ACTOR": "me"} + + +class FakeResponse: + def __init__(self, body): + self.body = body + + def read(self): + return self.body.encode() + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + +def opener_replying(*replies): + sent = [] + + def opener(req, timeout): + sent.append(json.loads(req.data)) + return FakeResponse(replies[len(sent) - 1]) + + return opener, sent + + +def texts(payload): + out = [] + for b in payload["blocks"]: + if "text" in b: + out.append(b["text"]["text"]) + out += [f["text"] for f in b.get("fields", [])] + return out + + +class RiskPayloadTest(unittest.TestCase): + def test_failed_review_is_labelled_failed_not_high(self): + p = sp.risk_payload({"risk": "FAILED"}, "https://x/override", ENV) + self.assertIn("Review Failed", p["blocks"][0]["text"]["text"]) + self.assertIn("plans/javaBin/platform/42/tfplan", "\n".join(texts(p))) + + def test_missing_file_is_treated_as_failed(self): + p = sp.risk_payload(sp.load_review("/nonexistent/review.json"), "https://x", ENV) + self.assertIn("Review Failed", p["blocks"][0]["text"]["text"]) + + def test_long_findings_are_cut_to_slack_section_limit(self): + findings = [{"category": "security", "detail": "x" * 1500} for _ in range(5)] + p = sp.risk_payload({"risk": "HIGH", "summary": "y" * 5000, "findings": findings}, "https://x", ENV) + self.assertTrue(all(len(t) <= sp.SECTION_MAX for t in texts(p))) + self.assertLessEqual(len(p["text"]), sp.TEXT_MAX) + + def test_special_characters_produce_valid_json(self): + p = sp.risk_payload({"risk": "HIGH", "summary": 'a "quote" `tick` \\ <tag> & \n line', + "findings": [{"category": "cost", "detail": 'b"c'}]}, "https://x", ENV) + self.assertEqual(json.loads(json.dumps(p)), p) + + def test_review_file_with_findings_string_does_not_crash(self): + with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f: + json.dump({"risk": "HIGH", "summary": "s", "findings": "oops"}, f) + p = sp.risk_payload(sp.load_review(f.name), "https://x", ENV) + self.assertNotIn("*Findings*", "\n".join(texts(p))) + + +class SendTest(unittest.TestCase): + def test_ok_sends_once(self): + opener, sent = opener_replying("ok") + self.assertTrue(sp.send("https://hook", sp.message_payload("T", "m"), opener)) + self.assertEqual(len(sent), 1) + + def test_rejected_blocks_fall_back_to_plain_text(self): + opener, sent = opener_replying("invalid_blocks", "ok") + self.assertTrue(sp.send("https://hook", sp.risk_payload({"risk": "HIGH"}, "https://x", ENV), opener)) + self.assertEqual(len(sent), 2) + self.assertNotIn("blocks", sent[1]) + self.assertIn("Run ID: 42", sent[1]["text"]) + + def test_both_rejected_reports_failure(self): + opener, _ = opener_replying("invalid_blocks", "channel_is_archived") + self.assertFalse(sp.send("https://hook", sp.message_payload("T", "m"), opener)) + + def test_no_webhook_is_skipped(self): + self.assertEqual(sp.main(["slack_post.py", "message", "T", "m"], env={}), 0) + + +class MessagePayloadTest(unittest.TestCase): + def test_literal_backslash_n_becomes_newline(self): + # approve-override.yml passes "*Repo:* x\n*By:* y" as a literal \n. + p = sp.message_payload("Risk Override Approved", "*Repo:* x\\n*By:* y") + self.assertEqual(p["blocks"][1]["text"]["text"], "*Repo:* x\n*By:* y") + + def test_url_block(self): + p = sp.message_payload("T", "m", "https://u", "Open") + self.assertEqual(p["blocks"][2]["text"]["text"], "<https://u|Open>") + + +if __name__ == "__main__": + unittest.main() diff --git a/terraform/platform/iam/main.tf b/terraform/platform/iam/main.tf index 7ddfd45..a876816 100644 --- a/terraform/platform/iam/main.tf +++ b/terraform/platform/iam/main.tf @@ -755,7 +755,8 @@ resource "aws_iam_role_policy" "ci_deploy_ssm" { # Trust: GitHub OIDC pinned to approve-override.yml workflow on main. # Access control is handled by GitHub: only repo admins/CODEOWNERS can trigger # workflow_dispatch, and the override-approval environment can require reviewers. -# Permissions: ONLY ssm:PutParameter on /javabin/platform-overrides/* +# Permissions: invoke the apply-gate Lambda (it signs the override) and read the +# override-alerts Slack webhook. ################################################################################ resource "aws_iam_role" "ci_override_approver" { @@ -799,11 +800,20 @@ resource "aws_iam_role_policy" "ci_override_approver" { policy = jsonencode({ Version = "2012-10-17" - Statement = [{ - Effect = "Allow" - Action = "lambda:InvokeFunction" - Resource = "arn:aws:lambda:${var.region}:${var.aws_account_id}:function:${var.project}-apply-gate" - }] + Statement = [ + { + Effect = "Allow" + Action = "lambda:InvokeFunction" + Resource = "arn:aws:lambda:${var.region}:${var.aws_account_id}:function:${var.project}-apply-gate" + }, + { + # "Risk Override Approved" Slack message (approve-override.yml) + Sid = "ReadOverrideAlertsWebhook" + Effect = "Allow" + Action = "ssm:GetParameter" + Resource = "arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter/${var.project}/slack/platform-override-alerts-webhook" + }, + ] }) }