mirror of
https://github.com/samvallad33/vestige.git
synced 2026-07-02 22:01:01 +02:00
fix(security): close SSRF/token-exfil + credential leaks (swarm audit)
Multi-model audit (deepseek-v4/minimax/kimi/qwen) surfaced these; verified
against the real code and fixed the confirmed ones:
- github connector: host-pin failed OPEN when api_root was unparseable/hostless
— the bearer token would ride a Link `next` url to an attacker host. Now
fail-closed: no pinned host => drop the url. (CRITICAL: SSRF / token exfil)
- GithubConfig/RedmineConfig derived Debug leaked the token/api_key into any
{:?} log line or panic message. Replaced with manual redacting Debug impls.
- cross_project priority calc used `as u32 - i` which underflows/panics (debug)
or wraps + corrupts the sort (release). Use saturating_sub.
Verified false-positive (no change): path-traversal in get_file_context — it
only inspects the path string, never reads the file.
core: 535 passed / 0 failed, clippy clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
37c240a964
commit
5c9e66108d
3 changed files with 56 additions and 17 deletions
|
|
@ -411,7 +411,12 @@ impl CrossProjectLearner {
|
|||
based_on: pattern.pattern.name.clone(),
|
||||
confidence: applicable.applicability_confidence,
|
||||
evidence: applicable.supporting_memories.clone(),
|
||||
priority: (10.0 * applicable.applicability_confidence) as u32 - i as u32,
|
||||
// saturating_sub: when confidence is low (small base) and
|
||||
// the suggestion index i exceeds it, a plain `-` underflows
|
||||
// — panicking in debug, wrapping to a huge value in release
|
||||
// (which corrupts the Reverse-priority sort). Saturate to 0.
|
||||
priority: ((10.0 * applicable.applicability_confidence) as u32)
|
||||
.saturating_sub(i as u32),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ const USER_AGENT: &str = concat!("vestige-connector/", env!("CARGO_PKG_VERSION")
|
|||
const PER_PAGE: u32 = 100;
|
||||
|
||||
/// Configuration for a GitHub Issues connector instance.
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Clone)]
|
||||
pub struct GithubConfig {
|
||||
/// Repository owner (user or org).
|
||||
pub owner: String,
|
||||
|
|
@ -50,6 +50,20 @@ pub struct GithubConfig {
|
|||
pub max_comments: usize,
|
||||
}
|
||||
|
||||
// Manual Debug that NEVER prints the token — a derived Debug would leak the
|
||||
// bearer credential into any `{:?}` log line or panic message.
|
||||
impl std::fmt::Debug for GithubConfig {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("GithubConfig")
|
||||
.field("owner", &self.owner)
|
||||
.field("repo", &self.repo)
|
||||
.field("token", &self.token.as_ref().map(|_| "<redacted>"))
|
||||
.field("api_root", &self.api_root)
|
||||
.field("max_comments", &self.max_comments)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl GithubConfig {
|
||||
pub fn new(owner: impl Into<String>, repo: impl Into<String>) -> Self {
|
||||
Self {
|
||||
|
|
@ -162,22 +176,29 @@ impl GithubConnector {
|
|||
{
|
||||
let url = &part[start + 1..end];
|
||||
// Host-pin: only follow a next-url on the same host as the API
|
||||
// root we were configured with.
|
||||
if let Some(expected) = expected_host {
|
||||
match reqwest::Url::parse(url) {
|
||||
Ok(parsed) if parsed.host_str() == Some(expected) => {
|
||||
return Some(url.to_string());
|
||||
}
|
||||
_ => {
|
||||
tracing::warn!(
|
||||
next_url = url,
|
||||
"dropping cross-host Link next url (host pin)"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
// root we were configured with. FAIL-CLOSED: if we could not
|
||||
// determine the expected host (unparseable/hostless api_root), we
|
||||
// must NOT follow the url — the bearer token would otherwise ride
|
||||
// along to an attacker-influenced host (SSRF / token exfiltration).
|
||||
let Some(expected) = expected_host else {
|
||||
tracing::warn!(
|
||||
next_url = url,
|
||||
"dropping Link next url: no pinned host (fail-closed)"
|
||||
);
|
||||
return None;
|
||||
};
|
||||
match reqwest::Url::parse(url) {
|
||||
Ok(parsed) if parsed.host_str() == Some(expected) => {
|
||||
return Some(url.to_string());
|
||||
}
|
||||
_ => {
|
||||
tracing::warn!(
|
||||
next_url = url,
|
||||
"dropping cross-host Link next url (host pin)"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
}
|
||||
return Some(url.to_string());
|
||||
}
|
||||
}
|
||||
None
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ const USER_AGENT: &str = concat!("vestige-connector/", env!("CARGO_PKG_VERSION")
|
|||
const PAGE_LIMIT: u32 = 100;
|
||||
|
||||
/// Configuration for a Redmine connector instance bound to one project.
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Clone)]
|
||||
pub struct RedmineConfig {
|
||||
/// Base URL of the Redmine instance, e.g. `https://redmine.example.com`.
|
||||
pub base_url: String,
|
||||
|
|
@ -55,6 +55,19 @@ pub struct RedmineConfig {
|
|||
pub max_journals: usize,
|
||||
}
|
||||
|
||||
// Manual Debug that NEVER prints the api_key — a derived Debug would leak the
|
||||
// credential into any `{:?}` log line or panic message.
|
||||
impl std::fmt::Debug for RedmineConfig {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("RedmineConfig")
|
||||
.field("base_url", &self.base_url)
|
||||
.field("project", &self.project)
|
||||
.field("api_key", &self.api_key.as_ref().map(|_| "<redacted>"))
|
||||
.field("max_journals", &self.max_journals)
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
|
||||
impl RedmineConfig {
|
||||
pub fn new(base_url: impl Into<String>, project: impl Into<String>) -> Self {
|
||||
Self {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue