fix(cli): guard lazy .hostname ValueError in extension/preset add --from

[Noor-ul-ain001]

What

Two CLI install paths validated a user-supplied --from URL by reading parsed.hostname outside their try/except ValueError guard:

• extension add --from <url> — extensions/_commands.py:434
• preset add --from <url> — presets/_commands.py (_is_allowed_download_url)

A bracketed-but-invalid IPv6 authority (e.g. https://[not-an-ip]/x.zip) parses cleanly under urlparse() on Python < 3.14 and only raises ValueError lazily on the first .hostname access. On the interpreters spec-kit supports (>=3.11), that raw ValueError leaked past the CLI, printing an uncaught traceback instead of the clean Invalid URL error. (The raise moved eager into urlparse() only in 3.14, so the pre-fix code happened to be safe there — but not on 3.11–3.13, i.e. CI.)

Same bug class as the maintainer-fixed #3433 / #3435 / #3437 / #3577 (unguarded urlparse().hostname leaking a raw ValueError).

Fix

• extensions: read parsed.hostname inside the existing try and reuse it for the localhost check.
• presets: guard the up-front urlparse(from_url).hostname read (preserving the existing Invalid URL message), and harden the nested _is_allowed_download_url to accept a URL string and parse + read .hostname inside its own try/except → returns False on malformed input. This also covers the redirect-validator and post-redirect final-URL checks, where the URL is server-controlled (a malicious redirect target).

Tests

For each command:

• a bracketed-non-IP --from URL exits cleanly (exit 1, no traceback);
• a monkeypatched lazy-.hostname raiser reproducing the pre-3.14 shape independently of the running interpreter. Test-the-test: both fail with a raw ValueError before the fix and pass after.

Full tests/test_extensions.py + tests/test_presets.py pass locally (751 passed, 7 pre-existing version/pwsh skips).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Defensively normalizes URL parsing failures in extension and preset installation paths.

Changes:

• Guards hostname extraction against ValueError.
• Hardens preset redirect URL validation.
• Adds malformed-URL regression tests.

Show a summary per file

File	Description
src/specify_cli/extensions/_commands.py	Guards hostname extraction.
src/specify_cli/presets/_commands.py	Hardens initial and redirected URL validation.
tests/test_extensions.py	Adds extension URL tests.
tests/test_presets.py	Adds preset URL tests.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (3)

src/specify_cli/presets/_commands.py:125

• This repeats an incorrect stdlib behavior claim. .hostname does not lazily validate bracketed IPv6 text in CPython 3.13.0, while 3.11/3.12 and current 3.13/3.14 reject this example during parsing. Keep the useful policy statement without asserting that version history.

                # Parse and read .hostname inside the try: a bracketed-but-invalid
                # IPv6 authority (e.g. "https://[not-an-ip]/p.zip") parses cleanly
                # under urlparse() on Python < 3.14 and only raises ValueError
                # lazily on the first .hostname access (eager at urlparse() on
                # 3.14+). A malformed URL is simply not an allowed download URL.

tests/test_extensions.py:6640

• The monkeypatch is a synthetic defensive case, not “the Python < 3.14 shape”: the supported CPython implementations do not defer bracket validation to .hostname. Calling this the exact production path also overstates what this test proves; please label it as simulated defensive coverage.

        """Simulate the Python < 3.14 shape explicitly (independent of the running
        interpreter): urlparse() succeeds but .hostname raises ValueError lazily.
        This is the exact path the fix guards; it leaks a raw ValueError if
        .hostname is read outside the try/except.

tests/test_presets.py:5611

• This monkeypatched object does not reproduce a pre-3.14 CPython behavior: those implementations either reject the bracketed host during parsing or return the extracted hostname without raising. Please describe it as synthetic defensive coverage rather than the exact production failure path.

        """Simulate the Python < 3.14 shape explicitly (independent of the running
        interpreter): urlparse() succeeds but .hostname raises ValueError lazily.
        This is the exact path the fix guards; it leaks a raw ValueError if
        .hostname is read outside the try/except.

• Files reviewed: 4/4 changed files
• Comments generated: 4
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Medium

[mnriem]

Please address Copilot feedback