Merge pull request #367 from fettpl/fix/348-http-header-sanitization
fix(tools): use original headers for HTTP requests, redact only in display
This commit is contained in:
commit
703e701e31
3 changed files with 62 additions and 37 deletions
14
src/main.rs
14
src/main.rs
|
|
@ -272,20 +272,6 @@ enum ModelCommands {
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Subcommand, Debug)]
|
|
||||||
enum ModelCommands {
|
|
||||||
/// Refresh and cache provider models
|
|
||||||
Refresh {
|
|
||||||
/// Provider name (defaults to configured default provider)
|
|
||||||
#[arg(long)]
|
|
||||||
provider: Option<String>,
|
|
||||||
|
|
||||||
/// Force live refresh and ignore fresh cache
|
|
||||||
#[arg(long)]
|
|
||||||
force: bool,
|
|
||||||
},
|
|
||||||
}
|
|
||||||
|
|
||||||
#[derive(Subcommand, Debug)]
|
#[derive(Subcommand, Debug)]
|
||||||
enum ChannelCommands {
|
enum ChannelCommands {
|
||||||
/// List configured channels
|
/// List configured channels
|
||||||
|
|
|
||||||
|
|
@ -555,7 +555,6 @@ impl Tool for GitOperationsTool {
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::security::SecurityPolicy;
|
use crate::security::SecurityPolicy;
|
||||||
use std::path::Path;
|
|
||||||
use tempfile::TempDir;
|
use tempfile::TempDir;
|
||||||
|
|
||||||
fn test_tool(dir: &std::path::Path) -> GitOperationsTool {
|
fn test_tool(dir: &std::path::Path) -> GitOperationsTool {
|
||||||
|
|
|
||||||
|
|
@ -76,28 +76,37 @@ impl HttpRequestTool {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn sanitize_headers(&self, headers: &serde_json::Value) -> Vec<(String, String)> {
|
fn parse_headers(&self, headers: &serde_json::Value) -> Vec<(String, String)> {
|
||||||
let mut result = Vec::new();
|
let mut result = Vec::new();
|
||||||
if let Some(obj) = headers.as_object() {
|
if let Some(obj) = headers.as_object() {
|
||||||
for (key, value) in obj {
|
for (key, value) in obj {
|
||||||
if let Some(str_val) = value.as_str() {
|
if let Some(str_val) = value.as_str() {
|
||||||
// Redact sensitive headers from logs (we don't log headers, but this is defense-in-depth)
|
|
||||||
let is_sensitive = key.to_lowercase().contains("authorization")
|
|
||||||
|| key.to_lowercase().contains("api-key")
|
|
||||||
|| key.to_lowercase().contains("apikey")
|
|
||||||
|| key.to_lowercase().contains("token")
|
|
||||||
|| key.to_lowercase().contains("secret");
|
|
||||||
if is_sensitive {
|
|
||||||
result.push((key.clone(), "***REDACTED***".into()));
|
|
||||||
} else {
|
|
||||||
result.push((key.clone(), str_val.to_string()));
|
result.push((key.clone(), str_val.to_string()));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
result
|
result
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn redact_headers_for_display(headers: &[(String, String)]) -> Vec<(String, String)> {
|
||||||
|
headers
|
||||||
|
.iter()
|
||||||
|
.map(|(key, value)| {
|
||||||
|
let lower = key.to_lowercase();
|
||||||
|
let is_sensitive = lower.contains("authorization")
|
||||||
|
|| lower.contains("api-key")
|
||||||
|
|| lower.contains("apikey")
|
||||||
|
|| lower.contains("token")
|
||||||
|
|| lower.contains("secret");
|
||||||
|
if is_sensitive {
|
||||||
|
(key.clone(), "***REDACTED***".into())
|
||||||
|
} else {
|
||||||
|
(key.clone(), value.clone())
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
async fn execute_request(
|
async fn execute_request(
|
||||||
&self,
|
&self,
|
||||||
url: &str,
|
url: &str,
|
||||||
|
|
@ -222,10 +231,10 @@ impl Tool for HttpRequestTool {
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let sanitized_headers = self.sanitize_headers(&headers_val);
|
let request_headers = self.parse_headers(&headers_val);
|
||||||
|
|
||||||
match self
|
match self
|
||||||
.execute_request(&url, method, sanitized_headers, body)
|
.execute_request(&url, method, request_headers, body)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(response) => {
|
Ok(response) => {
|
||||||
|
|
@ -600,23 +609,54 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn sanitize_headers_redacts_sensitive() {
|
fn parse_headers_preserves_original_values() {
|
||||||
let tool = test_tool(vec!["example.com"]);
|
let tool = test_tool(vec!["example.com"]);
|
||||||
let headers = json!({
|
let headers = json!({
|
||||||
"Authorization": "Bearer secret",
|
"Authorization": "Bearer secret",
|
||||||
"Content-Type": "application/json",
|
"Content-Type": "application/json",
|
||||||
"X-API-Key": "my-key"
|
"X-API-Key": "my-key"
|
||||||
});
|
});
|
||||||
let sanitized = tool.sanitize_headers(&headers);
|
let parsed = tool.parse_headers(&headers);
|
||||||
assert_eq!(sanitized.len(), 3);
|
assert_eq!(parsed.len(), 3);
|
||||||
assert!(sanitized
|
assert!(parsed
|
||||||
.iter()
|
.iter()
|
||||||
.any(|(k, v)| k == "Authorization" && v == "***REDACTED***"));
|
.any(|(k, v)| k == "Authorization" && v == "Bearer secret"));
|
||||||
assert!(sanitized
|
assert!(parsed
|
||||||
.iter()
|
.iter()
|
||||||
.any(|(k, v)| k == "X-API-Key" && v == "***REDACTED***"));
|
.any(|(k, v)| k == "X-API-Key" && v == "my-key"));
|
||||||
assert!(sanitized
|
assert!(parsed
|
||||||
.iter()
|
.iter()
|
||||||
.any(|(k, v)| k == "Content-Type" && v == "application/json"));
|
.any(|(k, v)| k == "Content-Type" && v == "application/json"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn redact_headers_for_display_redacts_sensitive() {
|
||||||
|
let headers = vec![
|
||||||
|
("Authorization".into(), "Bearer secret".into()),
|
||||||
|
("Content-Type".into(), "application/json".into()),
|
||||||
|
("X-API-Key".into(), "my-key".into()),
|
||||||
|
("X-Secret-Token".into(), "tok-123".into()),
|
||||||
|
];
|
||||||
|
let redacted = HttpRequestTool::redact_headers_for_display(&headers);
|
||||||
|
assert_eq!(redacted.len(), 4);
|
||||||
|
assert!(redacted
|
||||||
|
.iter()
|
||||||
|
.any(|(k, v)| k == "Authorization" && v == "***REDACTED***"));
|
||||||
|
assert!(redacted
|
||||||
|
.iter()
|
||||||
|
.any(|(k, v)| k == "X-API-Key" && v == "***REDACTED***"));
|
||||||
|
assert!(redacted
|
||||||
|
.iter()
|
||||||
|
.any(|(k, v)| k == "X-Secret-Token" && v == "***REDACTED***"));
|
||||||
|
assert!(redacted
|
||||||
|
.iter()
|
||||||
|
.any(|(k, v)| k == "Content-Type" && v == "application/json"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn redact_headers_does_not_alter_original() {
|
||||||
|
let headers = vec![("Authorization".into(), "Bearer real-token".into())];
|
||||||
|
let _ = HttpRequestTool::redact_headers_for_display(&headers);
|
||||||
|
assert_eq!(headers[0].1, "Bearer real-token");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue