ashb commented on code in PR #67868:
URL: https://github.com/apache/airflow/pull/67868#discussion_r3348146588
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -413,6 +413,32 @@ def
test_should_respond_200_with_task_state_in_deferred(self, test_client, sessi
},
}
+ @pytest.mark.enable_redact
+ def test_deferred_trigger_kwargs_are_redacted(self, test_client, session):
+ """Sensitive values in a deferred task's trigger kwargs must be masked
in the API response."""
+ now = pendulum.now("UTC")
+ ti = self.create_task_instances(
+ session, task_instances=[{"state": State.DEFERRED}],
update_extras=True
+ )[0]
+ ti.trigger = Trigger(
+ "none",
+ {"gemini_api_key": "super-secret-value-123", "polling_interval":
30},
+ )
+ ti.trigger.created_date = now
+ ti.triggerer_job = Job()
+ TriggererJobRunner(job=ti.triggerer_job)
+ ti.triggerer_job.state = "running"
+ session.commit()
+ response = test_client.get(
+
"/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context"
+ )
+ assert response.status_code == 200
+ kwargs = response.json()["trigger"]["kwargs"]
+ # The sensitive value is masked; the key name and non-sensitive values
are preserved.
+ assert "super-secret-value-123" not in kwargs
+ assert "'gemini_api_key': '***'" in kwargs
+ assert "'polling_interval': 30" in kwargs
Review Comment:
```suggestion
# The sensitive value is masked; the key name and non-sensitive
values are preserved.
assert kwargs == {"gemini_api_key": "***", "polling_interval": 30}
```
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/trigger.py:
##########
@@ -17,21 +17,34 @@
from __future__ import annotations
from datetime import datetime
-from typing import Annotated
+from typing import Annotated, Any
from pydantic import BeforeValidator, ConfigDict
+from airflow._shared.secrets_masker import redact
from airflow.api_fastapi.core_api.base import BaseModel
+def redact_kwargs(value: Any) -> str:
+ """
+ Redact sensitive values from trigger kwargs before they are exposed via
the API.
+
+ Trigger kwargs may carry credential material (for example an API key
handed to a
+ deferred operator). They are encrypted at rest, but this response decrypts
them, so
+ sensitive keys are masked here for consistency with how connection extras,
variables
+ and rendered fields are already redacted.
+ """
+ return str(redact(value))
Review Comment:
Ah no I was wrong.
So my other comment (about is this even worth returning stands) but if it
does make sense to keep this:
This shouldn't have `str()` on it -- value is almost certainly a dict, which
means this is relying on the stringification of a python dict, rather than
keeping it as a json value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
