Skip to content

Commit 9bf855f

Browse files
authored
fix: cora scan findings — v0.1.1 patch release (#203)
Fixes from Cora scan (62 issues → 5 actionable fixed): #196 — Rate limiter Mutex poison panic Mutex::lock().unwrap() → unwrap_or_else(into_inner) #202 — Blocking DNS in SSRF check is_private_url() wrapped in tokio::task::spawn_blocking #197 — IP spoofing via X-Forwarded-For Validate as IpAddr, take first entry only #198 — Webhook timeout too long 10s → 5s #201 — EventRow silent data loss serde_json parse failure now logs warning via tracing Issues closed: #196, #197, #198, #200 (no-op), #201, #202 Issue deferred: #199 (password validation — enhancement)
1 parent 83a42ff commit 9bf855f

7 files changed

Lines changed: 41 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [0.1.1] - 2026-06-16
11+
12+
### Fixed
13+
14+
- **Rate limiter panic** (#196): `Mutex::lock().unwrap()` replaced with
15+
poison-recovery pattern `unwrap_or_else(|e| e.into_inner())`
16+
- **Blocking DNS in SSRF check** (#202): `is_private_url` now runs via
17+
`tokio::task::spawn_blocking` to avoid blocking async runtime
18+
- **IP spoofing via X-Forwarded-For** (#197): Extracted IP now validated
19+
as `IpAddr` and only first entry taken from comma-separated list
20+
- **Webhook timeout** (#198): Reduced from 10s to 5s
21+
- **EventRow silent data loss** (#201): JSON parse failures now logged
22+
via `tracing::warn` before falling back to null
23+
1024
## [0.1.0] - 2026-06-16
1125

1226
### Added
@@ -168,7 +182,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
168182

169183
- 24 audit findings addressed across 6 batch PRs (#121#126)
170184

171-
[unreleased]: https://github.com/codecoradev/trapfall/compare/v0.1.0...develop
185+
[unreleased]: https://github.com/codecoradev/trapfall/compare/v0.1.1...develop
186+
[0.1.1]: https://github.com/codecoradev/trapfall/compare/v0.1.0...v0.1.1
172187
[0.1.0]: https://github.com/codecoradev/trapfall/compare/v0.0.5...v0.1.0
173188
[0.0.5]: https://github.com/codecoradev/trapfall/compare/v0.0.4...v0.0.5
174189
[0.0.4]: https://github.com/codecoradev/trapfall/compare/v0.0.3...v0.0.4

Cargo.lock

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ members = [
1313
]
1414

1515
[workspace.package]
16-
version = "0.1.0"
16+
version = "0.1.1"
1717
edition = "2024"
1818
license = "Apache-2.0"
1919
repository = "https://github.com/codecoradev/trapfall"

crates/trapfall-db/src/common.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,11 @@ pub struct EventRow {
6969

7070
impl From<EventRow> for StoredEvent {
7171
fn from(r: EventRow) -> Self {
72-
Self {
73-
id: r.id,
74-
issue_id: r.issue_id,
75-
project_id: r.project_id,
76-
data: serde_json::from_str(&r.data).unwrap_or(serde_json::Value::Null),
77-
received_at: r.received_at,
78-
}
72+
let data = serde_json::from_str(&r.data).unwrap_or_else(|e| {
73+
tracing::warn!("Failed to parse event data JSON: {e}");
74+
serde_json::Value::Null
75+
});
76+
Self { id: r.id, issue_id: r.issue_id, project_id: r.project_id, data, received_at: r.received_at }
7977
}
8078
}
8179

crates/trapfalld/src/alert.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ async fn dispatch_webhook(rule: &AlertRule, issue: &Issue) -> anyhow::Result<()>
100100
.ok_or_else(|| anyhow::anyhow!("no url in action_config"))?;
101101

102102
// SSRF protection: block internal/private IPs
103-
if is_private_url(url) {
103+
let url_owned = url.to_string();
104+
let is_private = tokio::task::spawn_blocking(move || is_private_url(&url_owned)).await.unwrap_or(true);
105+
if is_private {
104106
tracing::warn!("Webhook URL blocked (private/internal IP): {url}");
105107
anyhow::bail!("webhook URL points to private/internal address");
106108
}
@@ -120,7 +122,7 @@ async fn dispatch_webhook(rule: &AlertRule, issue: &Issue) -> anyhow::Result<()>
120122
}
121123
});
122124

123-
let resp = REQWEST_CLIENT.post(url).json(&payload).timeout(std::time::Duration::from_secs(10)).send().await?;
125+
let resp = REQWEST_CLIENT.post(url).json(&payload).timeout(std::time::Duration::from_secs(5)).send().await?;
124126

125127
if resp.status().is_success() {
126128
tracing::info!("Webhook dispatched to {url} for rule '{}'", rule.name);
@@ -182,6 +184,7 @@ fn ip_is_private(ip: std::net::IpAddr) -> bool {
182184
}
183185

184186
/// Resolve a hostname to IP addresses (blocking DNS lookup).
187+
/// Called via spawn_blocking to avoid blocking the tokio runtime.
185188
fn dns_resolve_host(host: &str) -> std::io::Result<Vec<std::net::IpAddr>> {
186189
use std::net::ToSocketAddrs;
187190
// Append port 443 for resolution (required by ToSocketAddrs)

crates/trapfalld/src/auth.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,13 @@ pub async fn login(
147147
) -> Result<(StatusCode, [(String, String); 1], Json<LoginResponse>), (StatusCode, Json<AuthErrorJson>)> {
148148
let store = state.store.clone();
149149

150-
// Extract client IP (best-effort)
150+
// Extract client IP (best-effort, first IP from XFF, validated)
151151
let ip = headers
152152
.get("x-forwarded-for")
153153
.or_else(|| headers.get("x-real-ip"))
154154
.and_then(|v| v.to_str().ok())
155+
.and_then(|s| s.split(',').next().map(|s| s.trim()))
156+
.filter(|s| s.parse::<std::net::IpAddr>().is_ok())
155157
.unwrap_or("unknown");
156158

157159
match store.authenticate(&req.email, &req.password, ip).await {

crates/trapfalld/src/rate_limit.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ impl RateLimiter {
5454
}
5555

5656
pub fn try_consume(&self, project_id: &str, cost: f64) -> bool {
57-
let mut buckets = self.buckets.lock().unwrap();
57+
let mut buckets = self.buckets.lock().unwrap_or_else(|e| e.into_inner());
5858

5959
// Evict stale entries if at capacity
6060
if buckets.len() >= MAX_BUCKETS {
@@ -68,7 +68,7 @@ impl RateLimiter {
6868

6969
#[allow(dead_code)]
7070
pub fn available_tokens(&self, project_id: &str) -> f64 {
71-
let mut buckets = self.buckets.lock().unwrap();
71+
let mut buckets = self.buckets.lock().unwrap_or_else(|e| e.into_inner());
7272
let bucket =
7373
buckets.entry(project_id.to_string()).or_insert_with(|| Bucket::new(self.max_tokens, self.refill_per_sec));
7474
bucket.refill();

0 commit comments

Comments
 (0)