Skip to content

Commit 4afa292

Browse files
authored
Merge pull request #150 from blacklanternsecurity/fix-bad-mongo-uri-parsing
Update asset risk score to be float instead of string
2 parents fa8e31f + db172e0 commit 4afa292

File tree

2 files changed

+125
-51
lines changed

2 files changed

+125
-51
lines changed

bbot_server/modules/findings/findings_api.py

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,34 @@
11
from fastapi import Query
2-
from typing import Annotated, Literal, Optional
2+
from typing import Annotated, Optional
33

44
from bbot_server.assets import CustomAssetFields
55
from bbot_server.applets.base import BaseApplet, api_endpoint
66
from bbot_server.modules.findings.findings_models import Finding, SEVERITY_COLORS, SeverityScore, FindingsQuery
77

8+
# Max CVSS score for each severity band (top of range).
9+
# Used to derive a default risk score from finding_max_severity.
10+
SEVERITY_TO_CVSS = {
11+
"INFO": 0.0,
12+
"LOW": 0.1,
13+
"MEDIUM": 4.0,
14+
"HIGH": 7.0,
15+
"CRITICAL": 9.0,
16+
}
17+
818

919
# add 'findings' field to the main asset model
1020
class FindingFields(CustomAssetFields):
1121
findings: Annotated[list[str], "indexed", "indexed-text"] = []
1222
finding_severities: Annotated[dict[str, int], "indexed"] = {}
1323
finding_max_severity: Annotated[Optional[str], "indexed"] = None
1424
finding_max_severity_score: Annotated[int, "indexed"] = 0
15-
# Effective risk level for this asset. Uses the same enum as severity:
16-
# "INFO", "LOW", "MEDIUM", "HIGH", "CRITICAL", or None.
17-
# Auto-synced from finding_max_severity unless risk_override is True.
18-
# IMPORTANT: Every code path that updates finding_max_severity must also
19-
# update risk (when risk_override is False) to keep these fields in sync.
20-
risk: Annotated[Optional[Literal["INFO", "LOW", "MEDIUM", "HIGH", "CRITICAL"]], "indexed"] = None
25+
# Effective risk score for this asset: None or a float from 0.0 to 10.0
26+
# (1 decimal place). Auto-synced from finding_max_severity (using CVSS
27+
# thresholds) unless risk_override is True.
28+
risk: Annotated[Optional[float], "indexed"] = None
2129
# Whether risk has been manually overridden. When True, new findings
2230
# will NOT auto-update risk. Clearing the override resets this to False
23-
# and reverts risk to finding_max_severity.
31+
# and reverts risk to the CVSS-derived value.
2432
risk_override: Annotated[bool, "indexed"] = False
2533

2634

@@ -144,45 +152,59 @@ async def severity_counts(
144152
findings = dict(sorted(findings.items(), key=lambda x: x[1], reverse=True))
145153
return findings
146154

147-
@api_endpoint("/set_risk", methods=["PATCH"], summary="Set or clear a manual risk override for an asset")
155+
@api_endpoint("/set_risk", methods=["PATCH"], summary="Set or clear a manual risk score for an asset")
148156
async def set_risk(
149157
self,
150158
host: Annotated[str, Query(description="The host of the asset to update")],
151159
risk: Annotated[
152-
Optional[str],
160+
Optional[float],
153161
Query(
154-
description="Risk level: INFO, LOW, MEDIUM, HIGH, or CRITICAL. Omit or set to null to clear the override and revert to finding_max_severity."
162+
description=(
163+
"Risk score from 0.0 to 10.0 (1 decimal place). "
164+
"Omit to clear the override and revert to the auto-calculated CVSS value."
165+
)
155166
),
156167
] = None,
168+
override_none: Annotated[
169+
bool,
170+
Query(
171+
description=(
172+
"Set to true to explicitly override risk to None (no risk score). "
173+
"Takes precedence over the risk parameter."
174+
)
175+
),
176+
] = False,
157177
) -> dict:
158178
"""
159-
Manually override an asset's risk level, or clear the override to revert
160-
to the auto-calculated value (finding_max_severity).
179+
Manually set or clear an asset's risk score.
161180
162-
IMPORTANT: When risk is set to a value different from finding_max_severity,
163-
it is considered manually overridden and will not be auto-updated by new findings.
164-
Clearing it (risk=null) reverts to finding_max_severity.
181+
Three modes:
182+
- risk=<float> → override risk to the given value (0.0–10.0, 1 decimal).
183+
- override_none=true → override risk to None (e.g. "no risk score").
184+
- (omit both) → clear the override and revert to the CVSS-derived
185+
value from finding_max_severity.
165186
"""
166187
asset = await self.root._get_asset(host=host, fields=["finding_max_severity"])
167188
if not asset:
168189
raise self.BBOTServerNotFoundError(f"Asset {host} not found")
169190

170-
if risk is not None:
171-
# Validate the risk value against known severity levels
172-
risk = risk.upper()
173-
SeverityScore.to_score(risk) # raises if invalid
174-
# IMPORTANT: Both risk and risk_override must be updated together.
191+
if override_none:
192+
# Explicit override to None
193+
update = {"risk": None, "risk_override": True}
194+
description = f"Risk manually set to [bold]None[/bold] on [bold]{host}[/bold]"
195+
elif risk is not None:
196+
# Override to a specific float value
197+
if risk < 0.0 or risk > 10.0:
198+
raise self.BBOTServerValueError("risk must be between 0.0 and 10.0")
199+
risk = round(risk, 1)
175200
update = {"risk": risk, "risk_override": True}
176201
description = f"Risk manually set to [bold]{risk}[/bold] on [bold]{host}[/bold]"
177202
else:
178-
# Clear the override: revert risk to finding_max_severity.
179-
# IMPORTANT: risk_override must be set to False and risk must be
180-
# recalculated from finding_max_severity to restore the invariant.
203+
# Clear the override: revert to CVSS-derived value
181204
finding_max_severity = asset.get("finding_max_severity", None)
182-
update = {"risk": finding_max_severity, "risk_override": False}
183-
description = (
184-
f"Risk override cleared on [bold]{host}[/bold], reverted to [bold]{finding_max_severity}[/bold]"
185-
)
205+
reverted_risk = SEVERITY_TO_CVSS.get(finding_max_severity) if finding_max_severity else None
206+
update = {"risk": reverted_risk, "risk_override": False}
207+
description = f"Risk override cleared on [bold]{host}[/bold], reverted to [bold]{reverted_risk}[/bold]"
186208

187209
await self.root._update_asset(host, update)
188210
await self.emit_activity(
@@ -291,11 +313,13 @@ async def _insert_or_update_finding(self, finding: Finding, asset, event=None):
291313
else:
292314
asset.finding_max_severity_score = 0
293315
asset.finding_max_severity = None
294-
# IMPORTANT: Only sync risk when not manually overridden.
295-
# When risk_override is True, risk is user-controlled and must not be touched.
316+
# Auto-sync risk from finding_max_severity when not manually overridden.
296317
old_risk = getattr(asset, "risk", None)
297318
if not getattr(asset, "risk_override", False):
298-
asset.risk = asset.finding_max_severity
319+
if asset.finding_max_severity is not None:
320+
asset.risk = SEVERITY_TO_CVSS[asset.finding_max_severity]
321+
else:
322+
asset.risk = None
299323

300324
# insert the new vulnerability
301325
await self.root._insert_asset(finding.model_dump())

tests/test_applets/test_applet_findings.py

Lines changed: 70 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,42 @@ async def after_scan_1(self):
2828
assert {f.confidence for f in findings} == {"UNKNOWN"}
2929
assert {f.confidence_score for f in findings} == {1}
3030

31-
# risk should auto-sync from finding_max_severity
31+
# risk should auto-sync from finding_max_severity via CVSS: HIGH -> 7.0
3232
www_asset = await self.bbot_server.get_asset(host="www.evilcorp.com")
33-
assert www_asset.risk == "HIGH"
33+
assert www_asset.risk == 7.0
3434
assert www_asset.risk_override == False
3535
www2_asset = await self.bbot_server.get_asset(host="www2.evilcorp.com")
36-
assert www2_asset.risk == "HIGH"
36+
assert www2_asset.risk == 7.0
3737
assert www2_asset.risk_override == False
3838

39+
# api.evilcorp.com has no findings yet → risk should be None
40+
api_asset = await self.bbot_server.get_asset(host="api.evilcorp.com")
41+
assert api_asset.risk is None
42+
assert api_asset.risk_override == False
43+
44+
# set risk on asset with no findings, then clear → should revert to None
45+
result = await self.bbot_server.set_risk(host="api.evilcorp.com", risk=5.0)
46+
assert result["risk"] == 5.0
47+
assert result["risk_override"] == True
48+
result = await self.bbot_server.set_risk(host="api.evilcorp.com")
49+
assert result["risk"] is None
50+
assert result["risk_override"] == False
51+
api_asset = await self.bbot_server.get_asset(host="api.evilcorp.com")
52+
assert api_asset.risk is None
53+
assert api_asset.risk_override == False
54+
55+
# override risk to None on asset with no findings (explicit "no risk score")
56+
result = await self.bbot_server.set_risk(host="api.evilcorp.com", override_none=True)
57+
assert result["risk"] is None
58+
assert result["risk_override"] == True
59+
api_asset = await self.bbot_server.get_asset(host="api.evilcorp.com")
60+
assert api_asset.risk is None
61+
assert api_asset.risk_override == True
62+
# clear → should revert to None (no findings = no CVSS value)
63+
result = await self.bbot_server.set_risk(host="api.evilcorp.com")
64+
assert result["risk"] is None
65+
assert result["risk_override"] == False
66+
3967
async def after_scan_2(self):
4068
findings = [f async for f in self.bbot_server.list_findings()]
4169
assert len(findings) == 4
@@ -166,38 +194,60 @@ async def after_scan_2(self):
166194

167195
# --- risk field tests ---
168196

169-
# after scan 2, www2 and api have CRITICAL findings, so risk should auto-update
197+
# after scan 2, www2 and api have CRITICAL findings → CVSS 9.0
170198
www2_asset = await self.bbot_server.get_asset(host="www2.evilcorp.com")
171-
assert www2_asset.risk == "CRITICAL"
199+
assert www2_asset.risk == 9.0
172200
assert www2_asset.risk_override == False
173201
api_asset = await self.bbot_server.get_asset(host="api.evilcorp.com")
174-
assert api_asset.risk == "CRITICAL"
202+
assert api_asset.risk == 9.0
175203
assert api_asset.risk_override == False
176-
# www only had HIGH findings from scan 1, risk should still be HIGH
204+
# www only had HIGH findings from scan 1 → CVSS 7.0
177205
www_asset = await self.bbot_server.get_asset(host="www.evilcorp.com")
178-
assert www_asset.risk == "HIGH"
206+
assert www_asset.risk == 7.0
179207
assert www_asset.risk_override == False
180208

181-
# manually override risk on www2
182-
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", risk="LOW")
183-
assert result["risk"] == "LOW"
209+
# manually set risk on www2 (float 0.0-10.0)
210+
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", risk=7.5)
211+
assert result["risk"] == 7.5
212+
assert result["risk_override"] == True
213+
www2_asset = await self.bbot_server.get_asset(host="www2.evilcorp.com")
214+
assert www2_asset.risk == 7.5
215+
assert www2_asset.risk_override == True
216+
217+
# set risk with extra precision — should round to 1 decimal
218+
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", risk=3.14)
219+
assert result["risk"] == 3.1
220+
assert result["risk_override"] == True
221+
222+
# boundary values
223+
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", risk=0.0)
224+
assert result["risk"] == 0.0
225+
assert result["risk_override"] == True
226+
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", risk=10.0)
227+
assert result["risk"] == 10.0
228+
assert result["risk_override"] == True
229+
230+
# override risk to None — explicit "no risk score"
231+
result = await self.bbot_server.set_risk(host="www2.evilcorp.com", override_none=True)
232+
assert result["risk"] is None
184233
assert result["risk_override"] == True
185234
www2_asset = await self.bbot_server.get_asset(host="www2.evilcorp.com")
186-
assert www2_asset.risk == "LOW"
235+
assert www2_asset.risk is None
187236
assert www2_asset.risk_override == True
188237

189-
# clear the override, risk should revert to finding_max_severity
238+
# clear override should revert to CVSS-derived value (CRITICAL → 9.0)
190239
result = await self.bbot_server.set_risk(host="www2.evilcorp.com")
191-
assert result["risk"] == "CRITICAL"
240+
assert result["risk"] == 9.0
192241
assert result["risk_override"] == False
193242
www2_asset = await self.bbot_server.get_asset(host="www2.evilcorp.com")
194-
assert www2_asset.risk == "CRITICAL"
243+
assert www2_asset.risk == 9.0
195244
assert www2_asset.risk_override == False
196245

197-
# verify RISK_UPDATED activities were emitted (allow time for async queue processing)
198-
# expected: 2 from scan 1 (www + www2: None->HIGH),
199-
# 2 from scan 2 (www2: HIGH->CRITICAL, api: None->CRITICAL),
200-
# 2 from manual set + clear above
246+
# verify RISK_UPDATED activities were emitted
247+
# expected: 2 from scan 1 auto-sync (www + www2: None->7.0),
248+
# 4 from after_scan_1 manual set_risk (api: set 5.0, clear, set None, clear),
249+
# 2 from scan 2 auto-sync (www2: 7.0->9.0, api: None->9.0),
250+
# 6 from after_scan_2 manual set_risk (7.5, 3.1, 0.0, 10.0, None, clear)
201251
await asyncio.sleep(1.0)
202252
activities = [a async for a in self.bbot_server.list_activities() if a.type == "RISK_UPDATED"]
203-
assert len(activities) == 6
253+
assert len(activities) == 14

0 commit comments

Comments
 (0)