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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions registry/app/api/controller/pkg/nuget/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ type Controller interface {
CountPackageV2(ctx context.Context, info nugettype.ArtifactInfo, searchTerm string) *EntityCountResponse

GetServiceMetadataV2(ctx context.Context, info nugettype.ArtifactInfo) *GetServiceMetadataV2Response

GetReadme(ctx context.Context, info nugettype.ArtifactInfo) *GetReadmeResponse
}

// Controller handles Python package operations.
Expand Down
90 changes: 90 additions & 0 deletions registry/app/api/controller/pkg/nuget/get_readme.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package nuget

import (
"context"
"fmt"

"github.com/harness/gitness/registry/app/pkg"
"github.com/harness/gitness/registry/app/pkg/base"
"github.com/harness/gitness/registry/app/pkg/commons"
"github.com/harness/gitness/registry/app/pkg/nuget"
"github.com/harness/gitness/registry/app/pkg/response"
nugettype "github.com/harness/gitness/registry/app/pkg/types/nuget"
registrytypes "github.com/harness/gitness/registry/types"
)

func (c *controller) GetReadme(
ctx context.Context,
info nugettype.ArtifactInfo,
) *GetReadmeResponse {
f := func(registry registrytypes.Registry, a pkg.Artifact) response.Response {
info.RegIdentifier = registry.Name
info.RegistryID = registry.ID
nugetRegistry, ok := a.(nuget.Registry)
if !ok {
return &GetReadmeResponse{
BaseResponse{
fmt.Errorf("invalid registry type: expected nuget.Registry"),
nil,
}, "",
}
}
readmeContent, err := nugetRegistry.GetReadme(ctx, info)
if err != nil {
return &GetReadmeResponse{
BaseResponse{
err,
nil,
}, "",
}
}

headers := &commons.ResponseHeaders{
Headers: map[string]string{
"Content-Type": "text/markdown; charset=utf-8",
},
}
Comment on lines +56 to +60

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how other nuget controller methods set ResponseHeaders to see if Code is typically set
rg -n "ResponseHeaders{" --type=go -C3 -g '**/controller/pkg/nuget/**'

Repository: abhinavcode/harness

Length of output: 158


🏁 Script executed:

# Also check the ResponseHeaders struct definition
cat -n registry/app/pkg/commons/request.go | head -40

Repository: abhinavcode/harness

Length of output: 1338


🏁 Script executed:

# Look at the specific file mentioned in the review
cat -n registry/app/api/controller/pkg/nuget/get_readme.go | sed -n '50,70p'

Repository: abhinavcode/harness

Length of output: 545


🏁 Script executed:

# Search for ResponseHeaders usage with escaped pattern
rg -n "ResponseHeaders" --type=go -C3 'registry/app/api/controller/pkg/nuget/'

Repository: abhinavcode/harness

Length of output: 4402


🏁 Script executed:

# Check what files exist in the nuget controller directory
fd -t f --type f 'registry/app/api/controller/pkg/nuget/' | head -20

Repository: abhinavcode/harness

Length of output: 520


🏁 Script executed:

# Search for how ResponseHeaders is typically initialized across the codebase
rg -n "Code\s*:" --type=go -B2 -A2 'registry/app/api/controller/pkg/'

Repository: abhinavcode/harness

Length of output: 1912


🏁 Script executed:

# Find where ResponseHeaders.Code is used in handler/middleware code
rg -n "ResponseHeaders|\.Code" --type=go -C2 'registry/app/api/handler\|registry/app/api/middleware' | head -60

Repository: abhinavcode/harness

Length of output: 156


🏁 Script executed:

# Search for where BaseResponse is processed
rg -n "BaseResponse" --type=go -C3 'registry/app/api/handler' | head -80

Repository: abhinavcode/harness

Length of output: 6438


🏁 Script executed:

# Look for HTTP status code handling
rg -n "http.StatusOK\|w.WriteHeader\|statusCode\|StatusCode" --type=go 'registry/app/api' -C2 | grep -A2 -B2 "ResponseHeaders\|Code"

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Find nuget handler files
fd -type f --full-path '*/handler/nuget' --type f

Repository: abhinavcode/harness

Length of output: 233


🏁 Script executed:

# Search for where responses are converted to HTTP responses
rg -n "WriteHeader\|StatusCode\|Code.*http" --type=go 'registry/app/api/handler' -C3 | head -100

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Look at the handler structure and how it processes controller responses
fd -type f -name "*.go" 'registry/app/api/handler' | xargs -I {} basename {} | sort | uniq

Repository: abhinavcode/harness

Length of output: 233


🏁 Script executed:

# Find nuget handler files
find registry/app/api/handler -type f -name "*.go" 2>/dev/null | grep nuget

Repository: abhinavcode/harness

Length of output: 1008


🏁 Script executed:

# Look for all handler directories
find registry/app/api/handler -type d 2>/dev/null

Repository: abhinavcode/harness

Length of output: 502


🏁 Script executed:

# Search for how controller responses are handled - look for function that handles BaseResponse
rg -n "GetError\|ResponseHeaders" --type=go 'registry/app/api' -C2 | head -150

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Look at the nuget handler to see how it processes responses
cat -n registry/app/api/handler/nuget/handler.go

Repository: abhinavcode/harness

Length of output: 4976


🏁 Script executed:

# Look at one of the handler methods to understand the pattern
cat -n registry/app/api/handler/nuget/download_package.go | head -100

Repository: abhinavcode/harness

Length of output: 2596


🏁 Script executed:

# Search for where the Code field from ResponseHeaders is used
rg -n "\.Code" --type=go 'registry/app/api/handler/nuget' -C3

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Find the WriteHeadersToResponse and WriteToResponse method definitions
rg -n "WriteHeadersToResponse\|WriteToResponse" --type=go -B2 -A10 'registry/app'

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Also check the commons package for these methods
cat -n registry/app/pkg/commons/request.go

Repository: abhinavcode/harness

Length of output: 3184


🏁 Script executed:

# Double-check if other nuget controller methods also omit Code without setting ResponseHeaders to nil
rg -n "ResponseHeaders\{" --type=go 'registry/app/api/controller/pkg/nuget/' -C3

Repository: abhinavcode/harness

Length of output: 585


Set Code: 200 explicitly for clarity and consistency with other handler patterns.

The handler layer does write HTTP 200 OK (via writer.WriteHeader(http.StatusOK) on line 127 of registry/app/api/handler/nuget/handler.go), so the current code functions correctly. However, the Code field is left at zero, which creates confusion given that ResponseHeaders.WriteToResponse() only writes the status if Code != 0. Other handlers like DownloadPackage use the WriteToResponse() pattern, which would fail silently if Code remained zero. For consistency and clarity, explicitly set Code: 200 in the controller response.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@registry/app/api/controller/pkg/nuget/get_readme.go` around lines 56 - 60,
The ResponseHeaders instance in GetReadme (the headers variable of type
commons.ResponseHeaders) leaves Code at zero which can cause WriteToResponse()
to skip writing the status; set Code to http.StatusOK (200) when constructing
headers so the status is explicit and consistent with other handlers (e.g.,
DownloadPackage) and with ResponseHeaders.WriteToResponse().


return &GetReadmeResponse{
BaseResponse{
nil,
headers,
}, readmeContent,
}
}

result, err := base.NoProxyWrapper(ctx, c.registryDao, f, info)

if err != nil {
return &GetReadmeResponse{
BaseResponse{
err,
nil,
}, "",
}
}
readmeResponse, ok := result.(*GetReadmeResponse)
if !ok {
return &GetReadmeResponse{
BaseResponse{
fmt.Errorf("invalid response type: expected GetReadmeResponse"),
nil,
}, "",
}
}
return readmeResponse
}
5 changes: 5 additions & 0 deletions registry/app/api/controller/pkg/nuget/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,8 @@ type GetPackageVersionMetadataResponse struct {
BaseResponse
RegistrationLeafResponse *nuget.RegistrationLeafResponse
}

type GetReadmeResponse struct {
BaseResponse
ReadmeContent string
}
35 changes: 35 additions & 0 deletions registry/app/api/handler/nuget/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package nuget

import (
"fmt"
"net/http"
"strings"

Expand All @@ -33,6 +34,7 @@ type Handler interface {
UploadSymbolPackage(writer http.ResponseWriter, request *http.Request)
DownloadPackage(http.ResponseWriter, *http.Request)
DeletePackage(writer http.ResponseWriter, request *http.Request)
GetReadme(writer http.ResponseWriter, request *http.Request)
GetServiceEndpoint(http.ResponseWriter, *http.Request)
GetServiceEndpointV2(http.ResponseWriter, *http.Request)
ListPackageVersion(http.ResponseWriter, *http.Request)
Expand Down Expand Up @@ -92,3 +94,36 @@ func (h *handler) GetPackageArtifactInfo(r *http.Request) (pkg.PackageArtifactIn
NestedPath: strings.TrimSuffix(r.PathValue("*"), "/"),
}, nil
}

func (h *handler) GetReadme(writer http.ResponseWriter, request *http.Request) {
ctx := request.Context()

// Get artifact info from request
info, err := h.GetPackageArtifactInfo(request)
if err != nil {
h.HandleErrors(ctx, []error{err}, writer)
return
}

nugetInfo, ok := info.(*nugettype.ArtifactInfo)
if !ok {
h.HandleErrors(ctx, []error{fmt.Errorf("failed to fetch info from context")}, writer)
return
}

// Call controller to get readme
response := h.controller.GetReadme(ctx, *nugetInfo)
if response.Error != nil {
h.HandleError(ctx, writer, response.Error)
return
}

// Write response headers
if response.ResponseHeaders != nil {
response.ResponseHeaders.WriteHeadersToResponse(writer)
}

// Write readme content
writer.WriteHeader(http.StatusOK)
_, _ = writer.Write([]byte(response.ReadmeContent))
}
Comment on lines +98 to +129

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find ResponseHeaders type definition and WriteHeadersToResponse implementation
rg -n "type ResponseHeaders|WriteHeadersToResponse" -C 5 --type=go

Repository: abhinavcode/harness

Length of output: 8039


🏁 Script executed:

# Search for other NuGet handlers to see standard error handling patterns
rg -n "func \(h \*handler\) Get" -C 3 --type=go | head -100

Repository: abhinavcode/harness

Length of output: 7184


🏁 Script executed:

# Look for standard error mapping patterns in NuGet handlers
rg -n "TranslatedUserError|StatusCodeMapper|GetPackageMetadata|GetServiceEndpoint" -C 3 --type=go | head -150

Repository: abhinavcode/harness

Length of output: 10135


🏁 Script executed:

# Find the response type/structure for GetReadme
rg -n "GetReadme" -C 5 --type=go | grep -A 20 "func.*GetReadme"

Repository: abhinavcode/harness

Length of output: 6266


🏁 Script executed:

# Find GetReadmeResponse type definition
rg -n "type GetReadmeResponse|ReadmeContent|ResponseHeaders" registry/app/api/controller/pkg/nuget/ -C 3 --type=go

Repository: abhinavcode/harness

Length of output: 4914


🏁 Script executed:

# Look at GetPackageMetadata handler implementation to see standard pattern
cat -n registry/app/api/handler/nuget/package_metadata.go

Repository: abhinavcode/harness

Length of output: 1860


🏁 Script executed:

# Check GetServiceEndpoint handler implementation
cat -n registry/app/api/handler/nuget/service_endpoint.go

Repository: abhinavcode/harness

Length of output: 1847


🏁 Script executed:

# Find how other handlers map controller responses to HTTP responses
rg -n "controller\." registry/app/api/handler/nuget/ -A 5 --type=go | head -200

Repository: abhinavcode/harness

Length of output: 9256


🏁 Script executed:

# Check WriteToResponse vs WriteHeadersToResponse difference
rg -n "func.*WriteToResponse|func.*WriteHeadersToResponse" registry/app/pkg/commons/request.go -A 10

Repository: abhinavcode/harness

Length of output: 551


🏁 Script executed:

# Find HandleError implementation
rg -n "func.*HandleError" registry/app/api/handler/nuget/ -A 8 --type=go

Repository: abhinavcode/harness

Length of output: 45


🏁 Script executed:

# Look at the complete get_readme.go controller to see full flow
cat -n registry/app/api/controller/pkg/nuget/get_readme.go

Repository: abhinavcode/harness

Length of output: 2962


🏁 Script executed:

# Search for HandleError in the handler base class
rg -n "HandleError" registry/app/api/handler/ -C 3 --type=go | head -100

Repository: abhinavcode/harness

Length of output: 6677


🏁 Script executed:

# Check the handler base implementation
find registry/app/api/handler -name "handler.go" -o -name "base.go" | xargs grep -l "HandleError" 2>/dev/null

Repository: abhinavcode/harness

Length of output: 195


🏁 Script executed:

# Look at the handler struct definition in nuget
rg -n "type handler struct" registry/app/api/handler/nuget/ -A 10 --type=go

Repository: abhinavcode/harness

Length of output: 770


🏁 Script executed:

# Find HandleError in packages.Handler
cat -n registry/app/api/handler/packages/handler.go | grep -A 15 "func.*HandleError"

Repository: abhinavcode/harness

Length of output: 1901


Use standard error handling pattern consistent with other NuGet handlers.

GetReadme returns a hardcoded 404 for all controller errors (line 117), masking authentication/validation/internal failures with the same status code. It also ignores any status in response.ResponseHeaders (the controller never sets Code, so WriteHeadersToResponse won't write one), then unconditionally writes 200 OK (line 127). This bypasses proper error mapping and exposes raw internal error text to clients.

Follow the pattern used by GetPackageMetadata, GetServiceEndpoint, and other handlers: call h.HandleError(ctx, w, response.GetError()) instead of http.Error, which routes errors through TranslatedUserError for correct status mapping. Then call response.ResponseHeaders.WriteToResponse(w) to write headers and status together before writing body.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@registry/app/api/handler/nuget/handler.go` around lines 98 - 129, GetReadme
improperly maps all controller errors to a hardcoded 404 and writes raw error
text and a forced 200 response; change it to use the same error/response flow as
other NuGet handlers: after calling h.controller.GetReadme(ctx, *nugetInfo)
invoke h.HandleError(ctx, writer, response.GetError()) when
response.Error/response.GetError() is non-nil (instead of http.Error), and when
successful call response.ResponseHeaders.WriteToResponse(writer) (so headers and
status are written together) before writing response.ReadmeContent; refer to the
GetReadme function, h.GetPackageArtifactInfo, nugettype.ArtifactInfo,
h.controller.GetReadme, response.GetError, h.HandleError and
response.ResponseHeaders.WriteToResponse to locate and update the code.

3 changes: 3 additions & 0 deletions registry/app/api/router/packages/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ func NewRouter(
r.With(middleware.StoreArtifactInfo(nugetHandler)).
With(middleware.RequestNugetPackageAccess(packageHandler, enum.PermissionArtifactsDownload)).
Get("/Packages(Id='{id:[^']+}',Version='{version:[^']+}')", nugetHandler.GetPackageVersionMetadataV2)
r.With(middleware.StoreArtifactInfo(nugetHandler)).
With(middleware.RequestNugetPackageAccess(packageHandler, enum.PermissionArtifactsDownload)).
Get("/{id}/{version}/readme", nugetHandler.GetReadme)
r.With(middleware.StoreArtifactInfo(nugetHandler)).
With(middleware.RequestNugetPackageAccess(packageHandler, enum.PermissionArtifactsDownload)).
Get("/registration/{id}/{version}", nugetHandler.GetPackageVersionMetadata)
Expand Down
10 changes: 10 additions & 0 deletions registry/app/pkg/nuget/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ func buildServiceEndpoint(baseURL string) *nuget.ServiceEndpoint {
ID: baseURL + "/package",
Type: "PackageBaseAddress/3.0.0",
},
{
ID: baseURL + "/{lower_id}/{lower_version}/readme",
Type: "ReadmeUriTemplate/6.13.0",
},
{
ID: baseURL,
Type: "PackagePublish/2.0.0",
Expand Down Expand Up @@ -128,6 +132,11 @@ func getPackageDownloadURL(baseURL, id, version string) string {
return fmt.Sprintf("%s/package/%s/%s/%s.%s.nupkg", baseURL, id, version, id, version)
}

// getReadmeURL builds the readme url.
func getReadmeURL(baseURL, id, version string) string {
return fmt.Sprintf("%s/%s/%s/readme", baseURL, id, version)
}

// GetPackageMetadataURL builds the package metadata url.
func getPackageMetadataURL(baseURL, id, version string) string {
return fmt.Sprintf("%s/Packages(Id='%s',Version='%s')", baseURL, id, version)
Expand Down Expand Up @@ -672,6 +681,7 @@ func createRegistrationIndexPageItem(baseURL string, info nuget.ArtifactInfo, ar
Tags: metadata.PackageMetadata.Tags,
Title: metadata.PackageMetadata.Title,
Published: artifact.CreatedAt.Format(time.RFC3339),
ReadmeURL: getReadmeURL(baseURL, info.Image, artifact.Version),
},
}
return res, nil
Expand Down
30 changes: 29 additions & 1 deletion registry/app/pkg/nuget/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package nuget

import (
"context"
"encoding/json"
"encoding/xml"
"errors"
"fmt"
Expand Down Expand Up @@ -331,7 +332,7 @@ func (c *localRegistry) buildMetadata(fileReader io.Reader) (metadata nugetmetad
if err != nil {
return metadata, fmt.Errorf("failed to parse metadata from .nuspec file: %w", err2)
}
} else if strings.HasSuffix(header.Name, "README.md") {
} else if strings.HasSuffix(strings.ToLower(header.Name), "readme.md") {
readme, err2 = c.parseReadme(zr)
if err2 != nil {
return metadata, fmt.Errorf("failed to parse metadata from README.md file: %w", err2)
Expand Down Expand Up @@ -447,3 +448,30 @@ func (c *localRegistry) GetArtifactType() apicontract.RegistryType {
func (c *localRegistry) GetPackageTypes() []apicontract.PackageType {
return []apicontract.PackageType{apicontract.PackageTypeNUGET}
}

func (c *localRegistry) GetReadme(
ctx context.Context,
info nugettype.ArtifactInfo,
) (string, error) {
image, err := c.imageDao.GetByName(ctx, info.RegistryID, info.Image)
if err != nil {
return "", fmt.Errorf("failed to get image: %w", err)
}

artifact, err := c.artifactDao.GetByName(ctx, image.ID, info.Version)
if err != nil {
return "", fmt.Errorf("failed to get artifact: %w", err)
}

var metadata nugetmetadata.NugetMetadata
if err := json.Unmarshal(artifact.Metadata, &metadata); err != nil {
return "", fmt.Errorf("failed to unmarshal metadata: %w", err)
}

readmeContent := metadata.Metadata.PackageMetadata.Readme
if readmeContent == "" {
return "", fmt.Errorf("readme not found for package %s version %s", info.Image, info.Version)
}

return readmeContent, nil
}
8 changes: 8 additions & 0 deletions registry/app/pkg/nuget/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,14 @@ func (r *proxy) GetPackageTypes() []artifact.PackageType {
return []artifact.PackageType{artifact.PackageTypeNUGET}
}

func (r *proxy) GetReadme(
ctx context.Context,
_ nugettype.ArtifactInfo,
) (string, error) {
log.Error().Ctx(ctx).Msg("Not implemented")
return "", errcode.ErrCodeInvalidRequest.WithDetail(fmt.Errorf("not implemented"))
}
Comment on lines +520 to +526

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Readme retrieval always fails for proxy registries.

The new endpoint is now routable for NuGet, but the proxy implementation hard-fails with “not implemented,” which means readme fetches for upstream/proxy registries will consistently error. If proxy registries are meant to support readmes, this needs a real upstream fetch; if not, consider explicitly signaling “not supported” and hiding readme URLs for proxy metadata to avoid broken links.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@registry/app/pkg/nuget/proxy.go` around lines 520 - 526, The proxy GetReadme
method currently always returns a "not implemented" error; replace this stub in
func (r *proxy) GetReadme so proxy registries either fetch the readme from the
upstream registry or explicitly signal "not supported": implement an upstream
fetch by delegating to the proxy's upstream client/transport (e.g., call the
existing upstream fetch method on r such as r.fetchUpstream /
r.upstreamClient.GetReadme with ctx and the provided ArtifactInfo, return the
fetched string on success, translate an upstream 404 into a nil/empty/no-readme
response, and map other upstream errors into appropriate errcodes), or if proxy
readmes are unsupported return a clear "not supported" error (not
errcode.ErrCodeInvalidRequest) such as errcode.ErrCodeNotSupported and ensure
metadata generation hides readme URLs for proxy artifacts; update error handling
to stop using the current
errcode.ErrCodeInvalidRequest.WithDetail(fmt.Errorf("not implemented")).


func (r *proxy) putFileToLocal(
ctx context.Context, info *nugettype.ArtifactInfo,
remote RemoteRegistryHelper,
Expand Down
2 changes: 2 additions & 0 deletions registry/app/pkg/nuget/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,6 @@ type Registry interface {
GetServiceEndpointV2(ctx context.Context, info nuget.ArtifactInfo) *nuget.ServiceEndpointV2

GetServiceMetadataV2(ctx context.Context, info nuget.ArtifactInfo) *nuget.ServiceMetadataV2

GetReadme(ctx context.Context, info nuget.ArtifactInfo) (string, error)
}
2 changes: 2 additions & 0 deletions registry/app/pkg/types/nuget/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,8 @@ type CatalogEntry struct {
Tags string `json:"tags,omitempty"`
Title string `json:"title,omitempty"`
Published string `json:"published,omitempty"`
//nolint: tagliatelle
ReadmeURL string `json:"readmeUrl,omitempty"`
}

// https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#package-dependency-group
Expand Down