Skip to content

Potential fix for code scanning alert no. 2: Incorrect conversion between integer types - #1

Merged
edjeffreys merged 1 commit into
mainfrom
alert-autofix-2
Mar 8, 2026
Merged

edjeffreys merged 1 commit into
mainfrom
alert-autofix-2

Conversation

@edjeffreys

Copy link
Copy Markdown
Owner

Potential fix for https://github.com/edjeffreys/nomad-tailscale-controller/security/code-scanning/2

In general, to fix this class of issue you must ensure that any integer value parsed from a string (especially with strconv.Atoi returning an int) is either (a) parsed directly into the target-sized type using strconv.ParseInt/ParseUint with an appropriate bit size, or (b) range-checked before converting to a smaller-width type. For ports, the valid range is 0–65535, and typically 1–65535 (disallowing zero) for externally visible services.

For this specific case, the best fix with minimal behavioral change is to add an explicit upper-bound (and optionally lower-bound) check before converting svc.Port to uint16 in buildServeConfig. We already default zero ports to 443, so it's reasonable to only accept non-negative ports up to math.MaxUint16 (which is 65535) and treat anything else as invalid, falling back to 443. This prevents wraparound when converting to uint16 without changing the public API or the structure of Service.

Concretely:

  • Edit internal/tailscale/client.go, inside buildServeConfig.
  • Above the loop (after cfg := &ServeConfig{...}), define a local const defaultPort = uint16(443) to keep the default consistent and typed correctly, or just use uint16(443) inline.
  • Replace port := uint16(svc.Port) and its if port == 0 { port = 443 } with logic that:
    • Starts with port := uint16(443) (the default).
    • If svc.Port is > 0 and svc.Port <= math.MaxUint16, set port = uint16(svc.Port).
  • Import math at the top of internal/tailscale/client.go to access math.MaxUint16.

This ensures that any out-of-range or zero port results in a safe default (443), and any valid port (1–65535) is preserved, while eliminating the unsafe narrowing conversion.

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

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@edjeffreys
edjeffreys marked this pull request as ready for review March 8, 2026 12:58
@edjeffreys
edjeffreys merged commit 81b76f0 into main Mar 8, 2026
5 checks passed
@edjeffreys
edjeffreys deleted the alert-autofix-2 branch March 8, 2026 13:03
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.

1 participant