Skip to content

fix: add nil check for Password in ToDockerV2AuthString#98

Open
AR21SM wants to merge 1 commit intokrkn-chaos:mainfrom
AR21SM:fix/nil-pointer-auth-string
Open

fix: add nil check for Password in ToDockerV2AuthString#98
AR21SM wants to merge 1 commit intokrkn-chaos:mainfrom
AR21SM:fix/nil-pointer-auth-string

Conversation

@AR21SM
Copy link
Contributor

@AR21SM AR21SM commented Jan 27, 2026

User description

Description

Fixes #97

In ToDockerV2AuthString(), the code checks if Username is not nil before dereferencing it, but doesn't check Password. This causes a nil pointer panic if username is provided without a password.

Before:

if r.Username != nil {
    authConfig.Username = *r.Username
    authConfig.Password = *r.Password  // panic if Password is nil
}

After:

if r.Username != nil && r.Password != nil {
    authConfig.Username = *r.Username
    authConfig.Password = *r.Password
}

Documentation

  • Is documentation needed for this update?

No - this is a bug fix with no user-facing changes.

Related Documentation PR (if applicable)

N/A


PR Type

Bug fix


Description

  • Add nil check for Password before dereferencing in ToDockerV2AuthString()

  • Prevents nil pointer panic when username provided without password


Diagram Walkthrough

flowchart LR
  A["ToDockerV2AuthString method"] -->|checks Username and Password| B["Both not nil"]
  A -->|Username nil or Password nil| C["Return nil, nil"]
  B -->|dereference safely| D["Set authConfig values"]
Loading

File Walkthrough

Relevant files
Bug fix
models.go
Add Password nil check in auth string conversion                 

pkg/provider/models/models.go

  • Added nil check for r.Password alongside existing r.Username check
  • Prevents panic when dereferencing Password pointer in else branch
  • Maintains existing logic flow with improved safety
+1/-1     

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 27, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢
🎫 #97
🟢 Prevent nil pointer panic in pkg/provider/models/models.go within ToDockerV2AuthString()
when Username is provided but Password is nil.
Add a nil check for Password alongside the existing Username nil check (i.e., require both
to be non-nil before dereferencing).
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 27, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Return an error on incomplete credentials

Return an error if credentials are incomplete (i.e., username or password is
provided, but not both) to avoid silent failures.

pkg/provider/models/models.go [108-113]

 		if r.Username != nil && r.Password != nil {
 			authConfig.Username = *r.Username
 			authConfig.Password = *r.Password
+		} else if r.Username != nil || r.Password != nil {
+			return nil, fmt.Errorf("incomplete credentials: both username and password must be provided")
 		} else {
 			return nil, nil
 		}
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that incomplete credentials (e.g., only a username) are silently ignored, which can be confusing. Returning an error in this specific case, while still allowing for no credentials, improves the function's robustness and provides better feedback to the user.

Medium
  • Update

@AR21SM
Copy link
Contributor Author

AR21SM commented Jan 27, 2026

@tsebastiani @paigerube14

@tsebastiani
Copy link
Contributor

please update the fork and rebase on main

Signed-off-by: AR21SM <mahajanashishar21sm@gmail.com>
@AR21SM AR21SM force-pushed the fix/nil-pointer-auth-string branch from 88d412d to 719daa6 Compare January 30, 2026 02:50
@AR21SM
Copy link
Contributor Author

AR21SM commented Jan 30, 2026

please update the fork and rebase on main

done

@AR21SM
Copy link
Contributor Author

AR21SM commented Feb 9, 2026

@tsebastiani

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Nil pointer dereference in ToDockerV2AuthString()

2 participants