Skip to content

fix: more sanitization for imgproxy bad requests#1142

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/more-imgproxy-errors
Jun 9, 2026
Merged

fix: more sanitization for imgproxy bad requests#1142
ferhatelmas merged 1 commit into
masterfrom
ferhat/more-imgproxy-errors

Conversation

@ferhatelmas

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Some imgproxy errors are matched and enriched. 5XX are always sanitized but unmatched 4XX is passed to client.

What is the new behavior?

All error messages are sanitized.

Additional context

Related to #1138

Copilot AI review requested due to automatic review settings June 9, 2026 09:51
@ferhatelmas ferhatelmas requested a review from a team as a code owner June 9, 2026 09:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the ImageRenderer’s handling of imgproxy failures to ensure all upstream error messages returned to callers are sanitized (including previously unsanitized/unmatched 4xx responses), aligning with the goal described in #1138 and this PR’s description.

Changes:

  • Added a set of “public” imgproxy error patterns and mapped them to controlled, non-leaking messages (with appropriate internal httpStatusCode values).
  • Changed the fallback behavior for unmatched imgproxy errors to return a controlled message (Invalid image request) instead of passing through the upstream body.
  • Expanded/updated the test suite to cover new sanitization cases (including trimming whitespace and additional imgproxy error strings).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/storage/renderer/image.ts Adds broader imgproxy error pattern mapping and ensures sanitized, controlled error messages for all non-OK responses.
src/storage/renderer/image.test.ts Updates and extends tests to validate the new mapping/sanitization behavior across more upstream status/body combinations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls

coveralls commented Jun 9, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27200273215

Coverage increased (+0.03%) to 76.458%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 16 of 16 lines across 1 file are fully covered (100%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/storage/protocols/tus/s3-locker.ts 2 78.37%

Coverage Stats

Coverage Status
Relevant Lines: 11045
Covered Lines: 8876
Line Coverage: 80.36%
Relevant Branches: 6464
Covered Branches: 4511
Branch Coverage: 69.79%
Branches in Coverage %: Yes
Coverage Strength: 370.0 hits per line

💛 - Coveralls

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ferhatelmas ferhatelmas merged commit 5e0d3f2 into master Jun 9, 2026
25 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/more-imgproxy-errors branch June 9, 2026 10:39
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.

4 participants