goharbor / goharbor/harbor-cli
[bug]: Silent (nil, nil) return from GetRegistryResponse causes panic in replication policies create
- 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
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