Skip to content
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

Mask password in logs when calling interlis utilities #203

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Conversation

domi4484
Copy link
Contributor

Fix #167

@domi4484 domi4484 requested a review from 3nids April 12, 2024 09:44
@ponceta ponceta self-requested a review April 12, 2024 11:10
@ponceta
Copy link
Member

ponceta commented Apr 12, 2024

Add a comment?

# This command replaces the password in the command string with "[PASSWORD]" 
# to avoid exposing sensitive information in the logs, then saves the result with the level of information.

@@ -52,7 +53,8 @@ class CmdException(BaseException):


def exec_(command, check=True, output_content=False):
logger.info(f"EXECUTING: {command}")
command_masked_pwd = re.sub(r"(--dbpwd)\s\"[\w\.*#?!@$%^&-]+\"", r'\1 "[PASSWORD]"', command)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about this to be safe?

Suggested change
command_masked_pwd = re.sub(r"(--dbpwd)\s\"[\w\.*#?!@$%^&-]+\"", r'\1 "[PASSWORD]"', command)
command_masked_pwd = re.sub(r"(--dbpwd)\s\".+\"", r'\1 "[PASSWORD]"', command)

Copy link
Contributor

Choose a reason for hiding this comment

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

and is the password is always in double quotes?
i.e. is there a risk of $ sign substitution if it's in double rather than single quotes? (which would an issue on its own)

@sjib sjib added the security label Apr 17, 2024
@ponceta ponceta merged commit 29b12c4 into main Apr 17, 2024
4 checks passed
@ponceta ponceta deleted the maskPwd branch April 17, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interlis export log include credentials in cleartext
4 participants