fix: enforce HTTP Problem response redaction - #1222
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughHTTP 응답 경계에서 Problem 직렬화 시 등록된 레지스트리 기반 레딕션 정책을 적용하는 유틸이 추가되었습니다. ErrorHandler와 PipelineRunner가 이를 사용해 Problem 응답과 필터 반환값을 마스킹하고, 관련 문서·테스트·정적 기준이 함께 갱신되었습니다. ChangesProblem 레딕션 강제 구현
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Filter
participant PipelineRunner
participant ErrorHandler
participant problemResponseSerializer
participant Client
Filter-->>PipelineRunner: Response(application/problem+json) or FilterResponse
PipelineRunner->>PipelineRunner: clone().json() 파싱
PipelineRunner->>ErrorHandler: createFilterResponseBody(error, body, ctx)
ErrorHandler->>problemResponseSerializer: redactHttpProblemDetailsBody(body, options)
problemResponseSerializer-->>ErrorHandler: redacted ProblemDetails
ErrorHandler-->>PipelineRunner: metadata 병합된 body
PipelineRunner-->>Client: 새 JSON Response(헤더 필터링)
Related issues: Suggested labels: review_needed_junior_swe, review_depth_deep Suggested reviewers: (분석 정보 부족으로 특정 리뷰어 추천 불가) Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
5151398 to
27f78dc
Compare
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-06T04:47:30.342Z · Commit: 7234ae3 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/transports-http/src/libs/PipelineRunner.ts`:
- Line 30: Update the BODY_SPECIFIC_RESPONSE_HEADERS set in PipelineRunner to
also include entity-validation headers that become stale after body
reserialization, such as ETag, Digest/Content-Digest, and Content-MD5. Make the
change where BODY_SPECIFIC_RESPONSE_HEADERS is defined so the response header
cleanup logic removes these headers along with the existing content-length and
content-encoding entries.
- Around line 231-245: `PipelineRunner` currently returns the original response
on `application/problem+json` parse failures or non-object/problem-unknown
bodies, which can bypass redaction. Update the response handling in
`PipelineRunner` so that when the content type is `application/problem+json`,
any JSON parse failure, non-record payload, or unrecognized Problem Details
shape falls back to the `ErrorHandler`-redacted response instead of returning
the raw response. Keep the logic localized around `response.clone().json()`,
`isRecord`, and `errorHandler.createFilterResponseBody(...)`, and only preserve
the original response for non-Problem Details content.
In `@packages/transports-http/src/libs/problemResponseSerializer.ts`:
- Around line 147-158: `extractProblemDetailsExtensions` can be polluted by a
`__proto__` key coming from untrusted `ProblemDetails` input, because it writes
into a plain `{}` accumulator. Update this helper to defensively ignore
`__proto__` (and any similar prototype-polluting keys) when building the
extensions object, and keep the change localized to
`extractProblemDetailsExtensions`/`RESERVED_PROBLEM_EXTENSION_FIELDS` so the
rest of `problemResponseSerializer` stays unchanged.
- Around line 169-192: The `toFallbackCategory` helper is collapsing all 422
responses into `ProblemCategory.BusinessRuleViolation`, which can misclassify
fallback redaction when the original category was `ValidationError`. Update the
fallback handling in `problemResponseSerializer` so 422 is resolved more
deliberately, using the available problem context (for example the source
problem or registered code) instead of always defaulting to
`BusinessRuleViolation`. Keep the change localized to `toFallbackCategory` and
the nearby fallback resolution path so `ProblemCategory.ValidationError` and
`ProblemCategory.BusinessRuleViolation` remain distinguishable when possible.
In `@packages/transports-http/src/tests/PipelineRunner.spec.ts`:
- Around line 285-343: Add a regression test in PipelineRunner.spec around the
existing redaction case for async exception filters to cover malformed
application/problem+json responses that fail parsing, are non-object, or
otherwise unrecognized. Use the current createRunner, HttpExecutionContext, and
filter catch path to verify the fallback Response still redacts the original
body and does not leak raw payload fields. Also assert that header scrubbing
removes ETag and Digest-family headers in addition to the existing
Content-Encoding and Content-Length checks.
🪄 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
Run ID: 1ff687a3-655e-4fb9-9159-c9f84c984f2a
📒 Files selected for processing (8)
.changeset/http-problem-redaction.mdpackages/problems-core/README.mdpackages/transports-http/README.mdpackages/transports-http/src/libs/ErrorHandler.tspackages/transports-http/src/libs/PipelineRunner.tspackages/transports-http/src/libs/problemResponseSerializer.tspackages/transports-http/src/tests/ErrorHandler.spec.tspackages/transports-http/src/tests/PipelineRunner.spec.ts
a6480f3 to
ca207ac
Compare
d368d28 to
8985d2d
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 `@packages/transports-http/src/libs/PipelineRunner.ts`:
- Around line 216-223: In PipelineRunner’s isFilterResponse branch, the
FilterResponse path is still returning a jsonResponse even when
createFilterResponseBody() falls back to the original body, so update this flow
to fail closed like the Response path by routing schema-mismatched
application/problem+json cases through handleError() instead of exposing the
payload. Use the existing nextError, errorHandler.createFilterResponseBody(),
httpCtx.jsonResponse(), and copyFilterResponseHeaders() logic in PipelineRunner
to detect the unsafe fallback and return the handled error response, and add a
regression test covering this FilterResponse case.
🪄 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
Run ID: 96ed3427-17de-493b-bf78-33ca9f0685ee
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (14)
.changeset/http-problem-redaction.mddocs/problem-code-registry.jsonpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/transports-http/src/classes/ErrorHandler.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/problems-core/README.mdpackages/transports-http/README.mdpackages/transports-http/src/libs/ErrorHandler.tspackages/transports-http/src/libs/PipelineRunner.tspackages/transports-http/src/libs/problemResponseSerializer.tspackages/transports-http/src/tests/ErrorHandler.spec.tspackages/transports-http/src/tests/PipelineRunner.spec.tsscripts/static-misuse-empty-catch-allowlist.jsontsconfig/contract-strict.baseline.json
💤 Files with no reviewable changes (2)
- scripts/static-misuse-empty-catch-allowlist.json
- tsconfig/contract-strict.baseline.json
8985d2d to
7f9ca91
Compare
7f9ca91 to
7eec699
Compare
Fixes #1172
Summary
Verification
Summary by CodeRabbit
새 기능
버그 수정
문서