Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| print(f"Data length: {len(data)}, plan length: {len(plan)}") | ||
| for key in data: | ||
| if key in plan and data[key] is not None: | ||
| print("executing plan for", key) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 7 hours ago
To fix the issue, remove or properly gate the clear‑text logging in TelegramObject.de_json, replacing the print calls with either nothing or a controlled, opt‑in logging mechanism that does not emit sensitive data in normal operation. Since we must not change overall functionality and we have no existing logging framework imported in this file, the minimal, safest fix is simply to remove these three print statements. They are purely diagnostic and not part of the method’s public contract; removing them will not affect parsing behavior.
Concretely, in src/telegram/_telegramobject.py, within TelegramObject.de_json, delete:
print(f"checking for removed API fields for {cls.__name__}")print(f"building plan for {cls.__name__}")print(f"Data length: {len(data)}, plan length: {len(plan)}")print("executing plan for", key)print(f"skipping {key} because value is None")
No changes are needed in src/telegram/_chatfullinfo.py or src/telegram/_utils/argumentparsing.py, because they only propagate data into TelegramObject.de_json; the vulnerable sink is in _telegramobject.py. The test files do not perform logging and can be left untouched. This single change will address all the alert variants tied to the logging sink.
| @@ -537,14 +537,12 @@ | ||
| # Move removed/legacy API fields into api_kwargs | ||
| api_kwargs: JSONDict | None = None | ||
| if cls.__REMOVED_API_FIELDS__: | ||
| print(f"checking for removed API fields for {cls.__name__}") | ||
| removed = {f: data.pop(f) for f in cls.__REMOVED_API_FIELDS__ if f in data} | ||
| if removed: | ||
| api_kwargs = removed | ||
|
|
||
| # Make the plan for de_json. This is done only on the first call to de_json for each class. | ||
| if "__DE_JSON_PLAN__" not in cls.__dict__: | ||
| print(f"building plan for {cls.__name__}") | ||
| cls._build_plan() | ||
| plan = cls.__DE_JSON_PLAN__ | ||
|
|
||
| @@ -559,12 +551,8 @@ | ||
| # Only for classes with a plan. We avoid looping through the data keys for no plan classes. | ||
| if plan: | ||
| # This is O(M). This part is unavoidable. | ||
| print(f"Data length: {len(data)}, plan length: {len(plan)}") | ||
| for key in data: | ||
| if key in plan and data[key] is not None: | ||
| print("executing plan for", key) | ||
| if data[key] is None: | ||
| print(f"skipping {key} because value is None") | ||
| data[key] = plan[key](data[key], bot) | ||
|
|
||
| return cls._de_json(data=data, bot=bot, api_kwargs=api_kwargs) |
| if key in plan and data[key] is not None: | ||
| print("executing plan for", key) | ||
| if data[key] is None: | ||
| print(f"skipping {key} because value is None") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 7 hours ago
Generally, the problem is fixed by ensuring that deserialization code does not write any information derived from untrusted or potentially sensitive input to stdout/stderr or logs, unless that logging is explicitly controlled and appropriately redacted. Here, the simplest and safest fix is to remove the ad-hoc print statements from TelegramObject.de_json. Doing so does not change functional behavior of the library, only its noisy debug output.
Concretely, in src/telegram/_telegramobject.py within the TelegramObject.de_json method, remove the following print calls:
print(f"checking for removed API fields for {cls.__name__}")print(f"building plan for {cls.__name__}")print(f"Data length: {len(data)}, plan length: {len(plan)}")print("executing plan for", key)print(f"skipping {key} because value is None")
The rest of the logic (constructing api_kwargs, building the plan, transforming data) should remain unchanged. No additional imports or helper methods are required; we are only deleting these debug statements. This single change addresses the original alert and its variants because the problematic sink is removed altogether.
| @@ -537,14 +537,12 @@ | ||
| # Move removed/legacy API fields into api_kwargs | ||
| api_kwargs: JSONDict | None = None | ||
| if cls.__REMOVED_API_FIELDS__: | ||
| print(f"checking for removed API fields for {cls.__name__}") | ||
| removed = {f: data.pop(f) for f in cls.__REMOVED_API_FIELDS__ if f in data} | ||
| if removed: | ||
| api_kwargs = removed | ||
|
|
||
| # Make the plan for de_json. This is done only on the first call to de_json for each class. | ||
| if "__DE_JSON_PLAN__" not in cls.__dict__: | ||
| print(f"building plan for {cls.__name__}") | ||
| cls._build_plan() | ||
| plan = cls.__DE_JSON_PLAN__ | ||
|
|
||
| @@ -559,12 +551,10 @@ | ||
| # Only for classes with a plan. We avoid looping through the data keys for no plan classes. | ||
| if plan: | ||
| # This is O(M). This part is unavoidable. | ||
| print(f"Data length: {len(data)}, plan length: {len(plan)}") | ||
| for key in data: | ||
| if key in plan and data[key] is not None: | ||
| print("executing plan for", key) | ||
| if data[key] is None: | ||
| print(f"skipping {key} because value is None") | ||
| continue | ||
| data[key] = plan[key](data[key], bot) | ||
|
|
||
| return cls._de_json(data=data, bot=bot, api_kwargs=api_kwargs) |
1 participant
Draft implementation for "Make issue and link it"