Skip to content

Conversation

akaila-splunk
Copy link
Contributor

@akaila-splunk akaila-splunk commented Jan 20, 2023

added new method to mask sensitive data in logs like password. changes wrt issue #506

@ashah-splunk ashah-splunk requested a review from l00py July 10, 2023 15:44
Copy link

@l00py l00py left a comment

Choose a reason for hiding this comment

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

Added comments for review

except Exception as ex:
return data

if not isinstance(data, dict):
Copy link

Choose a reason for hiding this comment

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

Duplicate to lines 87-91?

@@ -60,12 +61,14 @@
"HTTPError"
]

SENSITIVE_KEYS = ["password", "token", "Authorization"]
Copy link

Choose a reason for hiding this comment

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

May want to review the request params for the supported endpoints specified here: https://github.com/splunk/splunk-sdk-python/blob/develop/splunklib/client.py#L90-L113

For example:

  • apps/local
    • auth
    • session
  • configs/conf
    • POST to conf files may contain sensitive information, but we can do a best-effort based on the current conf specs, and what files are normally interacted with. It may be best to provide a different logic for masking sensitive information for that specific endpoint.

@@ -844,7 +863,7 @@ def request(self, path_segment, method="GET", headers=None, body={},

all_headers = headers + self.additional_headers + self._auth_headers
logger.debug("%s request to %s (headers: %s, body: %s)",
method, path, str(all_headers), repr(body))
method, path, str(all_headers), mask_sensitive_data(body))
Copy link

Choose a reason for hiding this comment

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

The headers can contain sensitive data, specifically the original headers being passed via the handler in https://github.com/splunk/splunk-sdk-python/blob/develop/splunklib/binding.py#L1443-L1444

- updated keys in SENSITIVE_KEYS list
- masked headers data in logger
Copy link

@l00py l00py left a comment

Choose a reason for hiding this comment

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

LGTM

@akaila-splunk akaila-splunk merged commit e1f1855 into develop Jul 14, 2023
@akaila-splunk akaila-splunk mentioned this pull request Jul 14, 2023
@Ickerday Ickerday deleted the DVPL-11952 branch August 18, 2025 09:09
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.

2 participants