mirror of
https://github.com/katanemo/plano.git
synced 2026-06-11 15:05:14 +02:00
fix: address code review feedback (log warnings, redact Redis URL credentials, add doc comment)
Agent-Logs-Url: https://github.com/katanemo/plano/sessions/6a75240b-4578-409d-b8c7-eff47dba8a03 Co-authored-by: adilhafeez <13196462+adilhafeez@users.noreply.github.com>
This commit is contained in:
parent
66f8230dd5
commit
f1e2876a8b
3 changed files with 44 additions and 6 deletions
|
|
@ -53,6 +53,29 @@ fn parse_semver(version: &str) -> (u32, u32, u32) {
|
|||
(major, minor, patch)
|
||||
}
|
||||
|
||||
/// Redact the userinfo (username:password) from a URL string for safe logging.
|
||||
///
|
||||
/// `redis://user:secret@localhost:6379` → `redis://<redacted>@localhost:6379`
|
||||
///
|
||||
/// URLs without credentials are returned unchanged.
|
||||
fn redact_url_credentials(url: &str) -> String {
|
||||
// Find "://" and then look for "@" before any "/" in the authority.
|
||||
if let Some(after_scheme) = url.find("://").map(|i| i + 3) {
|
||||
let authority = &url[after_scheme..];
|
||||
if let Some(at_pos) = authority.find('@') {
|
||||
// Only redact if the '@' comes before any path separator.
|
||||
if !authority[..at_pos].contains('/') {
|
||||
return format!(
|
||||
"{}<redacted>@{}",
|
||||
&url[..after_scheme],
|
||||
&authority[at_pos + 1..]
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
url.to_string()
|
||||
}
|
||||
|
||||
/// CORS pre-flight response for the models endpoint.
|
||||
fn cors_preflight() -> Result<Response<BoxBody<Bytes, hyper::Error>>, hyper::Error> {
|
||||
let mut response = Response::new(empty());
|
||||
|
|
@ -412,8 +435,10 @@ async fn init_session_cache(
|
|||
.and_then(|c| c.url.as_deref())
|
||||
.ok_or("session_cache.url is required for redis session cache")?;
|
||||
|
||||
// Log only the host portion to avoid exposing credentials in plain-text logs.
|
||||
let display_url = redact_url_credentials(url);
|
||||
debug!(url = %url, "redis session cache connection");
|
||||
info!(cache_type = "redis", url = %url, "initializing session cache");
|
||||
info!(cache_type = "redis", url = %display_url, "initializing session cache");
|
||||
|
||||
Arc::new(
|
||||
RedisSessionCache::new(url, ttl_secs)
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use std::collections::HashMap;
|
|||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use tokio::sync::RwLock;
|
||||
use tracing::info;
|
||||
use tracing::{debug, info, warn};
|
||||
|
||||
/// A cached routing decision stored by session ID.
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
|
|
@ -156,16 +156,27 @@ impl SessionCache for RedisSessionCache {
|
|||
}
|
||||
|
||||
async fn put(&self, session_id: &str, route: CachedRoute) {
|
||||
let Ok(json) = serde_json::to_string(&route) else {
|
||||
return;
|
||||
let json = match serde_json::to_string(&route) {
|
||||
Ok(j) => j,
|
||||
Err(e) => {
|
||||
warn!(session_id = %session_id, error = %e, "failed to serialize CachedRoute for Redis");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let mut conn = self.conn.write().await;
|
||||
let _: redis::RedisResult<()> = conn.set_ex(session_id, json, self.ttl_secs).await;
|
||||
if let Err(e) = conn
|
||||
.set_ex::<_, _, ()>(session_id, json, self.ttl_secs)
|
||||
.await
|
||||
{
|
||||
warn!(session_id = %session_id, error = %e, "failed to write session cache entry to Redis");
|
||||
}
|
||||
}
|
||||
|
||||
async fn remove(&self, session_id: &str) {
|
||||
let mut conn = self.conn.write().await;
|
||||
let _: redis::RedisResult<()> = conn.del(session_id).await;
|
||||
if let Err(e) = conn.del::<_, ()>(session_id).await {
|
||||
debug!(session_id = %session_id, error = %e, "failed to delete session cache entry from Redis");
|
||||
}
|
||||
}
|
||||
|
||||
/// Redis handles expiry natively — this is a no-op.
|
||||
|
|
|
|||
|
|
@ -18,6 +18,8 @@ pub enum SessionCacheType {
|
|||
pub struct SessionCacheConfig {
|
||||
#[serde(rename = "type")]
|
||||
pub cache_type: SessionCacheType,
|
||||
/// Redis connection URL (e.g. `redis://localhost:6379`).
|
||||
/// Required when `cache_type` is [`SessionCacheType::Redis`]; ignored otherwise.
|
||||
pub url: Option<String>,
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue