Skip to content

Facade credentials augur startup#3769

Draft
IsaacMilarky wants to merge 7 commits intomainfrom
facade-credentials-augur-startup
Draft

Facade credentials augur startup#3769
IsaacMilarky wants to merge 7 commits intomainfrom
facade-credentials-augur-startup

Conversation

@IsaacMilarky
Copy link
Contributor

Description
Move credential storing logic to when augur is started instead of storing them on install.

Also, move the .git-credentials file to a temporary file and symbolic link the file to the facade directory.

This PR fixes #2928

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
with open("/tmp/.git-credentials", "w") as f:
f.write(credential_file_text)

os.symlink("/tmp/.git-credentials", f"{repo_base_directory}/.git-credentials")

Check warning

Code scanning / Bandit

Probable insecure usage of temp file/directory. Warning

Probable insecure usage of temp file/directory.
os.symlink("/tmp/.git-credentials", f"{repo_base_directory}/.git-credentials")

facade_credential_store = f"git config --global credential.helper \"store --file {repo_base_directory}/.git-credentials\""
subprocess.Popen(facade_credential_store.split(" "))

Check notice

Code scanning / Bandit

subprocess call - check for execution of untrusted input. Note

subprocess call - check for execution of untrusted input.
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
import traceback
import requests
import tempfile
from redis.exceptions import ConnectionError as RedisConnectionError

Choose a reason for hiding this comment

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

[pylint] reported by reviewdog 🐶
W0611: Unused ConnectionError imported from redis.exceptions as RedisConnectionError (unused-import)

Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
@MoralCode
Copy link
Contributor

Can you document the core reason for making this change in the PR as well?

As far as i noticed recently, it seems like we are already writing this file.

if we can use the timeout parameter to have git keep these credentials in memory (https://stackoverflow.com/a/35943882/), or some other, more robust secret storage method like GCM (https://stackoverflow.com/questions/36585496/error-when-using-git-credential-helper-with-gnome-keyring-as-sudo/40312117#40312117) that would be better than storing keys on disk IMO.

If we need to have the augur instance hosts come up with a secret value (like other self hosted open source apps have) to use as the credential to unlock the secret store/decrypt credentials, that may help us be a little safer with how we handle git credentials

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache git credentials in facade on augur startup instead of installation

2 participants