fix(audit): round-2 panics, overflows, XML/SSRF hardening (swarm)

Second swarm pass (complete every-line sweep), verified against real code
(38/101 confirmed real; 63 false positives excluded). This commit lands the
main-compatible subset:

- redmine SSRF guard: rewrite host check to use host_str()+std::net::IpAddr
  instead of the `url` crate (url is not a direct dep of vestige-core on main;
  the previous form only compiled on the feature branch). Same protection:
  blocks localhost + loopback/private/link-local/unspecified IPs.
- bin/restore: guard wrapper[0] index (empty backup array would panic)
- bin/cli: char-boundary-safe node.id truncation (2 byte-slice panics);
  XML-escape the model/home strings before launchd plist substitution
- prospective_memory: Duration::try_hours/try_days (panic on out-of-range
  user config); case-insensitive " at " split now uses the lowercased index
  so the contains() check and the split agree

core 477/0, mcp builds, clippy clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sam Valladares 2026-06-28 16:08:53 -05:00
parent 5f6ba0e7d1
commit 1bf2c4b92a
4 changed files with 58 additions and 38 deletions

View file

@ -117,35 +117,34 @@ impl RedmineConnector {
)));
}
if std::env::var("VESTIGE_ALLOW_PRIVATE_CONNECTOR_HOSTS").is_err() {
match url.host() {
None => {
return Err(ConnectorError::Config(
"base_url has no host".to_string(),
));
}
Some(url::Host::Ipv4(ip))
if ip.is_loopback()
|| ip.is_private()
|| ip.is_link_local()
|| ip.is_unspecified() =>
{
// Use host_str() + std IP parsing to avoid a direct `url`
// crate dependency. A literal IP host is checked for
// reserved/internal ranges; a "localhost" domain is blocked.
let host = url.host_str().ok_or_else(|| {
ConnectorError::Config("base_url has no host".to_string())
})?;
// strip optional IPv6 brackets, e.g. [::1]
let host_clean = host.trim_start_matches('[').trim_end_matches(']');
if host_clean.eq_ignore_ascii_case("localhost") {
return Err(ConnectorError::Config(
"base_url host localhost is blocked (SSRF guard)".to_string(),
));
}
if let Ok(ip) = host_clean.parse::<std::net::IpAddr>() {
let reserved = match ip {
std::net::IpAddr::V4(v4) => {
v4.is_loopback()
|| v4.is_private()
|| v4.is_link_local()
|| v4.is_unspecified()
}
std::net::IpAddr::V6(v6) => v6.is_loopback() || v6.is_unspecified(),
};
if reserved {
return Err(ConnectorError::Config(format!(
"base_url host {ip} is a reserved/internal address (SSRF guard)"
)));
}
Some(url::Host::Ipv6(ip)) if ip.is_loopback() || ip.is_unspecified() => {
return Err(ConnectorError::Config(format!(
"base_url host {ip} is a reserved/internal address (SSRF guard)"
)));
}
Some(url::Host::Domain(d))
if d.eq_ignore_ascii_case("localhost") =>
{
return Err(ConnectorError::Config(
"base_url host localhost is blocked (SSRF guard)".to_string(),
));
}
_ => {}
}
}
}

View file

@ -988,10 +988,11 @@ impl IntentionParser {
}
}
// Check for "at X" time pattern
if text_lower.contains(" at ") {
// For now, treat as a simple event trigger
let parts: Vec<&str> = original.splitn(2, " at ").collect();
// Check for "at X" time pattern. Split using the byte index found in the
// lowercased text so the case-insensitive contains() and the split agree
// (otherwise "Meeting AT 5pm" passes the check but fails the split).
if let Some(idx) = text_lower.find(" at ") {
let parts: Vec<&str> = vec![&original[..idx], &original[idx + 4..]];
if parts.len() == 2 {
let part0_lower = parts[0].to_lowercase();
let content: String = if part0_lower.starts_with("remind me to ") {
@ -1290,7 +1291,11 @@ impl ProspectiveMemory {
// Check for deadline escalation
if self.config.enable_escalation {
let threshold = Duration::hours(self.config.escalation_threshold_hours);
// try_hours avoids the panic Duration::hours has on an
// out-of-range (user-configurable) value; fall back to a large
// but safe default if the config is absurd.
let threshold = Duration::try_hours(self.config.escalation_threshold_hours)
.unwrap_or_else(|| Duration::hours(24));
if intention.is_deadline_approaching(threshold) {
// Priority will be automatically escalated via effective_priority()
}
@ -1346,9 +1351,11 @@ impl ProspectiveMemory {
history.push_back(fulfilled_intention);
// Maintain history size
let retention_cutoff =
Utc::now() - Duration::days(self.config.completed_retention_days);
// Maintain history size. try_days avoids the panic on an
// out-of-range user-configurable retention value.
let retention_window = Duration::try_days(self.config.completed_retention_days)
.unwrap_or_else(|| Duration::days(30));
let retention_cutoff = Utc::now() - retention_window;
while history
.front()
.map(|i| i.fulfilled_at.unwrap_or(i.created_at) < retention_cutoff)

View file

@ -746,9 +746,19 @@ fn install_launchd_job(source_root: &Path, home: &Path, model: &str) -> anyhow::
.join("com.vestige.mlx-server.plist.template");
let template = fs::read_to_string(&template_path)
.with_context(|| format!("failed to read {}", template_path.display()))?;
// XML-escape interpolated values: this plist is XML, and an unescaped model
// string containing &, <, >, " or ' would corrupt the plist (or inject
// elements). Escape before substitution.
let xml_escape = |s: &str| {
s.replace('&', "&amp;")
.replace('<', "&lt;")
.replace('>', "&gt;")
.replace('"', "&quot;")
.replace('\'', "&apos;")
};
let rendered = template
.replace("__HOME__", &home.display().to_string())
.replace("__MODEL__", model);
.replace("__HOME__", &xml_escape(&home.display().to_string()))
.replace("__MODEL__", &xml_escape(model));
let plist = launchd_dir.join("com.vestige.mlx-server.plist");
fs::write(&plist, rendered)?;
@ -2297,7 +2307,7 @@ fn run_gc(
let age_days = (now - node.created_at).num_days();
println!(
" {} [ret={:.3}, age={}d] {}",
node.id[..8].dimmed(),
node.id.get(..8).unwrap_or(&node.id).dimmed(),
node.retention_strength,
age_days,
truncate(&node.content, 60).dimmed()
@ -2358,7 +2368,7 @@ fn run_gc(
eprintln!(
" {} Failed to delete {}: {}",
"ERR".red(),
&node.id[..8],
node.id.get(..8).unwrap_or(&node.id),
e
);
errors += 1;

View file

@ -49,7 +49,11 @@ fn main() -> anyhow::Result<()> {
// Read and parse backup
let backup_content = std::fs::read_to_string(&backup_path)?;
let wrapper: Vec<BackupWrapper> = serde_json::from_str(&backup_content)?;
let recall_result: RecallResult = serde_json::from_str(&wrapper[0].text)?;
// Guard the index: an empty backup array would panic on wrapper[0].
let first = wrapper
.first()
.ok_or_else(|| anyhow::anyhow!("backup wrapper array is empty — nothing to restore"))?;
let recall_result: RecallResult = serde_json::from_str(&first.text)?;
let memories = recall_result.results;
println!("Found {} memories to restore", memories.len());