Skip to content

Keep masked Variable values when saving an edited Variable - #76012

Merged
pierrejeambrun merged 2 commits into
apache:mainfrom
hkc-8010:fix/variable-patch-masked-merge
Nov 2, 2026
Merged

pierrejeambrun merged 2 commits into
apache:mainfrom
hkc-8010:fix/variable-patch-masked-merge

Conversation

@hkc-8010

Copy link
Copy Markdown
Contributor

Saving a Variable from the UI edit form writes the literal *** over every sensitive field in it, because the form is filled from the redacted VariableResponse and the PATCH path stores the body as sent. Connections avoid this by running secrets_masker.merge() on update; this does the same for Variables.

update_orm_from_pydantic in services/public/variables.py is shared by PATCH /variables/{key} and the bulk update action, so the merge goes there and covers both. It runs only when value is actually being updated (respects update_mask).

  • JSON values: both sides are parsed and merged with the variable key as the name, so a sensitive key name (db_password) and sensitive fields at any depth are both restored, matching how redact_val masks them. If nothing was restored the value is stored exactly as sent, formatting included.
  • Non-JSON values: plain string merge, so *** on a sensitive key keeps the stored string.
  • Fields the user changed, and fields that are new, are saved as sent.

Same limits as the Connections path, unchanged here: a sensitive field cannot be set to the literal ***, and a value masked only because it matches a registered secret (not by key name) is not restored.

Tests: four masked shapes round-tripped through GET then PATCH, a mixed edit (one secret changed, one field added, other secrets untouched), and the bulk update path. All six fail on main with the stored value replaced by ***. Full test_variables.py passes on Python 3.11 and 3.14.

closes: #76011


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code following the guidelines

@boring-cyborg boring-cyborg Bot added the area:API Airflow's REST/HTTP API label Oct 30, 2026
@hkc-8010
hkc-8010 marked this pull request as ready for review October 30, 2026 06:32
@ashb

ashb commented Oct 30, 2026

Copy link
Copy Markdown
Member

@pierrejeambrun @bbovenzi I thought we fixed this a while back already? It def rings a bell.

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

Left one small note inline.

@yuseok89

Copy link
Copy Markdown
Contributor

I think #53943 might be the similar fix, for Connections.

@bbovenzi bbovenzi added the backport-to-v3-4-test Backport to v3-4-test label Oct 30, 2026
@bbovenzi bbovenzi added this to the Airflow 3.4.0 milestone Oct 30, 2026
@bbovenzi

Copy link
Copy Markdown
Contributor

Yes, it looks like we fixed for connections but not variables.

hkc-8010 and others added 2 commits November 2, 2026 08:27
closes: apache#76011

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>

@pierrejeambrun pierrejeambrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thanks

@pierrejeambrun
pierrejeambrun merged commit ffd9a2d into apache:main Nov 2, 2026
78 checks passed
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-4-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
✅ v3-4-test PR Link

pierrejeambrun pushed a commit that referenced this pull request Nov 3, 2026
#76012) (#76308)

* Keep masked Variable values when saving an edited Variable

closes: #76011



* Restore masked values at any depth and keep Variable JSON layout



---------
(cherry picked from commit ffd9a2d)

Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
vatsrahul1001 pushed a commit that referenced this pull request Nov 4, 2026
#76012) (#76308)

* Keep masked Variable values when saving an edited Variable

closes: #76011



* Restore masked values at any depth and keep Variable JSON layout



---------
(cherry picked from commit ffd9a2d)

Co-authored-by: Hemkumar Chheda <95332229+hkc-8010@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
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 backport-to-v3-4-test Backport to v3-4-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editing a Variable in the UI overwrites masked JSON fields with ***

6 participants