Skip to content

Censor ip addresses in failed_task messages. #2415

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

Open
vdbergh wants to merge 1 commit into
base: master
Choose a base branch
from
+9 −2
Open

Censor ip addresses in failed_task messages. #2415

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion worker/sri.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"__version": 305, "updater.py": "eDDBPKA/vrTCadgtEJFdL06vSoiysF0JhiHKdEnjQv3zS4kfdOAqnco/DJpDWbvh", "worker.py": "Y0kkVAk+3q3+HuWmVqgcPx7/h+CoLyPihLUszUGrvphfzNn55qXwqC+xh0fPz/Zt", "games.py": "uUpIoWu/jB6IoWN4aqSmRvtb7chhVswcFqIiT+G/cKNUWTam8ARxMZARNmTqGQ9p"}
{"__version": 305, "updater.py": "eDDBPKA/vrTCadgtEJFdL06vSoiysF0JhiHKdEnjQv3zS4kfdOAqnco/DJpDWbvh", "worker.py": "t4tOovi0IPG0K/DFZ1/6wC7rOOTh7ZzKEkAJm8zUMvPoiPv52H3jnyXoefbfckjO", "games.py": "uUpIoWu/jB6IoWN4aqSmRvtb7chhVswcFqIiT+G/cKNUWTam8ARxMZARNmTqGQ9p"}
9 changes: 8 additions & 1 deletion worker/worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,11 +1369,18 @@ def fetch_and_handle_task(
current_state["task_id"] = None
current_state["run"] = None

def censor(s):
ipv4 = re.compile(r"(?:[0-9]{1,3}\.){3}[0-9]{1,3}")
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The IPv4 regex pattern is too permissive and will match invalid IP addresses. The pattern allows each octet to be 0-999, which means it would match invalid addresses like "999.999.999.999". Each octet in an IPv4 address should be in the range 0-255. Consider using a more precise regex pattern that properly validates IPv4 addresses, or at minimum restricts octets to 0-255 range.

Suggested change
ipv4 = re.compile(r"(?:[0-9]{1,3}\.){3}[0-9]{1,3}")
ipv4 = re.compile(
r"\b(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}"
r"(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\b"
)

Copilot uses AI. Check for mistakes.
ipv6 = re.compile(r"(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}")
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The IPv6 regex pattern has several issues: 1) It's case-sensitive and only matches uppercase hex digits (A-F), but IPv6 addresses commonly use lowercase. 2) It only matches the full uncompressed form with exactly 8 groups, missing compressed forms using '::' notation and mixed notation. Consider using a case-insensitive flag (re.IGNORECASE) or including lowercase letters [A-Fa-f], and handling compressed IPv6 notation to ensure all valid IPv6 addresses are censored.

Copilot uses AI. Check for mistakes.
Comment on lines +1373 to +1374
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The regex patterns are being compiled on every function call. Since this function is called whenever a task fails, consider moving the regex compilation outside the function (as module-level constants) to avoid the overhead of recompiling the patterns repeatedly.

Copilot uses AI. Check for mistakes.
s = re.sub(ipv6, "?.?.?.?.?.?.?.?", s)
s = re.sub(ipv4, "?.?.?.?", s)
return s
Comment on lines +1372 to +1377
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

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

The new IP address censoring functionality lacks test coverage. Since the test file worker/tests/test_worker.py already contains tests for other worker.py functions, consider adding tests to verify that IPv4 and IPv6 addresses are properly censored in error messages, including edge cases like compressed IPv6 notation and invalid IP-like strings.

Copilot uses AI. Check for mistakes.

payload = {
"password": password,
"run_id": str(run["_id"]),
"task_id": task_id,
"message": server_message,
"message": censor(server_message),
"worker_info": worker_info,
}

Expand Down
Loading