Skip to content

fix(security): HTML-escape error in OAuth callback response - #223

Merged
vishnusrichand merged 6 commits into
redhat-data-and-ai:mainfrom
NP-compete:fix/c12-oauth-error-html-escape
Aug 20, 2026
Merged

fix(security): HTML-escape error in OAuth callback response#223
vishnusrichand merged 6 commits into
redhat-data-and-ai:mainfrom
NP-compete:fix/c12-oauth-error-html-escape

Conversation

@NP-compete

Copy link
Copy Markdown
Member

Summary

  • Apply html.escape() to the error string in _callback_html() before embedding in the HTML response
  • Defense-in-depth hardening; all current callers pass hardcoded strings

Closes #222

Test plan

  • test_error_html_escapes_tags -- verifies <script> tags are escaped to &lt;script&gt;
  • All 12 OAuth handler tests pass

The error parameter in _callback_html() was interpolated directly into
HTML without escaping. While all current callers pass hardcoded strings,
this hardens against future changes that might pass user-controlled
input.

Signed-off-by: Soham Dutta <19648293+NP-compete@users.noreply.github.com>
@NP-compete
NP-compete requested a review from a team as a code owner August 15, 2026 20:46
@NP-compete NP-compete self-assigned this Aug 15, 2026
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 8955767b-6f85-4823-8c9b-0aa40b56e624

📥 Commits

Reviewing files that changed from the base of the PR and between c2ee694 and 79e5143.

📒 Files selected for processing (1)
  • deep_agent/aegra/mcp_oauth_handlers.py

Walkthrough

The OAuth callback error page now applies html.escape() to error messages before inserting them into HTML. Unit tests verify that script tags appear escaped in the rendered response.

Suggested reviewers: anish701, pratistha19

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the security fix for HTML-escaping OAuth callback errors.
Description check ✅ Passed The description accurately explains the HTML-escaping change, security rationale, linked issue, and test coverage.
Linked Issues check ✅ Passed The implementation and test cover issue #222 by escaping the OAuth callback error before HTML embedding.
Out of Scope Changes check ✅ Passed The changes are limited to the requested OAuth callback escaping fix and its focused test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
🚀 Post-Merge Actions
  • Update changelog

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@vishnusrichand vishnusrichand 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.

LGTM

@vishnusrichand
vishnusrichand merged commit 25eb0f8 into redhat-data-and-ai:main Aug 20, 2026
8 of 9 checks passed
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

🚀 Post-Merge Actions

  • Update changelog — Changes were generated but the follow-up pull request could not be opened. Stacked pull requests cannot be created when the source branch lives on a fork — for security reasons, CodeRabbit cannot push to the fork's repository. Re-run this command from a branch in the upstream repository, or open the PR from a branch on the same repository.

@NP-compete one or more post-merge actions failed.

@NP-compete
NP-compete deleted the fix/c12-oauth-error-html-escape branch August 20, 2026 14:42
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(security): HTML-escape error in OAuth callback response

3 participants