Skip to content

Potential fix for code scanning alert no. 29: Clear-text logging of sensitive information #278

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.

Sign up for GitHub

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

Merged
traoreera merged 1 commit into from
Sep 28, 2026
Merged

Potential fix for code scanning alert no. 29: Clear-text logging of sensitive information #278

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion xcore/kernel/observability/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,22 @@ class XcoreLogger:
"""

__slots__ = ("_log",)
_REDACTED = "***REDACTED***"
_SENSITIVE_KEYS = (
"password",
"passwd",
"pwd",
"secret",
"token",
"api_key",
"apikey",
"access_key",
"private_key",
"authorization",
"auth",
"cookie",
"session",
)
Comment on lines +96 to +110

@sourcery-ai sourcery-ai Bot Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚨 issue (security): Hyphenated API-key field names such as api-key and x-api-key do not match any entry in _SENSITIVE_KEYS, so their clear-text values are passed into xcore_ctx and emitted by both formatters.

Triggers: When callers log HTTP-style headers or nested mappings using api-key/x-api-key keys.

Suggested fix: Normalize separators in the key before matching, or include hyphenated API-key variants such as api-key in the sensitive-key set.


def __init__(self, logger: logging.Logger) -> None:
self._log = logger
Expand All @@ -100,6 +116,20 @@ def __init__(self, logger: logging.Logger) -> None:
def name(self) -> str:
return self._log.name

def _sanitize_fields(self, value: Any, key: str | None = None) -> Any:
key_l = key.lower() if isinstance(key, str) else ""
if any(s in key_l for s in self._SENSITIVE_KEYS):

@sourcery-ai sourcery-ai Bot Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): The substring match for auth redacts unrelated fields such as author, authority, and authenticated, so ordinary structured logging data is replaced with ***REDACTED***.

Triggers: When applications log non-secret fields whose names contain auth as a substring.

Suggested fix: Match normalized complete key names or explicit sensitive suffixes instead of matching auth anywhere in the key.

Suggested change
if any(s in key_l for s in self._SENSITIVE_KEYS):
if key_l in self._SENSITIVE_KEYS or any(
key_l.endswith(f"_{s}") for s in self._SENSITIVE_KEYS
):

return self._REDACTED

if isinstance(value, dict):
return {
str(k): self._sanitize_fields(v, key=str(k))
for k, v in value.items()
}
if isinstance(value, (list, tuple, set)):
return [self._sanitize_fields(v) for v in value]
return value
Comment on lines +124 to +131

@sourcery-ai sourcery-ai Bot Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): The recursive traversal does not detect cycles, so logging a self-referential dictionary or list raises RecursionError before the log record is created.

Triggers: When a structured field contains a cyclic container or nesting deeper than Python's recursion limit.

Suggested fix: Track visited container identities during traversal and replace cyclic references with a safe marker, or bound recursion depth.


def _emit(
self,
level: int,
Expand All @@ -110,7 +140,8 @@ def _emit(
) -> None:
if not self._log.isEnabledFor(level):
return
extra = {"xcore_ctx": fields} if fields else {}
safe_fields = self._sanitize_fields(fields) if fields else {}
extra = {"xcore_ctx": safe_fields} if safe_fields else {}
self._log.log(level, msg, *args, exc_info=exc_info, extra=extra)

def debug(self, msg: str, *args: Any, **fields: Any) -> None:
Expand Down
Loading