[Detail Bug] CLI auth errors misreport access token source when env var uses lowercase/mixed case

[detail-app]

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_1e9efd2b-fad4-45e4-8a83-ed9fa7d4abd6

Summary

• Context: The access_token_source function in cli/src/config.rs was recently added to determine whether an access token came from an environment variable or config file, so that authentication error messages can provide helpful guidance about where to update the token.
• Bug: The function incorrectly identifies the token source as "config file" when lowercase or mixed-case environment variable names are used (e.g., s2_access_token, S2_access_token), even though the token actually came from the environment.
• Actual vs. expected: The function only checks for the uppercase S2_ACCESS_TOKEN environment variable, but the config loading logic accepts environment variables case-insensitively. When s2_access_token is set, the token is loaded from the environment, but access_token_source reports it came from the config file.
• Impact: Authentication errors display incorrect guidance, telling users to update their config file when they should actually update their environment variable. This causes confusion and makes debugging authentication issues more difficult.

Code with Bug

pub fn access_token_source(config: &CliConfig) -> Option<TokenSource> {
    if std::env::var_os("S2_ACCESS_TOKEN").is_some() {  // <-- BUG 🔴 Only checks uppercase variant; env loading is case-insensitive
        return Some(TokenSource::Environment);
    }

    if config.access_token.is_some() {
        Some(TokenSource::ConfigFile)  // <-- BUG 🔴 Returns ConfigFile even when token actually came from lowercase/mixed-case env var
    } else {
        None
    }
}

Related inconsistency with config loading:

pub fn load_cli_config() -> Result<CliConfig, CliConfigError> {
    let path = config_path()?;
    let mut builder = Config::builder();
    if path.exists() {
        builder = builder.add_source(config::File::new(
            path.to_str().expect("config path is valid utf8"),
            FileFormat::Toml,
        ));
    }
    builder = builder.add_source(config::Environment::with_prefix("S2")); // <-- Accepts case-insensitive env vars
    Ok(builder.build()?.try_deserialize::<CliConfig>()?)
}

Explanation

load_cli_config() uses the config crate’s Environment::with_prefix("S2"), which accepts environment variables case-insensitively (e.g., s2_access_token, S2_access_token). However, access_token_source() checks only std::env::var_os("S2_ACCESS_TOKEN"), which is case-sensitive. As a result, when the token is supplied via a lowercase/mixed-case environment variable, the CLI correctly loads it from the environment, but error reporting incorrectly labels the source as ConfigFile.

Codebase Inconsistency

The CLI’s auth error help text includes the token source, so misclassification directly produces misleading guidance:

#[error("{}: {}", .0, .1)]
#[diagnostic(help(
    "Verify the token loaded from {2} is valid and has permission for this operation, then retry.\n\\
     Update it with `s2 config set access_token <token>` or set `S2_ACCESS_TOKEN`."
))]
OperationWithTokenSource(OpKind, #[source] S2Error, TokenSource),

Recommended Fix

Check for S2_ACCESS_TOKEN case-insensitively to match config crate behavior (e.g., iterate std::env::vars_os() and use eq_ignore_ascii_case("S2_ACCESS_TOKEN")).

History

This bug was introduced in commit 480f293. The commit added a new access_token_source() function to provide better error messages by identifying whether an access token came from an environment variable or config file, but it used a case-sensitive std::env::var_os("S2_ACCESS_TOKEN") check that doesn't match the existing case-insensitive environment variable loading logic in load_cli_config() (which uses the config crate's Environment::with_prefix("S2")).