Skip to content

Commit 61696b3

Browse files
arctic-hen7arctic-hen7
committed
feat(cli): added hot reloading
Ended up making the CLI spawn another version of itself and then the original acts as a watcher. This introduces a privilege escalation vulnerability though if installed as root (happened with PulseAudio several years ago).
1 parent 998a041 commit 61696b3

File tree

3 files changed

+133
-133
lines changed

3 files changed

+133
-133
lines changed

‎packages/perseus-cli/Cargo.toml

+3-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ clap = { version = "=3.0.0-beta.5", features = ["color"] }
3131
fs_extra = "1"
3232
tokio = { version = "1", features = [ "macros", "rt-multi-thread", "sync" ] }
3333
warp = "0.3"
34-
notify = "4"
34+
command-group = "1"
35+
ctrlc = { version = "3.0", features = ["termination"] }
36+
notify = "=5.0.0-pre.13"
3537

3638
[lib]
3739
name = "perseus_cli"

‎packages/perseus-cli/src/bin/main.rs

+120-132
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use clap::Parser;
2+
use command_group::stdlib::CommandGroup;
23
use fmterr::fmt_err;
3-
use notify::{watcher, RecursiveMode, Watcher};
4-
use perseus_cli::parse::SnoopSubcommand;
4+
use notify::{recommended_watcher, RecursiveMode, Watcher};
5+
use perseus_cli::parse::{ExportOpts, ServeOpts, SnoopSubcommand};
56
use perseus_cli::{
67
build, check_env, delete_artifacts, delete_bad_dir, deploy, eject, export, has_ejected,
78
parse::{Opts, Subcommand},
@@ -11,8 +12,8 @@ use perseus_cli::{errors::*, snoop_build, snoop_server, snoop_wasm_build};
1112
use std::env;
1213
use std::io::Write;
1314
use std::path::PathBuf;
15+
use std::process::Command;
1416
use std::sync::mpsc::channel;
15-
use std::time::Duration;
1617

1718
// All this does is run the program and terminate with the acquired exit code
1819
#[tokio::main]
@@ -62,6 +63,14 @@ async fn real_main() -> i32 {
6263
}
6364
}
6465

66+
// This is used internally for message passing
67+
enum Event {
68+
// Sent if we should restart the child process
69+
Reload,
70+
// Sent if we should temrinate the child process
71+
Terminate,
72+
}
73+
6574
// This performs the actual logic, separated for deduplication of error handling and destructor control
6675
// This returns the exit code of the executed command, which we should return from the process itself
6776
// This prints warnings using the `writeln!` macro, which allows the parsing of `stdout` in production or a vector in testing
@@ -80,6 +89,109 @@ async fn core(dir: PathBuf) -> Result<i32, Error> {
8089
// Check the user's environment to make sure they have prerequisites
8190
// We do this after any help pages or version numbers have been parsed for snappiness
8291
check_env()?;
92+
93+
// Check if this process is allowed to watch for changes
94+
// This will be set to `true` if this is a child process
95+
// The CLI will actually spawn another version of itself if we're watching for changes
96+
// The reason for this is to avoid having to manage handlers for multiple threads and other child processes
97+
// After several days of attempting, this is the only feasible solution (short of a full rewrite of the CLI)
98+
let watch_allowed = env::var("PERSEUS_WATCHING_PROHIBITED").is_err();
99+
// Check if the user wants to watch for changes
100+
match &opts.subcmd {
101+
Subcommand::Export(ExportOpts { watch, .. })
102+
| Subcommand::Serve(ServeOpts { watch, .. })
103+
if *watch && watch_allowed =>
104+
{
105+
let (tx_term, rx) = channel();
106+
let tx_fs = tx_term.clone();
107+
// Set the handler for termination events (more than just SIGINT) on all platforms
108+
// We do this before anything else so that, if it fails, we don't have servers left open
109+
ctrlc::set_handler(move || {
110+
tx_term
111+
.send(Event::Terminate)
112+
.expect("couldn't shut down child processes (servers may have been left open)")
113+
})
114+
.expect("couldn't set handlers to gracefully terminate process");
115+
116+
// Find out where this binary is
117+
// SECURITY: If the CLI were installed with root privileges, it would be possible to create a hard link to the
118+
// binary, execute through that, and then replace it with a malicious binary before we got here which would
119+
// allow privilege escalation. See https://vulners.com/securityvulns/SECURITYVULNS:DOC:22183.
120+
// TODO Drop root privileges at startup
121+
let bin_name =
122+
env::current_exe().map_err(|err| WatchError::GetSelfPathFailed { source: err })?;
123+
// Get the arguments to provide
124+
// These are the same, but we'll disallow watching with an environment variable
125+
let mut args = env::args().collect::<Vec<String>>();
126+
// We'll remove the first element of the arguments (binary name, but less reliable)
127+
args.remove(0);
128+
129+
// Set up a watcher
130+
let mut watcher = recommended_watcher(move |_| {
131+
// If this fails, the watcher channel was completely disconnected, which should never happen (it's in a loop)
132+
tx_fs.send(Event::Reload).unwrap();
133+
})
134+
.map_err(|err| WatchError::WatcherSetupFailed { source: err })?;
135+
// Watch the current directory
136+
for entry in std::fs::read_dir(".")
137+
.map_err(|err| WatchError::ReadCurrentDirFailed { source: err })?
138+
{
139+
// We want to exclude `target/` and `.perseus/`, otherwise we should watch everything
140+
let entry = entry.map_err(|err| WatchError::ReadDirEntryFailed { source: err })?;
141+
let name = entry.file_name();
142+
if name != "target" && name != ".perseus" {
143+
watcher
144+
.watch(&entry.path(), RecursiveMode::Recursive)
145+
.map_err(|err| WatchError::WatchFileFailed {
146+
filename: entry.path().to_str().unwrap().to_string(),
147+
source: err,
148+
})?;
149+
}
150+
}
151+
152+
// This will store the handle to the child process
153+
// This will be updated every time we re-create the process
154+
// We spawn it as a process group, whcih means signals go to grandchild processes as well, which means hot reloading
155+
// can actually work!
156+
let mut child = Command::new(&bin_name)
157+
.args(&args)
158+
.env("PERSEUS_WATCHING_PROHIBITED", "true")
159+
.group_spawn()
160+
.map_err(|err| WatchError::SpawnSelfFailed { source: err })?;
161+
162+
let res = loop {
163+
match rx.recv() {
164+
Ok(Event::Reload) => {
165+
// Kill the current child process
166+
// This will return an error if the child has already exited, which is fine
167+
// This gracefully kills the process in the sense that it kills it and all its children
168+
let _ = child.kill();
169+
// Restart it
170+
child = Command::new(&bin_name)
171+
.args(&args)
172+
.env("PERSEUS_WATCHING_PROHIBITED", "true")
173+
.group_spawn()
174+
.map_err(|err| WatchError::SpawnSelfFailed { source: err })?;
175+
}
176+
Ok(Event::Terminate) => {
177+
// This means the user is trying to stop the process
178+
// We have to manually terminate the process group, because it's a process *group*
179+
let _ = child.kill();
180+
// From here, we can let the prgoram terminate naturally
181+
break Ok(0);
182+
}
183+
Err(err) => break Err(WatchError::WatcherError { source: err }),
184+
}
185+
};
186+
let exit_code = res?;
187+
Ok(exit_code)
188+
}
189+
// If not, just run the central logic normally
190+
_ => core_watch(dir, opts).await,
191+
}
192+
}
193+
194+
async fn core_watch(dir: PathBuf, opts: Opts) -> Result<i32, Error> {
83195
// If we're not cleaning up artifacts, create them if needed
84196
if !matches!(opts.subcmd, Subcommand::Clean(_)) {
85197
prepare(dir.clone())?;
@@ -98,141 +210,17 @@ async fn core(dir: PathBuf) -> Result<i32, Error> {
98210
if exit_code != 0 {
99211
return Ok(exit_code);
100212
}
101-
102-
if export_opts.watch {
103-
let dir_2 = dir.clone();
104-
let export_opts_2 = export_opts.clone();
105-
if export_opts.serve {
106-
tokio::spawn(async move {
107-
serve_exported(dir_2, export_opts_2.host, export_opts_2.port).await
108-
});
109-
}
110-
// Now watch for changes
111-
let (tx, rx) = channel();
112-
let mut watcher = watcher(tx, Duration::from_secs(2))
113-
.map_err(|err| WatchError::WatcherSetupFailed { source: err })?;
114-
// Watch the current directory
115-
for entry in std::fs::read_dir(".")
116-
.map_err(|err| WatchError::ReadCurrentDirFailed { source: err })?
117-
{
118-
// We want to exclude `target/` and `.perseus/`, otherwise we should watch everything
119-
let entry =
120-
entry.map_err(|err| WatchError::ReadDirEntryFailed { source: err })?;
121-
let name = entry.file_name();
122-
if name != "target" && name != ".perseus" {
123-
watcher
124-
.watch(entry.path(), RecursiveMode::Recursive)
125-
.map_err(|err| WatchError::WatchFileFailed {
126-
filename: entry.path().to_str().unwrap().to_string(),
127-
source: err,
128-
})?;
129-
}
130-
}
131-
132-
let res: Result<i32, Error> = loop {
133-
match rx.recv() {
134-
Ok(_) => {
135-
// Delete old build/exportation artifacts
136-
delete_artifacts(dir.clone(), "static")?;
137-
delete_artifacts(dir.clone(), "exported")?;
138-
let dir_2 = dir.clone();
139-
let opts = export_opts.clone();
140-
match export(dir_2.clone(), opts.clone()) {
141-
// We'l let the user know if there's a non-zero exit code
142-
Ok(exit_code) => {
143-
if exit_code != 0 {
144-
eprintln!("Non-zero exit code returned from exporting process: {}.", exit_code)
145-
}
146-
}
147-
// Because we're watching for changes, we can manage errors here
148-
// We won't actually terminate unless the user tells us to
149-
Err(err) => eprintln!("{}", fmt_err(&err)),
150-
}
151-
// TODO Reload the browser automatically
152-
}
153-
Err(err) => break Err(WatchError::WatcherError { source: err }.into()),
154-
}
155-
};
156-
return res;
157-
} else {
158-
if export_opts.serve {
159-
serve_exported(dir, export_opts.host, export_opts.port).await;
160-
}
161-
0
213+
if export_opts.serve {
214+
serve_exported(dir, export_opts.host, export_opts.port).await;
162215
}
216+
0
163217
}
164218
Subcommand::Serve(serve_opts) => {
165219
if !serve_opts.no_build {
166220
delete_artifacts(dir.clone(), "static")?;
167221
}
168-
if serve_opts.watch {
169-
match serve(dir.clone(), serve_opts.clone()) {
170-
// We'll let the user know if there's a non-zero exit code
171-
Ok((exit_code, _server_path)) => {
172-
if exit_code != 0 {
173-
eprintln!(
174-
"Non-zero exit code returned from serving process: {}.",
175-
exit_code
176-
)
177-
}
178-
}
179-
// Because we're watching for changes, we can manage errors here
180-
// We won't actually terminate unless the user tells us to
181-
Err(err) => eprintln!("{}", fmt_err(&err)),
182-
};
183-
// Now watch for changes
184-
let (tx, rx) = channel();
185-
let mut watcher = watcher(tx, Duration::from_secs(2))
186-
.map_err(|err| WatchError::WatcherSetupFailed { source: err })?;
187-
// Watch the current directory
188-
for entry in std::fs::read_dir(".")
189-
.map_err(|err| WatchError::ReadCurrentDirFailed { source: err })?
190-
{
191-
// We want to exclude `target/` and `.perseus/`, otherwise we should watch everything
192-
let entry =
193-
entry.map_err(|err| WatchError::ReadDirEntryFailed { source: err })?;
194-
let name = entry.file_name();
195-
if name != "target" && name != ".perseus" {
196-
watcher
197-
.watch(entry.path(), RecursiveMode::Recursive)
198-
.map_err(|err| WatchError::WatchFileFailed {
199-
filename: entry.path().to_str().unwrap().to_string(),
200-
source: err,
201-
})?;
202-
}
203-
}
204-
205-
let res: Result<i32, Error> = loop {
206-
match rx.recv() {
207-
Ok(_) => {
208-
// Delete old build artifacts if `--no-build` wasn't specified
209-
if !serve_opts.no_build {
210-
delete_artifacts(dir.clone(), "static")?;
211-
}
212-
match serve(dir.clone(), serve_opts.clone()) {
213-
// We'll let the user know if there's a non-zero exit code
214-
Ok((exit_code, _server_path)) => {
215-
if exit_code != 0 {
216-
eprintln!(
217-
"Non-zero exit code returned from serving process: {}.",
218-
exit_code
219-
)
220-
}
221-
}
222-
// Because we're watching for changes, we can manage errors here
223-
// We won't actually terminate unless the user tells us to
224-
Err(err) => eprintln!("{}", fmt_err(&err)),
225-
};
226-
// TODO Reload the browser automatically
227-
}
228-
Err(err) => break Err(WatchError::WatcherError { source: err }.into()),
229-
}
230-
};
231-
return res;
232-
} else {
233-
let (exit_code, _server_path) = serve(dir, serve_opts)?;
234-
exit_code
235-
}
222+
let (exit_code, _server_path) = serve(dir, serve_opts)?;
223+
exit_code
236224
}
237225
Subcommand::Test(test_opts) => {
238226
// This will be used by the subcrates

‎packages/perseus-cli/src/errors.rs

+10
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,14 @@ pub enum WatchError {
228228
#[source]
229229
source: std::sync::mpsc::RecvError,
230230
},
231+
#[error("couldn't spawn a child process to build your app in watcher mode")]
232+
SpawnSelfFailed {
233+
#[source]
234+
source: std::io::Error,
235+
},
236+
#[error("couldn't get the path to the cli's executable, try re-running the command")]
237+
GetSelfPathFailed {
238+
#[source]
239+
source: std::io::Error,
240+
},
231241
}

0 commit comments

Comments
 (0)