From 5786ec20af48edfd3a505b2f02652a6205e72c5c Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sat, 12 Sep 2026 17:22:31 -0500 Subject: [PATCH 1/2] Bandit audit cleanup --- .github/workflows/bandit.yml | 59 ++++++++++ CLAUDE.md | 106 ++++++++++++++++++ .../translations/run_translation_cycle.py | 20 +++- TelemetryPreferences.py | 3 +- bandit.yaml | 23 ++++ 5 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/bandit.yml create mode 100644 CLAUDE.md create mode 100644 bandit.yaml diff --git a/.github/workflows/bandit.yml b/.github/workflows/bandit.yml new file mode 100644 index 0000000..5d37787 --- /dev/null +++ b/.github/workflows/bandit.yml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# SPDX-FileNotice: Part of the Telemetry addon. + +################################################################################ +# # +# © 2026 FreeCAD Project Association # +# # +# This addon is free software: you can redistribute it and/or modify # +# it under the terms of the GNU Lesser General Public License as # +# published by the Free Software Foundation, either version 2.1 # +# of the License, or (at your option) any later version. # +# # +# This addon is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty # +# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. # +# See the GNU Lesser General Public License for more details. # +# # +# You should have received a copy of the GNU Lesser General Public # +# License along with this addon. If not, see https://www.gnu.org/licenses # +# # +################################################################################ + +# Static security analysis of the Telemetry addon with Bandit. Uses the same +# configuration as the FreeCAD addon quality report (FreeCAD/Addon-Reports). + +name: Bandit security scan + +permissions: + contents: read + +on: + push: + branches: + - main + pull_request: + branches: + - main + +jobs: + bandit: + name: Bandit + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: Install Bandit + run: | + python -m pip install --upgrade pip + pip install bandit + + - name: Run Bandit + run: bandit -c bandit.yaml -r . diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..d01c858 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,106 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +The FreeCAD **Telemetry** addon: an optional FreeCAD workbench (pure Python, no C++) that sends +non-identifying usage metrics to PostHog. It is installed into `{userAppDataDir}/Mod/Telemetry` and +loaded by FreeCAD at startup. It is not a real workbench — the `Telemetry` class in `InitGui.py` is an +empty shell that exists only so FreeCAD will load the addon; all work happens at import time. + +`README.md` and `PRIVACY_POLICY.md` are user-facing contracts about exactly what data is collected. +**Any change that adds a new collected field must be reflected in both**, per the project's stated +promise that no data is added to the collection list without informing the user. + +## Commands + +```bash +# Install dev deps (hash-pinned; same set CI installs) +pip install --require-hashes -r requirements.txt + +# Run all tests (must be run from the repo root) +python -m unittest discover Tests + +# Run a single test — note: plain `python -m unittest Tests.test_posthog...` FAILS because +# Tests/test_posthog.py does `import mocks`, which needs Tests/ on sys.path (discover adds it). +python -m unittest discover Tests -k test_posthog_launch_addon_enabled + +# Formatting (enforced by pre-commit / pre-commit.ci) +black --line-length 100 . +pre-commit run --all-files + +# Security lint (enforced by .github/workflows/bandit.yml; same config as FreeCAD/Addon-Reports). +# Must report zero issues. Fix findings, or add `# nosec BXXX` preceded by an +# `# Audited: (added nosec BXXX)` comment, as the AddonManager does. +pip install bandit +bandit -c bandit.yaml -r . +``` + +Dependency pinning: edit `requirements.in` (currently just `posthog`), then regenerate with +`pip-compile --generate-hashes`. Dependabot bumps `requirements.txt` automatically. + +Translations: see `Resources/translations/README.md`. `update_translation.sh -U` regenerates the +`.ts` template from `../../*.py ../panels/*.ui`; `run_translation_cycle.py` drives the full CrowdIn +round-trip. `update_translation.sh` hardcodes Windows Qt paths for `lupdate`/`lrelease` — override +`LUPDATE`/`LRELEASE` in the script for your system. + +## Architecture + +Load order is dictated by FreeCAD, not by us: + +- `Init.py` — console/non-GUI startup. Currently a no-op: the Sentry crash-reporting path + (`setup_sentry()` and the `Sentry.py` import) is deliberately commented out and unused. +- `InitGui.py` — GUI startup, the real entry point. Runs `setup()` at import time, which: + registers the preference page, shows the `first_start.ui` consent dialog on first run (writing the + four opt-in params), calls `posthog_launch()`, and wires `posthog_shutdown` to Qt's `aboutToQuit`. + The shutdown event is what lets the project infer crash rates (startup without matching shutdown). +- `PosthogFC.py` — all event capture. Five events: `freecad_startup`, `freecad_version`, + `freecad_system_info`, `freecad_preferences`, `freecad_addon_list`, plus `freecad_shutdown`. + Module-global `posthog` client and `posthog_id`. +- `TelemetryPreferences.py` — the preference page class (FreeCAD calls `loadSettings`/`saveSettings` + by name), plus the GDPR data-removal button which DELETEs + `https://www.freecad.org/deletetelemetry.php?person_id={uuid}`. +- `TelemetryPaths.py` — path constants for `Resources/{icons,panels,translations}`. +- `Sentry.py` — dormant. Kept for planned crash reporting; every call site is commented out. + +### State lives in FreeCAD's parameter system + +There is no config file. Everything is read/written through +`FreeCAD.ParamGet("User parameter:BaseApp/Preferences/Mod/Telemetry")`: +`Enable`, `SendSystemInformation`, `SendAddonInformation`, `SendPreferences`, `FirstStart`, +`PostHogUUID` (the random per-install identity), `PostHogURL`, `PostHogAPIKey`, `DSN`, `LastSendTime`. +Defaults are inlined at each call site rather than centralized, so a default changed in one file must +be changed in all of them. + +Gotcha: `InitGui.py` and `TelemetryPreferences.py` use the key **`Enable`**, while +`PosthogFC.posthog_launch()`/`posthog_shutdown()` check **`Enabled`** — a key the UI never writes, so +that inner check always falls through to its `True` default. The effective kill switch is the +`Enable` check in `InitGui.setup_posthog()`. Don't "fix" one side in isolation; changing the key +silently changes opt-out behavior for existing users. + +### Reading tracked preferences + +`posthog_preferences()` reads *FreeCAD's own* preferences (not the addon's) through the +`TrackedPreference` helpers in `PosthogFC.py`, which take a full `Group/Path/Name` string, a type, a +default, and an optional `transform`. `ui_panel_preferences()` additionally scans the +`BaseApp/MainWindow/DockWindows/Overlay{Left,Right,Top,Bottom}` groups to work out where a dock panel +is placed. Add new tracked preferences as entries in the `preferences` dict, not as new events. + +## Testing + +`Tests/mocks.py` provides `FreeCADMock` (a `MagicMock` subclass with a working `ParamGet`/parameter +dict) and `ParameterMock`. Tests never touch a real FreeCAD. The pattern in `Tests/test_posthog.py`: +patch `sys.modules` for `FreeCAD`, `FreeCADGui`, and `posthog` in `setUp`, pop them in `tearDown`, +then `import PosthogFC` *inside* the test body — the module raises on import outside FreeCAD, so the +import must happen after the patches are live. Seed behavior with +`self.fc.add_parameter_dict("User parameter:BaseApp/Preferences/Mod/Telemetry", {...})` and assert +against `MockPosthog.return_value.capture.call_args_list`. + +## Conventions + +- Black, line length 100. +- Every source file carries the SPDX header and the FPA LGPL-2.1-or-later boilerplate block; copy it + into new files. +- User-visible strings go through `FreeCAD.Qt.translate("Telemetry", ...)` so CrowdIn picks them up. +- Bump `` and `` in `package.xml` for a release; the Addon Manager reads it. diff --git a/Resources/translations/run_translation_cycle.py b/Resources/translations/run_translation_cycle.py index 7272a45..23fb1c0 100644 --- a/Resources/translations/run_translation_cycle.py +++ b/Resources/translations/run_translation_cycle.py @@ -34,7 +34,10 @@ import os import shutil import stat -import subprocess + +# Audited: subprocess is only used to run the Qt lupdate/lrelease tools with fixed argument lists +# and no shell (added nosec B404) +import subprocess # nosec B404 import sys import tempfile import time @@ -85,7 +88,8 @@ def _make_api_req(self, url, extra_headers=None, method="GET", data=None): data = json.dumps(data).encode("utf-8") request = Request(url, headers=headers, method=method, data=data) - request_result = urlopen(request) + # Audited: every URL is built on the hardcoded HTTPS CROWDIN_API_URL (added nosec B310) + request_result = urlopen(request) # nosec B310 if request_result.getcode() >= 300: print(f"Failed to make API request {url}: return code {request_result.getcode()}") raise Exception("Failed to make API request") @@ -135,7 +139,11 @@ def status(self): def download(self, build_id): filename = f"{self.project_identifier}.zip" response = self._make_project_api_req(f"/translations/builds/{build_id}/download") - urlretrieve(response["url"], filename) + download_url = response["url"] + if not download_url.startswith("https://"): + raise Exception(f"Refusing to download from non-HTTPS URL {download_url}") + # Audited: only HTTPS URLs reach this point (added nosec B310) + urlretrieve(download_url, filename) # nosec B310 print("download of " + filename + " complete") def build(self): @@ -185,7 +193,8 @@ def process_single_translation_file(source_path: str, target_path: str): print("Generating qm file for", basename, "...") try: - subprocess.run( + # Audited: fixed arguments, no shell; lrelease is expected on PATH (added nosec B603, B607) + subprocess.run( # nosec B603 B607 [ "lrelease", new_path, @@ -348,7 +357,8 @@ def run_and_download_build(crowdin_updater: CrowdinUpdater): "-ts", os.path.join(TS_FILE_PATH, CROWDIN_FILE_NAME), ] - result = subprocess.run( + # Audited: fixed arguments, no shell; lupdate is expected on PATH (added nosec B603) + result = subprocess.run( # nosec B603 args, timeout=30, check=True, diff --git a/TelemetryPreferences.py b/TelemetryPreferences.py index 175c02b..ad66497 100644 --- a/TelemetryPreferences.py +++ b/TelemetryPreferences.py @@ -156,7 +156,8 @@ def _remove_user_data() -> (bool, str): url = f"https://www.freecad.org/deletetelemetry.php?person_id={uuid}" req = urllib.request.Request(url, method="DELETE") try: - with urllib.request.urlopen(req) as response: + # Audited: the URL is a hardcoded HTTPS endpoint (added nosec B310) + with urllib.request.urlopen(req) as response: # nosec B310 if 200 <= response.status < 300: return True, FreeCAD.Qt.translate( "Telemetry", "Your user data was successfully removed from the database." diff --git a/bandit.yaml b/bandit.yaml new file mode 100644 index 0000000..8425c4e --- /dev/null +++ b/bandit.yaml @@ -0,0 +1,23 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# SPDX-FileNotice: Part of the Telemetry addon. + +# Configuration for Bandit static security analysis. Run locally with: +# bandit -c bandit.yaml -r . +# +# The settings mirror the ones used by the FreeCAD addon quality report +# (https://github.com/FreeCAD/Addon-Reports), so a clean run here means a clean +# entry there. Any finding must either be fixed or carry an audited "nosec" +# marker explaining why it is safe. +# +# B101 (assert_used) and B110 (try_except_pass) are informational style checks +# rather than security defects, and the addon report skips them as well. +# +# Bandit treats each exclude entry as a plain substring of the file path, so the +# patterns are anchored with path separators. + +exclude_dirs: + - "*/.venv/*" + +skips: + - B101 + - B110 From 8339f041b6a19a2a558963748989b75c149629e0 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sat, 12 Sep 2026 17:23:09 -0500 Subject: [PATCH 2/2] Increment patch version to 1.0.7 --- CLAUDE.md | 106 ---------------------------------------------------- package.xml | 2 +- 2 files changed, 1 insertion(+), 107 deletions(-) delete mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index d01c858..0000000 --- a/CLAUDE.md +++ /dev/null @@ -1,106 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## What this is - -The FreeCAD **Telemetry** addon: an optional FreeCAD workbench (pure Python, no C++) that sends -non-identifying usage metrics to PostHog. It is installed into `{userAppDataDir}/Mod/Telemetry` and -loaded by FreeCAD at startup. It is not a real workbench — the `Telemetry` class in `InitGui.py` is an -empty shell that exists only so FreeCAD will load the addon; all work happens at import time. - -`README.md` and `PRIVACY_POLICY.md` are user-facing contracts about exactly what data is collected. -**Any change that adds a new collected field must be reflected in both**, per the project's stated -promise that no data is added to the collection list without informing the user. - -## Commands - -```bash -# Install dev deps (hash-pinned; same set CI installs) -pip install --require-hashes -r requirements.txt - -# Run all tests (must be run from the repo root) -python -m unittest discover Tests - -# Run a single test — note: plain `python -m unittest Tests.test_posthog...` FAILS because -# Tests/test_posthog.py does `import mocks`, which needs Tests/ on sys.path (discover adds it). -python -m unittest discover Tests -k test_posthog_launch_addon_enabled - -# Formatting (enforced by pre-commit / pre-commit.ci) -black --line-length 100 . -pre-commit run --all-files - -# Security lint (enforced by .github/workflows/bandit.yml; same config as FreeCAD/Addon-Reports). -# Must report zero issues. Fix findings, or add `# nosec BXXX` preceded by an -# `# Audited: (added nosec BXXX)` comment, as the AddonManager does. -pip install bandit -bandit -c bandit.yaml -r . -``` - -Dependency pinning: edit `requirements.in` (currently just `posthog`), then regenerate with -`pip-compile --generate-hashes`. Dependabot bumps `requirements.txt` automatically. - -Translations: see `Resources/translations/README.md`. `update_translation.sh -U` regenerates the -`.ts` template from `../../*.py ../panels/*.ui`; `run_translation_cycle.py` drives the full CrowdIn -round-trip. `update_translation.sh` hardcodes Windows Qt paths for `lupdate`/`lrelease` — override -`LUPDATE`/`LRELEASE` in the script for your system. - -## Architecture - -Load order is dictated by FreeCAD, not by us: - -- `Init.py` — console/non-GUI startup. Currently a no-op: the Sentry crash-reporting path - (`setup_sentry()` and the `Sentry.py` import) is deliberately commented out and unused. -- `InitGui.py` — GUI startup, the real entry point. Runs `setup()` at import time, which: - registers the preference page, shows the `first_start.ui` consent dialog on first run (writing the - four opt-in params), calls `posthog_launch()`, and wires `posthog_shutdown` to Qt's `aboutToQuit`. - The shutdown event is what lets the project infer crash rates (startup without matching shutdown). -- `PosthogFC.py` — all event capture. Five events: `freecad_startup`, `freecad_version`, - `freecad_system_info`, `freecad_preferences`, `freecad_addon_list`, plus `freecad_shutdown`. - Module-global `posthog` client and `posthog_id`. -- `TelemetryPreferences.py` — the preference page class (FreeCAD calls `loadSettings`/`saveSettings` - by name), plus the GDPR data-removal button which DELETEs - `https://www.freecad.org/deletetelemetry.php?person_id={uuid}`. -- `TelemetryPaths.py` — path constants for `Resources/{icons,panels,translations}`. -- `Sentry.py` — dormant. Kept for planned crash reporting; every call site is commented out. - -### State lives in FreeCAD's parameter system - -There is no config file. Everything is read/written through -`FreeCAD.ParamGet("User parameter:BaseApp/Preferences/Mod/Telemetry")`: -`Enable`, `SendSystemInformation`, `SendAddonInformation`, `SendPreferences`, `FirstStart`, -`PostHogUUID` (the random per-install identity), `PostHogURL`, `PostHogAPIKey`, `DSN`, `LastSendTime`. -Defaults are inlined at each call site rather than centralized, so a default changed in one file must -be changed in all of them. - -Gotcha: `InitGui.py` and `TelemetryPreferences.py` use the key **`Enable`**, while -`PosthogFC.posthog_launch()`/`posthog_shutdown()` check **`Enabled`** — a key the UI never writes, so -that inner check always falls through to its `True` default. The effective kill switch is the -`Enable` check in `InitGui.setup_posthog()`. Don't "fix" one side in isolation; changing the key -silently changes opt-out behavior for existing users. - -### Reading tracked preferences - -`posthog_preferences()` reads *FreeCAD's own* preferences (not the addon's) through the -`TrackedPreference` helpers in `PosthogFC.py`, which take a full `Group/Path/Name` string, a type, a -default, and an optional `transform`. `ui_panel_preferences()` additionally scans the -`BaseApp/MainWindow/DockWindows/Overlay{Left,Right,Top,Bottom}` groups to work out where a dock panel -is placed. Add new tracked preferences as entries in the `preferences` dict, not as new events. - -## Testing - -`Tests/mocks.py` provides `FreeCADMock` (a `MagicMock` subclass with a working `ParamGet`/parameter -dict) and `ParameterMock`. Tests never touch a real FreeCAD. The pattern in `Tests/test_posthog.py`: -patch `sys.modules` for `FreeCAD`, `FreeCADGui`, and `posthog` in `setUp`, pop them in `tearDown`, -then `import PosthogFC` *inside* the test body — the module raises on import outside FreeCAD, so the -import must happen after the patches are live. Seed behavior with -`self.fc.add_parameter_dict("User parameter:BaseApp/Preferences/Mod/Telemetry", {...})` and assert -against `MockPosthog.return_value.capture.call_args_list`. - -## Conventions - -- Black, line length 100. -- Every source file carries the SPDX header and the FPA LGPL-2.1-or-later boilerplate block; copy it - into new files. -- User-visible strings go through `FreeCAD.Qt.translate("Telemetry", ...)` so CrowdIn picks them up. -- Bump `` and `` in `package.xml` for a release; the Addon Manager reads it. diff --git a/package.xml b/package.xml index eda413f..3240e95 100644 --- a/package.xml +++ b/package.xml @@ -2,7 +2,7 @@ Telemetry Help improve FreeCAD by sending basic metrics to the development team. - 1.0.6 + 1.0.7 2026-6-2 The FreeCAD project association AISBL LGPL-2.1-or-later