Skip to content

Commit ee1ad36

Browse files
haasonsaasclaude
andcommitted
Fix Opus code review findings: config race condition, error logging, CORS warnings
- Fix race condition in update_config: build response under write lock instead of dropping lock then re-reading (avoids stale data) - Log config file rename errors instead of silently ignoring with let _ = - Log CORS origin parse failures as warnings for debuggability - Harden Ollama sidecar security context: drop all capabilities Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ae32ade commit ee1ad36

File tree

4 files changed

+25
-12
lines changed

4 files changed

+25
-12
lines changed

charts/diffscope/templates/deployment.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ spec:
157157
- name: ollama
158158
securityContext:
159159
allowPrivilegeEscalation: false
160+
runAsNonRoot: false
161+
capabilities:
162+
drop:
163+
- ALL
160164
image: "{{ .Values.ollama.image.repository }}:{{ .Values.ollama.image.tag }}"
161165
imagePullPolicy: {{ .Values.ollama.image.pullPolicy }}
162166
ports:

src/server/api.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -457,19 +457,19 @@ pub async fn update_config(
457457

458458
*config = new_config;
459459
config.normalize();
460-
drop(config);
461460

462-
// Persist config to disk
463-
AppState::save_config_async(&state);
464-
465-
// Return updated config (redacted)
466-
let config = state.config.read().await;
461+
// Build response while still holding the write lock
467462
let mut result = serde_json::to_value(&*config).unwrap_or_default();
468463
if let Some(obj) = result.as_object_mut() {
469464
if obj.contains_key("api_key") {
470465
obj.insert("api_key".to_string(), serde_json::json!("***"));
471466
}
472467
}
473468

469+
drop(config);
470+
471+
// Persist config to disk
472+
AppState::save_config_async(&state);
473+
474474
Ok(Json(result))
475475
}

src/server/mod.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,21 @@ async fn serve_embedded(uri: axum::http::Uri) -> Response {
6161
pub async fn start_server(config: Config, host: &str, port: u16) -> anyhow::Result<()> {
6262
let state = Arc::new(state::AppState::new(config)?);
6363

64-
let allowed_origins: Vec<axum::http::HeaderValue> = [
64+
let origin_strings = [
6565
format!("http://localhost:{}", port),
6666
format!("http://127.0.0.1:{}", port),
6767
"http://localhost:5173".to_string(),
68-
]
69-
.iter()
70-
.filter_map(|s| s.parse().ok())
71-
.collect();
68+
];
69+
let allowed_origins: Vec<axum::http::HeaderValue> = origin_strings
70+
.iter()
71+
.filter_map(|s| match s.parse() {
72+
Ok(v) => Some(v),
73+
Err(e) => {
74+
eprintln!("Warning: failed to parse CORS origin '{}': {}", s, e);
75+
None
76+
}
77+
})
78+
.collect();
7279

7380
let cors = CorsLayer::new()
7481
.allow_origin(allowed_origins)

src/server/state.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ impl AppState {
134134
eprintln!("Failed to write config: {}", e);
135135
return;
136136
}
137-
let _ = tokio::fs::rename(&tmp_path, &state.config_path).await;
137+
if let Err(e) = tokio::fs::rename(&tmp_path, &state.config_path).await {
138+
eprintln!("Failed to rename config file: {}", e);
139+
}
138140
});
139141
}
140142

0 commit comments

Comments
 (0)