From 56ea7d4671f29d9cf899b89a0c72b0df1e8c14ec Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 29 Jul 2026 12:16:21 +0500 Subject: [PATCH 1/2] fix: narrow bare except Exception in version fallback Replace overly broad except Exception with specific exception types: - importlib.metadata.PackageNotFoundError for missing package - (OSError, KeyError, ValueError) for pyproject.toml read/parse errors This prevents silently swallowing unexpected errors like AttributeError or RecursionError from broken tomllib or malformed pyproject.toml. --- src/specify_cli/_assets.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/_assets.py b/src/specify_cli/_assets.py index 31fb9708e6..9299506597 100644 --- a/src/specify_cli/_assets.py +++ b/src/specify_cli/_assets.py @@ -105,7 +105,7 @@ def get_speckit_version() -> str: """Get current spec-kit version.""" try: return importlib.metadata.version("specify-cli") - except Exception: + except importlib.metadata.PackageNotFoundError: # Fallback: try reading from pyproject.toml try: import tomllib @@ -114,8 +114,6 @@ def get_speckit_version() -> str: with open(pyproject_path, "rb") as f: data = tomllib.load(f) return data.get("project", {}).get("version", "unknown") - except Exception: - # Intentionally ignore any errors while reading/parsing pyproject.toml. - # If this lookup fails for any reason, we fall back to returning "unknown" below. + except (OSError, KeyError, ValueError): pass return "unknown" From a94b1f4925768b8b7767d15798d515686fac00a8 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 6 Oct 2026 22:59:39 +0500 Subject: [PATCH 2/2] fix(assets): guard the optional InvalidMetadataError in the version fallback importlib.metadata.version() can raise InvalidMetadataError for a malformed installed distribution, which is not a PackageNotFoundError, so the narrowed handler let it escape get_speckit_version() instead of using the pyproject/unknown fallback. Build the guarded exception tuple before the lookup, mirroring _version._get_installed_version(). The narrowed pyproject branch also called .get() on whatever the project key held, so a non-mapping value turned the fallback into an AttributeError; read it through isinstance checks instead. Regression tests cover both paths (corrupt metadata and a non-mapping project table) plus the pyproject fallback itself. Both new tests fail on the previous implementation: one with the escaping InvalidMetadataError, the other with AttributeError: 'int' object has no attribute 'get'. Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous) --- src/specify_cli/_assets.py | 19 +++++++++- tests/test_utils_assets_imports.py | 61 ++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/_assets.py b/src/specify_cli/_assets.py index 9299506597..acb7edd26a 100644 --- a/src/specify_cli/_assets.py +++ b/src/specify_cli/_assets.py @@ -103,9 +103,17 @@ def _locate_bundled_preset(preset_id: str) -> Path | None: def get_speckit_version() -> str: """Get current spec-kit version.""" + # Mirror _version._get_installed_version(): a malformed installed + # distribution raises InvalidMetadataError, which is not a + # PackageNotFoundError and must not escape the fallback path. + metadata_errors = [importlib.metadata.PackageNotFoundError] + invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None) + if invalid_metadata_error is not None: + metadata_errors.append(invalid_metadata_error) + try: return importlib.metadata.version("specify-cli") - except importlib.metadata.PackageNotFoundError: + except tuple(metadata_errors): # Fallback: try reading from pyproject.toml try: import tomllib @@ -113,7 +121,14 @@ def get_speckit_version() -> str: if pyproject_path.exists(): with open(pyproject_path, "rb") as f: data = tomllib.load(f) - return data.get("project", {}).get("version", "unknown") + project = data.get("project") if isinstance(data, dict) else None + # A present but non-mapping ``project`` table must not turn + # into an AttributeError from the narrowing of this branch. + if isinstance(project, dict): + version = project.get("version") + if isinstance(version, str) and version: + return version + return "unknown" except (OSError, KeyError, ValueError): pass return "unknown" diff --git a/tests/test_utils_assets_imports.py b/tests/test_utils_assets_imports.py index 8a41fa5e97..edacce6cfc 100644 --- a/tests/test_utils_assets_imports.py +++ b/tests/test_utils_assets_imports.py @@ -1,9 +1,12 @@ """Regression guard: utility and asset symbols importable from specify_cli.""" +import importlib.metadata + from specify_cli import ( check_tool, merge_json_files, get_speckit_version, CLAUDE_LOCAL_PATH, CLAUDE_NPM_LOCAL_PATH, ) +from specify_cli import _assets from pathlib import Path def test_utils_symbols_importable(): @@ -17,3 +20,61 @@ def test_get_speckit_version_returns_string(): def test_claude_paths_are_paths(): assert isinstance(CLAUDE_LOCAL_PATH, Path) assert isinstance(CLAUDE_NPM_LOCAL_PATH, Path) + + +def test_get_speckit_version_survives_invalid_metadata(monkeypatch): + """A corrupt installed distribution must fall back, not raise. + + ``InvalidMetadataError`` is not a ``PackageNotFoundError``, so catching + only the latter lets it escape the version fallback (the same guard + _version._get_installed_version() already applies). The class is looked up + dynamically and was removed from the stdlib in 3.14, so install a stand-in + to exercise the guard on every supported interpreter. + """ + + class _InvalidMetadataError(Exception): + pass + + monkeypatch.setattr( + importlib.metadata, "InvalidMetadataError", _InvalidMetadataError, raising=False + ) + + def _corrupt(name): + raise _InvalidMetadataError("corrupt metadata") + + monkeypatch.setattr(importlib.metadata, "version", _corrupt) + + assert isinstance(get_speckit_version(), str) + + +def test_get_speckit_version_survives_non_mapping_project(monkeypatch, tmp_path): + """A present but non-mapping ``project`` value must not raise. + + The narrowed pyproject branch previously called ``.get`` on whatever the + ``project`` key held, so ``project = 5`` turned the fallback into an + AttributeError. + """ + (tmp_path / "pyproject.toml").write_text("project = 5\n", encoding="utf-8") + monkeypatch.setattr(_assets, "_repo_root", lambda: tmp_path) + + def _not_found(name): + raise importlib.metadata.PackageNotFoundError(name) + + monkeypatch.setattr(importlib.metadata, "version", _not_found) + + assert get_speckit_version() == "unknown" + + +def test_get_speckit_version_reads_pyproject_fallback(monkeypatch, tmp_path): + """When the distribution is missing, a valid pyproject.toml is the source.""" + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "demo"\nversion = "9.9.9"\n', encoding="utf-8" + ) + monkeypatch.setattr(_assets, "_repo_root", lambda: tmp_path) + + def _not_found(name): + raise importlib.metadata.PackageNotFoundError(name) + + monkeypatch.setattr(importlib.metadata, "version", _not_found) + + assert get_speckit_version() == "9.9.9"