From e03c6f2c2867c2f880924e467de7e1b8a9ad95bb Mon Sep 17 00:00:00 2001 From: Thomas Juul Dyhr Date: Mon, 30 Mar 2026 07:49:17 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20stabilisation=20P0-P5=20=E2=80=94=20Home?= =?UTF-8?q?brew=20cmd,=20progress=20config,=20CLI=20drift,=20exception=20n?= =?UTF-8?q?arrowing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 (homebrew.py): - Replace shell-substitution command in get_all_homebrew_casks() with `brew info --json=v2 --eval-all --cask` via run_command_secure() (argv list). The old command used $(...) which shlex.split passes literally when shell=False. - Fix is_homebrew_available() to use get_brew_command() (configured path) instead of bare "brew --version". - Fix get_homebrew_path() except block: re-raise HomebrewError before the broad OSError fallback so it propagates correctly. - Update tests to assert exact argv shape, not just that mock was called. P1 (setup_handlers.py, outdated_handlers.py): - setup_handlers: replace _config["ui"]["show_progress"] mutation with config.set("no_progress", True) — the old write targeted a dead key; Config.show_progress reads _config["no_progress"], not _config["ui"][...]. - setup_handlers: replace remaining _config direct mutations with config.set() for no_color and adaptive_rate_limiting. - outdated_handlers: remove config.set("show_progress", False) — redundant since show_progress is derived from no_progress, not stored independently. - Update tests to assert config.set() calls instead of _config state. P2 (cli.py): - Add --output-file / dest=output_file to the Export Options group. brew_handlers.py accessed options.output_file unconditionally but the option had no parser backing, causing AttributeError on real CLI invocations when --export was used. P4 (homebrew.py, apps/finder.py, outdated_handlers.py): - Narrow except Exception in get_homebrew_path (HomebrewError + OSError). - Narrow async availability check in finder.py (AttributeError + RuntimeError). - Narrow get_applications parsing error (KeyError + IndexError + TypeError). - Narrow filter_out_brews fallback (ValueError + TypeError + AttributeError). - Top-level and setup handler catches remain broad — justified last resort. P5 (README.md, TODO.md): - Replace "Production-Ready" badge/heading with "Beta — Stabilising". - Update test count (1,885 → 2,173) and coverage claim (61% → 78%). - Rewrite Overview paragraph to reference stabilisation roadmap. - Rewrite TODO.md Active Work section with P0-P5 issue definitions, acceptance criteria, and verify commands from PROJECT_REVIEW.md. Co-Authored-By: Claude Sonnet 4.6 --- README.md | 18 +-- TODO.md | 142 +++++++++++++++---- tests/handlers/test_setup_handlers.py | 15 +- tests/test_homebrew_advanced.py | 37 +++-- tests/test_outdated_handlers.py | 5 +- versiontracker/apps/finder.py | 4 +- versiontracker/cli.py | 7 + versiontracker/handlers/outdated_handlers.py | 3 +- versiontracker/handlers/setup_handlers.py | 9 +- versiontracker/homebrew.py | 22 +-- 10 files changed, 180 insertions(+), 82 deletions(-) diff --git a/README.md b/README.md index b32fc13..d23d11d 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,10 @@
-## 🚀 Production-Ready macOS Application Manager +## macOS Application Version Manager -[![Project Grade](https://img.shields.io/badge/Grade-A-brightgreen?style=for-the-badge&logo=gradle&logoColor=white)](PROJECT_REVIEW.md) -[![Production Ready](https://img.shields.io/badge/Status-Production%20Ready-success?style=for-the-badge&logo=checkmarx&logoColor=white)](PROJECT_REVIEW.md) +[![Project Grade](https://img.shields.io/badge/Grade-B-blue?style=for-the-badge&logo=gradle&logoColor=white)](TODO.md) +[![Status](https://img.shields.io/badge/Status-Beta%20%E2%80%94%20Stabilising-orange?style=for-the-badge&logo=checkmarx&logoColor=white)](TODO.md)
@@ -36,7 +36,7 @@ [![Code Coverage](https://img.shields.io/codecov/c/github/docdyhr/versiontracker/master?logo=codecov&logoColor=white&label=Codecov)](https://codecov.io/gh/docdyhr/versiontracker) [![Test Coverage](https://img.shields.io/badge/Coverage-70%2B%25-brightgreen?logo=pytest&logoColor=white)](https://github.com/docdyhr/versiontracker) -[![Tests Passing](https://img.shields.io/badge/Tests-1%2C885%20Passing-success?logo=pytest&logoColor=white)](https://github.com/docdyhr/versiontracker/actions/workflows/ci.yml) +[![Tests Passing](https://img.shields.io/badge/Tests-2%2C173%20Passing-success?logo=pytest&logoColor=white)](https://github.com/docdyhr/versiontracker/actions/workflows/ci.yml) [![Security: Bandit](https://img.shields.io/badge/Bandit-Passing-success?logo=python&logoColor=white)](https://github.com/docdyhr/versiontracker/actions/workflows/security.yml) [![Security: pip-audit](https://img.shields.io/badge/pip--audit-No%20Vulnerabilities-success?logo=python&logoColor=white)](https://github.com/docdyhr/versiontracker/actions/workflows/security.yml) [![Security: Safety](https://img.shields.io/badge/Safety-No%20Vulnerabilities-success?logo=python&logoColor=white)](https://github.com/docdyhr/versiontracker/actions/workflows/security.yml) @@ -76,9 +76,8 @@ * Author: thomas * Purpose: CLI versiontracker and update tool for macOS * Release date: 21. Feb 2022 (Updated: March 2026) -* Code Quality: **~61% overall test coverage with 1,885+ passing tests**, - **all previously identified high & medium complexity issues resolved**, - **AI/ML capabilities and advanced analytics platform** +* Code Quality: **~78% overall test coverage with 2,173+ passing tests**, + **all previously identified high & medium complexity issues resolved** ## Quick Start @@ -105,8 +104,9 @@ versiontracker --help ## Overview Versiontracker is a command-line tool for macOS that helps you manage applications -installed outside of the App Store. Recently undergone complete technical debt cleanup -with **1,885+ passing tests** and **all high & medium-priority complexity issues resolved**. +installed outside of the App Store. Currently in active stabilisation (v0.9.x → v1.0): +core CLI/config/Homebrew paths are being made operationally consistent before the v1.0 release. +See [TODO.md](TODO.md) for the current stabilisation roadmap. It identifies applications that aren't managed through Apple's official channels and suggests which ones can be managed using Homebrew casks, making it easier to keep your applications up to date. diff --git a/TODO.md b/TODO.md index d50f166..749745e 100644 --- a/TODO.md +++ b/TODO.md @@ -4,8 +4,8 @@ ### Project Health -- **Version**: 0.9.0 -- **Tests**: 1,993 collected, 16 skipped +- **Version**: 0.9.0 (beta — stabilisation in progress) +- **Tests**: 2,173 collected, 16 skipped - **Coverage**: ~78% overall - **CI/CD**: All workflows passing on master (all green) - **Python Support**: 3.12+ (with 3.13 compatibility) @@ -16,49 +16,136 @@ ### Recent Completions -- ~~PR #108~~ **CodeQL security fixes** — 3 high-severity URL sanitization - alerts resolved in `verify_badges.py` (strict scheme+hostname allowlist); - 12 medium-severity missing-workflow-permissions alerts resolved across - `ci.yml`, `lint.yml`, `performance.yml`, `release.yml`, `security.yml` - (job-level `permissions:` blocks added to all jobs) -- ~~PR #106~~ **Dependency update** — `actions/upload-artifact` v6→v7, - `actions/download-artifact` v7→v8 across all workflows +- ~~PR #115~~ **CI badges + mypy** — test matrix badges, mypy consistency fix, CodeQL concurrency +- ~~PR #114~~ **Fuzzy matching + CI consolidation** — fallback fix, pipeline cleanup +- ~~PR #113~~ **Audit improvements** — dead code removal, plugin CLI, test coverage +- ~~PR #108~~ **CodeQL security fixes** — 3 high-severity URL sanitization alerts resolved; + 12 medium-severity missing-workflow-permissions alerts resolved +- ~~PR #106~~ **Dependency update** — `actions/upload-artifact` v6→v7, `actions/download-artifact` v7→v8 ### Previous Completions (v0.9.0) - ~~P10~~ **Async Homebrew wiring** — `check_brew_install_candidates()` and `check_brew_update_candidates()` now route through async Homebrew API by - default; deadlock bug in `async_check_brew_update_candidates` fixed; - automatic sync fallback on error; `get_casks_with_auto_updates()` deferred - to v0.10.x (no async equivalent yet) + default; deadlock bug in `async_check_brew_update_candidates` fixed - ~~P17~~ **Test coverage push** — 77 new handler/utility tests; coverage 61% → 78%; non-public modules excluded from metrics - ~~P9~~ **Config split** — extracted `ConfigLoader` class with static methods - for file I/O, env-var loading, brew detection, save, and - generate_default_config; `Config` simplified to data container + accessors -- ~~P15~~ **Test coverage improvement** — 122 new tests: - - `apps/matcher.py`: 54% → 98% - - `apps/finder.py`: 68% → 78% - - `config.py`: 43% → 68% + for file I/O, env-var loading, brew detection, save, and generate_default_config +- ~~P15~~ **Test coverage improvement** — 122 new tests (matcher 98%, finder 78%, config 68%) - ~~P1–P8, P11–P14~~ All completed in v0.8.2 (module migration, dead code removal, security fixes) --- -## Active Work — Prioritised Fix List +## Active Work — Stabilisation Cycle (v0.9.x → v1.0) -> Issues are ordered by impact. Work top-to-bottom. +> Objective: make the project operationally consistent before adding features. +> Work P0→P5 in order. Do not start P3/P4 before P0/P1 are stable. -### 🟢 P16 — Low: Remaining 16 Skipped Tests +### 🔴 P0 — Homebrew command execution contract + +**Problem**: `get_all_homebrew_casks()` builds a command using shell substitution +(`$(ls $(brew --repository)/...)`), but `run_command` executes with `shell=False` via +`shlex.split`. The substitution is never evaluated. +Also: `is_homebrew_available()` uses bare `"brew"` instead of the configured path. + +**Files**: `versiontracker/homebrew.py`, `tests/test_homebrew_advanced.py` + +- [ ] Replace shell-substitution command with `brew info --json=v2 --eval-all --cask` +- [ ] Use `run_command_secure()` (argv list) in `get_all_homebrew_casks()` +- [ ] Fix `is_homebrew_available()` to use configured brew path +- [ ] Update tests to assert exact command/argv shape + +**Verify**: `pytest -q tests/test_homebrew_advanced.py tests/test_homebrew.py` + +--- + +### 🔴 P1 — Progress flag canonicalisation + +**Problem**: `--no-progress` is stored in inconsistent config locations. +`setup_handlers.py` writes to `_config["ui"]["show_progress"]` but +`Config.show_progress` is derived from `_config["no_progress"]` — the write +has no effect. `outdated_handlers.py` calls `config.set("show_progress", False)` +which is also a dead key. + +**Files**: `versiontracker/handlers/setup_handlers.py`, +`versiontracker/handlers/outdated_handlers.py` + +- [ ] `setup_handlers.py`: replace `_config["ui"]["show_progress"]` mutation with `config.set("no_progress", True)` +- [ ] `setup_handlers.py`: replace other `_config[...]` mutations with `config.set()` calls +- [ ] `outdated_handlers.py`: remove dead `config.set("show_progress", False)` call +- [ ] Regression tests for `--no-progress` across `--apps` and `--outdated` + +**Verify**: `pytest -q tests/handlers/test_setup_handlers.py tests/test_outdated_handlers.py tests/test_cli.py` + +--- + +### 🟡 P2 — CLI/handler option drift + +**Problem**: Some handler branches access `options` attributes not backed by the +parser (e.g., `options.output_file`, `options.notify`). Currently guarded by +`hasattr`, so they don't crash, but are dead paths. + +**Files**: `versiontracker/cli.py`, `versiontracker/handlers/outdated_handlers.py` + +- [ ] Audit every `options.` in handlers and `__main__.py` +- [ ] For each ungated access: add parser argument or remove the branch +- [ ] For each `hasattr`-gated dead path: evaluate adding to CLI or removing +- [ ] Update help text to reflect actual CLI surface + +**Verify**: `pytest -q tests/test_cli.py tests/test_main.py tests/test_outdated_handlers.py` + +--- + +### 🟡 P3 — Import-time side effects (defer until P0/P1 stable) + +**Problem**: Config singleton creation at import time triggers Homebrew detection +and env inspection before CLI args are parsed. + +**Files**: `versiontracker/config.py`, `versiontracker/__main__.py` + +- [ ] Delay expensive config initialisation until CLI startup +- [ ] Minimise subprocess/filesystem work at import time + +**Verify**: `pytest -q tests/test_config.py tests/test_main.py tests/test_integration.py` + +--- + +### 🟡 P4 — Exception narrowing + +**Problem**: Broad `except Exception` blocks in core modules mask root causes. + +**Files** (start here): `versiontracker/homebrew.py`, `versiontracker/apps/finder.py`, +`versiontracker/__main__.py`, `versiontracker/handlers/setup_handlers.py`, +`versiontracker/handlers/outdated_handlers.py` + +- [ ] Replace broad `except Exception` with specific exception types where feasible +- [ ] Preserve diagnostic detail in logs +- [ ] Cover expected failure classes in tests + +**Verify**: `pytest -q tests/test_homebrew_advanced.py tests/test_outdated_handlers.py tests/test_integration.py` + +--- + +### 🟢 P5 — Documentation alignment + +**Problem**: README presents project as more mature than current code supports. + +- [ ] Replace inflated maturity language with accurate beta/stabilisation wording +- [ ] Ensure CHANGELOG reflects any user-visible behaviour changes from P0–P4 +- [ ] Remove `PROJECT_REVIEW.md` from repo root after stabilisation completes + +--- + +### 🟢 P16 — Remaining 16 skipped tests (low priority) | File | Count | Root Cause | Action | |---|---|---|---| | `test_ui.py` | 12 | Environment-specific terminal/colour | Leave as-is | -| `test_platform_compatibility.py` | 2 | macOS-only / non-macOS guards | Leave as-is | -| `test_ui_new.py` | 1 | Environment-specific colour handling | Leave as-is | +| `test_platform_compatibility.py` | 2 | macOS-only guards | Leave as-is | +| `test_ui_new.py` | 1 | Environment-specific colour | Leave as-is | | `test_apps_extra.py` | 1 | Complex mocking requirements | Consider fixing | -All skips are environment-specific or CI-specific — no action needed for most. - --- ## Homebrew Release (v0.9.0) — Complete @@ -72,7 +159,7 @@ All skips are environment-specific or CI-specific — no action needed for most. --- -## Future Enhancements +## Future Enhancements (post-stabilisation) ### Extended Package Manager Support @@ -102,6 +189,7 @@ All skips are environment-specific or CI-specific — no action needed for most. - [ ] Enhance ML-powered recommendations with user feedback loop - [ ] Usage pattern analysis for personalised suggestions - [ ] Confidence scoring improvements for app-cask matching +- [ ] Async wiring for `get_casks_with_auto_updates()` (deferred from P10) --- @@ -121,7 +209,7 @@ For detailed strategic planning see `docs/future_roadmap.md`. ### Advanced Contributions - MacPorts integration -- Async wiring for `get_casks_with_auto_updates()` (deferred from P10) +- P3: Lazy config initialisation --- diff --git a/tests/handlers/test_setup_handlers.py b/tests/handlers/test_setup_handlers.py index 42ef00c..d6d5d94 100644 --- a/tests/handlers/test_setup_handlers.py +++ b/tests/handlers/test_setup_handlers.py @@ -90,23 +90,16 @@ def test_handle_configure_from_options_success(self, mock_logging, mock_get_conf mock_options.no_adaptive_rate = True mock_config = mock.MagicMock() - mock_config._config = { - "ui": { - "use_color": True, - "show_progress": True, - "adaptive_rate_limiting": True, - } - } mock_get_config.return_value = mock_config # Execute result = handle_configure_from_options(mock_options) - # Assert + # Assert config.set() was called with the canonical keys assert result == 0 - assert mock_config._config["ui"]["use_color"] is False - assert mock_config._config["ui"]["show_progress"] is False - assert mock_config._config["ui"]["adaptive_rate_limiting"] is False + mock_config.set.assert_any_call("ui.use_color", False) + mock_config.set.assert_any_call("no_progress", True) + mock_config.set.assert_any_call("ui.adaptive_rate_limiting", False) @mock.patch("versiontracker.handlers.setup_handlers.get_config") @mock.patch("versiontracker.handlers.setup_handlers.logging") diff --git a/tests/test_homebrew_advanced.py b/tests/test_homebrew_advanced.py index caf0e4d..6299165 100644 --- a/tests/test_homebrew_advanced.py +++ b/tests/test_homebrew_advanced.py @@ -46,16 +46,21 @@ def tearDown(self): # Clear the test cache self.test_cache.clear() - @patch("versiontracker.homebrew.run_command") - def test_is_homebrew_available_success(self, mock_run_command): + @patch("versiontracker.homebrew.run_command_secure") + @patch("versiontracker.homebrew.get_brew_command") + def test_is_homebrew_available_success(self, mock_get_brew_command, mock_run_command_secure): """Test checking Homebrew availability when it is available.""" - mock_run_command.return_value = ("Homebrew 3.6.0", 0) + mock_get_brew_command.return_value = "/opt/homebrew/bin/brew" + mock_run_command_secure.return_value = ("Homebrew 4.0.0", 0) self.assertTrue(is_homebrew_available()) + mock_run_command_secure.assert_called_once_with(["/opt/homebrew/bin/brew", "--version"], timeout=5) - @patch("versiontracker.homebrew.run_command") - def test_is_homebrew_available_failure(self, mock_run_command): + @patch("versiontracker.homebrew.run_command_secure") + @patch("versiontracker.homebrew.get_brew_command") + def test_is_homebrew_available_failure(self, mock_get_brew_command, mock_run_command_secure): """Test checking Homebrew availability when it is not available.""" - mock_run_command.return_value = ("Command not found", 1) + mock_get_brew_command.return_value = "/opt/homebrew/bin/brew" + mock_run_command_secure.return_value = ("", 1) self.assertFalse(is_homebrew_available()) @patch("versiontracker.homebrew.run_command") @@ -130,24 +135,26 @@ def test_get_all_homebrew_casks_cached(self, mock_run_command, mock_get_homebrew mock_run_command.assert_not_called() mock_get_homebrew_path.assert_not_called() - @patch("versiontracker.homebrew.get_homebrew_path") - @patch("versiontracker.homebrew.run_command") - def test_get_all_homebrew_casks_uncached(self, mock_run_command, mock_get_homebrew_path): + @patch("versiontracker.homebrew.run_command_secure") + @patch("versiontracker.homebrew.get_brew_command") + def test_get_all_homebrew_casks_uncached(self, mock_get_brew_command, mock_run_command_secure): """Test getting all Homebrew casks when they are not cached.""" - # Setup mocks - mock_get_homebrew_path.return_value = "/usr/local/bin/brew" + brew_path = "/usr/local/bin/brew" + mock_get_brew_command.return_value = brew_path test_casks = [ {"token": "firefox", "name": "Firefox", "version": "100.0"}, {"token": "chrome", "name": "Google Chrome", "version": "90.0"}, ] - mock_run_command.return_value = (json.dumps({"casks": test_casks}), 0) + mock_run_command_secure.return_value = (json.dumps({"casks": test_casks}), 0) - # Test function result = get_all_homebrew_casks() - # Verify result self.assertEqual(result, test_casks) - mock_run_command.assert_called_once() + # Assert exact command shape — no shell substitution + mock_run_command_secure.assert_called_once_with( + [brew_path, "info", "--json=v2", "--eval-all", "--cask"], + timeout=120, + ) # Verify cache was updated cached = self.test_cache.get("homebrew:all_casks") diff --git a/tests/test_outdated_handlers.py b/tests/test_outdated_handlers.py index 5c066d4..a8986a1 100644 --- a/tests/test_outdated_handlers.py +++ b/tests/test_outdated_handlers.py @@ -33,8 +33,7 @@ def test_update_config_with_no_progress_true(self, mock_get_config): _update_config_from_options(options) - mock_config.set.assert_any_call("no_progress", True) - mock_config.set.assert_any_call("show_progress", False) + mock_config.set.assert_called_once_with("no_progress", True) @patch("versiontracker.handlers.outdated_handlers.get_config") def test_update_config_with_no_progress_false(self, mock_get_config): @@ -209,7 +208,7 @@ def test_filter_applications_filter_error(self, mock_progress, mock_filter): """Test handling error during filtering.""" apps = [("App1", "1.0"), ("App2", "2.0")] brews = ["app1"] - mock_filter.side_effect = Exception("Filter error") + mock_filter.side_effect = ValueError("Filter error") mock_progress.return_value.color.return_value = lambda x: x result = _filter_applications(apps, brews, include_brews=False) diff --git a/versiontracker/apps/finder.py b/versiontracker/apps/finder.py index d12db75..a734600 100644 --- a/versiontracker/apps/finder.py +++ b/versiontracker/apps/finder.py @@ -105,7 +105,7 @@ def _is_async_homebrew_available() -> bool: logging.debug("Async Homebrew module not available: %s", e) _async_homebrew_available = False return False - except Exception as e: + except (AttributeError, RuntimeError) as e: logging.warning("Error checking async Homebrew availability: %s", e) _async_homebrew_available = False return False @@ -258,7 +258,7 @@ def get_applications_from_system_profiler( apps_list.append((app_name, version)) return apps_list - except Exception as e: + except (KeyError, IndexError, TypeError) as e: logging.error("Error parsing application data: %s", e) raise DataParsingError(f"Error parsing application data: {e}") from e diff --git a/versiontracker/cli.py b/versiontracker/cli.py index cb8bd81..f12c76b 100644 --- a/versiontracker/cli.py +++ b/versiontracker/cli.py @@ -168,6 +168,13 @@ def get_arguments() -> Any: metavar="FORMAT", help="Export results in specified format (json, yaml, csv)", ) + export_group.add_argument( + "--output-file", + dest="output_file", + type=str, + metavar="PATH", + help="Write export output to file instead of stdout (requires --export)", + ) # Filter management filter_group = parser.add_argument_group("Filter Management") diff --git a/versiontracker/handlers/outdated_handlers.py b/versiontracker/handlers/outdated_handlers.py index 4d967a1..f7317d5 100644 --- a/versiontracker/handlers/outdated_handlers.py +++ b/versiontracker/handlers/outdated_handlers.py @@ -54,7 +54,6 @@ def _update_config_from_options(options: Any) -> None: config = get_config() if hasattr(config, "set"): config.set("no_progress", True) - config.set("show_progress", False) def _get_installed_applications() -> list[tuple[str, str]]: @@ -109,7 +108,7 @@ def _filter_applications(apps: list[tuple[str, str]], brews: list[str], include_ if not include_brews: try: return filter_out_brews(apps, brews) - except Exception as e: + except (ValueError, TypeError, AttributeError) as e: print(create_progress_bar().color("yellow")(f"Warning: Error filtering applications: {e}")) print(create_progress_bar().color("yellow")("Proceeding with all applications.")) return apps diff --git a/versiontracker/handlers/setup_handlers.py b/versiontracker/handlers/setup_handlers.py index 98bb587..c5a15d6 100644 --- a/versiontracker/handlers/setup_handlers.py +++ b/versiontracker/handlers/setup_handlers.py @@ -35,7 +35,7 @@ def handle_initialize_config(options: Any) -> int: if not hasattr(get_config(), "_config"): # Create a new Config instance if needed Config(config_file=config_file) - except Exception as e: + except (OSError, ValueError) as e: logging.debug("Config initialization error: %s", e) # Create a new Config instance with defaults Config() @@ -61,13 +61,14 @@ def handle_configure_from_options(options: Any) -> int: # Configure UI options from command-line arguments if hasattr(options, "no_color") and options.no_color: - current_config._config["ui"]["use_color"] = False + current_config.set("ui.use_color", False) if hasattr(options, "no_progress") and options.no_progress: - current_config._config["ui"]["show_progress"] = False + # no_progress is the canonical key; Config.show_progress is derived from it + current_config.set("no_progress", True) if hasattr(options, "no_adaptive_rate") and options.no_adaptive_rate: - current_config._config["ui"]["adaptive_rate_limiting"] = False + current_config.set("ui.adaptive_rate_limiting", False) return 0 except Exception as e: diff --git a/versiontracker/homebrew.py b/versiontracker/homebrew.py index 8d77dc5..6be7e5c 100644 --- a/versiontracker/homebrew.py +++ b/versiontracker/homebrew.py @@ -21,7 +21,7 @@ from versiontracker.config import get_config from versiontracker.exceptions import DataParsingError, HomebrewError, NetworkError from versiontracker.ui import create_progress_bar -from versiontracker.utils import run_command +from versiontracker.utils import run_command, run_command_secure # Cache keys for different Homebrew operations CACHE_KEY_ALL_CASKS = "homebrew:all_casks" @@ -45,10 +45,12 @@ def is_homebrew_available() -> bool: bool: True if Homebrew is available """ try: - # Try to run brew --version - stdout, returncode = run_command("brew --version", timeout=5) + brew_path = get_brew_command() + stdout, returncode = run_command_secure([brew_path, "--version"], timeout=5) return returncode == 0 - except Exception as e: + except HomebrewError: + return False + except OSError as e: logging.warning("Homebrew availability check failed: %s", e) return False @@ -80,7 +82,9 @@ def get_homebrew_path() -> str: return stdout.strip() raise HomebrewError("Homebrew not found in common locations") - except Exception as e: + except HomebrewError: + raise + except OSError as e: logging.error("Failed to find Homebrew: %s", e) raise HomebrewError(f"Homebrew not found: {e}") from e @@ -122,16 +126,16 @@ def get_all_homebrew_casks() -> list[dict[str, Any]]: try: brew_path = get_brew_command() - command = ( - f"{brew_path} info --json=v2 --cask $(ls $(brew --repository)/Library/Taps/homebrew/homebrew-cask/Casks/)" - ) + # Use --eval-all to enumerate all known casks without shell substitution. + # Requires Homebrew 4.0+ (default since 2023). + cmd_args = [brew_path, "info", "--json=v2", "--eval-all", "--cask"] # Show progress message progress_bar = create_progress_bar() print(progress_bar.color("blue")("Fetching all Homebrew casks (this may take a while)...")) # Execute command with timeout - stdout, returncode = run_command(command, timeout=120) # 2 minute timeout + stdout, returncode = run_command_secure(cmd_args, timeout=120) # 2 minute timeout if returncode != 0: error_msg = f"Failed to retrieve Homebrew casks: {stdout}"