From 29fce54ab4c9a3aea2c8dabce96e784d66d2c790 Mon Sep 17 00:00:00 2001 From: yxshee <84263676+yxshee@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:59:17 +0530 Subject: [PATCH 1/2] fix(catalogs): name a scheme-less catalog URL instead of reporting "got ://" Extension, integration, preset, workflow and step catalog URL validation built the HTTPS error from parsed.scheme, so a value without a scheme such as example.com/catalog.json was reported as "(got ://)". Show the value itself when there is no scheme. --- src/specify_cli/catalogs.py | 4 +++- src/specify_cli/presets/_catalog.py | 4 +++- src/specify_cli/workflows/catalog/_domain.py | 4 +++- src/specify_cli/workflows/step/catalog/_domain.py | 4 +++- tests/specify_cli/integrations/test_catalog.py | 6 ++++++ tests/specify_cli/presets/test_catalog.py | 6 ++++++ tests/test_workflows.py | 11 +++++++++++ 7 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/catalogs.py b/src/specify_cli/catalogs.py index 323bb4f740..477fb663aa 100644 --- a/src/specify_cli/catalogs.py +++ b/src/specify_cli/catalogs.py @@ -85,8 +85,10 @@ def _validate_catalog_url(cls, url: str) -> None: raise cls._error(f"Catalog URL is malformed: {url}") from None is_localhost = hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): + # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". + got = f"{parsed.scheme}://" if parsed.scheme else repr(url) raise cls._error( - f"Catalog URL must use HTTPS (got {parsed.scheme}://). " + f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." ) # Check hostname, not netloc: netloc is truthy for host-less URLs like diff --git a/src/specify_cli/presets/_catalog.py b/src/specify_cli/presets/_catalog.py index 7f99443551..68efa65016 100644 --- a/src/specify_cli/presets/_catalog.py +++ b/src/specify_cli/presets/_catalog.py @@ -133,8 +133,10 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): + # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". + got = f"{parsed.scheme}://" if parsed.scheme else repr(url) raise PresetValidationError( - f"Catalog URL must use HTTPS (got {parsed.scheme}://). " + f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." ) # Check hostname, not netloc: netloc is truthy for host-less URLs like diff --git a/src/specify_cli/workflows/catalog/_domain.py b/src/specify_cli/workflows/catalog/_domain.py index 4aaf82532a..48c762caa4 100644 --- a/src/specify_cli/workflows/catalog/_domain.py +++ b/src/specify_cli/workflows/catalog/_domain.py @@ -350,8 +350,10 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): + # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". + got = f"{parsed.scheme}://" if parsed.scheme else repr(url) raise WorkflowValidationError( - f"Catalog URL must use HTTPS (got {parsed.scheme}://). " + f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." ) if not hostname: diff --git a/src/specify_cli/workflows/step/catalog/_domain.py b/src/specify_cli/workflows/step/catalog/_domain.py index 1f0e2503d8..99f745f0d3 100644 --- a/src/specify_cli/workflows/step/catalog/_domain.py +++ b/src/specify_cli/workflows/step/catalog/_domain.py @@ -298,8 +298,10 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): + # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". + got = f"{parsed.scheme}://" if parsed.scheme else repr(url) raise StepValidationError( - f"Catalog URL must use HTTPS (got {parsed.scheme}://). " + f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." ) if not hostname: diff --git a/tests/specify_cli/integrations/test_catalog.py b/tests/specify_cli/integrations/test_catalog.py index 876edf8f42..3bcb3f91d2 100644 --- a/tests/specify_cli/integrations/test_catalog.py +++ b/tests/specify_cli/integrations/test_catalog.py @@ -85,6 +85,12 @@ def test_http_rejected(self): with pytest.raises(IntegrationCatalogError, match="HTTPS"): IntegrationCatalog._validate_catalog_url("http://example.com/catalog.json") + def test_schemeless_url_names_the_value(self): + with pytest.raises(IntegrationCatalogError) as excinfo: + IntegrationCatalog._validate_catalog_url("example.com/catalog.json") + assert "(got 'example.com/catalog.json')" in str(excinfo.value) + assert "got ://" not in str(excinfo.value) + def test_http_localhost_allowed(self): IntegrationCatalog._validate_catalog_url("http://localhost:8080/catalog.json") IntegrationCatalog._validate_catalog_url("http://127.0.0.1/catalog.json") diff --git a/tests/specify_cli/presets/test_catalog.py b/tests/specify_cli/presets/test_catalog.py index 6d50da9ca1..98cbefb130 100644 --- a/tests/specify_cli/presets/test_catalog.py +++ b/tests/specify_cli/presets/test_catalog.py @@ -187,6 +187,12 @@ def test_validate_catalog_url_http_rejected(self, project_dir): with pytest.raises(PresetValidationError, match="must use HTTPS"): catalog._validate_catalog_url("http://example.com/catalog.json") + def test_validate_catalog_url_schemeless_names_the_value(self, project_dir): + """A URL without a scheme is named instead of reported as "got ://".""" + catalog = PresetCatalog(project_dir) + with pytest.raises(PresetValidationError, match=r"\(got 'example\.com/catalog\.json'\)"): + catalog._validate_catalog_url("example.com/catalog.json") + def test_validate_catalog_url_localhost_http_allowed(self, project_dir): """Test that HTTP is allowed for localhost.""" catalog = PresetCatalog(project_dir) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e54d985f53..abf4876138 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -10349,6 +10349,17 @@ def test_validate_url_http_rejected(self, project_dir): with pytest.raises(WorkflowValidationError, match="HTTPS"): catalog._validate_catalog_url("http://evil.com/catalog.json") + def test_validate_url_schemeless_names_the_value(self, project_dir): + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowValidationError + from specify_cli.workflows.step.catalog import StepCatalog, StepValidationError + + for catalog, error in ( + (WorkflowCatalog(project_dir), WorkflowValidationError), + (StepCatalog(project_dir), StepValidationError), + ): + with pytest.raises(error, match=r"\(got 'example\.com/catalog\.json'\)"): + catalog._validate_catalog_url("example.com/catalog.json") + def test_validate_url_localhost_http_allowed(self, project_dir): from specify_cli.workflows.catalog import WorkflowCatalog From 1fe145d20c39de2dedd507dcc8421410ca0e6709 Mon Sep 17 00:00:00 2001 From: yxshee <84263676+yxshee@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:56:34 +0530 Subject: [PATCH 2/2] fix(catalogs): describe a scheme-less URL without echoing it into Rich output Review: integration, workflow and step catalog commands print validation errors as Rich markup, so echoing the user value let input such as "[/red]x" raise MarkupError. Say "a URL with no scheme" instead. --- src/specify_cli/catalogs.py | 5 +++-- src/specify_cli/presets/_catalog.py | 5 +++-- src/specify_cli/workflows/catalog/_domain.py | 5 +++-- src/specify_cli/workflows/step/catalog/_domain.py | 5 +++-- tests/specify_cli/integrations/test_catalog.py | 2 +- tests/specify_cli/presets/test_catalog.py | 2 +- tests/test_workflows.py | 2 +- 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/catalogs.py b/src/specify_cli/catalogs.py index 477fb663aa..9e9fc96d6a 100644 --- a/src/specify_cli/catalogs.py +++ b/src/specify_cli/catalogs.py @@ -85,8 +85,9 @@ def _validate_catalog_url(cls, url: str) -> None: raise cls._error(f"Catalog URL is malformed: {url}") from None is_localhost = hostname in ("localhost", "127.0.0.1", "::1") if parsed.scheme != "https" and not (parsed.scheme == "http" and is_localhost): - # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". - got = f"{parsed.scheme}://" if parsed.scheme else repr(url) + # A scheme-less value (``example.com/catalog.json``) would otherwise read + # "got ://". The value itself is not echoed: callers render errors as Rich markup. + got = f"{parsed.scheme}://" if parsed.scheme else "a URL with no scheme" raise cls._error( f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." diff --git a/src/specify_cli/presets/_catalog.py b/src/specify_cli/presets/_catalog.py index 68efa65016..a55e7d11e3 100644 --- a/src/specify_cli/presets/_catalog.py +++ b/src/specify_cli/presets/_catalog.py @@ -133,8 +133,9 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): - # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". - got = f"{parsed.scheme}://" if parsed.scheme else repr(url) + # A scheme-less value (``example.com/catalog.json``) would otherwise read + # "got ://". The value itself is not echoed: callers render errors as Rich markup. + got = f"{parsed.scheme}://" if parsed.scheme else "a URL with no scheme" raise PresetValidationError( f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." diff --git a/src/specify_cli/workflows/catalog/_domain.py b/src/specify_cli/workflows/catalog/_domain.py index 48c762caa4..29717e1556 100644 --- a/src/specify_cli/workflows/catalog/_domain.py +++ b/src/specify_cli/workflows/catalog/_domain.py @@ -350,8 +350,9 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): - # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". - got = f"{parsed.scheme}://" if parsed.scheme else repr(url) + # A scheme-less value (``example.com/catalog.json``) would otherwise read + # "got ://". The value itself is not echoed: callers render errors as Rich markup. + got = f"{parsed.scheme}://" if parsed.scheme else "a URL with no scheme" raise WorkflowValidationError( f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." diff --git a/src/specify_cli/workflows/step/catalog/_domain.py b/src/specify_cli/workflows/step/catalog/_domain.py index 99f745f0d3..50e79daa47 100644 --- a/src/specify_cli/workflows/step/catalog/_domain.py +++ b/src/specify_cli/workflows/step/catalog/_domain.py @@ -298,8 +298,9 @@ def _validate_catalog_url(self, url: str) -> None: if parsed.scheme != "https" and not ( parsed.scheme == "http" and is_localhost ): - # A scheme-less value (``example.com/catalog.json``) would otherwise read "got ://". - got = f"{parsed.scheme}://" if parsed.scheme else repr(url) + # A scheme-less value (``example.com/catalog.json``) would otherwise read + # "got ://". The value itself is not echoed: callers render errors as Rich markup. + got = f"{parsed.scheme}://" if parsed.scheme else "a URL with no scheme" raise StepValidationError( f"Catalog URL must use HTTPS (got {got}). " "HTTP is only allowed for localhost." diff --git a/tests/specify_cli/integrations/test_catalog.py b/tests/specify_cli/integrations/test_catalog.py index 3bcb3f91d2..81d217eec1 100644 --- a/tests/specify_cli/integrations/test_catalog.py +++ b/tests/specify_cli/integrations/test_catalog.py @@ -88,7 +88,7 @@ def test_http_rejected(self): def test_schemeless_url_names_the_value(self): with pytest.raises(IntegrationCatalogError) as excinfo: IntegrationCatalog._validate_catalog_url("example.com/catalog.json") - assert "(got 'example.com/catalog.json')" in str(excinfo.value) + assert "(got a URL with no scheme)" in str(excinfo.value) assert "got ://" not in str(excinfo.value) def test_http_localhost_allowed(self): diff --git a/tests/specify_cli/presets/test_catalog.py b/tests/specify_cli/presets/test_catalog.py index 98cbefb130..aadbca2f20 100644 --- a/tests/specify_cli/presets/test_catalog.py +++ b/tests/specify_cli/presets/test_catalog.py @@ -190,7 +190,7 @@ def test_validate_catalog_url_http_rejected(self, project_dir): def test_validate_catalog_url_schemeless_names_the_value(self, project_dir): """A URL without a scheme is named instead of reported as "got ://".""" catalog = PresetCatalog(project_dir) - with pytest.raises(PresetValidationError, match=r"\(got 'example\.com/catalog\.json'\)"): + with pytest.raises(PresetValidationError, match=r"\(got a URL with no scheme\)"): catalog._validate_catalog_url("example.com/catalog.json") def test_validate_catalog_url_localhost_http_allowed(self, project_dir): diff --git a/tests/test_workflows.py b/tests/test_workflows.py index abf4876138..b4a583e3fb 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -10357,7 +10357,7 @@ def test_validate_url_schemeless_names_the_value(self, project_dir): (WorkflowCatalog(project_dir), WorkflowValidationError), (StepCatalog(project_dir), StepValidationError), ): - with pytest.raises(error, match=r"\(got 'example\.com/catalog\.json'\)"): + with pytest.raises(error, match=r"\(got a URL with no scheme\)"): catalog._validate_catalog_url("example.com/catalog.json") def test_validate_url_localhost_http_allowed(self, project_dir):