Skip to content

fix(279): avoid leaking sanitization error payloads#281

Merged
ioncache merged 3 commits into
mainfrom
fix/279/sanitization_error_details
May 17, 2026
Merged

fix(279): avoid leaking sanitization error payloads#281
ioncache merged 3 commits into
mainfrom
fix/279/sanitization_error_details

Conversation

@ioncache
Copy link
Copy Markdown
Owner

@ioncache ioncache commented May 17, 2026

Overview

Prevent sanitization failures from exposing caller payloads in DataSanitizationError.details while keeping structured diagnostics available for logging and debugging.

Details

  • Replace raw originalData and wrapped error details with safe metadata such as inputType and errorName.
  • Preserve the existing DataSanitizationError.details shape so callers can continue reading structured error context.
  • Add regression coverage for parse failures caused by sensitive object input to ensure raw values are not present in public error details.
  • Update README error-handling guidance to show safe details and clarify that original payloads are not included.
  • Add the required implementation plan for the sanitization error details change.

Related Tickets and/or Pull Requests

Fixes #279

Summary by CodeRabbit

  • Bug Fixes

    • Error details now omit original user input and raw error objects, exposing only safe diagnostic metadata (e.g., input type and error name) to prevent sensitive-data leakage.
  • Documentation

    • Updated README examples and added a planning document describing the sanitization error details approach and expected public behaviour.
  • Tests

    • Expanded tests to assert sanitized error detail shapes and confirm sensitive fields are not present.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bd8928a4-d146-41da-b392-aa2d599885b9

📥 Commits

Reviewing files that changed from the base of the PR and between 9b5f0a2 and 0aab408.

📒 Files selected for processing (1)
  • test/index-errors.test.ts

📝 Walkthrough

Walkthrough

This PR changes sanitization error construction so DataSanitizationError.details contain only safe metadata (inputType, and optionally errorName). Internal helpers implement the classification and safe-details creation; sanitizeData error paths, tests, JSDoc, and README are updated to stop exposing original payloads.

Changes

Safe sanitization error details

Layer / File(s) Summary
Change planning and context
docs/plans/003-sanitization-error-details.md
Planning document describes the approach to expose only safe metadata while preserving the DataSanitizationError.details shape, and lists implementation, test, and doc targets.
Safe error details helpers
src/index.ts
Adds getInputType and createSafeErrorDetails to produce a safe diagnostic record with inputType and optional errorName without including caller payloads or raw error objects.
Error handling path updates
src/index.ts
Both "invalid data type" and "error parsing data" code paths now call createSafeErrorDetails(...) when constructing DataSanitizationError, replacing prior details that included originalData or raw error objects.
Error type documentation
src/errors.ts
JSDoc examples updated to show inputType in details instead of originalData.
Test coverage for safe details
test/index-errors.test.ts
Tests capture thrown errors and assert exact details shapes for invalid-type and wrapped-error scenarios, add null/array cases, and include a parse-error regression asserting absence of originalData/error and that sensitive fields are not present in serialized details.
README error handling documentation
README.md
Error-handling example and explanatory text updated to show error.details as safe diagnostic metadata and clarify that details omit the original input payload.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit nibbles through the log,
Hiding secrets in the fog,
InputType whispers where values slept,
No passwords leak, the garden kept,
Quiet fields hop home with a grin. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the issue number (279) and summarizes the primary change: preventing sanitization error payloads from leaking into error details.
Linked Issues check ✅ Passed All changes directly address issue #279 requirements: preventing sensitive payload leaks [src/index.ts, src/errors.ts, README.md], adding safe metadata [src/index.ts], and adding regression tests [test/index-errors.test.ts].
Out of Scope Changes check ✅ Passed All file changes are within scope of issue #279: error details safeguarding, implementation plan documentation, updated examples, and regression test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/279/sanitization_error_details

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@test/index-errors.test.ts`:
- Around line 189-222: The test uses an ad-hoc try/catch to capture the error
while other tests use a consistent wrapper that throws — refactor this test to
match that pattern: call sanitizeData inside a small function wrapper (same
style as the other tests) and use the existing expect(...).toThrow /
expect(...).toBeInstanceOf assertions against that wrapper; keep references to
sanitizeData and DataSanitizationError and remove the manual thrownError
variable and try/catch so the test structure matches the rest of the suite.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 875ff5d3-63dd-4678-92bd-5d7c5a889856

📥 Commits

Reviewing files that changed from the base of the PR and between 9088ebc and 2910d5a.

📒 Files selected for processing (5)
  • README.md
  • docs/plans/003-sanitization-error-details.md
  • src/errors.ts
  • src/index.ts
  • test/index-errors.test.ts

Comment thread test/index-errors.test.ts
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 17, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 100% (🎯 100%) 67 / 67
🔵 Statements 100% (🎯 100%) 67 / 67
🔵 Functions 100% (🎯 100%) 9 / 9
🔵 Branches 100% (🎯 100%) 41 / 41
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/errors.ts 100% 100% 100% 100%
src/index.ts 100% 100% 100% 100%
Generated in workflow #115 for commit 0aab408 by the Vitest Coverage Report Action

@ioncache ioncache merged commit deadafb into main May 17, 2026
5 checks passed
@ioncache ioncache deleted the fix/279/sanitization_error_details branch May 17, 2026 16:52
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.

fix: avoid leaking original payloads in sanitization errors

1 participant