Skip to content

Add regression tests for cli safe flash escaping#66648

Open
Sanskar121543 wants to merge 1 commit into
apache:v3-1-testfrom
Sanskar121543:fix-html-escaping-validation
Open

Add regression tests for cli safe flash escaping#66648
Sanskar121543 wants to merge 1 commit into
apache:v3-1-testfrom
Sanskar121543:fix-html-escaping-validation

Conversation

@Sanskar121543
Copy link
Copy Markdown

Summary

Adds regression coverage for _cli_safe_flash to verify:

  • HTML content is safely escaped before rendering in flash messages
  • CLI output formatting correctly converts HTML tags to terminal-safe text

Why

This helps prevent accidental unsafe rendering of user-controlled content in FAB auth manager flash messages.

@potiuk
Copy link
Copy Markdown
Member

potiuk commented May 11, 2026

@Sanskar121543 A few things need addressing before review — see our Pull Request quality criteria.

  • Pre-commit / static checks — Failing: CI image checks / Static checks. See docs.

No rush.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@Sanskar121543 Sanskar121543 force-pushed the fix-html-escaping-validation branch from 6aa3049 to b442c6e Compare May 11, 2026 05:18
@Sanskar121543
Copy link
Copy Markdown
Author

@Sanskar121543 A few things need addressing before review — see our Pull Request quality criteria.

  • Pre-commit / static checks — Failing: CI image checks / Static checks. See docs.

No rush.

Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

Thanks! I fixed the static check issue by removing the unintended generated file change and updated the PR branch. The checks should now be clean.

@potiuk
Copy link
Copy Markdown
Member

potiuk commented May 18, 2026

@Sanskar121543 A few things need addressing before review — see our Pull Request quality criteria.

Issues found:

  • Pre-commit / static checks: CI image checks / Static checks is failing. Run prek run --from-ref main --stage pre-commit locally and fix anything that flags. See the static-checks docs.

What to do next:

  • Push a fix for the static-check failure.

No rush — take your time. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.


Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

@Sanskar121543
Copy link
Copy Markdown
Author

Hi @vincbeck, just following up on this PR when you get a chance.

The static check issue mentioned by @potiuk has been addressed and the branch has been updated. All relevant checks should now be passing.

Would appreciate a review whenever you have time. Thanks!

@vincbeck
Copy link
Copy Markdown
Contributor

CI is still failing

@Sanskar121543 Sanskar121543 force-pushed the fix-html-escaping-validation branch from b442c6e to 71260b9 Compare May 19, 2026 17:23
@Sanskar121543
Copy link
Copy Markdown
Author

CI is still failing

Thanks for checking. I pushed another fix in commit 71260b9 to address the remaining static check issues (newline at EOF and removal of the unintended generated OpenAPI file change). The workflows are rerunning now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants