Fix nil pointer dereference in pullImage for public registries

[ddjain]

Fixes #132

Problem

krknctl run panics when using --private-registry with a public repository (no auth credentials provided).
ToDockerV2AuthString() returns (nil, nil) when no credentials are set, and pullImage dereferences the nil pointer unconditionally:

pullOptions.RegistryAuth = *registryAuth // panic: registryAuth is nil

Fix

Added a nil check before dereferencing registryAuth:

if registryAuth != nil {
    pullOptions.RegistryAuth = *registryAuth
}

Testing

Verified locally -- krknctl run with --private-registry and no auth flags now pulls the image and executes the scenario successfully.

[qodo-code-review]

Review Summary by Qodo

Fix nil pointer dereference in pullImage for public registries

🐞 Bug fix

Walkthroughs

Description

• Fix nil pointer dereference when pulling images from public registries
• Add nil check before dereferencing registryAuth pointer
• Prevents panic when using --private-registry flag without auth credentials

Diagram

flowchart LR
  A["pullImage called<br/>with registry"] --> B["ToDockerV2AuthString<br/>returns nil, nil"]
  B --> C["Check if registryAuth<br/>is not nil"]
  C --> D["Safely dereference<br/>or skip assignment"]
  D --> E["ImagePull succeeds<br/>without panic"]

Loading

File Changes

1. pkg/scenarioorchestrator/docker/scenario_orchestrator.go 🐞 Bug fix +3/-1

Add nil check for registryAuth pointer dereference

• Added nil check before dereferencing registryAuth pointer in pullImage function
• Prevents panic when ToDockerV2AuthString() returns nil for public registries
• Allows krknctl run to work with --private-registry flag and public repositories

pkg/scenarioorchestrator/docker/scenario_orchestrator.go

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)

🐞\ ≡ Correctness (1) ☼ Reliability (1)

1. Nil password panic path 🐞 ≡

Description

RegistryV2.ToDockerV2AuthString() can still panic by dereferencing r.Password when r.Username is set
but r.Password is nil, so krknctl can still crash in private-registry mode despite the new
registryAuth nil-check. This can happen because CLI parsing allows setting
--private-registry-username without requiring --private-registry-password.

Code

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[R246-248]

+		if registryAuth != nil {
+			pullOptions.RegistryAuth = *registryAuth
+		}

Evidence

pullImage calls registry.ToDockerV2AuthString() and then conditionally dereferences the returned
pointer; the PR prevents dereferencing a nil return value but does not prevent a panic inside
ToDockerV2AuthString(). CLI parsing can populate Username without Password, and ToDockerV2AuthString
dereferences *r.Password whenever Username != nil.

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[240-249]
cmd/utils.go[130-158]
pkg/provider/models/models.go[104-115]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RegistryV2.ToDockerV2AuthString()` dereferences `*r.Password` when `r.Username != nil`, which can panic if the password was not provided (nil pointer). This leaves a remaining crash path in private registry flows.

### Issue Context
`parsePrivateRepoArgs` can set `registrySettings.Username` without setting `registrySettings.Password` (e.g., user supplies `--private-registry-username` but omits `--private-registry-password`). `pullImage` calls `ToDockerV2AuthString()` when `registry != nil`.

### Fix Focus Areas
- pkg/provider/models/models.go[104-115]
- cmd/utils.go[144-158]

### Implementation notes
- In `ToDockerV2AuthString`, avoid dereferencing `r.Password` without a nil check. Either:
 - Treat missing password as empty string (`""`) and continue, or
 - Return a clear error requiring both username and password.
- Optionally enforce `--private-registry-username` and `--private-registry-password` to be provided together (or validate early and return an error message).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Empty token overrides auth 🐞 ☼

Description

NewRegistryV2FromEnv sets Token to a non-nil pointer even when the token env var is unset/empty,
which forces token-auth precedence and can break private-registry flows. In Podman runtime this
unconditionally triggers the 'token authentication not yet supported' error whenever private
registry is configured via env vars.

Code

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[R246-248]

+		if registryAuth != nil {
+			pullOptions.RegistryAuth = *registryAuth
+		}

Evidence

Env-based registry config always sets registryV2.Token = &token even if token == "".
ToDockerV2AuthString uses the token branch whenever r.Token != nil, and the Podman orchestrator
immediately errors if registry.Token != nil (without checking emptiness).

pkg/provider/models/models.go[42-48]
pkg/provider/models/models.go[104-112]
pkg/scenarioorchestrator/podman/scenario_orchestrator.go[56-71]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`NewRegistryV2FromEnv` assigns pointers for `Username`/`Password`/`Token` unconditionally. This makes `Token` non-nil even when the env var is unset (empty string), which (a) forces token auth precedence in `ToDockerV2AuthString`, and (b) causes Podman pulls to fail because Podman errors when `registry.Token != nil`.

### Issue Context
This affects private-registry mode configured via environment variables and can break both Docker (wrong auth selection) and Podman (hard error) execution paths.

### Fix Focus Areas
- pkg/provider/models/models.go[42-48]
- pkg/provider/models/models.go[104-112]
- pkg/scenarioorchestrator/podman/scenario_orchestrator.go[66-71]

### Implementation notes
- In `NewRegistryV2FromEnv`, only set `registryV2.Token` when the env var is non-empty; otherwise keep it `nil`.
- Consider similarly only setting `Username`/`Password` pointers when non-empty to align pointer-nil semantics across env vs CLI.
- In Podman orchestrator, consider treating empty token as not set (`registry.Token != nil && *registry.Token != ""`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. Nil password panic path 🐞 Bug ≡ Correctness

RegistryV2.ToDockerV2AuthString() can still panic by dereferencing r.Password when r.Username is set
but r.Password is nil, so krknctl can still crash in private-registry mode despite the new
registryAuth nil-check. This can happen because CLI parsing allows setting
--private-registry-username without requiring --private-registry-password.

Agent Prompt

### Issue description
`RegistryV2.ToDockerV2AuthString()` dereferences `*r.Password` when `r.Username != nil`, which can panic if the password was not provided (nil pointer). This leaves a remaining crash path in private registry flows.

### Issue Context
`parsePrivateRepoArgs` can set `registrySettings.Username` without setting `registrySettings.Password` (e.g., user supplies `--private-registry-username` but omits `--private-registry-password`). `pullImage` calls `ToDockerV2AuthString()` when `registry != nil`.

### Fix Focus Areas
- pkg/provider/models/models.go[104-115]
- cmd/utils.go[144-158]

### Implementation notes
- In `ToDockerV2AuthString`, avoid dereferencing `r.Password` without a nil check. Either:
  - Treat missing password as empty string (`""`) and continue, or
  - Return a clear error requiring both username and password.
- Optionally enforce `--private-registry-username` and `--private-registry-password` to be provided together (or validate early and return an error message).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools