feat(publish): read token from stdin

[omskscream]

FCP comment

What does this PR try to resolve?

Part of #15274
This PR adds support for reading auth token from stdin as an alternative to the existing --token command line option. This is needed to deprecate the mentioned --token option.
Which is, in turn, needed to avoid tokens being saved in shell history.

How to test and review this PR?

Please check added test case in first and second commits to see how behavior changes.
(I hope I understand the "tests and commits" guidance better this time 🤔)

r? @epage

[omskscream]

I would really like to know why most windows-based and one mac-based "Test" checks were terminated in Github before I added this not a terminal condition here like in similar implementation in login command. Process hanged waiting input with no input provided? 🤔

[weihanglo]

(I hope I understand the "tests and commits" guidance better this time 🤔)

Yes that seems correct :)

And thank you for the contribution.

[weihanglo]

Thanks. I'll do an FCP now.

[weihanglo]

@rfcbot fcp merge

I'd like to propose we

• (this PR) Support reading the registry token from stdin for cargo publish
• (future PR) Deprecate the --token option in cargo publish in terms of
  • Remove the mention of the --token option from help text and doc.
  • Give a warning when the --token option is used

Note that cargo login already does it since 1.86 and an FCP was done: #15057 (comment).

[rust-rfcbot]

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

• @0xPoe
• @Eh2406
• @Muscraft
• @arlosi
• @ehuss
• @epage
• @joshtriplett
• @weihanglo

Concerns:

• behavior-change resolved by feat(publish): read token from stdin #16001 (comment)
• heuristics (feat(publish): read token from stdin #16001 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[epage]

@rfcbot concern behavior-change

https://github.com/rust-lang/cargo/pull/16001/files#r2375989885

[weihanglo]

https://github.com/rust-lang/cargo/actions/runs/17979102378/job/51140134992?pr=16001

#7 [ 2/10] RUN apk add --no-cache openssh git
#7 0.138 fetch https://dl-cdn.alpinelinux.org/alpine/v3.22/main/x86_64/APKINDEX.tar.gz
#7 0.284 fetch https://dl-cdn.alpinelinux.org/alpine/v3.22/community/x86_64/APKINDEX.tar.gz
#7 0.563 (1/22) Installing brotli-libs (1.1.0-r2)
#7 0.585 (2/22) Installing c-ares (1.34.5-r0)
#7 0.601 (3/22) Installing libunistring (1.3-r0)
#7 0.626 (4/22) Installing libidn2 (2.3.7-r0)
#7 0.642 (5/22) Installing nghttp2-libs (1.65.0-r0)
#7 0.658 (6/22) Installing libpsl (0.21.5-r3)
#7 0.673 (7/22) Installing zstd-libs (1.5.7-r0)
#7 0.693 (8/22) Installing libcurl (8.14.1-r1)
#7 0.712 (9/22) Installing libexpat (2.7.2-r0)
#7 0.727 (10/22) Installing pcre2 (10.43-r1)
#7 0.746 (11/22) Installing git (2.49.1-r0)
#7 0.820 (12/22) Installing git-init-template (2.49.1-r0)
#7 0.835 (13/22) Installing openssh-keygen (10.0_p1-r8)
#7 0.853 (14/22) Installing ncurses-terminfo-base (6.5_p20250503-r0)
#7 0.871 (15/22) Installing libncursesw (6.5_p20250503-r0)
#7 0.888 (16/22) Installing libedit (20250104.3.1-r1)
#7 0.904 (17/22) Installing openssh-client-common (10.0_p1-r8)
#7 0.933 (18/22) Installing openssh-client-default (10.0_p1-r8)
#7 0.953 (19/22) Installing openssh-sftp-server (10.0_p1-r8)
#7 0.969 (20/22) Installing openssh-server-common (10.0_p1-r8)
#7 0.984 (21/22) Installing openssh-server (10.0_p1-r8)
#7 1.014 (22/22) Installing openssh (10.0_p1-r8)
#7 1.031 Executing busybox-1.37.0-r18.trigger
#7 1.037 OK: 26 MiB in 38 packages
#7 DONE 2.9s

#8 [ 3/10] RUN ssh-keygen -A
#8 0.122 OpenSSL version mismatch. Built against 3050003f, you have 30500010
#8 ERROR: process "/bin/sh -c ssh-keygen -A" did not complete successfully: exit code: 255

A fix is on the way: #16010

[epage]

@rfcbot resolve behavior-change

[0xPoe]

Thanks!

[ehuss]

I was a little confused reading this. I'm wondering if it would help to be more explicit about only reading stdin if it is not a terminal. Maybe something like this?

Suggested change

	Instead of `--token` option, _token_ value might also be provided via stdin.
	Instead of using the `--token` option, the _token_ value may be piped in via stdin.

[omskscream]

I rephrased it a bit more to mention new flag too.

[ehuss]

@rfcbot concern heuristics

I worry that this is making a heavy assumption that in non-interactive environments stdin is connected to /dev/null (or equivalent), thus reading an "empty" token. I don't know how universally that works. I believe things like cron set stdin to /dev/null, but do all environments do the same? What about the myriad CI environments? What about various Windows environments? I worry that cargo publish could get stuck in some situations trying to read from stdin. IsTerminal uses a bunch of heuristics that may not always be correct.

I also worry that cargo publish won't get much widespread testing on nightly. If it hits stable, and then people have problems, what do we do? Once it is stable, we can't (easily) just revert it.

I understand there is some concern that there would be a lack of symmetry with cargo login, but I think cargo login is a fundamentally different command or action than cargo publish.

Can we consider some explicit means of saying "read from stdin"?

[omskscream]

Can we consider some explicit means of saying "read from stdin"?

Hi, and thanks for your concern. As I mentioned in a conversation earlier, my main reason for this was to keep token related behavior consistent across cargo commands.
Note that there're also cargo owner and cargo yank commands that accept --token argument from command line. I would assume that these will be updated the same way as cargo publish in the future, and would prefer them to share same interface -- they are more closely related than cargo login probably.

However, regarding the main concern I agree that explicitness might be less risky option here.
I can introduce a new command line flag for cargo publish, e.g. --token-stdin whose presence will be the required condition for reading token from stdin and no stdin read should happen if the flag is not set.
Another option might be to reuse existing --token option and treat missing value as a condition for reading from stdin. However, I feel that it might be confusing, especially for planned "deprecation" of --token option. I also feel that it is more prone to errors, e.g. when user forgets to provide --token value, then cargo tries to read token from stdin but actually reads nothing, and it's not clear what error to show then. It's a change of an existing behavior, so there might be unpredictable consequences somewhere.

To summarize: yeah, let's make this feature safer with a new command line flag? I'm in 🙂
Would also like to hear opinions on this approach change from people who already approved.

[omskscream]

new command line flag

I now wonder if I introduce a new cmd flag to cargo publish command, should I make this flag unstable first? I do not see any reason for skipping the "unstable" step for this one 🤔

[0xPoe]

I worry that this is making a heavy assumption that in non-interactive environments stdin is connected to /dev/null (or equivalent), thus reading an "empty" token. I don't know how universally that works. I believe things like cron set stdin to /dev/null, but do all environments do the same? What about the myriad CI environments? What about various Windows environments? I worry that cargo publish could get stuck in some situations trying to read from stdin.

If I understand correctly, this is a showcase of the concern mentioned here: #16001 (comment)

[omskscream]

In three latest commits I introduced a flag --token-stdin (flag naming suggestions are very welcome btw!), to see how users could explicitly tell cargo publish to read token from stdin.
I made it unstable, and marked as such in help docs. I also marked it as conflicting with --token to prevent possible token duplication -- maybe it's unnecessary, hm.

[weihanglo]

@rfcbot fcp cancel

The team discussed the proposal. The concern raised was that implicitly reading from stdin introduces risks: subprocesses might inadvertently consume the token, and passing stdin through could lead to confusing or unsafe behavior. Unlike cargo login, cargo publish tends to do more load-bearing stuff. For example, we have #14685 asking for running tests through cargo publish. With implicit stdin reading stdin the behavior becomes confusing and not compatible.

We came to the conclusion that we should stick with the original plan of just deprecating the flag, and to not try to read from stdin. Instead, we guide users toward cargo login and environment variables as safer and more explicit mechanisms. Deprecating without a direct replacement also leaves room for users to provide feedback on missing use cases.

[rust-rfcbot]

@weihanglo proposal cancelled.

[weihanglo]

@omskscream I really appreicate you trying to add an unstable flag. That is useful. However, as I concluded earlier, instead of speculation, we might want to deprecate it (a warning) without a direct replacement, so we'll know what is missing better. Sorry I should have posted this conclusion earlier before you added more commits 😞.

[omskscream]

@weihanglo no problem at all, thanks for letting me know 🙂 I'm contributing to learn Rust and its ecosystem, and I've learned a lot already. Good thing that my PR lead to a team conclusion on how to make things even better.