Skip to content

fix: prevent stack overflow in InputField.Validate (#25)#106

Open
AR21SM wants to merge 1 commit into
krkn-chaos:mainfrom
AR21SM:fix/stack-overflow-validate-issue-25
Open

fix: prevent stack overflow in InputField.Validate (#25)#106
AR21SM wants to merge 1 commit into
krkn-chaos:mainfrom
AR21SM:fix/stack-overflow-validate-issue-25

Conversation

@AR21SM

@AR21SM AR21SM commented Feb 1, 2026

Copy link
Copy Markdown
Contributor

User description

Description

Fixes #25 - Stack overflow on node scenarios.

Added internal validate(value, validateDefault) helper with a boolean flag to prevent infinite recursion when validating default values for non-String types.

Documentation

  • Is documentation needed for this update?

Related Documentation PR (if applicable)

N/A


PR Type

Bug fix


Description

  • Prevent stack overflow in InputField validation logic

  • Add internal validate helper with recursion control flag

  • Validate default values only once to avoid infinite loops


Diagram Walkthrough

flowchart LR
  A["InputField.Validate"] -->|calls with flag true| B["validate helper"]
  B -->|validates default value| C["validate helper"]
  C -->|flag false prevents| D["recursive validation"]
  D -->|avoids| E["stack overflow"]
Loading

File Walkthrough

Relevant files
Bug fix
field.go
Add recursion control to prevent stack overflow                   

pkg/typing/field.go

  • Refactored Validate method to delegate to internal validate helper
    function
  • Added validateDefault boolean parameter to control recursion depth
  • Modified default value validation to call validate with
    validateDefault=false to prevent infinite recursion
  • Updated comment to clarify recursion prevention mechanism
+9/-3     

@qodo-code-review

qodo-code-review Bot commented Feb 1, 2026

Copy link
Copy Markdown

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
🟢
🎫 #25
🟢 Prevent the goroutine stack exceeds ... fatal error: stack overflow occurring in
github.com/krkn-chaos/krknctl/pkg/typing.(*InputField).Validate, particularly for node
scenarios where validation recurses indefinitely.
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 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

Generic: Secure Error Handling

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

Status:
Error detail exposure: The returned error message concatenates underlying validation details (err.Error()) which
may expose internal implementation context if surfaced to end-users.

Referred Code
// the default value is validated recursively (only once to prevent stack overflow)
if validateDefault {
	if _, err := f.validate(selectedValue, false); err != nil {
		return nil, errors.New("schema validation error on default value: " + err.Error())
	}

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

Signed-off-by: AR21SM <mahajanashishar21sm@gmail.com>
@AR21SM AR21SM force-pushed the fix/stack-overflow-validate-issue-25 branch from 473e27a to 154f310 Compare February 1, 2026 16:53
@qodo-code-review

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent panic by checking for nil

*To prevent a potential panic from f.Name if f.Name is nil, add a nil check
before dereferencing it and use a default value in the error message.

pkg/typing/field.go [116-118]

 	if (value == nil || *value == "") && (f.Default == nil || *f.Default == "") && f.Required && f.Type != String {
-		return nil, fmt.Errorf("field `%s` doesn't have a `default` and cannot be nil or empty for type `%s`", *f.Name, f.Type)
+		fieldName := ""
+		if f.Name != nil {
+			fieldName = *f.Name
+		}
+		return nil, fmt.Errorf("field `%s` doesn't have a `default` and cannot be nil or empty for type `%s`", fieldName, f.Type)
 	}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential nil pointer dereference that would cause a panic and provides a robust fix, significantly improving the code's reliability.

Medium
General
Wrap error with fmt.Errorf

Use fmt.Errorf with the %w verb to wrap the underlying error instead of using
string concatenation, improving error propagation.

pkg/typing/field.go [134]

-return nil, errors.New("schema validation error on default value: " + err.Error())
+return nil, fmt.Errorf("schema validation error on default value: %w", err)
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion improves error handling by correctly wrapping the error, which is a Go best practice that preserves the original error context for better debugging.

Medium
Fix error message formatting

Add a missing closing backtick to the error message string for f.Type.String()
to ensure proper formatting.

pkg/typing/field.go [113]

-return nil, errors.New("`" + f.Type.String() + " can be blank, but not null without a default if required")
+return nil, errors.New("`" + f.Type.String() + "` can be blank, but not null without a default if required")
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: This is a minor but correct suggestion that improves the formatting and readability of an error message, making it consistent with other error messages in the file.

Low
  • More

@AR21SM

AR21SM commented Feb 9, 2026

Copy link
Copy Markdown
Contributor Author

@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.

stack overflow on node scenarios

1 participant