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 <thomas@eizinger.io>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
This commit is contained in:
Thomas Eizinger
2025-01-21 22:22:54 +08:00
committed by GitHub
parent 8c2d15b8d7
commit e50b719d5c
2 changed files with 11 additions and 13 deletions

View File

@@ -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:
// <https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#Environment=>
#[arg(env = TOKEN_ENV_KEY, hide = true)]
token: Option<String>,
/// 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

View File

@@ -27,6 +27,10 @@ export default function Headless() {
<ChangeItem pull="7551">
Fixes an issue where large DNS responses were incorrectly discarded.
</ChangeItem>
<ChangeItem pull="7770">
BREAKING: Removes the positional token argument on the CLI. Use
`FIREZONE_TOKEN` or `FIREZONE_TOKEN_PATH` env variables instead.
</ChangeItem>
</Unreleased>
<Entry version="1.4.0" date={new Date("2024-12-13")}>
<ChangeItem pull="7350">