From 59fe1e325465d27621f0aab6a6b4c3eab6585dea Mon Sep 17 00:00:00 2001 From: Tim Pugh Date: Tue, 7 Jul 2026 16:32:27 -0700 Subject: [PATCH] fix: bound the retry budget for two idempotency DynamoDB writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The @idempotent layer makes two sequential DynamoDB writes per request (save_inprogress before the handler, save_success after), so a DynamoDB brownout hits the shared retry budget twice. With total_max_attempts=3 (~7.5s per call) the two writes could reach ~15s and exceed the 10s function timeout — a 502 off the OpenAPI contract that feeds the canary rollback alarm. Drop to total_max_attempts=2 (~4s/call, ~8s for both) and correct the budget comment and the test assertion. Also document (no behavior change) that the RUM guest identity pool lets anyone call rum:PutRumEvents directly, and that session_sample_rate is client-side and does not bound that vector — the real guardrail is a Budgets / RUM-volume alarm. --- TODO.md | 1 + infrastructure/frontend_stack.py | 8 ++++++++ lambda/app.py | 24 +++++++++++++----------- tests/unit/test_handler.py | 10 ++++++---- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/TODO.md b/TODO.md index dee16dd..59b0fae 100644 --- a/TODO.md +++ b/TODO.md @@ -194,6 +194,7 @@ The table and its dedicated CMK live in their own stack, [`DataStack`](infrastru ### CloudWatch RUM - [ ] **Lower the RUM session sample rate at scale** — currently `session_sample_rate=1.0` (100%). Per the [RUM authorization docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-RUM-get-started-authorization.html) and [pricing](https://aws.amazon.com/cloudwatch/pricing/), RUM is billed per ingested event so 100% sampling becomes expensive once real traffic hits the page. 5–10% is a common production starting point. Also revisit `allow_cookies` and `enable_x_ray` against the workload's privacy posture — both are on for full session attribution today. +- [ ] **Bound RUM ingestion cost against guest-pool abuse** — the RUM identity pool is `allow_unauthenticated_identities=true` (inherent to browser RUM — guests need credentials to call `rum:PutRumEvents`), so anyone can mint guest credentials via `cognito-identity:GetId` / `GetCredentialsForIdentity` and POST events to the RUM data plane directly. RUM is billed per ingested event and there is no per-caller server-side cap. **`session_sample_rate` does not mitigate this** — it is a client-side knob the RUM *browser* client honors; a caller hitting `PutRumEvents` directly ignores it entirely (lowering it only reduces *legitimate* traffic, above). The public guest pool can't be eliminated without breaking browser RUM, so the guardrail is **detection**: an [AWS Budgets](https://docs.aws.amazon.com/cost-management/latest/userguide/budgets-managing-costs.html) alarm on RUM spend and/or a CloudWatch alarm on RUM ingestion volume, so abuse is caught quickly. The existing `bytes_scanned_cutoff_per_query` (Athena) and API Gateway stage throttling guards do not cover this path. ### CloudTrail diff --git a/infrastructure/frontend_stack.py b/infrastructure/frontend_stack.py index 6d3b07c..4bb8cce 100644 --- a/infrastructure/frontend_stack.py +++ b/infrastructure/frontend_stack.py @@ -396,6 +396,14 @@ def __init__( app_monitor_configuration=rum.CfnAppMonitor.AppMonitorConfigurationProperty( allow_cookies=True, enable_x_ray=True, + # session_sample_rate is a CLIENT-SIDE knob: the RUM browser + # client reads it to decide what fraction of sessions to record. + # It does NOT bound the data plane — the public unauthenticated + # identity pool above lets anyone mint guest credentials and call + # rum:PutRumEvents directly, ignoring this rate entirely. So + # lowering it is a legitimate-traffic COST lever only; the + # adversarial-ingestion vector needs a Budgets / RUM-volume alarm, + # not a smaller sample rate (see TODO.md "CloudWatch RUM"). session_sample_rate=1.0, # CloudFormation's schema only accepts ["errors", "performance", "http"] here — # "interaction" is rejected as an invalid enum value despite being a real RUM diff --git a/lambda/app.py b/lambda/app.py index 0ce09a1..cd0476a 100644 --- a/lambda/app.py +++ b/lambda/app.py @@ -82,18 +82,20 @@ # retry ever fired. Bounding the attempt turns a hang into a fast, retryable # error; 0.5s/1s is generous for the small same-region SSM/AppConfig/DynamoDB # calls this handler makes. -# Budget math (keep it under the function timeout when tuning): the FULL retry -# budget for one hung dependency is total attempts x (connect + read) plus -# exponential-backoff sleeps between attempts — here 3 x 1.5s = 4.5s plus -# ≤ ~3s of jittered sleeps ≈ 7.5s worst case, inside the function's 10s -# timeout with headroom for the up-to-three sequential AWS calls a cold -# request makes. (The previous config — max_attempts=4, i.e. 5 total attempts -# x 3s = 15s — exceeded the timeout on its own: a browned-out dependency -# became an API Gateway 502 — off the OpenAPI contract, no CORS header — -# instead of the designed fast 500, and fed the canary rollback alarm during -# deploys.) +# Budget math (keep it under the function timeout when tuning). The @idempotent +# layer makes TWO sequential DynamoDB writes per request — save_inprogress +# (conditional PutItem) before the handler body, save_success (UpdateItem) +# after — so a DynamoDB brownout hits this retry budget TWICE, not once. Size it +# per call so even two serial browned-out DynamoDB calls stay under the 10s +# function timeout: total_max_attempts=2 bounds ONE call at 2 x (0.5 + 1) +# attempt-timeouts + ~1s of backoff ≈ 4s, so the two writes together reach ~8s +# < 10s, with headroom for the SSM/AppConfig reads in between. (History: +# total_max_attempts=3 put one call at ~7.5s, so the two writes reached ~15s and +# timed out on a DynamoDB brownout — an API Gateway 502, off the OpenAPI +# contract with no CORS header, feeding the canary rollback alarm during +# deploys; the earlier max_attempts=4 was worse still.) boto_config = Config( - retries={"mode": "adaptive", "total_max_attempts": 3}, + retries={"mode": "adaptive", "total_max_attempts": 2}, connect_timeout=0.5, read_timeout=1, ) diff --git a/tests/unit/test_handler.py b/tests/unit/test_handler.py index b2a570a..0420193 100644 --- a/tests/unit/test_handler.py +++ b/tests/unit/test_handler.py @@ -468,12 +468,14 @@ def test_sdk_socket_timeouts_bounded(lambda_app_module): retry policy ever fires. Explicit per-attempt bounds convert a hang into a fast, retryable error. The values are pinned (not just asserted non-default) because the FULL retry budget — total attempts x (connect + read) + backoff - sleeps — must stay under the function's 10s timeout; see the budget math on - boto_config in lambda/app.py. Asserting on the live clients (not just the - config object) catches a refactor that drops boto_config= from a constructor. + sleeps — must stay under the function's 10s timeout even though @idempotent + makes two serial DynamoDB writes per request (so a DynamoDB brownout hits the + budget twice); see the budget math on boto_config in lambda/app.py. Asserting + on the live clients (not just the config object) catches a refactor that + drops boto_config= from a constructor. """ # botocore normalizes max_attempts to total_max_attempts inside Config. - assert lambda_app_module.boto_config.retries["total_max_attempts"] == 3 + assert lambda_app_module.boto_config.retries["total_max_attempts"] == 2 assert lambda_app_module.boto_config.connect_timeout == 0.5 assert lambda_app_module.boto_config.read_timeout == 1 assert lambda_app_module.ssm_provider.client.meta.config.connect_timeout == 0.5