fix: reject mismatched step catalog versions - #4468
Conversation
Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
馃煛 Changes recommended
The version check must validate explicitly declared falsey values rather than relying on truthiness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Rejects catalog step packages whose downloaded version differs from catalog metadata.
Changes:
- Compares normalized catalog and downloaded step versions.
- Adds regression coverage for mismatched versions and cleanup.
File summaries
| File | Description |
|---|---|
tests/test_workflows.py |
Adds mismatch rejection and cleanup tests. |
src/specify_cli/workflows/_commands.py |
Adds version validation, but falsey declared versions can bypass it. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| catalog_version = info.get("version") | ||
| downloaded_version = step_meta.get("version") | ||
| if catalog_version and downloaded_version: |
There was a problem hiding this comment.
Addressed on 9180bf5b: validation now checks key presence rather than truthiness, with regression cases for 0, false, empty string, and null in addition to a normal mismatched version. Full runnable suite: 7,536 passed, 195 skipped; Ruff and CLI smoke are clean. Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-5.6 Sol).
Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
馃煛 Changes recommended
The fallback may accept mismatched non-PEP-440 versions that differ only by whitespace.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| versions_match = str(downloaded_version).strip() == str( | ||
| catalog_version | ||
| ).strip() |
There was a problem hiding this comment.
Addressed on 74431804: invalid-version fallback now compares original strings exactly, matching the existing workflow implementation. Added a regression for non-PEP-440 values that differ only by whitespace. Full runnable suite: 7,537 passed, 195 skipped; Ruff and CLI smoke are clean. Posted on behalf of @marcelsafin by GitHub Copilot (model: GPT-5.6 Sol).
Assisted-by: GitHub Copilot (model: gpt-5.6-sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Reject catalog step packages when the downloaded
step.ymldeclares adifferent version from the catalog entry.
Previously, installation validated the downloaded type key but stored the
catalog version without checking the package version. A stale or
misconfigured URL could therefore install different code under trusted
catalog metadata. Version comparison now follows the normalized behavior used
for workflow packages and occurs before any files are installed.
Testing
uv run specify --helpuv sync && uv run pytestTargeted:
TestWorkflowStepAddCLI(29 passed).Full suite: 7,537 passed, 195 skipped. One existing PowerShell-launcher test
was omitted because
pwshis unavailable locally; it fails identically on theunchanged upstream commit.
AI Disclosure
GitHub Copilot (GPT-5.6 Sol) autonomously reproduced the bug, wrote the
regression test and implementation, and ran verification under
@marcelsafin's direction and review.