Skip to content

Commit a86345d

Browse files
committed
merge: security fixes, prompt optimization, test coverage, API hygiene
- Block project config base URL overrides to prevent SSRF/exfiltration - Cap streaming line_buffer at 1 MB in all LLM providers - Strip URLs from reqwest error messages - Fix subject budget to account for breaking change ! suffix - Omit EVIDENCE section when all flags are default - Replace emoji with text labels, add symbol marker legend - Remove duplicate JSON schema from system prompt - Broaden OpenAI secret pattern for service account keys - Replace Box::leak with Cow for custom secret pattern names - Enhance Python tree-sitter queries with decorated definitions - Demote SymbolChangeType, GitService, Progress to pub(crate) - Add tests for detect_primary_change, detect_metadata_breaking, detect_bug_evidence, Deleted/Renamed status, signature edge cases, classify_span_change None path, connection content assertions * fixes/deferred-issues: test: add Deleted/Renamed fixtures, classify_span_change None path, HARD LIMIT check fix(safety): fix OpenAI pattern to not match Anthropic keys, enhance Python queries refactor(api): demote internal types to pub(crate) test: add coverage for detect_primary_change, metadata_breaking, bug_evidence, connections fix(safety): broaden OpenAI pattern and replace Box::leak with Cow fix(llm): strip URLs from reqwest error messages refactor(prompt): optimize for small models and fix breaking change budget fix(security): block base URL overrides from project config and cap streaming buffers
2 parents f6e92cf + d9488c9 commit a86345d

15 files changed

Lines changed: 478 additions & 79 deletions

File tree

src/config.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,12 +296,16 @@ impl Config {
296296
config.api_key = None;
297297
}
298298
if table.contains_key("openai_base_url") {
299-
warn!("project .commitbee.toml overrides openai_base_url — verify this is trusted");
299+
warn!("project .commitbee.toml sets openai_base_url — blocked for security");
300+
config.openai_base_url = None;
300301
}
301302
if table.contains_key("anthropic_base_url") {
302-
warn!(
303-
"project .commitbee.toml overrides anthropic_base_url — verify this is trusted"
304-
);
303+
warn!("project .commitbee.toml sets anthropic_base_url — blocked for security");
304+
config.anthropic_base_url = None;
305+
}
306+
if table.contains_key("ollama_host") {
307+
warn!("project .commitbee.toml sets ollama_host — blocked for security");
308+
config.ollama_host = Config::default().ollama_host;
305309
}
306310
}
307311

src/domain/context.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,14 @@ impl PromptContext {
5858
)
5959
};
6060

61-
// Calculate available chars for subject after type(scope): prefix
61+
// Calculate available chars for subject after type(scope)[!]: prefix
6262
let prefix_len = self.suggested_type.as_str().len()
6363
+ self
6464
.suggested_scope
6565
.as_ref()
6666
.map(|s| s.len() + 2)
6767
.unwrap_or(0) // "(scope)"
68+
+ if self.public_api_removed_count > 0 { 1 } else { 0 } // "!" for breaking
6869
+ 2; // ": "
6970
let subject_budget = 72_usize.saturating_sub(prefix_len);
7071

@@ -107,7 +108,7 @@ impl PromptContext {
107108
String::new()
108109
} else {
109110
format!(
110-
"\n METADATA BREAKING CHANGES DETECTED:\n{}\n",
111+
"\nWARNING: METADATA BREAKING CHANGES DETECTED:\n{}\n",
111112
self.metadata_breaking_signals
112113
.iter()
113114
.map(|s| format!("- {}", s))
@@ -198,12 +199,22 @@ Respond with ONLY this JSON:
198199
}
199200

200201
format!(
201-
"\n PUBLIC API REMOVED — describe this in breaking_change field:\n {}\n",
202+
"\nWARNING: PUBLIC API REMOVED — describe this in breaking_change field:\n {}\n",
202203
self.public_api_removed.replace('\n', "\n ")
203204
)
204205
}
205206

206207
fn format_evidence_section(&self) -> String {
208+
// Skip when all flags are at default — saves ~200 chars for small models
209+
if !self.is_mechanical
210+
&& !self.has_bug_evidence
211+
&& self.public_api_removed_count == 0
212+
&& !self.has_new_public_api
213+
&& !self.is_dependency_only
214+
{
215+
return String::new();
216+
}
217+
207218
let yn = |b: bool| if b { "yes" } else { "no" };
208219

209220
format!(

src/domain/symbol.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ pub enum SymbolKind {
2121

2222
/// How a symbol was affected by the change.
2323
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
24+
#[non_exhaustive]
2425
#[allow(dead_code)]
25-
pub enum SymbolChangeType {
26+
pub(crate) enum SymbolChangeType {
2627
/// Symbol only exists in staged content (new symbol)
2728
Added,
2829
/// Symbol only exists in HEAD content (removed symbol)
@@ -57,7 +58,7 @@ impl CodeSymbol {
5758
/// Determine the change type for this symbol.
5859
#[must_use]
5960
#[allow(dead_code)]
60-
pub fn change_type(&self) -> SymbolChangeType {
61+
pub(crate) fn change_type(&self) -> SymbolChangeType {
6162
match (self.is_added, self.is_whitespace_only) {
6263
(true, None) => SymbolChangeType::Added,
6364
(false, None) => SymbolChangeType::Removed,

src/queries/python.scm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@
44

55
(function_definition name: (identifier) @name) @definition
66
(class_definition name: (identifier) @name) @definition
7+
(decorated_definition definition: (function_definition name: (identifier) @name)) @definition
8+
(decorated_definition definition: (class_definition name: (identifier) @name)) @definition

src/services/analyzer.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,4 +636,48 @@ mod tests {
636636
let sig = AnalyzerService::extract_signature(node, source);
637637
assert_eq!(sig.as_deref(), Some("pub trait Handler"));
638638
}
639+
640+
#[cfg(feature = "lang-rust")]
641+
#[test]
642+
fn extract_signature_single_line_function() {
643+
let source = "fn foo() {}\n";
644+
let mut parser = Parser::new();
645+
parser
646+
.set_language(&tree_sitter_rust::LANGUAGE.into())
647+
.unwrap();
648+
let tree = parser.parse(source, None).unwrap();
649+
let node = tree.root_node().child(0).unwrap();
650+
let sig = AnalyzerService::extract_signature(node, source);
651+
assert!(
652+
sig.is_some(),
653+
"single-line function should have a signature"
654+
);
655+
assert!(
656+
sig.as_ref().unwrap().contains("fn foo()"),
657+
"signature should contain fn foo(), got: {:?}",
658+
sig
659+
);
660+
}
661+
662+
#[cfg(feature = "lang-rust")]
663+
#[test]
664+
fn extract_signature_const_item_no_body() {
665+
let source = "pub const MAX: usize = 100;\n";
666+
let mut parser = Parser::new();
667+
parser
668+
.set_language(&tree_sitter_rust::LANGUAGE.into())
669+
.unwrap();
670+
let tree = parser.parse(source, None).unwrap();
671+
let node = tree.root_node().child(0).unwrap();
672+
let sig = AnalyzerService::extract_signature(node, source);
673+
assert!(
674+
sig.is_some(),
675+
"const item should have a signature (first-line fallback)"
676+
);
677+
assert!(
678+
sig.as_ref().unwrap().contains("MAX"),
679+
"signature should contain const name, got: {:?}",
680+
sig
681+
);
682+
}
639683
}

src/services/git.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use tokio::process::Command;
1111
use crate::domain::{ChangeStatus, DiffStats, FileCategory, FileChange, StagedChanges};
1212
use crate::error::{Error, Result};
1313

14-
pub struct GitService {
14+
pub(crate) struct GitService {
1515
repo: gix::Repository,
1616
work_dir: PathBuf,
1717
}

src/services/llm/anthropic.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ impl AnthropicProvider {
129129
} else {
130130
Error::Provider {
131131
provider: "anthropic".into(),
132-
message: e.to_string(),
132+
message: e.without_url().to_string(),
133133
}
134134
}
135135
})?;
@@ -160,11 +160,18 @@ impl AnthropicProvider {
160160

161161
let chunk = chunk.map_err(|e| Error::Provider {
162162
provider: "anthropic".into(),
163-
message: e.to_string(),
163+
message: e.without_url().to_string(),
164164
})?;
165165

166166
line_buffer.push_str(&String::from_utf8_lossy(&chunk));
167167

168+
if line_buffer.len() > MAX_RESPONSE_BYTES {
169+
return Err(Error::Provider {
170+
provider: "anthropic".into(),
171+
message: "line buffer exceeded 1 MB limit".into(),
172+
});
173+
}
174+
168175
while let Some(newline_pos) = line_buffer.find('\n') {
169176
// Parse from slice to avoid allocating a String per line
170177
let result = {

src/services/llm/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ Rules:
2929
- Never copy labels, field names, or evidence tags from the prompt into your output.
3030
- If public APIs are both added and removed, this is an API replacement (refactor), not a new feature.
3131
- When SYMBOLS CHANGED shows full signatures, reference the actual parameter/type names in your subject rather than generic descriptions.
32-
- When CONNECTIONS shows that a caller and callee both changed, mention the relationship in the body (e.g., "updates parse() signature and all call sites").
32+
- When CONNECTIONS shows that a caller and callee both changed, consider mentioning the relationship in the body.
33+
34+
Symbol markers: [+] added, [-] removed, [~] modified (signature changed).
3335
3436
Examples:
3537
GOOD: "replace path-only grouping with diff-shape fingerprinting"
@@ -43,8 +45,7 @@ BAD: "refactor code for better performance and add validation" — two concerns
4345
"[+] pub fn connect(host: &str, timeout: Duration) -> Result<Connection>"
4446
→ subject: "add connect function with host and timeout parameters"
4547
46-
Respond with ONLY the JSON object, nothing else:
47-
{"type":"<type>","scope":null,"subject":"<subject>","body":null,"breaking_change":null}
48+
Respond with ONLY the JSON object as specified in the user prompt.
4849
"#;
4950

5051
pub mod anthropic;

src/services/llm/ollama.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl OllamaProvider {
9090
} else {
9191
Error::Provider {
9292
provider: "ollama".into(),
93-
message: e.to_string(),
93+
message: e.without_url().to_string(),
9494
}
9595
}
9696
})?;
@@ -162,7 +162,7 @@ impl OllamaProvider {
162162
} else {
163163
Error::Provider {
164164
provider: "ollama".into(),
165-
message: e.to_string(),
165+
message: e.without_url().to_string(),
166166
}
167167
}
168168
})?;
@@ -197,12 +197,21 @@ impl OllamaProvider {
197197

198198
let chunk = chunk.map_err(|e| Error::Provider {
199199
provider: "ollama".into(),
200-
message: e.to_string(),
200+
message: e.without_url().to_string(),
201201
})?;
202202

203203
// Append chunk to buffer
204204
line_buffer.push_str(&String::from_utf8_lossy(&chunk));
205205

206+
// Cap line_buffer to prevent unbounded growth from servers
207+
// that send continuous bytes without newlines
208+
if line_buffer.len() > MAX_RESPONSE_BYTES {
209+
return Err(Error::Provider {
210+
provider: "ollama".into(),
211+
message: "line buffer exceeded 1 MB limit".into(),
212+
});
213+
}
214+
206215
// Process complete lines (newline-delimited JSON)
207216
while let Some(newline_pos) = line_buffer.find('\n') {
208217
// Parse from slice to avoid allocating a String per line

src/services/llm/openai.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl OpenAiProvider {
9393
.await
9494
.map_err(|e| Error::Provider {
9595
provider: "openai".into(),
96-
message: e.to_string(),
96+
message: e.without_url().to_string(),
9797
})?;
9898

9999
if response.status() == reqwest::StatusCode::UNAUTHORIZED {
@@ -146,7 +146,7 @@ impl OpenAiProvider {
146146
} else {
147147
Error::Provider {
148148
provider: "openai".into(),
149-
message: e.to_string(),
149+
message: e.without_url().to_string(),
150150
}
151151
}
152152
})?;
@@ -177,11 +177,18 @@ impl OpenAiProvider {
177177

178178
let chunk = chunk.map_err(|e| Error::Provider {
179179
provider: "openai".into(),
180-
message: e.to_string(),
180+
message: e.without_url().to_string(),
181181
})?;
182182

183183
line_buffer.push_str(&String::from_utf8_lossy(&chunk));
184184

185+
if line_buffer.len() > MAX_RESPONSE_BYTES {
186+
return Err(Error::Provider {
187+
provider: "openai".into(),
188+
message: "line buffer exceeded 1 MB limit".into(),
189+
});
190+
}
191+
185192
while let Some(newline_pos) = line_buffer.find('\n') {
186193
// Parse from slice to avoid allocating a String per line
187194
let result = {

0 commit comments

Comments
 (0)