-
Notifications
You must be signed in to change notification settings - Fork 15.9k
Unmask values in UI variables export #57594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
I don't think our permissions model has the distinction, but I think it should need more than just write permission on connections to be able to export them un-redacted. |
|
Current model absolutely does not allow any exposure of sensitive data via remote API - indepently on permissions. My proposal that I am going to bring to devlist if thewre will be no opposal in the security thread is to remove that export/import functionality for UI and leave a comment that you need to use local CLI. I think it would be. very wrong to allow any remote access to sensitive data when there were voices how "lame" it is that we do it - previously we were doing exactly this the sensitive data was sent over to UI for those who had "connection write access" so if we allow that, we would be back to square one and we would be invalidating the CVE we just announced, so I would be surprised to see that happening. |
|
More details on the CVE: https://www.cvedetails.com/cve/CVE-2025-54831/
Unmasking values in export in UI, explicitly brings back the condition that triggered the CVE in the first place. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting change as the PR directly violates the security model we currently have in Airflow 3.
If we agree - in the security team - to change the model, we might want to re-evaluate this, but the current proposal that looks for consensus in the security team (and later in the devlist) is that the export functionality should be removed from the UI and users directed to the CLI.
|
Thanks for the detailed security context @potiuk. I understand the concern and will be holding off on further changes until the security team and devlist reach consensus. I'm happy to pivot this PR to implement that solution by removing the export button from the UI and adding appropriate messaging to guide users to export unmasked variables via CLI. |
|
Started discussion at the devlist https://lists.apache.org/thread/c79668yh42m5g7f7xck3oh6vft0z2kb6 |
Summary
Fixed variable export functionality in the Airflow UI to return unmasked/real values instead of masked values (***), and added admin/elevated access protection to ensure only authorized users can export unmasked values.
Problem
This problem occurred because the UI was downloading cached data from the GET endpoint which uses VariableResponse model that masks sensitive fields. CLI export worked correctly because it directly accesses the database without masking
Solution
Created a new dedicated export endpoint with proper security controls. Requires OP level access as it grants controls over editing variables
Test
Added test_export_variables_with_unmasked_values that verifies password variables export with real values
Related Issue
Fixes #57428