Repository navigation
feat: have the S3 Object Lambda reader ask the API - #2118
jackmisbach wants to merge 6 commits into
Conversation
Second of three deploys. The reader Lambda now asks the API's /s3_reader/authorize (deployed in the first step) instead of reading .models.json and Identity Store itself, and answers 503, never a denial, when the API cannot decide. It no longer logs whole events, which carry the reader's presigned URL. Its old grants stay for this deploy so in-flight invocations of the previous version keep working; the next deploy removes them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sion The API judges restricted reads by these ARNs, and the whole-event log line that used to hold them is gone, so a not_identity_center_session denial had nothing to explain it. Neither ARN is a secret. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An unavailable decision used to be an unhandled exception, which Sentry caught. Now the reader answers 503 and logs it, so log it at ERROR with the cause attached: otherwise a broken network path or audience mismatch would fail every read through the access point without paging anyone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ects - The handler tests sent only a principalId, so forwarding just that (as main does) passed them while denying every Identity Center read in prd. Send and expect the full identity instead. - Don't follow redirects from the API: requests would re-send the proof headers to the new location. - Name only the keys of an unknown event, never its presigned URL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
PSA: The priority scale is not very intuitive.
|
🥥
|
🥥
|
There was a problem hiding this comment.
🟡 Changes recommended
Credential refresh or proof-signing failures can bypass the intended retryable 503 handling and audit logging.
1 open finding
What changed in this PR
Moves S3 Object Lambda authorization decisions from the reader Lambda to the Hawk API.
Changes:
- Signs and submits each read decision to
/s3_reader/authorize. - Returns retryable 503 responses when authorization is unavailable.
- Updates dependencies, tests, infrastructure, and documentation.
| File | Description |
|---|---|
infra/tests/test_components.py |
Tests API URL wiring. |
infra/hawk/eval_log_reader.py |
Configures the reader API URL. |
infra/hawk/__init__.py |
Passes the API URL to the reader. |
hawk/uv.lock |
Updates resolved reader dependencies. |
hawk/services/modules/eval_log_reader/uv.lock |
Updates module lock dependencies. |
hawk/services/modules/eval_log_reader/tests/test_eval_log_reader.py |
Tests API authorization and failure handling. |
hawk/services/modules/eval_log_reader/pyproject.toml |
Replaces obsolete dependencies. |
hawk/services/modules/eval_log_reader/eval_log_reader/index.py |
Delegates authorization to the API. |
hawk/AGENTS.md |
Updates system-flow documentation. |
docs/infrastructure/security.md |
Documents the authorization boundary. |
docs/infrastructure/architecture.md |
Updates the Object Lambda flow. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| proof = aws_caller_proof.sign( | ||
| credentials.get_frozen_credentials(), |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Overview
Deploy 2 of 3: Object Lambda asks the API from #2115 to authorize each read, then continues delivering permitted files. Merge only after #2115 is live on prd, tst and stg; stg deploys by hand.
Approach
The Lambda signs each request with its role. Its
access_decisionlog line adds the session and issuer ARNs andauthorize_ms(replacingdecision_age_seconds), and undecidable reads log at ERROR so an outage reaches Sentry. It keeps its old grants for rollback until cleanup. Sharing policy logic gives future database checks the API's existing access/pool. Today's model policy remains S3.models.json.Risks
Testing & validation
Combined dev test, 2026-10-11, heads
318f5f4/8e1c7fd:Twenty observations per operation in consecutive serial batches; same laptop, SSO login and synthetic object; SDK retries disabled. All 120 old and 120 new serial reads passed. Added API and STS components measured 5.6 ms and 5.0 ms medians inside AWS (details); existing policy/identity lookups move into the API.
This small measured median cost buys shared permission logic and keeps future database connectivity in the API. Cold lookups/initialization can add hundreds of milliseconds or seconds; this is not a production latency guarantee.
Scan reads, 18 expected denials and 60 reads at up to four clients/five starts per second passed. An isolated connection failure returned 503 with zero bytes for HEAD/GET/range. Lambda, infra and type checks pass. Sustained load, recursive copies and #1871/#2099 were not benchmarked.
Code quality
pre-commit run --all-filespasses (ruff, basedpyright/mypy, eslint/prettier/tsc, shellcheck — what CI's Lint job runs)Before merging