max / alloy

Move polkit's three leaves out, and give Prompt a constructor The agent itself is not a god file and stays whole: Prompt holds the sender Listener sets, Agent's Drop closes the receiver in an order its comment calls load-bearing, Tracked withdraws before it forgets, and Conversation is a hand-rolled Future so zbus's executor thread is not blocked. Carving that apart would scatter one argument across five headers. What did accrete are three leaves that reference nothing above them: the helper's wire protocol, the identity choice, and the printable filter. Prompt::new replaces converse's field literal, which reached past two private fields by ancestor privilege and repeated the three printable calls the for_test constructor also made. The stripping now happens in one place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WFBzMprSmNCfvdj2cGZyka

Author: Max Johnson <me@maxj.phd> · 2026-09-08 17:20 UTC

Signed with PGP, not checked

Commit: 2337315435ce09cd5f26ec22eab383cd48730eed

Parent: c50c57a

7 files changed, +1088 insertions, -867 deletions

M crates/alloy/src/polkit.rs +66 -384

			@@ -39,21 +39,19 @@
39	39		use std::collections::HashMap;
40	40		use std::collections::hash_map::Entry;
41	41		use std::future::Future;
42		-	use std::io::{BufRead, BufReader, Write};
43	42		use std::path::{Path, PathBuf};
44	43		use std::pin::Pin;
45		-	use std::process::{Child, Stdio};
46	44		use std::sync::atomic::{AtomicBool, Ordering};
47	45		use std::sync::mpsc::{Receiver, SyncSender, sync_channel};
48	46		use std::sync::{Arc, Mutex, PoisonError};
49	47		use std::task::{Context as TaskContext, Poll, Waker};
50	48
51		-	use anyhow::{Context, Result, anyhow, bail};
	49	+	use anyhow::{Context, Result, anyhow};
52	50		use zbus::blocking::Connection;
53	51		use zbus::interface;
54	52		use zbus::zvariant::{OwnedValue, Value};
55	53
56		-	use crate::cli::{Secret, child_command};
	54	+	use crate::cli::Secret;
57	55
58	56		/// Where polkit keeps the setuid helper that runs the PAM conversation.
59	57		///
			@@ -111,6 +109,52 @@
111	109		}
112	110
113	111		impl Prompt {
	112	+	/// Build a prompt and the channel its answer comes back on.
	113	+	///
	114	+	/// The one place [`printable`](sanitize::printable) is applied. All three
	115	+	/// strings are polkit's and PAM's rather than this code's, and every one of
	116	+	/// them is drawn into a ratatui buffer, which writes graphemes into cells
	117	+	/// as they come. An escape in one would reach the terminal as an escape,
	118	+	/// and a bidi override would reorder a sentence about what is being
	119	+	/// authorized. All three sources are root-owned today, so this is the
	120	+	/// invariant being stated where it is relied on rather than a hole being
	121	+	/// closed.
	122	+	///
	123	+	/// `bound` is the channel's buffer. Zero on the live path, where the send
	124	+	/// is a rendezvous with the screen; one under test, where nothing is
	125	+	/// receiving and a rendezvous would make a dismissal block.
	126	+	fn build(
	127	+	action_id: &str,
	128	+	message: &str,
	129	+	question: &str,
	130	+	echo: bool,
	131	+	withdrawn: Arc<AtomicBool>,
	132	+	bound: usize,
	133	+	) -> (Self, Receiver<Option<Secret>>) {
	134	+	let (reply, answers) = sync_channel(bound);
	135	+	let prompt = Self {
	136	+	action_id: sanitize::printable(action_id),
	137	+	message: sanitize::printable(message),
	138	+	question: sanitize::printable(question),
	139	+	echo,
	140	+	reply,
	141	+	withdrawn,
	142	+	};
	143	+	(prompt, answers)
	144	+	}
	145	+
	146	+	/// The live path's constructor: a rendezvous channel, since the screen is
	147	+	/// on the other end of it.
	148	+	pub(super) fn new(
	149	+	action_id: &str,
	150	+	message: &str,
	151	+	question: &str,
	152	+	echo: bool,
	153	+	withdrawn: Arc<AtomicBool>,
	154	+	) -> (Self, Receiver<Option<Secret>>) {
	155	+	Self::build(action_id, message, question, echo, withdrawn, 0)
	156	+	}
	157	+
114	158		/// Answer the prompt.
115	159		///
116	160		/// The answer is a [`Secret`] and not a `String`. Nothing here is
			@@ -169,18 +213,17 @@
169	213		action_id: &str,
170	214		message: &str,
171	215		) -> (Self, Receiver<Option<Secret>>, Arc<AtomicBool>) {
	216	+	let withdrawn = Arc::new(AtomicBool::new(false));
172	217		// Buffered, unlike the live path: nothing is receiving here, and a
173	218		// rendezvous channel would make a dismissal block the test.
174		-	let (reply, answers) = sync_channel(1);
175		-	let withdrawn = Arc::new(AtomicBool::new(false));
176		-	let prompt = Self {
177		-	action_id: printable(action_id),
178		-	message: printable(message),
179		-	question: printable("Password:"),
180		-	echo: false,
181		-	reply,
182		-	withdrawn: Arc::clone(&withdrawn),
183		-	};
	219	+	let (prompt, answers) = Self::build(
	220	+	action_id,
	221	+	message,
	222	+	"Password:",
	223	+	false,
	224	+	Arc::clone(&withdrawn),
	225	+	1,
	226	+	);
184	227		(prompt, answers, withdrawn)
185	228		}
186	229		}
			@@ -529,7 +572,7 @@
529	572		) -> zbus::fdo::Result<()> {
530	573		drop((icon_name, details));
531	574
532		-	let user = choose_identity(&identities)
	575	+	let user = identity::choose_identity(&identities)
533	576		.ok_or_else(\|\| zbus::fdo::Error::Failed("no identity this agent can ask".into()))?;
534	577
535	578		// Refused rather than served if polkit is already having a conversation
			@@ -545,7 +588,7 @@
545	588		let sender = self.sender.clone();
546	589		let asked_about = cookie.clone();
547	590		let outcome = Conversation::spawn(move \|\| {
548		-	converse(
	591	+	protocol::converse(
549	592		&helper,
550	593		&user,
551	594		&asked_about,
			@@ -651,376 +694,15 @@
651	694		}
652	695		}
653	696
654		-	/// Which of the identities polkit will accept this agent can actually ask.
655		-	///
656		-	/// **This machine's own user first, and that ordering is the security-relevant
657		-	/// part.** polkit sends every identity that would satisfy the action, which on
658		-	/// a `wheel`-administered box is every administrator. Asking for the first
659		-	/// one would mean a console at a laptop routinely prompting for root, teaching
660		-	/// its owner to type the root password into a screen that could have been
661		-	/// anything.
662		-	///
663		-	/// Group identities are skipped. The helper takes a user name, so a group is
664		-	/// not something this can ask for, and expanding one to its members would mean
665		-	/// choosing an administrator on the user's behalf.
666		-	fn choose_identity(identities: &[(String, HashMap<String, OwnedValue>)]) -> Option<String> {
667		-	let uids: Vec<u32> = identities
668		-	.iter()
669		-	.filter(\|(kind, _)\| kind == "unix-user")
670		-	.filter_map(\|(_, details)\| details.get("uid"))
671		-	.filter_map(\|uid\| u32::try_from(uid).ok())
672		-	.collect();
	697	+	mod identity;
	698	+	mod protocol;
	699	+	mod sanitize;
673	700
674		-	let self_uid = self_uid();
675		-	if let Some(uid) = self_uid.filter(\|uid\| uids.contains(uid)) {
676		-	return username_of(uid);
677		-	}
678		-	uids.first().copied().and_then(username_of)
679		-	}
	701	+	// Nothing outside this file names the children's items, so there are no
	702	+	// re-exports: the three are leaves that reference nothing above them.
680	703
681		-	/// This process's real uid, from `/proc/self/status`.
682		-	///
683		-	/// Read rather than asked of libc, which the console does not link. The `Uid:`
684		-	/// line is four values — real, effective, saved, filesystem — and the first is
685		-	/// the one that answers "who is sitting here".
686		-	fn self_uid() -> Option<u32> {
687		-	let status = std::fs::read_to_string("/proc/self/status").ok()?;
688		-	status
689		-	.lines()
690		-	.find_map(\|line\| line.strip_prefix("Uid:"))?
691		-	.split_whitespace()
692		-	.next()?
693		-	.parse()
694		-	.ok()
695		-	}
696		-
697		-	/// Resolve a uid to a login name out of `/etc/passwd`.
698		-	///
699		-	/// Parsed directly rather than through `getent`, for the same reason the uid is
700		-	/// read from `/proc`: this is an authentication path, and a name that decides
701		-	/// whose password is being asked for should not depend on a subprocess, a
702		-	/// `$PATH`, or the console's command log — which would otherwise show a lookup
703		-	/// the user never asked for, in the middle of a prompt.
704		-	///
705		-	/// NSS is the cost, and it is a bounded one. A machine whose users live in LDAP
706		-	/// or in systemd-homed has entries `/etc/passwd` does not carry, and this
707		-	/// returns nothing for them rather than the wrong name. Alloy installs a local
708		-	/// account (`install.rs`), so the case is a machine that has been joined to a
709		-	/// directory since.
710		-	fn username_of(uid: u32) -> Option<String> {
711		-	let passwd = std::fs::read_to_string("/etc/passwd").ok()?;
712		-	username_in(&passwd, uid)
713		-	}
714		-
715		-	/// The `/etc/passwd` lookup itself, split out so it can be tested without
716		-	/// writing to the real one.
717		-	fn username_in(passwd: &str, uid: u32) -> Option<String> {
718		-	passwd.lines().find_map(\|line\| {
719		-	let mut fields = line.split(':');
720		-	let name = fields.next()?;
721		-	let _password = fields.next()?;
722		-	let found: u32 = fields.next()?.parse().ok()?;
723		-	(found == uid).then(\|\| name.to_string())
724		-	})
725		-	}
726		-
727		-	/// Run the helper's conversation to its end.
728		-	///
729		-	/// `ask` is how a question reaches the screen; it returns once the prompt has
730		-	/// been handed over, and the answer comes back through the prompt's own reply
731		-	/// channel. Taking it as a closure is what keeps this function testable against
732		-	/// a scripted helper with no D-Bus and no terminal anywhere near it.
733		-	///
734		-	/// `withdrawn` is polkit's cancellation, shared with every prompt this hands
735		-	/// out. Checked between messages so a conversation whose command has gone away
736		-	/// stops rather than asking the next question in a sequence nobody is waiting
737		-	/// on the end of.
738		-	fn converse(
739		-	helper: &Path,
740		-	user: &str,
741		-	cookie: &str,
742		-	action_id: &str,
743		-	message: &str,
744		-	withdrawn: &Arc<AtomicBool>,
745		-	ask: impl Fn(Prompt) -> Result<()>,
746		-	) -> Result<()> {
747		-	// Wrapped in the guard on the same expression that spawns it, and that is
748		-	// load-bearing rather than style. Everything below this line can leave by
749		-	// `?`, and `std::process::Child` has no `Drop`: a helper let go of that way
750		-	// is neither killed nor reaped. Measured, not reasoned about — see
751		-	// `a_question_nobody_can_receive_still_closes_the_helper`, which failed
752		-	// with one process left behind before this existed.
753		-	let mut child = Helper(
754		-	child_command(helper)
755		-	.arg(user)
756		-	.stdin(Stdio::piped())
757		-	.stdout(Stdio::piped())
758		-	.stderr(Stdio::null())
759		-	.spawn()
760		-	.with_context(\|\| format!("could not run {}", helper.display()))?,
761		-	);
762		-
763		-	let mut stdin = child.0.stdin.take().context("the helper took no stdin")?;
764		-	let stdout = child
765		-	.0
766		-	.stdout
767		-	.take()
768		-	.context("the helper wrote no stdout")?;
769		-	let mut lines = BufReader::new(stdout).lines();
770		-
771		-	// The cookie first, on its own line. It is what polkit gave us and what the
772		-	// helper hands back to prove this conversation is the one polkit asked for.
773		-	//
774		-	// Checked for a newline for exactly the reason the answer is, further down,
775		-	// and it was the half that was not. The helper's protocol is one line per
776		-	// message, so a cookie carrying a newline ends the line early and everything
777		-	// after it is read as the next message — which at that point in the
778		-	// exchange is the response to the first PAM prompt. That is a password
779		-	// supplied by the caller without anyone being asked.
780		-	//
781		-	// Unreachable today and stated anyway, which is this file's habit
782		-	// everywhere else: polkitd generates the cookie, the bus lets nobody else
783		-	// call this interface (see [`Listener`]), and both of those are somebody
784		-	// else's file rather than an invariant this code holds. The same argument
785		-	// the `printable` calls are made under.
786		-	if cookie.contains('\n') {
787		-	bail!("polkit sent a cookie containing a newline");
788		-	}
789		-	writeln!(stdin, "{cookie}").context("the helper closed before the cookie")?;
790		-
791		-	while let Some(line) = lines.next().transpose().context("the helper stopped")? {
792		-	if withdrawn.load(Ordering::Relaxed) {
793		-	bail!("withdrawn");
794		-	}
795		-	match Directive::parse(&line) {
796		-	Some(Directive::Prompt { question, echo }) => {
797		-	let (reply, answers) = sync_channel(0);
798		-	ask(Prompt {
799		-	action_id: printable(action_id),
800		-	// All three strings are polkit's and PAM's rather than
801		-	// this code's, and every one of them is drawn into a
802		-	// ratatui buffer, which writes graphemes into cells as they
803		-	// come. An escape in one would reach the terminal as an
804		-	// escape, and a bidi override would reorder a sentence
805		-	// about what is being authorized. All three sources are
806		-	// root-owned today, so this is the invariant being stated
807		-	// where it is relied on rather than a hole being closed.
808		-	message: printable(message),
809		-	question: printable(&question),
810		-	echo,
811		-	reply,
812		-	withdrawn: Arc::clone(withdrawn),
813		-	})?;
814		-
815		-	// A closed channel is the screen going away mid-prompt, which
816		-	// is a dismissal rather than an answer.
817		-	let answer = answers.recv().unwrap_or(None);
818		-	let Some(answer) = answer else {
819		-	bail!("dismissed");
820		-	};
821		-	// Checked again here, and not only at the top of the loop.
822		-	// The loop's check is taken before the question goes up, so on
823		-	// its own it covers a withdrawal that arrives before anyone is
824		-	// asked and nothing after: a withdrawal landing while the
825		-	// person is typing would be seen by the screen, which takes the
826		-	// modal down, and by nothing here until the next message —
827		-	// and there is no next message, because the answer is written
828		-	// first. So an Enter that beats the screen's poll would put the
829		-	// password on the wire of a conversation polkit has abandoned,
830		-	// which is precisely the failure the flag exists to stop.
831		-	//
832		-	// What is left after this is the window between the load and
833		-	// the write, and it is not closed here on purpose. Closing it
834		-	// would mean the withdrawal and the write taking one lock, and
835		-	// the withdrawal arrives on
836		-	// [`cancel_authentication`](Listener::cancel_authentication),
837		-	// which zbus dispatches on the single executor thread this
838		-	// handler was made `async` to stop blocking. A write to a pipe
839		-	// whose reader is a setuid helper mid-PAM can block, so that
840		-	// lock would hand the console's freeze back for a race whose
841		-	// outcome is already indistinguishable: a withdrawal that lands
842		-	// after the bytes leave is one that lands after the helper has
843		-	// them, and no amount of locking on this side changes that.
844		-	if withdrawn.load(Ordering::Relaxed) {
845		-	bail!("withdrawn");
846		-	}
847		-	// The caller's obligation from [`Prompt::answer`], enforced
848		-	// where it is relied on. A newline inside the answer would end
849		-	// the line early and turn the rest into the next message of a
850		-	// protocol that is deciding whether to authorize something, so
851		-	// an answer carrying one is refused rather than written. The
852		-	// value is not named in the error: it is the password.
853		-	if answer.expose().contains(&b'\n') {
854		-	bail!("an answer cannot contain a newline");
855		-	}
856		-	// Written as bytes rather than through `writeln!`, because the
857		-	// answer is a `Secret` and a `Secret` is bytes: formatting it
858		-	// would mean a `String` copy of the password that nothing
859		-	// scrubs, which is the copy the type exists to avoid.
860		-	stdin
861		-	.write_all(answer.expose())
862		-	.and_then(\|()\| stdin.write_all(b"\n"))
863		-	.context("the helper closed mid-answer")?;
864		-	}
865		-	Some(Directive::Success) => return Ok(()),
866		-	Some(Directive::Failure) => bail!("not authorized"),
867		-	// PAM_ERROR_MSG and PAM_TEXT_INFO carry text for the user, and
868		-	// anything unrecognized is a helper newer than this code. Neither
869		-	// is a reason to abandon a conversation that is still going: the
870		-	// helper says SUCCESS or FAILURE either way, and that is what this
871		-	// waits for.
872		-	None => {}
873		-	}
874		-	}
875		-
876		-	bail!("the helper ended without saying whether it worked")
877		-	}
878		-
879		-	/// The helper, closed out however the conversation ends.
880		-	///
881		-	/// A guard rather than a call at each exit, and the difference was a real leak
882		-	/// rather than a tidiness argument. [`converse`] has six places it can leave by
883		-	/// `?` — stdin, stdout, the cookie write, the read of each line, the write of
884		-	/// the answer, and the `ask` that hands the question to the screen — and
885		-	/// `std::process::Child` implements no `Drop`, so a helper let go of on any of
886		-	/// them is neither killed nor reaped.
887		-	///
888		-	/// The `ask` one is not hypothetical. It is the path [`Agent::drop`]
889		-	/// deliberately creates: closing the prompt receiver turns a parked `send` into
890		-	/// an error so the conversation ends rather than holding the console open. That
891		-	/// case is "the console is quitting", and without this it leaves a setuid helper
892		-	/// inside `pam_authenticate` for a console that no longer exists.
893		-	///
894		-	/// Killing rather than waiting politely, because every path here has already
895		-	/// decided the conversation is over and a helper mid-`pam_authenticate` can sit
896		-	/// for as long as its PAM stack wants. The wait is what stops it becoming a
897		-	/// zombie for the life of the console.
898		-	struct Helper(Child);
899		-
900		-	impl Drop for Helper {
901		-	fn drop(&mut self) {
902		-	let _ = self.0.kill();
903		-	let _ = self.0.wait();
904		-	}
905		-	}
906		-
907		-	/// Strip control and format characters out of text this code did not write.
908		-	///
909		-	/// polkit's action, its message and PAM's prompt are drawn as-is into the modal
910		-	/// and into the log pane, and ratatui writes what it is given into terminal
911		-	/// cells: an ESC in any of them would reach the terminal as the start of an
912		-	/// escape sequence, which is a screen doing whatever the string said rather
913		-	/// than showing it. All three come from root-owned code today and this is
914		-	/// defence in depth, which is the argument for stripping rather than escaping:
915		-	/// there is nothing here worth rendering visibly, and a modal is one line of
916		-	/// prose plus a question.
917		-	///
918		-	/// `char::is_control` rather than the C0 range alone, so DEL and the C1 set go
919		-	/// too. Those are also introducers on a terminal that reads 8-bit controls, and
920		-	/// none of them is text a prompt wants.
921		-	///
922		-	/// [`is_format`] is the half `is_control` does not cover, and on an
923		-	/// authentication surface it is the more interesting one. A stray ESC garbles a
924		-	/// screen; U+202E reverses the rest of the line, so a message can be made to
925		-	/// read as a sentence about one action while naming another, and the
926		-	/// zero-width set can hide the difference between two names that render
927		-	/// identically.
928		-	///
929		-	/// Not the whole of `Cf`, though, and that limit is deliberate. polkit's
930		-	/// messages are localized through gettext under the caller's `$LANG`, so some
931		-	/// of the category is ordinary orthography: U+200C and U+200D are required to
932		-	/// spell Persian and Urdu and the Indic scripts, and U+061C, U+200E and U+200F
933		-	/// are how a correct mixed-direction Arabic sentence pins the direction of a
934		-	/// number or a Latin word inside it. Stripping those mangles legitimate text in
935		-	/// exactly the locales least able to report it. They are left in, because none
936		-	/// of them can do what the attack needs: each orders the characters beside it
937		-	/// and none opens a state that runs to the end of the string. What is filtered
938		-	/// is the half that does, the embeddings and overrides and isolates, plus the
939		-	/// genuinely invisible.
940		-	///
941		-	/// Provisional, and deliberately not researched further. The line above is read
942		-	/// off the Bidi Algorithm's own distinction between explicit
943		-	/// formatting and implicit marks; nobody who reads these scripts has looked at
944		-	/// it, and no Alloy user has been observed running a locale where it matters.
945		-	/// Before spending more here, ask someone who reads the language and find out
946		-	/// whether anyone needs it. Widening this back to the whole of `Cf` is a
947		-	/// defensible call and costs one range.
948		-	fn printable(text: &str) -> String {
949		-	text.chars()
950		-	.filter(\|c\| !c.is_control() && !is_format(*c))
951		-	.collect()
952		-	}
953		-
954		-	/// The invisible and reordering characters, Unicode's `Cf` category.
955		-	///
956		-	/// Listed rather than derived: `char` answers `is_control`, `is_alphabetic` and
957		-	/// the rest out of std's tables, and the general category is not among what it
958		-	/// exposes, so the alternative is a Unicode-table dependency for one filter on
959		-	/// three short strings. The list is the part of `Cf` that a terminal can be
960		-	/// asked to act on and that no language needs — the bidi controls, the zero-width joiners and spaces, the
961		-	/// invisible operators, the interlinear annotation marks, and the tag block —
962		-	/// and it is written as ranges in code-point order so a reader can check it
963		-	/// against the Unicode chart rather than against this comment.
964		-	///
965		-	/// What it is not is a claim to have made the strings safe. A homoglyph is an
966		-	/// ordinary letter and stays; so is any script this filter does not know it is
967		-	/// looking at. This closes the class where the rendered text and the bytes
968		-	/// disagree without a single visible character to say so.
969		-	fn is_format(c: char) -> bool {
970		-	matches!(
971		-	c,
972		-	'\u{00ad}' // soft hyphen
973		-	\| '\u{0600}'..='\u{0605}' // arabic number-sign prefixes
974		-	\| '\u{06dd}' // arabic end of ayah
975		-	\| '\u{070f}' // syriac abbreviation mark
976		-	\| '\u{180e}' // mongolian vowel separator
977		-	\| '\u{200b}' // zero width space
978		-	// U+200c ZWNJ, U+200d ZWJ, U+200e LRM and U+200f RLM are skipped on
979		-	// purpose: see `printable`. They are spelling, not an attack.
980		-	\| '\u{202a}'..='\u{202e}' // the bidi embeddings and overrides
981		-	\| '\u{2060}'..='\u{2064}' // word joiner and the invisible operators
982		-	\| '\u{2066}'..='\u{206f}' // the bidi isolates and the deprecated formats
983		-	\| '\u{feff}' // zero width no-break space, the BOM
984		-	\| '\u{fff9}'..='\u{fffb}' // interlinear annotation
985		-	\| '\u{110bd}' \| '\u{110cd}' // kaithi number signs
986		-	\| '\u{13430}'..='\u{1343f}' // egyptian hieroglyph format controls
987		-	\| '\u{1bca0}'..='\u{1bca3}' // shorthand format controls
988		-	\| '\u{1d173}'..='\u{1d17a}' // musical beam and phrase controls
989		-	\| '\u{e0001}' // language tag
990		-	\| '\u{e0020}'..='\u{e007f}' // the tag block
991		-	)
992		-	}
993		-
994		-	/// One line of the helper's protocol.
995		-	#[derive(Debug, PartialEq, Eq)]
996		-	enum Directive {
997		-	Prompt { question: String, echo: bool },
998		-	Success,
999		-	Failure,
1000		-	}
1001		-
1002		-	impl Directive {
1003		-	fn parse(line: &str) -> Option<Self> {
1004		-	// Trailing whitespace is significant in the other direction: the prompt
1005		-	// is usually `Password: ` and the trailing space is part of what a GUI
1006		-	// agent would draw. It is trimmed here because the console draws its
1007		-	// own label and the space would land in the middle of a rendered line.
1008		-	let (verb, rest) = line.split_once(' ').unwrap_or((line, ""));
1009		-	match verb {
1010		-	"PAM_PROMPT_ECHO_OFF" => Some(Directive::Prompt {
1011		-	question: rest.trim_end().to_string(),
1012		-	echo: false,
1013		-	}),
1014		-	"PAM_PROMPT_ECHO_ON" => Some(Directive::Prompt {
1015		-	question: rest.trim_end().to_string(),
1016		-	echo: true,
1017		-	}),
1018		-	"SUCCESS" => Some(Directive::Success),
1019		-	"FAILURE" => Some(Directive::Failure),
1020		-	_ => None,
1021		-	}
1022		-	}
1023		-	}
	704	+	#[cfg(test)]
	705	+	mod fixtures;
1024	706
1025	707		#[cfg(test)]

Lines truncated

M crates/alloy/src/polkit/tests.rs +6 -483

			@@ -1,10 +1,14 @@
1	1		//! Tests for [`super`].
2	2
3		-	use std::os::unix::fs::PermissionsExt;
4		-	use std::process::Command;
5	3		use std::time::Duration;
6	4
7	5		use super::*;
	6	+	use std::process::Command;
	7	+
	8	+	use zbus::zvariant::Value;
	9	+
	10	+	use super::fixtures::scripted_helper;
	11	+	use super::identity::{self_uid, username_of};
8	12
9	13		// Captured from a real /proc/self/stat, with the command name replaced by
10	14		// one that is hostile in the two ways a comm field can be: it contains
			@@ -24,602 +28,6 @@
24	28		assert_eq!(start_time(""), None);
25	29		}
26	30
27		-	#[test]
28		-	fn the_helper_protocol_is_read_exactly() {
29		-	assert_eq!(
30		-	Directive::parse("PAM_PROMPT_ECHO_OFF Password: "),
31		-	Some(Directive::Prompt {
32		-	question: "Password:".into(),
33		-	echo: false,
34		-	}),
35		-	);
36		-	assert_eq!(
37		-	Directive::parse("PAM_PROMPT_ECHO_ON One-time code: "),
38		-	Some(Directive::Prompt {
39		-	question: "One-time code:".into(),
40		-	echo: true,
41		-	}),
42		-	);
43		-	assert_eq!(Directive::parse("SUCCESS"), Some(Directive::Success));
44		-	assert_eq!(Directive::parse("FAILURE"), Some(Directive::Failure));
45		-	}
46		-
47		-	// Everything else is text for the user or a helper newer than this code,
48		-	// and neither ends a conversation that is still going.
49		-	#[test]
50		-	fn unknown_lines_are_ignored_rather_than_fatal() {
51		-	assert_eq!(Directive::parse("PAM_TEXT_INFO Insert your key"), None);
52		-	assert_eq!(Directive::parse("PAM_ERROR_MSG Try again"), None);
53		-	assert_eq!(Directive::parse(""), None);
54		-	}
55		-
56		-	const PASSWD: &str = "root:x:0:0:root:/root:/bin/bash\n\
57		-	max:x:1000:1000:Max:/home/max:/bin/bash\n\
58		-	polkitd:x:996:993::/:/usr/sbin/nologin\n";
59		-
60		-	#[test]
61		-	fn a_uid_resolves_to_the_name_beside_it() {
62		-	assert_eq!(username_in(PASSWD, 1000).as_deref(), Some("max"));
63		-	assert_eq!(username_in(PASSWD, 0).as_deref(), Some("root"));
64		-	assert_eq!(username_in(PASSWD, 4242), None);
65		-	}
66		-
67		-	fn identity(kind: &str, key: &str, value: u32) -> (String, HashMap<String, OwnedValue>) {
68		-	let mut details = HashMap::new();
69		-	details.insert(
70		-	key.to_string(),
71		-	OwnedValue::try_from(Value::from(value)).expect("u32"),
72		-	);
73		-	(kind.to_string(), details)
74		-	}
75		-
76		-	// The ordering that matters: polkit offers every administrator, and a
77		-	// console that asked for the first one would teach a laptop owner to type
78		-	// the root password into whatever is on screen.
79		-	#[test]
80		-	fn the_users_own_identity_is_preferred_over_root() {
81		-	let uid = self_uid().expect("this process has a uid");
82		-	let identities = vec![
83		-	identity("unix-user", "uid", 0),
84		-	identity("unix-user", "uid", uid),
85		-	];
86		-	let chosen = choose_identity(&identities);
87		-	assert_eq!(chosen, username_of(uid), "{chosen:?}");
88		-	}
89		-
90		-	// A group is not something the helper can be asked about, and expanding one
91		-	// would mean picking an administrator for the user.
92		-	#[test]
93		-	fn group_identities_are_not_asked_for() {
94		-	let identities = vec![identity("unix-group", "gid", 10)];
95		-	assert_eq!(choose_identity(&identities), None);
96		-	}
97		-
98		-	#[test]
99		-	fn an_empty_identity_list_asks_nobody() {
100		-	assert_eq!(choose_identity(&[]), None);
101		-	}
102		-
103		-	// ---- the helper conversation, against a scripted helper ----
104		-
105		-	/// Held for as long as a scripted helper exists, so no two of these tests
106		-	/// have a script open for writing while another is executing one.
107		-	///
108		-	/// Not tidiness and not a fixture: without it these tests fail together
109		-	/// about one run in eight, with `converse` reporting that it could not run
110		-	/// the helper. The cause is `ETXTBSY` and it is a race between tests rather
111		-	/// than inside one. `Command::spawn` forks, and the child holds a copy of
112		-	/// every descriptor the parent had open until it execs; a script another
113		-	/// test is in the middle of writing is therefore open for writing in that
114		-	/// child, and Linux refuses to execute a file any process has open for
115		-	/// writing. The window is microseconds wide and there is nothing to fix in
116		-	/// `converse`, which is doing the ordinary thing.
117		-	///
118		-	/// It closes half of the race and not all of it: the fork that loses can
119		-	/// come from any thread in the test binary. [`retrying`] covers the rest.
120		-	static SCRIPTS: Mutex<()> = Mutex::new(());
121		-
122		-	/// Linux refuses to execute a file that some process holds open for
123		-	/// writing, and this is the errno it says so with.
124		-	const ETXTBSY: i32 = 26;
125		-
126		-	/// Run a scripted-helper conversation, re-attempting while the exec is
127		-	/// refused with `ETXTBSY`.
128		-	///
129		-	/// The [`SCRIPTS`] lock serialises these tests against each other and
130		-	/// cannot cover this on its own: `Command::spawn` forks, and any thread in
131		-	/// the binary that forks between the script being written and its exec
132		-	/// holds a writable descriptor to that inode. A `cargo test` run that is
133		-	/// also compiling supplies those forks, which is when the failure shows up.
134		-	/// Test-only: production `converse` execs a setuid helper it never wrote,
135		-	/// so it cannot hit this.
136		-	///
137		-	/// A refused exec runs nothing, so a retry repeats no side effect.
138		-	fn retrying(attempt: impl Fn() -> Result<()>) -> Result<()> {
139		-	for wait in [1u64, 2, 5, 10, 25, 50] {
140		-	match attempt() {
141		-	Err(err) if text_file_busy(&err) => {
142		-	std::thread::sleep(Duration::from_millis(wait));
143		-	}
144		-	outcome => return outcome,
145		-	}
146		-	}
147		-	attempt()
148		-	}
149		-
150		-	/// Whether anything in the error chain is `ETXTBSY`.
151		-	fn text_file_busy(err: &anyhow::Error) -> bool {
152		-	err.chain().any(\|cause\| {
153		-	cause
154		-	.downcast_ref::<std::io::Error>()
155		-	.and_then(std::io::Error::raw_os_error)
156		-	== Some(ETXTBSY)
157		-	})
158		-	}
159		-
160		-	/// A stand-in for `polkit-agent-helper-1`: the same line protocol, written
161		-	/// out as a shell script so a conversation can be tested end to end with no
162		-	/// D-Bus, no polkit, and no setuid binary anywhere near it. This is what
163		-	/// [`converse`] taking `ask` as a closure was for.
164		-	///
165		-	/// A guard rather than a path, for two reasons. It carries the [`SCRIPTS`]
166		-	/// lock, which is what the exec race above needs. And it removes what it
167		-	/// wrote however the test ends, where a `remove_file` on the last line of
168		-	/// each test leaves the file behind on every failure.
169		-	struct ScriptedHelper {
170		-	path: PathBuf,
171		-	serialised: Option<std::sync::MutexGuard<'static, ()>>,
172		-	}
173		-
174		-	impl ScriptedHelper {
175		-	/// Named per test as well as locked, so a leftover from an earlier run
176		-	/// is never the file a test is reading.
177		-	fn new(name: &str, script: &str) -> Self {
178		-	let serialised = SCRIPTS.lock().unwrap_or_else(PoisonError::into_inner);
179		-	let path = std::env::temp_dir().join(format!(
180		-	"alloy-polkit-{name}-{}-{:?}",
181		-	std::process::id(),
182		-	std::thread::current().id(),
183		-	));
184		-	std::fs::write(&path, script).expect("the script is written");
185		-	std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o700))
186		-	.expect("the script is executable");
187		-	Self {
188		-	path,
189		-	serialised: Some(serialised),
190		-	}
191		-	}
192		-
193		-	fn path(&self) -> &Path {
194		-	&self.path
195		-	}
196		-
197		-	/// Where a script that records what it was told writes it.
198		-	fn sidecar(&self) -> PathBuf {
199		-	PathBuf::from(format!("{}.seen", self.path.display()))
200		-	}
201		-	}
202		-
203		-	impl Drop for ScriptedHelper {
204		-	fn drop(&mut self) {
205		-	let _ = std::fs::remove_file(&self.path);
206		-	let _ = std::fs::remove_file(self.sidecar());
207		-	// Explicit and last: the lock is what keeps another test from
208		-	// writing a script while this one's is still on disk to be run.
209		-	drop(self.serialised.take());
210		-	}
211		-	}
212		-
213		-	fn scripted_helper(name: &str, script: &str) -> ScriptedHelper {
214		-	ScriptedHelper::new(name, script)
215		-	}
216		-
217		-	/// Reads the cookie, asks once, and says whether the answer was the one it
218		-	/// wanted. `read -r` is the same one-line-at-a-time protocol the real
219		-	/// helper speaks.
220		-	const ASKS_ONCE: &str = "#!/bin/sh\n\
221		-	read -r cookie\n\
222		-	printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
223		-	read -r answer\n\
224		-	if [ \"$answer\" = letmein ]; then printf 'SUCCESS\\n'; else printf 'FAILURE\\n'; fi\n";
225		-
226		-	/// Answer every prompt with `answer`, recording the questions asked.
227		-	///
228		-	/// The answer goes back from a thread of its own because the reply channel
229		-	/// is a rendezvous: [`converse`] hands the prompt over and only then waits
230		-	/// on it, so answering inline would be a send nobody has reached the
231		-	/// receive for. On the real path the answering thread is the one drawing
232		-	/// the screen.
233		-	fn answering(
234		-	answer: &'static str,
235		-	asked: &Arc<Mutex<Vec<String>>>,
236		-	) -> impl Fn(Prompt) -> Result<()> {
237		-	let asked = Arc::clone(asked);
238		-	move \|prompt\| {
239		-	asked
240		-	.lock()
241		-	.expect("not poisoned")
242		-	.push(prompt.question.clone());
243		-	std::thread::spawn(move \|\| prompt.answer(Secret::new(answer)));
244		-	Ok(())
245		-	}
246		-	}
247		-
248		-	#[test]
249		-	fn the_typed_answer_reaches_the_helper_as_one_line() {
250		-	let helper = scripted_helper("accepted", ASKS_ONCE);
251		-	let asked = Arc::new(Mutex::new(Vec::new()));
252		-	let ask = answering("letmein", &asked);
253		-	let outcome = retrying(\|\| {
254		-	converse(
255		-	helper.path(),
256		-	"someone",
257		-	"cookie",
258		-	"an.action",
259		-	"polkit's sentence",
260		-	&Arc::new(AtomicBool::new(false)),
261		-	&ask,
262		-	)
263		-	});
264		-	assert!(outcome.is_ok(), "{outcome:?}");
265		-	assert_eq!(
266		-	asked.lock().expect("not poisoned").as_slice(),
267		-	["Password:"]
268		-	);
269		-	}
270		-
271		-	#[test]
272		-	fn a_wrong_answer_is_the_helpers_verdict_and_not_an_error_here() {
273		-	let helper = scripted_helper("refused", ASKS_ONCE);
274		-	let asked = Arc::new(Mutex::new(Vec::new()));
275		-	let ask = answering("guess", &asked);
276		-	let outcome = retrying(\|\| {
277		-	converse(
278		-	helper.path(),
279		-	"someone",
280		-	"cookie",
281		-	"an.action",
282		-	"polkit's sentence",
283		-	&Arc::new(AtomicBool::new(false)),
284		-	&ask,
285		-	)
286		-	});
287		-	assert_eq!(outcome.unwrap_err().to_string(), "not authorized");
288		-	}
289		-
290		-	// The invariant [`Prompt::answer`] states, enforced where it is relied on.
291		-	// A newline would end the line early and leave the rest to be read as the
292		-	// next message of a protocol deciding whether to authorize something.
293		-	#[test]
294		-	fn an_answer_carrying_a_newline_is_refused_rather_than_written() {
295		-	let helper = scripted_helper("newline", ASKS_ONCE);
296		-	let asked = Arc::new(Mutex::new(Vec::new()));
297		-	let ask = answering("letmein\nSUCCESS", &asked);
298		-	let outcome = retrying(\|\| {
299		-	converse(
300		-	helper.path(),
301		-	"someone",
302		-	"cookie",
303		-	"an.action",
304		-	"polkit's sentence",
305		-	&Arc::new(AtomicBool::new(false)),
306		-	&ask,
307		-	)
308		-	});
309		-	assert_eq!(
310		-	outcome.unwrap_err().to_string(),
311		-	"an answer cannot contain a newline",
312		-	);
313		-	}
314		-
315		-	// polkit cancelled before this conversation got as far as its question, so
316		-	// nothing should be put on the screen at all.
317		-	#[test]
318		-	fn a_withdrawn_conversation_asks_nothing() {
319		-	let helper = scripted_helper("withdrawn", ASKS_ONCE);
320		-	let asked = Arc::new(Mutex::new(Vec::new()));
321		-	let ask = answering("letmein", &asked);
322		-	let outcome = retrying(\|\| {
323		-	converse(
324		-	helper.path(),
325		-	"someone",
326		-	"cookie",
327		-	"an.action",
328		-	"polkit's sentence",
329		-	&Arc::new(AtomicBool::new(true)),
330		-	&ask,
331		-	)
332		-	});
333		-	assert_eq!(outcome.unwrap_err().to_string(), "withdrawn");
334		-	assert!(
335		-	asked.lock().expect("not poisoned").is_empty(),
336		-	"a cancelled conversation puts no question on the screen",
337		-	);
338		-	}
339		-
340		-	// A prompt already on the screen learns about the withdrawal through the
341		-	// flag it carries, which is the only route: the screen owns the prompt and
342		-	// the bus thread cannot reach into it.
343		-	#[test]
344		-	fn a_prompt_reports_the_withdrawal_of_the_conversation_it_belongs_to() {
345		-	let helper = scripted_helper("reports", ASKS_ONCE);
346		-	// The path rather than the guard: the guard holds a `MutexGuard`, which
347		-	// is not `Send`, and the file has to outlive the thread either way.
348		-	let path = helper.path().to_path_buf();
349		-	let withdrawn = Arc::new(AtomicBool::new(false));
350		-	let (seen, prompts) = sync_channel(1);
351		-	let watching = Arc::clone(&withdrawn);
352		-	let conversing = std::thread::spawn(move \|\| {
353		-	let ask = move \|prompt\| seen.send(prompt).map_err(\|_\| anyhow!("nobody listening"));
354		-	retrying(\|\| {
355		-	converse(
356		-	&path,
357		-	"someone",
358		-	"cookie",
359		-	"an.action",
360		-	"polkit's sentence",
361		-	&watching,
362		-	&ask,
363		-	)
364		-	})
365		-	});
366		-
367		-	let prompt = prompts
368		-	.recv_timeout(Duration::from_secs(10))
369		-	.expect("the question reaches the screen");
370		-	assert!(!prompt.withdrawn(), "nothing has been cancelled yet");
371		-	withdrawn.store(true, Ordering::Relaxed);
372		-	assert!(prompt.withdrawn(), "the screen can see the cancellation");
373		-
374		-	// What the screen then does: take the modal down, which is a dismissal.
375		-	prompt.dismiss();
376		-	assert_eq!(
377		-	conversing
378		-	.join()
379		-	.expect("the conversation thread")
380		-	.unwrap_err()
381		-	.to_string(),
382		-	"dismissed",
383		-	);
384		-	}
385		-
386		-	// The window between the answer arriving and it being written. The screen
387		-	// takes a withdrawn modal down, but a keypress can beat its next poll, and
388		-	// an answer for a conversation polkit has abandoned must not reach the
389		-	// helper. Nothing else in this file would notice: the top-of-loop check is
390		-	// taken before the question goes up, and there is no next message after an
391		-	// answer is written.
392		-	#[test]
393		-	fn an_answer_arriving_after_a_withdrawal_is_not_written_to_the_helper() {
394		-	// Records what it was told, so "the answer never reached it" is an
395		-	// assertion about the helper rather than about the error text.
396		-	const RECORDS: &str = "#!/bin/sh\n\
397		-	read -r cookie\n\
398		-	printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
399		-	read -r answer\n\
400		-	printf '%s' \"$answer\" > \"$0.seen\"\n\
401		-	printf 'SUCCESS\\n'\n";
402		-
403		-	let helper = scripted_helper("late", RECORDS);
404		-	let seen = helper.sidecar();
405		-	let withdrawn = Arc::new(AtomicBool::new(false));
406		-
407		-	let cancelling = Arc::clone(&withdrawn);
408		-	let ask = move \|prompt: Prompt\| {
409		-	// polkit withdraws while the question is up, and the answer is
410		-	// sent anyway: the keypress and the withdrawal crossed.
411		-	cancelling.store(true, Ordering::Relaxed);
412		-	std::thread::spawn(move \|\| prompt.answer(Secret::new("letmein")));
413		-	Ok(())
414		-	};
415		-	let outcome = retrying(\|\| {
416		-	converse(
417		-	helper.path(),
418		-	"someone",
419		-	"cookie",
420		-	"an.action",
421		-	"polkit's sentence",
422		-	&withdrawn,
423		-	&ask,
424		-	)
425		-	});
426		-
427		-	assert_eq!(outcome.unwrap_err().to_string(), "withdrawn");
428		-	assert!(
429		-	!seen.exists(),
430		-	"the password reached a helper whose conversation was over",
431		-	);
432		-	}
433		-
434		-	// ---- text this code did not write ----
435		-
436		-	// polkit localizes its messages, so the filter has to leave the marks that
437		-	// spell a language. Guarding the carve-out rather than the removal: the
438		-	// stripping tests below pass whether or not these survive, so without this
439		-	// one a later widening back to the whole `Cf` block goes unnoticed until an
440		-	// RTL locale reads a mangled prompt.
441		-	#[test]
442		-	fn the_marks_that_spell_a_language_survive_the_filter() {
443		-	// ZWNJ, without which the Persian is misspelled.
444		-	let persian = "\u{645}\u{6cc}\u{200c}\u{62e}\u{648}\u{627}\u{647}\u{645}";
445		-	assert_eq!(printable(persian), persian);
446		-	// ZWJ, which the Indic scripts need for the same reason.
447		-	let devanagari = "\u{915}\u{94d}\u{200d}\u{937}";
448		-	assert_eq!(printable(devanagari), devanagari);
449		-	// The directional marks that pin a Latin word inside an Arabic sentence.
450		-	for mark in ['\u{061c}', '\u{200e}', '\u{200f}'] {
451		-	assert_eq!(
452		-	printable(&format!("a{mark}b")),
453		-	format!("a{mark}b"),
454		-	"{mark:?} orders its neighbours and cannot run to end of line",
455		-	);
456		-	}
457		-	// The half that does run to end of line still goes.
458		-	for attack in ['\u{202a}', '\u{202e}', '\u{2066}', '\u{2069}'] {
459		-	assert_eq!(
460		-	printable(&format!("a{attack}b")),
461		-	"ab",
462		-	"{attack:?} opens a state the rest of the string is read in",
463		-	);
464		-	}
465		-	}
466		-
467		-	#[test]
468		-	fn control_characters_are_stripped_out_of_borrowed_text() {
469		-	assert_eq!(printable("Password:"), "Password:");
470		-	assert_eq!(
471		-	printable("\u{1b}]0;pwned\u{7}Password:"),
472		-	"]0;pwnedPassword:"
473		-	);
474		-	assert_eq!(printable("two\nlines\ttabbed"), "twolinestabbed");
475		-	assert_eq!(printable("\u{7f}\u{9b}"), "", "DEL and the C1 set go too");
476		-	assert_eq!(
477		-	printable("no\u{202e}drawrofkcab"),
478		-	"nodrawrofkcab",
479		-	"a bidi override cannot reorder a sentence about what is authorized",
480		-	);
481		-	assert_eq!(
482		-	printable("ad\u{200b}min"),
483		-	"admin",
484		-	"and a zero-width space cannot hide the difference between two names",
485		-	);
486		-	assert_eq!(printable("\u{feff}\u{2066}\u{e0041}"), "");
487		-	assert_eq!(
488		-	printable("naïve café"),
489		-	"naïve café",
490		-	"ordinary text is untouched"
491		-	);
492		-	}
493		-
494		-	// The strings polkit and PAM send reach the modal through the prompt, so
495		-	// that is where the stripping has to have happened.
496		-	#[test]
497		-	fn polkits_and_pams_own_strings_reach_the_screen_stripped() {
498		-	const HOSTILE: &str = "#!/bin/sh\n\
499		-	read -r cookie\n\
500		-	printf 'PAM_PROMPT_ECHO_OFF \\033]0;pwned\\007Password: \\n'\n\
501		-	read -r answer\n\
502		-	printf 'SUCCESS\\n'\n";
503		-
504		-	let helper = scripted_helper("hostile", HOSTILE);
505		-	let asked = Arc::new(Mutex::new(Vec::new()));
506		-	let seen = Arc::new(Mutex::new(Vec::new()));
507		-	let messages = Arc::clone(&seen);

Lines truncated

A crates/alloy/src/polkit/fixtures.rs +125

		@@ -1,0 +1,125 @@
1	+	//! A scripted stand-in for `polkit-agent-helper-1`.
2	+	//!
3	+	//! The same line protocol, written out as a shell script, so a conversation
4	+	//! can be driven end to end with no D-Bus, no polkit, and no setuid binary
5	+	//! anywhere near it. Shared, because both the protocol's own tests and the
6	+	//! agent registry's drive one.
7	+
8	+	use std::os::unix::fs::PermissionsExt;
9	+	use std::path::{Path, PathBuf};
10	+	use std::sync::{Mutex, PoisonError};
11	+	use std::time::Duration;
12	+
13	+	use anyhow::Result;
14	+
15	+	/// Held for as long as a scripted helper exists, so no two of these tests
16	+	/// have a script open for writing while another is executing one.
17	+	///
18	+	/// Not tidiness and not a fixture: without it these tests fail together
19	+	/// about one run in eight, with `converse` reporting that it could not run
20	+	/// the helper. The cause is `ETXTBSY` and it is a race between tests rather
21	+	/// than inside one. `Command::spawn` forks, and the child holds a copy of
22	+	/// every descriptor the parent had open until it execs; a script another
23	+	/// test is in the middle of writing is therefore open for writing in that
24	+	/// child, and Linux refuses to execute a file any process has open for
25	+	/// writing. The window is microseconds wide and there is nothing to fix in
26	+	/// `converse`, which is doing the ordinary thing.
27	+	///
28	+	/// It closes half of the race and not all of it: the fork that loses can
29	+	/// come from any thread in the test binary. [`retrying`] covers the rest.
30	+	pub(super) static SCRIPTS: Mutex<()> = Mutex::new(());
31	+
32	+	/// Linux refuses to execute a file that some process holds open for
33	+	/// writing, and this is the errno it says so with.
34	+	pub(super) const ETXTBSY: i32 = 26;
35	+
36	+	/// Run a scripted-helper conversation, re-attempting while the exec is
37	+	/// refused with `ETXTBSY`.
38	+	///
39	+	/// The [`SCRIPTS`] lock serialises these tests against each other and
40	+	/// cannot cover this on its own: `Command::spawn` forks, and any thread in
41	+	/// the binary that forks between the script being written and its exec
42	+	/// holds a writable descriptor to that inode. A `cargo test` run that is
43	+	/// also compiling supplies those forks, which is when the failure shows up.
44	+	/// Test-only: production `converse` execs a setuid helper it never wrote,
45	+	/// so it cannot hit this.
46	+	///
47	+	/// A refused exec runs nothing, so a retry repeats no side effect.
48	+	pub(super) fn retrying(attempt: impl Fn() -> Result<()>) -> Result<()> {
49	+	for wait in [1u64, 2, 5, 10, 25, 50] {
50	+	match attempt() {
51	+	Err(err) if text_file_busy(&err) => {
52	+	std::thread::sleep(Duration::from_millis(wait));
53	+	}
54	+	outcome => return outcome,
55	+	}
56	+	}
57	+	attempt()
58	+	}
59	+
60	+	/// Whether anything in the error chain is `ETXTBSY`.
61	+	pub(super) fn text_file_busy(err: &anyhow::Error) -> bool {
62	+	err.chain().any(\|cause\| {
63	+	cause
64	+	.downcast_ref::<std::io::Error>()
65	+	.and_then(std::io::Error::raw_os_error)
66	+	== Some(ETXTBSY)
67	+	})
68	+	}
69	+
70	+	/// A stand-in for `polkit-agent-helper-1`: the same line protocol, written
71	+	/// out as a shell script so a conversation can be tested end to end with no
72	+	/// D-Bus, no polkit, and no setuid binary anywhere near it. This is what
73	+	/// [`converse`] taking `ask` as a closure was for.
74	+	///
75	+	/// A guard rather than a path, for two reasons. It carries the [`SCRIPTS`]
76	+	/// lock, which is what the exec race above needs. And it removes what it
77	+	/// wrote however the test ends, where a `remove_file` on the last line of
78	+	/// each test leaves the file behind on every failure.
79	+	pub(super) struct ScriptedHelper {
80	+	path: PathBuf,
81	+	serialised: Option<std::sync::MutexGuard<'static, ()>>,
82	+	}
83	+
84	+	impl ScriptedHelper {
85	+	/// Named per test as well as locked, so a leftover from an earlier run
86	+	/// is never the file a test is reading.
87	+	fn new(name: &str, script: &str) -> Self {
88	+	let serialised = SCRIPTS.lock().unwrap_or_else(PoisonError::into_inner);
89	+	let path = std::env::temp_dir().join(format!(
90	+	"alloy-polkit-{name}-{}-{:?}",
91	+	std::process::id(),
92	+	std::thread::current().id(),
93	+	));
94	+	std::fs::write(&path, script).expect("the script is written");
95	+	std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o700))
96	+	.expect("the script is executable");
97	+	Self {
98	+	path,
99	+	serialised: Some(serialised),
100	+	}
101	+	}
102	+
103	+	pub(super) fn path(&self) -> &Path {
104	+	&self.path
105	+	}
106	+
107	+	/// Where a script that records what it was told writes it.
108	+	pub(super) fn sidecar(&self) -> PathBuf {
109	+	PathBuf::from(format!("{}.seen", self.path.display()))
110	+	}
111	+	}
112	+
113	+	impl Drop for ScriptedHelper {
114	+	fn drop(&mut self) {
115	+	let _ = std::fs::remove_file(&self.path);
116	+	let _ = std::fs::remove_file(self.sidecar());
117	+	// Explicit and last: the lock is what keeps another test from
118	+	// writing a script while this one's is still on disk to be run.
119	+	drop(self.serialised.take());
120	+	}
121	+	}
122	+
123	+	pub(super) fn scripted_helper(name: &str, script: &str) -> ScriptedHelper {
124	+	ScriptedHelper::new(name, script)
125	+	}

A crates/alloy/src/polkit/identity.rs +134

		@@ -1,0 +1,134 @@
1	+	//! Which identity to ask for, and who this process is running as.
2	+
3	+	use std::collections::HashMap;
4	+
5	+	use zbus::zvariant::OwnedValue;
6	+
7	+	/// Which of the identities polkit will accept this agent can actually ask.
8	+	///
9	+	/// **This machine's own user first, and that ordering is the security-relevant
10	+	/// part.** polkit sends every identity that would satisfy the action, which on
11	+	/// a `wheel`-administered box is every administrator. Asking for the first
12	+	/// one would mean a console at a laptop routinely prompting for root, teaching
13	+	/// its owner to type the root password into a screen that could have been
14	+	/// anything.
15	+	///
16	+	/// Group identities are skipped. The helper takes a user name, so a group is
17	+	/// not something this can ask for, and expanding one to its members would mean
18	+	/// choosing an administrator on the user's behalf.
19	+	pub(super) fn choose_identity(
20	+	identities: &[(String, HashMap<String, OwnedValue>)],
21	+	) -> Option<String> {
22	+	let uids: Vec<u32> = identities
23	+	.iter()
24	+	.filter(\|(kind, _)\| kind == "unix-user")
25	+	.filter_map(\|(_, details)\| details.get("uid"))
26	+	.filter_map(\|uid\| u32::try_from(uid).ok())
27	+	.collect();
28	+
29	+	let self_uid = self_uid();
30	+	if let Some(uid) = self_uid.filter(\|uid\| uids.contains(uid)) {
31	+	return username_of(uid);
32	+	}
33	+	uids.first().copied().and_then(username_of)
34	+	}
35	+
36	+	/// This process's real uid, from `/proc/self/status`.
37	+	///
38	+	/// Read rather than asked of libc, which the console does not link. The `Uid:`
39	+	/// line is four values — real, effective, saved, filesystem — and the first is
40	+	/// the one that answers "who is sitting here".
41	+	pub(super) fn self_uid() -> Option<u32> {
42	+	let status = std::fs::read_to_string("/proc/self/status").ok()?;
43	+	status
44	+	.lines()
45	+	.find_map(\|line\| line.strip_prefix("Uid:"))?
46	+	.split_whitespace()
47	+	.next()?
48	+	.parse()
49	+	.ok()
50	+	}
51	+
52	+	/// Resolve a uid to a login name out of `/etc/passwd`.
53	+	///
54	+	/// Parsed directly rather than through `getent`, for the same reason the uid is
55	+	/// read from `/proc`: this is an authentication path, and a name that decides
56	+	/// whose password is being asked for should not depend on a subprocess, a
57	+	/// `$PATH`, or the console's command log — which would otherwise show a lookup
58	+	/// the user never asked for, in the middle of a prompt.
59	+	///
60	+	/// NSS is the cost, and it is a bounded one. A machine whose users live in LDAP
61	+	/// or in systemd-homed has entries `/etc/passwd` does not carry, and this
62	+	/// returns nothing for them rather than the wrong name. Alloy installs a local
63	+	/// account (`install.rs`), so the case is a machine that has been joined to a
64	+	/// directory since.
65	+	pub(super) fn username_of(uid: u32) -> Option<String> {
66	+	let passwd = std::fs::read_to_string("/etc/passwd").ok()?;
67	+	username_in(&passwd, uid)
68	+	}
69	+
70	+	/// The `/etc/passwd` lookup itself, split out so it can be tested without
71	+	/// writing to the real one.
72	+	pub(super) fn username_in(passwd: &str, uid: u32) -> Option<String> {
73	+	passwd.lines().find_map(\|line\| {
74	+	let mut fields = line.split(':');
75	+	let name = fields.next()?;
76	+	let _password = fields.next()?;
77	+	let found: u32 = fields.next()?.parse().ok()?;
78	+	(found == uid).then(\|\| name.to_string())
79	+	})
80	+	}
81	+
82	+	#[cfg(test)]
83	+	mod tests {
84	+	use zbus::zvariant::Value;
85	+
86	+	use super::*;
87	+
88	+	const PASSWD: &str = "root:x:0:0:root:/root:/bin/bash\n\
89	+	max:x:1000:1000:Max:/home/max:/bin/bash\n\
90	+	polkitd:x:996:993::/:/usr/sbin/nologin\n";
91	+
92	+	#[test]
93	+	fn a_uid_resolves_to_the_name_beside_it() {
94	+	assert_eq!(username_in(PASSWD, 1000).as_deref(), Some("max"));
95	+	assert_eq!(username_in(PASSWD, 0).as_deref(), Some("root"));
96	+	assert_eq!(username_in(PASSWD, 4242), None);
97	+	}
98	+
99	+	fn identity(kind: &str, key: &str, value: u32) -> (String, HashMap<String, OwnedValue>) {
100	+	let mut details = HashMap::new();
101	+	details.insert(
102	+	key.to_string(),
103	+	OwnedValue::try_from(Value::from(value)).expect("u32"),
104	+	);
105	+	(kind.to_string(), details)
106	+	}
107	+
108	+	// The ordering that matters: polkit offers every administrator, and a
109	+	// console that asked for the first one would teach a laptop owner to type
110	+	// the root password into whatever is on screen.
111	+	#[test]
112	+	fn the_users_own_identity_is_preferred_over_root() {
113	+	let uid = self_uid().expect("this process has a uid");
114	+	let identities = vec![
115	+	identity("unix-user", "uid", 0),
116	+	identity("unix-user", "uid", uid),
117	+	];
118	+	let chosen = choose_identity(&identities);
119	+	assert_eq!(chosen, username_of(uid), "{chosen:?}");
120	+	}
121	+
122	+	// A group is not something the helper can be asked about, and expanding one
123	+	// would mean picking an administrator for the user.
124	+	#[test]
125	+	fn group_identities_are_not_asked_for() {
126	+	let identities = vec![identity("unix-group", "gid", 10)];
127	+	assert_eq!(choose_identity(&identities), None);
128	+	}
129	+
130	+	#[test]
131	+	fn an_empty_identity_list_asks_nobody() {
132	+	assert_eq!(choose_identity(&[]), None);
133	+	}
134	+	}

A crates/alloy/src/polkit/protocol.rs +216

		@@ -1,0 +1,216 @@
1	+	//! The setuid helper's wire protocol.
2	+	//!
3	+	//! Plain text on stdin and stdout, one message per line. Taking `ask` as a
4	+	//! closure is what keeps the whole conversation testable against a scripted
5	+	//! helper, with no D-Bus and no terminal anywhere near it.
6	+
7	+	use std::io::{BufRead, BufReader, Write};
8	+	use std::path::Path;
9	+	use std::process::{Child, Stdio};
10	+	use std::sync::Arc;
11	+	use std::sync::atomic::{AtomicBool, Ordering};
12	+
13	+	use anyhow::{Context, Result, bail};
14	+
15	+	use super::Prompt;
16	+	use crate::cli::child_command;
17	+
18	+	/// Run the helper's conversation to its end.
19	+	///
20	+	/// `ask` is how a question reaches the screen; it returns once the prompt has
21	+	/// been handed over, and the answer comes back through the prompt's own reply
22	+	/// channel. Taking it as a closure is what keeps this function testable against
23	+	/// a scripted helper with no D-Bus and no terminal anywhere near it.
24	+	///
25	+	/// `withdrawn` is polkit's cancellation, shared with every prompt this hands
26	+	/// out. Checked between messages so a conversation whose command has gone away
27	+	/// stops rather than asking the next question in a sequence nobody is waiting
28	+	/// on the end of.
29	+	pub(super) fn converse(
30	+	helper: &Path,
31	+	user: &str,
32	+	cookie: &str,
33	+	action_id: &str,
34	+	message: &str,
35	+	withdrawn: &Arc<AtomicBool>,
36	+	ask: impl Fn(Prompt) -> Result<()>,
37	+	) -> Result<()> {
38	+	// Wrapped in the guard on the same expression that spawns it, and that is
39	+	// load-bearing rather than style. Everything below this line can leave by
40	+	// `?`, and `std::process::Child` has no `Drop`: a helper let go of that way
41	+	// is neither killed nor reaped. Measured, not reasoned about — see
42	+	// `a_question_nobody_can_receive_still_closes_the_helper`, which failed
43	+	// with one process left behind before this existed.
44	+	let mut child = Helper(
45	+	child_command(helper)
46	+	.arg(user)
47	+	.stdin(Stdio::piped())
48	+	.stdout(Stdio::piped())
49	+	.stderr(Stdio::null())
50	+	.spawn()
51	+	.with_context(\|\| format!("could not run {}", helper.display()))?,
52	+	);
53	+
54	+	let mut stdin = child.0.stdin.take().context("the helper took no stdin")?;
55	+	let stdout = child
56	+	.0
57	+	.stdout
58	+	.take()
59	+	.context("the helper wrote no stdout")?;
60	+	let mut lines = BufReader::new(stdout).lines();
61	+
62	+	// The cookie first, on its own line. It is what polkit gave us and what the
63	+	// helper hands back to prove this conversation is the one polkit asked for.
64	+	//
65	+	// Checked for a newline for exactly the reason the answer is, further down,
66	+	// and it was the half that was not. The helper's protocol is one line per
67	+	// message, so a cookie carrying a newline ends the line early and everything
68	+	// after it is read as the next message — which at that point in the
69	+	// exchange is the response to the first PAM prompt. That is a password
70	+	// supplied by the caller without anyone being asked.
71	+	//
72	+	// Unreachable today and stated anyway, which is this file's habit
73	+	// everywhere else: polkitd generates the cookie, the bus lets nobody else
74	+	// call this interface (see [`Listener`]), and both of those are somebody
75	+	// else's file rather than an invariant this code holds. The same argument
76	+	// the `printable` calls are made under.
77	+	if cookie.contains('\n') {
78	+	bail!("polkit sent a cookie containing a newline");
79	+	}
80	+	writeln!(stdin, "{cookie}").context("the helper closed before the cookie")?;
81	+
82	+	while let Some(line) = lines.next().transpose().context("the helper stopped")? {
83	+	if withdrawn.load(Ordering::Relaxed) {
84	+	bail!("withdrawn");
85	+	}
86	+	match Directive::parse(&line) {
87	+	Some(Directive::Prompt { question, echo }) => {
88	+	let (prompt, answers) =
89	+	Prompt::new(action_id, message, &question, echo, Arc::clone(withdrawn));
90	+	ask(prompt)?;
91	+
92	+	// A closed channel is the screen going away mid-prompt, which
93	+	// is a dismissal rather than an answer.
94	+	let answer = answers.recv().unwrap_or(None);
95	+	let Some(answer) = answer else {
96	+	bail!("dismissed");
97	+	};
98	+	// Checked again here, and not only at the top of the loop.
99	+	// The loop's check is taken before the question goes up, so on
100	+	// its own it covers a withdrawal that arrives before anyone is
101	+	// asked and nothing after: a withdrawal landing while the
102	+	// person is typing would be seen by the screen, which takes the
103	+	// modal down, and by nothing here until the next message —
104	+	// and there is no next message, because the answer is written
105	+	// first. So an Enter that beats the screen's poll would put the
106	+	// password on the wire of a conversation polkit has abandoned,
107	+	// which is precisely the failure the flag exists to stop.
108	+	//
109	+	// What is left after this is the window between the load and
110	+	// the write, and it is not closed here on purpose. Closing it
111	+	// would mean the withdrawal and the write taking one lock, and
112	+	// the withdrawal arrives on
113	+	// [`cancel_authentication`](Listener::cancel_authentication),
114	+	// which zbus dispatches on the single executor thread this
115	+	// handler was made `async` to stop blocking. A write to a pipe
116	+	// whose reader is a setuid helper mid-PAM can block, so that
117	+	// lock would hand the console's freeze back for a race whose
118	+	// outcome is already indistinguishable: a withdrawal that lands
119	+	// after the bytes leave is one that lands after the helper has
120	+	// them, and no amount of locking on this side changes that.
121	+	if withdrawn.load(Ordering::Relaxed) {
122	+	bail!("withdrawn");
123	+	}
124	+	// The caller's obligation from [`Prompt::answer`], enforced
125	+	// where it is relied on. A newline inside the answer would end
126	+	// the line early and turn the rest into the next message of a
127	+	// protocol that is deciding whether to authorize something, so
128	+	// an answer carrying one is refused rather than written. The
129	+	// value is not named in the error: it is the password.
130	+	if answer.expose().contains(&b'\n') {
131	+	bail!("an answer cannot contain a newline");
132	+	}
133	+	// Written as bytes rather than through `writeln!`, because the
134	+	// answer is a `Secret` and a `Secret` is bytes: formatting it
135	+	// would mean a `String` copy of the password that nothing
136	+	// scrubs, which is the copy the type exists to avoid.
137	+	stdin
138	+	.write_all(answer.expose())
139	+	.and_then(\|()\| stdin.write_all(b"\n"))
140	+	.context("the helper closed mid-answer")?;
141	+	}
142	+	Some(Directive::Success) => return Ok(()),
143	+	Some(Directive::Failure) => bail!("not authorized"),
144	+	// PAM_ERROR_MSG and PAM_TEXT_INFO carry text for the user, and
145	+	// anything unrecognized is a helper newer than this code. Neither
146	+	// is a reason to abandon a conversation that is still going: the
147	+	// helper says SUCCESS or FAILURE either way, and that is what this
148	+	// waits for.
149	+	None => {}
150	+	}
151	+	}
152	+
153	+	bail!("the helper ended without saying whether it worked")
154	+	}
155	+
156	+	/// The helper, closed out however the conversation ends.
157	+	///
158	+	/// A guard rather than a call at each exit, and the difference was a real leak
159	+	/// rather than a tidiness argument. [`converse`] has six places it can leave by
160	+	/// `?` — stdin, stdout, the cookie write, the read of each line, the write of
161	+	/// the answer, and the `ask` that hands the question to the screen — and
162	+	/// `std::process::Child` implements no `Drop`, so a helper let go of on any of
163	+	/// them is neither killed nor reaped.
164	+	///
165	+	/// The `ask` one is not hypothetical. It is the path [`Agent::drop`]
166	+	/// deliberately creates: closing the prompt receiver turns a parked `send` into
167	+	/// an error so the conversation ends rather than holding the console open. That
168	+	/// case is "the console is quitting", and without this it leaves a setuid helper
169	+	/// inside `pam_authenticate` for a console that no longer exists.
170	+	///
171	+	/// Killing rather than waiting politely, because every path here has already
172	+	/// decided the conversation is over and a helper mid-`pam_authenticate` can sit
173	+	/// for as long as its PAM stack wants. The wait is what stops it becoming a
174	+	/// zombie for the life of the console.
175	+	struct Helper(Child);
176	+
177	+	impl Drop for Helper {
178	+	fn drop(&mut self) {
179	+	let _ = self.0.kill();
180	+	let _ = self.0.wait();
181	+	}
182	+	}
183	+
184	+	/// One line of the helper's protocol.
185	+	#[derive(Debug, PartialEq, Eq)]
186	+	pub(super) enum Directive {
187	+	Prompt { question: String, echo: bool },
188	+	Success,
189	+	Failure,
190	+	}
191	+
192	+	impl Directive {
193	+	fn parse(line: &str) -> Option<Self> {
194	+	// Trailing whitespace is significant in the other direction: the prompt
195	+	// is usually `Password: ` and the trailing space is part of what a GUI
196	+	// agent would draw. It is trimmed here because the console draws its
197	+	// own label and the space would land in the middle of a rendered line.
198	+	let (verb, rest) = line.split_once(' ').unwrap_or((line, ""));
199	+	match verb {
200	+	"PAM_PROMPT_ECHO_OFF" => Some(Directive::Prompt {
201	+	question: rest.trim_end().to_string(),
202	+	echo: false,
203	+	}),
204	+	"PAM_PROMPT_ECHO_ON" => Some(Directive::Prompt {
205	+	question: rest.trim_end().to_string(),
206	+	echo: true,
207	+	}),
208	+	"SUCCESS" => Some(Directive::Success),
209	+	"FAILURE" => Some(Directive::Failure),
210	+	_ => None,
211	+	}
212	+	}
213	+	}
214	+
215	+	#[cfg(test)]
216	+	mod tests;

A crates/alloy/src/polkit/protocol/tests.rs +388

		@@ -1,0 +1,388 @@
1	+	//! Tests for [`super`].
2	+
3	+	use std::sync::Mutex;
4	+	use std::time::Duration;
5	+
6	+	use std::sync::mpsc::sync_channel;
7	+
8	+	use anyhow::anyhow;
9	+
10	+	use crate::cli::Secret;
11	+
12	+	use super::super::fixtures::{retrying, scripted_helper};
13	+	use super::*;
14	+
15	+	#[test]
16	+	fn the_helper_protocol_is_read_exactly() {
17	+	assert_eq!(
18	+	Directive::parse("PAM_PROMPT_ECHO_OFF Password: "),
19	+	Some(Directive::Prompt {
20	+	question: "Password:".into(),
21	+	echo: false,
22	+	}),
23	+	);
24	+	assert_eq!(
25	+	Directive::parse("PAM_PROMPT_ECHO_ON One-time code: "),
26	+	Some(Directive::Prompt {
27	+	question: "One-time code:".into(),
28	+	echo: true,
29	+	}),
30	+	);
31	+	assert_eq!(Directive::parse("SUCCESS"), Some(Directive::Success));
32	+	assert_eq!(Directive::parse("FAILURE"), Some(Directive::Failure));
33	+	}
34	+
35	+	// Everything else is text for the user or a helper newer than this code,
36	+	// and neither ends a conversation that is still going.
37	+	#[test]
38	+	fn unknown_lines_are_ignored_rather_than_fatal() {
39	+	assert_eq!(Directive::parse("PAM_TEXT_INFO Insert your key"), None);
40	+	assert_eq!(Directive::parse("PAM_ERROR_MSG Try again"), None);
41	+	assert_eq!(Directive::parse(""), None);
42	+	}
43	+
44	+	/// Reads the cookie, asks once, and says whether the answer was the one it
45	+	/// wanted. `read -r` is the same one-line-at-a-time protocol the real
46	+	/// helper speaks.
47	+	const ASKS_ONCE: &str = "#!/bin/sh\n\
48	+	read -r cookie\n\
49	+	printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
50	+	read -r answer\n\
51	+	if [ \"$answer\" = letmein ]; then printf 'SUCCESS\\n'; else printf 'FAILURE\\n'; fi\n";
52	+
53	+	/// Answer every prompt with `answer`, recording the questions asked.
54	+	///
55	+	/// The answer goes back from a thread of its own because the reply channel
56	+	/// is a rendezvous: [`converse`] hands the prompt over and only then waits
57	+	/// on it, so answering inline would be a send nobody has reached the
58	+	/// receive for. On the real path the answering thread is the one drawing
59	+	/// the screen.
60	+	fn answering(
61	+	answer: &'static str,
62	+	asked: &Arc<Mutex<Vec<String>>>,
63	+	) -> impl Fn(Prompt) -> Result<()> {
64	+	let asked = Arc::clone(asked);
65	+	move \|prompt\| {
66	+	asked
67	+	.lock()
68	+	.expect("not poisoned")
69	+	.push(prompt.question.clone());
70	+	std::thread::spawn(move \|\| prompt.answer(Secret::new(answer)));
71	+	Ok(())
72	+	}
73	+	}
74	+
75	+	#[test]
76	+	fn the_typed_answer_reaches_the_helper_as_one_line() {
77	+	let helper = scripted_helper("accepted", ASKS_ONCE);
78	+	let asked = Arc::new(Mutex::new(Vec::new()));
79	+	let ask = answering("letmein", &asked);
80	+	let outcome = retrying(\|\| {
81	+	converse(
82	+	helper.path(),
83	+	"someone",
84	+	"cookie",
85	+	"an.action",
86	+	"polkit's sentence",
87	+	&Arc::new(AtomicBool::new(false)),
88	+	&ask,
89	+	)
90	+	});
91	+	assert!(outcome.is_ok(), "{outcome:?}");
92	+	assert_eq!(
93	+	asked.lock().expect("not poisoned").as_slice(),
94	+	["Password:"]
95	+	);
96	+	}
97	+
98	+	#[test]
99	+	fn a_wrong_answer_is_the_helpers_verdict_and_not_an_error_here() {
100	+	let helper = scripted_helper("refused", ASKS_ONCE);
101	+	let asked = Arc::new(Mutex::new(Vec::new()));
102	+	let ask = answering("guess", &asked);
103	+	let outcome = retrying(\|\| {
104	+	converse(
105	+	helper.path(),
106	+	"someone",
107	+	"cookie",
108	+	"an.action",
109	+	"polkit's sentence",
110	+	&Arc::new(AtomicBool::new(false)),
111	+	&ask,
112	+	)
113	+	});
114	+	assert_eq!(outcome.unwrap_err().to_string(), "not authorized");
115	+	}
116	+
117	+	// The invariant [`Prompt::answer`] states, enforced where it is relied on.
118	+	// A newline would end the line early and leave the rest to be read as the
119	+	// next message of a protocol deciding whether to authorize something.
120	+	#[test]
121	+	fn an_answer_carrying_a_newline_is_refused_rather_than_written() {
122	+	let helper = scripted_helper("newline", ASKS_ONCE);
123	+	let asked = Arc::new(Mutex::new(Vec::new()));
124	+	let ask = answering("letmein\nSUCCESS", &asked);
125	+	let outcome = retrying(\|\| {
126	+	converse(
127	+	helper.path(),
128	+	"someone",
129	+	"cookie",
130	+	"an.action",
131	+	"polkit's sentence",
132	+	&Arc::new(AtomicBool::new(false)),
133	+	&ask,
134	+	)
135	+	});
136	+	assert_eq!(
137	+	outcome.unwrap_err().to_string(),
138	+	"an answer cannot contain a newline",
139	+	);
140	+	}
141	+
142	+	// polkit cancelled before this conversation got as far as its question, so
143	+	// nothing should be put on the screen at all.
144	+	#[test]
145	+	fn a_withdrawn_conversation_asks_nothing() {
146	+	let helper = scripted_helper("withdrawn", ASKS_ONCE);
147	+	let asked = Arc::new(Mutex::new(Vec::new()));
148	+	let ask = answering("letmein", &asked);
149	+	let outcome = retrying(\|\| {
150	+	converse(
151	+	helper.path(),
152	+	"someone",
153	+	"cookie",
154	+	"an.action",
155	+	"polkit's sentence",
156	+	&Arc::new(AtomicBool::new(true)),
157	+	&ask,
158	+	)
159	+	});
160	+	assert_eq!(outcome.unwrap_err().to_string(), "withdrawn");
161	+	assert!(
162	+	asked.lock().expect("not poisoned").is_empty(),
163	+	"a cancelled conversation puts no question on the screen",
164	+	);
165	+	}
166	+
167	+	// A prompt already on the screen learns about the withdrawal through the
168	+	// flag it carries, which is the only route: the screen owns the prompt and
169	+	// the bus thread cannot reach into it.
170	+	#[test]
171	+	fn a_prompt_reports_the_withdrawal_of_the_conversation_it_belongs_to() {
172	+	let helper = scripted_helper("reports", ASKS_ONCE);
173	+	// The path rather than the guard: the guard holds a `MutexGuard`, which
174	+	// is not `Send`, and the file has to outlive the thread either way.
175	+	let path = helper.path().to_path_buf();
176	+	let withdrawn = Arc::new(AtomicBool::new(false));
177	+	let (seen, prompts) = sync_channel(1);
178	+	let watching = Arc::clone(&withdrawn);
179	+	let conversing = std::thread::spawn(move \|\| {
180	+	let ask = move \|prompt\| seen.send(prompt).map_err(\|_\| anyhow!("nobody listening"));
181	+	retrying(\|\| {
182	+	converse(
183	+	&path,
184	+	"someone",
185	+	"cookie",
186	+	"an.action",
187	+	"polkit's sentence",
188	+	&watching,
189	+	&ask,
190	+	)
191	+	})
192	+	});
193	+
194	+	let prompt = prompts
195	+	.recv_timeout(Duration::from_secs(10))
196	+	.expect("the question reaches the screen");
197	+	assert!(!prompt.withdrawn(), "nothing has been cancelled yet");
198	+	withdrawn.store(true, Ordering::Relaxed);
199	+	assert!(prompt.withdrawn(), "the screen can see the cancellation");
200	+
201	+	// What the screen then does: take the modal down, which is a dismissal.
202	+	prompt.dismiss();
203	+	assert_eq!(
204	+	conversing
205	+	.join()
206	+	.expect("the conversation thread")
207	+	.unwrap_err()
208	+	.to_string(),
209	+	"dismissed",
210	+	);
211	+	}
212	+
213	+	// The window between the answer arriving and it being written. The screen
214	+	// takes a withdrawn modal down, but a keypress can beat its next poll, and
215	+	// an answer for a conversation polkit has abandoned must not reach the
216	+	// helper. Nothing else in this file would notice: the top-of-loop check is
217	+	// taken before the question goes up, and there is no next message after an
218	+	// answer is written.
219	+	#[test]
220	+	fn an_answer_arriving_after_a_withdrawal_is_not_written_to_the_helper() {
221	+	// Records what it was told, so "the answer never reached it" is an
222	+	// assertion about the helper rather than about the error text.
223	+	const RECORDS: &str = "#!/bin/sh\n\
224	+	read -r cookie\n\
225	+	printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
226	+	read -r answer\n\
227	+	printf '%s' \"$answer\" > \"$0.seen\"\n\
228	+	printf 'SUCCESS\\n'\n";
229	+
230	+	let helper = scripted_helper("late", RECORDS);
231	+	let seen = helper.sidecar();
232	+	let withdrawn = Arc::new(AtomicBool::new(false));
233	+
234	+	let cancelling = Arc::clone(&withdrawn);
235	+	let ask = move \|prompt: Prompt\| {
236	+	// polkit withdraws while the question is up, and the answer is
237	+	// sent anyway: the keypress and the withdrawal crossed.
238	+	cancelling.store(true, Ordering::Relaxed);
239	+	std::thread::spawn(move \|\| prompt.answer(Secret::new("letmein")));
240	+	Ok(())
241	+	};
242	+	let outcome = retrying(\|\| {
243	+	converse(
244	+	helper.path(),
245	+	"someone",
246	+	"cookie",
247	+	"an.action",
248	+	"polkit's sentence",
249	+	&withdrawn,
250	+	&ask,
251	+	)
252	+	});
253	+
254	+	assert_eq!(outcome.unwrap_err().to_string(), "withdrawn");
255	+	assert!(
256	+	!seen.exists(),
257	+	"the password reached a helper whose conversation was over",
258	+	);
259	+	}
260	+
261	+	// The strings polkit and PAM send reach the modal through the prompt, so
262	+	// that is where the stripping has to have happened.
263	+	#[test]
264	+	fn polkits_and_pams_own_strings_reach_the_screen_stripped() {
265	+	const HOSTILE: &str = "#!/bin/sh\n\
266	+	read -r cookie\n\
267	+	printf 'PAM_PROMPT_ECHO_OFF \\033]0;pwned\\007Password: \\n'\n\
268	+	read -r answer\n\
269	+	printf 'SUCCESS\\n'\n";
270	+
271	+	let helper = scripted_helper("hostile", HOSTILE);
272	+	let asked = Arc::new(Mutex::new(Vec::new()));
273	+	let seen = Arc::new(Mutex::new(Vec::new()));
274	+	let messages = Arc::clone(&seen);
275	+	let ask = {
276	+	let answer = answering("letmein", &asked);
277	+	move \|prompt: Prompt\| {
278	+	messages
279	+	.lock()
280	+	.expect("not poisoned")
281	+	.push(format!("{}\|{}", prompt.action_id, prompt.message));
282	+	answer(prompt)
283	+	}
284	+	};
285	+	let outcome = retrying(\|\| {
286	+	converse(
287	+	helper.path(),
288	+	"someone",
289	+	"cookie",
290	+	// A bidi override in the one string that was not being stripped.
291	+	// The action is what the log pane names twice, and reversing the
292	+	// rest of a line is how a name reads as one action and is another.
293	+	"an.\u{202e}action",
294	+	"polkit's \u{1b}[2Jsentence",
295	+	&Arc::new(AtomicBool::new(false)),
296	+	&ask,
297	+	)
298	+	});
299	+	assert!(outcome.is_ok(), "{outcome:?}");
300	+	assert_eq!(
301	+	asked.lock().expect("not poisoned").as_slice(),
302	+	["]0;pwnedPassword:"]
303	+	);
304	+	assert_eq!(
305	+	seen.lock().expect("not poisoned").as_slice(),
306	+	["an.action\|polkit's [2Jsentence"]
307	+	);
308	+	}
309	+
310	+	// A conversation whose question cannot be delivered still has to close the
311	+	// helper out. This is not a hypothetical path: it is the one
312	+	// [`Agent::drop`] deliberately creates, by closing the prompt receiver so a
313	+	// parked `send` fails rather than holding the console open. The helper is
314	+	// setuid and sits inside `pam_authenticate`, which `finish`'s own doc notes
315	+	// "can sit for as long as its PAM stack wants", so leaving it is a live
316	+	// process holding a PAM conversation for a console that has exited.
317	+	//
318	+	// Uses a script that would outlive the conversation on its own, so the
319	+	// assertion is about this code closing it rather than about the child
320	+	// happening to end.
321	+	#[test]
322	+	fn a_question_nobody_can_receive_still_closes_the_helper() {
323	+	// Writes its own pid before it prompts, so the assertion below is
324	+	// about this conversation's helper and not about whatever else the
325	+	// test binary happens to have running. `converse` cannot return until
326	+	// it has read the prompt line, so the file is on disk by then.
327	+	const SLEEPS: &str = "#!/bin/sh\n\
328	+	read -r cookie\n\
329	+	printf '%s\\n' \"$$\" > \"$0.seen\"\n\
330	+	printf 'PAM_PROMPT_ECHO_OFF Password: \\n'\n\
331	+	sleep 30\n\
332	+	printf 'FAILURE\\n'\n";
333	+
334	+	let helper = scripted_helper("undeliverable", SLEEPS);
335	+	let seen = helper.sidecar();
336	+	// A stale file from an earlier run would name a pid this test never
337	+	// spawned, so the read below is of this run or of nothing.
338	+	let _ = std::fs::remove_file(&seen);
339	+
340	+	let ask = // Exactly what `begin_authentication` passes when the receiver has
341	+	// gone: the console stopped listening.
342	+	\|_prompt\| bail!("the console stopped listening");
343	+	let outcome = retrying(\|\| {
344	+	converse(
345	+	helper.path(),
346	+	"someone",
347	+	"cookie",
348	+	"an.action",
349	+	"a message",
350	+	&Arc::new(AtomicBool::new(false)),
351	+	ask,
352	+	)
353	+	});
354	+	assert!(
355	+	outcome.is_err(),
356	+	"the conversation should not have succeeded"
357	+	);
358	+
359	+	// The child is closed out synchronously by `finish`, so by the time
360	+	// converse has returned there is nothing left to wait for.
361	+	let recorded = std::fs::read_to_string(&seen).expect("the helper recorded its pid");
362	+	let pid: u32 = recorded
363	+	.trim()
364	+	.parse()
365	+	.expect("the pid it recorded is a pid");
366	+	assert_ne!(
367	+	parent_of(pid),
368	+	Some(std::process::id()),
369	+	"converse returned leaving helper {pid} behind",
370	+	);
371	+	}
372	+
373	+	/// The parent pid of `pid`, from /proc, or `None` if it is gone.
374	+	///
375	+	/// Reads the process table rather than trusting a handle, because the thing
376	+	/// under test is precisely whether a handle was dropped without being
377	+	/// waited on. A zombie still answers here, which is the point: an unreaped
378	+	/// helper is a leak even once it has stopped running. Answering about one
379	+	/// named pid rather than about every child is what keeps a helper another
380	+	/// test spawned out of this one's verdict, since the whole binary is one
381	+	/// process.
382	+	fn parent_of(pid: u32) -> Option<u32> {
383	+	let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?;
384	+	// Field 4 is the parent pid; same hostile-comm rule as `start_time`, so
385	+	// read past the last `)`.
386	+	let after_comm = stat.rfind(')').map(\|end\| &stat[end + 1..])?;
387	+	after_comm.split_whitespace().nth(1)?.parse().ok()
388	+	}

A crates/alloy/src/polkit/sanitize.rs +153

		@@ -1,0 +1,153 @@
1	+	//! Text this code did not write, made safe to draw.
2	+
3	+	/// Strip control and format characters out of text this code did not write.
4	+	///
5	+	/// polkit's action, its message and PAM's prompt are drawn as-is into the modal
6	+	/// and into the log pane, and ratatui writes what it is given into terminal
7	+	/// cells: an ESC in any of them would reach the terminal as the start of an
8	+	/// escape sequence, which is a screen doing whatever the string said rather
9	+	/// than showing it. All three come from root-owned code today and this is
10	+	/// defence in depth, which is the argument for stripping rather than escaping:
11	+	/// there is nothing here worth rendering visibly, and a modal is one line of
12	+	/// prose plus a question.
13	+	///
14	+	/// `char::is_control` rather than the C0 range alone, so DEL and the C1 set go
15	+	/// too. Those are also introducers on a terminal that reads 8-bit controls, and
16	+	/// none of them is text a prompt wants.
17	+	///
18	+	/// [`is_format`] is the half `is_control` does not cover, and on an
19	+	/// authentication surface it is the more interesting one. A stray ESC garbles a
20	+	/// screen; U+202E reverses the rest of the line, so a message can be made to
21	+	/// read as a sentence about one action while naming another, and the
22	+	/// zero-width set can hide the difference between two names that render
23	+	/// identically.
24	+	///
25	+	/// Not the whole of `Cf`, though, and that limit is deliberate. polkit's
26	+	/// messages are localized through gettext under the caller's `$LANG`, so some
27	+	/// of the category is ordinary orthography: U+200C and U+200D are required to
28	+	/// spell Persian and Urdu and the Indic scripts, and U+061C, U+200E and U+200F
29	+	/// are how a correct mixed-direction Arabic sentence pins the direction of a
30	+	/// number or a Latin word inside it. Stripping those mangles legitimate text in
31	+	/// exactly the locales least able to report it. They are left in, because none
32	+	/// of them can do what the attack needs: each orders the characters beside it
33	+	/// and none opens a state that runs to the end of the string. What is filtered
34	+	/// is the half that does, the embeddings and overrides and isolates, plus the
35	+	/// genuinely invisible.
36	+	///
37	+	/// Provisional, and deliberately not researched further. The line above is read
38	+	/// off the Bidi Algorithm's own distinction between explicit
39	+	/// formatting and implicit marks; nobody who reads these scripts has looked at
40	+	/// it, and no Alloy user has been observed running a locale where it matters.
41	+	/// Before spending more here, ask someone who reads the language and find out
42	+	/// whether anyone needs it. Widening this back to the whole of `Cf` is a
43	+	/// defensible call and costs one range.
44	+	pub(super) fn printable(text: &str) -> String {
45	+	text.chars()
46	+	.filter(\|c\| !c.is_control() && !is_format(*c))
47	+	.collect()
48	+	}
49	+
50	+	/// The invisible and reordering characters, Unicode's `Cf` category.
51	+	///
52	+	/// Listed rather than derived: `char` answers `is_control`, `is_alphabetic` and
53	+	/// the rest out of std's tables, and the general category is not among what it
54	+	/// exposes, so the alternative is a Unicode-table dependency for one filter on
55	+	/// three short strings. The list is the part of `Cf` that a terminal can be
56	+	/// asked to act on and that no language needs — the bidi controls, the zero-width joiners and spaces, the
57	+	/// invisible operators, the interlinear annotation marks, and the tag block —
58	+	/// and it is written as ranges in code-point order so a reader can check it
59	+	/// against the Unicode chart rather than against this comment.
60	+	///
61	+	/// What it is not is a claim to have made the strings safe. A homoglyph is an
62	+	/// ordinary letter and stays; so is any script this filter does not know it is
63	+	/// looking at. This closes the class where the rendered text and the bytes
64	+	/// disagree without a single visible character to say so.
65	+	fn is_format(c: char) -> bool {
66	+	matches!(
67	+	c,
68	+	'\u{00ad}' // soft hyphen
69	+	\| '\u{0600}'..='\u{0605}' // arabic number-sign prefixes
70	+	\| '\u{06dd}' // arabic end of ayah
71	+	\| '\u{070f}' // syriac abbreviation mark
72	+	\| '\u{180e}' // mongolian vowel separator
73	+	\| '\u{200b}' // zero width space
74	+	// U+200c ZWNJ, U+200d ZWJ, U+200e LRM and U+200f RLM are skipped on
75	+	// purpose: see `printable`. They are spelling, not an attack.
76	+	\| '\u{202a}'..='\u{202e}' // the bidi embeddings and overrides
77	+	\| '\u{2060}'..='\u{2064}' // word joiner and the invisible operators
78	+	\| '\u{2066}'..='\u{206f}' // the bidi isolates and the deprecated formats
79	+	\| '\u{feff}' // zero width no-break space, the BOM
80	+	\| '\u{fff9}'..='\u{fffb}' // interlinear annotation
81	+	\| '\u{110bd}' \| '\u{110cd}' // kaithi number signs
82	+	\| '\u{13430}'..='\u{1343f}' // egyptian hieroglyph format controls
83	+	\| '\u{1bca0}'..='\u{1bca3}' // shorthand format controls
84	+	\| '\u{1d173}'..='\u{1d17a}' // musical beam and phrase controls
85	+	\| '\u{e0001}' // language tag
86	+	\| '\u{e0020}'..='\u{e007f}' // the tag block
87	+	)
88	+	}
89	+
90	+	#[cfg(test)]
91	+	mod tests {
92	+	use super::*;
93	+
94	+	// polkit localizes its messages, so the filter has to leave the marks that
95	+	// spell a language. Guarding the carve-out rather than the removal: the
96	+	// stripping tests below pass whether or not these survive, so without this
97	+	// one a later widening back to the whole `Cf` block goes unnoticed until an
98	+	// RTL locale reads a mangled prompt.
99	+	#[test]
100	+	fn the_marks_that_spell_a_language_survive_the_filter() {
101	+	// ZWNJ, without which the Persian is misspelled.
102	+	let persian = "\u{645}\u{6cc}\u{200c}\u{62e}\u{648}\u{627}\u{647}\u{645}";
103	+	assert_eq!(printable(persian), persian);
104	+	// ZWJ, which the Indic scripts need for the same reason.
105	+	let devanagari = "\u{915}\u{94d}\u{200d}\u{937}";
106	+	assert_eq!(printable(devanagari), devanagari);
107	+	// The directional marks that pin a Latin word inside an Arabic sentence.
108	+	for mark in ['\u{061c}', '\u{200e}', '\u{200f}'] {
109	+	assert_eq!(
110	+	printable(&format!("a{mark}b")),
111	+	format!("a{mark}b"),
112	+	"{mark:?} orders its neighbours and cannot run to end of line",
113	+	);
114	+	}
115	+	// The half that does run to end of line still goes.
116	+	for attack in ['\u{202a}', '\u{202e}', '\u{2066}', '\u{2069}'] {
117	+	assert_eq!(
118	+	printable(&format!("a{attack}b")),
119	+	"ab",
120	+	"{attack:?} opens a state the rest of the string is read in",
121	+	);
122	+	}
123	+	}
124	+
125	+	#[test]
126	+	fn control_characters_are_stripped_out_of_borrowed_text() {
127	+	assert_eq!(printable("Password:"), "Password:");
128	+	assert_eq!(
129	+	printable("\u{1b}]0;pwned\u{7}Password:"),
130	+	"]0;pwnedPassword:"
131	+	);
132	+	assert_eq!(printable("two\nlines\ttabbed"), "twolinestabbed");
133	+	assert_eq!(printable("\u{7f}\u{9b}"), "", "DEL and the C1 set go too");
134	+	assert_eq!(
135	+	printable("no\u{202e}drawrofkcab"),
136	+	"nodrawrofkcab",
137	+	"a bidi override cannot reorder a sentence about what is authorized",
138	+	);
139	+	assert_eq!(
140	+	printable("ad\u{200b}min"),
141	+	"admin",
142	+	"and a zero-width space cannot hide the difference between two names",
143	+	);
144	+	assert_eq!(printable("\u{feff}\u{2066}\u{e0041}"), "");
145	+	assert_eq!(
146	+	printable("naïve café"),
147	+	"naïve café",
148	+	"ordinary text is untouched"
149	+	);
150	+	}
151	+
152	+	// The strings polkit and PAM send reach the modal through the prompt, so
153	+	}