From fb3a181e0cb1d524eb9adf389636ab5d75fd6984 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:48:09 +0000 Subject: [PATCH] feat(api): add flask sentry sdk (#205) Initialize the SDK on gunicorn and the SQS worker with afterhours-style scrubbing. Store the DSN in SSM and inject only the parameter name onto the live task. --- .github/workflows/deploy-api.yaml | 1 + requirements-api.txt | 1 + src/server/app.py | 1 + src/server/sentry_init.py | 177 ++++++++++++++++++++++++ src/server/worker.py | 4 + terraform/ecs.tf | 1 + terraform/ssm.tf | 11 ++ tests/conftest.py | 13 ++ tests/test_sentry_init.py | 220 ++++++++++++++++++++++++++++++ tests/test_terraform_sentry.py | 39 ++++++ tests/test_worker.py | 17 +++ 11 files changed, 485 insertions(+) create mode 100644 src/server/sentry_init.py create mode 100644 tests/test_sentry_init.py create mode 100644 tests/test_terraform_sentry.py diff --git a/.github/workflows/deploy-api.yaml b/.github/workflows/deploy-api.yaml index cb5d63c..cc30166 100644 --- a/.github/workflows/deploy-api.yaml +++ b/.github/workflows/deploy-api.yaml @@ -206,6 +206,7 @@ jobs: container["image"] = image env = {item["name"]: item["value"] for item in container.get("environment", [])} env["GIT_SHA"] = sha + env["SENTRY_DSN_PARAM"] = "/meal-order-manager/sentry-dsn" container["environment"] = [{"name": key, "value": value} for key, value in env.items()] container.pop("command", None) json.dump(td, sys.stdout) diff --git a/requirements-api.txt b/requirements-api.txt index 1b0b2dc..275ea27 100644 --- a/requirements-api.txt +++ b/requirements-api.txt @@ -1,6 +1,7 @@ flask==3.1.3 gunicorn==23.0.0 boto3==1.43.97 +sentry-sdk==2.69.2 jinja2==3.1.6 fpdf2==2.8.8 PyJWT[crypto]==2.14.0 diff --git a/src/server/app.py b/src/server/app.py index 6148b86..1958e5b 100644 --- a/src/server/app.py +++ b/src/server/app.py @@ -13,6 +13,7 @@ import json from flask import Flask, Response, jsonify, request, send_file +import server.sentry_init # noqa: F401 from server import http_api CORS_ORIGINS = [ diff --git a/src/server/sentry_init.py b/src/server/sentry_init.py new file mode 100644 index 0000000..8f99bb2 --- /dev/null +++ b/src/server/sentry_init.py @@ -0,0 +1,177 @@ +"""Flask Sentry SDK init for meal-order-manager. + +Imported for side effect from ``server.app``. ``init_sentry()`` is a no-op when +the DSN is unset, empty, or the literal ``unset``, so pytest and local +``python -m server.app`` never talk to Sentry. When ``SENTRY_DSN`` is absent, +the DSN is read from SSM via ``SENTRY_DSN_PARAM``. ``ParameterNotFound`` is +treated as unset so a mixed-PR race cannot crash gunicorn. +``before_send`` strips auth, cookies, the publish HMAC header, request bodies, +secrety extras, and exception stack-frame locals. +``include_local_variables=False`` keeps those locals out of the event in the +first place. +""" + +from __future__ import annotations + +import os + +import sentry_sdk +from botocore.exceptions import ClientError +from sentry_sdk.integrations.flask import FlaskIntegration + +_HEADER_DROP_NAMES = frozenset( + { + "authorization", + "x-auth-token", + "cookie", + "x-amz-security-token", + "x-slack-signature", + "x-meals-publish-key", + } +) +_DROP_REQUEST_KEYS = frozenset( + { + "body", + "Body", + "data", + "cookies", + "raw_email", + "prompt", + "secret", + "SecretString", + "hmac", + "keys", + } +) +_DROP_EXTRA_NEEDLES = ( + "body", + "email", + "prompt", + "secret", + "hmac", + "token", + "mime", + "raw_email", + "password", + "signing", +) + + +def _drop_header(name): + lower = str(name).lower() + return lower in _HEADER_DROP_NAMES or lower.startswith("x-amz-") + + +def _scrub_headers(headers): + if isinstance(headers, dict): + return {k: v for k, v in headers.items() if not _drop_header(k)} + if isinstance(headers, list): + kept = [] + for pair in headers: + if isinstance(pair, (list, tuple)) and pair and _drop_header(pair[0]): + continue + kept.append(pair) + return kept + return headers + + +def _stacktraces(event): + traces = [] + stacktrace = event.get("stacktrace") + if isinstance(stacktrace, dict): + traces.append(stacktrace) + for section in ("exception", "threads"): + container = event.get(section) + if not isinstance(container, dict): + continue + values = container.get("values") + if not isinstance(values, list): + continue + for item in values: + if not isinstance(item, dict): + continue + inner = item.get("stacktrace") + if isinstance(inner, dict): + traces.append(inner) + return traces + + +def _strip_stack_locals(event): + """Drop frame locals. Names like ``raw``/``item`` still hold secrets.""" + for stacktrace in _stacktraces(event): + frames = stacktrace.get("frames") + if not isinstance(frames, list): + continue + for frame in frames: + if isinstance(frame, dict): + frame.pop("vars", None) + + +def _before_send(event, _hint): + request = event.get("request") + if isinstance(request, dict): + headers = request.get("headers") + if headers is not None: + request["headers"] = _scrub_headers(headers) + for key in list(request): + if key in _DROP_REQUEST_KEYS or str(key).lower() in {"body", "data"}: + request.pop(key, None) + extra = event.get("extra") + if isinstance(extra, dict): + for key in list(extra): + lower = str(key).lower() + if any(needle in lower for needle in _DROP_EXTRA_NEEDLES): + extra.pop(key, None) + _strip_stack_locals(event) + return event + + +def _is_parameter_not_found(exc: Exception) -> bool: + response_data = getattr(exc, "response", {}) + if not isinstance(response_data, dict): + return False + return response_data.get("Error", {}).get("Code") == "ParameterNotFound" + + +def _dsn_from_param() -> str: + param = os.environ.get("SENTRY_DSN_PARAM", "").strip() + if not param: + return "" + from shared.secrets import get_parameter + + try: + return get_parameter(param, decrypt=True).strip() + except ClientError as exc: + if _is_parameter_not_found(exc): + return "" + raise + + +def _resolve_dsn() -> str: + dsn = os.environ.get("SENTRY_DSN", "").strip() + if dsn: + return dsn + return _dsn_from_param() + + +def init_sentry() -> None: + dsn = _resolve_dsn() + if not dsn or dsn.lower() == "unset": + return + kwargs = { + "dsn": dsn, + "integrations": [FlaskIntegration()], + "send_default_pii": False, + "include_local_variables": False, + "enable_logs": False, + "traces_sample_rate": 0.0, + "before_send": _before_send, + "environment": os.environ.get("STAGE", "").strip() or "local", + } + sha = os.environ.get("GIT_SHA", "").strip() + if sha: + kwargs["release"] = sha + sentry_sdk.init(**kwargs) + + +init_sentry() diff --git a/src/server/worker.py b/src/server/worker.py index 524e297..bcce830 100644 --- a/src/server/worker.py +++ b/src/server/worker.py @@ -10,8 +10,10 @@ import sys import time import boto3 +import sentry_sdk from server.jobs import run_job +from server.sentry_init import init_sentry logger = logging.getLogger(__name__) logging.basicConfig(level=logging.INFO, stream=sys.stderr) @@ -27,6 +29,7 @@ def _stop(_signum, _frame) -> None: def main() -> None: signal.signal(signal.SIGTERM, _stop) signal.signal(signal.SIGINT, _stop) + init_sentry() queue_url = os.environ.get("JOBS_QUEUE_URL", "").strip() if not queue_url: @@ -57,6 +60,7 @@ def main() -> None: continue sqs.delete_message(QueueUrl=queue_url, ReceiptHandle=receipt) except Exception: + sentry_sdk.capture_exception() logger.exception("job failed; leaving message for retry") diff --git a/terraform/ecs.tf b/terraform/ecs.tf index 7f6018a..80b3b2d 100644 --- a/terraform/ecs.tf +++ b/terraform/ecs.tf @@ -150,6 +150,7 @@ locals { { name = "PORTAL_COGNITO_AUDIENCE_PARAM", value = aws_ssm_parameter.portal_cognito_audience.name }, { name = "PORTAL_COGNITO_TRUST_PARAM", value = aws_ssm_parameter.portal_cognito_trust.name }, { name = "PUBLISH_KEY_PARAM", value = aws_ssm_parameter.publish_key.name }, + { name = "SENTRY_DSN_PARAM", value = aws_ssm_parameter.sentry_dsn.name }, { name = "JOBS_QUEUE_URL", value = aws_sqs_queue.jobs.id }, { name = "CHECKCOMPONENTS_QUEUE_URL", value = var.checkcomponents_queue_url }, { name = "AWS_DEFAULT_REGION", value = var.aws_region }, diff --git a/terraform/ssm.tf b/terraform/ssm.tf index da030d6..307b3e7 100644 --- a/terraform/ssm.tf +++ b/terraform/ssm.tf @@ -4,6 +4,17 @@ # created and rotated out-of-band because it varies per environment; data.tf # reads it. Do not turn that lookup into a resource. +resource "aws_ssm_parameter" "sentry_dsn" { + name = "${local.ssm_prefix}/sentry-dsn" + type = "SecureString" + value = "unset" + description = "Sentry DSN for meal-order-manager. PutParameter writes the live value; Terraform ignores it. Empty or unset disables the SDK." + + lifecycle { + ignore_changes = [value] + } +} + resource "aws_ssm_parameter" "slack_channel_id" { name = local.slack_channel_param type = "String" diff --git a/tests/conftest.py b/tests/conftest.py index 4cfadd6..9718725 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,12 +3,16 @@ import os import sys +import pytest + # Several Lambda handlers construct boto3.client(...) at module load. On a CI # runner with no AWS config this raises NoRegionError during test collection # (a real region is read from ~/.aws/config locally, masking it). Set a default # region before any import. Client construction is offline; real calls are mocked. os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1") os.environ.setdefault("AWS_REGION", "us-east-1") +os.environ.pop("SENTRY_DSN", None) +os.environ.pop("SENTRY_DSN_PARAM", None) # Add the repo root so `import functions..handler` resolves under a bare # `pytest` invocation. `python -m pytest` injects the CWD automatically, but CI @@ -25,3 +29,12 @@ sys.path.insert(0, os.path.abspath(_src_dir)) # without requiring a real Lambda layer or .aws-sam build. _shared_layer_dir = os.path.join(_src_dir, "shared") sys.path.insert(0, os.path.abspath(_shared_layer_dir)) + + +@pytest.fixture(autouse=True) +def _clear_sentry_dsn_env(): + os.environ.pop("SENTRY_DSN", None) + os.environ.pop("SENTRY_DSN_PARAM", None) + yield + os.environ.pop("SENTRY_DSN", None) + os.environ.pop("SENTRY_DSN_PARAM", None) diff --git a/tests/test_sentry_init.py b/tests/test_sentry_init.py new file mode 100644 index 0000000..e001c6a --- /dev/null +++ b/tests/test_sentry_init.py @@ -0,0 +1,220 @@ +"""sentry_init: DSN no-op, FlaskIntegration, and before_send scrub.""" + +from unittest.mock import patch + +from botocore.exceptions import ClientError +from sentry_sdk.integrations.flask import FlaskIntegration + +import server.sentry_init as sentry_mod + +_FAKE_DSN = "https://key@o1.ingest.sentry.io/1" + + +def _parameter_not_found(): + return ClientError( + {"Error": {"Code": "ParameterNotFound", "Message": "not found"}}, + "GetParameter", + ) + + +def test_unset_dsn_does_not_init(): + with ( + patch.dict("os.environ", {}, clear=False), + patch("sentry_sdk.init") as mocked, + ): + # Ensure both sources are absent even if a prior test set them. + import os + + os.environ.pop("SENTRY_DSN", None) + os.environ.pop("SENTRY_DSN_PARAM", None) + sentry_mod.init_sentry() + mocked.assert_not_called() + + +def test_empty_dsn_does_not_init(monkeypatch): + monkeypatch.setenv("SENTRY_DSN", "") + monkeypatch.delenv("SENTRY_DSN_PARAM", raising=False) + with patch("sentry_sdk.init") as mocked: + sentry_mod.init_sentry() + mocked.assert_not_called() + + +def test_literal_unset_dsn_does_not_init(monkeypatch): + monkeypatch.setenv("SENTRY_DSN", "unset") + with patch("sentry_sdk.init") as mocked: + sentry_mod.init_sentry() + mocked.assert_not_called() + + +def test_set_dsn_inits_flask_integration(monkeypatch): + monkeypatch.setenv("SENTRY_DSN", _FAKE_DSN) + monkeypatch.setenv("STAGE", "dev") + monkeypatch.setenv("GIT_SHA", "abc123def") + with patch("sentry_sdk.init") as mocked: + sentry_mod.init_sentry() + mocked.assert_called_once() + kwargs = mocked.call_args.kwargs + assert kwargs["dsn"] == _FAKE_DSN + assert kwargs["send_default_pii"] is False + assert kwargs["include_local_variables"] is False + assert kwargs["enable_logs"] is False + assert kwargs["traces_sample_rate"] == 0.0 + assert kwargs["before_send"] is sentry_mod._before_send + assert kwargs["environment"] == "dev" + assert kwargs["release"] == "abc123def" + integrations = kwargs["integrations"] + assert len(integrations) == 1 + assert isinstance(integrations[0], FlaskIntegration) + + +def test_missing_stage_defaults_environment_to_local(monkeypatch): + monkeypatch.setenv("SENTRY_DSN", _FAKE_DSN) + monkeypatch.delenv("STAGE", raising=False) + monkeypatch.delenv("GIT_SHA", raising=False) + with patch("sentry_sdk.init") as mocked: + sentry_mod.init_sentry() + kwargs = mocked.call_args.kwargs + assert kwargs["environment"] == "local" + assert "release" not in kwargs + + +def test_sentry_dsn_param_fetches_from_ssm(monkeypatch): + monkeypatch.delenv("SENTRY_DSN", raising=False) + monkeypatch.setenv("SENTRY_DSN_PARAM", "/meal-order-manager/sentry-dsn") + monkeypatch.setenv("STAGE", "dev") + monkeypatch.setenv("GIT_SHA", "deadbeef") + with ( + patch("shared.secrets.get_parameter", return_value=_FAKE_DSN) as mock_get, + patch("sentry_sdk.init") as mocked, + ): + sentry_mod.init_sentry() + mock_get.assert_called_once_with("/meal-order-manager/sentry-dsn", decrypt=True) + mocked.assert_called_once() + assert mocked.call_args.kwargs["dsn"] == _FAKE_DSN + assert isinstance(mocked.call_args.kwargs["integrations"][0], FlaskIntegration) + + +def test_sentry_dsn_param_unset_value_does_not_init(monkeypatch): + monkeypatch.delenv("SENTRY_DSN", raising=False) + monkeypatch.setenv("SENTRY_DSN_PARAM", "/meal-order-manager/sentry-dsn") + with ( + patch("shared.secrets.get_parameter", return_value="unset"), + patch("sentry_sdk.init") as mocked, + ): + sentry_mod.init_sentry() + mocked.assert_not_called() + + +def test_sentry_dsn_param_not_found_does_not_init(monkeypatch): + monkeypatch.delenv("SENTRY_DSN", raising=False) + monkeypatch.setenv("SENTRY_DSN_PARAM", "/meal-order-manager/sentry-dsn") + with ( + patch("shared.secrets.get_parameter", side_effect=_parameter_not_found()), + patch("sentry_sdk.init") as mocked, + ): + sentry_mod.init_sentry() + mocked.assert_not_called() + + +def test_before_send_strips_auth_and_publish_key_headers(): + event = { + "request": { + "headers": { + "Authorization": "Bearer secret", + "X-Meals-Publish-Key": "hmac-secret", + "X-Auth-Token": "tok", + "Cookie": "session=abc", + "X-Amz-Date": "20260101T000000Z", + "Content-Type": "application/json", + }, + "url": "https://example.invalid/api/submit-order", + } + } + out = sentry_mod._before_send(event, {}) + assert out["request"]["headers"] == {"Content-Type": "application/json"} + assert out["request"]["url"] == "https://example.invalid/api/submit-order" + + +def test_before_send_strips_list_headers(): + event = { + "request": { + "headers": [ + ("Authorization", "Bearer secret"), + ("X-Meals-Publish-Key", "hmac-secret"), + ("Content-Type", "application/json"), + ] + } + } + out = sentry_mod._before_send(event, {}) + assert out["request"]["headers"] == [("Content-Type", "application/json")] + + +def test_before_send_drops_body_and_secret_keys(): + event = { + "request": { + "body": '{"google_id_token":"ya29.secret"}', + "data": {"google_id_token": "ya29.secret"}, + "method": "POST", + }, + "extra": { + "google_id_token": "ya29.secret", + "publish_hmac": "aabbcc", + "bot_token": "xoxb-secret", + "week": "2026-W38", + }, + } + out = sentry_mod._before_send(event, {}) + assert "body" not in out["request"] + assert "data" not in out["request"] + assert out["request"]["method"] == "POST" + assert "google_id_token" not in out["extra"] + assert "publish_hmac" not in out["extra"] + assert "bot_token" not in out["extra"] + assert out["extra"]["week"] == "2026-W38" + + +def test_before_send_drops_exception_and_thread_frame_locals(): + event = { + "exception": { + "values": [ + { + "stacktrace": { + "frames": [ + { + "function": "handler", + "vars": { + "google_id_token": "ya29.secret", + "SecretString": "aabbcc", + }, + } + ] + } + } + ] + }, + "threads": { + "values": [ + { + "stacktrace": { + "frames": [ + { + "function": "_require_publish_key", + "vars": {"provided": "hmac-secret"}, + } + ] + } + } + ] + }, + "stacktrace": { + "frames": [{"function": "get_secret", "vars": {"item": {"token": "x"}}}] + }, + } + out = sentry_mod._before_send(event, {}) + assert "vars" not in out["exception"]["values"][0]["stacktrace"]["frames"][0] + assert "vars" not in out["threads"]["values"][0]["stacktrace"]["frames"][0] + assert "vars" not in out["stacktrace"]["frames"][0] + assert ( + out["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] + == "handler" + ) diff --git a/tests/test_terraform_sentry.py b/tests/test_terraform_sentry.py new file mode 100644 index 0000000..e779f61 --- /dev/null +++ b/tests/test_terraform_sentry.py @@ -0,0 +1,39 @@ +"""Sentry DSN is an SSM SecureString; the task receives the parameter name.""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +TERRAFORM = ROOT / "terraform" + + +def _read(name: str) -> str: + return (TERRAFORM / name).read_text() + + +def test_sentry_dsn_is_secure_string_stub(): + ssm_tf = _read("ssm.tf") + assert 'resource "aws_ssm_parameter" "sentry_dsn"' in ssm_tf + assert 'name = "${local.ssm_prefix}/sentry-dsn"' in ssm_tf + assert 'type = "SecureString"' in ssm_tf + assert 'value = "unset"' in ssm_tf + assert "ignore_changes = [value]" in ssm_tf + assert 'data "aws_ssm_parameter" "sentry_dsn_value"' not in ssm_tf + + +def test_ecs_task_receives_sentry_dsn_parameter_name(): + ecs_tf = _read("ecs.tf") + assert ecs_tf.count("SENTRY_DSN_PARAM") == 1 + assert "aws_ssm_parameter.sentry_dsn.name" in ecs_tf + assert "SENTRY_DSN " not in ecs_tf + + +def test_terraform_does_not_embed_a_sentry_dsn(): + for path in TERRAFORM.glob("*.tf"): + text = path.read_text() + assert "ingest.sentry.io" not in text + assert "SENTRY_DSN =" not in text + + +def test_deploy_api_injects_sentry_dsn_param(): + workflow = (ROOT / ".github/workflows/deploy-api.yaml").read_text() + assert 'env["SENTRY_DSN_PARAM"] = "/meal-order-manager/sentry-dsn"' in workflow diff --git a/tests/test_worker.py b/tests/test_worker.py index 00fa848..12183f3 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -51,3 +51,20 @@ def test_worker_deletes_completed_jobs(mock_run, mock_client): sqs.delete_message.assert_called_once_with( QueueUrl="https://sqs.example/jobs", ReceiptHandle="rh-1" ) + + +@patch("server.worker.sentry_sdk.capture_exception") +@patch("server.worker.boto3.client") +@patch("server.worker.run_job") +def test_worker_captures_job_failures(mock_run, mock_client, mock_capture): + sqs = MagicMock() + mock_client.return_value = sqs + sqs.receive_message.side_effect = _one_message_then_stop({"event": "close"}) + mock_run.side_effect = RuntimeError("boom") + + with patch.dict("os.environ", {"JOBS_QUEUE_URL": "https://sqs.example/jobs"}): + worker._running = True + worker.main() + + mock_capture.assert_called_once() + sqs.delete_message.assert_not_called()