diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index bc46cdb..c4f07e1 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -120,6 +120,11 @@ jobs: shell: bash env: KEENABLE_API_KEY: ${{ secrets.KEENABLE_API_KEY }} + # Tag every request from the suite as e2e traffic via the + # X-Keenable-Title header (logged as app_title), so first-party CI + # calls can be told apart from real CLI users (app_title=keenable-cli) + # in Grafana. The CLI reads KEENABLE_APP_TITLE; the daemon inherits it. + KEENABLE_APP_TITLE: keenable-cli-e2e # Empty on Linux (full suite); "not semantic and not latency" on the # macOS/Windows legs so the OS-independent live-index checks run once. MARKERS: ${{ matrix.markers }} diff --git a/src/api.rs b/src/api.rs index d2bacfb..117e266 100644 --- a/src/api.rs +++ b/src/api.rs @@ -5,6 +5,38 @@ use crate::constants::API_BASE_URL; const USER_AGENT: &str = concat!("keenable-cli/", env!("CARGO_PKG_VERSION")); +/// Default `X-Keenable-Title` value. The backend requires this header on +/// token-less (public) endpoints and records it as `app_title` for +/// observability. Sending it always keeps the unauthenticated flow working and +/// makes first-party CLI traffic attributable in dashboards. +const DEFAULT_APP_TITLE: &str = "keenable-cli"; + +/// Resolve the app title from an optional env value, falling back to the +/// default. Override via `KEENABLE_APP_TITLE` to separate first-party +/// automation (the e2e suite sets `keenable-cli-e2e`) from real CLI users. +/// Pure so it can be unit-tested without touching the process environment. +fn resolve_app_title(env_value: Option) -> String { + env_value + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| DEFAULT_APP_TITLE.to_string()) +} + +fn app_title() -> String { + resolve_app_title(std::env::var("KEENABLE_APP_TITLE").ok()) +} + +/// Headers common to every keenable API client. Carries `X-Keenable-Title`, +/// which is mandatory on public endpoints. A non-parseable override is dropped +/// rather than panicking — the default is always a valid header value. +fn base_headers() -> reqwest::header::HeaderMap { + let mut headers = reqwest::header::HeaderMap::new(); + if let Ok(value) = app_title().parse() { + headers.insert("X-Keenable-Title", value); + } + headers +} + /// Structured API error matching the backend's `{error, message, retryAfter}` format. pub struct ApiError { pub status: u16, @@ -81,7 +113,7 @@ pub async fn validate_api_key(api_key: &str) -> KeyCheck { } pub fn api_key_client(api_key: &str) -> Client { - let mut headers = reqwest::header::HeaderMap::new(); + let mut headers = base_headers(); // Keys from --api-key or a hand-edited config may carry stray whitespace // or control chars; a bad header value must yield a 401, not a panic. if let Ok(value) = api_key.trim().parse() { @@ -98,6 +130,7 @@ pub fn api_key_client(api_key: &str) -> Client { pub fn bare_client() -> Client { Client::builder() .user_agent(USER_AGENT) + .default_headers(base_headers()) .timeout(std::time::Duration::from_secs(60)) .build() .unwrap() @@ -184,6 +217,25 @@ mod tests { assert!(!err(500, "boom").is_auth_error()); } + #[test] + fn app_title_defaults_when_env_absent_or_blank() { + assert_eq!(resolve_app_title(None), "keenable-cli"); + assert_eq!(resolve_app_title(Some("".into())), "keenable-cli"); + assert_eq!(resolve_app_title(Some(" ".into())), "keenable-cli"); + } + + #[test] + fn app_title_uses_trimmed_override() { + assert_eq!( + resolve_app_title(Some("keenable-cli-e2e".into())), + "keenable-cli-e2e" + ); + assert_eq!( + resolve_app_title(Some(" custom-app ".into())), + "custom-app" + ); + } + #[test] fn display_joins_error_and_message() { let mut e = err(500, "Server error"); diff --git a/tests/e2e/test_free_tier.py b/tests/e2e/test_free_tier.py index 4688963..cfc4b81 100644 --- a/tests/e2e/test_free_tier.py +++ b/tests/e2e/test_free_tier.py @@ -10,6 +10,12 @@ the env), so a fresh home means a genuinely unauthenticated request. Public endpoints are IP-rate-limited, so each test tolerates a 429 carrying the free-tier login hint instead of asserting results unconditionally. + +Public endpoints also require the X-Keenable-Title header (the CLI sends it on +every request, logged server-side as app_title). A token-less call must +therefore never fail with "Missing app identifier" — if it does, the CLI has +stopped sending the header and the free-tier flow is broken, so the tests +assert against that error explicitly. """ import json @@ -25,6 +31,9 @@ def test_free_tier_search_hits_public_endpoint(kn_fresh): else: data = res.yaml() assert "error" in data + # The CLI sends the mandatory X-Keenable-Title header, so the only + # tolerated failure is an IP rate-limit — never a missing app identifier. + assert data["error"] != "Missing app identifier", data # Unauthenticated 429 invites login to raise limits (the authenticated # wording is "switch accounts" instead). hint = data.get("hint", "") @@ -39,6 +48,7 @@ def test_free_tier_fetch_hits_public_endpoint(kn_fresh): else: data = res.yaml() assert "error" in data + assert data["error"] != "Missing app identifier", data assert "keenable login" in data.get("hint", "") diff --git a/tests/e2e/test_login_flow.py b/tests/e2e/test_login_flow.py index 2a80663..b3bfc1d 100644 --- a/tests/e2e/test_login_flow.py +++ b/tests/e2e/test_login_flow.py @@ -85,6 +85,9 @@ def test_logout_clears_key_and_falls_back_to_public(logged_in): else: data = res.yaml() assert "error" in data + # The CLI still sends X-Keenable-Title after logout, so the public call + # must not be rejected for a missing app identifier (see test_free_tier). + assert data["error"] != "Missing app identifier", data assert "keenable login" in data.get("hint", ""), data