fix(cli): remove dev-token env auth override

This commit is contained in:
Bryan Helmkamp 2026-05-01 14:30:05 -04:00
parent 79e0783838
commit 2668d16799
No known key found for this signature in database
10 changed files with 59 additions and 103 deletions

View file

@ -48,7 +48,7 @@ url = "https://fabro.example.com/api/v1"
`fabro model` uses `[cli.target]` by default when no explicit `--storage-dir` is passed. `fabro exec` remains a local session unless you pass `--server`, even if `[cli.target]` is configured.
Explicit `http(s)://...` targets are always treated as remote servers. They do not inherit identity from a local daemon, a local storage dir, or `~/.fabro/dev-token`. For remote URLs, use `fabro auth login --server ...`, a stored CLI auth session, or an explicit `FABRO_DEV_TOKEN`.
Explicit `http(s)://...` targets are always treated as remote servers. They do not inherit identity from a local daemon, a local storage dir, or `~/.fabro/dev-token`. For remote URLs, use `fabro auth login --server ...` or a stored CLI auth session.
CLI flags always override `settings.toml` values, which override hardcoded defaults.

View file

@ -1,9 +1,6 @@
use anyhow::Result;
use chrono::{DateTime, Utc};
use fabro_client::{AuthEntry, AuthStore, OAuthEntry};
use fabro_static::EnvVars;
use fabro_util::dev_token::validate_dev_token_format;
use fabro_util::printer::Printer;
use serde::Serialize;
use crate::args::AuthStatusArgs;
@ -45,8 +42,7 @@ enum StatusRow {
#[derive(Serialize)]
struct StatusOutput {
servers: Vec<StatusRow>,
env_dev_token: &'static str,
servers: Vec<StatusRow>,
}
pub(super) fn status_command(args: &AuthStatusArgs, ctx: &CommandContext) -> Result<()> {
@ -59,23 +55,13 @@ pub(super) fn status_command(args: &AuthStatusArgs, ctx: &CommandContext) -> Res
} else {
all_rows(&store, now)?
};
let env_dev_token = if load_env_dev_token_if_available() {
"active"
} else {
"not_set"
};
if ctx.explicit_json_requested() {
print_json_pretty(&StatusOutput {
servers: rows,
env_dev_token,
})?;
print_json_pretty(&StatusOutput { servers: rows })?;
return Ok(());
}
if rows.is_empty() {
fabro_util::printerr!(printer, "Not logged in to any servers.");
print_env_dev_token_status(env_dev_token, printer);
return Ok(());
}
@ -141,7 +127,6 @@ pub(super) fn status_command(args: &AuthStatusArgs, ctx: &CommandContext) -> Res
}
}
fabro_util::printerr!(printer, "");
print_env_dev_token_status(env_dev_token, printer);
Ok(())
}
@ -196,17 +181,6 @@ fn oauth_state(entry: &OAuthEntry, now: DateTime<Utc>) -> OAuthState {
}
}
fn print_env_dev_token_status(env_dev_token: &str, printer: Printer) {
if env_dev_token == "active" {
fabro_util::printerr!(
printer,
"FABRO_DEV_TOKEN: active (overrides persisted credentials)"
);
} else {
fabro_util::printerr!(printer, "FABRO_DEV_TOKEN: not_set");
}
}
fn human_state(state: OAuthState) -> &'static str {
match state {
OAuthState::Active => "active",
@ -215,17 +189,6 @@ fn human_state(state: OAuthState) -> &'static str {
}
}
#[expect(
clippy::disallowed_methods,
reason = "Auth status reports whether the documented dev-token env source is configured."
)]
fn load_env_dev_token_if_available() -> bool {
let env_token = std::env::var(EnvVars::FABRO_DEV_TOKEN)
.ok()
.filter(|token| validate_dev_token_format(token));
env_token.is_some()
}
#[cfg(test)]
mod tests {
use chrono::Duration;

View file

@ -10,11 +10,9 @@ use fabro_client::{
pub(crate) use fabro_client::{Client, RunEventStream};
use fabro_config::Storage;
use fabro_config::bind::Bind;
use fabro_static::EnvVars;
pub(crate) use fabro_types::RunProjection;
use fabro_types::UserSettings;
use fabro_util::dev_token;
use fabro_util::dev_token::validate_dev_token_format;
use tokio::time::sleep;
use crate::args::ServerTargetArgs;
@ -207,14 +205,6 @@ fn connect_cli_target_transport(
Ok((http_client, "http://fabro".to_string()))
}
#[expect(
clippy::disallowed_methods,
reason = "Server client authentication supports the documented local dev-token env source."
)]
fn process_env_var(name: &str) -> Option<String> {
std::env::var(name).ok()
}
async fn wait_for_runtime_dev_token(path: &Path) -> Result<String> {
let deadline = std::time::Instant::now() + Duration::from_secs(5);
@ -273,14 +263,9 @@ async fn connect_unix_socket_http_client(
fn resolve_target_credential_with_store(
target: &ServerTarget,
env_token: Option<&str>,
store: &AuthStore,
now: chrono::DateTime<chrono::Utc>,
) -> Result<Option<Credential>> {
if let Some(token) = env_token.filter(|token| validate_dev_token_format(token)) {
return Ok(Some(Credential::DevToken(token.to_owned())));
}
let Some(entry) = store.get(target)? else {
return Ok(None);
};
@ -296,9 +281,8 @@ fn resolve_target_credential_with_store(
}
fn resolve_target_credential(target: &ServerTarget) -> Result<Option<Credential>> {
let env_token = process_env_var(EnvVars::FABRO_DEV_TOKEN);
let store = AuthStore::default();
resolve_target_credential_with_store(target, env_token.as_deref(), &store, chrono::Utc::now())
resolve_target_credential_with_store(target, &store, chrono::Utc::now())
}
#[expect(
@ -352,22 +336,6 @@ mod tests {
use super::*;
#[test]
fn resolve_local_tcp_credential_prefers_valid_env_token() {
let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap();
let store = AuthStore::new(tempfile::tempdir().unwrap().path().join("auth.json"));
let credential = resolve_target_credential_with_store(
&target,
Some("fabro_dev_cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd"),
&store,
Utc::now(),
)
.unwrap();
assert!(matches!(credential, Some(Credential::DevToken(_))));
}
#[test]
fn resolve_target_credential_uses_persisted_dev_token_entry() {
let dir = tempfile::tempdir().unwrap();
@ -384,8 +352,7 @@ mod tests {
)
.unwrap();
let credential =
resolve_target_credential_with_store(&target, None, &store, Utc::now()).unwrap();
let credential = resolve_target_credential_with_store(&target, &store, Utc::now()).unwrap();
assert!(matches!(credential, Some(Credential::DevToken(found)) if found == token));
}
@ -406,7 +373,7 @@ mod tests {
)
.unwrap();
let credential = resolve_target_credential_with_store(&target, None, &store, now).unwrap();
let credential = resolve_target_credential_with_store(&target, &store, now).unwrap();
assert!(matches!(credential, Some(Credential::OAuth(_))));
}
@ -427,7 +394,7 @@ mod tests {
)
.unwrap();
let credential = resolve_target_credential_with_store(&target, None, &store, now).unwrap();
let credential = resolve_target_credential_with_store(&target, &store, now).unwrap();
assert!(matches!(credential, Some(Credential::OAuth(_))));
}
@ -448,7 +415,7 @@ mod tests {
)
.unwrap();
let credential = resolve_target_credential_with_store(&target, None, &store, now).unwrap();
let credential = resolve_target_credential_with_store(&target, &store, now).unwrap();
assert!(credential.is_none());
}

View file

@ -1,4 +1,5 @@
use fabro_test::{fabro_snapshot, test_context};
use serde_json::Value;
const DEV_TOKEN: &str =
"fabro_dev_abababababababababababababababababababababababababababababababab";
@ -149,19 +150,17 @@ fn status_help() {
}
#[test]
fn status_json_reports_env_dev_token_separately() {
fn status_json_ignores_env_dev_token() {
let context = test_context!();
let mut cmd = context.command();
cmd.args(["auth", "status", "--json"])
.env("FABRO_DEV_TOKEN", DEV_TOKEN);
fabro_snapshot!(context.filters(), cmd, @"
success: true
exit_code: 0
----- stdout -----
{
\"servers\": [],
\"env_dev_token\": \"active\"
}
----- stderr -----
");
let output = context
.command()
.env("FABRO_DEV_TOKEN", DEV_TOKEN)
.args(["auth", "status", "--json"])
.output()
.expect("auth status should run");
assert!(output.status.success());
let value: Value = serde_json::from_slice(&output.stdout).expect("status JSON should parse");
assert!(value["servers"].as_array().unwrap().is_empty());
assert!(value.get("env_dev_token").is_none());
}

View file

@ -6,6 +6,7 @@
use std::fs;
use std::time::Duration;
use fabro_client::ServerTarget;
use fabro_test::{fabro_snapshot, test_context};
use insta::assert_snapshot;
@ -13,7 +14,7 @@ use super::support::{
local_dev_token, server_target, setup_completed_dry_run, setup_seeded_completed_dry_run,
setup_seeded_created_dry_run,
};
use crate::support::{LightweightCli, unique_run_id};
use crate::support::{LightweightCli, seed_dev_token_auth, unique_run_id};
#[test]
fn help() {
@ -51,6 +52,12 @@ fn dump_accepts_server_target_from_separate_home() {
let cli = LightweightCli::new();
let output_dir = context.temp_dir.join("remote-export");
let server = server_target(&context.storage_dir);
if let Some(dev_token) = local_dev_token(&context.storage_dir) {
let target = server
.parse::<ServerTarget>()
.expect("server target should parse");
seed_dev_token_auth(cli.home(), &target, &dev_token);
}
let mut cmd = cli.command();
cmd.args([
@ -61,9 +68,6 @@ fn dump_accepts_server_target_from_separate_home() {
output_dir.to_str().unwrap(),
&run.run_id,
]);
if let Some(dev_token) = local_dev_token(&context.storage_dir) {
cmd.env("FABRO_DEV_TOKEN", dev_token);
}
let output = cmd.output().expect("dump should execute");
assert!(

View file

@ -69,7 +69,7 @@ fn auth_status_ignores_json_output_format_from_home_config() {
);
let stderr = output_stderr(&output);
assert!(stderr.contains("Not logged in to any servers."));
assert!(stderr.contains("FABRO_DEV_TOKEN:"));
assert!(!stderr.contains("FABRO_DEV_TOKEN:"));
}
#[test]

View file

@ -1,3 +1,4 @@
use fabro_client::ServerTarget;
use fabro_config::{Storage, envfile};
use fabro_test::{fabro_snapshot, test_context};
use fabro_util::dev_token;
@ -7,7 +8,7 @@ use serde_json::Value;
use super::support::{
local_dev_token, setup_seeded_completed_dry_run, setup_seeded_created_dry_run,
};
use crate::support::{fatal_error_line, unique_run_id};
use crate::support::{fatal_error_line, seed_dev_token_auth, unique_run_id};
const TEST_DEV_TOKEN: &str =
"fabro_dev_abababababababababababababababababababababababababababababababab";
@ -89,6 +90,7 @@ fn ps_explicit_local_tcp_server_target_requires_explicit_auth() {
let output = context
.command()
.env("FABRO_STORAGE_DIR", &storage_dir)
.env("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)
.args(["ps", "-a", "--json", "--server", &format!("http://{bind}")])
.output()
.expect("ps should run");
@ -102,7 +104,7 @@ fn ps_explicit_local_tcp_server_target_requires_explicit_auth() {
assert!(
!output.status.success(),
"ps against an explicit local TCP target should require explicit auth:\nstdout:\n{}\nstderr:\n{}",
"ps against an explicit local TCP target should ignore FABRO_DEV_TOKEN and require persisted auth:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
@ -139,10 +141,11 @@ fn ps_explicit_local_tcp_server_target_accepts_explicit_dev_token() {
.as_str()
.expect("bind should be present");
let token = local_dev_token(&storage_dir).expect("local dev token should exist");
let target = ServerTarget::http_url(format!("http://{bind}")).expect("bind should parse");
seed_dev_token_auth(&context.home_dir, &target, &token);
let output = context
.command()
.env("FABRO_DEV_TOKEN", &token)
.args(["ps", "-a", "--json", "--server", &format!("http://{bind}")])
.output()
.expect("ps should run");

View file

@ -12,6 +12,7 @@ use std::path::Path;
use std::process::{Child, ExitStatus, Output, Stdio};
use std::time::{Duration, Instant};
use fabro_client::ServerTarget;
use fabro_config::{Storage, envfile};
use fabro_store::EventEnvelope;
use fabro_test::{
@ -27,7 +28,7 @@ use super::support::{
command_log_text, find_run_dir, local_dev_token, output_stderr, run_events, run_state,
server_endpoint, server_target, wait_for_event_names, wait_for_status, write_gated_workflow,
};
use crate::support::unique_run_id;
use crate::support::{seed_dev_token_auth, unique_run_id};
const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
const LEAKED_WORKER_PARENT_TOKEN: &str = "leak-worker-parent-token";
@ -519,9 +520,10 @@ methods = ["dev-token"]
let run_id = unique_run_id();
let dev_token = local_dev_token(&storage_dir).expect("managed server should have a dev token");
let target = ServerTarget::unix_socket_path(&socket_path).expect("socket path should parse");
seed_dev_token_auth(&context.home_dir, &target, &dev_token);
let run_output = context
.run_cmd()
.env("FABRO_DEV_TOKEN", dev_token)
.args([
"--server",
socket_path.to_str().expect("socket path should be UTF-8"),

View file

@ -8,6 +8,7 @@
)]
use std::io::Read;
use std::path::Path;
use std::process::{Command, Output, Stdio};
use std::sync::{Arc, Mutex, mpsc};
use std::time::{Duration, Instant};
@ -17,6 +18,7 @@ use axum::extract::{Request, State as AxumState};
use axum::middleware::{self, Next};
use axum::response::Response as AxumResponse;
use chrono::{Duration as ChronoDuration, Utc};
use fabro_client::{AuthEntry, AuthStore, DevTokenEntry, ServerTarget};
use fabro_config::{RunLayer, ServerSettingsBuilder};
use fabro_server::auth::GithubEndpoints;
use fabro_server::ip_allowlist::IpAllowlistConfig;
@ -248,6 +250,18 @@ pub(crate) fn expire_saved_access_token(context: &TestContext, issuer: &str) {
.unwrap_or_else(|err| panic!("failed to write {}: {err}", path.display()));
}
pub(crate) fn seed_dev_token_auth(home_dir: &Path, target: &ServerTarget, token: &str) {
AuthStore::new(home_dir.join(".fabro/auth.json"))
.put(
target,
AuthEntry::DevToken(DevTokenEntry {
token: token.to_owned(),
logged_in_at: Utc::now(),
}),
)
.unwrap_or_else(|err| panic!("failed to seed dev-token auth: {err}"));
}
pub(crate) fn no_redirect_browser_client() -> fabro_http::HttpClient {
fabro_http::HttpClientBuilder::new()
.cookie_store(true)

View file

@ -4,7 +4,7 @@ mod auth_tokens;
use assert_cmd::Command;
pub(crate) use auth_harness::{
RealAuthHarness, TEST_DEV_TOKEN, complete_login_via_browser, expire_saved_access_token,
no_redirect_browser_client, run_detached, saved_auth_entry,
no_redirect_browser_client, run_detached, saved_auth_entry, seed_dev_token_auth,
};
pub(crate) use auth_tokens::{TEST_SESSION_SECRET, issue_test_github_jwt};
use fabro_store::EventEnvelope;
@ -60,6 +60,10 @@ impl LightweightCli {
}
}
pub(crate) fn home(&self) -> &std::path::Path {
self.home_dir.path()
}
#[expect(
clippy::disallowed_methods,
reason = "Lightweight CLI test harness reconstructs a minimal process env for subprocesses."