Shorten compositor comments and test names

compositor/Cargo.toml

@@ -1,6 +1,4 @@
-# Kryptik compositor layer (docs/roadmap.md, Compositor and GUI isolation).
-# A workspace apart from compartments/kryptikd, so no desktop dependency can
-# reach the privileged daemon's lockfile (ADR-010).
+# Apart from kryptikd's workspace, so no desktop dependency reaches its lockfile (ADR-010).
 [workspace]
 resolver = "2"
 members = ["zoneid", "wlproxy"]

compositor/wlproxy/Cargo.toml

@@ -5,8 +5,7 @@ version.workspace = true
 edition.workspace = true
 license.workspace = true
 
-# Dependency policy (ADR-010): this parses an untrusted zone's bytes, so libc
-# only. The wire format is hand-rolled; Wayland libraries are built to be permissive.
+# libc only (ADR-010): this parses a zone's bytes, and Wayland libraries are built to be permissive.
 [dependencies]
 libc = "0.2"
 

compositor/wlproxy/src/main.rs

@@ -1,12 +1,8 @@
-//! kryptik-wlproxy: the per-zone Wayland filtering proxy.
+//! kryptik-wlproxy: the per-zone Wayland filtering proxy, single-threaded and unprivileged.
 //!
 //!   kryptik-wlproxy --zone NAME --listen PATH --upstream PATH [--max-clients N] [--once]
 //!
-//! Each client on PATH (bound into the zone as /run/kryptik/wayland-0) gets a
-//! Session to the compositor at --upstream (session.rs, policy.rs). A client
-//! that breaks the protocol or reaches for something hidden gets a
-//! wl_display.error and nothing more is forwarded. Single-threaded and
-//! unprivileged; what a client can make it allocate is bounded.
+//! Each client of PATH (the zone's /run/kryptik/wayland-0) gets its own Session to --upstream.
 
 mod policy;
 mod protocol;
@@ -21,9 +17,7 @@ use std::time::{Duration, Instant};
 
 use session::{Dir, Session};
 
-/// The log is a file in /run, which is RAM, and a client can make lines at
-/// will: they are rate-limited and cut, the whole is bounded, and a failed
-/// write is ignored rather than fatal.
+/// Bounded log: the file is in /run, which is RAM, and a client can make lines at will.
 struct Log { zone: String, since: Instant, lines: u32, dropped: u32, left: usize }
 impl Log {
     const PER_SECOND: u32 = 20;
@@ -118,14 +112,13 @@ const FDS_PER_SESSION: usize = 2 + 2 * policy::MAX_PENDING_FDS;
 /// stdio, the listener and room to spare.
 const FDS_RESERVED: usize = 8;
 
-/// Clients whose descriptors fit under `limit`, and never none.
+/// How many clients' descriptors fit under `limit`; at least one.
 fn clients_within(limit: u64, wanted: usize) -> usize {
     let room = usize::try_from(limit).unwrap_or(usize::MAX).saturating_sub(FDS_RESERVED) / FDS_PER_SESSION;
     wanted.min(room).max(1)
 }
 
-/// Raise the descriptor limit to the hard one and fit max_clients under it:
-/// past the limit, accept and recvmsg fail for every client of the zone.
+/// Raise the fd limit to the hard one and fit max_clients under it; past it, accept and recvmsg fail.
 fn fit_descriptors(o: &mut Opts) {
     let mut rl = libc::rlimit { rlim_cur: 0, rlim_max: 0 };
     if unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, &mut rl) } != 0 {
@@ -168,8 +161,7 @@ fn main() {
             std::process::exit(1);
         }
     };
-    /* The zone's uid must be able to connect through the bind mount; the
-     * directory on this side keeps everyone else out. */
+    // 0666 so the zone's uid can connect through the bind mount; the directory keeps others out.
     let _ = std::fs::set_permissions(&o.listen, std::os::unix::fs::PermissionsExt::from_mode(0o666));
     listener.set_nonblocking(true).expect("nonblocking listener");
     eprintln!("kryptik-wlproxy[{}]: listening on {} -> {}", o.zone, o.listen.display(), o.upstream.display());
@@ -178,15 +170,11 @@ fn main() {
     let mut next_id = 1u64;
     let mut served = 0u64;
     let mut log = Log::new(&o.zone);
-    /* After a failed accept the listener rests until this passes: the failed
-     * connection is still pending, so polling again at once would spin (and a
-     * client can exhaust descriptors to cause that). */
+    // A failed accept leaves the connection pending, so the listener rests or poll would spin.
     let mut accept_after = Instant::now();
     let mut fds: Vec<libc::pollfd> = Vec::new();
     loop {
-        /* The poll set snapshots `sessions`: the listener, then session i at
-         * 1+2i and 2+2i. `sessions` must not change while entries are read, so
-         * accepting comes last and the service loop stops at `polled`. */
+        // fds: the listener, then session i at 1+2i and 2+2i; valid until `sessions` changes.
         let polled = sessions.len();
         fds.clear();
         let pause = accept_after.saturating_duration_since(Instant::now());
@@ -211,7 +199,6 @@ fn main() {
             eprintln!("kryptik-wlproxy[{}]: poll: {e}", o.zone);
             std::process::exit(1);
         }
-        // Service the sessions that were polled, against their own entries.
         let mut closed: Vec<usize> = Vec::new();
         let mut zone_pool_bytes: usize = sessions.iter().map(|l| l.s.shm_pool_bytes).sum();
         let mut zone_pool_count: usize = sessions.iter().map(|l| l.s.shm_pool_count).sum();
@@ -273,7 +260,7 @@ fn main() {
                 return;
             }
         }
-        // Accept last, when nothing indexes the snapshot any more.
+        // Accept last, once nothing indexes `fds` by session.
         if fds[0].revents & libc::POLLIN != 0 {
             match listener.accept() {
                 Ok((client, _)) => match UnixStream::connect(&o.upstream) {

compositor/wlproxy/src/policy.rs

@@ -1,13 +1,9 @@
-//! What a zone's client may reach through the proxy, and what it rewrites.
-//!
-//! Globals off the allowlist are never advertised, so they cannot be bound: no
-//! capture, global input, layer shell (nothing may draw over the chrome),
-//! clipboard (the broker is the only cross-zone channel), virtual keyboard,
-//! output management or activation; binding one anyway disconnects the client.
-//! Every toplevel title and app_id comes out carrying the zone's name.
+//! What a zone's client may bind, and how its titles and app_ids carry the zone's name.
+//! Nothing else is advertised: no capture, global input, layer shell (nothing may draw over the
+//! chrome), clipboard (the broker is the only cross-zone channel), virtual keyboard, output
+//! management or activation.
 
-/// Interfaces a zone client may bind, capped at the version the generated
-/// tables know: requests of a newer version could not be parsed here.
+/// Bindable interfaces, capped at the tables' versions: a newer request could not be parsed.
 pub const ALLOWED: &[(&str, u32)] = &[
     ("wl_compositor", 6),
     ("wl_subcompositor", 1),
@@ -26,11 +22,9 @@ pub fn allowed_version(interface: &str) -> Option<u32> {
 /// The most bytes a rewritten title may carry; only one line is ever shown.
 pub const MAX_TITLE_BYTES: usize = 256;
 
-/// Prefix a title with `[zone] `. A claimed `[vault] ` just follows the real
-/// prefix: `[work] [vault] ...`.
+/// Prefix a title with `[zone] `; a claimed `[vault] ` follows the real one: `[work] [vault] ...`.
 pub fn title_for(zone: &str, title: &str) -> String {
-    /* Titles reach dwl's line-based status stream and terminal chrome: no
-     * control may inject records, terminal escapes or bidi overrides. */
+    // Titles reach dwl's status lines and terminal chrome: controls and bidi marks become spaces.
     let title: String = title.chars().map(|c| {
         if c.is_control() || matches!(c, '\u{061c}' | '\u{200e}' | '\u{200f}' | '\u{2028}'..='\u{202e}' | '\u{2066}'..='\u{2069}') { ' ' } else { c }
     }).collect();
@@ -39,8 +33,7 @@ pub fn title_for(zone: &str, title: &str) -> String {
     out
 }
 
-/// Cut `s` to at most `max` bytes, ending in `...` if anything was cut. The
-/// cut lands on a character boundary: `String::truncate` panics inside one.
+/// Cut `s` to `max` bytes with a `...`, on a char boundary: `String::truncate` panics inside one.
 fn bound_utf8(s: &mut String, max: usize) {
     if s.len() <= max {
         return;
@@ -53,8 +46,7 @@ fn bound_utf8(s: &mut String, max: usize) {
     s.push_str("...");
 }
 
-/// `kryptik.<zone>.<claimed>`, the claimed part cut to a safe alphabet so a
-/// zone name inside it cannot pass for the real one.
+/// `kryptik.<zone>.<claimed>`, the claim cut to `[A-Za-z0-9_-]` so it cannot pass for another zone.
 pub fn app_id_for(zone: &str, claimed: &str) -> String {
     let cleaned: String = claimed
         .chars()
@@ -64,10 +56,9 @@ pub fn app_id_for(zone: &str, claimed: &str) -> String {
     format!("kryptik.{zone}.{}", if cleaned.is_empty() { "app".to_string() } else { cleaned })
 }
 
-/// Resource bounds per client connection and across a zone's connections.
+// Resource bounds per client connection and across a zone's connections.
 pub const MAX_OBJECTS: usize = 4096;
-/// Id slots per range. libwayland reuses freed ids, so its slots never outnumber
-/// its peak of live objects; a client that never reuses one stops here.
+/// Id slots per range: libwayland reuses freed ids, so only a client that never does reaches this.
 pub const MAX_ID_SLOTS: usize = 2 * MAX_OBJECTS;
 pub const MAX_PENDING_BYTES: usize = 1 << 20; // per direction
 pub const MAX_PENDING_FDS: usize = 64;

compositor/wlproxy/src/policy/tests.rs

@@ -22,8 +22,7 @@ fn capture_and_clipboard_are_hidden() {
     }
 }
 
-/// Objects are forgotten only on delete_id, which names client-created ids,
-/// so no event a zone can reach may create one.
+/// Only delete_id frees objects, and only client-created ones: no reachable event may create one.
 #[test]
 fn no_compositor_created_objects() {
     use crate::protocol::{find, Arg};
@@ -76,7 +75,7 @@ fn title_controls_are_replaced() {
 
 /// The bound is in bytes; the cut must still land on a character boundary.
 #[test]
-fn multibyte_title_cut_on_char_boundary() {
+fn title_cut_on_char_boundary() {
     // 200 x U+00E9 is 400 bytes; byte 253 is inside a character.
     let t = title_for("vault", &"\u{00e9}".repeat(200));
     assert!(t.len() <= MAX_TITLE_BYTES, "{}", t.len());
@@ -92,7 +91,7 @@ fn multibyte_title_cut_on_char_boundary() {
     assert!(t.ends_with("..."));
     assert!(t.trim_end_matches("...").chars().skip(4).all(|c| c == '\u{1F600}'), "{t:?}");
 
-    // Mixed widths: an accented character exactly straddling the cut.
+    // Mixed widths: an accented character straddling the cut.
     let mut title = "x".repeat(MAX_TITLE_BYTES - 3 - "[work] ".len() - 1);
     title.push('\u{00e9}');
     title.push_str("tail");

compositor/wlproxy/src/protocol.rs

@@ -1,6 +1,4 @@
-//! Message signatures from the generated tables (protocol_tables.rs): how many
-//! descriptors ride with each message and which object it creates, so the
-//! object map stays in step with both peers.
+//! Message signatures (protocol_tables.rs): the descriptors each carries and the object it creates.
 
 use crate::wire::{ArgReader, WireError};
 
@@ -24,8 +22,7 @@ pub struct Interface {
     pub events: &'static [Message],
 }
 
-/// The interface of that name; None is a refusal in every caller. Indexed on
-/// first use, since this runs for every new object and every advertised global.
+/// The interface of that name, indexed on first use: this runs for every new object and global.
 pub fn find(name: &str) -> Option<&'static Interface> {
     use std::collections::HashMap;
     use std::sync::OnceLock;
@@ -44,8 +41,7 @@ pub enum Arg {
     Fixed,
     String { nullable: bool },
     Object { nullable: bool },
-    /// A new object. `iface` is None for wl_registry.bind, whose new_id follows
-    /// the interface name and version the client chose.
+    /// A new object; `iface` is None for wl_registry.bind, where the client names it on the wire.
     NewId { iface: Option<&'static str> },
     Array,
     Fd,
@@ -57,12 +53,10 @@ impl Message {
     }
 }
 
-/// The object a message creates and the strings it carries (for rewriting),
-/// borrowed from the body: decoding allocates only for a message with strings.
+/// The object a message creates and its strings, borrowed: only a message with strings allocates.
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
 pub struct Decoded<'a> {
-    /// (new object id, interface name). No message creates two (the tables are
-    /// tested for it), and a body that tries is refused.
+    /// (id, interface). No message in the tables creates two; a body that tries is refused.
     pub new_object: Option<(u32, &'a str)>,
     /// (byte offset of the string's length word within the body, value)
     pub strings: Vec<(usize, &'a str)>,
@@ -80,8 +74,7 @@ impl<'a> Decoded<'a> {
     }
 }
 
-/// Validate a body against its signature and collect what the proxy needs. It
-/// must parse exactly to its end: a trailing byte is as suspect as a missing one.
+/// Check a body against its signature to its exact end: a trailing byte is as bad as a missing one.
 pub fn decode<'a>(msg: &Message, body: &'a [u8]) -> Result<Decoded<'a>, WireError> {
     let mut r = ArgReader::new(body);
     let mut d = Decoded::default();

compositor/wlproxy/src/protocol/tests.rs

@@ -75,11 +75,7 @@ fn strings_are_located_for_rewriting() {
     assert_eq!(d.strings, vec![(0, "hello")]);
 }
 
-// --- the decoder, attacked ---------------------------------------------
-/* `Header::parse` and `decode` see every byte a client sends. A well-formed
- * body for each message in the tables is damaged by a fixed-seed generator,
- * so a failure repeats on every machine. The decoder must never panic or
- * read past the body, and returns Ok only for an exact parse. */
+// --- Header::parse and decode see every byte a client sends: fuzz them ---
 
 /// xorshift64*: small, seeded, the same sequence everywhere.
 pub(crate) struct Rng(pub u64);
@@ -175,7 +171,7 @@ fn decoder_survives_any_body() {
             }
         }
     }
-    // The generator must actually reach both sides of the decoder.
+    // The generator must reach both sides of the decoder.
     assert!(tried > 10_000 && accepted > tried / 50 && accepted < tried, "tried {tried}, accepted {accepted}");
 }