Skip to content

fix: resolve security, reliability, and code quality issues across the codebase - #6

Merged
stepbeta merged 12 commits into
mainfrom
fix/various-issues
Jun 15, 2026
Merged

fix: resolve security, reliability, and code quality issues across the codebase#6
stepbeta merged 12 commits into
mainfrom
fix/various-issues

Conversation

@stepbeta

@stepbeta stepbeta commented Jun 15, 2026

Copy link
Copy Markdown
Owner

This PR addresses findings from a code audit, focusing on supply-chain security (checksum verification, path traversal protection), correctness (symlink race, error shadowing, concurrency-safe flag reads), and housekeeping (stderr routing, restrictive permissions, deduplication).

  • SHA256 checksum verification for downloaded binaries (best-effort)
  • Path traversal protection in tar extraction
  • Re-verify file existence after auto-install in use command
  • Replace panic with cobra.CheckErr on Viper binding failures
  • Convert mutable package-level vars to local flag reads in listRemote
  • Use TrimPrefix instead of Split("-") for version extraction
  • Restrict directory permissions to 0755
  • Direct cache errors and remaining log messages to stderr
  • Use indeterminate progress bar until page count is known
  • De-duplicate GitHub client creation in install
  • Fix deferred Close error variable shadowing

Stefano Ciarcià added 12 commits June 15, 2026 21:45
Using cobra.CheckErr instead of panic provides a cleaner user
experience with a proper error message and os.Exit(1) instead
of an abrupt stack trace.
… listRemote

Move includeDevel, limit, and forceRefresh from package-level variables
to local variables read directly from cobra command flags. This is
concurrency-safe and eliminates confusing viper.Get* mid-execution
overrides of bound flag values.
After the install subcommand succeeds, re-stat the version file
to ensure it was actually created before creating the symlink.
This prevents broken symlinks when install exits successfully
but the expected file is missing.
Add a guard in ExtractSpecificFile that skips archive entries
containing '..' or absolute paths, printing a warning when such
entries are encountered. This prevents malicious tarballs from
writing outside the intended destination directory.
Add best-effort checksum verification for both download paths:

- GitHub releases: search release assets for checksum files (.sha256,
  .sha256sum, SHA256SUMS, etc.), download and parse, verify binary
- Direct URL downloads (kubectl/helm): fetch <url>.sha256 and verify

On verification failure the binary is deleted and installation aborted.
If no checksum file is found, a warning is printed but installation
proceeds (best-effort).

Also fixes:
- Temp file cleanup in DownloadBinary now only runs on error
- Deferred Close error variable no longer shadows the function error
Using strings.Split(fileName, '-') with len==2 check breaks when
tool names or versions contain hyphens. Using strings.TrimPrefix
with the known tool prefix is robust against such inputs.
Cache operations are internal bookkeeping and their error messages
should not mix with CLI output on stdout.
Both getLatestVersion (for 'latest' resolution) and DownloadRelease
created a new GitHub client. Hoist it to the top of the install
function so a single instance is shared.
@stepbeta stepbeta self-assigned this Jun 15, 2026
@stepbeta
stepbeta merged commit c2ba444 into main Jun 15, 2026
2 checks passed
@stepbeta
stepbeta deleted the fix/various-issues branch June 15, 2026 20:36
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