fix: skip nil RequestOptions to prevent nil pointer dereferences on option.apply(r)

[esotuvaka]

🔧 Changes

Prevents API misuse. Perhaps I should've checked the function types closer before hitting this runtime error, but this should eliminate the panic.

• When iterating []RequestOption, skips when option == nil to prevent a runtime nil pointer dereference via option.apply(r)

🔬 Testing

Existing test suite can be extended to include passing nil into variadic RequestOption parameters on different management SDK functions, like {SDK}.Organization.Invitations(ctx, organizationID, nil)

📝 Checklist

• All new/changed/fixed functionality is covered by tests
• I have added documentation for all new/changed functionality (N/A)

[developerkunal]

Hi @esotuvaka ,

Thank you for submitting this pull request! I appreciate the effort to make the SDK more robust against potential misuse.

However, the RequestOption parameters are designed to be variadic and entirely optional—you shouldn't need to pass nil explicitly at all. If no options are required, simply omit them from the function call. For example:

// Instead of this (which could lead to issues if nil is passed):
{SDK}.Organization.Invitations(ctx, organizationID, nil)

// Just do this:
{SDK}.Organization.Invitations(ctx, organizationID)

This aligns with Go's idiomatic handling of optional parameters and avoids the nil pointer dereference risk without additional checks in the code[1]. If there's a specific use case where passing nil is unavoidable or if I'm missing context, could you elaborate? I'd be happy to discuss further or explore alternatives.

[esotuvaka]

Understood. I see the importance of sticking with idioms, but in this case nil is valid input for this function param, which if given crashes the entire process in the caller of the SDK.

We would have to rely on the developer to check that variadic RequestOptions are interfaces that can accept nil as a valid value, and there is no input validation against this.

The use case for this is putting an rbac controlled endpoint in front to allow Admin users of multi-tenant applications to add request params, or just general dynamic options for requests where the derivation of the option comes from a pointer to a thing.

If in the future list calls support filtering, the onus would be on the caller of the SDK to prevent nil input to these functions. This PR aims to prevent these nil pointer dereferences at the source by avoiding invalid input.

tl;dr: SDK as a whole has been really great to work with, I just want to patch this footgun.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.97%. Comparing base (f5d80a5) to head (3a52e07).

Files with missing lines	Patch %	Lines
authentication/authentication_request.go	0.00%	3 Missing and 1 partial ⚠️
management/actions.go	0.00%	1 Missing and 1 partial ⚠️
management/job.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
- Coverage   96.01%   95.97%   -0.05%     
==========================================
  Files          60       60              
  Lines       12236    12243       +7     
==========================================
+ Hits        11749    11750       +1     
- Misses        367      371       +4     
- Partials      120      122       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[developerkunal]

Thanks for the explanation, it makes sense, and I agree that this is a good safety check. Skipping nil RequestOptions is a practical improvement, especially for dynamic use cases like RBAC or conditional logic.

The change looks good overall! Could you add a test to cover the nil case so we can make sure it stays safe going forward? Also, please run make lint to fix the linting issues.

Really appreciate the contribution, thanks again!

[esotuvaka]

Will do! Cheers!

[esotuvaka]

Funny thing, the test cases were practically already there. I learned something interesting about the language today. I guess implicitly Golang interprets options... as an array, and with the TestNewRequest() function it was passing params as:

request, err := api.NewRequest(
	context.Background(),
	testCase.method,
	testCase.endpoint,
	testCase.payload,
	testCase.options...)

Which means:

api.NewRequest(
	context.Background(),
	testCase.method,
	testCase.endpoint,
	testCase.payload,
	nil...)
!=
api.NewRequest(
	context.Background(),
	testCase.method,
	testCase.endpoint,
	testCase.payload,
	nil)

Using values, passing nil... gets translated into empty slice [], which caused the misleading test. This case was hidden the whole time. I created a separate iteration over test cases, with variadic and nil suffixes. Hopefully this meets project testing styles. Happy to hear any feedback.

[developerkunal]

Hi @esotuvaka,

I've updated the test cases to be more focused and cleaner.

The original /Variadic and /Nil approach was testing the wrong thing - it was checking what happens when you pass nil as the entire options parameter, but your fix actually handles nil elements inside the options slice (like []RequestOption{nil, Header(...), nil}).

The new tests directly validate the actual bug you're fixing:

• Mixed nil and valid options in a slice
• All nil options in a slice
• Helper functions that also iterate over options

This approach is simpler, removes the need for conditional assertions, and tests exactly what the PR fixes. The tests now properly fail on the main branch and pass with your changes.

[esotuvaka]

Hi @esotuvaka,

I've updated the test cases to be more focused and cleaner.

The original /Variadic and /Nil approach was testing the wrong thing - it was checking what happens when you pass nil as the entire options parameter, but your fix actually handles nil elements inside the options slice (like []RequestOption{nil, Header(...), nil}).

The new tests directly validate the actual bug you're fixing:

• Mixed nil and valid options in a slice
• All nil options in a slice
• Helper functions that also iterate over options

This approach is simpler, removes the need for conditional assertions, and tests exactly what the PR fixes. The tests now properly fail on the main branch and pass with your changes.

Awesome, thank you so much Kunal. Also saw you tidied up my lint errors. With your finishing touches applied I hope this PR can help SDK users!

[developerkunal]

Hi @esotuvaka ,

I overlooked the signed commits. Can you sign the commits?

[esotuvaka]

Hi @esotuvaka ,

I overlooked the signed commits. Can you sign the commits?

Sure. I'm at work currently so don't have access to home PC with the GPG key configured. I'll take a look tonight

[developerkunal]

Sure! That would be awesome. Thank you for your contribution.

[esotuvaka]

Ah I think I've messed up the history. I'll close this one and try again with a cleaner history and GPG signing from the start. Thankfully this is a small PR

[developerkunal]

Hi @esotuvaka,

We’re releasing go-auth0 today. Would it be possible for you to create the PR for this feature today so we can get it merged?

[esotuvaka]

Apologies, I've had an unexpected deadline for work come up. I think I've missed this release cycle, but I'll get this in as soon as possible. Should have more availability to resolve after Wednesday. Thanks for your patience

[developerkunal]

No issues,
Thank you for working on this. Your contribution is valuable to us.