Skip to content

Require HTTPS consistently for catalog sources - #5355

Merged
mnriem merged 7 commits into
github:mainfrom
mnriem:fix/catalog-https-policy
Nov 10, 2026
Merged

mnriem merged 7 commits into
github:mainfrom
mnriem:fix/catalog-https-policy

Conversation

@mnriem

@mnriem mnriem commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Description

Require HTTPS for catalog source URLs consistently across extensions, presets, integrations, workflows, workflow steps, and bundles, including local development. Update configuration validation, fetch-time checks, CLI help, and documentation to match.

Preset search now reports invalid source configuration instead of returning an empty result. Local development uses a trusted HTTPS server; the installed-adapter tests now exercise that setup with a generated certificate. Archive-download policies and bundle local-file support are unchanged.

Testing

Set up this checkout's .venv and editable install using the local development guide. Run commands from the checkout root.

  • Tested locally with .venv/bin/specify --help (Windows PowerShell: .\.venv\Scripts\specify.exe --help)
  • Ran relevant existing tests with .venv/bin/python -m pytest tests -q (Windows PowerShell: .\.venv\Scripts\python.exe -m pytest tests -q); use affected test paths for focused checks and LC_ALL=en_US.UTF-8 on macOS
  • Tested with a sample project (if applicable)

Automated checks on macOS:

  • .venv/bin/specify --help — passed.
  • LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/test_catalog_contracts.py tests/specify_cli/bundles/test_catalog_config.py tests/specify_cli/bundles/test_offline.py -q — 167 passed.
  • LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/integrations/test_installed_adapters.py -q -x — 686 passed.
  • LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/test_catalog_contracts.py tests/specify_cli/bundles tests/specify_cli/integrations tests/specify_cli/presets tests/specify_cli/workflows tests/specify_cli/extensions tests/test_extensions.py tests/test_workflows.py tests/test_download_security.py tests/test_catalog_trust_docs.py --ignore=tests/specify_cli/integrations/test_installed_adapters.py -q — 5,410 passed, 29 skipped.
  • uvx ruff@0.15.0 check src tests — passed.
  • uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off — passed; no known vulnerabilities found.
  • git diff --check — passed.

No separate manual sample-project run; the installed-adapter suite exercises sample projects. The full repository suite was not run.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: GitHub Copilot using GPT-6.1 Sol (model ID: gpt-6.1-sol), default reasoning settings, autonomous agent execution at the contributor's direction; implementation, documentation, test generation, automated validation, and PR preparation.

Align catalog source validation and documentation across all catalog types. Report preset configuration errors and use trusted HTTPS in installed-adapter tests.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI auto review requested due to automatic review settings November 9, 2026 23:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Integration catalog fetches omit per-redirect validation, allowing HTTPS loopback catalogs to traverse a disallowed HTTP hop before the final URL is checked.

3 open issues
What changed in this PR

This PR standardizes HTTPS requirements for Spec Kit catalog sources, including local development, while leaving archive-download policies and bundle local-file support unchanged.

Changes:

  • Removes loopback HTTP exceptions from catalog validation and fetch guards.
  • Makes preset search report invalid source configuration and updates CLI help and documentation.
  • Adds rejection tests and moves installed-adapter fixtures to trusted HTTPS with generated certificates.

Review generated for the requester by GitHub Copilot (model/settings unavailable; autonomous code analysis).

File Description
tests/​test_workflows.py Rejects loopback HTTP for workflow and step catalogs.
tests/​test_extensions.py Rejects HTTP extension catalog configuration.
tests/​specify_cli/​test_catalog_contracts.py Adds cross-catalog HTTPS and pre-network rejection checks.
tests/​specify_cli/​presets/​test_catalog.py Rejects loopback HTTP preset URLs.
tests/​specify_cli/​integrations/​test_installed_adapters.py Generates certificates and serves fixtures over HTTPS.
tests/​specify_cli/​integrations/​test_catalog.py Rejects loopback HTTP integration URLs.
tests/​specify_cli/​bundles/​test_offline.py Checks HTTP rejection before network access.
tests/​specify_cli/​bundles/​test_catalog_config.py Checks HTTP rejection without writing configuration.
src/​specify_cli/​workflows/​step/​catalog/​_domain.py Tightens step catalog URL and fetch validation.
src/​specify_cli/​workflows/​catalog/​_domain.py Tightens workflow catalog URL and fetch validation.
src/​specify_cli/​presets/​_catalog.py Requires HTTPS and propagates validation errors from search.
src/​specify_cli/​integrations/​catalog/​command_add.py Updates catalog-add help to require HTTPS locally.
src/​specify_cli/​catalogs.py Removes the shared loopback HTTP exception.
src/​specify_cli/​bundles/​catalog_config.py Requires HTTPS when adding remote sources.
src/​specify_cli/​bundles/​adapters.py Rejects HTTP remote catalogs at fetch time.
pyproject.toml Adds cryptography to test dependencies.
docs/​workflows/​catalogs.md Documents HTTPS-only workflow sources.
docs/​workflow-steps/​catalogs.md Documents HTTPS-only step sources.
docs/​presets/​catalogs.md Updates preset source requirements.
docs/​integrations/​cli.md Updates integration catalog-add requirements.
docs/​extensions/​catalogs.md Replaces HTTP development examples with HTTPS guidance.
docs/​extensions/​api.md Updates the catalog URL field description.
docs/​bundles/​catalogs.md Removes HTTP from supported bundle source schemes.
design/​extension.md Separates catalog HTTPS rules from archive exceptions.
design/​catalogs.md Records HTTPS policy and preset search error behavior.
.github/​security-audit-requirements.txt Updates cryptography dependency provenance comments.

Review effort: Max (auto)

Comment thread src/specify_cli/catalogs.py Resolved
Comment thread src/specify_cli/presets/_catalog.py Resolved
Require TLS 1.2 or newer in the local HTTPS fixture and verify that the minimum is set explicitly rather than inherited from runtime defaults.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI auto review requested due to automatic review settings November 9, 2026 23:54
Validate every integration catalog redirect, separate preset source configuration errors from recoverable fetch failures, and retain end-to-end loopback HTTP archive coverage with HTTPS catalogs.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Integration catalog fetches lack per-hop HTTPS validation, allowing plaintext redirects that expose catalog metadata and package hashes to tampering even when the final URL uses HTTPS.

3 open issues
2 previously missed issues

In code that hasn't changed since last review

Medium severity Allow removal of legacy HTTP catalog entries

src/​specify_cli/​workflows/​catalog/​_domain.py:349

After upgrading, a previously valid loopback HTTP entry makes workflow catalog remove fail before deletion, because removal reparses the configuration through this validator. Preserve a safe way to remove these legacy entries, or document the manual migration and the project/user configuration paths. Same issue in src/specify_cli/workflows/step/catalog/_domain.py:297.

Low severity Cover preset fetch failures and invalid-HTTP CLI behavior

src/​specify_cli/​presets/​_catalog.py:766

The new contract test checks PresetValidationError propagation, but neither ordinary fetch failures returning [] nor the CLI result for invalid HTTP configuration is covered. Add an ordinary PresetError search test and a preset search regression asserting exit code 1, the HTTPS error, and no “No presets found” message. These cover the paired cases required by CONTRIBUTING.md:229–250 and the public command surface described in design/cli.md:248–272.

Review effort: Max (auto)

Copilot AI auto review requested due to automatic review settings November 10, 2026 00:09
Explain manual migration of legacy HTTP workflow and step catalog entries in project, user, and environment configuration. Cover fatal HTTP configuration and nonfatal fetch failures through the preset search CLI.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Catalog removal is blocked for previously accepted loopback HTTP sources, and the redirect tests can pass despite a regression in HTTPS enforcement.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Allow removing catalogs with URLs rejected by the new policy

src/​specify_cli/​workflows/​catalog/​_domain.py:349

Any previously accepted loopback HTTP source now makes specify workflow catalog remove <index> fail, because removal calls _parse_catalog_config() and rejects the URL before deleting an entry. Resolve removal indices without validating URLs so users can delete obsolete sources after upgrading. The same issue affects workflow-step removal in src/specify_cli/workflows/step/catalog/_domain.py:297.

2 resolved since last review

Review effort: Max (auto)

Copilot AI auto review requested due to automatic review settings November 10, 2026 00:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to weigh an unpublished, low-severity finding that URL guidance still recommends HTTP even though it is rejected.

2 open issues

Review effort: Max (auto)

Fail preset search tests immediately if an HTTP redirect is unexpectedly accepted, and recommend HTTPS rather than HTTP in bundle catalog source guidance.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI auto review requested due to automatic review settings November 10, 2026 13:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to judge the impact of repeated certificate-trust guidance that may leave Python clients failing TLS verification despite configuring macOS Keychain.

1 open issue
1 resolved since last review

Review effort: Max (auto)

@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Nov 10, 2026
Distinguish Python TLS trust from browser and operating-system trust, document SSL_CERT_FILE CA-bundle configuration, and link local catalog guidance to the shared instructions.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI auto review requested due to automatic review settings November 10, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because a previously accepted loopback HTTP source can now invalidate the entire configuration, blocking removal of both that source and unrelated HTTPS entries.

1 open issue

Review effort: Max (auto)

Skip URL validation only for local workflow and step catalog removal while preserving priority ordering, configuration validation, and HTTPS requirements for discovery and fetching. Update migration guidance and cover legacy entries and invalid indexes through the CLI.

Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI auto review requested due to automatic review settings November 10, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Removing documented loopback-HTTP support across six catalog types breaks existing local-development configurations and needs maintainer sign-off, while the new TLS fixture has only been validated on macOS.

1 open issue

Review effort: Max (auto)

@mnriem

mnriem commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

At 3b5684f0, requiring HTTPS for catalog sources across all six catalog types is intentional, including loopback and local-development sources. The migration guidance covers Python-trusted HTTPS certificates through SSL_CERT_FILE, configuration updates, and removal of legacy project workflow/step catalog sources without fetching them; archive-download rules and bundle local-file support remain unchanged. The integration catalog redirect fix is in place and its conversation is resolved. CI passed all nine pytest jobs across Linux, Windows, and macOS on Python 3.11, 3.14, and 3.15, including the installed-adapter TLS fixture. Could a maintainer confirm sign-off on this compatibility change before merging? AI-generated on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol (model ID: gpt-6.1-sol), default reasoning settings, autonomous execution; review follow-up wording and CI-result verification.

@mnriem
mnriem merged commit 6d2f8f0 into github:main Nov 10, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants