Skip to content

Avoid logging Execution API bearer credentials#67059

Open
Leondon9 wants to merge 3 commits into
apache:mainfrom
Leondon9:codex/execution-api-token-log-20260517
Open

Avoid logging Execution API bearer credentials#67059
Leondon9 wants to merge 3 commits into
apache:mainfrom
Leondon9:codex/execution-api-token-log-20260517

Conversation

@Leondon9
Copy link
Copy Markdown
Contributor

Avoid passing the supplied Execution API Bearer credential into the JWT validation failure log event.

This is a small defense-in-depth hardening change. The warning message, exception info, and HTTP 403 behavior are preserved; the raw credential is no longer attached as a structured token field.

Tests:

  • breeze run ruff format airflow-core/src/airflow/api_fastapi/execution_api/security.py airflow-core/tests/unit/api_fastapi/execution_api/test_security.py
  • breeze run ruff check --fix airflow-core/src/airflow/api_fastapi/execution_api/security.py airflow-core/tests/unit/api_fastapi/execution_api/test_security.py
  • breeze run pytest airflow-core/tests/unit/api_fastapi/execution_api/test_security.py::TestJWTBearerLogging -xvs

Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-5)

Generated-by: Codex (GPT-5) following the guidelines

@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:task-sdk labels May 17, 2026
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.

Thank you for your contribution.

I believe new fragment is for changelogs and I don't think we need it here, right, @jscheffl?

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.

Yes, please NO newsfragments for bug fixes. This is for IMPORTANT announcements in releases ONLY.

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.

Please revert or remove the fragment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comments. I removed the fragment.

except Exception as err:
log.warning("Failed to validate JWT", exc_info=True, token=creds.credentials)
log.warning("Failed to validate JWT", exc_info=True)
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=f"Invalid auth token: {err}")
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.

Would it make sense for these details to return something more generic? Without the err variable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that makes sense. I updated the client-facing detail to use a generic Invalid auth token message and kept the detailed exception available only through the server-side log via exc_info=True.

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.

Please revert or remove the fragment

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 area:task-sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants