scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix(install): clear carried clippy warnings across install code paths

Browse source 
The web-install feature was carrying nine pedantic-tier clippy errors
from its initial commit. Fix them in place:

- \`install.rs\` \`InstallAppState\` switches \`install_token\`,
  \`storage_dir\`, and \`config_path\` from \`Arc<String>/Arc<PathBuf>\` to
  \`Arc<str>/Arc<Path>\` so we stop heap-duplicating buffers.
- Bring \`Infallible\`, \`axum::middleware\`, \`axum::extract::Request\`,
  and \`fabro_types::settings::SettingsLayer\` into scope instead of
  using absolute paths inline.
- Replace \`Duration::from_secs(10 * 60)\` with \`Duration::from_mins(10)\`.
- \`generate_ephemeral_secret\` never returns \`Err\`; drop the \`Result\`.
- \`server/start.rs ensure_storage_server_autostart_allowed\` takes
  \`Option<&OsStr>\` instead of consuming an \`OsString\` it only reads.
- \`server/mod.rs\` storage_dir fallback uses \`map_or_else\` to satisfy
  \`map_unwrap_or\`.

CI now passes \`cargo +nightly-2026-04-14 clippy --workspace
--all-targets -- -D warnings\` cleanly and the 892-test suite still
passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-19 14:13:10 -04:00
parent 0e1d66b137
commit 87dc7de140
No known key found for this signature in database
3 changed files with 32 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
lib/crates/fabro-cli/src/commands/server/mod.rs
View file

@ -181,9 +181,10 @@ fn maybe_install_bootstrap(
None => default_install_bind_request(),
};
let storage_dir = storage_dir
.map(std::path::Path::to_path_buf)
.unwrap_or_else(|| legacy_default_storage_root().join("storage"));
let storage_dir = storage_dir.map_or_else(
|| legacy_default_storage_root().join("storage"),
std::path::Path::to_path_buf,
);
Ok(Some(InstallBootstrap {
bind_request,

4
lib/crates/fabro-cli/src/commands/server/start.rs
View file

@ -54,7 +54,7 @@ pub(crate) async fn ensure_server_running_for_storage(
config_path: &Path,
) -> Result<Bind> {
ensure_storage_server_autostart_allowed(
std::env::var_os(FABRO_CONFIG_ENV),
std::env::var_os(FABRO_CONFIG_ENV).as_deref(),
config_path,
&default_settings_path(),
)?;
@ -143,7 +143,7 @@ async fn ensure_server_running_with_bind(
}
fn ensure_storage_server_autostart_allowed(
config_env: Option<std::ffi::OsString>,
config_env: Option<&std::ffi::OsStr>,
config_path: &Path,
default_settings_path: &Path,
) -> Result<()> {

58
lib/crates/fabro-server/src/install.rs
View file

@ -1,14 +1,15 @@
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::convert::Infallible;
use std::path::Path;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex, MutexGuard};
use std::time::{Duration, Instant};
use axum::extract::{OriginalUri, Query, State};
use axum::extract::{OriginalUri, Query, Request, State};
use axum::http::{HeaderMap, Method, StatusCode, header};
use axum::response::{IntoResponse, Response};
use axum::routing::{get, post, put};
use axum::{Json, Router};
use axum::{Json, Router, middleware};
use base64::Engine as _;
use base64::engine::general_purpose::{STANDARD as BASE64_STANDARD, URL_SAFE_NO_PAD};
use fabro_auth::{AuthCredential, AuthDetails, credential_id_for};
@ -20,6 +21,7 @@ use fabro_install::{
};
use fabro_model::Provider;
use fabro_store::ArtifactStore;
use fabro_types::settings::SettingsLayer;
use fabro_util::version::FABRO_VERSION;
use fabro_util::{Home, dev_token, session_secret};
use fabro_vault::SecretType as VaultSecretType;
@ -37,10 +39,10 @@ use crate::{security_headers, static_files};
#[derive(Clone)]
pub struct InstallAppState {
install_token: Arc<String>,
install_token: Arc<str>,
pending_install: Arc<Mutex<PendingInstall>>,
storage_dir: Arc<PathBuf>,
config_path: Arc<PathBuf>,
storage_dir: Arc<Path>,
config_path: Arc<Path>,
home: Option<Home>,
install_listen: Arc<Mutex<InstallListenConfig>>,
first_operator: Arc<Mutex<Option<InstallOperatorFingerprint>>>,
@ -71,10 +73,10 @@ impl InstallAppState {
#[must_use]
pub fn new(token: String, storage_dir: &Path, config_path: &Path) -> Self {
Self {
install_token: Arc::new(token),
install_token: Arc::from(token),
pending_install: Arc::new(Mutex::new(PendingInstall::default())),
storage_dir: Arc::new(storage_dir.to_path_buf()),
config_path: Arc::new(config_path.to_path_buf()),
storage_dir: Arc::from(storage_dir),
config_path: Arc::from(config_path),
home: None,
install_listen: Arc::new(Mutex::new(InstallListenConfig::Tcp(
DEFAULT_INSTALL_TCP_LISTEN_ADDRESS.to_string(),
@ -95,10 +97,10 @@ impl InstallAppState {
#[must_use]
pub fn for_test_with_paths(token: &str, storage_dir: &Path, config_path: &Path) -> Self {
Self {
install_token: Arc::new(token.to_string()),
install_token: Arc::from(token),
pending_install: Arc::new(Mutex::new(PendingInstall::default())),
storage_dir: Arc::new(storage_dir.to_path_buf()),
config_path: Arc::new(config_path.to_path_buf()),
storage_dir: Arc::from(storage_dir),
config_path: Arc::from(config_path),
home: None,
install_listen: Arc::new(Mutex::new(InstallListenConfig::Tcp(
DEFAULT_INSTALL_TCP_LISTEN_ADDRESS.to_string(),
@ -338,18 +340,18 @@ pub fn build_install_router(state: InstallAppState) -> Router {
)
.route("/install/finish", post(post_install_finish))
.with_state(state)
.fallback_service(service_fn(move |req: axum::extract::Request| async move {
.fallback_service(service_fn(move |req: Request| async move {
let path = req.uri().path().to_string();
if path.starts_with("/api/") {
Ok::<_, std::convert::Infallible>(StatusCode::NOT_FOUND.into_response())
Ok::<_, Infallible>(StatusCode::NOT_FOUND.into_response())
} else if matches!(req.method(), &Method::GET | &Method::HEAD) {
let headers = req.headers().clone();
Ok::<_, std::convert::Infallible>(static_files::serve_install(&path, &headers))
Ok::<_, Infallible>(static_files::serve_install(&path, &headers))
} else {
Ok::<_, std::convert::Infallible>(StatusCode::NOT_FOUND.into_response())
Ok::<_, Infallible>(StatusCode::NOT_FOUND.into_response())
}
}))
.layer(axum::middleware::from_fn(security_headers::layer))
.layer(middleware::from_fn(security_headers::layer))
}
struct InstallFinishGuard {
@ -656,12 +658,7 @@ async fn post_install_github_app_manifest(
);
}
let state_token = match generate_ephemeral_secret() {
Ok(token) => token,
Err(err) => {
return install_error_response(StatusCode::INTERNAL_SERVER_ERROR, err.to_string());
}
};
let state_token = generate_ephemeral_secret();
let manifest = build_github_app_manifest(
input.app_name.trim(),
&format!(
@ -677,7 +674,7 @@ async fn post_install_github_app_manifest(
owner: owner.clone(),
app_name: input.app_name.trim().to_string(),
allowed_username: input.allowed_username.trim().to_string(),
expires_at: now + Duration::from_secs(600),
expires_at: now + Duration::from_mins(10),
});
Json(serde_json::json!({
@ -747,10 +744,7 @@ async fn get_install_github_app_redirect(
info!(step = "github_app", "install step completed");
(StatusCode::FOUND, [(
header::LOCATION,
format!(
"/install/github/done?token={}",
state.install_token.as_str()
),
format!("/install/github/done?token={}", &*state.install_token),
)])
.into_response()
}
@ -978,7 +972,7 @@ fn token_is_valid(state: &InstallAppState, headers: &HeaderMap, query_token: Opt
]
.into_iter()
.flatten()
.any(|token| token == state.install_token.as_str())
.any(|token| token == &*state.install_token)
}
fn require_valid_token(
@ -1144,8 +1138,8 @@ fn validate_canonical_url(value: &str) -> Result<(), String> {
Ok(())
}
fn generate_ephemeral_secret() -> anyhow::Result<String> {
Ok(URL_SAFE_NO_PAD.encode(rand::random::<[u8; 32]>()))
fn generate_ephemeral_secret() -> String {
URL_SAFE_NO_PAD.encode(rand::random::<[u8; 32]>())
}
fn build_github_app_manifest(
@ -1304,7 +1298,7 @@ async fn exchange_github_app_manifest_code(
}
async fn write_artifact_store_metadata(
settings: &fabro_types::settings::SettingsLayer,
settings: &SettingsLayer,
storage_dir: &Path,
) -> anyhow::Result<()> {
use fabro_types::settings::interp::InterpString;