Skip to content

Commit d6a9307

Browse files
authored
Hardened opt-in upload privacy and reliability (#1343)
1 parent 3eff119 commit d6a9307

7 files changed

Lines changed: 489 additions & 49 deletions

File tree

PRIVACY_POLICY.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ This will be an optional _opt-in_ feature that is not required to use PythonTA.
55

66
## What data will be sent?
77

8-
When PyTA check a file/directory (by calling `python_ta.check_all`), two types of data may be sent:
8+
When PyTA checks a file/directory (by calling `python_ta.check_all`), two types of data may be sent:
99

1010
- The errors detected by PyTA during the check.
1111
- The source files that you ran PyTA on.
1212

1313
These forms of data submission are independent and optional.
1414
If you use a custom PyTA configuration, this information will be sent alongside either of the above data.
15+
Each upload also includes the PythonTA version and an anonymous client ID used to group opt-in submissions.
1516

1617
## How can I opt in or opt out of this data collection?
1718

@@ -21,7 +22,12 @@ The default configuration in the `python_ta` directory is `no` for both options.
2122
## How will the data be anonymised?
2223

2324
PyTA will not collect or send identifying information about you or your computer. (_Note_: if you choose to submit source files checked by PyTA, those files may contain identifying information about you.)
24-
PyTA does record a hash of your device's MAC address in order to identify when two runs come from the same device, but this is not used to deanonymize the collected data.
25+
PyTA does not derive its anonymous client ID from hardware identifiers such as your device's MAC address.
26+
Instead, when data is first submitted, PyTA generates a random ID and stores it locally.
27+
Future opt-in submissions include a hash of this random ID, allowing submissions to be grouped without sending the locally stored ID itself.
28+
By default, this is stored in the platform-specific user data directory for PythonTA, in a file named `anonymous_id`.
29+
You can override this directory by setting the `PYTHON_TA_DATA_DIR` environment variable.
30+
Deleting this file resets the anonymous ID.
2531

2632
## Who will the data be sent to?
2733

@@ -31,4 +37,4 @@ PyTA maintainers and computer science education researchers at the University of
3137
## How will this data be used?
3238

3339
This data will be used to better understand how PyTA is used by students for the purpose of making it a better educational tool.
34-
Potential research analyses of collected data include identifying common errors detected by PyTA and identifying errors that persist across multiple PyTA runs.
40+
Potential research analyses of collected data include identifying common errors detected by PyTA and identifying errors that persist across multiple PyTA runs associated with the same anonymous ID.

packages/python-ta/.coveragerc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ omit =
33
packages/python-ta/src/python_ta/debug/*
44
packages/python-ta/src/python_ta/reporters/templates/*
55
packages/python-ta/src/python_ta/util/*
6-
packages/python-ta/src/python_ta/upload.py
76
packages/python-ta/src/python_ta/utils.py
87

98
patch = subprocess

packages/python-ta/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,15 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
3232
- Fixed bug that allowed users to inject code into the browser template through the E9920 unnecessary f-string checker
3333
- Fixed bug that caused user input containing markdown characters to be rendered by the markdown renderer in certain error messages
3434
- Fixed memory leak issue that caused the memory usage to increase with each call to `python_ta.check_all()`.
35+
- Fixed opt-in data uploads to use a random anonymous ID instead of a MAC-address-derived hash, close uploaded files reliably, and time out stalled network requests.
36+
Existing opt-in users will receive a new anonymous client ID after upgrading.
3537
- Fixed webstepper build to include only a single file.
3638
- Fixed bug in `invalid_name_checker.py` that allowed three leading underscores in `_check_method_and_attr_name`
3739

3840
### 🔧 Internal changes
3941

4042
- Removed old documentation files under `python_ta/reporters/`
43+
- Added tests for the opt-in data upload path.
4144
- Added tests for `infinite_loop_checker.py` to improve coverage for the `_name_holds_generator` function and the generator portion of the `_check_constant_loop_cond` function.
4245
- Refactored `test_main.py` calls to use click's testing helpers.
4346
- The `Z3Visitor`, `Z3Parser`, and `Z3ParseException` classes have been extracted into a _new Python package_, `python-ta-z3`.

packages/python-ta/pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ dependencies = [
1717
"lsprotocol >= 2025.0.0",
1818
"markdown-it-py < 5",
1919
"mypy ~= 1.13",
20+
"platformdirs >= 4.0, < 5",
2021
"pycodestyle ~= 2.11",
2122
"pygments >= 2.14,< 2.21",
2223
"pylint ~= 4.0.3",

packages/python-ta/src/python_ta/upload.py

Lines changed: 101 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,96 +2,152 @@
22

33
import hashlib
44
import json
5+
import os
56
import uuid
6-
from typing import NamedTuple
7+
import warnings
8+
from contextlib import ExitStack
9+
from functools import cache
10+
from pathlib import Path
11+
from typing import TYPE_CHECKING, Any, Generator, Iterable
712

813
import requests
14+
from platformdirs import user_data_path
915

16+
if TYPE_CHECKING:
17+
from pylint.message import Message
1018

11-
def errors_to_dict(errors: list[NamedTuple]) -> dict[str, list[str]]:
12-
"""Convert PyTA errors from MessageSet format to a json format Dictionary."""
19+
UPLOAD_TIMEOUT_SECONDS = 5
20+
PYTHON_TA_DATA_DIR_ENV_VAR = "PYTHON_TA_DATA_DIR"
21+
22+
23+
def errors_to_dict(errors: Iterable[list[Message]]) -> dict[str, list[dict[str, Any]]]:
24+
"""Convert PyTA errors to a JSON-compatible dictionary."""
1325
error_info = ["msg_id", "msg", "symbol", "module", "category", "line"]
14-
error_types = ["code", "style"]
1526
err_as_dict = {}
16-
for msg_set in errors: # This iterates over the (filename, code, style) MessageSets
17-
for error_type in error_types: # This iterates over the code and style attributes
18-
current_type = getattr(msg_set, error_type) # Gets either the code or style dictionary
19-
for key in current_type.keys(): # Iterates over the error id's of caught errors
20-
err_as_dict[key] = []
21-
info_set = current_type.get(key)
22-
for (
23-
msg
24-
) in (
25-
info_set.messages
26-
): # Iterates over the messages for each error of the given code
27-
err_as_dict[key].append({k: getattr(msg, k) for k in error_info})
27+
for msg in _iter_error_messages(errors):
28+
err_as_dict.setdefault(msg.msg_id, []).append(
29+
{field: getattr(msg, field) for field in error_info}
30+
)
2831
return err_as_dict
2932

3033

3134
def upload_to_server(
32-
errors: list[NamedTuple], paths: list[str], config: dict[str, str], url: str, version: str
35+
errors: Iterable[list[Message]],
36+
paths: list[str],
37+
config: dict[str, Any],
38+
url: str,
39+
version: str,
3340
) -> None:
3441
"""Send POST request to server with formatted data."""
35-
unique_id = get_hashed_id() # Generates a device-specific ID
36-
files = []
37-
for path in paths:
38-
f = open(path)
39-
files.append(f)
40-
upload = {str(i): f for i, f in enumerate(files)} # requests.post() requires passing a dict
41-
# 'upload' is an empty dict in the case that 'files' is empty
42+
unique_id = _get_anonymous_id()
4243
errors_dict = errors_to_dict(errors)
4344
to_json = {"errors": errors_dict}
4445
if config: # 'config' is an empty dictionary if the default was used
4546
to_json["cfg"] = config
46-
payload = json.dumps(to_json)
47+
payload = json.dumps(to_json, default=str)
48+
4749
try:
48-
response = requests.post(
49-
url=url, files=upload, data={"id": unique_id, "version": version, "payload": payload}
50-
)
51-
for f in files:
52-
f.close()
50+
with ExitStack() as stack:
51+
upload = {str(i): stack.enter_context(open(path, "rb")) for i, path in enumerate(paths)}
52+
response = requests.post(
53+
url=url,
54+
files=upload,
55+
data={"id": unique_id, "version": version, "payload": payload},
56+
timeout=UPLOAD_TIMEOUT_SECONDS,
57+
)
5358
response.raise_for_status()
5459
print("[INFO] Upload successful")
5560
except requests.HTTPError as e:
5661
print("[ERROR] Upload failed")
57-
if e.response.status_code == 400:
62+
status_code = e.response.status_code if e.response is not None else None
63+
if status_code == 400:
5864
print(
5965
"[ERROR] HTTP Response Status 400: Client-side error, likely due to improper syntax. "
6066
"Please report this to your instructor (and attach the code that caused the error)."
6167
)
62-
elif e.response.status_code == 403:
68+
elif status_code == 403:
6369
print(
6470
"[ERROR] HTTP Response Status 403: Authorization is currently required for submission."
6571
)
66-
elif e.response.status_code == 500:
72+
elif status_code == 500:
6773
print(
6874
"[ERROR] HTTP Response Status 500: The server ran into a situation it doesn't know how to handle. "
6975
)
7076
print(
7177
"Please report this to your instructor (and attach the code that caused the error)."
7278
)
73-
elif e.response.status_code == 503:
79+
elif status_code == 503:
7480
print(
7581
"[ERROR] HTTP Response Status 503: The server is not ready to handle your request. "
7682
)
7783
print("It may be down for maintenance.")
7884
else:
7985
print('[ERROR] Error message: "{}"'.format(e))
80-
81-
except requests.ConnectionError as e:
86+
except requests.Timeout:
87+
print("[ERROR] Upload failed")
88+
print("[ERROR] Error message: Connection timed out. The server may be temporarily down.")
89+
except requests.ConnectionError:
8290
print("[ERROR] Upload failed")
8391
print(
84-
"[ERROR] Error message: Connection timed out. This may be caused by your firewall, or the server may be "
92+
"[ERROR] Error message: Could not connect. This may be caused by your firewall, or the server may be "
8593
"temporarily down."
8694
)
95+
except requests.RequestException as e:
96+
print("[ERROR] Upload failed")
97+
print('[ERROR] Error message: "{}"'.format(e))
98+
except OSError as e:
99+
print("[ERROR] Upload failed")
100+
print(f'[ERROR] Could not read a file selected for upload: "{e}"')
87101

88102

89-
def get_hashed_id() -> str:
90-
"""
91-
Generates a unique ID by hashing the user's mac-address.
103+
def _get_anonymous_id() -> str:
104+
"""Return an anonymous ID for opt-in data uploads.
105+
106+
This is a hash of a random local ID so multiple opt-in uploads can be
107+
grouped without deriving an identifier from hardware information.
92108
"""
93-
mac = str(uuid.uuid1())[24:]
94-
hash_gen = hashlib.sha512()
95-
encoded = mac.encode("utf-8")
96-
hash_gen.update(encoded)
97-
return hash_gen.hexdigest()
109+
local_anonymous_id = _get_or_create_local_anonymous_id(_get_anonymous_id_path())
110+
return hashlib.sha512(local_anonymous_id.encode("utf-8")).hexdigest()
111+
112+
113+
def get_hashed_id() -> str:
114+
"""Return the anonymous upload ID."""
115+
warnings.warn(
116+
"get_hashed_id is deprecated and should not be called directly.",
117+
DeprecationWarning,
118+
stacklevel=2,
119+
)
120+
return _get_anonymous_id()
121+
122+
123+
@cache
124+
def _get_or_create_local_anonymous_id(anonymous_id_path: Path) -> str:
125+
"""Return the random local ID used as input for the anonymous upload ID."""
126+
try:
127+
anonymous_id = anonymous_id_path.read_text(encoding="utf-8").strip()
128+
uuid.UUID(anonymous_id)
129+
return anonymous_id
130+
except (OSError, ValueError):
131+
anonymous_id = str(uuid.uuid4())
132+
133+
try:
134+
anonymous_id_path.parent.mkdir(parents=True, exist_ok=True)
135+
anonymous_id_path.write_text(anonymous_id + "\n", encoding="utf-8")
136+
print(f"[INFO] Saved anonymous ID to {anonymous_id_path}")
137+
except OSError:
138+
pass
139+
return anonymous_id
140+
141+
142+
def _iter_error_messages(errors: Iterable[list[Message]]) -> Generator[Message, None, None]:
143+
"""Yield individual messages from current reporter upload data."""
144+
for error_group in errors:
145+
yield from error_group
146+
147+
148+
def _get_anonymous_id_path() -> Path:
149+
"""Return the local path used to store the anonymous upload ID."""
150+
if PYTHON_TA_DATA_DIR_ENV_VAR in os.environ:
151+
return Path(os.environ[PYTHON_TA_DATA_DIR_ENV_VAR]).expanduser() / "anonymous_id"
152+
153+
return user_data_path("PythonTA", appauthor=False) / "anonymous_id"

0 commit comments

Comments
 (0)