From 2e5430fa491a16f355096efd96d3a0ef48851b51 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 12:57:39 -0700 Subject: [PATCH 01/18] fix: tighten validation and interactive-prompt gaps Close six gaps where interactive prompts and validators diverged from the flag/JSON paths: - Validate vNIC VLAN input during interactive MVE ordering - Reject empty required credentials in VXC partner prompts (AWS, Azure, IBM) and stop silently defaulting an unparseable Azure peering VLAN to 0; validate the parsed VLAN range too - Count runes, not bytes, in port/MVE/AWS name-length validators - Validate MCR prefix-filter entries as CIDRs with address-family consistency checks - Reject a float value of exactly 2^63 in GetIntFromInterface - Allow removing a tag with an empty existing value during interactive tag editing, and handle a newly entered empty-value key deterministically --- internal/commands/mve/mve_prompts.go | 3 + internal/commands/mve/mve_prompts_test.go | 11 ++ internal/commands/vxc/vxc_prompts_partner.go | 24 +++- internal/commands/vxc/vxc_prompts_test.go | 114 +++++++++++++++++++ internal/utils/prompts.go | 14 ++- internal/utils/prompts_test.go | 28 +++++ internal/validation/conversion.go | 2 +- internal/validation/conversion_test.go | 2 + internal/validation/mcr.go | 38 ++++--- internal/validation/mcr_test.go | 37 +++++- internal/validation/mve.go | 5 +- internal/validation/mve_test.go | 16 +++ internal/validation/port.go | 7 +- internal/validation/port_test.go | 2 + internal/validation/vxc.go | 7 +- internal/validation/vxc_test.go | 18 +++ 16 files changed, 294 insertions(+), 34 deletions(-) diff --git a/internal/commands/mve/mve_prompts.go b/internal/commands/mve/mve_prompts.go index a7aa9796..a3e468f0 100644 --- a/internal/commands/mve/mve_prompts.go +++ b/internal/commands/mve/mve_prompts.go @@ -390,6 +390,9 @@ func promptMVEVnics(noColor bool) ([]megaport.MVENetworkInterface, error) { if err != nil { return nil, err } + if err := validation.ValidateVLAN(vlan); err != nil { + return nil, err + } } vnics = append(vnics, megaport.MVENetworkInterface{ diff --git a/internal/commands/mve/mve_prompts_test.go b/internal/commands/mve/mve_prompts_test.go index a2135b40..7756540c 100644 --- a/internal/commands/mve/mve_prompts_test.go +++ b/internal/commands/mve/mve_prompts_test.go @@ -400,6 +400,17 @@ func TestPromptMVEVnics_InvalidVLAN(t *testing.T) { assert.Contains(t, err.Error(), "invalid VLAN ID") } +func TestPromptMVEVnics_OutOfRangeVLAN(t *testing.T) { + original := utils.GetResourcePrompt() + defer func() { utils.SetResourcePrompt(original) }() + + utils.SetResourcePrompt(mockPromptSequence([]string{"eth0", "4100"})) + + _, err := promptMVEVnics(true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "VLAN ID") +} + // promptMVEVendorConfig tests — cisco/palo_alto admin password handling func TestPromptMVEVendorConfig_Cisco_WithAdminPassword(t *testing.T) { diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index 3792f641..9a220911 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -98,11 +98,17 @@ func promptAWSConfig(noColor bool) (*megaport.VXCPartnerConfigAWS, error) { if err != nil { return nil, err } + if ownerAccount == "" { + return nil, fmt.Errorf("owner account ID is required") + } connectionName, err := utils.ResourcePrompt("vxc", "Enter connection name (required): ", noColor) if err != nil { return nil, err } + if connectionName == "" { + return nil, fmt.Errorf("connection name is required") + } asnStr, err := utils.ResourcePrompt("vxc", "Enter ASN (required): ", noColor) if err != nil { @@ -177,6 +183,9 @@ func promptAzureConfig(ctx context.Context, svc megaport.VXCService, noColor boo if err != nil { return nil, "", err } + if serviceKey == "" { + return nil, "", fmt.Errorf("service key is required") + } portChoice, err := utils.ResourcePrompt("vxc", "Enter port choice (primary/secondary, optional, default value is primary): ", noColor) if err != nil { @@ -268,9 +277,15 @@ func promptAzurePeeringConfig(noColor bool) (megaport.PartnerOrderAzurePeeringCo if err != nil { return megaport.PartnerOrderAzurePeeringConfig{}, err } - vlan, err := strconv.Atoi(vlanStr) - if err != nil { - vlan = 0 + var vlan int + if vlanStr != "" { + vlan, err = strconv.Atoi(vlanStr) + if err != nil { + return megaport.PartnerOrderAzurePeeringConfig{}, fmt.Errorf("invalid VLAN: %w", err) + } + if err := validation.ValidateVLAN(vlan); err != nil { + return megaport.PartnerOrderAzurePeeringConfig{}, err + } } return megaport.PartnerOrderAzurePeeringConfig{ @@ -323,6 +338,9 @@ func promptIBMConfig(noColor bool) (*megaport.VXCPartnerConfigIBM, error) { if err != nil { return nil, err } + if accountID == "" { + return nil, fmt.Errorf("account ID is required") + } name, err := utils.ResourcePrompt("vxc", "Enter name (required): ", noColor) if err != nil { diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 424e191a..333d996d 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -78,6 +78,37 @@ func TestPromptAWSConfig(t *testing.T) { } } +func TestPromptAWSConfig_RequiresCredentials(t *testing.T) { + tests := []struct { + name string + responses []string + errContains string + }{ + { + name: "empty owner account rejected", + responses: []string{"AWS", ""}, + errContains: "owner account ID is required", + }, + { + name: "empty connection name rejected", + responses: []string{"AWS", "123456789", ""}, + errContains: "connection name is required", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cleanup := mockPrompts(tc.responses) + defer cleanup() + + cfg, err := promptAWSConfig(true) + assert.Error(t, err) + assert.Nil(t, cfg) + assert.Contains(t, err.Error(), tc.errContains) + }) + } +} + func TestPromptGoogleConfig(t *testing.T) { tests := []struct { name string @@ -207,6 +238,16 @@ func TestPromptIBMConfig(t *testing.T) { } } +func TestPromptIBMConfig_RequiresAccountID(t *testing.T) { + cleanup := mockPrompts([]string{""}) + defer cleanup() + + cfg, err := promptIBMConfig(true) + assert.Error(t, err) + assert.Nil(t, cfg) + assert.Contains(t, err.Error(), "account ID is required") +} + func TestPromptBFDConfig(t *testing.T) { tests := []struct { name string @@ -439,6 +480,28 @@ func TestPromptAzureConfig(t *testing.T) { assert.Equal(t, "azure-uid-1", uid) } +func TestPromptAzureConfig_RequiresServiceKey(t *testing.T) { + cleanup := mockPrompts([]string{""}) + defer cleanup() + + mockSvc := &MockVXCService{ + ListPartnerPortsResponse: &megaport.ListPartnerPortsResponse{ + Data: megaport.PartnerLookup{ + Megaports: []megaport.PartnerLookupItem{ + {ProductUID: "azure-uid-1", Type: "primary"}, + }, + }, + }, + } + + ctx := context.Background() + cfg, uid, err := promptAzureConfig(ctx, mockSvc, true) + assert.Error(t, err) + assert.Nil(t, cfg) + assert.Empty(t, uid) + assert.Contains(t, err.Error(), "service key is required") +} + func TestPromptAzurePeeringConfig(t *testing.T) { cleanup := mockPrompts([]string{ "Microsoft", // peering type @@ -462,6 +525,57 @@ func TestPromptAzurePeeringConfig(t *testing.T) { assert.Equal(t, 200, peer.VLAN) } +func TestPromptAzurePeeringConfig_InvalidVLAN(t *testing.T) { + cleanup := mockPrompts([]string{ + "Microsoft", // peering type + "12076", // peer ASN + "10.0.0.0/30", + "10.0.0.4/30", + "10.2.0.0/16", + "key123", + "not-a-number", + }) + defer cleanup() + + _, err := promptAzurePeeringConfig(true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid VLAN") +} + +func TestPromptAzurePeeringConfig_OutOfRangeVLAN(t *testing.T) { + cleanup := mockPrompts([]string{ + "Microsoft", // peering type + "12076", // peer ASN + "10.0.0.0/30", + "10.0.0.4/30", + "10.2.0.0/16", + "key123", + "4100", + }) + defer cleanup() + + _, err := promptAzurePeeringConfig(true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "VLAN ID") +} + +func TestPromptAzurePeeringConfig_EmptyVLANDefaultsToZero(t *testing.T) { + cleanup := mockPrompts([]string{ + "Microsoft", // peering type + "12076", // peer ASN + "10.0.0.0/30", + "10.0.0.4/30", + "10.2.0.0/16", + "key123", + "", + }) + defer cleanup() + + peer, err := promptAzurePeeringConfig(true) + assert.NoError(t, err) + assert.Equal(t, 0, peer.VLAN) +} + func TestPromptPartnerConfig(t *testing.T) { tests := []struct { name string diff --git a/internal/utils/prompts.go b/internal/utils/prompts.go index 889f24dd..7282d02d 100644 --- a/internal/utils/prompts.go +++ b/internal/utils/prompts.go @@ -252,11 +252,15 @@ var updateResourceTagsPromptFn = func(existingTags map[string]string, noColor bo return nil, err } - // Empty value means remove the tag - if value == "" && tags[key] != "" { - delete(tags, key) - fmt.Fprintf(os.Stderr, " Removed tag: %s\n", key) - } else if value != "" { + // Empty value means remove the tag, if it exists. + if value == "" { + if _, exists := tags[key]; exists { + delete(tags, key) + fmt.Fprintf(os.Stderr, " Removed tag: %s\n", key) + } else { + fmt.Fprintf(os.Stderr, " Tag '%s' does not exist, nothing to remove\n", key) + } + } else { tags[key] = value fmt.Fprintf(os.Stderr, " Updated tag: %s: %s\n", key, value) } diff --git a/internal/utils/prompts_test.go b/internal/utils/prompts_test.go index 1639dfa7..369fda64 100644 --- a/internal/utils/prompts_test.go +++ b/internal/utils/prompts_test.go @@ -703,6 +703,34 @@ func TestUpdateResourceTagsPrompt_DefaultModifyExistingRemovesTag(t *testing.T) assert.Empty(t, stdout) } +// TestUpdateResourceTagsPrompt_DefaultModifyRemovesEmptyValueTag exercises +// removing an existing tag whose current value is already empty. +func TestUpdateResourceTagsPrompt_DefaultModifyRemovesEmptyValueTag(t *testing.T) { + mockPromptSequence(t, []bool{true, true}, []string{"2", "foo", "", ""}) + stdout, stderr := withMockedIO("", func() { + tags, err := UpdateResourceTagsPrompt(map[string]string{"foo": ""}, true) + assert.NoError(t, err) + assert.Empty(t, tags) + }) + assert.Contains(t, stderr, "Removed tag: foo") + assert.Empty(t, stdout) +} + +// TestUpdateResourceTagsPrompt_DefaultModifyEmptyValueForNewKey exercises +// entering an empty value for a key that does not exist yet: it must not be +// added to the tag set, and the user gets a clear "nothing to remove" notice +// instead of the key silently vanishing. +func TestUpdateResourceTagsPrompt_DefaultModifyEmptyValueForNewKey(t *testing.T) { + mockPromptSequence(t, []bool{true, true}, []string{"2", "baz", "", ""}) + stdout, stderr := withMockedIO("", func() { + tags, err := UpdateResourceTagsPrompt(map[string]string{"foo": "bar"}, true) + assert.NoError(t, err) + assert.Equal(t, map[string]string{"foo": "bar"}, tags) + }) + assert.Contains(t, stderr, "Tag 'baz' does not exist, nothing to remove") + assert.Empty(t, stdout) +} + // TestDesignConfirmPrompt_DefaultRendering verifies the default // designConfirmPromptFn renders design-stage wording instead of // BuyConfirmPrompt's purchase wording. diff --git a/internal/validation/conversion.go b/internal/validation/conversion.go index c313bab8..5ccaf82a 100644 --- a/internal/validation/conversion.go +++ b/internal/validation/conversion.go @@ -16,7 +16,7 @@ func GetIntFromInterface(value interface{}) (int, bool) { // Reject fractional or out-of-range values rather than silently truncating // JSON-derived numbers (e.g. 3.9 -> 3). Bounds use platform int width so // 32-bit targets (js/wasm) don't accept values that overflow int. - if v != math.Trunc(v) || v < math.MinInt || v > math.MaxInt { + if v != math.Trunc(v) || v < math.MinInt || v >= float64(math.MaxInt) { return 0, false } return int(v), true diff --git a/internal/validation/conversion_test.go b/internal/validation/conversion_test.go index 8a13ff36..f4e30d61 100644 --- a/internal/validation/conversion_test.go +++ b/internal/validation/conversion_test.go @@ -20,6 +20,8 @@ func TestGetIntFromInterface(t *testing.T) { {"float64 whole", float64(10), 10, true}, {"float64 fractional rejected", float64(3.9), 0, false}, {"float64 above int range rejected", float64(math.MaxInt) * 2, 0, false}, + {"float64 exactly 2^63 rejected", math.Exp2(63), 0, false}, + {"float64 just below 2^63 accepted", math.Exp2(63) - 2048, int(math.Exp2(63) - 2048), true}, {"numeric string", "123", 123, true}, {"negative numeric string", "-5", -5, true}, {"empty string", "", 0, false}, diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 4b76a04b..69ae0730 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "net" "strings" megaport "github.com/megaport/megaportgo" @@ -117,16 +118,31 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ return NewValidationError("entries", req.PrefixFilterList.Entries, "must contain at least one entry") } - // Validate each entry - for i, entry := range req.PrefixFilterList.Entries { + return validatePrefixFilterEntries(req.PrefixFilterList.Entries, req.PrefixFilterList.AddressFamily) +} + +// validatePrefixFilterEntries validates each prefix filter entry's prefix as a +// CIDR consistent with the list's declared address family, and that the +// action is permit or deny. +func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, addressFamily string) error { + for i, entry := range entries { if entry.Prefix == "" { return NewValidationError("entry prefix index", i, "prefix cannot be empty") } + if addressFamily == "IPv4" { + if err := ValidateCIDR(entry.Prefix, fmt.Sprintf("entry prefix index %d", i)); err != nil { + return err + } + } else { + ip, _, err := net.ParseCIDR(entry.Prefix) + if err != nil || ip.To4() != nil { + return NewValidationError(fmt.Sprintf("entry prefix index %d", i), entry.Prefix, "must be a valid IPv6 CIDR notation") + } + } if entry.Action != "permit" && entry.Action != "deny" { return NewValidationError("entry action", entry.Action, "must be permit or deny") } } - return nil } @@ -146,18 +162,8 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ // - A ValidationError if any validation check fails // - nil if all validation checks pass func ValidateUpdatePrefixFilterList(prefixFilterList *megaport.MCRPrefixFilterList) error { - // If entries are provided, validate them - if len(prefixFilterList.Entries) > 0 { - // Validate each entry - for i, entry := range prefixFilterList.Entries { - if entry.Prefix == "" { - return NewValidationError("entry prefix index", i, "prefix cannot be empty") - } - if entry.Action != "permit" && entry.Action != "deny" { - return NewValidationError("entry action", entry.Action, "must be permit or deny") - } - } + if len(prefixFilterList.Entries) == 0 { + return nil } - - return nil + return validatePrefixFilterEntries(prefixFilterList.Entries, prefixFilterList.AddressFamily) } diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index 43d86b71..9218d0cb 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -335,7 +335,7 @@ func TestValidateUpdatePrefixFilterList(t *testing.T) { name: "Invalid entry action", req: &megaport.MCRPrefixFilterList{ Description: "Updated filter list", - AddressFamily: "IPv6", + AddressFamily: "IPv4", Entries: []*megaport.MCRPrefixListEntry{ {Action: "allow", Prefix: "10.0.0.0/8"}, }, @@ -343,6 +343,41 @@ func TestValidateUpdatePrefixFilterList(t *testing.T) { wantErr: true, errText: "Invalid entry action: allow - must be permit or deny", }, + { + name: "Invalid prefix CIDR", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "not-a-cidr"}, + }, + }, + wantErr: true, + errText: "Invalid entry prefix index 0: not-a-cidr - must be a valid IPv4 CIDR notation", + }, + { + name: "Prefix does not match declared IPv6 address family", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "IPv6", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8"}, + }, + }, + wantErr: true, + errText: "Invalid entry prefix index 0: 10.0.0.0/8 - must be a valid IPv6 CIDR notation", + }, + { + name: "Valid IPv6 update with entries", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "IPv6", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "2001:db8::/32"}, + }, + }, + wantErr: false, + }, } for _, tt := range tests { diff --git a/internal/validation/mve.go b/internal/validation/mve.go index ec78249b..e4bed4a3 100644 --- a/internal/validation/mve.go +++ b/internal/validation/mve.go @@ -3,6 +3,7 @@ package validation import ( "fmt" "strings" + "unicode/utf8" megaport "github.com/megaport/megaportgo" ) @@ -89,7 +90,7 @@ func ValidateBuyMVERequest(req *megaport.BuyMVERequest) error { if req.Name == "" { return NewValidationError("MVE name", req.Name, "cannot be empty") } - if len(req.Name) > MaxMVENameLength { + if utf8.RuneCountInString(req.Name) > MaxMVENameLength { return NewValidationError("MVE name", req.Name, fmt.Sprintf("cannot exceed %d characters", MaxMVENameLength)) } if err := ValidateContractTerm(req.Term); err != nil { @@ -173,7 +174,7 @@ func ValidateMVERequest(name string, term int, locationID int) error { if name == "" { return NewValidationError("MVE name", name, "cannot be empty") } - if len(name) > MaxMVENameLength { + if utf8.RuneCountInString(name) > MaxMVENameLength { return NewValidationError("MVE name", name, fmt.Sprintf("cannot exceed %d characters", MaxMVENameLength)) } if err := ValidateContractTerm(term); err != nil { diff --git a/internal/validation/mve_test.go b/internal/validation/mve_test.go index d83754a8..e75cd85a 100644 --- a/internal/validation/mve_test.go +++ b/internal/validation/mve_test.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "strings" "testing" megaport "github.com/megaport/megaportgo" @@ -56,6 +57,21 @@ func TestValidateMVERequest(t *testing.T) { wantErr: true, errText: "Invalid MVE name: This name is way too long and should exceed the 64 character limit for MVE product names which will cause validation to fail - cannot exceed 64 characters", }, + { + name: "64 multibyte character name accepted", + productName: strings.Repeat("日", MaxMVENameLength), + term: 12, + locationID: 123, + wantErr: false, + }, + { + name: "65 multibyte character name rejected", + productName: strings.Repeat("日", MaxMVENameLength+1), + term: 12, + locationID: 123, + wantErr: true, + errText: fmt.Sprintf("Invalid MVE name: %s - cannot exceed %d characters", strings.Repeat("日", MaxMVENameLength+1), MaxMVENameLength), + }, } for _, tt := range tests { diff --git a/internal/validation/port.go b/internal/validation/port.go index 9dbc1352..eb8429e7 100644 --- a/internal/validation/port.go +++ b/internal/validation/port.go @@ -3,6 +3,7 @@ package validation import ( "fmt" "slices" + "unicode/utf8" megaport "github.com/megaport/megaportgo" ) @@ -56,7 +57,7 @@ func ValidatePortName(name string) error { } // The spec says names can be up to MaxPortNameLength characters (inclusive) - if len(name) > MaxPortNameLength { + if utf8.RuneCountInString(name) > MaxPortNameLength { return NewValidationError("port name", name, fmt.Sprintf("cannot exceed %d characters", MaxPortNameLength)) } @@ -83,7 +84,7 @@ func ValidatePortRequest(req *megaport.BuyPortRequest) error { if req.Name == "" { return NewValidationError("port name", req.Name, "cannot be empty") } - if len(req.Name) > MaxPortNameLength { + if utf8.RuneCountInString(req.Name) > MaxPortNameLength { return NewValidationError("port name", req.Name, fmt.Sprintf("cannot exceed %d characters", MaxPortNameLength)) } if req.LocationId <= 0 { @@ -119,7 +120,7 @@ func ValidateLAGPortRequest(req *megaport.BuyPortRequest) error { if req.Name == "" { return NewValidationError("port name", req.Name, "cannot be empty") } - if len(req.Name) > MaxPortNameLength { + if utf8.RuneCountInString(req.Name) > MaxPortNameLength { return NewValidationError("port name", req.Name, fmt.Sprintf("cannot exceed %d characters", MaxPortNameLength)) } if req.LocationId <= 0 { diff --git a/internal/validation/port_test.go b/internal/validation/port_test.go index a8a18feb..a75658c7 100644 --- a/internal/validation/port_test.go +++ b/internal/validation/port_test.go @@ -224,6 +224,8 @@ func TestValidatePortName(t *testing.T) { {"Single character (min non-empty)", "A", false}, {"64 character port name", strings.Repeat("A", MaxPortNameLength), false}, {"65 character port name", strings.Repeat("A", MaxPortNameLength+1), true}, + {"64 multibyte character port name", strings.Repeat("日", MaxPortNameLength), false}, + {"65 multibyte character port name", strings.Repeat("日", MaxPortNameLength+1), true}, } for _, tt := range tests { diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 0fd59e77..a457819c 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -3,6 +3,7 @@ package validation import ( "fmt" "strings" + "unicode/utf8" megaport "github.com/megaport/megaportgo" ) @@ -224,8 +225,8 @@ func ValidateAWSPartnerConfig(config *megaport.VXCPartnerConfigAWS) error { return NewValidationError("AWS owner account", config.OwnerAccount, "cannot be empty") } - if config.ConnectionName != "" && len(config.ConnectionName) > 255 { - return NewValidationError("AWS connection name", config.ConnectionName, "cannot exceed 255 characters") + if config.ConnectionName != "" && utf8.RuneCountInString(config.ConnectionName) > MaxAWSConnectionNameLength { + return NewValidationError("AWS connection name", config.ConnectionName, fmt.Sprintf("cannot exceed %d characters", MaxAWSConnectionNameLength)) } if config.CustomerIPAddress != "" { if err := ValidateCIDR(config.CustomerIPAddress, "AWS customer IP address"); err != nil { @@ -362,7 +363,7 @@ func ValidateIBMPartnerConfig(config *megaport.VXCPartnerConfigIBM) error { return NewValidationError("IBM account ID", config.AccountID, "must contain only hexadecimal characters (0-9, a-f, A-F)") } } - if config.Name != "" && len(config.Name) > MaxIBMNameLength { + if config.Name != "" && utf8.RuneCountInString(config.Name) > MaxIBMNameLength { return NewValidationError("IBM connection name", config.Name, fmt.Sprintf("cannot exceed %d characters", MaxIBMNameLength)) } if config.Name != "" && !isValidIBMName(config.Name) { diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index d3220fd6..819c9023 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -3,6 +3,7 @@ package validation import ( "fmt" "math" + "strings" "testing" megaport "github.com/megaport/megaportgo" @@ -862,6 +863,23 @@ func TestValidateAWSPartnerConfig(t *testing.T) { wantErr: true, errText: "Invalid AWS connection name: ", // Error message includes the long name, truncated here }, + { + name: "AWS name at max length with multibyte characters accepted", + connectType: "AWS", + ownerAccount: "123456789012", + asn: 65000, + awsName: strings.Repeat("日", MaxAWSConnectionNameLength), + wantErr: false, + }, + { + name: "AWS name over max length with multibyte characters rejected", + connectType: "AWS", + ownerAccount: "123456789012", + asn: 65000, + awsName: strings.Repeat("日", MaxAWSConnectionNameLength+1), + wantErr: true, + errText: "Invalid AWS connection name: ", + }, { name: "Invalid AWS type for AWS connect type", connectType: "AWS", From 8f6b2bbfab48df8d4c3c15713210929e4e96296d Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 13:03:49 -0700 Subject: [PATCH 02/18] test: cover name-too-long branch in port and MVE buy/LAG validators Codecov flagged three partial hits: ValidatePortRequest, ValidateLAGPortRequest, and ValidateBuyMVERequest never exercised the name-length-exceeded branch, only ValidatePortName and ValidateMVERequest did. --- internal/validation/mve_test.go | 18 ++++++++++++++++++ internal/validation/port_test.go | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/internal/validation/mve_test.go b/internal/validation/mve_test.go index e75cd85a..632ac407 100644 --- a/internal/validation/mve_test.go +++ b/internal/validation/mve_test.go @@ -373,6 +373,24 @@ func TestValidateBuyMVERequest(t *testing.T) { wantErr: true, errText: "Invalid vendor config: - cannot be nil", }, + { + name: "MVE name too long", + req: &megaport.BuyMVERequest{ + Name: strings.Repeat("A", MaxMVENameLength+1), + Term: 12, + LocationID: 100, + VendorConfig: &megaport.CiscoConfig{ + Vendor: "cisco", + ImageID: 123, + ProductSize: "MEDIUM", + AdminSSHPublicKey: "ssh-rsa AAAA...", + SSHPublicKey: "ssh-rsa AAAA...", + ManageLocally: true, + }, + }, + wantErr: true, + errText: fmt.Sprintf("Invalid MVE name: %s - cannot exceed %d characters", strings.Repeat("A", MaxMVENameLength+1), MaxMVENameLength), + }, } for _, tt := range tests { diff --git a/internal/validation/port_test.go b/internal/validation/port_test.go index a75658c7..19dd081e 100644 --- a/internal/validation/port_test.go +++ b/internal/validation/port_test.go @@ -63,6 +63,15 @@ func TestValidatePortRequest(t *testing.T) { wantErr: true, errText: "Invalid location ID: 0 - must be a positive integer", }, + { + name: "Port name too long", + portName: strings.Repeat("A", MaxPortNameLength+1), + term: 12, + portSpeed: 10000, + locationID: 100, + wantErr: true, + errText: fmt.Sprintf("Invalid port name: %s - cannot exceed %d characters", strings.Repeat("A", MaxPortNameLength+1), MaxPortNameLength), + }, } for _, tt := range tests { @@ -196,6 +205,18 @@ func TestValidateLAGPortRequest(t *testing.T) { wantErr: true, errText: fmt.Sprintf("Invalid contract term: 5 - must be one of: %v", ValidContractTerms), }, + { + name: "LAG port name too long", + req: &megaport.BuyPortRequest{ + Name: strings.Repeat("A", MaxPortNameLength+1), + LocationId: 100, + PortSpeed: 10000, + LagCount: 2, + Term: 12, + }, + wantErr: true, + errText: fmt.Sprintf("Invalid port name: %s - cannot exceed %d characters", strings.Repeat("A", MaxPortNameLength+1), MaxPortNameLength), + }, } for _, tt := range tests { From 297c2753bcb6d7e5103a7a0c2d7ef420d6cb9ea3 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 13:11:57 -0700 Subject: [PATCH 03/18] fix: correct int-width bound in GetIntFromInterface, address review nits Comparing against float64(math.MaxInt) directly wrongly rejects the true max int on platforms where MaxInt fits exactly in a float64 (it only rounds up to 2^63 when int is 64 bits). Add 1 to get the correct exclusive upper bound on either width, and make the boundary tests derive from math.MaxInt instead of a hardcoded 2^63 so they stay in range regardless of platform int width. Also update the ValidatePrefixFilterListRequest / ValidateUpdatePrefixFilterList doc comments to mention the CIDR and address-family checks they now perform. --- internal/validation/conversion.go | 12 +++++++++--- internal/validation/conversion_test.go | 9 +++++++-- internal/validation/mcr.go | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/validation/conversion.go b/internal/validation/conversion.go index 5ccaf82a..ab0d2195 100644 --- a/internal/validation/conversion.go +++ b/internal/validation/conversion.go @@ -14,9 +14,15 @@ func GetIntFromInterface(value interface{}) (int, bool) { return v, true case float64: // Reject fractional or out-of-range values rather than silently truncating - // JSON-derived numbers (e.g. 3.9 -> 3). Bounds use platform int width so - // 32-bit targets (js/wasm) don't accept values that overflow int. - if v != math.Trunc(v) || v < math.MinInt || v >= float64(math.MaxInt) { + // JSON-derived numbers (e.g. 3.9 -> 3). Bounds use math.MaxInt so this + // stays correct across platform int widths. + // + // float64(math.MaxInt) itself rounds up to 2^63 on platforms where int is + // 64 bits (2^63-1 isn't exactly representable), so comparing v against it + // directly would wrongly reject the true max int on platforms where MaxInt + // fits exactly in a float64 (e.g. a 32-bit int). Add 1 to get the true + // exclusive upper bound on either width. + if v != math.Trunc(v) || v < math.MinInt || v >= float64(math.MaxInt)+1 { return 0, false } return int(v), true diff --git a/internal/validation/conversion_test.go b/internal/validation/conversion_test.go index f4e30d61..812ce173 100644 --- a/internal/validation/conversion_test.go +++ b/internal/validation/conversion_test.go @@ -8,6 +8,11 @@ import ( ) func TestGetIntFromInterface(t *testing.T) { + // Derived from math.MaxInt (rather than a hardcoded 2^63) so the boundary + // cases stay in range regardless of platform int width. + nearMaxInt := float64(math.MaxInt) - 4096 + justAboveMaxInt := float64(math.MaxInt) + 4096 + tests := []struct { name string value interface{} @@ -20,8 +25,8 @@ func TestGetIntFromInterface(t *testing.T) { {"float64 whole", float64(10), 10, true}, {"float64 fractional rejected", float64(3.9), 0, false}, {"float64 above int range rejected", float64(math.MaxInt) * 2, 0, false}, - {"float64 exactly 2^63 rejected", math.Exp2(63), 0, false}, - {"float64 just below 2^63 accepted", math.Exp2(63) - 2048, int(math.Exp2(63) - 2048), true}, + {"float64 just above max int rejected", justAboveMaxInt, 0, false}, + {"float64 just below max int accepted", nearMaxInt, int(nearMaxInt), true}, {"numeric string", "123", 123, true}, {"negative numeric string", "-5", -5, true}, {"empty string", "", 0, false}, diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 69ae0730..29bb9adb 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -99,6 +99,7 @@ func ValidateMCRRequest(req *megaport.BuyMCRRequest) error { // - At least one entry must be provided in the prefix filter list // - For each entry: // - Prefix cannot be empty +// - Prefix must be a valid CIDR consistent with the list's address family // - Action must be "permit" or "deny" // // Returns: @@ -156,6 +157,7 @@ func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, address // - If entries are provided: // - For each entry: // - Prefix cannot be empty +// - Prefix must be a valid CIDR consistent with the list's address family // - Action must be "permit" or "deny" // // Returns: From 728ad00108a2ae04a702dc55f01a0e3b12c01f03 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 13:25:59 -0700 Subject: [PATCH 04/18] fix(mcr): validate address family in prefix filter list update path ValidateUpdatePrefixFilterList dispatched into the shared entry validator without checking AddressFamily was present and valid, unlike the create path. An empty or invalid value would silently fall into the IPv6 branch. --- internal/validation/mcr.go | 7 +++++++ internal/validation/mcr_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 29bb9adb..e2419638 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -155,6 +155,7 @@ func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, address // // Validation checks: // - If entries are provided: +// - Address family must be provided and a valid value ("IPv4" or "IPv6") // - For each entry: // - Prefix cannot be empty // - Prefix must be a valid CIDR consistent with the list's address family @@ -167,5 +168,11 @@ func ValidateUpdatePrefixFilterList(prefixFilterList *megaport.MCRPrefixFilterLi if len(prefixFilterList.Entries) == 0 { return nil } + if prefixFilterList.AddressFamily == "" { + return NewValidationError("address family", prefixFilterList.AddressFamily, "cannot be empty") + } + if prefixFilterList.AddressFamily != "IPv4" && prefixFilterList.AddressFamily != "IPv6" { + return NewValidationError("address family", prefixFilterList.AddressFamily, "must be IPv4 or IPv6") + } return validatePrefixFilterEntries(prefixFilterList.Entries, prefixFilterList.AddressFamily) } diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index 9218d0cb..f3315e93 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -378,6 +378,30 @@ func TestValidateUpdatePrefixFilterList(t *testing.T) { }, wantErr: false, }, + { + name: "Empty address family with entries rejected", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8"}, + }, + }, + wantErr: true, + errText: "Invalid address family: - cannot be empty", + }, + { + name: "Invalid address family with entries rejected", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "IPv5", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8"}, + }, + }, + wantErr: true, + errText: "Invalid address family: IPv5 - must be IPv4 or IPv6", + }, } for _, tt := range tests { From bf06ee17df8565d6378d8f8806b5d87d814f5c56 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 13:42:44 -0700 Subject: [PATCH 05/18] fix(vxc,wasm): friendlier VLAN parse error, align WASM tag removal with native - Azure peering VLAN prompt now uses validation.ParseInt instead of a raw strconv.Atoi wrap, so a bad value gets the same friendly 'invalid VLAN' wording as other numeric prompts instead of a leaked Atoi error string. - The WASM interactive tag prompt still used the old value == "" && tags[key] != "" condition, so it refused to remove a tag with an empty current value and stayed silent on a key that didn't exist. Mirrored the native prompts.go fix: check key existence explicitly and report when there's nothing to remove. --- internal/commands/vxc/vxc_prompts_partner.go | 5 ++--- internal/utils/prompts_wasm.go | 14 +++++++++----- internal/utils/prompts_wasm_test.go | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index 9a220911..3fb48e3d 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -3,7 +3,6 @@ package vxc import ( "context" "fmt" - "strconv" "strings" "github.com/megaport/megaport-cli/internal/utils" @@ -279,9 +278,9 @@ func promptAzurePeeringConfig(noColor bool) (megaport.PartnerOrderAzurePeeringCo } var vlan int if vlanStr != "" { - vlan, err = strconv.Atoi(vlanStr) + vlan, err = validation.ParseInt("VLAN", vlanStr) if err != nil { - return megaport.PartnerOrderAzurePeeringConfig{}, fmt.Errorf("invalid VLAN: %w", err) + return megaport.PartnerOrderAzurePeeringConfig{}, err } if err := validation.ValidateVLAN(vlan); err != nil { return megaport.PartnerOrderAzurePeeringConfig{}, err diff --git a/internal/utils/prompts_wasm.go b/internal/utils/prompts_wasm.go index 5dd228c6..38148ebe 100644 --- a/internal/utils/prompts_wasm.go +++ b/internal/utils/prompts_wasm.go @@ -270,11 +270,15 @@ func wasmUpdateResourceTagsPrompt(existingTags map[string]string, noColor bool) return nil, err } - // Empty value means remove the tag - if value == "" && tags[key] != "" { - delete(tags, key) - buf.add(fmt.Sprintf(" Removed tag: %s", key)) - } else if value != "" { + // Empty value means remove the tag, if it exists. + if value == "" { + if _, exists := tags[key]; exists { + delete(tags, key) + buf.add(fmt.Sprintf(" Removed tag: %s", key)) + } else { + buf.add(fmt.Sprintf(" Tag '%s' does not exist, nothing to remove", key)) + } + } else { tags[key] = value buf.add(fmt.Sprintf(" Updated tag: %s: %s", key, value)) } diff --git a/internal/utils/prompts_wasm_test.go b/internal/utils/prompts_wasm_test.go index 6866b0df..85f700a3 100644 --- a/internal/utils/prompts_wasm_test.go +++ b/internal/utils/prompts_wasm_test.go @@ -389,6 +389,24 @@ func TestWasmUpdateResourceTagsPrompt(t *testing.T) { }, expectError: false, }, + { + name: "empty value for key that does not exist is a no-op", + existingTags: map[string]string{ + "foo": "bar", + }, + mockResponses: []string{ + "y", // Continue + "2", // Start with existing + "baz", // Key that does not exist + "", // Empty value = attempt remove + "", // Finish + "y", // Apply changes + }, + expectedTags: map[string]string{ + "foo": "bar", + }, + expectError: false, + }, { name: "no existing tags - add new", existingTags: map[string]string{}, From 85067ad4cd5d0dcb99668528d4e5f056956735b6 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 13:59:03 -0700 Subject: [PATCH 06/18] fix(vxc): require AWS connection name in shared validator ValidateAWSPartnerConfig allowed an empty ConnectionName and only checked max length when non-empty, while the interactive prompt path already treats connection name as required. Since JSON and flag-built configs both funnel through this shared validator, the gap let those paths bypass a requirement the prompt enforces. Update the fixtures across three test functions that constructed VXCPartnerConfigAWS without a connection name. --- internal/validation/vxc.go | 7 +++++-- internal/validation/vxc_test.go | 26 ++++++++++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index a457819c..337e1b3b 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -199,7 +199,7 @@ func ValidateVXCRequest(req *megaport.BuyVXCRequest) error { // - ASN must be provided and within the valid range (1-4294967295) // - If customer IP address is provided, it must be in valid IPv4 CIDR notation // - If Amazon IP address is provided, it must be in valid IPv4 CIDR notation -// - If connection name is provided, it must not exceed 255 characters +// - Connection name must be provided and not exceed 255 characters // - For 'AWS' connect type with a specified connection type, it must be 'private' or 'public' // // Returns: @@ -225,7 +225,10 @@ func ValidateAWSPartnerConfig(config *megaport.VXCPartnerConfigAWS) error { return NewValidationError("AWS owner account", config.OwnerAccount, "cannot be empty") } - if config.ConnectionName != "" && utf8.RuneCountInString(config.ConnectionName) > MaxAWSConnectionNameLength { + if config.ConnectionName == "" { + return NewValidationError("AWS connection name", config.ConnectionName, "cannot be empty") + } + if utf8.RuneCountInString(config.ConnectionName) > MaxAWSConnectionNameLength { return NewValidationError("AWS connection name", config.ConnectionName, fmt.Sprintf("cannot exceed %d characters", MaxAWSConnectionNameLength)) } if config.CustomerIPAddress != "" { diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index 819c9023..ef41ee52 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -664,9 +664,10 @@ func TestValidateVXCRequest(t *testing.T) { PortUID: "a-end-uid", BEndConfiguration: megaport.VXCOrderEndpointConfiguration{ PartnerConfig: &megaport.VXCPartnerConfigAWS{ - ConnectType: "AWS", - OwnerAccount: "12345", - ASN: 65000, + ConnectType: "AWS", + OwnerAccount: "12345", + ASN: 65000, + ConnectionName: "MyAWSConnection", }, }, }, @@ -842,6 +843,7 @@ func TestValidateAWSPartnerConfig(t *testing.T) { ownerAccount: "123456789012", asn: 65000, customerIPAddress: "invalid-ip", + awsName: "MyAWSConnection", wantErr: true, errText: "Invalid AWS customer IP address: invalid-ip - must be a valid IPv4 CIDR notation", // Updated error message }, @@ -851,6 +853,7 @@ func TestValidateAWSPartnerConfig(t *testing.T) { ownerAccount: "123456789012", asn: 65000, amazonIPAddress: "192.168.1.2/33", // Invalid mask + awsName: "MyAWSConnection", wantErr: true, errText: "Invalid AWS Amazon IP address: 192.168.1.2/33 - must be a valid IPv4 CIDR notation", // Updated error message }, @@ -886,9 +889,19 @@ func TestValidateAWSPartnerConfig(t *testing.T) { ownerAccount: "123456789012", asn: 65000, awsType: "invalid", + awsName: "MyAWSConnection", wantErr: true, errText: "Invalid AWS type: invalid - must be 'private' or 'public' for AWS connect type", }, + { + name: "Empty connection name", + connectType: "AWS", + ownerAccount: "123456789012", + asn: 65000, + awsName: "", + wantErr: true, + errText: "Invalid AWS connection name: - cannot be empty", + }, } for _, tt := range tests { @@ -1125,9 +1138,10 @@ func TestValidateVXCPartnerConfig(t *testing.T) { { name: "Valid AWS partner config", config: &megaport.VXCPartnerConfigAWS{ // Use struct pointer - ConnectType: "AWS", - OwnerAccount: "123456789012", - ASN: 65000, + ConnectType: "AWS", + OwnerAccount: "123456789012", + ASN: 65000, + ConnectionName: "MyAWSConnection", }, wantErr: false, }, From 014f9d8907772600ef63b5060041d0d102f0742a Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 14:11:04 -0700 Subject: [PATCH 07/18] fix(mcr): use consistent error shape for empty prefix filter entries The empty-prefix check used fieldName="entry prefix index" with the index as the value, while the sibling CIDR checks added in this PR use fieldName="entry prefix index " with the prefix as the value. Align the empty-prefix error to the same shape and add test coverage for it. --- internal/validation/mcr.go | 2 +- internal/validation/mcr_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index e2419638..6eb56b54 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -128,7 +128,7 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, addressFamily string) error { for i, entry := range entries { if entry.Prefix == "" { - return NewValidationError("entry prefix index", i, "prefix cannot be empty") + return NewValidationError(fmt.Sprintf("entry prefix index %d", i), entry.Prefix, "prefix cannot be empty") } if addressFamily == "IPv4" { if err := ValidateCIDR(entry.Prefix, fmt.Sprintf("entry prefix index %d", i)); err != nil { diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index f3315e93..e37ac585 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -216,6 +216,21 @@ func TestValidatePrefixFilterListRequest(t *testing.T) { }, wantErr: false, }, + { + name: "Empty entry prefix", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: ""}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry prefix index 0: - prefix cannot be empty", + }, { name: "Missing description", req: &megaport.CreateMCRPrefixFilterListRequest{ From 663b7794d8741c2b7121baecaa3d51ab5a169044 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 17:23:07 -0700 Subject: [PATCH 08/18] fix(vxc): require IBM connection name in shared validator ValidateIBMPartnerConfig allowed an empty Name and only checked length/charset when non-empty, while the interactive prompt path (promptIBMConfig) has long required a name. This is the same class of gap as the AWS connection name fix earlier in this PR: JSON and flag-built configs funnel through this shared validator too, so the gap let those paths bypass a requirement the prompt enforces. Update the two test fixtures that constructed VXCPartnerConfigIBM without a name. --- internal/validation/vxc.go | 11 +++++++---- internal/validation/vxc_test.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 337e1b3b..83d082ef 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -346,8 +346,8 @@ func ValidateOraclePartnerConfig(config *megaport.VXCPartnerConfigOracle) error // - Account ID must be provided // - Account ID must be exactly 32 characters (IBMAccountIDLength) // - Account ID must contain only hexadecimal characters (0-9, a-f, A-F) -// - If connection name is provided, it must not exceed the maximum length (MaxIBMNameLength) -// - If connection name is provided, it must contain only allowed characters (0-9, a-z, A-Z, /, -, _, ,) +// - Connection name must be provided and not exceed the maximum length (MaxIBMNameLength) +// - Connection name must contain only allowed characters (0-9, a-z, A-Z, /, -, _, ,) // - If customer IP address is provided, it must be in valid IPv4 CIDR notation // - If provider IP address is provided, it must be in valid IPv4 CIDR notation // @@ -366,10 +366,13 @@ func ValidateIBMPartnerConfig(config *megaport.VXCPartnerConfigIBM) error { return NewValidationError("IBM account ID", config.AccountID, "must contain only hexadecimal characters (0-9, a-f, A-F)") } } - if config.Name != "" && utf8.RuneCountInString(config.Name) > MaxIBMNameLength { + if config.Name == "" { + return NewValidationError("IBM connection name", config.Name, "cannot be empty") + } + if utf8.RuneCountInString(config.Name) > MaxIBMNameLength { return NewValidationError("IBM connection name", config.Name, fmt.Sprintf("cannot exceed %d characters", MaxIBMNameLength)) } - if config.Name != "" && !isValidIBMName(config.Name) { + if !isValidIBMName(config.Name) { return NewValidationError("IBM connection name", config.Name, "must only contain characters 0-9, a-z, A-Z, /, -, _, or ,") } if config.CustomerIPAddress != "" { diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index ef41ee52..9207f69e 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -1093,6 +1093,7 @@ func TestValidateIBMPartnerConfig(t *testing.T) { { name: "Invalid customer IP", accountID: validAccountID, + ibmName: "MyIBMConnection", customerIPAddress: "invalid-ip", wantErr: true, errText: "Invalid IBM customer IP address: invalid-ip - must be a valid IPv4 CIDR notation", @@ -1100,10 +1101,18 @@ func TestValidateIBMPartnerConfig(t *testing.T) { { name: "Invalid provider IP", accountID: validAccountID, + ibmName: "MyIBMConnection", providerIPAddress: "10.1.1.2/33", // Invalid mask wantErr: true, errText: "Invalid IBM provider IP address: 10.1.1.2/33 - must be a valid IPv4 CIDR notation", }, + { + name: "Empty connection name", + accountID: validAccountID, + ibmName: "", + wantErr: true, + errText: "Invalid IBM connection name: - cannot be empty", + }, } for _, tt := range tests { @@ -1174,6 +1183,7 @@ func TestValidateVXCPartnerConfig(t *testing.T) { config: &megaport.VXCPartnerConfigIBM{ // Use struct pointer ConnectType: "IBM", // Assuming ConnectType is needed AccountID: "abcdef0123456789abcdef0123456789", + Name: "MyIBMConnection", }, wantErr: false, }, From 5809ba302512e5145e6ff63e324ef68e40327c6f Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 17:53:50 -0700 Subject: [PATCH 09/18] fix(vxc): add missing Transit case to partner config validator ValidateVXCPartnerConfig had no switch case for VXCPartnerConfigTransit, so any Transit VXC (interactive prompt, JSON, or flags) always failed validation with "Partner configuration type ... is not supported". Transit carries no partner-specific fields, so it now passes through with a nil check like the others. --- internal/validation/vxc.go | 3 +++ internal/validation/vxc_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 83d082ef..31700dc0 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -480,6 +480,7 @@ func ValidateVrouterPartnerConfig(config *megaport.VXCOrderVrouterPartnerConfig) // - ValidateOraclePartnerConfig // - ValidateIBMPartnerConfig // - ValidateVrouterPartnerConfig +// - Transit carries no partner-specific fields, so it always passes // - Configuration type must be one of the supported types // // Returns: @@ -497,6 +498,8 @@ func ValidateVXCPartnerConfig(config megaport.VXCPartnerConfiguration) error { return ValidateOraclePartnerConfig(v) case *megaport.VXCPartnerConfigIBM: return ValidateIBMPartnerConfig(v) + case *megaport.VXCPartnerConfigTransit: + return nil case *megaport.VXCOrderVrouterPartnerConfig: return ValidateVrouterPartnerConfig(v) default: diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index 9207f69e..f4ec77a7 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -1201,6 +1201,13 @@ func TestValidateVXCPartnerConfig(t *testing.T) { }, wantErr: false, }, + { + name: "Valid Transit partner config", + config: &megaport.VXCPartnerConfigTransit{ + ConnectType: "TRANSIT", + }, + wantErr: false, + }, { name: "Missing partner type (nil config)", // Test case for nil config config: nil, From ab3d1733abecf3b369faa38def013e0233de8515 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 18:05:26 -0700 Subject: [PATCH 10/18] fix(vxc): reject empty AWS ASN immediately, align Azure VLAN field name promptAWSConfig let an empty ASN fall through as 0 and only fail later during request validation, unlike the owner account and connection name prompts on the same path which already reject empty input immediately. Also renamed the Azure peering VLAN parse error field from "VLAN" to "VLAN ID" to match ValidateVLAN's error text and the naming convention used elsewhere (MVE, service keys, ports). --- internal/commands/vxc/vxc_prompts_partner.go | 14 +++++++------- internal/commands/vxc/vxc_prompts_test.go | 5 +++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index 3fb48e3d..e1d2446f 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -113,12 +113,12 @@ func promptAWSConfig(noColor bool) (*megaport.VXCPartnerConfigAWS, error) { if err != nil { return nil, err } - var asn int - if asnStr != "" { - asn, err = validation.ParseInt("ASN", asnStr) - if err != nil { - return nil, err - } + if asnStr == "" { + return nil, fmt.Errorf("ASN is required") + } + asn, err := validation.ParseInt("ASN", asnStr) + if err != nil { + return nil, err } amazonASNStr, err := utils.ResourcePrompt("vxc", "Enter Amazon ASN (optional): ", noColor) @@ -278,7 +278,7 @@ func promptAzurePeeringConfig(noColor bool) (megaport.PartnerOrderAzurePeeringCo } var vlan int if vlanStr != "" { - vlan, err = validation.ParseInt("VLAN", vlanStr) + vlan, err = validation.ParseInt("VLAN ID", vlanStr) if err != nil { return megaport.PartnerOrderAzurePeeringConfig{}, err } diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 333d996d..5a733ea9 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -94,6 +94,11 @@ func TestPromptAWSConfig_RequiresCredentials(t *testing.T) { responses: []string{"AWS", "123456789", ""}, errContains: "connection name is required", }, + { + name: "empty ASN rejected", + responses: []string{"AWS", "123456789", "my-conn", ""}, + errContains: "ASN is required", + }, } for _, tc := range tests { From f13213546cfd69ff78eea787657017ca3204b1cf Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 18:18:11 -0700 Subject: [PATCH 11/18] fix(vxc): reject nil or mistyped Transit partner config ValidateVXCPartnerConfig treated any *VXCPartnerConfigTransit as always valid, so a nil pointer or a wrong ConnectType would pass validation and only fail later against the API. Extracted a ValidateTransitPartnerConfig function that checks both, matching the nil and connect-type checks the other partner validators already do. --- internal/validation/vxc.go | 18 ++++++++-- internal/validation/vxc_test.go | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 31700dc0..22e6c292 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -398,6 +398,20 @@ func isValidIBMName(name string) bool { return true } +// ValidateTransitPartnerConfig validates a Transit partner configuration for a +// VXC connection. Transit carries no partner-specific fields beyond the +// connect type, so this only guards against a nil config or a connect type +// that doesn't match "TRANSIT". +func ValidateTransitPartnerConfig(config *megaport.VXCPartnerConfigTransit) error { + if config == nil { + return NewValidationError("Transit partner config", nil, "cannot be nil") + } + if config.ConnectType != "TRANSIT" { + return NewValidationError("Transit connect type", config.ConnectType, "must be 'TRANSIT'") + } + return nil +} + // ValidateVrouterPartnerConfig validates a vRouter partner configuration: it // requires at least one interface and validates each interface's VLAN, IP // addresses, NAT IPs, routes, BFD, BGP connections, interface type, and IPsec @@ -480,7 +494,7 @@ func ValidateVrouterPartnerConfig(config *megaport.VXCOrderVrouterPartnerConfig) // - ValidateOraclePartnerConfig // - ValidateIBMPartnerConfig // - ValidateVrouterPartnerConfig -// - Transit carries no partner-specific fields, so it always passes +// - ValidateTransitPartnerConfig // - Configuration type must be one of the supported types // // Returns: @@ -499,7 +513,7 @@ func ValidateVXCPartnerConfig(config megaport.VXCPartnerConfiguration) error { case *megaport.VXCPartnerConfigIBM: return ValidateIBMPartnerConfig(v) case *megaport.VXCPartnerConfigTransit: - return nil + return ValidateTransitPartnerConfig(v) case *megaport.VXCOrderVrouterPartnerConfig: return ValidateVrouterPartnerConfig(v) default: diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index f4ec77a7..4f745552 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -1247,6 +1247,14 @@ func TestValidateVXCPartnerConfig(t *testing.T) { wantErr: true, errText: "Invalid Azure service key: - cannot be empty", }, + { + name: "Invalid Transit config details", + config: &megaport.VXCPartnerConfigTransit{ + ConnectType: "", // Invalid connect type + }, + wantErr: true, + errText: "Invalid Transit connect type: - must be 'TRANSIT'", + }, { name: "Invalid vRouter config details", config: &megaport.VXCOrderVrouterPartnerConfig{ @@ -1276,6 +1284,59 @@ func TestValidateVXCPartnerConfig(t *testing.T) { } } +func TestValidateTransitPartnerConfig(t *testing.T) { + tests := []struct { + name string + config *megaport.VXCPartnerConfigTransit + wantErr bool + errText string + }{ + { + name: "Valid Transit config", + config: &megaport.VXCPartnerConfigTransit{ + ConnectType: "TRANSIT", + }, + wantErr: false, + }, + { + name: "Nil config", + config: nil, + wantErr: true, + errText: "Invalid Transit partner config: - cannot be nil", + }, + { + name: "Empty connect type", + config: &megaport.VXCPartnerConfigTransit{ + ConnectType: "", + }, + wantErr: true, + errText: "Invalid Transit connect type: - must be 'TRANSIT'", + }, + { + name: "Wrong connect type", + config: &megaport.VXCPartnerConfigTransit{ + ConnectType: "AWS", + }, + wantErr: true, + errText: "Invalid Transit connect type: AWS - must be 'TRANSIT'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTransitPartnerConfig(tt.config) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateTransitPartnerConfig() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil && tt.wantErr { + assert.IsType(t, &ValidationError{}, err, "Expected ValidationError type") + assert.Equal(t, tt.errText, err.Error(), "Error message mismatch") + } + }) + } +} + func TestIsValidIBMName(t *testing.T) { tests := []struct { name string From 38a73b3d7f0c5cb129f473a18bb374f9da7f7a0d Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 18:31:22 -0700 Subject: [PATCH 12/18] fix(validation): guard against nil partner configs and nil prefix entries ValidateAWSPartnerConfig and ValidateIBMPartnerConfig could panic on a typed-nil pointer reaching the dispatch switch, unlike their Azure, vRouter, and Transit siblings which already nil-check. Also guard validatePrefixFilterEntries against a nil entry in the list, which would otherwise panic instead of returning a validation error. --- internal/validation/mcr.go | 3 +++ internal/validation/mcr_test.go | 16 ++++++++++++++++ internal/validation/vxc.go | 8 ++++++++ internal/validation/vxc_test.go | 12 ++++++++++++ 4 files changed, 39 insertions(+) diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 6eb56b54..0fdead8c 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -127,6 +127,9 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ // action is permit or deny. func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, addressFamily string) error { for i, entry := range entries { + if entry == nil { + return NewValidationError(fmt.Sprintf("entry index %d", i), nil, "entry cannot be nil") + } if entry.Prefix == "" { return NewValidationError(fmt.Sprintf("entry prefix index %d", i), entry.Prefix, "prefix cannot be empty") } diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index e37ac585..39b4ed1d 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -302,6 +302,22 @@ func TestValidatePrefixFilterListRequest(t *testing.T) { wantErr: true, errText: "Invalid entries: [] - must contain at least one entry", }, + { + name: "Nil entry in entries", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8"}, + nil, + }, + }, + }, + wantErr: true, + errText: "Invalid entry index 1: - entry cannot be nil", + }, } for _, tt := range tests { diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 22e6c292..4b2a1869 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -194,6 +194,7 @@ func ValidateVXCRequest(req *megaport.BuyVXCRequest) error { // - config: The AWS partner configuration to validate // // Validation checks include: +// - Configuration cannot be nil // - Connect type must be provided and be one of the valid types ('AWS', 'AWSHC', 'private', 'public') // - Owner account must be provided (AWS account ID) // - ASN must be provided and within the valid range (1-4294967295) @@ -206,6 +207,9 @@ func ValidateVXCRequest(req *megaport.BuyVXCRequest) error { // - A ValidationError if any validation check fails // - nil if all validation checks pass func ValidateAWSPartnerConfig(config *megaport.VXCPartnerConfigAWS) error { + if config == nil { + return NewValidationError("AWS partner config", nil, "cannot be nil") + } if config.ConnectType == "" { return NewValidationError("AWS connect type", config.ConnectType, "cannot be empty") } @@ -343,6 +347,7 @@ func ValidateOraclePartnerConfig(config *megaport.VXCPartnerConfigOracle) error // - config: The IBM partner configuration to validate // // Validation checks include: +// - Configuration cannot be nil // - Account ID must be provided // - Account ID must be exactly 32 characters (IBMAccountIDLength) // - Account ID must contain only hexadecimal characters (0-9, a-f, A-F) @@ -355,6 +360,9 @@ func ValidateOraclePartnerConfig(config *megaport.VXCPartnerConfigOracle) error // - A ValidationError if any validation check fails // - nil if all validation checks pass func ValidateIBMPartnerConfig(config *megaport.VXCPartnerConfigIBM) error { + if config == nil { + return NewValidationError("IBM partner config", nil, "cannot be nil") + } if config.AccountID == "" { return NewValidationError("IBM account ID", config.AccountID, "cannot be empty") } diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index 4f745552..3b392033 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -930,6 +930,12 @@ func TestValidateAWSPartnerConfig(t *testing.T) { } } +func TestValidateAWSPartnerConfig_NilConfig(t *testing.T) { + err := ValidateAWSPartnerConfig(nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid AWS partner config: - cannot be nil") +} + // TestValidateAWSPartnerConfig_HighASN mirrors TestValidateBGPConnectionConfig_HighASN: // it confirms the AWS validator agrees with ValidateASN at the 32-bit max boundary. func TestValidateAWSPartnerConfig_HighASN(t *testing.T) { @@ -1137,6 +1143,12 @@ func TestValidateIBMPartnerConfig(t *testing.T) { } } +func TestValidateIBMPartnerConfig_NilConfig(t *testing.T) { + err := ValidateIBMPartnerConfig(nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid IBM partner config: - cannot be nil") +} + func TestValidateVXCPartnerConfig(t *testing.T) { tests := []struct { name string From e704b6639dd83b274ccaae1423cdb5b369d1d2c2 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 19:15:52 -0700 Subject: [PATCH 13/18] fix(vxc): nil-guard Google/Oracle partner configs, fix IBM ASN prompt Google and Oracle partner config validators lacked the nil-guard that AWS, IBM, Azure, vRouter, and Transit already have, so a nil pointer would panic instead of returning a validation error. The IBM interactive prompt also required a customer ASN unconditionally, even though the prompt text and validator both treat it as optional when the opposite end is an MCR. --- internal/commands/vxc/vxc_prompts_partner.go | 11 ++++++----- internal/commands/vxc/vxc_prompts_test.go | 12 ++++++++++++ internal/validation/vxc.go | 8 ++++++++ internal/validation/vxc_test.go | 12 ++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index e1d2446f..45e7f5c7 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -349,15 +349,16 @@ func promptIBMConfig(noColor bool) (*megaport.VXCPartnerConfigIBM, error) { return nil, fmt.Errorf("name is required") } - var customerASN int - customerASNStr, err := utils.ResourcePrompt("vxc", "Enter customer ASN (required if opposite end is not an MCR): ", noColor) if err != nil { return nil, err } - customerASN, err = validation.ParseInt("customer ASN", customerASNStr) - if err != nil { - return nil, err + var customerASN int + if customerASNStr != "" { + customerASN, err = validation.ParseInt("customer ASN", customerASNStr) + if err != nil { + return nil, err + } } customerIPAddress, err := utils.ResourcePrompt("vxc", "Enter customer IP address (optional): ", noColor) diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 5a733ea9..9f34ebc8 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -228,6 +228,18 @@ func TestPromptIBMConfig(t *testing.T) { assert.Equal(t, "6.7.8.9", cfg.ProviderIPAddress) }, }, + { + name: "customer ASN left blank", + responses: []string{"acct-789", "ibm-blank-asn", "", "3.4.5.6", "7.8.9.10"}, + verify: func(t *testing.T, cfg *megaport.VXCPartnerConfigIBM) { + assert.Equal(t, "IBM", cfg.ConnectType) + assert.Equal(t, "acct-789", cfg.AccountID) + assert.Equal(t, "ibm-blank-asn", cfg.Name) + assert.Equal(t, 0, cfg.CustomerASN) + assert.Equal(t, "3.4.5.6", cfg.CustomerIPAddress) + assert.Equal(t, "7.8.9.10", cfg.ProviderIPAddress) + }, + }, } for _, tc := range tests { diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 4b2a1869..4a094ac4 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -309,12 +309,16 @@ func ValidateAzurePartnerConfig(config *megaport.VXCPartnerConfigAzure) error { // - config: The Google partner configuration to validate // // Validation checks include: +// - Configuration cannot be nil // - Pairing key must be provided (required for Google Cloud connections) // // Returns: // - A ValidationError if any validation check fails // - nil if all validation checks pass func ValidateGooglePartnerConfig(config *megaport.VXCPartnerConfigGoogle) error { + if config == nil { + return NewValidationError("Google partner config", nil, "cannot be nil") + } if config.PairingKey == "" { return NewValidationError("Google pairing key", config.PairingKey, "cannot be empty") } @@ -328,12 +332,16 @@ func ValidateGooglePartnerConfig(config *megaport.VXCPartnerConfigGoogle) error // - config: The Oracle partner configuration to validate // // Validation checks include: +// - Configuration cannot be nil // - Virtual Circuit ID must be provided (required for Oracle Cloud connections) // // Returns: // - A ValidationError if any validation check fails // - nil if all validation checks pass func ValidateOraclePartnerConfig(config *megaport.VXCPartnerConfigOracle) error { + if config == nil { + return NewValidationError("Oracle partner config", nil, "cannot be nil") + } if config.VirtualCircuitId == "" { return NewValidationError("Oracle virtual circuit ID", config.VirtualCircuitId, "cannot be empty") } diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index 3b392033..1b9411e2 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -1008,6 +1008,12 @@ func TestValidateGooglePartnerConfig(t *testing.T) { } } +func TestValidateGooglePartnerConfig_NilConfig(t *testing.T) { + err := ValidateGooglePartnerConfig(nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid Google partner config: - cannot be nil") +} + func TestValidateOraclePartnerConfig(t *testing.T) { tests := []struct { name string @@ -1037,6 +1043,12 @@ func TestValidateOraclePartnerConfig(t *testing.T) { } } +func TestValidateOraclePartnerConfig_NilConfig(t *testing.T) { + err := ValidateOraclePartnerConfig(nil) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Invalid Oracle partner config: - cannot be nil") +} + func TestValidateIBMPartnerConfig(t *testing.T) { validAccountID := "abcdef0123456789abcdef0123456789" // 32 hex chars tests := []struct { From 940494adc91a40f74b85971ce693e28fb5a7a721 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 19:29:04 -0700 Subject: [PATCH 14/18] fix(vxc): make IBM connection name optional to match the API contract The IBM partner config name is optional in the API (it defaults to "MEGAPORT" server-side), and the SDK and Terraform provider both treat it that way. The validator was requiring it, which broke JSON and flag inputs that omitted the name, and the interactive prompt required it too. Length and character-set checks still apply when a name is given. --- internal/commands/vxc/vxc_prompts_partner.go | 5 +---- internal/commands/vxc/vxc_prompts_test.go | 10 ++++++++++ internal/validation/vxc.go | 20 ++++++++++---------- internal/validation/vxc_test.go | 5 ++--- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index 45e7f5c7..a1348d03 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -341,13 +341,10 @@ func promptIBMConfig(noColor bool) (*megaport.VXCPartnerConfigIBM, error) { return nil, fmt.Errorf("account ID is required") } - name, err := utils.ResourcePrompt("vxc", "Enter name (required): ", noColor) + name, err := utils.ResourcePrompt("vxc", "Enter name (optional, defaults to MEGAPORT): ", noColor) if err != nil { return nil, err } - if name == "" { - return nil, fmt.Errorf("name is required") - } customerASNStr, err := utils.ResourcePrompt("vxc", "Enter customer ASN (required if opposite end is not an MCR): ", noColor) if err != nil { diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 9f34ebc8..10529568 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -228,6 +228,16 @@ func TestPromptIBMConfig(t *testing.T) { assert.Equal(t, "6.7.8.9", cfg.ProviderIPAddress) }, }, + { + name: "name left blank (API defaults to MEGAPORT)", + responses: []string{"acct-000", "", "65001", "4.5.6.7", "8.9.10.11"}, + verify: func(t *testing.T, cfg *megaport.VXCPartnerConfigIBM) { + assert.Equal(t, "IBM", cfg.ConnectType) + assert.Equal(t, "acct-000", cfg.AccountID) + assert.Equal(t, "", cfg.Name) + assert.Equal(t, 65001, cfg.CustomerASN) + }, + }, { name: "customer ASN left blank", responses: []string{"acct-789", "ibm-blank-asn", "", "3.4.5.6", "7.8.9.10"}, diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 4a094ac4..485edfd8 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -359,8 +359,9 @@ func ValidateOraclePartnerConfig(config *megaport.VXCPartnerConfigOracle) error // - Account ID must be provided // - Account ID must be exactly 32 characters (IBMAccountIDLength) // - Account ID must contain only hexadecimal characters (0-9, a-f, A-F) -// - Connection name must be provided and not exceed the maximum length (MaxIBMNameLength) -// - Connection name must contain only allowed characters (0-9, a-z, A-Z, /, -, _, ,) +// - Connection name is optional (the API defaults it to "MEGAPORT"); when +// provided it must not exceed the maximum length (MaxIBMNameLength) and +// must contain only allowed characters (0-9, a-z, A-Z, /, -, _, ,) // - If customer IP address is provided, it must be in valid IPv4 CIDR notation // - If provider IP address is provided, it must be in valid IPv4 CIDR notation // @@ -382,14 +383,13 @@ func ValidateIBMPartnerConfig(config *megaport.VXCPartnerConfigIBM) error { return NewValidationError("IBM account ID", config.AccountID, "must contain only hexadecimal characters (0-9, a-f, A-F)") } } - if config.Name == "" { - return NewValidationError("IBM connection name", config.Name, "cannot be empty") - } - if utf8.RuneCountInString(config.Name) > MaxIBMNameLength { - return NewValidationError("IBM connection name", config.Name, fmt.Sprintf("cannot exceed %d characters", MaxIBMNameLength)) - } - if !isValidIBMName(config.Name) { - return NewValidationError("IBM connection name", config.Name, "must only contain characters 0-9, a-z, A-Z, /, -, _, or ,") + if config.Name != "" { + if utf8.RuneCountInString(config.Name) > MaxIBMNameLength { + return NewValidationError("IBM connection name", config.Name, fmt.Sprintf("cannot exceed %d characters", MaxIBMNameLength)) + } + if !isValidIBMName(config.Name) { + return NewValidationError("IBM connection name", config.Name, "must only contain characters 0-9, a-z, A-Z, /, -, _, or ,") + } } if config.CustomerIPAddress != "" { if err := ValidateCIDR(config.CustomerIPAddress, "IBM customer IP address"); err != nil { diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index 1b9411e2..f405b7e9 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -1125,11 +1125,10 @@ func TestValidateIBMPartnerConfig(t *testing.T) { errText: "Invalid IBM provider IP address: 10.1.1.2/33 - must be a valid IPv4 CIDR notation", }, { - name: "Empty connection name", + name: "Empty connection name is valid (API defaults to MEGAPORT)", accountID: validAccountID, ibmName: "", - wantErr: true, - errText: "Invalid IBM connection name: - cannot be empty", + wantErr: false, }, } From 622a557568940d4d19fc66c1599933d0954a397b Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Fri, 10 Jul 2026 22:57:19 -0700 Subject: [PATCH 15/18] fix(validation): close round-3 review gaps in prompts and validators - validate interactive MCR prefix filter list creation before returning - require pairing key / virtual circuit ID in Google and Oracle prompts - enforce MVE name length on update requests - validate prefix filter entry GE/LE bounds per address family and GE <= LE --- internal/commands/mcr/mcr_prompts.go | 4 + internal/commands/mcr/mcr_prompts_test.go | 32 ++++++ internal/commands/vxc/vxc_prompts_partner.go | 6 + internal/commands/vxc/vxc_prompts_test.go | 22 ++++ internal/validation/mcr.go | 18 ++- internal/validation/mcr_test.go | 115 +++++++++++++++++++ internal/validation/mve.go | 5 + internal/validation/mve_test.go | 17 +++ 8 files changed, 217 insertions(+), 2 deletions(-) diff --git a/internal/commands/mcr/mcr_prompts.go b/internal/commands/mcr/mcr_prompts.go index 7460888d..931de676 100644 --- a/internal/commands/mcr/mcr_prompts.go +++ b/internal/commands/mcr/mcr_prompts.go @@ -431,6 +431,10 @@ func promptForPrefixFilterListDetails(mcrUID string, noColor bool) (*megaport.Cr }, } + if err := validation.ValidatePrefixFilterListRequest(req); err != nil { + return nil, err + } + return req, nil } diff --git a/internal/commands/mcr/mcr_prompts_test.go b/internal/commands/mcr/mcr_prompts_test.go index 51d08a83..17ce0726 100644 --- a/internal/commands/mcr/mcr_prompts_test.go +++ b/internal/commands/mcr/mcr_prompts_test.go @@ -413,6 +413,38 @@ func TestPromptForPrefixFilterListDetails_NoEntries(t *testing.T) { assert.Contains(t, err.Error(), "at least one entry is required") } +func TestPromptForPrefixFilterListDetails_InvalidPrefix(t *testing.T) { + originalPrompt := utils.GetResourcePrompt() + defer func() { utils.SetResourcePrompt(originalPrompt) }() + + // Prefix that isn't a CIDR passes the entry prompt but must fail + // request-level validation before anything reaches the API. + utils.SetResourcePrompt(mockPromptSequence([]string{ + "My PFL", "IPv4", + "banana", "permit", "", "", + "", // stop adding entries + })) + + _, err := promptForPrefixFilterListDetails("mcr-123", true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "must be a valid IPv4 CIDR notation") +} + +func TestPromptForPrefixFilterListDetails_WrongFamilyPrefix(t *testing.T) { + originalPrompt := utils.GetResourcePrompt() + defer func() { utils.SetResourcePrompt(originalPrompt) }() + + utils.SetResourcePrompt(mockPromptSequence([]string{ + "My PFL", "IPv6", + "10.0.0.0/8", "permit", "", "", + "", // stop adding entries + })) + + _, err := promptForPrefixFilterListDetails("mcr-123", true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "must be a valid IPv6 CIDR notation") +} + func TestPromptForIPSecTunnelCount(t *testing.T) { tests := []struct { name string diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index a1348d03..e7fc694c 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -303,6 +303,9 @@ func promptGoogleConfig(ctx context.Context, svc megaport.VXCService, noColor bo if err != nil { return nil, "", err } + if pairingKey == "" { + return nil, "", fmt.Errorf("pairing key is required") + } uid, err := getPartnerPortUID(ctx, svc, pairingKey, "GOOGLE") if err != nil { @@ -320,6 +323,9 @@ func promptOracleConfig(ctx context.Context, svc megaport.VXCService, noColor bo if err != nil { return nil, "", err } + if virtualCircuitId == "" { + return nil, "", fmt.Errorf("virtual circuit ID is required") + } uid, err := getPartnerPortUID(ctx, svc, virtualCircuitId, "ORACLE") if err != nil { diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 10529568..91cb6a81 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -157,6 +157,17 @@ func TestPromptGoogleConfig(t *testing.T) { } } +func TestPromptGoogleConfig_RequiresPairingKey(t *testing.T) { + cleanup := mockPrompts([]string{""}) + defer cleanup() + + cfg, uid, err := promptGoogleConfig(context.Background(), &MockVXCService{}, true) + assert.Error(t, err) + assert.Nil(t, cfg) + assert.Empty(t, uid) + assert.Contains(t, err.Error(), "pairing key is required") +} + func TestPromptOracleConfig(t *testing.T) { tests := []struct { name string @@ -198,6 +209,17 @@ func TestPromptOracleConfig(t *testing.T) { } } +func TestPromptOracleConfig_RequiresVirtualCircuitID(t *testing.T) { + cleanup := mockPrompts([]string{""}) + defer cleanup() + + cfg, uid, err := promptOracleConfig(context.Background(), &MockVXCService{}, true) + assert.Error(t, err) + assert.Nil(t, cfg) + assert.Empty(t, uid) + assert.Contains(t, err.Error(), "virtual circuit ID is required") +} + func TestPromptIBMConfig(t *testing.T) { tests := []struct { name string diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 0fdead8c..311bae59 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -123,9 +123,14 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ } // validatePrefixFilterEntries validates each prefix filter entry's prefix as a -// CIDR consistent with the list's declared address family, and that the -// action is permit or deny. +// CIDR consistent with the list's declared address family, that the action is +// permit or deny, and that any GE/LE bounds fit the family's prefix length +// (GE/LE are optional; 0 means unset, matching their omitempty JSON encoding). func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, addressFamily string) error { + maxPrefixLen := 32 + if addressFamily == "IPv6" { + maxPrefixLen = 128 + } for i, entry := range entries { if entry == nil { return NewValidationError(fmt.Sprintf("entry index %d", i), nil, "entry cannot be nil") @@ -146,6 +151,15 @@ func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, address if entry.Action != "permit" && entry.Action != "deny" { return NewValidationError("entry action", entry.Action, "must be permit or deny") } + if entry.Ge != 0 && (entry.Ge < 0 || entry.Ge > maxPrefixLen) { + return NewValidationError(fmt.Sprintf("entry GE index %d", i), entry.Ge, fmt.Sprintf("must be between 0 and %d for %s", maxPrefixLen, addressFamily)) + } + if entry.Le != 0 && (entry.Le < 0 || entry.Le > maxPrefixLen) { + return NewValidationError(fmt.Sprintf("entry LE index %d", i), entry.Le, fmt.Sprintf("must be between 0 and %d for %s", maxPrefixLen, addressFamily)) + } + if entry.Ge != 0 && entry.Le != 0 && entry.Ge > entry.Le { + return NewValidationError(fmt.Sprintf("entry GE index %d", i), entry.Ge, "must not exceed the LE value") + } } return nil } diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index 39b4ed1d..68bf1b8e 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -318,6 +318,109 @@ func TestValidatePrefixFilterListRequest(t *testing.T) { wantErr: true, errText: "Invalid entry index 1: - entry cannot be nil", }, + { + name: "Valid GE/LE bounds", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Ge: 16, Le: 24}, + }, + }, + }, + wantErr: false, + }, + { + name: "GE above IPv4 maximum", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Ge: 33}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry GE index 0: 33 - must be between 0 and 32 for IPv4", + }, + { + name: "Negative GE", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Ge: -1}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry GE index 0: -1 - must be between 0 and 32 for IPv4", + }, + { + name: "LE above IPv4 maximum", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Le: 33}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry LE index 0: 33 - must be between 0 and 32 for IPv4", + }, + { + name: "GE greater than LE", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Ge: 28, Le: 24}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry GE index 0: 28 - must not exceed the LE value", + }, + { + name: "IPv6 GE/LE beyond IPv4 range accepted", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv6", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "2001:db8::/32", Ge: 48, Le: 64}, + }, + }, + }, + wantErr: false, + }, + { + name: "IPv6 LE above maximum", + req: &megaport.CreateMCRPrefixFilterListRequest{ + MCRID: "mcr-uid-123", + PrefixFilterList: megaport.MCRPrefixFilterList{ + Description: "Test filter list", + AddressFamily: "IPv6", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "2001:db8::/32", Le: 129}, + }, + }, + }, + wantErr: true, + errText: "Invalid entry LE index 0: 129 - must be between 0 and 128 for IPv6", + }, } for _, tt := range tests { @@ -409,6 +512,18 @@ func TestValidateUpdatePrefixFilterList(t *testing.T) { }, wantErr: false, }, + { + name: "GE greater than LE rejected on update", + req: &megaport.MCRPrefixFilterList{ + Description: "Updated filter list", + AddressFamily: "IPv4", + Entries: []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8", Ge: 28, Le: 24}, + }, + }, + wantErr: true, + errText: "Invalid entry GE index 0: 28 - must not exceed the LE value", + }, { name: "Empty address family with entries rejected", req: &megaport.MCRPrefixFilterList{ diff --git a/internal/validation/mve.go b/internal/validation/mve.go index e4bed4a3..7d5fd022 100644 --- a/internal/validation/mve.go +++ b/internal/validation/mve.go @@ -121,6 +121,7 @@ func ValidateBuyMVERequest(req *megaport.BuyMVERequest) error { // // Validation checks: // - At least one updateable field must be provided (name, cost center, contract term, or vNICs) +// - If a name is provided, it cannot exceed the maximum length (MaxMVENameLength) // - If contract term is provided, it must be valid (typically 1, 12, 24, 36, 48, or 60 months) // // Returns: @@ -135,6 +136,10 @@ func ValidateUpdateMVERequest(req *megaport.ModifyMVERequest) error { return NewValidationError("update request", req, "at least one field must be provided for update") } + if req.Name != "" && utf8.RuneCountInString(req.Name) > MaxMVENameLength { + return NewValidationError("MVE name", req.Name, fmt.Sprintf("cannot exceed %d characters", MaxMVENameLength)) + } + // If contract term is provided, validate it if req.ContractTermMonths != nil { term := *req.ContractTermMonths diff --git a/internal/validation/mve_test.go b/internal/validation/mve_test.go index 632ac407..a2d9011d 100644 --- a/internal/validation/mve_test.go +++ b/internal/validation/mve_test.go @@ -467,6 +467,23 @@ func TestValidateUpdateMVERequest(t *testing.T) { wantErr: true, errText: "vnics[1].description", }, + { + name: "Update name at max length accepted", + req: &megaport.ModifyMVERequest{ + MVEID: "mve-uid-123", + Name: strings.Repeat("日", MaxMVENameLength), + }, + wantErr: false, + }, + { + name: "Update name exceeding max length rejected", + req: &megaport.ModifyMVERequest{ + MVEID: "mve-uid-123", + Name: strings.Repeat("日", MaxMVENameLength+1), + }, + wantErr: true, + errText: fmt.Sprintf("cannot exceed %d characters", MaxMVENameLength), + }, } for _, tt := range tests { From 1630b872a2667dd672ae0e1eefe58888afad2b15 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Sat, 11 Jul 2026 08:21:48 -0700 Subject: [PATCH 16/18] chore: extract conversion + tag-editing fixes to separate PRs The GetIntFromInterface int-width bound fix and the interactive tag-editing empty-value fix are orthogonal to the prompt/validator parity work here, so they were pulled into their own PRs: - GetIntFromInterface overflow bound -> #538 - interactive tag-edit empty-value handling -> #539 Restores those six files to the main baseline; this PR now stays focused on partner-config and prefix-filter validation parity. --- internal/utils/prompts.go | 14 +++++-------- internal/utils/prompts_test.go | 28 -------------------------- internal/utils/prompts_wasm.go | 14 +++++-------- internal/utils/prompts_wasm_test.go | 18 ----------------- internal/validation/conversion.go | 12 +++-------- internal/validation/conversion_test.go | 7 ------- 6 files changed, 13 insertions(+), 80 deletions(-) diff --git a/internal/utils/prompts.go b/internal/utils/prompts.go index 7282d02d..889f24dd 100644 --- a/internal/utils/prompts.go +++ b/internal/utils/prompts.go @@ -252,15 +252,11 @@ var updateResourceTagsPromptFn = func(existingTags map[string]string, noColor bo return nil, err } - // Empty value means remove the tag, if it exists. - if value == "" { - if _, exists := tags[key]; exists { - delete(tags, key) - fmt.Fprintf(os.Stderr, " Removed tag: %s\n", key) - } else { - fmt.Fprintf(os.Stderr, " Tag '%s' does not exist, nothing to remove\n", key) - } - } else { + // Empty value means remove the tag + if value == "" && tags[key] != "" { + delete(tags, key) + fmt.Fprintf(os.Stderr, " Removed tag: %s\n", key) + } else if value != "" { tags[key] = value fmt.Fprintf(os.Stderr, " Updated tag: %s: %s\n", key, value) } diff --git a/internal/utils/prompts_test.go b/internal/utils/prompts_test.go index 369fda64..1639dfa7 100644 --- a/internal/utils/prompts_test.go +++ b/internal/utils/prompts_test.go @@ -703,34 +703,6 @@ func TestUpdateResourceTagsPrompt_DefaultModifyExistingRemovesTag(t *testing.T) assert.Empty(t, stdout) } -// TestUpdateResourceTagsPrompt_DefaultModifyRemovesEmptyValueTag exercises -// removing an existing tag whose current value is already empty. -func TestUpdateResourceTagsPrompt_DefaultModifyRemovesEmptyValueTag(t *testing.T) { - mockPromptSequence(t, []bool{true, true}, []string{"2", "foo", "", ""}) - stdout, stderr := withMockedIO("", func() { - tags, err := UpdateResourceTagsPrompt(map[string]string{"foo": ""}, true) - assert.NoError(t, err) - assert.Empty(t, tags) - }) - assert.Contains(t, stderr, "Removed tag: foo") - assert.Empty(t, stdout) -} - -// TestUpdateResourceTagsPrompt_DefaultModifyEmptyValueForNewKey exercises -// entering an empty value for a key that does not exist yet: it must not be -// added to the tag set, and the user gets a clear "nothing to remove" notice -// instead of the key silently vanishing. -func TestUpdateResourceTagsPrompt_DefaultModifyEmptyValueForNewKey(t *testing.T) { - mockPromptSequence(t, []bool{true, true}, []string{"2", "baz", "", ""}) - stdout, stderr := withMockedIO("", func() { - tags, err := UpdateResourceTagsPrompt(map[string]string{"foo": "bar"}, true) - assert.NoError(t, err) - assert.Equal(t, map[string]string{"foo": "bar"}, tags) - }) - assert.Contains(t, stderr, "Tag 'baz' does not exist, nothing to remove") - assert.Empty(t, stdout) -} - // TestDesignConfirmPrompt_DefaultRendering verifies the default // designConfirmPromptFn renders design-stage wording instead of // BuyConfirmPrompt's purchase wording. diff --git a/internal/utils/prompts_wasm.go b/internal/utils/prompts_wasm.go index 38148ebe..5dd228c6 100644 --- a/internal/utils/prompts_wasm.go +++ b/internal/utils/prompts_wasm.go @@ -270,15 +270,11 @@ func wasmUpdateResourceTagsPrompt(existingTags map[string]string, noColor bool) return nil, err } - // Empty value means remove the tag, if it exists. - if value == "" { - if _, exists := tags[key]; exists { - delete(tags, key) - buf.add(fmt.Sprintf(" Removed tag: %s", key)) - } else { - buf.add(fmt.Sprintf(" Tag '%s' does not exist, nothing to remove", key)) - } - } else { + // Empty value means remove the tag + if value == "" && tags[key] != "" { + delete(tags, key) + buf.add(fmt.Sprintf(" Removed tag: %s", key)) + } else if value != "" { tags[key] = value buf.add(fmt.Sprintf(" Updated tag: %s: %s", key, value)) } diff --git a/internal/utils/prompts_wasm_test.go b/internal/utils/prompts_wasm_test.go index 85f700a3..6866b0df 100644 --- a/internal/utils/prompts_wasm_test.go +++ b/internal/utils/prompts_wasm_test.go @@ -389,24 +389,6 @@ func TestWasmUpdateResourceTagsPrompt(t *testing.T) { }, expectError: false, }, - { - name: "empty value for key that does not exist is a no-op", - existingTags: map[string]string{ - "foo": "bar", - }, - mockResponses: []string{ - "y", // Continue - "2", // Start with existing - "baz", // Key that does not exist - "", // Empty value = attempt remove - "", // Finish - "y", // Apply changes - }, - expectedTags: map[string]string{ - "foo": "bar", - }, - expectError: false, - }, { name: "no existing tags - add new", existingTags: map[string]string{}, diff --git a/internal/validation/conversion.go b/internal/validation/conversion.go index ab0d2195..c313bab8 100644 --- a/internal/validation/conversion.go +++ b/internal/validation/conversion.go @@ -14,15 +14,9 @@ func GetIntFromInterface(value interface{}) (int, bool) { return v, true case float64: // Reject fractional or out-of-range values rather than silently truncating - // JSON-derived numbers (e.g. 3.9 -> 3). Bounds use math.MaxInt so this - // stays correct across platform int widths. - // - // float64(math.MaxInt) itself rounds up to 2^63 on platforms where int is - // 64 bits (2^63-1 isn't exactly representable), so comparing v against it - // directly would wrongly reject the true max int on platforms where MaxInt - // fits exactly in a float64 (e.g. a 32-bit int). Add 1 to get the true - // exclusive upper bound on either width. - if v != math.Trunc(v) || v < math.MinInt || v >= float64(math.MaxInt)+1 { + // JSON-derived numbers (e.g. 3.9 -> 3). Bounds use platform int width so + // 32-bit targets (js/wasm) don't accept values that overflow int. + if v != math.Trunc(v) || v < math.MinInt || v > math.MaxInt { return 0, false } return int(v), true diff --git a/internal/validation/conversion_test.go b/internal/validation/conversion_test.go index 812ce173..8a13ff36 100644 --- a/internal/validation/conversion_test.go +++ b/internal/validation/conversion_test.go @@ -8,11 +8,6 @@ import ( ) func TestGetIntFromInterface(t *testing.T) { - // Derived from math.MaxInt (rather than a hardcoded 2^63) so the boundary - // cases stay in range regardless of platform int width. - nearMaxInt := float64(math.MaxInt) - 4096 - justAboveMaxInt := float64(math.MaxInt) + 4096 - tests := []struct { name string value interface{} @@ -25,8 +20,6 @@ func TestGetIntFromInterface(t *testing.T) { {"float64 whole", float64(10), 10, true}, {"float64 fractional rejected", float64(3.9), 0, false}, {"float64 above int range rejected", float64(math.MaxInt) * 2, 0, false}, - {"float64 just above max int rejected", justAboveMaxInt, 0, false}, - {"float64 just below max int accepted", nearMaxInt, int(nearMaxInt), true}, {"numeric string", "123", 123, true}, {"negative numeric string", "-5", -5, true}, {"empty string", "", 0, false}, From a4f570a7c8abddcb5b6a00526b4a5745d3242854 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Mon, 13 Jul 2026 07:22:37 -0700 Subject: [PATCH 17/18] fix(vxc): make AWS connection name optional to match the API contract The AWS partner config name is optional in the API (both AwsRequest and AwshcRequest schemas omit it from required fields and default it to "MEGAPORT" server-side), same as IBM. An earlier commit in this PR made it required in the shared validator and the interactive prompt, which blocked previously-valid AWS VXC orders via flags and JSON. Length check still applies when a name is given. --- internal/commands/vxc/vxc_prompts_partner.go | 5 +---- internal/commands/vxc/vxc_prompts_test.go | 15 ++++++++++----- internal/validation/vxc.go | 8 +++----- internal/validation/vxc_test.go | 5 ++--- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/internal/commands/vxc/vxc_prompts_partner.go b/internal/commands/vxc/vxc_prompts_partner.go index e7fc694c..7091f490 100644 --- a/internal/commands/vxc/vxc_prompts_partner.go +++ b/internal/commands/vxc/vxc_prompts_partner.go @@ -101,13 +101,10 @@ func promptAWSConfig(noColor bool) (*megaport.VXCPartnerConfigAWS, error) { return nil, fmt.Errorf("owner account ID is required") } - connectionName, err := utils.ResourcePrompt("vxc", "Enter connection name (required): ", noColor) + connectionName, err := utils.ResourcePrompt("vxc", "Enter connection name (optional, defaults to MEGAPORT): ", noColor) if err != nil { return nil, err } - if connectionName == "" { - return nil, fmt.Errorf("connection name is required") - } asnStr, err := utils.ResourcePrompt("vxc", "Enter ASN (required): ", noColor) if err != nil { diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index 91cb6a81..6fac20c5 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -63,6 +63,16 @@ func TestPromptAWSConfig(t *testing.T) { assert.Equal(t, "", cfg.Type) }, }, + { + name: "connection name left blank (API defaults to MEGAPORT)", + responses: []string{"AWS", "123456789", "", "65000", "", "", "", "", "", "private"}, + verify: func(t *testing.T, cfg *megaport.VXCPartnerConfigAWS) { + assert.Equal(t, "AWS", cfg.ConnectType) + assert.Equal(t, "123456789", cfg.OwnerAccount) + assert.Equal(t, "", cfg.ConnectionName) + assert.Equal(t, 65000, cfg.ASN) + }, + }, } for _, tc := range tests { @@ -89,11 +99,6 @@ func TestPromptAWSConfig_RequiresCredentials(t *testing.T) { responses: []string{"AWS", ""}, errContains: "owner account ID is required", }, - { - name: "empty connection name rejected", - responses: []string{"AWS", "123456789", ""}, - errContains: "connection name is required", - }, { name: "empty ASN rejected", responses: []string{"AWS", "123456789", "my-conn", ""}, diff --git a/internal/validation/vxc.go b/internal/validation/vxc.go index 485edfd8..0b85f94f 100644 --- a/internal/validation/vxc.go +++ b/internal/validation/vxc.go @@ -200,7 +200,8 @@ func ValidateVXCRequest(req *megaport.BuyVXCRequest) error { // - ASN must be provided and within the valid range (1-4294967295) // - If customer IP address is provided, it must be in valid IPv4 CIDR notation // - If Amazon IP address is provided, it must be in valid IPv4 CIDR notation -// - Connection name must be provided and not exceed 255 characters +// - Connection name is optional (the API defaults it to "MEGAPORT"); when +// provided it must not exceed 255 characters // - For 'AWS' connect type with a specified connection type, it must be 'private' or 'public' // // Returns: @@ -229,10 +230,7 @@ func ValidateAWSPartnerConfig(config *megaport.VXCPartnerConfigAWS) error { return NewValidationError("AWS owner account", config.OwnerAccount, "cannot be empty") } - if config.ConnectionName == "" { - return NewValidationError("AWS connection name", config.ConnectionName, "cannot be empty") - } - if utf8.RuneCountInString(config.ConnectionName) > MaxAWSConnectionNameLength { + if config.ConnectionName != "" && utf8.RuneCountInString(config.ConnectionName) > MaxAWSConnectionNameLength { return NewValidationError("AWS connection name", config.ConnectionName, fmt.Sprintf("cannot exceed %d characters", MaxAWSConnectionNameLength)) } if config.CustomerIPAddress != "" { diff --git a/internal/validation/vxc_test.go b/internal/validation/vxc_test.go index f405b7e9..79110b68 100644 --- a/internal/validation/vxc_test.go +++ b/internal/validation/vxc_test.go @@ -894,13 +894,12 @@ func TestValidateAWSPartnerConfig(t *testing.T) { errText: "Invalid AWS type: invalid - must be 'private' or 'public' for AWS connect type", }, { - name: "Empty connection name", + name: "Empty connection name is valid (API defaults to MEGAPORT)", connectType: "AWS", ownerAccount: "123456789012", asn: 65000, awsName: "", - wantErr: true, - errText: "Invalid AWS connection name: - cannot be empty", + wantErr: false, }, } From f9a3ebd7c770c4d085bcb3901a19883104cf8b2b Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Wed, 22 Jul 2026 06:56:57 -0700 Subject: [PATCH 18/18] ESD-1648: Reject unknown address family in prefix filter entry validation validatePrefixFilterEntries treated any non-IPv4 family as IPv6 and defaulted the max prefix length to 32 for unknown values. Replace the implicit branch with a switch that rejects unknown families up front, so the function is self-contained rather than relying on callers to pre-validate. --- internal/validation/mcr.go | 9 +++++++-- internal/validation/mcr_test.go | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/internal/validation/mcr.go b/internal/validation/mcr.go index 311bae59..b362dc66 100644 --- a/internal/validation/mcr.go +++ b/internal/validation/mcr.go @@ -127,9 +127,14 @@ func ValidatePrefixFilterListRequest(req *megaport.CreateMCRPrefixFilterListRequ // permit or deny, and that any GE/LE bounds fit the family's prefix length // (GE/LE are optional; 0 means unset, matching their omitempty JSON encoding). func validatePrefixFilterEntries(entries []*megaport.MCRPrefixListEntry, addressFamily string) error { - maxPrefixLen := 32 - if addressFamily == "IPv6" { + var maxPrefixLen int + switch addressFamily { + case "IPv4": + maxPrefixLen = 32 + case "IPv6": maxPrefixLen = 128 + default: + return NewValidationError("address family", addressFamily, "must be IPv4 or IPv6") } for i, entry := range entries { if entry == nil { diff --git a/internal/validation/mcr_test.go b/internal/validation/mcr_test.go index 68bf1b8e..373e1e12 100644 --- a/internal/validation/mcr_test.go +++ b/internal/validation/mcr_test.go @@ -8,6 +8,15 @@ import ( "github.com/stretchr/testify/assert" ) +func TestValidatePrefixFilterEntriesRejectsUnknownAddressFamily(t *testing.T) { + entries := []*megaport.MCRPrefixListEntry{ + {Action: "permit", Prefix: "10.0.0.0/8"}, + } + err := validatePrefixFilterEntries(entries, "IPv5") + assert.IsType(t, &ValidationError{}, err) + assert.Equal(t, "Invalid address family: IPv5 - must be IPv4 or IPv6", err.Error()) +} + func TestValidateIPSecTunnelCount(t *testing.T) { tests := []struct { name string