From e50b719d5c0fad266d43bf3675e220374dc1310d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 21 Jan 2025 22:22:54 +0800 Subject: [PATCH] refactor(headless-client): remove `FIREZONE_TOKEN` CLI arg (#7770) The current CLI of the headless-client allows passing the token as a positional parameter in addition to an env variable. This can be very confusing if you make a spelling error in the _command_ that you are trying to pass to the CLI, i.e. `standalone`. A misspelled command will be interpreted as the token to use to connect to the portal without any warning that it is similar to a command. The env variable `FIREZONE_TOKEN` is completely ignored in that case. To fix this, we remove the ability to pass the token via stdin. The token should instead be set via en env variable or read from a file at `FIREZONE_TOKEN_PATH`. --------- Signed-off-by: Thomas Eizinger Co-authored-by: Jamil --- rust/headless-client/src/main.rs | 20 +++++++------------ website/src/components/Changelog/Headless.tsx | 4 ++++ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 988747e84..80b819100 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -86,12 +86,6 @@ struct Cli { #[arg(long, env = "FIREZONE_NO_TELEMETRY", default_value_t = false)] no_telemetry: bool, - /// Token generated by the portal to authorize websocket connection. - // systemd recommends against passing secrets through env vars: - // - #[arg(env = TOKEN_ENV_KEY, hide = true)] - token: Option, - /// A filesystem path where the token can be found // Apparently passing secrets through stdin is the most secure method, but // until anyone asks for it, env vars are okay and files on disk are slightly better. @@ -119,7 +113,13 @@ fn main() -> Result<()> { .install_default() .expect("Calling `install_default` only once per process should always succeed"); - let mut cli = Cli::try_parse()?; + let cli = Cli::try_parse()?; + + // Modifying the environment of a running process is unsafe. If any other + // thread is reading or writing the environment, something bad can happen. + // So `run` must take over as early as possible during startup, and + // read the token env var before any other threads spawn. + let token_env_var = std::env::var(TOKEN_ENV_KEY).ok().map(SecretString::from); // Docs indicate that `remove_var` should actually be marked unsafe // SAFETY: We haven't spawned any other threads, this code should be the first @@ -131,12 +131,6 @@ fn main() -> Result<()> { } assert!(std::env::var(TOKEN_ENV_KEY).is_err()); - // Modifying the environment of a running process is unsafe. If any other - // thread is reading or writing the environment, something bad can happen. - // So `run` must take over as early as possible during startup, and - // take the token env var before any other threads spawn. - let token_env_var = cli.token.take().map(SecretString::from); - // TODO: This might have the same issue with fatal errors not getting logged // as addressed for the IPC service in PR #5216 let (layer, _handle) = cli diff --git a/website/src/components/Changelog/Headless.tsx b/website/src/components/Changelog/Headless.tsx index 13e0845cc..cb1af77b0 100644 --- a/website/src/components/Changelog/Headless.tsx +++ b/website/src/components/Changelog/Headless.tsx @@ -27,6 +27,10 @@ export default function Headless() { Fixes an issue where large DNS responses were incorrectly discarded. + + BREAKING: Removes the positional token argument on the CLI. Use + `FIREZONE_TOKEN` or `FIREZONE_TOKEN_PATH` env variables instead. +