From 52ba0ae1b477ccf8334312c9dd9875bcb3a4c43c Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Oct 2026 14:18:41 +0100 Subject: [PATCH 1/5] fix(update): send GitHub token on every API call (Refs terraphim/terraphim-ai#3428) `terraphim-agent update` failed with a GitHub 403 even with GITHUB_TOKEN set, while `check-update` in the same shell succeeded: the verified install path (`update_with_verification` -> `get_latest_release_info`) built its own self_update builder and never applied the token. The auto-check (`check_for_updates_auto`, used by the REPL, terraphim-cli, startup and scheduler checks) still hard-coded the old `terraphim-ai` repository. - Add a private `GitHubSource` that builds every self_update GitHub builder (repo, hyphenated asset name, version, token, API URL, install path), so call sites can no longer drift. - Default repo comes from new `DEFAULT_REPO_OWNER`/`DEFAULT_REPO_NAME` constants (terraphim/terraphim-clients); the auto-check uses them. - `github_token_from_env()`: GITHUB_TOKEN, then GH_TOKEN; blank = unset. - 403 wording: only suggest setting a token when none was sent. - `UpdaterConfig::with_github_api_url` for GitHub Enterprise mirrors and local test servers. - Tests run against a real local HTTP server (no mocks) and assert the endpoint and Authorization header for check, update, verified update and auto-check paths. - Fix the feature-gated agent test that still expected `terraphim-ai`. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit ab57d348b378457d012e23ad283ec0b8bc778b05) --- .../tests/update_functionality_tests.rs | 8 +- crates/terraphim_update/src/lib.rs | 542 ++++++++++++++---- 2 files changed, 440 insertions(+), 110 deletions(-) diff --git a/crates/terraphim_agent/tests/update_functionality_tests.rs b/crates/terraphim_agent/tests/update_functionality_tests.rs index 5c104abd..0536116e 100644 --- a/crates/terraphim_agent/tests/update_functionality_tests.rs +++ b/crates/terraphim_agent/tests/update_functionality_tests.rs @@ -150,13 +150,15 @@ async fn test_version_comparison_logic() { /// Test update configuration #[tokio::test] async fn test_updater_configuration() { - use terraphim_update::{TerraphimUpdater, UpdaterConfig}; + use terraphim_update::{ + DEFAULT_REPO_NAME, DEFAULT_REPO_OWNER, TerraphimUpdater, UpdaterConfig, + }; // Test default configuration let config = UpdaterConfig::new("terraphim-agent"); assert_eq!(config.bin_name, "terraphim-agent"); - assert_eq!(config.repo_owner, "terraphim"); - assert_eq!(config.repo_name, "terraphim-ai"); + assert_eq!(config.repo_owner, DEFAULT_REPO_OWNER); + assert_eq!(config.repo_name, DEFAULT_REPO_NAME); assert!(config.show_progress); diff --git a/crates/terraphim_update/src/lib.rs b/crates/terraphim_update/src/lib.rs index 92a595bd..cb21223b 100644 --- a/crates/terraphim_update/src/lib.rs +++ b/crates/terraphim_update/src/lib.rs @@ -30,6 +30,108 @@ use tracing::{error, info, warn}; // `terraphim_update::UpdateBackend` rather than `...::manifest::UpdateBackend`. pub use manifest::{ManifestConfig, ManifestError, ReleaseManifest, UpdateBackend}; +/// GitHub owner of the repository that publishes Terraphim release assets. +pub const DEFAULT_REPO_OWNER: &str = "terraphim"; +/// GitHub repository that publishes Terraphim release assets. +pub const DEFAULT_REPO_NAME: &str = "terraphim-clients"; + +/// Resolve a GitHub API token from the environment: `GITHUB_TOKEN`, then +/// `GH_TOKEN` (the variable the `gh` CLI uses). Blank values count as unset. +pub fn github_token_from_env() -> Option { + first_token([ + std::env::var("GITHUB_TOKEN").ok(), + std::env::var("GH_TOKEN").ok(), + ]) +} + +/// First candidate that is non-empty after trimming, trimmed. +fn first_token(candidates: [Option; N]) -> Option { + candidates + .into_iter() + .flatten() + .map(|token| token.trim().to_string()) + .find(|token| !token.is_empty()) +} + +/// Where and how to reach GitHub Releases. +/// +/// Every `self_update` GitHub builder is created through +/// [`GitHubSource::builder`], so the repository and auth token cannot drift +/// between call sites (terraphim-ai#3428: `update` used to drop the token and +/// fail with 403 while `check-update` succeeded). +#[derive(Clone)] +struct GitHubSource { + repo_owner: String, + repo_name: String, + auth_token: Option, + /// API base URL override; `None` means `https://api.github.com`. + api_url: Option, +} + +impl GitHubSource { + fn from_config(config: &UpdaterConfig) -> Self { + Self { + repo_owner: config.repo_owner.clone(), + repo_name: config.repo_name.clone(), + auth_token: config.auth_token.clone(), + api_url: config.github_api_url.clone(), + } + } + + /// The default release repository, authenticated from the environment. + fn from_env() -> Self { + Self { + repo_owner: DEFAULT_REPO_OWNER.to_string(), + repo_name: DEFAULT_REPO_NAME.to_string(), + auth_token: github_token_from_env(), + api_url: None, + } + } + + /// A `self_update` GitHub builder with repository, binary name, version, + /// auth token, API URL and install path applied. + fn builder( + &self, + bin_name: &str, + current_version: &str, + ) -> Result { + let mut builder = self_update::backends::github::Update::configure(); + builder.repo_owner(&self.repo_owner); + builder.repo_name(&self.repo_name); + // Release assets use hyphenated names (terraphim_agent -> terraphim-agent) + builder.bin_name(&bin_name.replace('_', "-")); + builder.current_version(current_version); + if let Some(token) = &self.auth_token { + builder.auth_token(token); + } + if let Some(url) = &self.api_url { + builder.with_url(url); + } + // Install path keeps the original (possibly underscored) name + builder.bin_install_path(platform::get_binary_path(bin_name)?); + Ok(builder) + } + + /// Operator-facing message for a GitHub API error. Only suggests setting + /// a token when none was sent. + fn describe_error(&self, error: &str) -> String { + if !error.contains("403") { + return error.to_string(); + } + if self.auth_token.is_some() { + format!( + "GitHub API returned 403 although an auth token was sent (rate limit reached, or the token is invalid or lacks access). Error: {}", + error + ) + } else { + format!( + "GitHub API rate limit exceeded. Set GITHUB_TOKEN (or GH_TOKEN) for authenticated access (higher rate limit). Error: {}", + error + ) + } + } +} + /// Represents the status of an update operation #[derive(Debug, Clone)] pub enum UpdateStatus { @@ -161,8 +263,13 @@ pub struct UpdaterConfig { /// Configuration for the manifest (R2) backend. pub manifest: manifest::ManifestConfig, /// Optional GitHub auth token, forwarded to the GitHub fallback backend to - /// avoid rate limiting. Picked up from `GITHUB_TOKEN` by [`Self::new`]. + /// avoid rate limiting. Picked up from `GITHUB_TOKEN` or `GH_TOKEN` by + /// [`Self::new`]. pub auth_token: Option, + /// GitHub API base URL for the fallback backend; `None` means + /// `https://api.github.com`. Set for a GitHub Enterprise mirror, or a + /// local test server. + pub github_api_url: Option, /// Runtime update policy (Gitea #247): whether self-update is safe, or /// whether the running binary is package-managed and self-update must be /// refused. Resolved via [`policy::detect_update_policy_default`] by @@ -178,7 +285,7 @@ impl UpdaterConfig { /// base url = `https://downloads.terraphim.ai`. The backend and base url /// can be overridden at runtime via the `TERRAPHIM_UPDATE_BACKEND` /// (`r2`|`github`) and `TERRAPHIM_UPDATE_BASE_URL` env vars, and the - /// GitHub fallback picks up `GITHUB_TOKEN` automatically. + /// GitHub fallback picks up `GITHUB_TOKEN` (or `GH_TOKEN`) automatically. /// /// **IMPORTANT**: The default version returned by this constructor is the /// version of the `terraphim_update` library crate, NOT the version of the @@ -210,18 +317,14 @@ impl UpdaterConfig { let policy = policy::detect_update_policy_default(); Self { bin_name: bin, - repo_owner: "terraphim".to_string(), - // FIX: releases live on terraphim-clients, not terraphim-ai. - repo_name: "terraphim-clients".to_string(), + repo_owner: DEFAULT_REPO_OWNER.to_string(), + repo_name: DEFAULT_REPO_NAME.to_string(), current_version: cargo_crate_version!().to_string(), show_progress: true, backend, manifest, - // FIX: pick up GITHUB_TOKEN so the fallback backend isn't rate-limited. - auth_token: std::env::var("GITHUB_TOKEN") - .ok() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()), + auth_token: github_token_from_env(), + github_api_url: None, policy, } } @@ -260,6 +363,12 @@ impl UpdaterConfig { self } + /// Override the GitHub API base URL used by the fallback backend. + pub fn with_github_api_url(mut self, url: impl Into) -> Self { + self.github_api_url = Some(url.into()); + self + } + /// Explicitly inject the update policy, overriding the automatic /// [`policy::detect_update_policy_default`] resolution done by /// [`Self::new`]. Used by production call sites that need to @@ -540,31 +649,16 @@ impl TerraphimUpdater { ); // Clone data for the blocking task - let repo_owner = self.config.repo_owner.clone(); - let repo_name = self.config.repo_name.clone(); + let source = GitHubSource::from_config(&self.config); let bin_name = self.config.bin_name.clone(); let current_version = self.config.current_version.clone(); let show_progress = self.config.show_progress; - let auth_token = self.config.auth_token.clone(); // Move self_update operations to a blocking task to avoid runtime conflicts let result = tokio::task::spawn_blocking(move || { - // Normalize binary name for asset lookup (underscores to hyphens) - let bin_name_for_asset = bin_name.replace('_', "-"); - // Check if update is available - let mut builder = self_update::backends::github::Update::configure(); - builder.repo_owner(&repo_owner); - builder.repo_name(&repo_name); - builder.bin_name(&bin_name_for_asset); // Use hyphenated name for asset lookup - builder.current_version(¤t_version); + let mut builder = source.builder(&bin_name, ¤t_version)?; builder.show_download_progress(show_progress); - if let Some(token) = &auth_token { - builder.auth_token(token); - } - - // Set custom install path to preserve underscore naming - builder.bin_install_path(platform::get_binary_path(&bin_name)?); match builder.build() { Ok(updater) => { @@ -587,17 +681,14 @@ impl TerraphimUpdater { Err(e) => Err(e), } } - Err(e) => { - let err_msg = e.to_string(); - if err_msg.contains("403") { - Ok(UpdateStatus::Failed(format!( - "GitHub API rate limit exceeded. Set GITHUB_TOKEN env var for authenticated access (higher rate limit). Error: {}", - err_msg - ))) - } else { - Ok(UpdateStatus::Failed(format!("Check failed: {}", err_msg))) - } - } + Err(e) => { + let err_msg = e.to_string(); + if err_msg.contains("403") { + Ok(UpdateStatus::Failed(source.describe_error(&err_msg))) + } else { + Ok(UpdateStatus::Failed(format!("Check failed: {}", err_msg))) + } + } } } Err(e) => Ok(UpdateStatus::Failed(format!("Configuration error: {}", e))), @@ -688,12 +779,10 @@ impl TerraphimUpdater { ); // Clone data for the blocking task - let repo_owner = self.config.repo_owner.clone(); - let repo_name = self.config.repo_name.clone(); + let source = GitHubSource::from_config(&self.config); let bin_name = self.config.bin_name.clone(); let current_version = self.config.current_version.clone(); let show_progress = self.config.show_progress; - let auth_token = self.config.auth_token.clone(); // Decode every trusted embedded public key for signature verification // (key-rotation aware: an archive signed with any trusted key passes). @@ -720,23 +809,10 @@ impl TerraphimUpdater { // Move self_update operations to a blocking task to avoid runtime conflicts let result = tokio::task::spawn_blocking(move || { - // Normalize binary name for asset lookup (underscores to hyphens) - let bin_name_for_asset = bin_name.replace('_', "-"); - // Build the updater with signature verification enabled - let mut builder = self_update::backends::github::Update::configure(); - builder.repo_owner(&repo_owner); - builder.repo_name(&repo_name); - builder.bin_name(&bin_name_for_asset); // Use hyphenated name for asset lookup - builder.current_version(¤t_version); + let mut builder = source.builder(&bin_name, ¤t_version)?; builder.show_download_progress(show_progress); builder.verifying_keys(verifying_keys.clone()); // Enable signature verification - if let Some(token) = &auth_token { - builder.auth_token(token); - } - - // Set custom install path to preserve underscore naming - builder.bin_install_path(platform::get_binary_path(&bin_name)?); match builder.build() { Ok(updater) => match updater.update() { @@ -838,8 +914,7 @@ impl TerraphimUpdater { ); // Clone data for the blocking task - let repo_owner = self.config.repo_owner.clone(); - let repo_name = self.config.repo_name.clone(); + let source = GitHubSource::from_config(&self.config); let bin_name = self.config.bin_name.clone(); let current_version = self.config.current_version.clone(); let show_progress = self.config.show_progress; @@ -847,8 +922,7 @@ impl TerraphimUpdater { // Move self_update operations to a blocking task let result = tokio::task::spawn_blocking(move || { Self::update_with_verification_blocking( - &repo_owner, - &repo_name, + &source, &bin_name, ¤t_version, show_progress, @@ -891,8 +965,7 @@ impl TerraphimUpdater { /// Blocking version of update_with_verification for use in spawn_blocking fn update_with_verification_blocking( - repo_owner: &str, - repo_name: &str, + source: &GitHubSource, bin_name: &str, current_version: &str, show_progress: bool, @@ -903,23 +976,22 @@ impl TerraphimUpdater { ); // Step 1: Get latest release info from GitHub - let release = - match Self::get_latest_release_info(repo_owner, repo_name, bin_name, current_version) { - Ok(release) => release, - Err(e) => { - return Ok(UpdateStatus::Failed(format!( - "Failed to get release info: {}", - e - ))); - } - }; + let release = match Self::get_latest_release_info(source, bin_name, current_version) { + Ok(release) => release, + Err(e) => { + return Ok(UpdateStatus::Failed(format!( + "Failed to get release info: {}", + source.describe_error(&e.to_string()) + ))); + } + }; let latest_version = &release.version; // Step 2: Download archive to temp location let (_temp_dir, archive_path) = match Self::download_release_archive( - repo_owner, - repo_name, + &source.repo_owner, + &source.repo_name, bin_name, latest_version, show_progress, @@ -987,29 +1059,16 @@ impl TerraphimUpdater { /// Get latest release info from GitHub fn get_latest_release_info( - repo_owner: &str, - repo_name: &str, + source: &GitHubSource, bin_name: &str, current_version: &str, ) -> Result { info!( "Fetching latest release info for {}/{}", - repo_owner, repo_name + source.repo_owner, source.repo_name ); - // Normalize binary name for asset lookup (underscores to hyphens) - let bin_name_for_asset = bin_name.replace('_', "-"); - - let mut builder = self_update::backends::github::Update::configure(); - builder.repo_owner(repo_owner); - builder.repo_name(repo_name); - builder.bin_name(&bin_name_for_asset); // Use hyphenated name for asset lookup - builder.current_version(current_version); - - // Set custom install path to preserve underscore naming - builder.bin_install_path(platform::get_binary_path(bin_name)?); - - let updater = builder.build()?; + let updater = source.builder(bin_name, current_version)?.build()?; let release = updater.get_latest_release()?; @@ -1487,6 +1546,7 @@ pub async fn check_for_updates_auto_with_policy( current_version, policy, policy::detect_update_policy_default, + GitHubSource::from_env(), ) .await } @@ -1496,6 +1556,7 @@ async fn check_for_updates_auto_with_policy_and_detector( current_version: &str, policy: &policy::UpdatePolicy, detect_policy: D, + source: GitHubSource, ) -> Result where D: FnOnce() -> policy::UpdatePolicy, @@ -1540,22 +1601,7 @@ where let current_version = current_version.to_string(); let result = tokio::task::spawn_blocking(move || { - // Normalize binary name for asset lookup (underscores to hyphens) - let bin_name_for_asset = bin_name.replace('_', "-"); - - let mut builder = self_update::backends::github::Update::configure(); - builder.repo_owner("terraphim"); - builder.repo_name("terraphim-ai"); - builder.bin_name(&bin_name_for_asset); - builder.current_version(¤t_version); - - if let Ok(token) = std::env::var("GITHUB_TOKEN") { - builder.auth_token(&token); - } - - builder.bin_install_path(platform::get_binary_path(&bin_name)?); - - match builder.build() { + match source.builder(&bin_name, ¤t_version)?.build() { Ok(updater) => match updater.get_latest_release() { Ok(release) => { let latest_version = release.version.clone(); @@ -1571,7 +1617,10 @@ where Err(e) => Err(e), } } - Err(e) => Ok(UpdateStatus::Failed(format!("Check failed: {}", e))), + Err(e) => Ok(UpdateStatus::Failed(format!( + "Check failed: {}", + source.describe_error(&e.to_string()) + ))), }, Err(e) => Ok(UpdateStatus::Failed(format!("Configuration error: {}", e))), } @@ -1838,7 +1887,7 @@ mod tests { assert_eq!(config.repo_name, "terraphim-clients"); assert_eq!(config.backend, UpdateBackend::R2); assert_eq!(config.manifest.base_url, "https://downloads.terraphim.ai"); - assert!(config.auth_token.is_none(), "no GITHUB_TOKEN in test env"); + assert_eq!(config.auth_token, github_token_from_env()); } #[test] @@ -2087,6 +2136,7 @@ mod tests { manager: crate::policy::PackageManager::Pacman, update_command: "sudo pacman -Syu".to_string(), }, + GitHubSource::from_env(), ) .await .expect("check should short-circuit"); @@ -2107,6 +2157,7 @@ mod tests { update_command: "sudo pacman -Syu".to_string(), }, || panic!("detector must not run for an injected PackageManaged policy"), + GitHubSource::from_env(), ) .await .expect("check should short-circuit"); @@ -2117,6 +2168,283 @@ mod tests { ); } + // --- GitHub source: token, repository and error wording (terraphim-ai#3428) --- + + const TEST_TOKEN: &str = "test-token-3428"; + const LATEST_RELEASE_PATH: &str = "/repos/terraphim/terraphim-clients/releases/latest"; + + /// A real local HTTP server that answers every request with one fixed + /// status and body, and records each request head (lowercased). + struct RecordingServer { + url: String, + requests: std::sync::Arc>>, + } + + impl RecordingServer { + fn new(status: u16, body: &str) -> Self { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind"); + let url = format!("http://{}", listener.local_addr().expect("addr")); + let requests = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let recorded = requests.clone(); + let body = body.to_string(); + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(mut stream) = stream else { break }; + let mut head = Vec::new(); + let mut buf = [0u8; 1024]; + while !head.windows(4).any(|w| w == b"\r\n\r\n") { + match stream.read(&mut buf) { + Ok(0) | Err(_) => break, + Ok(n) => head.extend_from_slice(&buf[..n]), + } + } + recorded + .lock() + .unwrap() + .push(String::from_utf8_lossy(&head).to_lowercase()); + let response = format!( + "HTTP/1.1 {status} X\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let _ = stream.write_all(response.as_bytes()); + } + }); + Self { url, requests } + } + + fn requests(&self) -> Vec { + self.requests.lock().unwrap().clone() + } + } + + fn release_json(tag: &str) -> String { + format!( + r#"{{"tag_name":"{tag}","name":"{tag}","created_at":"2026-10-04T22:20:41Z","assets":[]}}"# + ) + } + + fn local_source(server: &RecordingServer, token: Option<&str>) -> GitHubSource { + GitHubSource { + repo_owner: DEFAULT_REPO_OWNER.to_string(), + repo_name: DEFAULT_REPO_NAME.to_string(), + auth_token: token.map(str::to_string), + api_url: Some(server.url.clone()), + } + } + + fn assert_single_release_request(server: &RecordingServer, token: Option<&str>) { + assert_single_request(server, &format!("{LATEST_RELEASE_PATH} "), token); + } + + /// Exactly one API request was made, its path starts with `path_prefix`, + /// and it carried `token` (or no auth header at all). + fn assert_single_request(server: &RecordingServer, path_prefix: &str, token: Option<&str>) { + let requests = server.requests(); + assert_eq!(requests.len(), 1, "expected one API request: {requests:?}"); + let request = &requests[0]; + assert!( + request.starts_with(&format!("get {path_prefix}")), + "wrong endpoint: {request}" + ); + match token { + Some(token) => assert!( + request.contains(&format!("authorization: token {token}")), + "token not sent: {request}" + ), + None => assert!( + !request.contains("authorization:"), + "unexpected auth header: {request}" + ), + } + } + + #[test] + fn verified_update_release_lookup_sends_auth_token() { + let server = RecordingServer::new(200, &release_json("v9.9.9")); + + let release = TerraphimUpdater::get_latest_release_info( + &local_source(&server, Some(TEST_TOKEN)), + "terraphim_agent", + "1.0.0", + ) + .expect("release info"); + + assert_eq!(release.version, "9.9.9"); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + + #[test] + fn release_lookup_without_token_sends_no_auth_header() { + let server = RecordingServer::new(200, &release_json("v9.9.9")); + + TerraphimUpdater::get_latest_release_info( + &local_source(&server, None), + "terraphim-agent", + "1.0.0", + ) + .expect("release info"); + + assert_single_release_request(&server, None); + } + + #[test] + fn verified_update_403_with_token_does_not_suggest_setting_one() { + let server = RecordingServer::new(403, r#"{"message":"API rate limit exceeded"}"#); + + let status = TerraphimUpdater::update_with_verification_blocking( + &local_source(&server, Some(TEST_TOKEN)), + "terraphim-agent", + "1.0.0", + false, + ) + .expect("status"); + + let UpdateStatus::Failed(message) = status else { + panic!("expected Failed, got {status:?}"); + }; + assert!( + message.starts_with("Failed to get release info: "), + "{message}" + ); + assert!(message.contains("auth token was sent"), "{message}"); + assert!(!message.contains("Set GITHUB_TOKEN"), "{message}"); + assert!(!message.contains(TEST_TOKEN), "token leaked: {message}"); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + + #[tokio::test] + async fn auto_check_queries_release_repo_with_token() { + let server = RecordingServer::new(200, &release_json("v9.9.9")); + + let status = check_for_updates_auto_with_policy_and_detector( + "terraphim-agent", + "1.0.0", + &crate::policy::UpdatePolicy::SelfManaged, + || crate::policy::UpdatePolicy::SelfManaged, + local_source(&server, Some(TEST_TOKEN)), + ) + .await + .expect("status"); + + assert!( + matches!(&status, UpdateStatus::Available { latest_version, .. } if latest_version == "9.9.9"), + "expected 9.9.9 available, got {status:?}" + ); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + + /// GitHub-backend config pointing at `server`, with the token supplied via + /// config (as `UpdaterConfig::new` does from `GITHUB_TOKEN`). + fn github_config(server: &RecordingServer) -> UpdaterConfig { + UpdaterConfig::new("terraphim-agent") + .with_version("1.0.0") + .with_backend(UpdateBackend::GitHub) + .with_github_api_url(server.url.clone()) + .with_auth_token(TEST_TOKEN) + .with_progress(false) + .with_policy(crate::policy::UpdatePolicy::SelfManaged) + } + + #[tokio::test] + async fn check_update_sends_config_token() { + let server = RecordingServer::new(200, &release_json("v9.9.9")); + + let status = TerraphimUpdater::new(github_config(&server)) + .check_update() + .await + .expect("status"); + + assert!( + matches!(&status, UpdateStatus::Available { latest_version, .. } if latest_version == "9.9.9"), + "expected 9.9.9 available, got {status:?}" + ); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + + /// The reported bug: `check-update` authenticated but the verified + /// `update` path dropped the token and failed with 403. + #[tokio::test] + async fn update_with_verification_sends_config_token() { + let server = RecordingServer::new(403, r#"{"message":"API rate limit exceeded"}"#); + + let status = TerraphimUpdater::new(github_config(&server)) + .update_with_verification() + .await + .expect("status"); + + let UpdateStatus::Failed(message) = status else { + panic!("expected Failed, got {status:?}"); + }; + assert!(message.contains("auth token was sent"), "{message}"); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + + #[tokio::test] + async fn update_sends_config_token() { + let server = RecordingServer::new(403, r#"{"message":"API rate limit exceeded"}"#); + + let status = TerraphimUpdater::new(github_config(&server)) + .update() + .await + .expect("status"); + + assert!( + matches!(status, UpdateStatus::Failed(_)), + "expected Failed, got {status:?}" + ); + // self_update's install path lists releases rather than asking for /latest + assert_single_request( + &server, + "/repos/terraphim/terraphim-clients/releases", + Some(TEST_TOKEN), + ); + } + + #[test] + fn github_source_from_config_carries_repo_and_token() { + let config = UpdaterConfig::new("terraphim-agent").with_auth_token(TEST_TOKEN); + + let source = GitHubSource::from_config(&config); + + assert_eq!(source.repo_owner, DEFAULT_REPO_OWNER); + assert_eq!(source.repo_name, DEFAULT_REPO_NAME); + assert_eq!(source.auth_token.as_deref(), Some(TEST_TOKEN)); + assert!(source.api_url.is_none()); + } + + #[test] + fn first_token_prefers_first_non_blank_and_trims() { + let s = |v: &str| Some(v.to_string()); + assert_eq!(first_token([s(" a "), s("b")]), s("a")); + assert_eq!(first_token([None, s("gh")]), s("gh")); + assert_eq!(first_token([s(" "), s("gh")]), s("gh")); + assert_eq!(first_token::<2>([None, None]), None); + } + + #[test] + fn describe_error_only_suggests_token_when_none_sent() { + let mut source = GitHubSource::from_env(); + let forbidden = "api request failed with status: 403"; + + source.auth_token = None; + assert!( + source + .describe_error(forbidden) + .contains("Set GITHUB_TOKEN") + ); + + source.auth_token = Some(TEST_TOKEN.to_string()); + let with_token = source.describe_error(forbidden); + assert!(with_token.contains("auth token was sent")); + assert!(!with_token.contains("Set GITHUB_TOKEN")); + + assert_eq!( + source.describe_error("connection refused"), + "connection refused" + ); + } + #[test] fn managed_status_injects_detector_without_global_test_state() { let status = TerraphimUpdater::managed_status_with_detector( From 8ee184e2d64890fc762ff736600454d2ad17e0b0 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Oct 2026 14:21:45 +0100 Subject: [PATCH 2/5] test(update): guard auto-check repo default and check 403 wording (Refs terraphim/terraphim-ai#3428) Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 9e72917d0e82213fdf4b87cf6b8fa0007efa2e0b) --- crates/terraphim_update/src/lib.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/terraphim_update/src/lib.rs b/crates/terraphim_update/src/lib.rs index cb21223b..4066c03b 100644 --- a/crates/terraphim_update/src/lib.rs +++ b/crates/terraphim_update/src/lib.rs @@ -2362,6 +2362,22 @@ mod tests { assert_single_release_request(&server, Some(TEST_TOKEN)); } + #[tokio::test] + async fn check_update_403_with_token_reports_token_was_sent() { + let server = RecordingServer::new(403, r#"{"message":"Bad credentials"}"#); + + let status = TerraphimUpdater::new(github_config(&server)) + .check_update() + .await + .expect("status"); + + let UpdateStatus::Failed(message) = status else { + panic!("expected Failed, got {status:?}"); + }; + assert!(message.contains("auth token was sent"), "{message}"); + assert_single_release_request(&server, Some(TEST_TOKEN)); + } + /// The reported bug: `check-update` authenticated but the verified /// `update` path dropped the token and failed with 403. #[tokio::test] @@ -2413,6 +2429,18 @@ mod tests { assert!(source.api_url.is_none()); } + /// The auto-check path builds its source from the environment; it must + /// target the release repo, not the old `terraphim-ai` one. + #[test] + fn github_source_from_env_targets_release_repo() { + let source = GitHubSource::from_env(); + + assert_eq!(source.repo_owner, "terraphim"); + assert_eq!(source.repo_name, "terraphim-clients"); + assert_eq!(source.auth_token, github_token_from_env()); + assert!(source.api_url.is_none()); + } + #[test] fn first_token_prefers_first_non_blank_and_trims() { let s = |v: &str| Some(v.to_string()); From c518198627296d40c4dddf8ddeae17a4d055006c Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Oct 2026 14:29:48 +0100 Subject: [PATCH 3/5] docs(update): github_api_url only affects API lookups, not asset downloads (Refs terraphim/terraphim-ai#3428) Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 596e34091c777a56ce3f2624f343deb4512534ac) --- crates/terraphim_update/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/terraphim_update/src/lib.rs b/crates/terraphim_update/src/lib.rs index 4066c03b..23f00ed3 100644 --- a/crates/terraphim_update/src/lib.rs +++ b/crates/terraphim_update/src/lib.rs @@ -266,9 +266,9 @@ pub struct UpdaterConfig { /// avoid rate limiting. Picked up from `GITHUB_TOKEN` or `GH_TOKEN` by /// [`Self::new`]. pub auth_token: Option, - /// GitHub API base URL for the fallback backend; `None` means - /// `https://api.github.com`. Set for a GitHub Enterprise mirror, or a - /// local test server. + /// GitHub API base URL for the fallback backend's release lookups; `None` + /// means `https://api.github.com`. Only API calls use it: the verified + /// update still downloads assets from `https://github.com/.../releases/download`. pub github_api_url: Option, /// Runtime update policy (Gitea #247): whether self-update is safe, or /// whether the running binary is package-managed and self-update must be @@ -363,7 +363,8 @@ impl UpdaterConfig { self } - /// Override the GitHub API base URL used by the fallback backend. + /// Override the GitHub API base URL used for release lookups (see + /// [`Self::github_api_url`]; asset downloads are unaffected). pub fn with_github_api_url(mut self, url: impl Into) -> Self { self.github_api_url = Some(url.into()); self From 4db4f4267d82be05043ad9edbc04b60898f96bfe Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Oct 2026 22:22:07 +0100 Subject: [PATCH 4/5] chore: bump workspace version to 1.21.18 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 14 +++++++------- Cargo.toml | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 01badd55..fd4e672e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6315,7 +6315,7 @@ dependencies = [ [[package]] name = "terraphim-cli" -version = "1.21.17" +version = "1.21.18" dependencies = [ "anyhow", "assert_cmd", @@ -6356,7 +6356,7 @@ dependencies = [ [[package]] name = "terraphim-session-analyzer" -version = "1.21.17" +version = "1.21.18" dependencies = [ "aho-corasick", "anyhow", @@ -6395,7 +6395,7 @@ dependencies = [ [[package]] name = "terraphim_agent" -version = "1.21.17" +version = "1.21.18" dependencies = [ "ahash", "anyhow", @@ -6580,7 +6580,7 @@ dependencies = [ [[package]] name = "terraphim_grep" -version = "1.21.17" +version = "1.21.18" dependencies = [ "anyhow", "async-trait", @@ -6609,7 +6609,7 @@ dependencies = [ [[package]] name = "terraphim_hooks" -version = "1.21.17" +version = "1.21.18" dependencies = [ "dirs 5.0.1", "serde", @@ -6623,7 +6623,7 @@ dependencies = [ [[package]] name = "terraphim_lsp" -version = "1.21.17" +version = "1.21.18" dependencies = [ "log", "serde", @@ -6713,7 +6713,7 @@ dependencies = [ [[package]] name = "terraphim_negative_contribution" -version = "1.21.17" +version = "1.21.18" dependencies = [ "log", "terraphim_automata", diff --git a/Cargo.toml b/Cargo.toml index f055683e..4a6c42bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ members = [ ] [workspace.package] -version = "1.21.17" +version = "1.21.18" edition = "2024" authors = ["Terraphim Team "] documentation = "https://terraphim.ai" From f94b8927ad4156b0aa2f178671a5f67b0cb623c7 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Oct 2026 22:28:56 +0100 Subject: [PATCH 5/5] test(release): track workspace version 1.21.18 in the producer contract test_release_uses_checked_in_version_and_never_mutates_source pins the checked-in workspace version; move it with the 1.21.18 bump. Co-Authored-By: Claude Opus 5.5 --- tests/test_release_binaries_workflow_contract.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_release_binaries_workflow_contract.py b/tests/test_release_binaries_workflow_contract.py index e6e0465f..6a97ac59 100644 --- a/tests/test_release_binaries_workflow_contract.py +++ b/tests/test_release_binaries_workflow_contract.py @@ -176,7 +176,7 @@ def test_preflight_recursively_peels_to_exact_expected_sha(self) -> None: def test_release_uses_checked_in_version_and_never_mutates_source(self) -> None: text = workflow_text() workspace = tomllib.loads((ROOT / "Cargo.toml").read_text()) - self.assertEqual(workspace["workspace"]["package"]["version"], "1.21.17") + self.assertEqual(workspace["workspace"]["package"]["version"], "1.21.18") for forbidden in ( "Set release version", "set_section_version",