Offer Change keys in the desktop app

README.md

@@ -349,6 +349,7 @@ The common ones:
 | `CODETRIAL_MAX_CONCURRENT_INTERVIEWS` | `16` | Interviews one `web` process hosts agents for |
 | `CODETRIAL_ALLOWED_HOSTS` | empty | Comma-separated `Host` values the server answers; any other is refused with 421. Empty answers every name, but a value that names no host is refused at startup. `--allowed-hosts` sets it from the command line |
 | `CODETRIAL_EXIT_WITH_STDIN` | unset | For a program that starts the server as its child: exit when standard input reaches end of file, so the server does not outlive a launcher that crashed. Read from the process environment even under `--data-dir` |
+| `CODETRIAL_CHANGE_KEYS_CODE` | unset | For a launcher that starts the server as its child with `CODETRIAL_EXIT_WITH_STDIN`: the code Change keys asks for, which such a child cannot print to a terminal. At least 24 letters and digits; a shorter one, or none, leaves Change keys off. Read from the process environment even under `--data-dir` |
 
 Two more matter in production. `SESSION_SECRET` signs the session cookie, and
 its built-in default is a string published in this repository, so the server

desktop/Cargo.toml

@@ -27,6 +27,8 @@ all = "deny"
 tauri-build = { version = "=2.7.4", features = [] }
 
 [dependencies]
+# Already in the tree through tauri; the Change keys code is read from it.
+getrandom = "=0.3.4"
 tauri = { version = "=2.12.5", features = [] }
 tauri-plugin-dialog = "=2.8.4"
 tauri-plugin-opener = "=2.7.2"

desktop/src/main.rs

@@ -18,7 +18,9 @@ use std::path::Path;
 use std::sync::{LazyLock, Mutex, mpsc};
 use std::time::Duration;
 
-use tauri::webview::{DownloadEvent, NewWindowResponse, PermissionKind, PermissionResponse};
+use tauri::webview::{
+    DownloadEvent, NewWindowResponse, PageLoadEvent, PermissionKind, PermissionResponse,
+};
 use tauri::{AppHandle, Manager, RunEvent, Url, WebviewUrl, WebviewWindowBuilder, WindowEvent};
 use tauri_plugin_dialog::{DialogExt, MessageDialogButtons, MessageDialogKind};
 
@@ -89,6 +91,8 @@ enum Close {
 
 struct Shell {
     server: Mutex<Option<server::Server>>,
+    /// The running server's Change keys code, filled into its Setup page.
+    change_keys_code: Mutex<Option<String>>,
     phase: Mutex<Phase>,
     close: Mutex<Close>,
 }
@@ -113,6 +117,7 @@ fn main() {
         .plugin(tauri_plugin_dialog::init())
         .manage(Shell {
             server: Mutex::new(None),
+            change_keys_code: Mutex::new(None),
             phase: Mutex::new(Phase::Starting {
                 server_ready: false,
                 webrtc: None,
@@ -208,6 +213,24 @@ fn open_window(app: &AppHandle) -> tauri::Result<()> {
                 Verdict::Refuse => false,
             }
         })
+        // Change keys asks for the code the server would print to a terminal,
+        // and this server has none: the code is filled in from out here, into
+        // the server's own Setup page and no other, and never travels in a URL
+        // where history would keep it.
+        .on_page_load(|webview, payload| {
+            let url = payload.url();
+            if payload.event() != PageLoadEvent::Finished
+                || !navigation::same_origin(url, &SERVER_URL)
+                || url.path() != "/setup"
+            {
+                return;
+            }
+            let shell = webview.app_handle().state::<Shell>();
+            let code = shell.change_keys_code.lock().expect("code lock").clone();
+            if let Some(code) = code {
+                let _ = webview.eval(fill_change_keys_code(&code));
+            }
+        })
         // `target="_blank"` and `window.open`. A second webview would be a
         // second window into the same origin with none of the rules above, so a
         // web page goes to the browser and nothing opens here.
@@ -300,6 +323,7 @@ fn launch(app: &AppHandle) {
         std::fs::create_dir_all(&data_dir)
             .map_err(|error| format!("cannot create {}: {error}", data_dir.display()))?;
         Ok(server::Launch {
+            change_keys_code: server::change_keys_code()?,
             program: server::program_path()?,
             data_dir,
             addr: SERVER_ADDR.to_string(),
@@ -313,6 +337,10 @@ fn launch(app: &AppHandle) {
         Err(message) => return show_failure(app, &message, None),
     };
 
+    *app.state::<Shell>()
+        .change_keys_code
+        .lock()
+        .expect("code lock") = Some(launch.change_keys_code.clone());
     let data_dir = launch.data_dir.clone();
     let reporter = app.clone();
     let started = server::start(&launch, move |status| {
@@ -327,6 +355,7 @@ fn launch(app: &AppHandle) {
                 }
                 open_server(app);
             }
+            server::Status::KeysSaved => restart(app),
             server::Status::Exited(message) => {
                 if !app.state::<Shell>().quitting() {
                     show_failure(app, &message, Some(data_dir.as_path()));
@@ -342,6 +371,51 @@ fn launch(app: &AppHandle) {
     }
 }
 
+/// The script that puts `code`, hex digits only, into the Setup page's code
+/// field and keeps the field out of the way: the person never sees the code.
+fn fill_change_keys_code(code: &str) -> String {
+    format!(
+        "(() => {{ const field = document.getElementById('code');          if (field) field.value = '{code}';          const row = document.getElementById('code-row');          if (row) row.style.display = 'none'; }})()"
+    )
+}
+
+/// Starts the server again onto the keys Change keys just saved, which the
+/// running one keeps until it exits. The save itself refused while an
+/// interview was running or writing its report. Back on the lobby once the new
+/// server is ready, in this window or the browser as before.
+///
+/// Off the reporting thread, which the stop below would otherwise wait on.
+fn restart(app: &AppHandle) {
+    let shell = app.state::<Shell>();
+    if shell.quitting() {
+        return;
+    }
+    {
+        let mut phase = shell.phase.lock().expect("phase lock");
+        let webrtc = match *phase {
+            Phase::Serving => true,
+            Phase::InBrowser => false,
+            Phase::Starting { .. } | Phase::Failed => return,
+        };
+        *phase = Phase::Starting {
+            server_ready: false,
+            webrtc: Some(webrtc),
+        };
+    }
+    note("keys saved: restarting the server");
+    let server = shell.server.lock().expect("server lock").take();
+    *shell.change_keys_code.lock().expect("code lock") = None;
+    let app = app.clone();
+    std::thread::spawn(move || {
+        if let Some(server) = server {
+            server.stop(STOP_GRACE);
+        }
+        if !app.state::<Shell>().quitting() {
+            launch(&app);
+        }
+    });
+}
+
 /// Shows the server in this window, or in the default browser when this
 /// window has no WebRTC, once both halves are known: the server is ready and
 /// the starting page has said what it has. Called as each arrives, in either

desktop/src/server.rs

@@ -19,6 +19,10 @@ use std::time::{Duration, Instant};
 /// rather than closed. The CLI tests run the real server against this text.
 const BOUND_LINE: &str = "codetrial: open http://";
 
+/// What the server prints once Change keys has rewritten its config. The
+/// server's copy is `KEYS_SAVED_LINE` in src/web/setup_edit.rs.
+const KEYS_SAVED_LINE: &str = "codetrial: keys saved; restart to use them";
+
 /// How much of the server's stderr explains an exit. The reason is its last
 /// line; the ones before it are the warnings it printed on the way.
 const REASON_LINES: usize = 12;
@@ -39,6 +43,19 @@ pub struct Launch {
     /// A CLI config whose keys Setup offers to use on a first launch. The
     /// server decides what is offered and what using it writes.
     pub import_config: Option<PathBuf>,
+    /// What Change keys asks for, new on every launch. The server has no
+    /// terminal to print one to, so the app makes it, hands it over in the
+    /// environment, and fills it into the Setup page in its own window.
+    pub change_keys_code: String,
+}
+
+/// A code for `Launch::change_keys_code`: 32 hex digits from the operating
+/// system's generator.
+pub fn change_keys_code() -> Result<String, String> {
+    let mut bytes = [0u8; 16];
+    getrandom::fill(&mut bytes)
+        .map_err(|error| format!("cannot make a Change keys code: {error}"))?;
+    Ok(bytes.iter().map(|byte| format!("{byte:02x}")).collect())
 }
 
 /// The CLI config in `start`: the CLI's own two places there and nowhere else.
@@ -57,6 +74,8 @@ pub fn cli_config_in(start: &Path) -> Option<PathBuf> {
 pub enum Status {
     /// Listening on `Launch::addr`.
     Ready,
+    /// Change keys saved new keys, which only a new launch uses.
+    KeysSaved,
     /// Gone, and why, in words for the person looking at the window.
     Exited(String),
 }
@@ -116,6 +135,7 @@ fn command(launch: &Launch) -> Command {
         // data folder is the one place it can sensibly mean.
         .current_dir(&launch.data_dir)
         .env("CODETRIAL_EXIT_WITH_STDIN", "1")
+        .env("CODETRIAL_CHANGE_KEYS_CODE", &launch.change_keys_code)
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped());
@@ -177,6 +197,13 @@ pub fn start(
         copiers.push(std::thread::spawn(move || {
             let mut bound = false;
             copy_lines(stdout, &log, |line| {
+                if bound && line == KEYS_SAVED_LINE {
+                    let finished = finished.lock().expect("report order lock");
+                    if !*finished {
+                        report(Status::KeysSaved);
+                    }
+                    return;
+                }
                 if bound || !line.starts_with(BOUND_LINE) {
                     return;
                 }

desktop/tests/unit/server.rs

@@ -22,6 +22,7 @@ fn the_server_is_started_on_the_data_dir_and_told_the_rest() {
         data_dir: PathBuf::from("/data"),
         addr: "127.0.0.1:3101".to_string(),
         import_config: None,
+        change_keys_code: "0123456789abcdef0123456789abcdef".to_string(),
     };
     let command = command(&launch);
     let args = command
@@ -43,17 +44,21 @@ fn the_server_is_started_on_the_data_dir_and_told_the_rest() {
     );
     assert_eq!(command.get_current_dir(), Some(Path::new("/data")));
 
-    // The pipe is the one thing it is told through the environment, which a
-    // `--data-dir` launch reads no configuration from.
+    // The pipe and the Change keys code are the two things it is told through
+    // the environment, which a `--data-dir` launch reads no configuration from.
     let env = |key: &str| {
         command
             .get_envs()
             .find(|(name, _)| *name == key)
             .and_then(|(_, value)| value)
             .map(|value| value.to_string_lossy().into_owned())
     };
-    assert_eq!(command.get_envs().count(), 1);
+    assert_eq!(command.get_envs().count(), 2);
     assert_eq!(env("CODETRIAL_EXIT_WITH_STDIN").as_deref(), Some("1"));
+    assert_eq!(
+        env("CODETRIAL_CHANGE_KEYS_CODE").as_deref(),
+        Some("0123456789abcdef0123456789abcdef")
+    );
 }
 
 #[test]
@@ -63,6 +68,7 @@ fn a_cli_config_found_is_offered_to_the_server_to_import() {
         data_dir: PathBuf::from("/data"),
         addr: "127.0.0.1:3101".to_string(),
         import_config: Some(PathBuf::from("/checkout/config/codetrial.env.local")),
+        change_keys_code: "0123456789abcdef0123456789abcdef".to_string(),
     };
     let args = command(&launch)
         .get_args()
@@ -144,6 +150,7 @@ fn launch_of(dir: &Path, program: PathBuf, addr: String) -> Launch {
         data_dir: dir.to_path_buf(),
         addr,
         import_config: None,
+        change_keys_code: "0123456789abcdef0123456789abcdef".to_string(),
     }
 }
 
@@ -218,3 +225,34 @@ fn a_server_is_ready_once_bound_and_stops_when_its_stdin_closes() {
     assert!(received.recv_timeout(Duration::from_millis(500)).is_err());
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+/// A save is reported once the server is up, so the app can restart it onto
+/// the new keys. Before the bind the line is only text.
+#[cfg(unix)]
+#[test]
+fn a_save_after_the_bind_is_reported() {
+    let dir = temp_dir("saved");
+    let program = fake_server(
+        &dir,
+        "echo 'codetrial: keys saved; restart to use them'\n\
+         echo 'codetrial: open http://127.0.0.1:9 in your browser'\n\
+         echo 'codetrial: keys saved; restart to use them'\n\
+         cat >/dev/null",
+    );
+    let (sender, received) = mpsc::channel();
+    let sender = Mutex::new(sender);
+    let server = start(
+        &launch_of(&dir, program, "127.0.0.1:9".to_string()),
+        move |status| {
+            let _ = sender.lock().unwrap().send(status);
+        },
+    )
+    .unwrap();
+
+    let wait = Duration::from_secs(10);
+    assert_eq!(received.recv_timeout(wait).unwrap(), Status::Ready);
+    assert_eq!(received.recv_timeout(wait).unwrap(), Status::KeysSaved);
+    assert!(received.recv_timeout(Duration::from_millis(300)).is_err());
+    server.stop(Duration::from_secs(5));
+    let _ = std::fs::remove_dir_all(&dir);
+}

docs/desktop.md

@@ -68,13 +68,24 @@ server's rules:
   quitting or by crashing, the server goes too and does not keep the port.
   This one stays in the environment even for `--data-dir`, because it is not
   configuration but a description of the pipe the launcher handed over.
+- `CODETRIAL_CHANGE_KEYS_CODE`: the code the lobby's Change keys asks for.
+  A server started from a terminal makes its own and prints it there; this
+  one has no terminal, so the app makes a new code at every launch, hands it
+  over here, and fills it into the Setup page when that page loads in its
+  window. It never travels in a URL or reaches the server log. Once a save
+  rewrites the config, the server prints `codetrial: keys saved; restart to
+  use them`, and the app stops the server and starts it again onto the new
+  keys, back on the lobby. The save itself refuses while an interview is
+  running or writing its report. With the interview in the default browser
+  the app cannot fill the code there, so Change keys works only in the
+  window.
 - `--no-update-check`: the lobby never offers a newer release, whatever the
   config file says about `CODETRIAL_UPDATE_CHECK`. That check compares the
   server with the `latest` release, and the app is published as
   `desktop-latest`, so it would offer every desktop install an update to the
   CLI binary.
 
-All five are ordinary server options, and a CLI install can use them the same
+All six are ordinary server options, and a CLI install can use them the same
 way. `codetrial --help` lists the four flags, and the README's configuration
 table lists `CODETRIAL_ALLOWED_HOSTS`.
 

src/config.rs

@@ -660,6 +660,16 @@ pub fn exit_with_stdin(value: Option<&str>) -> bool {
     parse_bool(value, false)
 }
 
+/// `CODETRIAL_CHANGE_KEYS_CODE`: the code the desktop app hands its child.
+/// One too short to stand in for the code `run_web` makes is no code, so a
+/// launcher that set it carelessly gets no Change keys rather than a weak one.
+pub fn change_keys_code(value: Option<&str>) -> Option<String> {
+    value
+        .map(str::trim)
+        .filter(|code| code.len() >= 24 && code.bytes().all(|byte| byte.is_ascii_alphanumeric()))
+        .map(str::to_string)
+}
+
 pub fn gemini_candidate_video_enabled(value: Option<&str>) -> bool {
     parse_bool(value, DEFAULT_GEMINI_CANDIDATE_VIDEO_ENABLED)
 }

src/main.rs

@@ -370,6 +370,20 @@ fn web_server_config(
     })
 }
 
+/// The code Change keys asks for: made here for a launch from a terminal, or
+/// the one the desktop app made and handed its child. A child given none gets
+/// no Change keys.
+fn change_keys_code(launched_as_child: bool) -> Result<Option<String>, String> {
+    if launched_as_child {
+        return Ok(codetrial::config::change_keys_code(
+            std::env::var("CODETRIAL_CHANGE_KEYS_CODE").ok().as_deref(),
+        ));
+    }
+    codetrial::accounts::random_token(18)
+        .map(Some)
+        .map_err(|error| format!("could not make a Change keys code: {error}"))
+}
+
 fn run_web(options: CliOptions) -> Result<(), String> {
     // Falls through: once `serve_setup` returns, the config exists and the rest
     // of this function is an ordinary launch, on the socket Setup served on.
@@ -445,12 +459,12 @@ fn run_web(options: CliOptions) -> Result<(), String> {
         primary_config_path(&options)?,
         values.clone(),
         bound,
-        !launched_as_child,
+        change_keys_code(launched_as_child)?,
         config.trusted_proxy_hops,
-    )?;
+    );
     // Same reason `serve_setup` prints this: say where to go.
     println!("codetrial: open http://{bound} in your browser");
-    if let Some(setup) = &setup {
+    if let Some(setup) = setup.as_ref().filter(|_| !launched_as_child) {
         println!(
             "codetrial: to change keys at http://{bound}/setup, enter {}",
             setup.code()