diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32a2d3c8d..31f6f9e95 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -434,6 +434,19 @@ jobs: if: github.event_name != 'pull_request' runs-on: ubuntu-latest continue-on-error: true + # `prev-tag` (nightly main) and `prev-release-tag` (stable release branches): + # the tag the current build's patches were actually generated FROM. Carried + # through to publish-nightly so it can record the SAME value as the + # `from-version` annotation — see push-patches below. If we let + # publish-nightly re-derive this from `oras repo tags` (or `gh release + # list`) it can disagree with what generate-patches used (e.g. a + # concurrent push lands a new tag between the two jobs, or semver + # ordering shifts when a 0.41 nightly precedes a 0.40 nightly), and + # `binpatch` consumers would then receive a patch annotated with a + # `from-version` that doesn't match the actual source binary. + outputs: + prev-tag: ${{ steps.set-prev-tag.outputs.prev-tag }} + prev-release-tag: ${{ steps.set-prev-release-tag.outputs.prev-release-tag }} steps: - name: Install ORAS CLI # Only needed on main (to fetch previous nightly from GHCR) @@ -467,6 +480,7 @@ jobs: merge-multiple: true - name: Download previous nightly binaries (main) + id: set-prev-tag if: github.ref == 'refs/heads/main' run: | VERSION="${{ needs.changes.outputs.nightly-version }}" @@ -491,6 +505,9 @@ jobs: exit 0 fi echo "HAS_PREV=true" >> "$GITHUB_ENV" + # Record the tag we actually used so publish-nightly can stamp the + # `from-version` annotation with the SAME value (see job output). + echo "prev-tag=${PREV_TAG}" >> "$GITHUB_OUTPUT" echo "Previous nightly: ${PREV_TAG}" # Download and decompress previous nightly binaries @@ -504,6 +521,7 @@ jobs: done - name: Download previous release binaries (release/**) + id: set-prev-release-tag if: startsWith(github.ref, 'refs/heads/release/') env: GH_TOKEN: ${{ github.token }} @@ -516,6 +534,9 @@ jobs: exit 0 fi echo "HAS_PREV=true" >> "$GITHUB_ENV" + # Record the tag we actually used so publish-nightly can stamp the + # `from-version` annotation with the SAME value (see job output). + echo "prev-release-tag=${PREV_TAG}" >> "$GITHUB_OUTPUT" echo "Previous release: ${PREV_TAG}" shopt -s nullglob @@ -727,19 +748,32 @@ jobs: exit 0 fi - # Find from-version by listing GHCR nightly tags - TAGS=$(oras repo tags "${REPO}" 2>/dev/null | grep '^nightly-[0-9]' | sort -V || echo "") - PREV_TAG="" - for tag in $TAGS; do - if [ "$tag" = "nightly-${VERSION}" ]; then break; fi - PREV_TAG="$tag" - done + # Use the from-version tag that generate-patches actually used to + # generate the patch bytes — NOT a re-derived tag listing here. + # Re-deriving (via `oras repo tags` or `gh release list`) can disagree + # with what generate-patches saw if the registry state changed + # between the two jobs (e.g. a concurrent push, semver re-ordering). + # binpatch verifies only the final output SHA, so a mismatched + # `from-version` annotation silently bypasses the user's expected + # upgrade path. See binpatch docs: + # https://github.com/BYK/binpatch/blob/main/website/src/content/docs/security.md + PREV_TAG_NIGHTLY="${{ needs.generate-patches.outputs.prev-tag }}" + PREV_TAG_RELEASE="${{ needs.generate-patches.outputs.prev-release-tag }}" + + if [ "${{ github.ref }}" = "refs/heads/main" ]; then + PREV_TAG="$PREV_TAG_NIGHTLY" + else + PREV_TAG="$PREV_TAG_RELEASE" + fi if [ -z "$PREV_TAG" ]; then - echo "No previous nightly tag found, skipping patch push" + echo "No previous tag recorded by generate-patches, skipping patch push" exit 0 fi PREV_VERSION="${PREV_TAG#nightly-}" + # For release branches the tag is the GitHub release tag, not a + # `nightly-` prefix — strip a leading `v` if present. + PREV_VERSION="${PREV_VERSION#v}" cd patches eval oras push "${REPO}:patch-${VERSION}" \ diff --git a/packages/cli/src/lib/upgrade.ts b/packages/cli/src/lib/upgrade.ts index a56bb98a6..00a8301bf 100644 --- a/packages/cli/src/lib/upgrade.ts +++ b/packages/cli/src/lib/upgrade.ts @@ -9,11 +9,13 @@ import { spawn } from "node:child_process"; import { chmodSync, - createWriteStream, + closeSync, existsSync, + openSync, realpathSync, statSync, unlinkSync, + writeSync, } from "node:fs"; import { writeFile } from "node:fs/promises"; import { homedir } from "node:os"; @@ -562,13 +564,75 @@ export type DownloadResult = { patchBytes?: number; }; +/** + * Write `chunk` to `fd` in full, looping to handle short writes + * (interrupted syscalls, partial writes under memory pressure). + * + * Throws on a 0-byte return — that's the kernel telling us the fd is + * unwritable and no further progress is possible, so we surface it + * rather than spin forever. + */ +function writeChunkSync(fd: number, chunk: Uint8Array): void { + let written = 0; + while (written < chunk.byteLength) { + const n = writeSync(fd, chunk, written, chunk.byteLength - written); + if (n <= 0) { + throw new Error( + `writeSync returned ${n} for chunk of ${chunk.byteLength - written} bytes` + ); + } + written += n; + } +} + +/** + * Drain a decompressed body into `fd`, awaiting the underlying stream + * pipeline. Returns the terminal stream error (if any) and any error + * raised by the synchronous write loop. The fd is owned by the caller — + * this function never closes it. + */ +async function drainBodyToFd( + body: ReadableStream, + fd: number, + onBytes: (n: number) => void +): Promise<{ streamError: unknown; writeError: Error | undefined }> { + let writeError: Error | undefined; + let streamError: unknown; + try { + for await (const chunk of body.pipeThrough( + new DecompressionStream("gzip") + )) { + if (writeError) { + break; + } + try { + writeChunkSync(fd, chunk); + onBytes(chunk.byteLength); + } catch (err) { + writeError = err instanceof Error ? err : new Error(String(err)); + break; + } + } + } catch (err) { + streamError = err; + } + return { streamError, writeError }; +} + /** * Stream a response body through a decompression transform and write to disk. * - * Uses a manual `for await` loop with `Bun.file().writer()` instead of - * `Bun.write(path, Response)` to work around a Bun event-loop bug where - * streaming response bodies get GC'd before completing. - * See: https://github.com/oven-sh/bun/issues/13237 + * Uses `fs.openSync` + `fs.writeSync` + `fs.closeSync` (NOT + * `fs.createWriteStream`) so the output fd is released synchronously + * before this function returns. Node's `writer.end()` callback fires on + * the `'finish'` event — data flushed, not fd released — and a + * subsequent `spawn` of the output file on Linux then fails with + * `ETXTBSY` ("text file busy") in a small but reproducible race window. + * `fs.writeFile` (used for the raw fallback path) does not exhibit the + * bug because it closes the fd synchronously; this function used to be + * inconsistent with that behavior — full-download fallbacks hit ETXTBSY + * while direct full-downloads succeeded. Closing synchronously makes + * both paths race-free. * * @param body - Readable stream from a fetch response * @param destPath - File path to write the decompressed output @@ -578,74 +642,33 @@ async function streamDecompressToFile( destPath: string, setMessage?: SetMessage ): Promise { - const stream = body.pipeThrough(new DecompressionStream("gzip")); - const writer = createWriteStream(destPath); // Indeterminate byte counter: the decompressed size isn't known ahead of // time (Content-Length covers only the compressed stream), so we show a live // byte count rather than a misleading fraction. Feeds the surrounding // spinner via setMessage — cosmetic, never aborts. const progress = makeByteProgress("Downloading", null, setMessage); - // Capture write errors early — without a listener, Node crashes with - // ERR_UNHANDLED_ERROR if a write fails (ENOSPC, EIO, etc.) during the loop. - let writeError: Error | undefined; - writer.on("error", (err) => { - writeError ??= err; - }); + const fd = openSync(destPath, "w", 0o644); - // Track the original streaming error so a later writer.end() rejection can't - // mask it: an exception thrown from a finally overwrites a pending try - // exception. We rethrow the ORIGINAL error preferentially. - let streamError: unknown; + // Drain the body to the fd. Awaited so we know before closeSync whether + // the drain terminated with an error. + const { streamError, writeError } = await drainBodyToFd(body, fd, (n) => + progress.onProgress(n) + ); + progress.done(); + + // Synchronous close — guarantees the fd is released before this function + // returns, so the caller can `spawn` the output file immediately. If a + // write error already exists, the close error is moot — surface the write. try { - for await (const chunk of stream) { - if (writeError) { - break; - } - const ok = writer.write(chunk); - progress.onProgress(chunk.byteLength); - if (!(ok || writeError)) { - // Race drain against error — an I/O failure (ENOSPC) while the - // buffer is full would never emit 'drain', causing a hang. - // Clean up the unused listener to avoid MaxListenersExceededWarning. - await new Promise((resolve) => { - const onDrain = (): void => { - writer.removeListener("error", onError); - resolve(); - }; - const onError = (): void => { - writer.removeListener("drain", onDrain); - resolve(); - }; - writer.once("drain", onDrain); - writer.once("error", onError); - }); - } - } + closeSync(fd); } catch (err) { - streamError = err; - } finally { - progress.done(); + if (!writeError) { + throw err instanceof Error ? err : new Error(String(err)); + } } - // Always flush/close the writer. If the stream already failed, surface THAT - // error (the root cause) and demote any end() rejection to a debug log so it - // can't mask the original. - try { - await new Promise((resolve, reject) => { - writer.end((err?: Error | null) => { - const finalErr = err ?? writeError; - if (finalErr) { - reject(finalErr); - } else { - resolve(); - } - }); - }); - } catch (endErr) { - if (streamError === undefined) { - throw endErr; - } - log.debug(`writer.end failed after a stream error: ${String(endErr)}`); + if (writeError) { + throw writeError; } if (streamError !== undefined) {