goharbor / goharbor/harbor-cli

[bug]: Silent (nil, nil) return from GetRegistryResponse causes panic in replication policies create

Open Beginner friendly
#1,065 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
163
Forks
211
Avg merge
1m
Merged PRs (30d)
1

Description

## Description
`api.GetRegistryResponse` returns `(nil, nil)` when the upstream `GetRegistry` call succeeds with a zero-value payload (`response.Payload.ID == 0`). This violates Go's `(T, error)` contract — the error is nil, but the returned pointer is also nil.

The caller in `cmd/harbor/root/replication/policies/create.go` checks `if err != nil` (which passes), then passes the nil `registry` straight into `ConvertToPolicy()`, which panics with a nil-pointer dereference. The sibling command `registry/update.go` already has the defensive nil-guard against this, but `replication policies create` does not.

## Steps to Reproduce
1. Run `harbor replication policies create --registry-id `
2. The CLI panics with a stack trace instead of printing a clean error

Also reproducible deterministically by mocking `GetRegistry` to return a `200 OK` with `Payload.ID == 0` — the handler returns `nil, nil` on that branch every time.

## Expected Behavior
The command should print a clear error like `"registry with ID N not found"` and exit with a non-zero code. The user should never see a panic.

## Actual Behavior
The CLI crashes with:
```text
panic: runtime error: invalid memory address or nil pointer dereference
signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=...
```
The panic originates inside `ConvertToPolicy` because `registry` is nil — but the root cause is upstream: `GetRegistryResponse` returns `(nil, nil)` in `registry_handler.go`.

## Environment
- OS: All
- Tool version: `main` branch
- Other relevant details: Go 1.26.3, `goharbor/go-client v0.213.1`

## Additional Context
**Root cause** — In `pkg/api/registry_handler.go`, `GetRegistryResponse` checks:
```go
if response.Payload.ID == 0 {
return nil, err // err is nil here → returns (nil, nil)
}
```

Missing nil-guard — In `cmd/harbor/root/replication/policies/create.go`:
```go
registry, err := api.GetRegistryResponse(registryID)
if err != nil {
return fmt.Errorf("failed to get registry with ID %d: %v", registryID, err)
}
policy := ConvertToPolicy(opts, registry) // registry can be nil here
```

Sibling fix exists — `registry/update.go` already guards against this:
```go
if existingRegistry == nil {
return fmt.Errorf("registry is not found")
}
```

The fix would be: (1) return a wrapped error from `GetRegistryResponse` instead of `(nil, nil)`, and (2) add the same nil-guard in `replication/policies/create.go`.

I am happy to open a PR for this if the approach sounds right. I'm also completely open to any suggestions or adjustments you might have!

Contributor guide

Open the contributing guide

Research direction

Start with pkg/api/registry_handler.go and cmd/harbor/root/replication/policies/create.go, then compare the nil handling in registry/update.go. Reproduce the command with a deleted or stale registry ID, and consider the issue complete when it returns a clear not-found error and non-zero exit status without panicking.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cli
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.