fix: CVE notifications during scan + help chat doc loading + Dockerfile #55
@@ -25,7 +25,7 @@ uuid = { workspace = true }
|
||||
secrecy = { workspace = true }
|
||||
regex = { workspace = true }
|
||||
axum = "0.8"
|
||||
tower-http = { version = "0.6", features = ["cors", "trace"] }
|
||||
tower-http = { version = "0.6", features = ["cors", "trace", "set-header"] }
|
||||
git2 = "0.20"
|
||||
octocrab = "0.44"
|
||||
tokio-cron-scheduler = "0.13"
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use axum::http::HeaderValue;
|
||||
use axum::{middleware, Extension};
|
||||
use tokio::sync::RwLock;
|
||||
use tower_http::cors::CorsLayer;
|
||||
use tower_http::set_header::SetResponseHeaderLayer;
|
||||
use tower_http::trace::TraceLayer;
|
||||
|
||||
use crate::agent::ComplianceAgent;
|
||||
@@ -14,7 +16,24 @@ pub async fn start_api_server(agent: ComplianceAgent, port: u16) -> Result<(), A
|
||||
let mut app = routes::build_router()
|
||||
.layer(Extension(Arc::new(agent.clone())))
|
||||
.layer(CorsLayer::permissive())
|
||||
.layer(TraceLayer::new_for_http());
|
||||
.layer(TraceLayer::new_for_http())
|
||||
// Security headers (defense-in-depth, primary enforcement via Traefik)
|
||||
|
|
||||
.layer(SetResponseHeaderLayer::overriding(
|
||||
axum::http::header::STRICT_TRANSPORT_SECURITY,
|
||||
HeaderValue::from_static("max-age=31536000; includeSubDomains"),
|
||||
sharang
commented
[medium] Deeply nested middleware layering in API server The API server setup creates deeply nested middleware layers (4+ levels) with security headers being added through SetResponseHeaderLayer calls. While this isn't inherently wrong, the pattern of adding multiple header layers sequentially increases the chance of ordering dependencies being misunderstood during future modifications. Suggested fix: Consider grouping related security headers together in a single middleware layer or using a builder pattern to make the intent clearer and reduce the visual nesting depth. *Scanner: code-review/complexity | * **[medium] Deeply nested middleware layering in API server**
The API server setup creates deeply nested middleware layers (4+ levels) with security headers being added through SetResponseHeaderLayer calls. While this isn't inherently wrong, the pattern of adding multiple header layers sequentially increases the chance of ordering dependencies being misunderstood during future modifications.
Suggested fix: Consider grouping related security headers together in a single middleware layer or using a builder pattern to make the intent clearer and reduce the visual nesting depth.
*Scanner: code-review/complexity | *
<!-- compliance-fp:e798830824894b5925dd637b0aee1bb13c3cbbb8b0c9bfe31670ffb2bf4a6382 -->
sharang
commented
[medium] Deeply nested middleware layering in API server The API server setup creates deeply nested middleware layers (4+ levels) with security headers being added through SetResponseHeaderLayer calls. While this isn't inherently wrong, the pattern of adding multiple header layers sequentially increases the chance of ordering dependencies being misunderstood during future modifications. Suggested fix: Consider grouping related security headers together in a single middleware layer or using a builder pattern to make the intent clearer and reduce the visual nesting depth. *Scanner: code-review/complexity | * **[medium] Deeply nested middleware layering in API server**
The API server setup creates deeply nested middleware layers (4+ levels) with security headers being added through SetResponseHeaderLayer calls. While this isn't inherently wrong, the pattern of adding multiple header layers sequentially increases the chance of ordering dependencies being misunderstood during future modifications.
Suggested fix: Consider grouping related security headers together in a single middleware layer or using a builder pattern to make the intent clearer and reduce the visual nesting depth.
*Scanner: code-review/complexity | *
<!-- compliance-fp:e798830824894b5925dd637b0aee1bb13c3cbbb8b0c9bfe31670ffb2bf4a6382 -->
|
||||
))
|
||||
.layer(SetResponseHeaderLayer::overriding(
|
||||
|
sharang
commented
[low] Potential panic in security header configuration The security header configuration uses Suggested fix: Consider using *Scanner: code-review/convention | * **[low] Potential panic in security header configuration**
The security header configuration uses `HeaderValue::from_static()` which can only accept string literals known at compile time. If any of the header values were to be constructed dynamically, this would cause a compilation error. While currently safe, this pattern makes the code less flexible for future modifications.
Suggested fix: Consider using `HeaderValue::from_str()` for dynamic values or ensure all header values remain compile-time constants to maintain safety.
*Scanner: code-review/convention | *
<!-- compliance-fp:636dcbdcbcb853a5757ce79c40e14a22fe93b0a58e7bc81c706e172f0a61f8f9 -->
sharang
commented
[low] Potential panic in security header configuration The security header configuration uses Suggested fix: Consider using *Scanner: code-review/convention | * **[low] Potential panic in security header configuration**
The security header configuration uses `HeaderValue::from_static()` which can only accept string literals known at compile time. If any of the header values were to be constructed dynamically, this would cause a compilation error. While currently safe, this pattern makes the code less flexible for future modifications.
Suggested fix: Consider using `HeaderValue::from_str()` for dynamic values or ensure all header values remain compile-time constants to maintain safety.
*Scanner: code-review/convention | *
<!-- compliance-fp:636dcbdcbcb853a5757ce79c40e14a22fe93b0a58e7bc81c706e172f0a61f8f9 -->
sharang
commented
[medium] Potential panic in security headers layer setup The security headers are set using Suggested fix: Consider using *Scanner: code-review/convention | * **[medium] Potential panic in security headers layer setup**
The security headers are set using `HeaderValue::from_static` which can panic at runtime if the header values are invalid. While the values appear correct, this pattern could be fragile if configuration changes are made later without proper validation.
Suggested fix: Consider using `HeaderValue::try_from` with proper error handling instead of `from_static` to prevent potential panics during header value construction.
*Scanner: code-review/convention | *
<!-- compliance-fp:b87da311777ac8e85d01e46443bb47c79442d5c67c9d26f4eae3cd92249c5ed3 -->
|
||||
axum::http::header::X_FRAME_OPTIONS,
|
||||
HeaderValue::from_static("DENY"),
|
||||
))
|
||||
.layer(SetResponseHeaderLayer::overriding(
|
||||
axum::http::header::X_CONTENT_TYPE_OPTIONS,
|
||||
HeaderValue::from_static("nosniff"),
|
||||
))
|
||||
.layer(SetResponseHeaderLayer::overriding(
|
||||
axum::http::header::REFERRER_POLICY,
|
||||
HeaderValue::from_static("strict-origin-when-cross-origin"),
|
||||
));
|
||||
|
||||
if let (Some(kc_url), Some(kc_realm)) =
|
||||
(&agent.config.keycloak_url, &agent.config.keycloak_realm)
|
||||
|
||||
[medium] Multiple interleaved responsibilities in API server layer configuration
The API server configuration in
start_api_servermixes security header configuration with tracing and CORS layers. This creates a single function responsible for multiple orthogonal concerns (tracing, CORS, security headers), which increases coupling and makes it harder to reason about the impact of changes to any one aspect.Suggested fix: Separate the layer configuration into distinct sections or extract security header configuration into its own function to improve modularity and reduce the cognitive load when reviewing changes to individual middleware components.
*Scanner: code-review/complexity | *