Skip to content

Commit d765a90

Browse files
committed
multi-tenant: belt-and-suspenders hardening on agent + attach paths
Audit-driven tightenings. No real cross-org leak vectors today, but all four are cheap defence-in-depth around the boundaries where a leaked agent key, a future schema change, or a TOCTOU race could turn a benign assumption into a real leak. - require_sentinel_agent: switch from `!=` to hmac.compare_digest so a timing side-channel can't reveal prefix matches against the configured secret. Empty-header short-circuit preserved (avoids type-mismatch issues with compare_digest on len-zero in some Python builds). - /complete: cross-check that body.incident_id belongs to the run's org before stamping it on the row. Today the agent is trusted single-tenant infrastructure; this prevents a leaked-key holder from writing a foreign org's incident id into another org's run drawer. - attach_snapshot / attach_clip: pin org_id on the second-session Incident touch (the parent-row updated_at bump). The first session already verified ownership and Incident.org_id is functionally immutable in this codebase, so this is a benign TOCTOU today — but the touch silently no-ops instead of mutating if anything ever changes.
1 parent 7234173 commit d765a90

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

backend/app/api/sentinel.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
(matches email-prefs at notifications.py:1110).
2828
"""
2929

30+
import hmac
3031
import logging
3132
from datetime import UTC, datetime
3233
from typing import Optional
@@ -45,7 +46,7 @@
4546
dispatch_manual_run,
4647
runs_used_this_month,
4748
)
48-
from app.models.models import SentinelConfig, SentinelRun
49+
from app.models.models import Incident, SentinelConfig, SentinelRun
4950

5051
logger = logging.getLogger(__name__)
5152
router = APIRouter(prefix="/api/sentinel", tags=["sentinel"])
@@ -131,7 +132,13 @@ async def require_sentinel_agent(
131132
"""
132133
if not settings.SENTINEL_AGENT_KEY:
133134
raise HTTPException(401, "agent auth not configured")
134-
if not x_sentinel_agent_key or x_sentinel_agent_key != settings.SENTINEL_AGENT_KEY:
135+
# Constant-time compare so a timing side-channel can't reveal
136+
# prefix matches against the configured secret. Empty header
137+
# short-circuits before the compare (compare_digest on length-zero
138+
# is False but raises on type mismatch in some Python builds).
139+
if not x_sentinel_agent_key or not hmac.compare_digest(
140+
x_sentinel_agent_key, settings.SENTINEL_AGENT_KEY
141+
):
135142
raise HTTPException(401, "invalid agent key")
136143

137144

@@ -445,6 +452,24 @@ async def post_run_complete(
445452
# Idempotent no-op — agent retried.
446453
return row.to_dict(include_trace=True)
447454

455+
# Cross-check that the agent isn't pointing the run at an incident
456+
# outside the run's org. The agent is trusted infrastructure (single
457+
# shared key) but a leaked key would let the holder write
458+
# `incident_id=<some other org's id>` into a run row, which would
459+
# surface as a wrong/foreign deep-link in that org's run drawer.
460+
# Defence-in-depth: only accept incident IDs that belong to row.org_id.
461+
if body.outcome == "incident" and body.incident_id is not None:
462+
owned = (
463+
db.query(Incident.id)
464+
.filter_by(id=body.incident_id, org_id=row.org_id)
465+
.first()
466+
)
467+
if owned is None:
468+
raise HTTPException(
469+
400,
470+
"incident_id does not belong to this run's org",
471+
)
472+
448473
now = datetime.now(tz=UTC).replace(tzinfo=None)
449474
row.outcome = body.outcome
450475
row.severity = body.severity if body.outcome == "incident" else None

backend/app/mcp/server.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,8 +1426,17 @@ async def attach_snapshot(
14261426
data_mime="image/jpeg",
14271427
)
14281428
db.add(evidence)
1429-
# Touch parent
1430-
incident = db.query(Incident).filter_by(id=incident_id).first()
1429+
# Touch parent. Org filter is belt-and-suspenders here — the
1430+
# first session at line ~1408 already verified ownership and
1431+
# Incident.org_id is functionally immutable in this codebase —
1432+
# but pinning it on the second-session lookup makes the touch
1433+
# silently no-op rather than mutate the wrong row in any
1434+
# future where org_id ever becomes mutable.
1435+
incident = (
1436+
db.query(Incident)
1437+
.filter_by(id=incident_id, org_id=org_id)
1438+
.first()
1439+
)
14311440
if incident:
14321441
incident.updated_at = datetime.now(tz=UTC).replace(tzinfo=None)
14331442
db.commit()
@@ -1541,7 +1550,14 @@ def attach_clip(
15411550
data_mime=f"video/mp2t;duration={approx_duration}",
15421551
)
15431552
db.add(evidence)
1544-
incident = db.query(Incident).filter_by(id=incident_id).first()
1553+
# Touch parent. Pin org_id on the lookup as belt-and-suspenders;
1554+
# the first session at line ~1492 already verified ownership and
1555+
# Incident.org_id is functionally immutable.
1556+
incident = (
1557+
db.query(Incident)
1558+
.filter_by(id=incident_id, org_id=org_id)
1559+
.first()
1560+
)
15451561
if incident:
15461562
incident.updated_at = datetime.now(tz=UTC).replace(tzinfo=None)
15471563
db.commit()

0 commit comments

Comments
 (0)