Skip to content

fix(debug): avoid retaining query objects when logging is disabled - #30556

Open
akasakariko wants to merge 1 commit into
prisma:v7from
akasakariko:fix/debug-history-retention-30494
Open

akasakariko wants to merge 1 commit into
prisma:v7from
akasakariko:fix/debug-history-retention-30494

Conversation

@akasakariko

@akasakariko akasakariko commented Oct 1, 2026 •

Copy link
Copy Markdown

Summary

Fixes #30494

  • Keep useful error report history when debug output is disabled without retaining query plans, parameter arrays, or closures
  • Preserve strings and primitive arguments, summarize errors as name and message, and replace other objects and functions with [Object]
  • Use the same effective enabled state for history and console output so enabled namespaces and instance overrides retain their existing behavior
  • Add regression coverage for summaries, error snapshots, namespace filters, runtime toggling, detailed output, history eviction, truncation, and clearing
  • Document the debug history behavior in the agent field notes

Validation

  • All 40 debug tests pass, including 12 new cases
  • Seven new regression cases fail against the original implementation
  • All 55 PostgreSQL adapter tests, 8 Neon adapter tests, and 45 MariaDB adapter tests pass
  • Debug package build and TypeScript checks pass
  • Formatting and staged file checks pass
  • ESLint reports no errors and the same two warnings as the original source
  • A focused Node GC check over 40 synthetic batches retains 100 of 160 tracked objects before the fix and zero after the fix

Scope

  • Targets v7 as requested in the issue
  • The full PostgreSQL reproduction from the issue was not run
  • Enabled debug history and primitive string sizes are unchanged

Summary by CodeRabbit

  • Bug Fixes

    • Debug history now stores summarized values for calls made while logging is disabled, reducing the detail retained. Calls made while logging is enabled continue to retain their original arguments.
    • History remains limited to the most recent 100 nonempty calls.
  • Documentation

    • Added guidance on debug history behavior, enabled checks, and a database-free regression test command.

@coderabbitai

coderabbitai Bot commented Oct 1, 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: Repository: prisma/orm/.coderabbit.yml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0447b75f-d77e-4a24-995e-03750780945a

📥 Commits

Reviewing files that changed from the base of the PR and between 9afb1a4 and 318fd09.

📒 Files selected for processing (3)
  • AGENTS.md
  • packages/debug/src/__tests__/history.test.ts
  • packages/debug/src/index.ts

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

Walkthrough

Debug history now stores summarized arguments for disabled calls and original arguments for enabled calls. Tests cover argument values, logging behavior, history limits, truncation, and clearing.

Changes

Debug history recording

Layer / File(s) Summary
Enabled-aware history recording
packages/debug/src/index.ts, packages/debug/src/__tests__/history.test.ts, AGENTS.md
debugCall checks whether logging is enabled before recording arguments. Disabled calls store errors as name: message, objects and functions as [Object], and other values unchanged. Enabled calls retain original arguments. Tests cover these behaviors, the 100-entry limit, and history controls. Documentation describes the behavior and regression test command.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 318fd

This change stops disabled debug calls from keeping large objects alive in history while preserving enabled-call behavior. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 318fd

The change reduces retention of query objects when logging is disabled and preserves the inspected credential-protection control. However, Error messages now become visible in diagnostic history and shareable report links. No actual secret disclosure was established, but message sensitivity and downstream report coverage remain incompletely assessed.

Retained concerns

  • Low · security · inferred: Disabled logging now promotes Error names and messages into shared diagnostic history. Driver Error arguments reach this history, and a known consumer embeds it in shareable issue-report links without message redaction. This widens report contents compared with ordinary Error serialization in the available base and could expose sensitive message text. No credential disclosure was verified; the inspected MariaDB URL parse-error control remains effective.
Security review details

Security Blast Radius

  • inferred — The relevant exposure is diagnostic content collected across namespaces sharing this module instance, not newly granted database or infrastructure privileges. An Error message can persist until eviction or clearing and appear in a later report. Complete application, tenant, and report-destination scope is not established.

Security Findings and Attack Paths

  • inferred — A conditional disclosure path is driver Error argument to disabled-history message snapshot to getLogs to an issue-report link, followed by user sharing. The representation change and consumer are established, but sensitive message contents, attacker influence over them, and an actual disclosure are not verified.

Trust Boundaries and Controls

  • observed — Object summarization limits disabled-history retention but is not a general secret-redaction control: strings and Error messages remain. The inspected report normalizer removes timing text, not sensitive message content. MariaDB instead protects the named parse-failure path at its producer by logging only a fixed string.

Resilience and Maintainability Implications

  • observed — Disabled calls no longer retain ordinary query objects, parameter arrays, or closures. The entry-count bound remains unchanged and does not impose a byte bound on preserved strings; enabled history also continues to retain detailed arguments. These remaining retention properties predate the PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change satisfies the coding requirements in [#30494]. When the effective namespace state is disabled, debugCall stores strings and primitive values unchanged, converts errors to name: message,…
Out of Scope Changes check ✅ Passed The changes stay within [#30494]. The added tests verify the requested debug-history behavior. The debug note documents the changed behavior. The source change only updates argument history handling a…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: preventing query objects from being retained when debug logging is disabled.
Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

Signed-off-by: KayanoLiam <Kayano04@outlook.jp>
@akasakariko
akasakariko force-pushed the fix/debug-history-retention-30494 branch from 358fe2f to 318fd09 Compare October 1, 2026 06:25
@akasakariko

Copy link
Copy Markdown
Author

Hi, just a friendly ping. This PR is ready for review whenever you have a chance. Happy to address any feedback

This branch has not been deployed

No deployments
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