Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkg/cmd/release/create/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2422,6 +2422,11 @@ func TestReleaseCreate_ToPackageOverrideString(t *testing.T) {
{name: "action-ref-ver", input: &packages.PackageVersionOverride{PackageReferenceName: "pterm-on-install", ActionName: "Install", Version: "6.1.2"}, expect: "Install:pterm-on-install:6.1.2"},
{name: "star-ref-ver", input: &packages.PackageVersionOverride{PackageReferenceName: "pterm-on-install", Version: "6.1.2"}, expect: "*:pterm-on-install:6.1.2"},
{name: "pkg-action-ref-ver", input: &packages.PackageVersionOverride{PackageReferenceName: "pterm", PackageID: "pterm", ActionName: "Install", Version: "1.2.3"}, expect: "pterm:pterm:1.2.3"},
// Maven package IDs with colons get escaped (FD-135)
{name: "maven-pkg-ver", input: &packages.PackageVersionOverride{PackageID: "com.yourcompany:project-name", Version: "1.0"}, expect: `com.yourcompany\:project-name:1.0`},
{name: "maven-pkg-ref-ver", input: &packages.PackageVersionOverride{PackageID: "com.juliusbaer.fi-master:deployment", PackageReferenceName: "ref", Version: "25.2026.04.1"}, expect: `com.juliusbaer.fi-master\:deployment:ref:25.2026.04.1`},
// Step name with slash gets escaped
{name: "step-slash-ver", input: &packages.PackageVersionOverride{ActionName: "Deploy Templates/templates", Version: "1.0"}, expect: `Deploy Templates\/templates:1.0`},
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

ToPackageOverrideString now escapes backslashes via escapePackageDelimiters, but the table-driven tests here don’t include a case that asserts this behavior. Adding a PackageID/ActionName containing \ (expecting \\ in the override string) would help prevent regressions and ensure the new escaping semantics are covered.

Suggested change
{name: "step-slash-ver", input: &packages.PackageVersionOverride{ActionName: "Deploy Templates/templates", Version: "1.0"}, expect: `Deploy Templates\/templates:1.0`},
{name: "step-slash-ver", input: &packages.PackageVersionOverride{ActionName: "Deploy Templates/templates", Version: "1.0"}, expect: `Deploy Templates\/templates:1.0`},
// Backslashes get escaped
{name: "step-backslash-ver", input: &packages.PackageVersionOverride{ActionName: `Deploy Templates\templates`, Version: "1.0"}, expect: `Deploy Templates\\templates:1.0`},
{name: "pkg-backslash-ver", input: &packages.PackageVersionOverride{PackageID: `com.yourcompany\project-name`, Version: "1.0"}, expect: `com.yourcompany\\project-name:1.0`},

Copilot uses AI. Check for mistakes.
}

for _, test := range tests {
Expand Down Expand Up @@ -2455,6 +2460,16 @@ func TestReleaseCreate_ParsePackageOverrideString(t *testing.T) {
{input: "pterm/Push Package=9.7-pre-xyz", expect: &packages.AmbiguousPackageVersionOverride{PackageReferenceName: "Push Package", ActionNameOrPackageID: "pterm", Version: "9.7-pre-xyz"}},
{input: "pterm=Push Package/9.7-pre-xyz", expect: &packages.AmbiguousPackageVersionOverride{PackageReferenceName: "Push Package", ActionNameOrPackageID: "pterm", Version: "9.7-pre-xyz"}},

// Maven packages with escaped colons (FD-135)
{input: `com.yourcompany\:project-name:1.0-SNAPSHOT`, expect: &packages.AmbiguousPackageVersionOverride{ActionNameOrPackageID: "com.yourcompany:project-name", Version: "1.0-SNAPSHOT"}},
{input: `com.juliusbaer.fi-master\:deployment:25.2026.04.1`, expect: &packages.AmbiguousPackageVersionOverride{ActionNameOrPackageID: "com.juliusbaer.fi-master:deployment", Version: "25.2026.04.1"}},
// Maven package with escaped colon and package reference name
{input: `com.yourcompany\:project-name:ref:1.0`, expect: &packages.AmbiguousPackageVersionOverride{ActionNameOrPackageID: "com.yourcompany:project-name", PackageReferenceName: "ref", Version: "1.0"}},
// Step name with escaped slash (additional packages)
{input: `Deploy Templates\/templates:1.0`, expect: &packages.AmbiguousPackageVersionOverride{ActionNameOrPackageID: "Deploy Templates/templates", Version: "1.0"}},
// Escaped backslash
{input: `foo\\bar:1.0`, expect: &packages.AmbiguousPackageVersionOverride{ActionNameOrPackageID: `foo\bar`, Version: "1.0"}},

{input: "", expectErr: errors.New("empty package version specification")},

// bare identifiers aren't valid
Expand Down
69 changes: 59 additions & 10 deletions pkg/packages/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/MakeNowJust/heredoc/v2"
"github.com/OctopusDeploy/cli/pkg/output"
"github.com/OctopusDeploy/cli/pkg/question"
"github.com/OctopusDeploy/cli/pkg/util"
octopusApiClient "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/client"
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/feeds"
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases"
Expand Down Expand Up @@ -193,19 +192,19 @@ type PackageVersionOverride struct {
func (p *PackageVersionOverride) ToPackageOverrideString() string {
components := make([]string, 0, 3)

// stepNameOrPackageID always comes first if we have it
// stepNameOrPackageID always comes first if we have it; escape delimiter chars so the server can parse correctly
if p.PackageID != "" {
components = append(components, p.PackageID)
components = append(components, escapePackageDelimiters(p.PackageID))
} else if p.ActionName != "" { // can't have both PackageID and ActionName; PackageID wins
components = append(components, p.ActionName)
components = append(components, escapePackageDelimiters(p.ActionName))
}

// followed by package reference name if we have it
if p.PackageReferenceName != "" {
if len(components) == 0 { // if we have an explicit packagereference but no packageId or action, we need to express it with *:ref:version
components = append(components, "*")
}
components = append(components, p.PackageReferenceName)
components = append(components, escapePackageDelimiters(p.PackageReferenceName))
}

if len(components) == 0 { // the server can't deal with just a number by itself; if we want to override everything we must pass *:Version
Expand All @@ -216,12 +215,62 @@ func (p *PackageVersionOverride) ToPackageOverrideString() string {
return strings.Join(components, ":")
}

// splitPackageOverrideString splits the input string into components based on delimiter characters.
// we want to pick up empty entries here; so "::5" and ":pterm:5" should both return THREE components, rather than one or two
// and we want to allow for multiple different delimeters.
// neither the builtin golang strings.Split or strings.FieldsFunc support this. Logic borrowed from strings.FieldsFunc with heavy modifications
// splitPackageOverrideString splits the input string into components based on delimiter characters (:, /, =).
// Supports escaping delimiters with a backslash (e.g. \: for a literal colon in Maven package IDs).
// Empty entries are preserved; "::5" and ":pterm:5" both return THREE components.
func splitPackageOverrideString(s string) []string {
return util.SplitString(s, []int32{':', '/', '='})
type span struct {
start int
end int
}
spans := make([]span, 0, 3)
start := 0
escaped := false

for idx, ch := range s {
if ch == '\\' && !escaped {
escaped = true
continue
}
if (ch == ':' || ch == '/' || ch == '=') && !escaped {
spans = append(spans, span{start, idx})
start = idx + 1
} else {
escaped = false
}
}
spans = append(spans, span{start, len(s)})

a := make([]string, len(spans))
for i, span := range spans {
a[i] = unescapePackageString(s[span.start:span.end])
}
return a
}

func unescapePackageString(s string) string {
result := make([]rune, 0, len(s))
escaped := false
for _, ch := range s {
if ch == '\\' && !escaped {
escaped = true
continue
}
result = append(result, ch)
escaped = false
}
Comment on lines +252 to +261
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

unescapePackageString currently treats backslash as a generic escape prefix and removes it regardless of what follows (and it also drops a trailing \ at end-of-string). That means inputs like foo\bar:1.0 are fine, but foo\b:1.0 / foo\bar (single backslash) will silently lose the backslash and change the parsed value, even though the doc comment says only delimiters are escapable. Consider only consuming the backslash when it precedes one of :, /, =, or \ (otherwise keep the backslash), and treating a trailing backslash as a literal backslash (or returning an error).

Suggested change
result := make([]rune, 0, len(s))
escaped := false
for _, ch := range s {
if ch == '\\' && !escaped {
escaped = true
continue
}
result = append(result, ch)
escaped = false
}
runes := []rune(s)
result := make([]rune, 0, len(runes))
for i := 0; i < len(runes); i++ {
ch := runes[i]
if ch == '\\' {
if i+1 < len(runes) {
next := runes[i+1]
if next == ':' || next == '/' || next == '=' || next == '\\' {
result = append(result, next)
i++
continue
}
}
result = append(result, ch)
continue
}
result = append(result, ch)
}

Copilot uses AI. Check for mistakes.
return string(result)
}

func escapePackageDelimiters(s string) string {
var result strings.Builder
for _, ch := range s {
if ch == ':' || ch == '/' || ch == '=' || ch == '\\' {
result.WriteRune('\\')
}
result.WriteRune(ch)
}
return result.String()
}

// AmbiguousPackageVersionOverride tells us that we want to set the version of some package to `Version`
Expand Down
Loading