From 0a0ac37df40b9e7998a8ddd42030c18478d09ce9 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Sun, 14 Jun 2026 16:58:46 -0400 Subject: [PATCH] fix: compare downgrade gate against landed version not source env version The downgrade gate compared every promotion's target env against the source env's raw version (result.SourceVersion). In a cascade across the publish boundary the version that lands differs per env: prerelease envs receive the rc (v1.0.0-rc.0) while release and prod receive the stripped semver (v1.0.0). An rc sorts below its release under semver precedence, so finalizing v1.0.0-rc.0 onto a release env already holding v1.0.0 was wrongly flagged as a downgrade and blocked. Compare each env against promo.Version (the version that actually lands on that env), falling back to SourceVersion only for version-less promotions. Real downgrades stay blocked because a genuine downgrade carries the older version in promo.Version for every affected env. Signed-off-by: Joshua Temple --- internal/promote/downgrade_test.go | 58 ++++++++++++++++++++++++++++++ internal/promote/preflight.go | 26 ++++++++++---- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/internal/promote/downgrade_test.go b/internal/promote/downgrade_test.go index 4f75a3c..cecf8ec 100644 --- a/internal/promote/downgrade_test.go +++ b/internal/promote/downgrade_test.go @@ -158,3 +158,61 @@ func TestCheckDowngrade_EmptyVersions_Skipped(t *testing.T) { require.NoError(t, err) require.Empty(t, result2.Warnings) } + +// TestCheckDowngrade_PublishBoundary_RCToReleaseNotDowngrade reproduces the +// cascade publish-boundary false positive: a cascade from a prerelease env at +// v1.0.0-rc.0 to prod materializes a "release" promotion carrying the stripped +// semver v1.0.0. The release env currently holds the previously published +// v1.0.0. The version that LANDS on release is v1.0.0 (promo.Version), which is +// equal, not a downgrade. The gate must compare against promo.Version, not the +// source env's raw rc version, otherwise it wrongly blocks the finalization. +func TestCheckDowngrade_PublishBoundary_RCToReleaseNotDowngrade(t *testing.T) { + p := newDowngradePreflighter(t, map[string]*config.EnvState{ + "release": {Version: "v1.0.0"}, + "prod": {Version: "v0.3.0"}, + }, false) + // SourceVersion is the source env's raw rc; promo.Version is what lands. + result := &PreflightResult{SourceVersion: "v1.0.0-rc.0"} + promotions := []EnvPromotion{ + {Environment: "release", Version: "v1.0.0"}, + {Environment: "prod", Version: "v1.0.0"}, + } + + err := p.checkDowngrade(promotions, result, "prod") + require.NoError(t, err) + require.Empty(t, result.Warnings) +} + +// TestCheckDowngrade_RealDowngrade_StillBlockedViaPromoVersion proves the fix +// does not weaken protection: when the version that actually lands (promo.Version) +// is older than the env's current version, the gate still blocks. +func TestCheckDowngrade_RealDowngrade_StillBlockedViaPromoVersion(t *testing.T) { + p := newDowngradePreflighter(t, map[string]*config.EnvState{ + "release": {Version: "v2.0.0"}, + }, false) + // Source raw version looks newer, but the version landing on release is older. + result := &PreflightResult{SourceVersion: "v2.1.0-rc.0"} + promotions := []EnvPromotion{{Environment: "release", Version: "v1.5.0"}} + + err := p.checkDowngrade(promotions, result, "prod") + require.Error(t, err) + require.Contains(t, err.Error(), "release") + require.Contains(t, err.Error(), "v2.0.0") + require.Contains(t, err.Error(), "v1.5.0") + require.Contains(t, err.Error(), "--allow-downgrade") +} + +// TestCheckDowngrade_PrereleaseEnv_RCProgressesForward confirms a cascade that +// carries an rc forward into prerelease envs is allowed when the rc is newer, +// using promo.Version (the rc) rather than a stripped value. +func TestCheckDowngrade_PrereleaseEnv_RCProgressesForward(t *testing.T) { + p := newDowngradePreflighter(t, map[string]*config.EnvState{ + "uat": {Version: "v0.3.0-rc.0"}, + }, false) + result := &PreflightResult{SourceVersion: "v1.0.0-rc.0"} + promotions := []EnvPromotion{{Environment: "uat", Version: "v1.0.0-rc.0"}} + + err := p.checkDowngrade(promotions, result, "prod") + require.NoError(t, err) + require.Empty(t, result.Warnings) +} diff --git a/internal/promote/preflight.go b/internal/promote/preflight.go index 97318a4..c2d38a1 100644 --- a/internal/promote/preflight.go +++ b/internal/promote/preflight.go @@ -289,18 +289,32 @@ func (p *Preflighter) checkDowngrade(promotions []EnvPromotion, result *Prefligh currentStr = state.Version } + // The incoming version is the one that actually LANDS on this env, which + // is promo.Version. In a cascade across the publish boundary these differ + // from the source env's raw version: the prerelease envs receive the RC + // (e.g. v1.0.0-rc.0) while the release/prod envs receive the stripped + // semver (v1.0.0). Comparing every env against result.SourceVersion (the + // single source env's raw version) wrongly flags the rc-to-release + // finalization as a downgrade, because an rc sorts below its release under + // semver precedence. Use promo.Version per env; fall back to SourceVersion + // only when a promotion carries no version (version-less manifests). + incomingStr := promo.Version + if incomingStr == "" { + incomingStr = result.SourceVersion + } + // Skip if either version is empty (first promotion / version-less). - if result.SourceVersion == "" || currentStr == "" { + if incomingStr == "" || currentStr == "" { continue } - incoming, errIn := version.Parse(result.SourceVersion) + incoming, errIn := version.Parse(incomingStr) current, errCur := version.Parse(currentStr) if errIn != nil || errCur != nil { // FAIL-OPEN: non-semver version -> warn and continue. result.Warnings = append(result.Warnings, fmt.Sprintf( "downgrade check skipped on env %q: could not compare current version %s to incoming %s; ensure both are semver to enforce monotonicity", - promo.Environment, currentStr, result.SourceVersion, + promo.Environment, currentStr, incomingStr, )) continue } @@ -315,7 +329,7 @@ func (p *Preflighter) checkDowngrade(promotions []EnvPromotion, result *Prefligh if p.allowDowngrade { result.Warnings = append(result.Warnings, fmt.Sprintf( "downgrade on %s from %s to %s permitted via --allow-downgrade", - promo.Environment, currentStr, result.SourceVersion, + promo.Environment, currentStr, incomingStr, )) continue } @@ -323,12 +337,12 @@ func (p *Preflighter) checkDowngrade(promotions []EnvPromotion, result *Prefligh if isProd { return fmt.Errorf( "downgrade blocked on prod env %q: current version %s is newer than incoming %s; prod downgrades always require --allow-downgrade", - promo.Environment, currentStr, result.SourceVersion, + promo.Environment, currentStr, incomingStr, ) } return fmt.Errorf( "downgrade blocked on env %q: current version %s is newer than incoming %s; pass --allow-downgrade to permit", - promo.Environment, currentStr, result.SourceVersion, + promo.Environment, currentStr, incomingStr, ) } return nil