Fix: Prevent cleartext credential storage in Git DAG bundles#64105
Fix: Prevent cleartext credential storage in Git DAG bundles#64105rjgoyln wants to merge 3 commits intoapache:mainfrom
Conversation
|
Just to be very clear - this is not a vulnerabilty - this is at most security improvement. |
potiuk
changed the title
|
I updated the description and removed the scary RED thingie - as this is not a vulnerability, it might be a nice security improvement though |
Thanks for the guidance, Jarek!😍 |
| self.repo_url = self.repo_url | ||
| elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"): | ||
|
|
||
| if not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"): |
There was a problem hiding this comment.
this condition would always be true unless the or here is replaced by "and".
Also the http:// urls still seem to embed the credentials in this case.
|
|
||
| with open(askpass_path) as f: | ||
| content = f.read() | ||
| assert f"echo '{ACCESS_TOKEN}'" in content |
There was a problem hiding this comment.
nit: better to add escape sequence to the single quote here.
| return | ||
|
|
||
| with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=True) as askpass_script: | ||
| askpass_script.write(f"#!/bin/sh\necho '{self.auth_token}'\n") |
There was a problem hiding this comment.
nit: might need escape sequences for the single quotes here as well.
There was a problem hiding this comment.
Thanks for your advice—I learned a lot 😊
rjgoyln
force-pushed
the
credential_leakage
branch
from
065f045 to
9470b1a
Compare
3 participants
The current GitHook stores sensitive auth_token by concatenating it directly into the repo_url (e.g., https://user:token@repo.git).
Solution
Remove URL Concatenation: Stopped injecting tokens into the URL string.
Implement GIT_ASKPASS: Switched to using the GIT_ASKPASS environment variable with a secure temporary script to handle authentication.
Verification
After the fix, the command remains clean, and all unit tests passed.
closes: #64099
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.