Skip to content

feat(database): Exercise least-privilege db roles - #236

Merged
tmikula-dev merged 1 commit into
masterfrom
feature/229-custom-db-roles
Sep 30, 2026
Merged

tmikula-dev merged 1 commit into
masterfrom
feature/229-custom-db-roles

Conversation

@tmikula-dev

@tmikula-dev tmikula-dev commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Overview

This pull request improves the handling and documentation of PostgreSQL credentials for Lambda functions, ensuring each Lambda uses its own least-privilege secret and role, and updates integration tests to mirror this production setup.

Related

Closes #229

Summary by CodeRabbit

  • Documentation
    • Clarified that each Lambda should use a secret for its own least-privilege role, and that the Postgres Writer secret should reference the eventgate_writer role.
  • Logging
    • PostgreSQL connection-established logs now include the configured database username alongside the database, host, and port.

@tmikula-dev tmikula-dev self-assigned this Sep 29, 2026
@tmikula-dev tmikula-dev added refactoring Improving code quality, paying off tech debt, aligning APIs no RN No release notes required labels Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 97cb8d89-72ba-4f26-b2b5-6ceec394a6cc

📥 Commits

Reviewing files that changed from the base of the PR and between 6c4ab92 and a828328.

📒 Files selected for processing (3)
  • README.md
  • src/utils/postgres_base.py
  • tests/integration/conftest.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

The README describes role-specific database secrets, and integration fixtures configure separate writer and reader secrets. The PostgreSQL connection-established log now includes the configured database user.

Changes

PostgreSQL role-specific secrets

Layer / File(s) Summary
Document and configure role-specific secrets
README.md, tests/integration/conftest.py
The README describes per-Lambda secrets and identifies the writer role. Integration fixtures create writer and reader secrets and select the matching secret for each Lambda.
Include the configured database user in connection logs
src/utils/postgres_base.py
The connection-established log adds the configured database user to its structured fields. Existing database, host, and port fields remain.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: oto-macenauer-absa

Merge Risk: ⚪ Minimal · up to a8283

The integration fixture assigns matching writer and reader credentials before Lambda initialization, and the suite exercises database operations through both clients. No concrete current-head failure remains that should block merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a8283

The changes do not show a new production permission or database access path. They improve role-specific test setup, but do not establish that deployed Lambdas use the intended secrets; the tests also do not directly verify each Lambda’s effective database identity.

Retained concerns

  • Low · security · inferred: The role tests verify privileges through direct database connections rather than the Lambdas’ configured secrets. The changed fixture selects a reader secret for the stats handler, but the inspected checks do not directly establish its effective database identity; a reader path using writer credentials could still pass read checks. This is an assurance gap, not evidence of a deployed misbinding.
Security review details

Security Blast Radius

  • inferred — A stats Lambda bound to writer credentials would have database write authority beyond the tested reader role. The available evidence does not establish such a deployed binding or a new attacker path.

Security Findings and Attack Paths

  • observed — The changed connection log records the configured username after a successful connection. The adjacent connection arguments contain a password, but the structured log fields do not.

Trust Boundaries and Controls

  • observed — The secret name comes from process environment configuration; the loader retrieves that secret’s value from Secrets Manager, and the connection helper uses its user and password for PostgreSQL authentication.

Resilience and Maintainability Implications

  • inferred — Successful direct role-permission checks cannot detect a Lambda selecting the wrong secret. The separate fixture import sequence exercises selection, but the inspected permission checks do not assert the resulting session identity.

Hardening Proposals

  • proposed — Verify the effective database user through each Lambda’s configured secret path, in addition to testing role privileges directly.
  • proposed — Before treating the documented role split as a deployed control, verify each Lambda’s environment binding and secret permissions, including how old and new secrets coexist during any deployment or rollback.
🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive For #229, src/utils/postgres_base.py loads credentials at runtime from POSTGRES_SECRET_NAME and passes the resolved user and password to psycopg2. The integration fixture creates separate `event… Provide reviewable evidence for the production Lambda environment values and the corresponding IAM policies. Confirm whether POSTGRES_SECRET_NAME values are secret ARNs or secret names, because #229 requires secret ARN values.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: exercising least-privilege database roles.
Description check ✅ Passed The description explains the change and links issue #229. It omits the required Release Notes section, but the main required context is present.
Out of Scope Changes check ✅ Passed The README update documents least-privilege secret configuration. The PostgreSQL log update records the resolved database user. The integration fixture uses separate writer and reader secrets and role…
Full details: Linked Issues check

Explanation

For #229, src/utils/postgres_base.py loads credentials at runtime from POSTGRES_SECRET_NAME and passes the resolved user and password to psycopg2. The integration fixture creates separate eventgate_writer and eventgate_reader secrets and selects them for the two Lambda handlers. The reviewable summary does not establish that production Lambdas receive secret ARNs, or that each Lambda IAM role allows secretsmanager:GetSecretValue only for its required secret. The whole-PR diff could not be read because repository fetching failed.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the secrets with care,
A writer key and reader share,
The logs now name the user too,
While carrots wait beside the queue,
Hop by hop, the tests shine through.

Comment @coderabbitai help to get the list of available commands.

@tmikula-dev
tmikula-dev merged commit 0d4ddf3 into master Sep 30, 2026
11 of 12 checks passed
@tmikula-dev
tmikula-dev deleted the feature/229-custom-db-roles branch September 30, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no RN No release notes required refactoring Improving code quality, paying off tech debt, aligning APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use custom DB roles in EventGate Lambdas

2 participants