Add durable PostgreSQL authorization rate controls#113
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughChangesAUTH-04B PostgreSQL rate controls
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant RateControlDependency
participant RateControlService
participant PostgreSQL
Caller->>RateControlDependency: Submit authenticated request
RateControlDependency->>RateControlService: Consume scoped allowance
RateControlService->>PostgreSQL: Upsert counter and prune expired rows
PostgreSQL-->>RateControlService: Return count and database timestamps
RateControlService-->>RateControlDependency: Return allow or deny decision
RateControlDependency-->>Caller: Continue or return 429/503
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/tests/test_api_rate_controls.py (1)
270-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for the repeated raw-insert boilerplate.
The same
insert into api_rate_control_counters (...) values (...)shape is copy-pasted across the concurrency, saturation, and deadlock tests, differing only in scope/digest/offsets/count. A small async helper (e.g._insert_counter_row(factory, scope, digest, *, started_offset, expires_offset, request_count)) would cut duplication and reduce the chance of the copies silently diverging as more tests are added here.Also applies to: 310-336, 364-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_api_rate_controls.py` around lines 270 - 280, The repeated raw INSERT statements in the concurrency, saturation, and deadlock tests should be centralized. Add an async helper such as _insert_counter_row accepting the factory, scope, digest, started/expires offsets, and request_count, then replace the duplicated insert blocks around the affected tests with calls to that helper while preserving each test’s values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/tests/test_api_rate_controls.py`:
- Around line 270-280: The repeated raw INSERT statements in the concurrency,
saturation, and deadlock tests should be centralized. Add an async helper such
as _insert_counter_row accepting the factory, scope, digest, started/expires
offsets, and request_count, then replace the duplicated insert blocks around the
affected tests with calls to that helper while preserving each test’s values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 744fce6c-2b51-4d6a-b7ef-3acf3d2d2b5e
📒 Files selected for processing (27)
.agent-loop/LOOP_STATE.md.agent-loop/REVIEW_LOG.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-02-flow-node-adapter-reconciliation.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/DECISIONS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/RISKS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/STATUS.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/chunks/WS-AUTH-001-04B-postgres-rate-controls.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-internal-review-evidence.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/reviews/WS-AUTH-001-04B-pr-trust-bundle.mdbackend/alembic/versions/0017_api_controls.pybackend/app/api/deps/api_controls.pybackend/app/core/config.pybackend/app/db/models.pybackend/app/modules/api_controls/__init__.pybackend/app/modules/api_controls/models.pybackend/app/modules/api_controls/repository.pybackend/app/modules/api_controls/service.pybackend/scripts/api_contract_e2e.pybackend/tests/test_alembic.pybackend/tests/test_api_rate_controls.pybackend/tests/test_auth.pybackend/tests/test_config.pydocs/architecture_data_model.mddocs/operations_authorization_service.md
PR Trust Bundle: WS-AUTH-001-04B
Chunk
WS-AUTH-001-04B- PostgreSQL Rate ControlsGoal
Provide privacy-keyed, cross-replica PostgreSQL rate controls for future first
access and authority mutations without attaching them to production routes.
Human-Approved Intent
The user explicitly started AUTH-04B after AUTH-04A and its post-merge memory
merged. The approved boundary is a cross-replica PostgreSQL rate-control
foundation only: dependencies remain unattached, product authority remains in
later AUTH chunks, and only the user may approve PR merge.
What Changed
saturation safety, and bounded lock-safe pruning.
first_accessandadmin_mutation; both remain unattached.0017_api_controlswith guarded downgrade custody.layered dotenv, and Pydantic's public model-validation methods.
service principals, and exact resource authorization before adapter work.
Design
Each consume owns a short independent committed session. One PostgreSQL upsert
uses one
statement_timestamp()CTE to insert, reset, increment, saturate, andreturn authoritative window state. HMAC framing stores only scope and a 32-byte
digest. The server key is private
SecretStrstate assigned only aftersuccessful Pydantic validation; Pydantic receives no recoverable key object.
Scope Control
The production delta is 480 non-comment lines, below the 500-line hard stop.
No route consumer, actor/grant behavior, product authority, artifact runtime,
AUTH-05 implementation, Redis counter, or in-memory cross-replica state was
added.
Behavior Proof
Tests cover exact limits, repeated denial, expiry, database-clock retry values,
concurrency, cross-session durability, downstream rollback, saturation,
pruning, unavailable database phases, cancellation, privacy-safe persistence,
schema constraints, migration races, and representative artifact preservation.
Secret tests traverse structured error objects and complete exception graphs
and instrument the BaseSettings boundary directly.
Acceptance Criteria Proof
and bounded lock-safe pruning.
retain no raw or recoverable secret in structured errors or exception graphs.
0016preservation, exact schema guards,refusal on nonempty downgrade, writer races, empty downgrade, and re-upgrade.
Evidence Commands And Results
statement coverage.
Alembic head following destructive cleanup.
additive test delta, and diff hygiene passed.
the interrupted local global run is not evidence.
Reviewer Results
Reviewed implementation SHA is
922778b; reviewed production SHA is67484b5. Later reviewed heads contain only tests, evidence, and lifecyclememory unless explicitly recorded in the external-review response.
CI And Gate Integrity
External Review
Ready PR #113 is open. Agent Gates passed on the prior published head.
CodeRabbit completed its available review with one valid test-deduplication
nit addressed; its later incremental review was rate-limited. The first Backend
run reached 82.12 percent coverage but exposed test-owned schema cleanup. All
final-head external checks and explicit human review remain pending. None
replaces the completed internal review.
Remaining Risks
route-specific permissions and consumer tests.
cross-replica quiescence procedure.
the local run was interrupted by host shutdown.
Human Review Focus
Inspect transaction custody, HMAC framing/privacy, database-time math, pruning
lock order, migration downgrade refusal, secret object-graph non-retention, and
the fact that no production route consumes either dependency.
Human Merge Ownership
Only the user may approve and merge this PR. Publication is not merge approval,
and AUTH-05 does not start automatically.
Summary by CodeRabbit
New Features
Documentation
Tests