Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cli/bunfig.zig
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ pub const Bunfig = struct {
}
}

if (comptime cmd.isNPMRelated() or cmd == .RunCommand or cmd == .AutoCommand or cmd == .TestCommand) {
if (comptime cmd.isNPMRelated() or cmd == .RunCommand or cmd == .AutoCommand or cmd == .TestCommand or cmd == .UpgradeCommand) {
if (json.getObject("install")) |install_obj| {
var install: *api.BunInstall = this.ctx.install orelse brk: {
const install = try this.allocator.create(api.BunInstall);
Expand Down
289 changes: 232 additions & 57 deletions src/cli/upgrade_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,125 @@
var unzip_path_buf: bun.PathBuffer = undefined;
var tmpdir_path_buf: bun.PathBuffer = undefined;

/// Result of parsing a single GitHub release entry for the current
/// platform's matching asset. `published_at_ms` is the release's
/// `published_at` timestamp converted to epoch ms, or `null` when
/// GitHub did not return the field (e.g. in mocked responses).
const ParsedRelease = struct {
version: Version,
published_at_ms: ?f64,
};

/// Fill in `version.zip_url` / `version.size` by walking a release's
/// `assets` array looking for the zip matching this platform. Returns
/// `true` if a matching asset was found. `version.tag` must already be
/// populated by the caller.
fn findAssetInRelease(
allocator: std.mem.Allocator,
release_expr: js_ast.Expr,
use_profile: bool,
version: *Version,
) bool {
const assets_ = release_expr.asProperty("assets") orelse return false;
var assets = assets_.expr.asArray() orelse return false;

while (assets.next()) |asset| {
if (asset.asProperty("content_type")) |content_type| {
const content_type_ = (content_type.expr.asString(allocator)) orelse continue;
if (comptime Environment.isDebug) {
Output.prettyln("Content-type: {s}", .{content_type_});
Output.flush();
}

if (!strings.eqlComptime(content_type_, "application/zip")) continue;
}

if (asset.asProperty("name")) |name_| {
if (name_.expr.asString(allocator)) |name| {
if (comptime Environment.isDebug) {
const filename = if (!use_profile) Version.zip_filename else Version.profile_zip_filename;
Output.prettyln("Comparing {s} vs {s}", .{ name, filename });
Output.flush();
}

if (!use_profile and !strings.eqlComptime(name, Version.zip_filename)) continue;
if (use_profile and !strings.eqlComptime(name, Version.profile_zip_filename)) continue;

version.zip_url = (asset.asProperty("browser_download_url") orelse return false).expr.asString(allocator) orelse return false;
if (comptime Environment.isDebug) {
Output.prettyln("Found Zip {s}", .{version.zip_url});
Output.flush();
}

if (asset.asProperty("size")) |size_| {
if (size_.expr.data == .e_number) {
version.size = @as(u32, @intCast(@max(@as(i32, @intFromFloat(std.math.ceil(size_.expr.data.e_number.value))), 0)));
}
}
return true;
}
}
}
return false;
}

/// Parse a single release object (as returned by the GitHub releases
/// API) and extract tag, matching asset, and publish timestamp.
///
/// Returns `null` when the release lacks a usable `tag_name` or a
/// matching platform asset. `version.buf` is not set here — the
/// caller owns the HTTP response body.
fn parseReleaseObject(
allocator: std.mem.Allocator,
release_expr: js_ast.Expr,
use_profile: bool,
) ?ParsedRelease {
if (release_expr.data != .e_object) return null;

var version = Version{ .zip_url = "", .tag = "", .buf = MutableString.initEmpty(allocator), .size = 0 };

if (release_expr.asProperty("tag_name")) |tag_name_| {
if (tag_name_.expr.asString(allocator)) |tag_name| {
version.tag = tag_name;
}
}

if (version.tag.len == 0) return null;

var published_at_ms: ?f64 = null;
if (release_expr.asProperty("published_at")) |published_at_expr| {
if (published_at_expr.expr.asString(allocator)) |published_at_str| {
if (published_at_str.len > 0) {
if (bun.jsc.wtf.parseES5Date(published_at_str) catch null) |ms| {
published_at_ms = ms;
}
}
}
}

if (!findAssetInRelease(allocator, release_expr, use_profile, &version)) return null;

return .{ .version = version, .published_at_ms = published_at_ms };
}

/// Epoch-ms "now" consistent with how the install-side resolver
/// computes it from `bun.start_time`. Used to age-gate upgrades.
fn currentTimestampMs() f64 {
return @floatFromInt(@divTrunc(bun.start_time, std.time.ns_per_ms));
}

fn isReleaseTooRecent(published_at_ms: f64, minimum_release_age_ms: f64) bool {
return published_at_ms > currentTimestampMs() - minimum_release_age_ms;
}

pub fn getLatestVersion(
allocator: std.mem.Allocator,
env_loader: *DotEnv.Loader,
refresher: ?*Progress,
progress: ?*Progress.Node,
use_profile: bool,
comptime silent: bool,
minimum_release_age_ms: ?f64,
) !?Version {
var headers_buf: string = default_github_headers;
// gonna have to free memory myself like a goddamn caveman due to a thread safety issue with ArenaAllocator
Expand Down Expand Up @@ -184,7 +296,7 @@
defer if (comptime silent) log.deinit();
const source = &logger.Source.initPathString("releases.json", metadata_body.list.items);
initializeStore();
var expr = JSON.parseUTF8(source, &log, allocator) catch |err| {
const expr = JSON.parseUTF8(source, &log, allocator) catch |err| {
if (!silent) {
progress.?.end();
refresher.?.refresh();
Expand Down Expand Up @@ -214,8 +326,6 @@
return null;
}

var version = Version{ .zip_url = "", .tag = "", .buf = metadata_body, .size = 0 };

if (expr.data != .e_object) {
if (!silent) {
progress.?.end();
Expand All @@ -229,77 +339,137 @@
return null;
}

if (expr.asProperty("tag_name")) |tag_name_| {
if (tag_name_.expr.asString(allocator)) |tag_name| {
version.tag = tag_name;
}
}

if (version.tag.len == 0) {
const latest = parseReleaseObject(allocator, expr, use_profile) orelse {
if (comptime !silent) {
progress.?.end();
refresher.?.refresh();

Output.prettyErrorln("JSON Error parsing releases from GitHub: <r><red>tag_name<r> is missing?\n{s}", .{metadata_body.list.items});
Global.exit(1);
// Distinguish "no tag_name" from "no matching asset" for clearer errors.
const has_tag = if (expr.asProperty("tag_name")) |t| t.expr.asString(allocator) != null else false;
if (!has_tag) {
Output.prettyErrorln("JSON Error parsing releases from GitHub: <r><red>tag_name<r> is missing?\n{s}", .{metadata_body.list.items});
Global.exit(1);
}
// Tag name is present but no asset for this platform.
var tmp = Version{ .zip_url = "", .tag = "", .buf = MutableString.initEmpty(allocator), .size = 0 };
if (expr.asProperty("tag_name")) |t| {
if (t.expr.asString(allocator)) |s| tmp.tag = s;
}
if (tmp.name()) |name| {
Output.prettyErrorln("Bun v{s} is out, but not for this platform ({s}) yet.", .{
name, Version.triplet,
});
}
Global.exit(0);
}

return null;
}
};

get_asset: {
const assets_ = expr.asProperty("assets") orelse break :get_asset;
var assets = assets_.expr.asArray() orelse break :get_asset;
// Honor `install.minimumReleaseAge`: if the latest release was
// published too recently, fall back to the releases list endpoint
// and pick the newest release that passes the window.
if (minimum_release_age_ms) |min_age_ms| {
if (min_age_ms > 0 and latest.published_at_ms != null and isReleaseTooRecent(latest.published_at_ms.?, min_age_ms)) {
if (try findEligibleVersionFromReleasesList(allocator, env_loader, progress, use_profile, silent, github_api_domain, header_entries, headers_buf, min_age_ms)) |fallback| {
Comment on lines +372 to +374

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail closed when minimumReleaseAge is set but published_at is missing.

Line 373 currently skips enforcement when latest.published_at_ms is null, which allows a policy bypass. With minimumReleaseAge enabled, unknown publish time should be treated as ineligible and force fallback/error.

Suggested fix
-        if (minimum_release_age_ms) |min_age_ms| {
-            if (min_age_ms > 0 and latest.published_at_ms != null and isReleaseTooRecent(latest.published_at_ms.?, min_age_ms)) {
+        if (minimum_release_age_ms) |min_age_ms| {
+            if (min_age_ms > 0) {
+                const latest_is_ineligible = if (latest.published_at_ms) |published_ms|
+                    isReleaseTooRecent(published_ms, min_age_ms)
+                else
+                    true; // fail closed when timestamp is unavailable
+
+                if (latest_is_ineligible) {
                     if (try findEligibleVersionFromReleasesList(allocator, env_loader, progress, use_profile, silent, github_api_domain, header_entries, headers_buf, min_age_ms)) |fallback| {
                         return fallback;
                     }
@@
-                return null;
-            }
+                    return null;
+                }
+            }
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/upgrade_command.zig` around lines 372 - 374, The current check skips
age enforcement when latest.published_at_ms is null; change the condition in the
upgrade flow so that when minimum_release_age_ms is set (>0) a missing
published_at_ms is treated as ineligible. Replace the existing conjunctive check
using latest.published_at_ms != null and isReleaseTooRecent(...) with a check
that triggers fallback when min_age_ms > 0 and (latest.published_at_ms == null
or isReleaseTooRecent(latest.published_at_ms.?, min_age_ms)); keep the call to
findEligibleVersionFromReleasesList(...) as the fallback path so unknown publish
times force fallback/error.

return fallback;
}

while (assets.next()) |asset| {
if (asset.asProperty("content_type")) |content_type| {
const content_type_ = (content_type.expr.asString(allocator)) orelse continue;
if (comptime Environment.isDebug) {
Output.prettyln("Content-type: {s}", .{content_type_});
Output.flush();
}
if (comptime !silent) {
progress.?.end();
refresher.?.refresh();

if (!strings.eqlComptime(content_type_, "application/zip")) continue;
const age_seconds = min_age_ms / std.time.ms_per_s;
if (latest.version.name()) |name| {
Output.prettyErrorln(
"<r><red>error:<r> No Bun release is older than the configured <b>minimumReleaseAge<r> ({d}s). Latest release is <b>v{s}<r>.",
.{ age_seconds, name },
);
} else {
Output.prettyErrorln(
"<r><red>error:<r> No Bun release is older than the configured <b>minimumReleaseAge<r> ({d}s).",
.{age_seconds},
);
}
Global.exit(1);
}
return null;
}
}

if (asset.asProperty("name")) |name_| {
if (name_.expr.asString(allocator)) |name| {
if (comptime Environment.isDebug) {
const filename = if (!use_profile) Version.zip_filename else Version.profile_zip_filename;
Output.prettyln("Comparing {s} vs {s}", .{ name, filename });
Output.flush();
}
var v = latest.version;
v.buf = metadata_body;
return v;
}

if (!use_profile and !strings.eqlComptime(name, Version.zip_filename)) continue;
if (use_profile and !strings.eqlComptime(name, Version.profile_zip_filename)) continue;
/// Fallback used when the latest release is inside the
/// `minimumReleaseAge` window. Fetches `/releases?per_page=10`
/// (newest first) and returns the newest release whose
/// `published_at` is older than `minimum_release_age_ms`.
fn findEligibleVersionFromReleasesList(
allocator: std.mem.Allocator,
env_loader: *DotEnv.Loader,
progress: ?*Progress.Node,
use_profile: bool,
comptime silent: bool,
github_api_domain: string,
header_entries: Headers.Entry.List,
headers_buf: string,
minimum_release_age_ms: f64,
) !?Version {
var list_url_buf: bun.PathBuffer = undefined;
const list_url = URL.parse(
try std.fmt.bufPrint(
&list_url_buf,
"https://{s}/repos/Jarred-Sumner/bun-releases-for-updater/releases?per_page=10",
.{github_api_domain},
),
);

version.zip_url = (asset.asProperty("browser_download_url") orelse break :get_asset).expr.asString(allocator) orelse break :get_asset;
if (comptime Environment.isDebug) {
Output.prettyln("Found Zip {s}", .{version.zip_url});
Output.flush();
}
const http_proxy: ?URL = env_loader.getHttpProxyFor(list_url);

if (asset.asProperty("size")) |size_| {
if (size_.expr.data == .e_number) {
version.size = @as(u32, @intCast(@max(@as(i32, @intFromFloat(std.math.ceil(size_.expr.data.e_number.value))), 0)));
}
}
return version;
}
}
}
var body = try MutableString.init(allocator, 8192);

var async_http: *HTTP.AsyncHTTP = try allocator.create(HTTP.AsyncHTTP);
async_http.* = HTTP.AsyncHTTP.initSync(
allocator,
.GET,
list_url,
header_entries,
headers_buf,
&body,
"",
http_proxy,
null,
HTTP.FetchRedirect.follow,
);
async_http.client.flags.reject_unauthorized = env_loader.getTLSRejectUnauthorized();
if (!silent) async_http.client.progress_node = progress.?;

const response = try async_http.sendSync();
switch (response.status_code) {
200 => {},
else => return null, // fall back to "no eligible release"
}
Comment on lines +450 to 453

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Propagate fallback endpoint HTTP failures instead of collapsing them to “no eligible release”.

Line 452 returns null for every non-200 from /releases, so rate-limit/auth/server failures are misreported as a minimumReleaseAge miss. Propagate specific HTTP errors like the primary /releases/latest path does.

Suggested fix
         const response = try async_http.sendSync();
         switch (response.status_code) {
             200 => {},
-            else => return null, // fall back to "no eligible release"
+            404 => return error.HTTP404,
+            403 => return error.HTTPForbidden,
+            429 => return error.HTTPTooManyRequests,
+            499...599 => return error.GitHubIsDown,
+            else => return error.HTTPError,
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/upgrade_command.zig` around lines 450 - 453, The switch on
response.status_code inside the fallback /releases handler currently maps every
non‑200 to "return null", hiding HTTP errors; change it to propagate HTTP
failures like the primary path does instead of collapsing them to a missing
eligible release. Specifically, in the same scope where you inspect
response.status_code (the switch around response.status_code), handle 200 as
success and for other status codes return or propagate an appropriate Zig
error/result that includes the HTTP status (mirroring the behavior used for
/releases/latest), so rate limits/auth/server errors surface instead of being
treated as a minimumReleaseAge miss.


if (comptime !silent) {
progress.?.end();
refresher.?.refresh();
if (version.name()) |name| {
Output.prettyErrorln("Bun v{s} is out, but not for this platform ({s}) yet.", .{
name, Version.triplet,
});
}
var log = logger.Log.init(allocator);
defer if (comptime silent) log.deinit();

Global.exit(0);
const source = &logger.Source.initPathString("releases-list.json", body.list.items);
initializeStore();
const list_expr = JSON.parseUTF8(source, &log, allocator) catch return null;
if (log.errors > 0) return null;

var releases = list_expr.asArray() orelse return null;
while (releases.next()) |release_expr| {
const parsed = parseReleaseObject(allocator, release_expr, use_profile) orelse continue;
// Skip releases we can't age-gate (missing published_at).
const published_at_ms = parsed.published_at_ms orelse continue;
if (isReleaseTooRecent(published_at_ms, minimum_release_age_ms)) continue;

var v = parsed.version;
v.buf = body;
return v;

Check failure on line 472 in src/cli/upgrade_command.zig

View check run for this annotation

Claude / Claude Code Review

Fallback releases-list loop can downgrade Bun and select prereleases

The fallback loop returns the first release whose `published_at` is outside the window with no other filtering, so it can return a release **older than the currently-installed Bun** — and since `_exec` only checks `version.isCurrent()` (exact equality, not `>`), `bun upgrade` will then silently downgrade the binary. The loop should skip candidates whose tag is `<=` the current version (or `_exec` should treat a selected version `<` current as "already on latest"); it would also be worth skipping
Comment on lines +462 to +472

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.

🔴 The fallback loop returns the first release whose published_at is outside the window with no other filtering, so it can return a release older than the currently-installed Bun — and since _exec only checks version.isCurrent() (exact equality, not >), bun upgrade will then silently downgrade the binary. The loop should skip candidates whose tag is <= the current version (or _exec should treat a selected version < current as "already on latest"); it would also be worth skipping entries with prerelease: true, since GET /releases includes prereleases whereas the primary /releases/latest path does not.

Extended reasoning...

What the bug is

findEligibleVersionFromReleasesList iterates the newest-first GET /releases?per_page=10 response and returns the first entry whose published_at falls outside the minimumReleaseAge window:

while (releases.next()) |release_expr| {
    const parsed = parseReleaseObject(allocator, release_expr, use_profile) orelse continue;
    const published_at_ms = parsed.published_at_ms orelse continue;
    if (isReleaseTooRecent(published_at_ms, minimum_release_age_ms)) continue;

    var v = parsed.version;
    v.buf = body;
    return v;
}

There is no comparison against the currently-installed version, and no check of the prerelease / draft flags. Back in _exec (src/cli/upgrade_command.zig:560-568), the only post-resolution guard is:

if (version.name() != null and version.isCurrent()) {
    Output.prettyErrorln("Congrats! You're already on the latest version of Bun ...");
    Global.exit(0);
}

isCurrent() is exact string equality with "bun-v" ++ Global.package_json_version. There is no semver > comparison anywhere in the upgrade path. Previously this was safe because getLatestVersion only ever returned the result of /releases/latest, which is by construction >= the installed version, so the only outcomes were "exact match → already latest" or "newer → upgrade". The new fallback path breaks that invariant.

Step-by-step proof (downgrade)

  1. User installs Bun via curl -fsSL https://bun.com/install | bash (or homebrew/npm). These installers ignore bunfig and install the latest release — say v1.2.5, published 3 days ago.
  2. User has [install] minimumReleaseAge = 604800 (7 days) in a project-local bunfig.toml for npm supply-chain safety.
  3. User runs bun upgrade from that directory.
  4. getLatestVersion fetches /releases/latestv1.2.6 (or even v1.2.5 itself), published < 7 days ago → isReleaseTooRecent is true → enters the fallback.
  5. findEligibleVersionFromReleasesList walks /releases newest-first: v1.2.6 (2 days, skip), v1.2.5 (3 days, skip), v1.2.4 (10 days, eligible) → returned.
  6. Back in _exec: version.isCurrent() compares "bun-v1.2.4" against "bun-v1.2.5"false.
  7. Bun prints Bun v1.2.4 is out! You're on v1.2.5 and proceeds to download, verify (the post-download check only confirms the binary's --version matches the selected tag 1.2.4, not that it's >= current), and replace the running binary with v1.2.4.

This is a real, easy-to-hit regression: Bun ships releases multiple times per week, and bunfig.toml is project-local while the Bun binary is global, so "installed version is newer than what minimumReleaseAge permits" is the common case, not an edge case.

Secondary issue: prereleases

Per the GitHub REST API, GET /releases/latest returns "the most recent non-prerelease, non-draft release", but GET /releases returns all releases including those with prerelease: true. The fallback loop does not check release_expr.asProperty("prerelease"), so a prerelease whose published_at passes the window would be selected as the "stable" upgrade target — an inconsistency between the primary and fallback paths. Drafts are incidentally covered (their published_at is null, caught by orelse continue), but prereleases have a real timestamp.

On the refutation

One reviewer noted that Jarred-Sumner/bun-releases-for-updater is a curated mirror that has historically never contained prereleases (canary uses a separate hardcoded oven-sh/bun/releases/download/canary URL that doesn't go through this code). That's accurate — the prerelease concern is low practical risk today. However: (a) the downgrade issue above does not depend on prereleases existing and is fully reproducible against the real repo as it stands; (b) for the prerelease filter specifically, this PR's stated motivation is supply-chain risk reduction, and adding if (prerelease == true) continue is a one-line defensive check that makes the fallback path semantically match the primary path rather than silently relying on an external repo's publishing conventions. So the prerelease point is a defensive nit, but the downgrade point is a blocking correctness bug.

How to fix

In the while (releases.next()) loop, before returning:

  • Skip when the candidate tag equals Version.current_version (treat as "already on latest" — return null so the caller prints the "no eligible release" message, or have _exec short-circuit when the selected tag <= current).
  • Skip when the candidate tag is older than the current version (a simple semver comparison on the bun-vX.Y.Z suffix, or stop iterating once you pass the current tag since the list is newest-first).
  • Skip when release_expr.asProperty("prerelease") is true (and optionally draft, though that's already covered by the null published_at).

Alternatively, guard in _exec after getLatestVersion returns: if the selected version < Global.package_json_version, print the "Congrats! You're already on the latest version" message and exit 0 instead of downloading.

}

return null;
Expand Down Expand Up @@ -373,11 +543,16 @@

const use_profile = strings.containsAny(bun.argv, "--profile");

// `install.minimumReleaseAge` (seconds → ms) from bunfig, if any.
// Only applies to the stable channel — canary upgrades don't use
// published_at timestamps and are opted into explicitly.
const minimum_release_age_ms: ?f64 = if (ctx.install) |install_config| install_config.minimum_release_age_ms else null;

var version: Version = if (!use_canary) v: {
var refresher = Progress{};
var progress = refresher.start("Fetching version tags", 0);

const version = (try getLatestVersion(ctx.allocator, &env_loader, &refresher, progress, use_profile, false)) orelse return;
const version = (try getLatestVersion(ctx.allocator, &env_loader, &refresher, progress, use_profile, false, minimum_release_age_ms)) orelse return;

progress.end();
refresher.refresh();
Expand Down
3 changes: 3 additions & 0 deletions src/options_types/CommandTag.zig
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub const Tag = enum {
.OutdatedCommand,
.PublishCommand,
.AuditCommand,
.UpgradeCommand,
=> true,
else => false,
};
Expand Down Expand Up @@ -136,6 +137,7 @@ pub const Tag = enum {
.UpdateInteractiveCommand = true,
.PublishCommand = true,
.AuditCommand = true,
.UpgradeCommand = true,
});

pub const always_loads_config: std.EnumArray(Tag, bool) = std.EnumArray(Tag, bool).initDefault(false, .{
Expand All @@ -153,6 +155,7 @@ pub const Tag = enum {
.UpdateInteractiveCommand = true,
.PublishCommand = true,
.AuditCommand = true,
.UpgradeCommand = true,
});

pub const uses_global_options: std.EnumArray(Tag, bool) = std.EnumArray(Tag, bool).initDefault(true, .{
Expand Down
Loading
Loading