Skip to content

Fix config secrets not masked in task logs after reset_secrets_masker (#63921)#64016

Draft
deepujain wants to merge 4 commits intoapache:mainfrom
deepujain:fix-63921-remask-config-secrets-after-reset
Draft

Fix config secrets not masked in task logs after reset_secrets_masker (#63921)#64016
deepujain wants to merge 4 commits intoapache:mainfrom
deepujain:fix-63921-remask-config-secrets-after-reset

Conversation

@deepujain
Copy link
Copy Markdown
Contributor

@deepujain deepujain commented Mar 20, 2026

Title: Fix config secrets not masked in task logs after reset_secrets_masker (#63921)

Summary

reset_secrets_masker() in supervise() clears all patterns from the SDK secrets masker — including config-level secrets (webserver.secret_key, api.secret_key, api_auth.jwt_secret) that were registered at startup by the SDK configuration layer. After the reset, these secrets appear in plaintext in task subprocess logs when printed via print() or structlog.

The fix adds an SDK-native conf.mask_secrets() implementation and calls it immediately after reset_secrets_masker() so those config-level secrets are re-registered in the SDK masker before the task subprocess is forked. This avoids depending on airflow-core masking internals during provider compatibility runs.

Changes

  • task-sdk/src/airflow/sdk/configuration.py — Add SDK-native conf.mask_secrets() support so the task SDK can re-register sensitive config values without relying on airflow-core.
  • task-sdk/src/airflow/sdk/execution_time/supervisor.py — Call the SDK config parser’s conf.mask_secrets() immediately after reset_secrets_masker() to restore config masking in task subprocesses.
  • task-sdk/tests/task_sdk/execution_time/test_supervisor.py — Regression test verifying that api.secret_key, webserver.secret_key, and api_auth.jwt_secret are re-masked after reset_secrets_masker() + conf.mask_secrets().

Test plan

  • Added regression test test_supervise_remasks_config_secrets_after_reset
  • CI passes (ruff, mypy, pytest)

Fixes #63921

@deepujain deepujain force-pushed the fix-63921-remask-config-secrets-after-reset branch from 1c4eb96 to 7839403 Compare March 20, 2026 20:51
@potiuk
Copy link
Copy Markdown
Member

potiuk commented Mar 22, 2026

@deepujain This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

  • Provider tests: Failing: provider distributions tests / Compat 2.11.1:P3.10:, provider distributions tests / Compat 3.0.6:P3.10:, provider distributions tests / Compat 3.1.8:P3.10:, Integration and System Tests / Integration: providers drill. Run provider tests with breeze run pytest <provider-test-path> -xvs. See Provider tests docs.

What to do next:

  • The comment informs you what you need to do.
  • Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
  • There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
  • Maintainers will then proceed with a normal review.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

@potiuk potiuk marked this pull request as draft March 22, 2026 11:33
@potiuk
Copy link
Copy Markdown
Member

potiuk commented Mar 22, 2026

See comments in related issues.

…apache#63921)

reset_secrets_masker() clears all patterns from the SDK secrets masker,
including config-level secrets (webserver.secret_key, api.secret_key,
api_auth.jwt_secret) that were registered at startup. After the reset,
task subprocess logs no longer mask these secrets.

Re-register config secrets by calling conf.mask_secrets() immediately
after the reset when airflow.configuration is available (which is always
the case since supervisors are spawned from workers).
@deepujain deepujain force-pushed the fix-63921-remask-config-secrets-after-reset branch from 7839403 to 002a237 Compare March 27, 2026 04:24
@deepujain
Copy link
Copy Markdown
Contributor Author

Pushed a no-op commit to rerun the required checks. Validation evidence for the fix path: the targeted regression for re-masking config secrets passed locally, the full task-sdk/tests/task_sdk/execution_time/test_supervisor.py suite passed locally (122 passed, 2 skipped), and the changed-file lint checks passed; in CI, task-sdk:P3.10 tests, mypy-task-sdk, static checks, provider compat jobs (2.11.1, 3.0.6, 3.1.8), and Integration: providers drill were green on the previous run. The remaining red Integration core kerberos job failed while pulling kdc-server-example-com from ghcr.io (Client.Timeout exceeded while awaiting headers), so this push is to rerun that required check on the same branch state.

@kaxil kaxil requested a review from Copilot April 2, 2026 00:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 fixes a Task SDK regression where calling reset_secrets_masker() inside supervise() clears previously registered config-derived secret masks, causing sensitive config values (e.g. API/webserver/JWT secrets) to appear in task subprocess logs.

Changes:

  • Add AirflowSDKConfigParser.mask_secrets() to (re-)register sensitive config values (and certain secrets-backend kwarg env vars) with the SDK secrets masker.
  • Call conf.mask_secrets() immediately after reset_secrets_masker() in supervise() to restore config masking before launching the task subprocess.
  • Add a regression test intended to validate re-masking behavior after a masker reset.

Reviewed changes

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

File Description
task-sdk/src/airflow/sdk/configuration.py Adds SDK-native conf.mask_secrets() to register sensitive config/env-derived values for masking.
task-sdk/src/airflow/sdk/execution_time/supervisor.py Re-applies config secret masking right after reset_secrets_masker() in supervise().
task-sdk/tests/task_sdk/execution_time/test_supervisor.py Adds a regression test around reset + re-mask behavior.

@deepujain
Copy link
Copy Markdown
Contributor Author

Picked up the follow-up review items on this branch. I moved mask_secret to module scope in the SDK config parser, and rewrote the regression so it actually goes through supervise() and verifies the config-derived masks are back in place before ActivitySubprocess.start() runs. Local validation: targeted test_supervise_remasks_config_secrets_after_reset passed, and ruff check plus ruff format --check passed on the touched task-sdk files. I also tried the full test_supervisor.py file locally, but one unrelated remote-logging test still fails in this workspace because airflow.providers.amazon is not installed here.

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.

Secrets from Airflow configuration are not masked in task logs

3 participants