Skip to content

Commit

A registration token never shows in the process list. The desktop app passed it to g1t-runner sandbox up as --token, which anyone on the computer can read while up runs; it goes in up's environment now, as G1T_RUNNER_TOKEN, which the launcher reads when --token is left out, as register-and-run already did.

syntaqxcommitted Parent5ca8722Browse files
6 files+34−140/6 viewed
+1−1
182182 let origin = origin(&server(app));
183183 let state = app.state::<State>();
184184 let mut allowed = state.allowed.lock().unwrap();
185− if allowed.iter().any(|known| *known == origin) {
185+ if allowed.contains(&origin) {
186186 return Ok(());
187187 }
188188 let mut site = CapabilityBuilder::new(format!("site-{}", allowed.len())).window(WINDOW).remote(format!("{origin}/*"));
+18−6
227227 args
228228 }
229229
230−/// `sandbox up`'s arguments.
230+/// `sandbox up`'s arguments. The token is never one of them: anyone on the
231+/// computer can read a process's command line while it runs, so it goes in
232+/// `up`'s environment instead ([`with_token`]).
231233 pub fn up_args(dir: &Path, up: &Up) -> Vec<OsString> {
232234 let mut args = args("up", dir);
233235 for (name, value) in [("--url", up.url), ("--name", up.name), ("--labels", up.labels)] {
234236 args.push(name.into());
235237 args.push(value.into());
236238 }
237− if let Some(token) = up.token {
238− args.push("--token".into());
239− args.push(token.into());
239+ args
240+}
241+
242+/// Gives a launcher command the registration token, as `G1T_RUNNER_TOKEN`
243+/// in its environment and nowhere else.
244+pub fn with_token(command: &mut Command, token: Option<&str>) {
245+ if let Some(token) = token {
246+ command.env("G1T_RUNNER_TOKEN", token);
240247 }
241− args
242248 }
243249
244250 /// How `down` stops the sandbox.
529535 let up = text(up_args(&dir, &first));
530536 assert_eq!(&up[..4], ["sandbox", "up", "--json", "--state-dir"]);
531537 assert_eq!(up[4], dir.to_string_lossy());
532− assert_eq!(&up[5..], ["--url", "https://g1t.sh", "--name", "build-box", "--labels", "desktop", "--token", "g1trt_secret"]);
538+ // Never the token: it goes in the environment, out of the process list.
539+ assert_eq!(&up[5..], ["--url", "https://g1t.sh", "--name", "build-box", "--labels", "desktop"]);
540+ assert!(!up.iter().any(|a| a == "--token" || a.starts_with("g1trt_")));
541+ let mut command = command(Path::new("g1t-runner"), up_args(&dir, &first));
542+ with_token(&mut command, first.token);
543+ assert!(command.get_envs().any(|(name, value)| name == "G1T_RUNNER_TOKEN" && value == Some(std::ffi::OsStr::new("g1trt_secret"))));
544+ assert!(!command.get_args().any(|a| a.to_string_lossy().contains("g1trt_")));
533545 // Its name and labels as the app has them: the launcher adds `-linux` and the OS labels.
534546 assert!(!up.iter().any(|a| a.ends_with("-linux") || a.contains("windows")));
535547 let again = text(up_args(&dir, &Up { token: None, ..first.clone() }));
+6−4
12001200 Step::Up => launcher::up_args(&dir, &up),
12011201 Step::Update => launcher::update_args(&dir),
12021202 };
1203− if let Err(why) = self.spawn_step(step, args) {
1203+ if let Err(why) = self.spawn_step(step, args, up.token) {
12041204 self.run = None;
12051205 return self.not_started(workspace, token, why);
12061206 }
12291229 }
12301230
12311231 /// Runs a launcher command for the sandbox's run, reading its output.
1232− fn spawn_step(&mut self, step: Step, args: Vec<std::ffi::OsString>) -> Result<(), String> {
1232+ fn spawn_step(&mut self, step: Step, args: Vec<std::ffi::OsString>, token: Option<&str>) -> Result<(), String> {
12331233 let binary = native::binary(&folder(&self.app));
1234− let mut child = launcher::command(&binary, args).spawn().map_err(|e| format!("g1t-runner sandbox {} could not start: {e}", step.name()))?;
1234+ let mut command = launcher::command(&binary, args);
1235+ launcher::with_token(&mut command, token);
1236+ let mut child = command.spawn().map_err(|e| format!("g1t-runner sandbox {} could not start: {e}", step.name()))?;
12351237 let _ = std::fs::write(folder(&self.app).join("pid"), child.id().to_string());
12361238 let Some(generation) = self.run.as_ref().map(|run| run.generation) else {
12371239 native::stop(&mut child);
12501252 return;
12511253 }
12521254 let dir = native_dir(&self.app, Backend::Sandboxed);
1253− if let Err(why) = self.spawn_step(step, launcher::update_args(&dir)) {
1255+ if let Err(why) = self.spawn_step(step, launcher::update_args(&dir), None) {
12541256 self.write_log(&format!("g1t: {why}"));
12551257 }
12561258 }
+1−1
199199
200200 | When | The app runs |
201201 | --- | --- |
202−| A few seconds after g1t opens | `g1t-runner sandbox up --json --state-dir <data folder>/sandbox --url <server> --name <computer> --labels desktop`, with `--token` the first time only. The first `up` downloads and checks the sandbox's image and makes the VM, which takes a few minutes. |
202+| A few seconds after g1t opens | `g1t-runner sandbox up --json --state-dir <data folder>/sandbox --url <server> --name <computer> --labels desktop`, with the registration token the first time only, in its environment as `G1T_RUNNER_TOKEN`, never on its command line, where anyone on the computer could read it. The first `up` downloads and checks the sandbox's image and makes the VM, which takes a few minutes. |
203203 | Every few seconds while it runs | `g1t-runner sandbox status --json`, for the tray, the settings and the Runners page. |
204204 | When g1t quits, or you press **Stop** | `g1t-runner sandbox down`: a job that is running is cut short. |
205205 | When you choose another way of running jobs | `g1t-runner sandbox down --drain`, once the job that is running ends. The sandbox keeps its registration, so choosing Sandboxed again needs no new one. |
+1−1
205205
206206 | Option (`up`) | |
207207 | --- | --- |
208−| `--url`, `--token` | Where to register and the registration token, for the first `up`. |
208+| `--url`, `--token` | Where to register and the registration token, for the first `up`. The token can come from `G1T_RUNNER_TOKEN` in the environment instead, which keeps it out of the process list. |
209209 | `--name` | The runners' name before `-linux` and `-windows`. The computer's name if left out. |
210210 | `--labels`, `--group` | As for [register](#register-options), for both runners. |
211211 | `--cpus` | The Linux VM's runner's CPUs, and so its jobs'. Half the computer's, at most 8, if left out. |
+7−1
730730 saved.runner_image = Some(image.to_owned());
731731 }
732732 saved.save(dir)?;
733− let token = options.value("token").map(str::to_owned);
733+ // From G1T_RUNNER_TOKEN too, as register-and-run takes it: a token on
734+ // the command line shows in the process list to anyone on the machine
735+ // while `up` runs, and the desktop app passes it this way.
736+ let token = options
737+ .value("token")
738+ .map(str::to_owned)
739+ .or_else(|| std::env::var("G1T_RUNNER_TOKEN").ok().filter(|token| !token.trim().is_empty()));
734740
735741 let backend = backend(dir);
736742 let found = backend.availability();