From 233f9418f63abf86e5e6299551f80662740c05da Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Tue, 4 Aug 2026 18:51:19 +1000 Subject: [PATCH 1/2] fix: produce valid NuGet packages from package nuget create The command wrote the nuspec and the matched files into a zip and named it .nupkg. A .nupkg is an Open Packaging Conventions container, so the result was missing [Content_Types].xml, _rels/.rels and the core properties part. Opened with a real OPC reader the package resolves to zero parts and no manifest relationship, which is why feeds such as Cloudsmith and Artifactory reject or fail to list it. Generate the three OPC parts alongside the nuspec. Content types are derived from the extensions actually present, with an Override for any part that has no extension, and the generated part and relationship names are derived from the package id and version so repacking the same inputs is reproducible. Directory records are also omitted from .nupkg archives, since OPC part names may not end in "/". `package zip create` is untouched and still includes them. Fixes #477 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/package/nuget/create/create.go | 9 +- pkg/cmd/package/nuget/create/opc.go | 210 +++++++++++++++++++++++ pkg/cmd/package/nuget/create/opc_test.go | 126 ++++++++++++++ pkg/cmd/package/support/pack.go | 82 ++++++++- 4 files changed, 424 insertions(+), 3 deletions(-) create mode 100644 pkg/cmd/package/nuget/create/opc.go create mode 100644 pkg/cmd/package/nuget/create/opc_test.go diff --git a/pkg/cmd/package/nuget/create/create.go b/pkg/cmd/package/nuget/create/create.go index 158ae317..7ba01d35 100644 --- a/pkg/cmd/package/nuget/create/create.go +++ b/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.BuildPackageContents(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: diff --git a/pkg/cmd/package/nuget/create/opc.go b/pkg/cmd/package/nuget/create/opc.go new file mode 100644 index 00000000..5ff58ac7 --- /dev/null +++ b/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/.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]) +} diff --git a/pkg/cmd/package/nuget/create/opc_test.go b/pkg/cmd/package/nuget/create/opc_test.go new file mode 100644 index 00000000..5c8a87f7 --- /dev/null +++ b/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, "Alice, Bob") + assert.Contains(t, coreProperties, "A widget") + assert.Contains(t, coreProperties, "Acme.Widget") + assert.Contains(t, coreProperties, "1.2.3") +} + +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) +} diff --git a/pkg/cmd/package/support/pack.go b/pkg/cmd/package/support/pack.go index ce02ad78..8d4c1234 100644 --- a/pkg/cmd/package/support/pack.go +++ b/pkg/cmd/package/support/pack.go @@ -165,7 +165,32 @@ func BuildOutFileName(packageType, id, version string) string { return fmt.Sprintf("%s.%s.%s", id, version, packageType) } +// ArchiveEntry is written into the archive from memory rather than read from +// disk, for parts that are generated rather than packaged. +type ArchiveEntry struct { + Name string + Content []byte +} + +// PackageContents adjusts how an archive is assembled, for formats that need +// more than a plain zip of the matched files. +type PackageContents struct { + // ExtraEntries is given the relative paths already selected for the archive + // and returns parts to append. Called after the file list is resolved, so a + // format can describe its own contents. + ExtraEntries func(paths []string) ([]ArchiveEntry, error) + + // ExcludeDirectories omits explicit directory records. Open Packaging + // Conventions containers, such as .nupkg, must not contain part names + // ending in "/". + ExcludeDirectories bool +} + func BuildPackage(opts *PackageCreateOptions, outFileName string) (*os.File, error) { + return BuildPackageContents(opts, outFileName, nil) +} + +func BuildPackageContents(opts *PackageCreateOptions, outFileName string, contents *PackageContents) (*os.File, error) { outFilePath := filepath.Join(opts.OutFolder.Value, outFileName) outPath, err := filepath.Abs(opts.OutFolder.Value) if err != nil { @@ -188,10 +213,47 @@ func BuildPackage(opts *PackageCreateOptions, outFileName string) (*os.File, err return nil, errors.New("no files identified to package") } - return buildArchive(opts.Writer, outFilePath, opts.BasePath.Value, filePaths, opts.Verbose.Value) + var extraEntries []ArchiveEntry + if contents != nil { + // filter before the hook runs, so a format describing its own contents + // is given the paths that will actually be in the archive + if contents.ExcludeDirectories { + filePaths, err = withoutDirectories(opts.BasePath.Value, filePaths) + if err != nil { + return nil, err + } + } + + if contents.ExtraEntries != nil { + extraEntries, err = contents.ExtraEntries(filePaths) + if err != nil { + return nil, err + } + } + } + + return buildArchive(opts.Writer, outFilePath, opts.BasePath.Value, filePaths, opts.Verbose.Value, extraEntries) } -func buildArchive(out io.Writer, outFilePath string, basePath string, filesToArchive []string, isVerbose bool) (*os.File, error) { +// withoutDirectories drops directory entries from a resolved file list. OPC +// containers such as .nupkg must not contain part names ending in "/". +func withoutDirectories(basePath string, paths []string) ([]string, error) { + var files []string + for _, path := range paths { + fileInfo, err := os.Stat(filepath.Join(basePath, path)) + if err != nil { + return nil, err + } + + if !fileInfo.IsDir() { + files = append(files, path) + } + } + + return files, nil +} + +func buildArchive(out io.Writer, outFilePath string, basePath string, filesToArchive []string, isVerbose bool, extraEntries []ArchiveEntry) (*os.File, error) { _, outFile := filepath.Split(outFilePath) zipFile, err := os.Create(outFilePath) if err != nil { @@ -251,6 +313,22 @@ func buildArchive(out io.Writer, outFilePath string, basePath string, filesToArc } } + for _, entry := range extraEntries { + header := &zip.FileHeader{Name: entry.Name, Method: zip.Deflate} + header.SetMode(0644) + + headerWriter, err := writer.CreateHeader(header) + if err != nil { + return nil, err + } + + if _, err = headerWriter.Write(entry.Content); err != nil { + return nil, err + } + + VerboseOut(out, isVerbose, "Added file: %s\n", entry.Name) + } + return zipFile, nil } From 21f1011eca1ffcd3356fab3297de4e7ee2dfd48f Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 5 Aug 2026 14:24:49 +1000 Subject: [PATCH 2/2] refactor: rename BuildPackageContents to BuildPackageWithContents Matches how the repo names a variant that takes extra behaviour: DeleteWithConfirmation, SelectMapWithNew, MapCollectionWithLookups. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/cmd/package/nuget/create/create.go | 2 +- pkg/cmd/package/support/pack.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/package/nuget/create/create.go b/pkg/cmd/package/nuget/create/create.go index 7ba01d35..84d6af69 100644 --- a/pkg/cmd/package/nuget/create/create.go +++ b/pkg/cmd/package/nuget/create/create.go @@ -152,7 +152,7 @@ func createRun(cmd *cobra.Command, opts *NuPkgCreateOptions) error { // 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.BuildPackageContents(opts.PackageCreateOptions, outFilePath, &pack.PackageContents{ + 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) diff --git a/pkg/cmd/package/support/pack.go b/pkg/cmd/package/support/pack.go index 8d4c1234..aee7c175 100644 --- a/pkg/cmd/package/support/pack.go +++ b/pkg/cmd/package/support/pack.go @@ -187,10 +187,10 @@ type PackageContents struct { } func BuildPackage(opts *PackageCreateOptions, outFileName string) (*os.File, error) { - return BuildPackageContents(opts, outFileName, nil) + return BuildPackageWithContents(opts, outFileName, nil) } -func BuildPackageContents(opts *PackageCreateOptions, outFileName string, contents *PackageContents) (*os.File, error) { +func BuildPackageWithContents(opts *PackageCreateOptions, outFileName string, contents *PackageContents) (*os.File, error) { outFilePath := filepath.Join(opts.OutFolder.Value, outFileName) outPath, err := filepath.Abs(opts.OutFolder.Value) if err != nil {