From e20ebcedb7c64c527bfa24fff0b2660f4662f79e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 25 Oct 2026 04:45:38 +0000 Subject: [PATCH 1/2] publish: wait for ENTER inside the web login's retry wait A web login started a thread that read stdin until a line end, and nothing stopped that thread when the login ended. On a terminal it took the first line typed at a postpublish question, or at the OTP question of a later package. From a piped stdin it took blocks of 1024 bytes. The wait between two requests to the done url is now the wait for ENTER, so no reader of stdin outlives the login. A terminal is read through a second, non-blocking open of it. A stdin that is not a terminal is read one byte at a time, up to the first line end. Windows makes no offer of ENTER, because its opener cannot be spawned. --- src/runtime/cli/publish_command.rs | 202 ++++++++---- test/cli/install/bun-publish.test.ts | 448 ++++++++++++++++++++++++++- 2 files changed, 585 insertions(+), 65 deletions(-) diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index fedb790237ec..46ad8cabc7ac 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -1,5 +1,6 @@ use bun_collections::VecExt; use std::io::Write as _; +use std::time::{Duration, Instant}; use crate::Error; use bun_alloc::AllocError; @@ -115,18 +116,127 @@ type SHA512Digest = [u8; sha::SHA512::DIGEST]; pub(crate) struct PublishCommand; -/// A thread that is blocked reading stdin cannot be stopped. When `--recursive` needs another -/// login, it reuses the thread that is still waiting for ENTER instead of starting a second one. -struct WebLogin { - /// The URL that pressing ENTER opens. `None` once the login has finished. - auth_url: Option<&'static [u8]>, - waits_for_enter: bool, +/// Where a web login looks for the ENTER that opens its page. +#[cfg(unix)] +enum EnterOffer { + /// A second, non-blocking open of the terminal that stdin is. + Terminal(File), + /// stdin itself: not a terminal, or a terminal that cannot be opened a second time. + Stdin, } -static WEB_LOGIN: bun_core::Mutex = bun_core::Mutex::new(WebLogin { - auth_url: None, - waits_for_enter: false, -}); +#[cfg(unix)] +enum Waited { + Enter, + Nothing, + EndOfInput, +} + +#[cfg(unix)] +impl EnterOffer { + fn new() -> Option { + if !Output::is_stdin_tty() { + return Some(Self::Stdin); + } + let reopened = crate::webcore::file_reader::open_as_nonblocking_tty( + Fd::stdin().native(), + bun_sys::O::RDONLY, + ); + let offer = if reopened >= 0 { + // Not fd 0: its read blocks when another reader of the terminal takes the line first. + Self::Terminal(File::from_fd(Fd::from_native(reopened))) + } else { + Self::Stdin + }; + // macOS cannot poll every terminal device (/dev/tty), and ENTER cannot be seen there. + (offer.readable_within(Duration::ZERO).is_some()).then_some(offer) + } + + fn fd(&self) -> Fd { + match self { + Self::Terminal(terminal) => terminal.handle(), + Self::Stdin => Fd::stdin(), + } + } + + /// `None`: the input cannot be watched, or it ended. + fn readable_within(&self, timeout: Duration) -> Option { + let mut polled = [bun_sys::posix::PollFd { + fd: self.fd().native(), + events: bun_sys::posix::POLL_IN, + revents: 0, + }]; + let timeout = + core::ffi::c_int::try_from(timeout.as_millis()).unwrap_or(core::ffi::c_int::MAX); + match bun_sys::posix::poll(&mut polled, timeout) { + Ok(0) => Some(false), + Ok(_) if polled[0].revents & bun_sys::posix::POLL_IN != 0 => Some(true), + _ => None, + } + } + + fn wait(&self, deadline: Instant) -> Waited { + let mut line = [0u8; 1024]; + // A terminal gives one line to a read. A pipe is read byte by byte, up to the line end. + let block = match self { + Self::Terminal(_) => line.len(), + Self::Stdin => 1, + }; + loop { + let left = deadline.saturating_duration_since(Instant::now()); + match self.readable_within(left) { + Some(true) => {} + Some(false) => return Waited::Nothing, + None => return Waited::EndOfInput, + } + match bun_sys::read(self.fd(), &mut line[..block]) { + Ok(0) => return Waited::EndOfInput, + Ok(len) if strings::contains_char(&line[..len], b'\n') => return Waited::Enter, + Ok(_) if left.is_zero() => return Waited::Nothing, + Ok(_) => {} + // Another reader of the terminal took the line first. + Err(err) if err.is_retry() => return Waited::Nothing, + Err(_) => return Waited::EndOfInput, + } + } + } + + /// Whether ENTER is pressed before `deadline`. It ends the offer, and so does the end of input. + fn pressed_before(offer: &mut Option, deadline: Instant) -> bool { + let Some(source) = offer else { + return false; + }; + let waited = source.wait(deadline); + if !matches!(waited, Waited::Nothing) { + *offer = None; + } + matches!(waited, Waited::Enter) + } + + /// A line that was typed on the terminal during the last request answered the offer. + fn take_late_enter(&self) { + if matches!(self, Self::Terminal(_)) { + let _ = self.wait(Instant::now()); + } + } +} + +/// Windows makes no offer: `start`, its opener, is not a program that can be spawned (#26231). +#[cfg(not(unix))] +enum EnterOffer {} + +#[cfg(not(unix))] +impl EnterOffer { + fn new() -> Option { + None + } + + fn pressed_before(_: &mut Option, _: Instant) -> bool { + false + } + + fn take_late_enter(&self) {} +} /// What `bun publish` sends. The scripts are `None` when publishing a tarball, which doesn't run /// any. @@ -880,33 +990,15 @@ impl PublishCommand { Ok(true) } - fn press_enter_to_open_in_browser() { - // This prevents backspace from deleting the entire line - #[cfg(windows)] - let _stdin_mode = - bun_sys::windows::StdinModeGuard::set(bun_sys::windows::UpdateStdioModeFlagsOpts { - unset: bun_sys::windows::ENABLE_VIRTUAL_TERMINAL_INPUT, - ..Default::default() + /// On a thread: `xdg-open` can stay until the browser is closed. + fn open_in_browser(auth_url: &[u8]) { + let auth_url = auth_url.to_vec(); + let _ = std::thread::Builder::new() + .name("WebLoginOpener".into()) + .stack_size(256 * 1024) + .spawn(move || { + let _ = bun_core::spawn_sync_inherit(&[open::OPENER, auth_url.as_slice()]); }); - - // This runs on its own thread, and `Output::buffered_stdin()` is for the main thread only. - let mut input = [0u8; 1024]; - loop { - match bun_sys::read(Fd::stdin(), &mut input) { - Ok(0) | Err(_) => return, - Ok(len) if strings::contains_char(&input[..len], b'\n') => break, - Ok(_) => continue, - } - } - - let auth_url = { - let mut login = WEB_LOGIN.lock(); - login.waits_for_enter = false; - login.auth_url.take() - }; - if let Some(auth_url) = auth_url { - let _ = bun_core::spawn_sync_inherit(&[open::OPENER, auth_url]); - } } fn get_otp( @@ -931,8 +1023,6 @@ impl PublishCommand { let Some(auth_url_str) = json.get_string_cloned(&bump, b"authUrl")? else { break 'try_web; }; - // The thread that opens it outlives `bump`. - let auth_url_str: &'static [u8] = cli_dupe(auth_url_str); let auth_url = URL::parse(auth_url_str); let shown_auth_url = bun_fmt::escape_control_chars(auth_url_str).to_string(); // ENTER only opens a URL that the box shows exactly as it is, with nothing escaped. @@ -952,7 +1042,12 @@ impl PublishCommand { break 'try_web; } - if auth_url_is_web { + let mut enter_offer = if auth_url_is_web { + EnterOffer::new() + } else { + None + }; + if enter_offer.is_some() { bun_core::prettyln!( "\nAuthenticate your account at (press ENTER to open in browser):\n", ); @@ -981,24 +1076,6 @@ impl PublishCommand { )); Output::flush(); - // on another thread because pressing enter is not required - if auth_url_is_web { - let mut login = WEB_LOGIN.lock(); - login.auth_url = Some(auth_url_str); - if !std::mem::replace(&mut login.waits_for_enter, true) - && std::thread::Builder::new() - .spawn(Self::press_enter_to_open_in_browser) - .is_err() - { - Output::err( - "ThreadSpawn", - "failed to spawn thread for opening auth url", - (), - ); - Global::crash(); - } - } - let auth_headers = Self::construct_publish_headers(manager, registry, None, None)?; loop { @@ -1022,10 +1099,17 @@ impl PublishCommand { strings::parse_int::(trimmed, 10).ok() }) .map_or(500, |seconds| u64::from(seconds) * 1000); - std::thread::sleep(std::time::Duration::from_millis(milliseconds)); + let retry_at = Instant::now() + Duration::from_millis(milliseconds); + // ENTER is awaited here, so no reader of stdin outlives the login. + if EnterOffer::pressed_before(&mut enter_offer, retry_at) { + Self::open_in_browser(auth_url_str); + } + std::thread::sleep(retry_at.saturating_duration_since(Instant::now())); } 200 => { - WEB_LOGIN.lock().auth_url = None; + if let Some(offer) = enter_offer.take() { + offer.take_late_enter(); + } let done_bump = bun_alloc::Arena::new(); let otp_done_source = diff --git a/test/cli/install/bun-publish.test.ts b/test/cli/install/bun-publish.test.ts index c39f7786d8ba..d4149022b4d8 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -51,6 +51,85 @@ export async function publish( return { out, err, exitCode }; } +/** Polls until `condition` holds. The timeout of the test is the deadline. */ +async function until(condition: () => boolean) { + while (!condition()) await Bun.sleep(1); +} + +/** + * Resolves once the main thread of the process `pid` waits between two requests to a done url: it is in poll(2), + * where a web login looks for ENTER, or in a sleep. Linux only, by the numbers of these system calls on x64 or arm64. + */ +function untilWaitingToRetry(pid: number) { + const waits = process.arch === "arm64" ? /^(73|101|115) / : /^(7|35|230|271) /; + return until(() => waits.test(readFileSync(`/proc/${pid}/task/${pid}/syscall`, "utf8"))); +} + +/** + * `cmd` (by default `bun publish` with `args`) on a terminal: its stdin, stdout and stderr are one PTY. On Windows, + * ConPTY redraws the output of the process, so only what is printed on one line can be waited for there. + */ +function publishOnTerminal(env: any, cwd: string, args: string[] = [], cmd = [bunExe(), "publish", ...args]) { + const decoder = new TextDecoder(); + // as the terminal has it: a "\n" of the process is "\r\n" here + let raw = ""; + let waiting: { text: string; from: number; resolve: () => void; reject: (error: Error) => void }[] = []; + const closed = Promise.withResolvers(); + const printed = () => (isWindows ? Bun.stripANSI(raw) : raw); + const proc = spawn({ + cmd, + cwd, + env, + terminal: { + cols: 200, + rows: 50, + data(_, chunk) { + raw += decoder.decode(chunk, { stream: true }); + const found = waiting.filter(({ text, from }) => printed().includes(text, from)); + waiting = waiting.filter(entry => !found.includes(entry)); + for (const { resolve } of found) resolve(); + }, + // The terminal is closed once the process has exited and the rest of its output was delivered. + exit() { + for (const { text, reject } of waiting) { + reject(new Error(`the terminal closed before it printed ${JSON.stringify(text)}:\n${raw}`)); + } + closed.resolve(); + }, + }, + }); + const printedFrom = (text: string, from: number) => { + if (printed().includes(text, from)) return Promise.resolve(); + const { promise, resolve, reject } = Promise.withResolvers(); + waiting.push({ text, from, resolve, reject }); + return promise; + }; + return { + pid: proc.pid, + /** Resolves once the process has printed `text`. */ + printed: (text: string) => printedFrom(text.replaceAll("\n", "\r\n"), 0), + /** Types `keys`, with "\r" for ENTER. */ + write: (keys: string) => void proc.terminal!.write(keys), + /** + * Types `keys` and waits for their echo. The terminal echoes a key when its line discipline has taken it, so + * after this a reader of the terminal finds the typed lines. The echo of ENTER alone is only a line end: to + * tell it from the output of the process, wait for what the process printed last before you type. Not Windows. + */ + type(keys: string) { + const from = raw.length; + proc.terminal!.write(keys); + return printedFrom(keys.replaceAll("\r", "\r\n"), from); + }, + /** The output with "\n" line ends, and the exit code. */ + async exited() { + const [exitCode] = await Promise.all([proc.exited, closed.promise]); + raw += decoder.decode(); + return { output: printed().replaceAll("\r\n", "\n"), exitCode }; + }, + [Symbol.asyncDispose]: () => proc[Symbol.asyncDispose](), + }; +} + /** * A directory with `package.json` and the auth bunfig for `registry`. A checkout that ran these tests against * verdaccio may still have the package in the fixture directory, so the registry is told to forget it first. @@ -230,6 +309,25 @@ async function expectPublishedManifest( return tarball; } +/** The answer of a web login's done url while the login is not complete. bun asks again at once. */ +const notDoneYet = () => new Response("{}", { status: 202, headers: { "retry-after": "0" } }); + +/** + * An `xdg-open`, which is what Linux opens a page with, and the environment that has it first on PATH. It records + * the url it was given, one line for each time it ran. macOS opens a page with the absolute `/usr/bin/open`, which a + * test cannot replace, so the tests in which ENTER opens a page are for Linux. + */ +function fakeOpener() { + const dir = tempDir("publish-opener", { "xdg-open": `#!/bin/sh\necho "$1" >> "$OPENED"\n` }); + chmodSync(join(dir, "xdg-open"), 0o755); + const log = join(dir, "opened.txt"); + return { + env: { ...env, OPENED: log, PATH: [String(dir), env.PATH ?? ""].join(delimiter) }, + opened: () => (existsSync(log) ? readFileSync(log, "utf8") : ""), + [Symbol.dispose]: () => dir[Symbol.dispose](), + }; +} + describe.concurrent("otp", () => { /** * The registry for the OTP tests. The first PUT is answered with a 401 that asks for a one-time password, the @@ -245,9 +343,11 @@ describe.concurrent("otp", () => { npmNotice?: boolean; /** add `x-local-cache`, which tells bun to ignore `npm-notice` */ xLocalCache?: boolean; + /** runs for every request to the done url. What it returns is the answer, and without one the token is */ + done?: () => Response | void | Promise; }) { const { otp, setAuthHeader = true, otpFail = false, npmNotice = false, xLocalCache = false } = opts; - return mockRegistry((req, recorded, mock) => { + return mockRegistry(async (req, recorded, mock) => { if (req.method === "PUT") { if (recorded.otp === otp) { if (otpFail) { @@ -273,13 +373,38 @@ describe.concurrent("otp", () => { ); } if (recorded.pathname === "/done") { - return Response.json({ token: otp }); + return (await opts.done?.()) ?? Response.json({ token: otp }); } // `/auth` is shown to the user. `bun publish` must never request it. return new Response(`unexpected request: ${req.method} ${recorded.pathname}`, { status: 500 }); }); } + // A web login offers to open its page on ENTER. Windows makes no offer, because `start`, its opener, is not a + // program that can be spawned. This gives the output of Windows the heading of the other platforms. + const withOffer = (out: string) => + isWindows + ? out.replace( + "\nAuthenticate your account at:\n", + "\nAuthenticate your account at (press ENTER to open in browser):\n", + ) + : out; + + /** + * Presses ENTER while bun waits for the answer of the done url. The box around the auth url is the last thing bun + * printed before it asked, so the line end that follows the box is the echo of ENTER. + */ + async function enterDuringRequest(terminal: ReturnType, mock: MockRegistry) { + const authUrl = `http://localhost:${mock.port}/auth`; + const border = `|${Buffer.alloc(authUrl.length + 2, "-")}|`; + await terminal.printed(`${border}\n| ${authUrl} |\n${border}\n`); + await terminal.type("\r"); + } + + /** A `postpublish` script for `sh` that asks a question and saves the line it reads from stdin as the answer. */ + const question = "Push the tag? "; + const askScript = `printf '${question}' && read -r answer && echo "$answer" > answer.txt`; + test.each([ ["", { setAuthHeader: true }], [" (without auth header)", { setAuthHeader: false }], @@ -291,7 +416,7 @@ describe.concurrent("otp", () => { const { out, err, exitCode } = await publish(env, String(dir)); expect(err).toBe(""); - expect(normalizePublishOutput(out, mock.port)).toMatchInlineSnapshot(` + expect(normalizePublishOutput(withOffer(out), mock.port)).toMatchInlineSnapshot(` "bun publish () packed 97B package.json @@ -333,7 +458,7 @@ describe.concurrent("otp", () => { }); const { out, err, exitCode } = await publish(env, String(dir)); - expect(normalizePublishOutput(out, mock.port)).toMatchInlineSnapshot(` + expect(normalizePublishOutput(withOffer(out), mock.port)).toMatchInlineSnapshot(` "bun publish () packed 97B package.json @@ -548,6 +673,249 @@ describe.concurrent("otp", () => { 30_000, ); + // Between two requests to the done url, bun waits as long as the registry asks. That wait is the wait for ENTER. + test.skipIf(!isLinux)( + "ENTER pressed while bun waits for the next request to the done url opens the page", + async () => { + const otp = "otp-13"; + using opener = fakeOpener(); + // A second request within the test means that the wait of 30 seconds was not kept. + const wait30 = () => new Response("{}", { status: 202, headers: { "retry-after": "30" } }); + using mock = otpRegistry({ otp, done: wait30 }); + using dir = tempDir("publish-web-login-enter-in-wait", { + "package.json": JSON.stringify({ name: "otp-pkg-13", version: "1.3.0" }), + "bunfig.toml": mockBunfig(mock), + }); + + await using terminal = publishOnTerminal(opener.env, String(dir)); + await untilWaitingToRetry(terminal.pid); + terminal.write("\r"); + await until(() => opener.opened() !== ""); + + expect(opener.opened()).toBe(`http://localhost:${mock.port}/auth\n`); + expect(mock.requests.filter(r => r.pathname === "/done")).toHaveLength(1); + }, + ); + + // While bun waits for the answer of the done url, it does not look at the terminal. It does when the answer is in. + test.skipIf(!isLinux)("ENTER pressed during a request to the done url opens the page after the answer", async () => { + const otp = "otp-11"; + let polls = 0; + using opener = fakeOpener(); + using mock = otpRegistry({ + otp, + async done() { + if (++polls === 1) await enterDuringRequest(terminal, mock); + if (opener.opened() === "") return notDoneYet(); + }, + }); + using dir = tempDir("publish-web-login-enter-in-flight", { + "package.json": JSON.stringify({ name: "otp-pkg-11", version: "1.1.1" }), + "bunfig.toml": mockBunfig(mock), + }); + + await using terminal = publishOnTerminal(opener.env, String(dir)); + const { output, exitCode } = await terminal.exited(); + + expect(output).toContain("\n + otp-pkg-11@1.1.1\n"); + expect(opener.opened()).toBe(`http://localhost:${mock.port}/auth\n`); + expect(exitCode).toBe(0); + }); + + // When the token arrives, the login is over and there is nothing left to open. The ENTER was still an answer to the + // offer, so whatever reads the terminal next must not get it as an empty line. + test.skipIf(isWindows)( + "ENTER pressed during the request that returns the token opens nothing and is not left for the postpublish script", + async () => { + const otp = "otp-10"; + using opener = fakeOpener(); + using mock = otpRegistry({ otp, done: () => enterDuringRequest(terminal, mock) }); + using dir = tempDir("publish-web-login-late-enter", { + "package.json": JSON.stringify({ name: "otp-pkg-10", version: "1.0.0", scripts: { postpublish: askScript } }), + "bunfig.toml": mockBunfig(mock), + }); + + await using terminal = publishOnTerminal(opener.env, String(dir)); + await terminal.printed(question); + await terminal.type("first\rsecond\r"); + const { exitCode } = await terminal.exited(); + + expect(opener.opened()).toBe(""); + expect(await file(join(dir, "answer.txt")).text()).toBe("first\n"); + expect(exitCode).toBe(0); + }, + ); + + // The login is over when the done url hands out the token. From then on, nothing of the login reads the terminal. + test.skipIf(isWindows)( + "after a web login on a terminal, a question of the postpublish script gets the first line that is typed", + async () => { + const otp = "otp-9"; + using mock = otpRegistry({ otp }); + using dir = tempDir("publish-web-login-question", { + "package.json": JSON.stringify({ name: "otp-pkg-9", version: "9.9.9", scripts: { postpublish: askScript } }), + "bunfig.toml": mockBunfig(mock), + }); + + await using terminal = publishOnTerminal(env, String(dir)); + await terminal.printed(question); + await terminal.type("first\rsecond\r"); + const { output, exitCode } = await terminal.exited(); + + expect(output).toContain("\nAuthenticate your account at (press ENTER to open in browser):\n"); + expect(await file(join(dir, "answer.txt")).text()).toBe("first\n"); + expect(exitCode).toBe(0); + }, + ); + + // `dd` reads the terminal before bun does, so it gets the typed line. A read of bun that waited for the next line + // would stop the login. It is a race that a wrong build loses in about 6 of 10 runs, hence the five runs. + test.skipIf(!isLinux).each([1, 2, 3, 4, 5])( + "a line that another reader of the terminal takes first does not stall the login (run %d)", + async () => { + const otp = "otp-15"; + using opener = fakeOpener(); + using other = tempDir("publish-web-login-other-reader", {}); + const taken = join(other, "taken.txt"); + const hasLine = () => existsSync(taken) && readFileSync(taken, "utf8") !== ""; + // Without `retry-after`, bun waits half a second. The login lasts until the other reader has the line. + using mock = otpRegistry({ otp, done: () => (hasLine() ? undefined : new Response("{}", { status: 202 })) }); + using dir = tempDir("publish-web-login-second-reader", { + "package.json": JSON.stringify({ name: "otp-pkg-15", version: "1.5.0" }), + "bunfig.toml": mockBunfig(mock), + }); + + // fd 3 keeps the terminal for `dd`: `sh` gives a command that it starts with `&` /dev/null as stdin. + const withReader = `exec 3<&0; dd bs=1024 count=1 <&3 >"$1" 2>/dev/null & exec "$0" publish 3<&-`; + await using terminal = publishOnTerminal(opener.env, String(dir), [], ["sh", "-c", withReader, bunExe(), taken]); + await untilWaitingToRetry(terminal.pid); + terminal.write("\r"); + const { output, exitCode } = await terminal.exited(); + + expect(output).toContain("\n + otp-pkg-15@1.5.0\n"); + expect(readFileSync(taken, "utf8")).toBe("\n"); + expect(opener.opened()).toBe(""); + expect(exitCode).toBe(0); + }, + ); + + // ENTER is a line end on stdin, whatever stdin is. Of a pipe, bun takes the bytes up to that line end and no more. + test.skipIf(!isLinux)( + "a line end on a piped stdin opens the page, and the rest of the pipe reaches the postpublish script", + async () => { + const otp = "otp-8"; + using opener = fakeOpener(); + // The login lasts until ENTER has opened the page. + using mock = otpRegistry({ otp, done: () => (opener.opened() === "" ? notDoneYet() : undefined) }); + using dir = tempDir("publish-web-login-piped-enter", { + "package.json": JSON.stringify({ + name: "otp-pkg-8", + version: "8.8.8", + scripts: { postpublish: `${bunExe()} save-stdin.js` }, + }), + "bunfig.toml": mockBunfig(mock), + "save-stdin.js": `const fs = require("fs"); fs.writeFileSync("stdin.txt", fs.readFileSync(0));`, + }); + + await using proc = spawn({ + cmd: [bunExe(), "publish"], + cwd: String(dir), + stdin: Buffer.from("\nfor the script\n"), + stdout: "pipe", + stderr: "pipe", + env: opener.env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(err).toBe(`$ ${bunExe()} save-stdin.js\n`); + expect(out).toContain("\n + otp-pkg-8@8.8.8\n"); + expect(opener.opened()).toBe(`http://localhost:${mock.port}/auth\n`); + expect(await file(join(dir, "stdin.txt")).text()).toBe("for the script\n"); + expect(exitCode).toBe(0); + }, + ); + + // bun looks for ENTER only while it waits for the next request to the done url. Here it never has to wait. + test("a web login that the done url completes at once reads nothing of a piped stdin", async () => { + const otp = "otp-16"; + using opener = fakeOpener(); + using mock = otpRegistry({ otp }); + using dir = tempDir("publish-web-login-piped", { + "package.json": JSON.stringify({ + name: "otp-pkg-16", + version: "1.6.0", + scripts: { postpublish: `${bunExe()} save-stdin.js` }, + }), + "bunfig.toml": mockBunfig(mock), + "save-stdin.js": `const fs = require("fs"); fs.writeFileSync("stdin.txt", fs.readFileSync(0));`, + }); + + await using proc = spawn({ + cmd: [bunExe(), "publish"], + cwd: String(dir), + stdin: Buffer.from("for the script\n"), + stdout: "pipe", + stderr: "pipe", + env: opener.env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(err).toBe(`$ ${bunExe()} save-stdin.js\n`); + expect(out).toContain("\n + otp-pkg-16@1.6.0\n"); + expect(opener.opened()).toBe(""); + expect(await file(join(dir, "stdin.txt")).text()).toBe("for the script\n"); + expect(exitCode).toBe(0); + }); + + // The done url says how long to wait before the next request. Without `retry-after` it is half a second. + test.each([ + ["a second for `retry-after: 1`", { "retry-after": "1" }, 1000], + ["half a second without `retry-after`", {}, 500], + ] as const)("a web login waits %s before it asks the done url again", async (_, headers, milliseconds) => { + const otp = "otp-14"; + const askedAt: number[] = []; + using mock = otpRegistry({ + otp, + done() { + askedAt.push(performance.now()); + if (askedAt.length === 1) return new Response("{}", { status: 202, headers }); + }, + }); + using dir = tempDir("publish-web-login-wait", { + "package.json": JSON.stringify({ name: "otp-pkg-14", version: "1.4.0" }), + "bunfig.toml": mockBunfig(mock), + }); + + const { err, exitCode } = await publish(env, String(dir)); + expect(err).toBe(""); + expect(askedAt).toHaveLength(2); + expect(askedAt[1] - askedAt[0]).toBeGreaterThanOrEqual(milliseconds); + expect(exitCode).toBe(0); + }); + + // The offer does not depend on what stdout is. + test.skipIf(isWindows)("ENTER is offered on a terminal when stdout is redirected", async () => { + const otp = "otp-12"; + using mock = otpRegistry({ otp }); + using dir = tempDir("publish-web-login-stdout-file", { + "package.json": JSON.stringify({ name: "otp-pkg-12", version: "1.2.0" }), + "bunfig.toml": mockBunfig(mock), + }); + using stdout = tempDir("publish-web-login-stdout", {}); + + const redirected = ["sh", "-c", `exec "$0" publish > "$1"`, bunExe(), join(stdout, "stdout.txt")]; + await using terminal = publishOnTerminal(env, String(dir), [], redirected); + const { output, exitCode } = await terminal.exited(); + + expect(output).toBe(""); + const authUrl = `http://localhost:${mock.port}/auth`; + const border = `|${Buffer.alloc(authUrl.length + 2, "-")}|`; + expect(await file(join(stdout, "stdout.txt")).text()).toEndWith( + `\nAuthenticate your account at (press ENTER to open in browser):\n${border}\n| ${authUrl} |\n${border}\n\n + otp-pkg-12@1.2.0\n`, + ); + expect(exitCode).toBe(0); + }); + // Situation: user has 2FA enabled account with faceid sign-in. // They run `bun publish` with --auth-type=legacy, prompting them // to enter their OTP. Because they have faceid sign-in, they don't @@ -566,7 +934,7 @@ describe.concurrent("otp", () => { const { out, err, exitCode } = await publish(env, String(dir)); expect(err).toBe(shouldIgnoreNotice ? "" : `\nnote: visit http://localhost:${mock.port}/auth to login\n`); - expect(normalizePublishOutput(out, mock.port)).toMatchInlineSnapshot(` + expect(normalizePublishOutput(withOffer(out), mock.port)).toMatchInlineSnapshot(` "bun publish () packed 97B package.json @@ -612,7 +980,7 @@ describe.concurrent("otp", () => { String(dir), ); expect(err).toBe(""); - expect(normalizePublishOutput(out, mock.port)).toMatchInlineSnapshot(` + expect(normalizePublishOutput(withOffer(out), mock.port)).toMatchInlineSnapshot(` "bun publish () packed 97B package.json @@ -4092,6 +4460,74 @@ describe.concurrent("--recursive and --filter", () => { expect(published(out)).toEqual([" + rec-a@1.0.0", " + rec-b@1.0.0", " + rec-c@1.0.0"]); expect(exitCode).toBe(0); }); + + describe("a one-time password question after a web login", () => { + const otp = "424242"; + + // rec-a is logged in through its done url, which hands out the token at once. rec-b is then asked for a one-time + // password, which is a question on stdin. rec-c needs neither. + function loginThenQuestion() { + return mockRegistry((req, { pathname, otp: sent }, mock) => { + const url = `http://localhost:${mock.port}`; + const challenge = { status: 401, headers: { "www-authenticate": "OTP" } }; + if (req.method === "GET") { + if (pathname === "/done/rec-a") return Response.json({ token: "from-the-done-url" }); + return new Response("not found", { status: 404 }); + } + if (pathname === "/rec-a" && sent === null) { + return Response.json({ authUrl: `${url}/auth`, doneUrl: `${url}/done/rec-a` }, challenge); + } + if (pathname === "/rec-b" && sent === null) return Response.json({}, challenge); + if (pathname === "/rec-b" && sent !== otp) return Response.json({ error: "wrong otp" }, { status: 401 }); + return new Response("OK"); + }); + } + const puts = (mock: MockRegistry) => mock.requests.filter(r => r.method === "PUT").map(r => [r.pathname, r.otp]); + const allPublished = [ + ["/rec-a", null], + ["/rec-a", "from-the-done-url"], + ["/rec-b", null], + ["/rec-b", otp], + ["/rec-c", null], + ]; + + test("gets the first line that is typed on a terminal", async () => { + using mock = loginThenQuestion(); + using dir = workspace(mock); + + await using terminal = publishOnTerminal(env, String(dir), ["-r"]); + await terminal.printed("Enter OTP:"); + terminal.write(`${otp}\r999999\r`); + const { output, exitCode } = await terminal.exited(); + + expect(puts(mock)).toEqual(allPublished); + // ConPTY redraws the output of the process. + if (!isWindows) expect(published(output)).toEqual([" + rec-a@1.0.0", " + rec-b@1.0.0", " + rec-c@1.0.0"]); + expect(exitCode).toBe(0); + }); + + // The login of rec-a does not have to wait for its done url, so it does not look for ENTER on the pipe. + test("gets the line of a piped stdin", async () => { + using mock = loginThenQuestion(); + using opener = fakeOpener(); + using dir = workspace(mock); + + await using proc = spawn({ + cmd: [bunExe(), "publish", "-r"], + cwd: String(dir), + stdin: Buffer.from(`${otp}\n`), + stdout: "pipe", + stderr: "ignore", + env: opener.env, + }); + const [out, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + + expect(puts(mock)).toEqual(allPublished); + expect(opener.opened()).toBe(""); + expect(published(out)).toEqual([" + rec-a@1.0.0", " + rec-b@1.0.0", " + rec-c@1.0.0"]); + expect(exitCode).toBe(0); + }); + }); }); describe.concurrent("NODE_TLS_REJECT_UNAUTHORIZED=0", () => { From b46ac228a1f8f184bf024c4a8148b4aaba1bddef Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:47:27 +0000 Subject: [PATCH 2/2] publish: take a line of stdin whole when the web login looks for ENTER The deadline of the retry wait stopped the read of a piped stdin after one byte when the wait was zero, so a line was taken in parts and the next reader got the rest of it. The deadline is now only for input that is not there yet. A wait ends the offer after 1024 bytes without a line end, so a stdin such as /dev/zero cannot hold up the login. The line that is typed during the last request is taken on every terminal stdin, and not only on one that could be opened a second time. When the thread for the opener cannot start, that is reported. --- src/runtime/cli/publish_command.rs | 21 ++++++++--- test/cli/install/bun-publish.test.ts | 55 +++++++++++++++++++++------- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index 46ad8cabc7ac..ead62dd5b3f8 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -182,7 +182,9 @@ impl EnterOffer { Self::Terminal(_) => line.len(), Self::Stdin => 1, }; - loop { + let mut taken = 0; + // The deadline is for input that is not there yet. A line that is there is taken whole. + while taken < line.len() { let left = deadline.saturating_duration_since(Instant::now()); match self.readable_within(left) { Some(true) => {} @@ -192,13 +194,14 @@ impl EnterOffer { match bun_sys::read(self.fd(), &mut line[..block]) { Ok(0) => return Waited::EndOfInput, Ok(len) if strings::contains_char(&line[..len], b'\n') => return Waited::Enter, - Ok(_) if left.is_zero() => return Waited::Nothing, - Ok(_) => {} + Ok(len) => taken += len, // Another reader of the terminal took the line first. Err(err) if err.is_retry() => return Waited::Nothing, Err(_) => return Waited::EndOfInput, } } + // This many bytes without a line end are not ENTER, and /dev/zero never has one. + Waited::EndOfInput } /// Whether ENTER is pressed before `deadline`. It ends the offer, and so does the end of input. @@ -215,7 +218,7 @@ impl EnterOffer { /// A line that was typed on the terminal during the last request answered the offer. fn take_late_enter(&self) { - if matches!(self, Self::Terminal(_)) { + if Output::is_stdin_tty() { let _ = self.wait(Instant::now()); } } @@ -993,12 +996,20 @@ impl PublishCommand { /// On a thread: `xdg-open` can stay until the browser is closed. fn open_in_browser(auth_url: &[u8]) { let auth_url = auth_url.to_vec(); - let _ = std::thread::Builder::new() + let opener = std::thread::Builder::new() .name("WebLoginOpener".into()) .stack_size(256 * 1024) .spawn(move || { let _ = bun_core::spawn_sync_inherit(&[open::OPENER, auth_url.as_slice()]); }); + if opener.is_err() { + Output::err( + "ThreadSpawn", + "failed to spawn thread for opening auth url", + (), + ); + Output::flush(); + } } fn get_otp( diff --git a/test/cli/install/bun-publish.test.ts b/test/cli/install/bun-publish.test.ts index d4149022b4d8..4f375fb7c5c7 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -381,14 +381,13 @@ describe.concurrent("otp", () => { } // A web login offers to open its page on ENTER. Windows makes no offer, because `start`, its opener, is not a - // program that can be spawned. This gives the output of Windows the heading of the other platforms. - const withOffer = (out: string) => - isWindows - ? out.replace( - "\nAuthenticate your account at:\n", - "\nAuthenticate your account at (press ENTER to open in browser):\n", - ) - : out; + // program that can be spawned. This checks the heading of Windows, then gives it the offer of the other platforms. + function withOffer(out: string) { + if (!isWindows) return out; + const heading = "\nAuthenticate your account at:\n"; + expect(out).toContain(heading); + return out.replace(heading, "\nAuthenticate your account at (press ENTER to open in browser):\n"); + } /** * Presses ENTER while bun waits for the answer of the done url. The box around the auth url is the last thing bun @@ -799,14 +798,15 @@ describe.concurrent("otp", () => { }, ); - // ENTER is a line end on stdin, whatever stdin is. Of a pipe, bun takes the bytes up to that line end and no more. + // ENTER is a line end on stdin, whatever stdin is. Of a pipe, bun takes the first line, all of it in one wait, and + // no byte after its line end. The done url makes bun wait once, so a line that is taken in parts would be cut. test.skipIf(!isLinux)( - "a line end on a piped stdin opens the page, and the rest of the pipe reaches the postpublish script", + "the first line of a piped stdin opens the page, and the rest of the pipe reaches the postpublish script", async () => { const otp = "otp-8"; + let polls = 0; using opener = fakeOpener(); - // The login lasts until ENTER has opened the page. - using mock = otpRegistry({ otp, done: () => (opener.opened() === "" ? notDoneYet() : undefined) }); + using mock = otpRegistry({ otp, done: () => (++polls === 1 ? notDoneYet() : undefined) }); using dir = tempDir("publish-web-login-piped-enter", { "package.json": JSON.stringify({ name: "otp-pkg-8", @@ -820,7 +820,7 @@ describe.concurrent("otp", () => { await using proc = spawn({ cmd: [bunExe(), "publish"], cwd: String(dir), - stdin: Buffer.from("\nfor the script\n"), + stdin: Buffer.from("open the page\nfor the script\n"), stdout: "pipe", stderr: "pipe", env: opener.env, @@ -829,12 +829,39 @@ describe.concurrent("otp", () => { expect(err).toBe(`$ ${bunExe()} save-stdin.js\n`); expect(out).toContain("\n + otp-pkg-8@8.8.8\n"); - expect(opener.opened()).toBe(`http://localhost:${mock.port}/auth\n`); expect(await file(join(dir, "stdin.txt")).text()).toBe("for the script\n"); + // The opener has its own thread, and bun does not wait for it. + await until(() => opener.opened() !== ""); + expect(opener.opened()).toBe(`http://localhost:${mock.port}/auth\n`); expect(exitCode).toBe(0); }, ); + // bun takes a line that is there whatever it costs, so the search for a line end must have an end of its own. + test.skipIf(isWindows)("a stdin that never has a line end does not hold up a web login", async () => { + const otp = "otp-17"; + let polls = 0; + using mock = otpRegistry({ otp, done: () => (++polls === 1 ? notDoneYet() : undefined) }); + using dir = tempDir("publish-web-login-dev-zero", { + "package.json": JSON.stringify({ name: "otp-pkg-17", version: "1.7.0" }), + "bunfig.toml": mockBunfig(mock), + }); + + await using proc = spawn({ + cmd: [bunExe(), "publish"], + cwd: String(dir), + stdin: file("/dev/zero"), + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(err).toBe(""); + expect(out).toContain("\n + otp-pkg-17@1.7.0\n"); + expect(exitCode).toBe(0); + }); + // bun looks for ENTER only while it waits for the next request to the done url. Here it never has to wait. test("a web login that the done url completes at once reads nothing of a piped stdin", async () => { const otp = "otp-16";