This is an automated email from the ASF dual-hosted git repository.
vatsrahul1001 pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 0de66c7997eac0b886d16bddd36e95588a882885
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Nov 3 11:37:34 2026 +0100
[v3-4-test] Keep masked Variable values when saving an edited Variable
(#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 ffd9a2d126a07d38cbc62f97fb790e4899dcca5c)
Co-authored-by: Hemkumar Chheda <[email protected]>
Co-authored-by: Claude <[email protected]>
---
.../core_api/services/public/variables.py | 29 ++++++
.../core_api/routes/public/test_variables.py | 104 +++++++++++++++++++++
.../secrets_masker/secrets_masker.py | 10 +-
.../tests/secrets_masker/test_secrets_masker.py | 24 +++--
4 files changed, 155 insertions(+), 12 deletions(-)
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/services/public/variables.py
b/airflow-core/src/airflow/api_fastapi/core_api/services/public/variables.py
index b5e63fb2d84..7b9e91a70fb 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/services/public/variables.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/services/public/variables.py
@@ -17,6 +17,7 @@
from __future__ import annotations
+import json
from typing import cast
from fastapi import HTTPException, status
@@ -24,6 +25,7 @@ from fastapi.exceptions import RequestValidationError
from pydantic import ValidationError
from sqlalchemy import select
+from airflow._shared.secrets_masker import merge
from airflow.api_fastapi.core_api.datamodels.common import (
BulkActionNotOnExistence,
BulkActionOnExistence,
@@ -40,6 +42,24 @@ from airflow.api_fastapi.core_api.services.public.common
import BulkService
from airflow.models.variable import Variable
+def _restore_redacted_value(new_value: str, old_value: str | None, key: str)
-> str:
+ """
+ Put the stored secret back wherever ``new_value`` still holds the mask
``VariableResponse`` sent out.
+
+ The UI edit form is filled from the redacted response, so saving it
unchanged would otherwise
+ write ``***`` over every sensitive field.
+ """
+ if old_value is None:
+ return new_value
+ try:
+ new_obj, old_obj = json.loads(new_value), json.loads(old_value)
+ except json.JSONDecodeError:
+ return merge(new_value, old_value, key)
+ merged = merge(new_obj, old_obj, key)
+ # Keep the caller's formatting when there was nothing to restore. indent=2
matches VariableBody.
+ return new_value if merged == new_obj else json.dumps(merged, indent=2)
+
+
def update_orm_from_pydantic(
old_variable: Variable, patch_body: VariableBody, update_mask: list[str] |
None
) -> Variable:
@@ -65,6 +85,15 @@ def update_orm_from_pydantic(
raise RequestValidationError(errors=e.errors())
non_update_fields = {"key"}
+ if (
+ "value" in patch_body.model_fields_set
+ and (not update_mask or "value" in update_mask)
+ and isinstance(patch_body.value, str)
+ ):
+ patch_body = patch_body.model_copy(
+ update={"value": _restore_redacted_value(patch_body.value,
old_variable.val, old_variable.key)}
+ )
+
# Apply patch via utility
return cast(
"Variable",
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py
index 29c1f4a87d5..dee13cd8d1a 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py
@@ -628,6 +628,90 @@ class TestPatchVariable(TestVariableEndpoint):
assert response.status_code == 422
assert Variable.get(key, session=session) == "old value"
+ @pytest.mark.enable_redact
+ @pytest.mark.parametrize("update_mask", [None, ["value"]], ids=["no-mask",
"value-mask"])
+ @pytest.mark.parametrize(
+ ("key", "stored_value"),
+ [
+ (TEST_VARIABLE_KEY2, TEST_VARIABLE_VALUE2),
+ (TEST_VARIABLE_KEY3, TEST_VARIABLE_VALUE3),
+ (TEST_VARIABLE_KEY5, TEST_VARIABLE_VALUE5),
+ (TEST_VARIABLE_KEY6, TEST_VARIABLE_VALUE6),
+ ],
+ ids=["sensitive-key", "sensitive-json-field",
"nested-sensitive-json-fields", "sensitive-key-json"],
+ )
+ def test_patch_with_redacted_value_keeps_stored_secrets(
+ self, test_client, key, stored_value, update_mask
+ ):
+ """A value sent back with fields still masked must not write ``***``
over the stored secrets."""
+ self.create_variables()
+ redacted = test_client.get(f"/variables/{key}").json()["value"]
+ assert "***" in redacted
+
+ response = test_client.patch(
+ f"/variables/{key}",
+ json={"key": key, "value": redacted},
+ params={"update_mask": update_mask} if update_mask else None,
+ )
+
+ assert response.status_code == 200
+ saved = Variable.get(key)
+ if stored_value.startswith("{"):
+ # A restored JSON value is re-serialized, so compare content
rather than layout
+ assert json.loads(saved) == json.loads(stored_value)
+ else:
+ assert saved == stored_value
+
+ @pytest.mark.enable_redact
+ @pytest.mark.parametrize("update_mask", [None, ["value"]], ids=["no-mask",
"value-mask"])
+ def test_patch_with_redacted_value_applies_other_changes(self,
test_client, update_mask):
+ """
+ Editing one field of a JSON value in the UI sends ``value`` with
``update_mask=["value"]``.
+
+ The fields the user did not touch are still ``***`` and get their
stored values back.
+ """
+ self.create_variables()
+ edited =
json.loads(test_client.get(f"/variables/{TEST_VARIABLE_KEY5}").json()["value"])
+ edited["config"]["password"] = "new_password"
+ edited["config"]["next"]["added"] = "new_field"
+
+ response = test_client.patch(
+ f"/variables/{TEST_VARIABLE_KEY5}",
+ json={"key": TEST_VARIABLE_KEY5, "value": json.dumps(edited)},
+ params={"update_mask": update_mask} if update_mask else None,
+ )
+
+ assert response.status_code == 200
+ expected = {
+ "config": {
+ "password": "new_password",
+ "next": {
+ "api_key": "some_api_key",
+ "next_again": {"token": "some_token"},
+ "added": "new_field",
+ },
+ }
+ }
+ # Same layout VariableBody uses for a JSON value, whether or not
anything was restored
+ assert Variable.get(TEST_VARIABLE_KEY5) == json.dumps(expected,
indent=2)
+
+ @pytest.mark.enable_redact
+ def test_patch_restores_secret_nested_deeper_than_max_depth(self,
test_client):
+ stored = {"a": {"b": {"c": {"d": {"e": {"f": {"token":
"some_token"}}}}}}, "schedule": "0 7 * * *"}
+ Variable.set(key="deep", value=stored, serialize_json=True)
+ edited = json.loads(test_client.get("/variables/deep").json()["value"])
+ assert edited["a"]["b"]["c"]["d"]["e"]["f"]["token"] == "***"
+ edited["schedule"] = "0 6 * * *"
+
+ response = test_client.patch(
+ "/variables/deep",
+ json={"key": "deep", "value": json.dumps(edited)},
+ params={"update_mask": ["value"]},
+ )
+
+ assert response.status_code == 200
+ assert Variable.get("deep", deserialize_json=True) == {**stored,
"schedule": "0 6 * * *"}
+
@pytest.mark.enable_redact
def test_patch_with_update_mask_description_only(self, test_client,
session):
"""PATCH with update_mask=['description'] should only update
description, keeping value unchanged."""
@@ -1547,6 +1631,26 @@ class TestBulkVariables(TestVariableEndpoint):
assert response.json()["update"] == {"success": [TEST_VARIABLE_KEY],
"errors": []}
assert Variable.get(TEST_VARIABLE_KEY, deserialize_json=True) ==
{"nested": [1, 2, True, None]}
+ @pytest.mark.enable_redact
+ def test_bulk_update_with_redacted_value_keeps_stored_secrets(self,
test_client):
+ self.create_variables()
+ redacted =
test_client.get(f"/variables/{TEST_VARIABLE_KEY5}").json()["value"]
+ actions = {
+ "actions": [
+ {
+ "action": "update",
+ "entities": [{"key": TEST_VARIABLE_KEY5, "value":
redacted}],
+ "action_on_non_existence": "fail",
+ }
+ ]
+ }
+
+ response = test_client.patch("/variables", json=actions)
+
+ assert response.status_code == 200
+ assert response.json()["update"] == {"success": [TEST_VARIABLE_KEY5],
"errors": []}
+ assert Variable.get(TEST_VARIABLE_KEY5, deserialize_json=True) ==
json.loads(TEST_VARIABLE_VALUE5)
+
def test_bulk_variables_should_respond_401(self,
unauthenticated_test_client):
response = unauthenticated_test_client.patch("/variables", json={})
assert response.status_code == 401
diff --git
a/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
b/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
index 178b451c10b..497e8f1ebcf 100644
--- a/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
+++ b/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py
@@ -452,11 +452,9 @@ class SecretsMasker(logging.Filter):
replacement: str,
) -> Redacted:
"""Merge a redacted item with its original unredacted counterpart."""
- if depth > max_depth:
- if isinstance(new_item, str) and new_item == "***":
- return old_item
- return new_item
-
+ # Walk containers at any depth, as _redact does, or a deeply nested
secret is saved as "***".
+ # A subtree _redact_all cut off at max_depth arrives as "***" with
force_sensitive set, and the
+ # last branch below restores it.
try:
# Determine if we should treat this as sensitive
is_sensitive = force_sensitive or (name is not None and
self.should_hide_value_for_key(name))
@@ -526,7 +524,7 @@ class SecretsMasker(logging.Filter):
return old_item
return new_item
- except (TypeError, AttributeError, ValueError):
+ except (TypeError, AttributeError, ValueError, RecursionError):
return new_item
def redact(
diff --git a/shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
b/shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
index f993a723eba..f1b415bde2c 100644
--- a/shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
+++ b/shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py
@@ -34,7 +34,6 @@ from airflow_shared.secrets_masker.secrets_masker import (
RedactedIO,
SecretsMasker,
mask_secret,
- merge,
redact,
reset_secrets_masker,
)
@@ -1541,15 +1540,28 @@ class TestSecretsMaskerMerge:
result = self.masker.merge(new_data, old_data)
assert result == expected
- def test_merge_max_depth(self):
+ def test_merge_ignores_max_depth_when_walking(self):
+ """redact() finds sensitive keys at any depth, so merge() has to
restore them at any depth."""
old_data = {"level1": {"level2": {"level3": {"password":
"original_password"}}}}
new_data = {"level1": {"level2": {"level3": {"password": "***"}}}}
- result = merge(new_data, old_data, max_depth=1)
- assert result == new_data
+ assert self.masker.merge(new_data, old_data, max_depth=1) == old_data
+ assert self.masker.merge(new_data, old_data, max_depth=10) == old_data
- result = self.masker.merge(new_data, old_data, max_depth=10)
- assert result["level1"]["level2"]["level3"]["password"] ==
"original_password"
+ @pytest.mark.parametrize(
+ "original",
+ [
+ {"clusters": [{"nodes": [{"conn": {"auth": {"password": "s"}}}]}]},
+ {"a": {"b": {"c": {"d": {"e": {"f": {"token": "s"}}}}}}},
+ {"a": {"b": {"c": {"d": {"password": {"e": {"f": {"g": "s", "h":
["t"]}}}}}}}},
+ ],
+ ids=["lists-of-dicts", "deeper-than-max-depth",
"sensitive-subtree-cut-off-by-redact-all"],
+ )
+ def test_merge_round_trip_deeply_nested(self, original):
+ redacted = self.masker.redact(original)
+ assert redacted != original
+
+ assert self.masker.merge(redacted, original) == original
def test_merge_enum_values(self):
old_enum = MyEnum.testname
