Skip to content

fix(cli): reject non-success HTTP responses #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andriypolanski wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(cli): reject non-success HTTP responses #903

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64ba58b
fix(cli): reject non-success HTTP responses
andriypolanski Jul 30, 2026
518a96f
run cargo fmt --all -- --check
andriypolanski Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 44 additions & 14 deletions crates/genie-ctl/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

use anyhow::{Context, Result};
use genie_common::config::{Config, ServiceProbeTarget, parse_service_probe_target};
use genie_common::http::{HttpResponseLimits, read_response};
use genie_common::probe::{ProbeTimeouts, probe_service_target};
use genie_core::skills::{
SkillLoader, SkillManifestAudit, find_manifest_sidecar, manifest_sidecar_candidates,
Expand All @@ -54,6 +55,7 @@ mod bfcl_import;

const GOVERNOR_SOCK: &str = "/run/geniepod/governor.sock";
const BFCL_LLM_DEFAULT_MAX_TOKENS: u32 = 160;
const CLI_MAX_HTTP_RESPONSE_BYTES: usize = 8 * 1024 * 1024;

fn load_core_addr() -> Result<String> {
Ok(Config::load()?.core_http_addr())
Expand Down Expand Up @@ -2885,23 +2887,19 @@ async fn http_post_inner(

async fn read_http_body(reader: tokio::net::tcp::OwnedReadHalf) -> Result<String> {
let mut buf_reader = BufReader::new(reader);
let mut body = String::new();
let mut in_body = false;

loop {
let mut line = String::new();
let n = buf_reader.read_line(&mut line).await?;
if n == 0 {
break;
}
if in_body {
body.push_str(&line);
} else if line.trim().is_empty() {
in_body = true;
let limits = HttpResponseLimits::with_body_cap(CLI_MAX_HTTP_RESPONSE_BYTES);
let response = read_response(&mut buf_reader, &limits)
.await
.map_err(|error| anyhow::anyhow!("invalid HTTP response: {error}"))?;
if !(200..300).contains(&response.status) {
let detail = response.body.trim().replace('\n', " ");
if detail.is_empty() {
anyhow::bail!("HTTP {}", response.status);
}
anyhow::bail!("HTTP {}: {}", response.status, detail);
Comment on lines +2894 to +2899

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Sanitize server-provided error text before formatting it.

Replacing only \n leaves embedded \r and ANSI escape sequences intact. A responding server can therefore forge or manipulate terminal/log output through the CLI diagnostic.

Proposed fix 
-        let detail = response.body.trim().replace('\n', " ");
+        let detail = response
+            .body
+            .chars()
+            .map(|ch| if ch.is_control() { ' ' } else { ch })
+            .collect::<String>()
+            .split_whitespace()
+            .collect::<Vec<_>>()
+            .join(" ");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if !(200..300).contains(&response.status) {
let detail = response.body.trim().replace('\n', " ");
if detail.is_empty() {
anyhow::bail!("HTTP {}", response.status);
}
anyhow::bail!("HTTP {}: {}", response.status, detail);
if !(200..300).contains(&response.status) {
let detail = response
.body
.chars()
.map(|ch| if ch.is_control() { ' ' } else { ch })
.collect::<String>()
.split_whitespace()
.collect::<Vec<_>>()
.join(" ");
if detail.is_empty() {
anyhow::bail!("HTTP {}", response.status);
}
anyhow::bail!("HTTP {}: {}", response.status, detail);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/genie-ctl/src/main.rs` around lines 2894 - 2899, Sanitize the
server-provided detail in the non-success response handling before passing it to
anyhow::bail!. Update the response.body processing in the visible status-check
block to remove carriage returns and ANSI escape sequences, while preserving the
existing trimming, empty-detail fallback, and HTTP status formatting behavior.

}

Ok(body.trim().to_string())
Ok(response.body.trim().to_string())
}

async fn governor_cmd(json: &str) -> Option<serde_json::Value> {
Expand Down Expand Up @@ -2942,6 +2940,38 @@ mod tests {
toml::from_str("").expect("default config should parse from empty TOML")
}

async fn serve_one_http_response(response: &'static str) -> String {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
tokio::spawn(async move {
let (mut stream, _) = listener.accept().await.unwrap();
stream.write_all(response.as_bytes()).await.unwrap();
});
addr.to_string()
}

#[tokio::test]
async fn http_get_returns_success_body() {
let addr = serve_one_http_response(
"HTTP/1.1 200 OK\r\nContent-Length: 11\r\nConnection: close\r\n\r\n{\"ok\":true}",
)
.await;

assert_eq!(http_get(&addr, "/test").await.unwrap(), r#"{"ok":true}"#);
}

#[tokio::test]
async fn http_get_rejects_error_status_with_response_detail() {
let addr = serve_one_http_response(
"HTTP/1.1 403 Forbidden\r\nContent-Length: 18\r\nConnection: close\r\n\r\n{\"error\":\"denied\"}",
)
.await;

let error = http_get(&addr, "/protected").await.unwrap_err().to_string();
assert!(error.contains("HTTP 403"), "got: {error}");
assert!(error.contains("denied"), "got: {error}");
}

#[test]
fn format_uptime_hm_floors_each_component() {
assert_eq!(format_uptime_hm(3000), "0h 50m");
Expand Down
Loading