fix(wfctl): pin plugin install to requested version, not stale manifest#426
Merged
fix(wfctl): pin plugin install to requested version, not stale manifest#426
Conversation
When running 'wfctl plugin install myplugin@v0.2.1', the registry manifest may contain an older version (e.g. v0.1.0) with download URLs pointing at v0.1.0 assets. The requested version was being silently ignored, causing the old version to be installed. Fix: capture the requested version from the name@version argument, and when it differs from the manifest version, call pinManifestToVersion to rewrite download URLs in-place (replaces /releases/download/v0.1.0/ with /releases/download/v0.2.1/) and update manifest.Version. SHA256 checksums are cleared since they are only valid for the old assets. If the rewritten URL returns a 404, the error is wrapped with context: "requested version v0.2.1 not available for X (registry manifest is at v0.1.0)" — no silent fallback to the stale version. Root cause in BMW: app.yaml requires workflow-plugin-payments@v0.2.1 but the registry manifest had v0.1.0, so deploys kept installing v0.1.0.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes wfctl plugin install <name>@<version> so the requested version drives the download URL (even when the registry manifest is stale), preventing silent installs of an older version.
Changes:
- Capture
requestedVersionfromname@versionand pin the fetched registry manifest to that version before downloading. - Add
pinManifestToVersionhelper to rewrite download URLs and clear stale checksums. - Add unit + integration tests covering URL rewriting and failure behavior when the requested version doesn’t exist.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/wfctl/plugin_install.go | Uses requested @version to pin manifest URLs before install; adds pinManifestToVersion and improves error context on failures. |
| cmd/wfctl/plugin_version_pin_test.go | Adds tests validating URL rewrite/no-op behavior and end-to-end install behavior with a stale manifest. |
|
|
||
| if err := installPluginFromManifest(pluginDirVal, pluginName, manifest); err != nil { | ||
| if requestedVersion != "" && requestedVersion != registryVersion { | ||
| return fmt.Errorf("requested version %s not available for %q (registry manifest is at %s): %w", |
There was a problem hiding this comment.
The wrapped error text says the requested version is “not available”, but installPluginFromManifest can fail for many other reasons (disk permissions, corrupt tarball, post-install verification, etc.). This message can be misleading; consider wording it as “failed to install requested version …” or only using the “not available” wording when the underlying failure is clearly a 404/missing asset.
Suggested change
| return fmt.Errorf("requested version %s not available for %q (registry manifest is at %s): %w", | |
| return fmt.Errorf("failed to install requested version %s for %q (registry manifest is at %s): %w", |
Comment on lines
+665
to
+667
| // If the version string also appears in the filename, rewrite that too. | ||
| if rewritten == url && oldVersion != "" { | ||
| rewritten = strings.ReplaceAll(url, oldVersion, requestedVersion) |
There was a problem hiding this comment.
pinManifestToVersion won’t rewrite version strings in the filename when the URL already matched the /releases/download/<old>/ pattern. For URLs like .../releases/download/v0.1.0/plugin-v0.1.0.tar.gz, the path gets updated but the filename remains at the old version, so the download can still 404 or fetch the wrong asset. Apply the second replacement against the already-rewritten URL (or check strings.Contains(rewritten, oldVersion)), not only when rewritten == url.
Suggested change
| // If the version string also appears in the filename, rewrite that too. | |
| if rewritten == url && oldVersion != "" { | |
| rewritten = strings.ReplaceAll(url, oldVersion, requestedVersion) | |
| // If the version string also appears elsewhere in the URL (for example, | |
| // in the asset filename), rewrite that too using the already-updated URL. | |
| if oldVersion != "" { | |
| rewritten = strings.ReplaceAll(rewritten, oldVersion, requestedVersion) |
| // pinManifestToVersion rewrites download URLs in-place so the right release is fetched. | ||
| registryVersion := manifest.Version | ||
| if requestedVersion != "" && requestedVersion != manifest.Version { | ||
| pinManifestToVersion(manifest, requestedVersion) |
There was a problem hiding this comment.
When version pinning is active, all manifest SHA256 values are cleared, which means the download step will silently skip checksum verification for the pinned install. Consider emitting a clear warning (or failing unless a checksum for the requested version can be obtained) so users understand they’re installing without integrity verification in this path.
Suggested change
| pinManifestToVersion(manifest, requestedVersion) | |
| pinManifestToVersion(manifest, requestedVersion) | |
| fmt.Fprintf(os.Stderr, | |
| "warning: installing %q at requested version %s using a pinned manifest derived from registry version %s; checksum verification may be skipped for this install if no checksum is available for the requested version\n", | |
| pluginName, requestedVersion, registryVersion, | |
| ) |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When running
wfctl plugin install workflow-plugin-payments@v0.2.1, the registry manifest may contain an older version (e.g. v0.1.0) with download URLs pointing at v0.1.0 assets. The requested version was silently ignored — v0.1.0 was installed instead.BMW root cause:
app.yamldeclaresrequires.plugins: [{name: workflow-plugin-payments, version: v0.2.1}], but the registry manifest had v0.1.0. Every deploy installed v0.1.0.Root cause
In
runPluginInstall,parseNameVersionwas called with_discarding the version:The manifest version from the registry then drove the download URL.
Fix
requestedVersionfromname@versionFetchManifest, ifrequestedVersion != manifest.Version, callpinManifestToVersionto rewrite download URLs in-place: replaces/releases/download/v0.1.0/with/releases/download/v0.2.1/, clears stale SHA256 checksums"requested version v0.2.1 not available for X (registry manifest is at v0.1.0)"— no silent fallbackTest plan
TestPinManifestToVersion_URLRewritten— unit: download URLs rewritten, SHA256 cleared, manifest.Version updatedTestPinManifestToVersion_SameVersion— unit: no-op when versions matchTestRunPluginInstall_VersionPinHitsNewURL— integration: fake registry serves v0.1.0 manifest; install request with@v0.2.1hits v0.2.1 download URL and installs with v0.2.1 in plugin.jsonTestRunPluginInstall_VersionPinNotFound— integration: v99.99.99 returns 404, error includes requested versionGOWORK=off go test ./cmd/wfctl/...✓🤖 Generated with Claude Code