fix: produce valid NuGet packages from package nuget create

pkg/cmd/package/nuget/create/create.go

@@ -150,7 +150,14 @@ func createRun(cmd *cobra.Command, opts *NuPkgCreateOptions) error {
 		fmt.Fprintf(opts.Writer, "\nAutomation Command: %s\n", autoCmd)
 	}
 
-	nuget, err := pack.BuildPackage(opts.PackageCreateOptions, outFilePath)
+	// a .nupkg is an OPC container, not a plain zip; it needs its content types,
+	// relationships and core properties or feeds will reject it
+	nuget, err := pack.BuildPackageWithContents(opts.PackageCreateOptions, outFilePath, &pack.PackageContents{
+		ExcludeDirectories: true,
+		ExtraEntries: func(paths []string) ([]pack.ArchiveEntry, error) {
+			return buildOpcParts(opts.Id.Value, opts.Version.Value, opts.Author.Value, opts.Description.Value, paths)
+		},
+	})
 	if nuget != nil {
 		switch outputFormat {
 		case constants.OutputFormatBasic:

pkg/cmd/package/nuget/create/opc.go

@@ -0,0 +1,210 @@
+package create
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/xml"
+	"fmt"
+	"path"
+	"sort"
+	"strings"
+
+	pack "github.com/OctopusDeploy/cli/pkg/cmd/package/support"
+	"github.com/OctopusDeploy/cli/pkg/constants"
+)
+
+// A .nupkg is not a plain zip. It is an Open Packaging Conventions container,
+// and feeds such as Cloudsmith, Artifactory and nuget.org reject one that is
+// missing the OPC parts below:
+//
+//	[Content_Types].xml                                       declares a content type per extension
+//	_rels/.rels                                               points at the manifest and core properties
+//	package/services/metadata/core-properties/<hash>.psmdcp   the core properties themselves
+//
+// The nuspec and the packaged files alone are not enough, even though most
+// tooling can still read them.
+const (
+	contentTypesPart  = "[Content_Types].xml"
+	relationshipsPart = "_rels/.rels"
+	corePropertiesDir = "package/services/metadata/core-properties"
+
+	contentTypesNamespace    = "http://schemas.openxmlformats.org/package/2006/content-types"
+	relationshipsNamespace   = "http://schemas.openxmlformats.org/package/2006/relationships"
+	corePropertiesNamespace  = "http://schemas.openxmlformats.org/package/2006/metadata/core-properties"
+	manifestRelationshipType = "http://schemas.microsoft.com/packaging/2010/07/manifest"
+	corePropertiesRelType    = "http://schemas.openxmlformats.org/package/2006/relationships/metadata/core-properties"
+
+	// what NuGet itself writes for parts it has no more specific type for
+	defaultContentType = "application/octet"
+)
+
+type contentTypes struct {
+	XMLName  xml.Name              `xml:"Types"`
+	Xmlns    string                `xml:"xmlns,attr"`
+	Defaults []contentTypeDefault  `xml:"Default"`
+	Override []contentTypeOverride `xml:"Override"`
+}
+
+type contentTypeDefault struct {
+	Extension   string `xml:"Extension,attr"`
+	ContentType string `xml:"ContentType,attr"`
+}
+
+type contentTypeOverride struct {
+	PartName    string `xml:"PartName,attr"`
+	ContentType string `xml:"ContentType,attr"`
+}
+
+type relationships struct {
+	XMLName       xml.Name       `xml:"Relationships"`
+	Xmlns         string         `xml:"xmlns,attr"`
+	Relationships []relationship `xml:"Relationship"`
+}
+
+type relationship struct {
+	Type   string `xml:"Type,attr"`
+	Target string `xml:"Target,attr"`
+	Id     string `xml:"Id,attr"`
+}
+
+type coreProperties struct {
+	XMLName        xml.Name `xml:"coreProperties"`
+	Xmlns          string   `xml:"xmlns,attr"`
+	XmlnsDc        string   `xml:"xmlns:dc,attr"`
+	XmlnsDcterms   string   `xml:"xmlns:dcterms,attr"`
+	XmlnsXsi       string   `xml:"xmlns:xsi,attr"`
+	Creator        string   `xml:"dc:creator,omitempty"`
+	Description    string   `xml:"dc:description,omitempty"`
+	Identifier     string   `xml:"dc:identifier"`
+	Version        string   `xml:"version"`
+	Keywords       string   `xml:"keywords,omitempty"`
+	LastModifiedBy string   `xml:"lastModifiedBy"`
+}
+
+// buildOpcParts produces the OPC parts for a package whose archive will contain
+// packagedPaths plus a nuspec named after the package id.
+func buildOpcParts(id string, version string, authors []string, description string, packagedPaths []string) ([]pack.ArchiveEntry, error) {
+	nuspecPart := id + ".nuspec"
+	corePropertiesPart := fmt.Sprintf("%s/%s.psmdcp", corePropertiesDir, deterministicHex(id+version, 16))
+
+	// every part in the finished archive needs a declared content type
+	allParts := append([]string{}, packagedPaths...)
+	allParts = append(allParts, nuspecPart, relationshipsPart, corePropertiesPart)
+
+	contentTypesXml, err := marshalPart(buildContentTypes(allParts))
+	if err != nil {
+		return nil, err
+	}
+
+	relationshipsXml, err := marshalPart(relationships{
+		Xmlns: relationshipsNamespace,
+		Relationships: []relationship{
+			{
+				Type:   manifestRelationshipType,
+				Target: "/" + nuspecPart,
+				Id:     "R" + deterministicHex("manifest"+id+version, 8),
+			},
+			{
+				Type:   corePropertiesRelType,
+				Target: "/" + corePropertiesPart,
+				Id:     "R" + deterministicHex("coreproperties"+id+version, 8),
+			},
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	creator := strings.Join(authors, ", ")
+	corePropertiesXml, err := marshalPart(coreProperties{
+		Xmlns:          corePropertiesNamespace,
+		XmlnsDc:        "http://purl.org/dc/elements/1.1/",
+		XmlnsDcterms:   "http://purl.org/dc/terms/",
+		XmlnsXsi:       "http://www.w3.org/2001/XMLSchema-instance",
+		Creator:        creator,
+		Description:    description,
+		Identifier:     id,
+		Version:        version,
+		LastModifiedBy: constants.ExecutableName,
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return []pack.ArchiveEntry{
+		{Name: contentTypesPart, Content: contentTypesXml},
+		{Name: relationshipsPart, Content: relationshipsXml},
+		{Name: corePropertiesPart, Content: corePropertiesXml},
+	}, nil
+}
+
+// buildContentTypes declares one Default per distinct extension. Parts without
+// an extension cannot be covered by a Default, so they get an Override each.
+func buildContentTypes(parts []string) contentTypes {
+	types := contentTypes{Xmlns: contentTypesNamespace}
+
+	seenExtensions := map[string]bool{}
+	var extensions []string
+	var overrides []contentTypeOverride
+
+	for _, part := range parts {
+		// a trailing slash marks a directory, which is not a part at all
+		if part == "" || part == "." || strings.HasSuffix(part, "/") {
+			continue
+		}
+
+		extension := strings.TrimPrefix(strings.ToLower(path.Ext(part)), ".")
+		if extension == "" {
+			overrides = append(overrides, contentTypeOverride{
+				PartName:    "/" + part,
+				ContentType: defaultContentType,
+			})
+			continue
+		}
+
+		if !seenExtensions[extension] {
+			seenExtensions[extension] = true
+			extensions = append(extensions, extension)
+		}
+	}
+
+	sort.Strings(extensions)
+	for _, extension := range extensions {
+		types.Defaults = append(types.Defaults, contentTypeDefault{
+			Extension:   extension,
+			ContentType: contentTypeFor(extension),
+		})
+	}
+
+	sort.Slice(overrides, func(i, j int) bool { return overrides[i].PartName < overrides[j].PartName })
+	types.Override = overrides
+
+	return types
+}
+
+func contentTypeFor(extension string) string {
+	switch extension {
+	case "rels":
+		return "application/vnd.openxmlformats-package.relationships+xml"
+	case "psmdcp":
+		return "application/vnd.openxmlformats-package.core-properties+xml"
+	default:
+		return defaultContentType
+	}
+}
+
+func marshalPart(part any) ([]byte, error) {
+	body, err := xml.Marshal(part)
+	if err != nil {
+		return nil, err
+	}
+
+	return append([]byte(xml.Header), body...), nil
+}
+
+// deterministicHex keeps the generated part and relationship names stable for a
+// given package, so repacking the same inputs produces the same archive.
+func deterministicHex(seed string, bytes int) string {
+	sum := sha256.Sum256([]byte(seed))
+	return hex.EncodeToString(sum[:bytes])
+}

pkg/cmd/package/nuget/create/opc_test.go

@@ -0,0 +1,126 @@
+package create
+
+import (
+	"encoding/xml"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func partsByName(t *testing.T, id, version string, authors []string, description string, packaged []string) map[string]string {
+	t.Helper()
+
+	entries, err := buildOpcParts(id, version, authors, description, packaged)
+	assert.NoError(t, err)
+
+	byName := map[string]string{}
+	for _, entry := range entries {
+		byName[entry.Name] = string(entry.Content)
+	}
+
+	return byName
+}
+
+// Without these three parts a .nupkg is a plain zip, and feeds such as
+// Cloudsmith and Artifactory refuse to index it.
+func TestBuildOpcParts_ProducesTheRequiredParts(t *testing.T) {
+	parts := partsByName(t, "Acme.Widget", "1.2.3", []string{"Acme"}, "A widget", []string{"lib/thing.dll"})
+
+	assert.Len(t, parts, 3)
+	assert.Contains(t, parts, "[Content_Types].xml")
+	assert.Contains(t, parts, "_rels/.rels")
+
+	var corePropertiesPart string
+	for name := range parts {
+		if strings.HasPrefix(name, "package/services/metadata/core-properties/") {
+			corePropertiesPart = name
+		}
+	}
+	assert.NotEmpty(t, corePropertiesPart, "expected a core properties part")
+	assert.True(t, strings.HasSuffix(corePropertiesPart, ".psmdcp"))
+}
+
+func TestBuildOpcParts_PartsAreWellFormedXml(t *testing.T) {
+	parts := partsByName(t, "Acme.Widget", "1.2.3", []string{"Acme"}, "A widget", []string{"lib/thing.dll"})
+
+	for name, content := range parts {
+		assert.True(t, strings.HasPrefix(content, xml.Header), "%s should start with an XML declaration", name)
+
+		var discard any
+		assert.NoError(t, xml.Unmarshal([]byte(content), &discard), "%s should be well formed", name)
+	}
+}
+
+func TestBuildOpcParts_RelationshipsPointAtTheManifestAndCoreProperties(t *testing.T) {
+	parts := partsByName(t, "Acme.Widget", "1.2.3", []string{"Acme"}, "A widget", []string{"lib/thing.dll"})
+	rels := parts["_rels/.rels"]
+
+	assert.Contains(t, rels, `Target="/Acme.Widget.nuspec"`)
+	assert.Contains(t, rels, manifestRelationshipType)
+	assert.Contains(t, rels, corePropertiesRelType)
+	assert.Contains(t, rels, `Target="/package/services/metadata/core-properties/`)
+}
+
+func TestBuildOpcParts_CorePropertiesCarryTheMetadata(t *testing.T) {
+	parts := partsByName(t, "Acme.Widget", "1.2.3", []string{"Alice", "Bob"}, "A widget", []string{"lib/thing.dll"})
+
+	var coreProperties string
+	for name, content := range parts {
+		if strings.HasSuffix(name, ".psmdcp") {
+			coreProperties = content
+		}
+	}
+
+	assert.Contains(t, coreProperties, "<dc:creator>Alice, Bob</dc:creator>")
+	assert.Contains(t, coreProperties, "<dc:description>A widget</dc:description>")
+	assert.Contains(t, coreProperties, "<dc:identifier>Acme.Widget</dc:identifier>")
+	assert.Contains(t, coreProperties, "<version>1.2.3</version>")
+}
+
+func TestBuildOpcParts_IsDeterministic(t *testing.T) {
+	first := partsByName(t, "Acme.Widget", "1.2.3", []string{"Acme"}, "A widget", []string{"lib/thing.dll"})
+	second := partsByName(t, "Acme.Widget", "1.2.3", []string{"Acme"}, "A widget", []string{"lib/thing.dll"})
+
+	assert.Equal(t, first, second, "packing the same inputs twice should produce identical parts")
+}
+
+func TestBuildContentTypes_DeclaresEachExtensionOnce(t *testing.T) {
+	types := buildContentTypes([]string{"lib/a.dll", "lib/b.dll", "readme.txt", "Acme.nuspec", "_rels/.rels"})
+
+	extensions := map[string]string{}
+	for _, entry := range types.Defaults {
+		assert.NotContains(t, extensions, entry.Extension, "extension %s declared twice", entry.Extension)
+		extensions[entry.Extension] = entry.ContentType
+	}
+
+	assert.Equal(t, "application/octet", extensions["dll"])
+	assert.Equal(t, "application/octet", extensions["txt"])
+	assert.Equal(t, "application/vnd.openxmlformats-package.relationships+xml", extensions["rels"])
+	assert.Empty(t, types.Override)
+}
+
+// A Default only covers parts that have an extension, so anything without one
+// needs an Override or the container is not fully described.
+func TestBuildContentTypes_OverridesPartsWithoutAnExtension(t *testing.T) {
+	types := buildContentTypes([]string{"tools/run", "lib/a.dll"})
+
+	assert.Len(t, types.Override, 1)
+	assert.Equal(t, "/tools/run", types.Override[0].PartName)
+	assert.Equal(t, "application/octet", types.Override[0].ContentType)
+}
+
+func TestBuildContentTypes_IgnoresDirectoryAndCurrentPathEntries(t *testing.T) {
+	types := buildContentTypes([]string{".", "lib/", "lib/a.dll"})
+
+	assert.Empty(t, types.Override)
+	assert.Len(t, types.Defaults, 1)
+	assert.Equal(t, "dll", types.Defaults[0].Extension)
+}
+
+func TestBuildContentTypes_ExtensionMatchingIsCaseInsensitive(t *testing.T) {
+	types := buildContentTypes([]string{"lib/a.DLL", "lib/b.dll"})
+
+	assert.Len(t, types.Defaults, 1)
+	assert.Equal(t, "dll", types.Defaults[0].Extension)
+}