Skip to content

fix: redact secrets from summary database logs#307

Open
harmke wants to merge 3 commits into
NVIDIA-AI-Blueprints:developfrom
harmke:fix/summary-db-password-redaction
Open

fix: redact secrets from summary database logs#307
harmke wants to merge 3 commits into
NVIDIA-AI-Blueprints:developfrom
harmke:fix/summary-db-password-redaction

Conversation

@harmke

@harmke harmke commented Jul 3, 2026

Copy link
Copy Markdown

fix: redact secrets from summary database logs

Overview

Summary Store logging truncated raw database URLs to 50 characters. PostgreSQL URLs place credentials near the beginning, so passwords could be written to application logs.

This change:

  • Parses database URLs with SQLAlchemy before logging.
  • Replaces authority passwords with ***.
  • Removes query parameters, which may contain passwords, tokens, or other secrets.
  • Preserves useful diagnostics such as driver, username, host, port, database name, and SQLite path.
  • Applies redaction to all Summary Store URL log sites.
  • Adds regression coverage for plain passwords, percent-encoded passwords, query-string secrets, and SQLite URLs.
  • Updates .secrets.baseline metadata after test line numbers changed; no new secret was added to the baseline.

Validation

uv run pytest tests/knowledge_layer_tests/test_summary_store.py -k redact_db_url

Result: 1 passed, 53 deselected

uv run pytest tests/knowledge_layer_tests/test_summary_store.py

Result: 52 passed, 2 skipped

uv run ruff check \
  src/aiq_agent/knowledge/summary_store.py \
  src/aiq_agent/knowledge/factory.py \
  tests/knowledge_layer_tests/test_summary_store.py

Result: All checks passed!

uv run ruff format --check \
  src/aiq_agent/knowledge/summary_store.py \
  src/aiq_agent/knowledge/factory.py \
  tests/knowledge_layer_tests/test_summary_store.py

Result: 3 files already formatted

pre-commit run --files \
  .secrets.baseline \
  src/aiq_agent/knowledge/summary_store.py \
  src/aiq_agent/knowledge/factory.py \
  tests/knowledge_layer_tests/test_summary_store.py

Result: all applicable hooks passed, including detect-secrets.

  • I ran the relevant local checks or explained why they are not applicable.
  • I added or updated tests for behavior changes.
  • I updated documentation for user-facing or contributor-facing changes. Not applicable: this is an internal logging security fix with no configuration or user-facing behavior change.
  • I confirmed this PR does not include secrets, credentials, or internal-only data.
  • I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with git commit -s or an equivalent sign-off.

Where should reviewers start?

Start with _redact_db_url() in src/aiq_agent/knowledge/summary_store.py, then review test_redact_db_url() in tests/knowledge_layer_tests/test_summary_store.py.

The main design decision is to remove all query parameters from the logged representation instead of maintaining a potentially incomplete allowlist of sensitive query keys.

Related Issues

  • None.

Summary by CodeRabbit

  • Bug Fixes

    • Improved logging so database connection details are now redacted before being written, reducing the risk of exposing credentials.
    • Updated connection and lifecycle logs to consistently hide sensitive URL parts across both synchronous and asynchronous flows.
  • Tests

    • Added coverage for redacting database URLs, including encoded passwords and query-parameter credentials, while preserving safe URLs unchanged.

harmke added 3 commits July 3, 2026 10:34
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: de6a2bd8-b31f-4a14-8313-a078c743139a

📥 Commits

Reviewing files that changed from the base of the PR and between db86f8f and 592c0b9.

📒 Files selected for processing (4)
  • .secrets.baseline
  • src/aiq_agent/knowledge/factory.py
  • src/aiq_agent/knowledge/summary_store.py
  • tests/knowledge_layer_tests/test_summary_store.py
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run ruff check and ruff format validation for Python code changes

**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style

Files:

  • src/aiq_agent/knowledge/factory.py
  • src/aiq_agent/knowledge/summary_store.py
  • tests/knowledge_layer_tests/test_summary_store.py
src/aiq_agent/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion

Files:

  • src/aiq_agent/knowledge/factory.py
  • src/aiq_agent/knowledge/summary_store.py
**

⚙️ CodeRabbit configuration file

**:

AI-Q Agent Guidance

Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in .agents/skills/ — load the
relevant skill before starting a workflow it covers.

Project overview

AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.

Primary boundaries:

  • Backend Python package: src/aiq_agent/.
  • Data-source and tool packages: sources/ (each is its own package).
  • Frontends and tooling: frontends/ (web UI in frontends/ui/, eval harnesses
    in frontends/benchmarks/).
  • Configs, deployment, docs: configs/, deploy/, docs/.

Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treat sources/* as independent packages: prefer the smallest change
scoped to the package you are touching.

Repository structure

Path Purpose
src/aiq_agent/ Backend agent, FastAPI extensions, auth, observability, knowledge
sources/ Data-source / tool packages (e.g. tavily_web_search, google_scholar_paper_search)
configs/ Workflow YAML configs (e.g. config_cli_default.yml)
frontends/ui/ Next.js / React / TypeScript / Tailwind / KUI web UI
frontends/benchmarks/ Eval harnesses: freshqa, deepsearch_qa, deepresearch_bench
deploy/ Docker Compose and Helm/Kubernetes assets; deploy/.env for secrets
docs/source/ ...

Files:

  • src/aiq_agent/knowledge/factory.py
  • src/aiq_agent/knowledge/summary_store.py
  • tests/knowledge_layer_tests/test_summary_store.py
{src/aiq_agent/knowledge/**,sources/**}

⚙️ CodeRabbit configuration file

{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.

Files:

  • src/aiq_agent/knowledge/factory.py
  • src/aiq_agent/knowledge/summary_store.py
**/*test*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run pytest for all behavior changes in Python code

Files:

  • tests/knowledge_layer_tests/test_summary_store.py
🔇 Additional comments (4)
src/aiq_agent/knowledge/summary_store.py (1)

39-43: LGTM!

Also applies to: 81-81, 108-108, 134-134, 147-147, 192-192, 230-230

src/aiq_agent/knowledge/factory.py (1)

294-297: LGTM!

tests/knowledge_layer_tests/test_summary_store.py (1)

33-61: LGTM!

.secrets.baseline (1)

289-293: LGTM!


Walkthrough

Adds a _redact_db_url() helper in summary_store.py to strip credentials/query params from database URLs, applies it to all related log statements (init, engine creation, disposal, table creation) and the factory's configuration log, adds a unit test, and updates the secrets baseline.

Changes

DB URL redaction in logging

Layer / File(s) Summary
Redaction helper and logging call sites
src/aiq_agent/knowledge/summary_store.py, src/aiq_agent/knowledge/factory.py
New _redact_db_url() helper clears query params and hides passwords; used across init, sync/async engine creation, stale engine disposal, table creation logs, and the factory's configuration log message.
Test coverage and secrets baseline update
tests/knowledge_layer_tests/test_summary_store.py, .secrets.baseline
New test_redact_db_url validates password redaction for PostgreSQL URLs (plain, percent-encoded, query-param) and no-op for SQLite; secrets baseline line number and timestamp refreshed.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Factory as configure_summary_db
  participant Store as SummaryStore
  participant Redactor as _redact_db_url

  Factory->>Store: SummaryStore(db_url)
  Store->>Redactor: _redact_db_url(db_url)
  Redactor-->>Store: safe URL string
  Store-->>Factory: logger.info(redacted URL)
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title uses valid Conventional Commits syntax and accurately summarizes the logging redaction change.
Description check ✅ Passed The description matches the required template with overview, validation, reviewer guidance, related issues, and checklist items.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant