Skip to content

fix: upgrade/uninstall hardening and security dependency bumps - #188

Open
nullun wants to merge 10 commits into
algorandfoundation:mainfrom
nullun:fix/upgrade-uninstall-hardening
Open

fix: upgrade/uninstall hardening and security dependency bumps#188
nullun wants to merge 10 commits into
algorandfoundation:mainfrom
nullun:fix/upgrade-uninstall-hardening

Conversation

@nullun

@nullun nullun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

ℹ Overview

Fixes prompted by a user report of nodekit upgrade failing on macOS with
"No available formula with the name algorand" for a node that was installed
with update.sh rather than Homebrew. Also includes the dependency bumps from
#187 so both changes ship in a single release.

Upgrade/uninstall fixes

  • macOS: upgrade/uninstall now check whether algod is actually managed by
    Homebrew before running brew commands, and return a clear message pointing
    non-brew installs back to their original install method
  • Linux: the same guard using dpkg -s / rpm -q before running apt/dnf
  • nodekit upgrade now exits non-zero when the algod upgrade fails, instead
    of logging the error and exiting 0
  • uninstall --force on macOS now passes each data directory to rm as a
    separate argument; previously they were space-joined into a single path and
    never deleted
  • the self-upgrade debug log now prints the backup path instead of the temp
    path

Dependencies

  • merges fix: bump dependencies #187 (dependency bumps), which will auto-close as merged
  • bumps Go to 1.26.6 on top: govulncheck reports five reachable stdlib
    vulnerabilities in 1.26.5 (GO-2026-6218, -6090, -6088, -5972, -5026), all
    fixed in 1.26.6. The scan is clean after the bump.

Testing

  • go build ./..., go vet, and the full test suite pass on the merged tree
  • go mod verify and go mod tidy are clean
  • govulncheck ./... reports zero reachable vulnerabilities

CI

  • test workflows now derive the Go version from go.mod via go-version-file
    instead of pinning 1.22; builds previously relied on toolchain auto-download

PhearZero and others added 8 commits August 13, 2026 11:13
A failed algod upgrade was only logged, then the command slept,
attempted to start algod anyway and exited 0, hiding the failure
from scripts.
Homebrew-based upgrade and uninstall now check whether the algorand
formula is actually installed with brew before running brew commands,
returning a clear message for updater/manual installs instead of
"No available formula with the name algorand".
Mirrors the macOS Homebrew guard: upgrade and uninstall now verify the
algorand package is installed with dpkg/rpm before running apt or dnf,
returning a clear message for updater/manual installs.
…tall

The paths were joined with spaces into a single rm argument, so the
known data directories were never actually removed.
govulncheck reports five reachable stdlib vulnerabilities in go1.26.5
(GO-2026-6218, GO-2026-6090, GO-2026-6088, GO-2026-5972, GO-2026-5026),
all fixed in go1.26.6.
@nullun
nullun requested a review from PhearZero August 14, 2026 09:15
@nullun nullun added bug Something isn't working enhancement New feature or request labels Aug 14, 2026
@nullun nullun mentioned this pull request Aug 14, 2026
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 35.71429% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.42%. Comparing base (bf1df43) to head (cd6f8ff).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/algod/linux/linux.go 33.33% 6 Missing and 2 partials ⚠️
internal/algod/mac/mac.go 42.85% 6 Missing and 2 partials ⚠️
cmd/upgrade.go 0.00% 1 Missing ⚠️
internal/system/upgrade.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   36.15%   45.42%   +9.27%     
==========================================
  Files          96       96              
  Lines        6005     6025      +20     
==========================================
+ Hits         2171     2737     +566     
+ Misses       3681     3006     -675     
- Partials      153      282     +129     

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 hardens nodekit upgrade/uninstall behavior to better handle nodes not installed via the package manager NodeKit expects (Homebrew on macOS, apt/dnf on Linux), while also rolling in dependency/security upgrades (including a Go version bump).

Changes:

  • Add “package-manager managed?” guards for macOS (Homebrew) and Linux (dpkg/rpm) upgrade/uninstall flows, with clearer user-facing errors for non-managed installs.
  • Make nodekit upgrade fail fast with a non-zero exit when the algod upgrade step fails.
  • Bump Go and multiple Go module dependencies (per the merged dependency update work).

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/system/upgrade.go Fixes self-upgrade debug log to reference the actual backup path.
internal/algod/mac/mac.go Adds Homebrew-managed checks and fixes forced uninstall data-dir deletion argument handling.
internal/algod/linux/linux.go Adds dpkg/rpm “managed install” checks before running apt/dnf operations.
cmd/upgrade.go Ensures algod upgrade failures exit non-zero.
go.mod Updates Go version and dependency versions.
go.sum Updates module checksums to match dependency bumps.
Suppressed comments (1)

internal/algod/mac/mac.go:158

  • trustTap() runs before the Homebrew-managed guard. On systems with Homebrew installed but algod not installed via the managed formula, nodekit upgrade will still execute brew trust ... before returning NotBrewInstalledMsg. Consider moving trustTap() to after the isBrewManaged() check to avoid unnecessary Homebrew side effects for non-brew installs.
	trustTap()

	if !isBrewManaged() {
		return errors.New(NotBrewInstalledMsg)
	}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread go.mod
Comment thread internal/algod/mac/mac.go
nullun added 2 commits August 14, 2026 10:38
Homebrew 6 refuses to load formulae from untrusted taps, so running
isBrewManaged first would misreport a brew-managed install with an
untrusted tap as not managed.
The workflows pinned Go 1.22 while go.mod requires 1.26.6; builds only
worked via toolchain auto-download. Using go-version-file keeps CI in
step with the module directive.
@tasosbit

Copy link
Copy Markdown
Contributor

LGTM but leaving review to @PhearZero

I added a related UI task btw in case you want to add this too @nullun: #189

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants