Skip to content

Conversation

@NLstn
Copy link
Owner

@NLstn NLstn commented Jan 4, 2026

Potential fix for https://github.com/NLstn/go-odata/security/code-scanning/25

In general, the problem is caused by wrapping a non-interface value (here, a map[string]interface{}) into an interface{} and then comparing that interface to nil. Because processMapEntity is declared to return map[string]interface{}, assigning its result directly to a variable of type interface{} ensures the resulting interface value is never nil, making if entityMap != nil always true. To fix this, keep values in their concrete types as much as possible, and avoid unnecessary interface wrapping when your logic depends on distinguishing nil/non-nil.

The best targeted fix here is to (1) change addNavigationLinks so that it uses the same concrete type as processMapEntitymap[string]interface{}—for the processed entity, and (2) remove the impossible nil check. Specifically:

  • Change the type of entityMap in addNavigationLinks from interface{} to map[string]interface{}.
  • The assignments from processMapEntity already match that type.
  • For the processStructEntityOrdered call, perform an explicit type assertion to map[string]interface{} (which is what it presumably returns or is convertible to), and handle the “not ok” case by skipping the assignment.
  • Remove the if entityMap != nil wrapper and instead unconditionally assign to result[i] when the type assertion succeeds.
    These changes are confined to internal/response/navigation_links.go and don’t require new imports or behavior changes; they only tighten the typing and make the control flow explicit.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…l check

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@NLstn
Copy link
Owner Author

NLstn commented Jan 4, 2026

@copilot fix the syntax error

Copy link
Contributor

Copilot AI commented Jan 4, 2026

@NLstn I've opened a new pull request, #377, to work on those changes. Once the pull request is ready, I'll request review from you.

@NLstn NLstn marked this pull request as ready for review January 6, 2026 13:16
Copilot AI review requested due to automatic review settings January 6, 2026 13:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a code scanning alert about an "impossible interface nil check" by refactoring how entity processing results are handled in the addNavigationLinks function. The original issue arose from wrapping a non-nil concrete type (map[string]interface{}) into an interface{} variable, which made the subsequent nil check always evaluate to true.

Key changes:

  • Replaced a shared interface{} variable with scoped concrete-type variables for each processing path
  • Added explicit nil checks for both processMapEntity and processStructEntityOrdered return values
  • Entities that fail processing now leave nil entries in the result array rather than being skipped

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings January 6, 2026 13:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NLstn NLstn merged commit ad850a0 into main Jan 6, 2026
16 checks passed
@NLstn NLstn deleted the alert-autofix-25 branch January 6, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants