Skip to content

Strip CR/LF from user-supplied logical_date before stdlib logging#67500

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:sanitize-logical-date-log-injection
May 25, 2026
Merged

Strip CR/LF from user-supplied logical_date before stdlib logging#67500
potiuk merged 1 commit into
apache:mainfrom
potiuk:sanitize-logical-date-log-injection

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented May 25, 2026

action_logging passes the raw logical_date query parameter into logger.exception("... %s", value) via Python's standard logging module on parse failure. On deployments configured with a non-JSON (plain-text) log formatter, an attacker could supply a value containing newline characters to forge fake log entries (CWE-117 log injection).

The path is narrow — only exploitable on non-default plain-text formatters AND only when the user triggers a parse failure — but the fix is cheap.

Reported as F-018 in the apache/tooling-agents L3 ASVS sweep 0920c77.

Change

Add _sanitize_for_stdlib_log() that replaces \r and \n with spaces, and apply it before formatting the logical_date value into the logger.exception message. The helper is extracted so the guard is testable in isolation. logger.exception stays on the stdlib logger (rather than swapping to structlog) to keep the change minimal and avoid coupling unrelated behaviour changes into a security fix.

Test plan

  • Parametrised TestSanitizeForStdlibLog::test_strips_cr_and_lf covers \n, \r, \r\n, multi-line, empty, and the no-op case.
  • prek run ruff clean.
  • prek run mypy-airflow-core clean.
  • 6 tests pass.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

``action_logging`` passed the raw ``logical_date`` query parameter into
``logger.exception("... %s", value)`` via Python's standard logging
module on parse failure. On deployments configured with a non-JSON
(plain-text) log formatter, an attacker could supply a value containing
newline characters to forge fake log entries (CWE-117 log injection).

The path is bounded — only exploitable on non-default plain-text
formatters and only when the user actually triggers a parse failure —
but the fix is cheap: replace ``\r`` and ``\n`` with spaces before
formatting.

Extract the sanitisation into ``_sanitize_for_stdlib_log()`` so the
guard is testable in isolation. ``logger.exception`` is left in place
on the stdlib logger (rather than swapped for ``structlog``) to keep
the change minimal and avoid coupling the audit-log path's other
behaviour changes into a security fix.
@potiuk
Copy link
Copy Markdown
Member Author

potiuk commented May 25, 2026

defense-in-depth

@potiuk potiuk merged commit 4a61b84 into apache:main May 25, 2026
137 of 138 checks passed
@potiuk potiuk deleted the sanitize-logical-date-log-injection branch May 25, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants