Skip to content

Commit 389b2b0

Browse files
fix(sidecar): harden band-math eval, job caps, and result bounds
Block ndarray attribute I/O in raster calculator expressions, refuse unbounded in-flight conversion/Whitebox jobs, cap Sedona result size, sanitize conversion errors, and enforce the shared feature limit on Whitebox layer inputs. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a6b280b commit 389b2b0

8 files changed

Lines changed: 271 additions & 3 deletions

File tree

backend/geolibre_server/geolibre_server/app/conversion.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ def shapefile_field_warnings(column_names):
141141
_JOBS_LOCK = threading.Lock()
142142
_RUNTIME_SETUP_LOCK = threading.Lock()
143143
MAX_RETAINED_JOBS = 100
144+
# Concurrent pending/running conversion (and raster) jobs. Finished jobs are
145+
# retained up to MAX_RETAINED_JOBS; in-flight work is refused with HTTP 429 once
146+
# this cap is hit so a burst of /run calls cannot spawn unbounded subprocesses.
147+
MAX_IN_FLIGHT_JOBS = 8
144148

145149

146150
class VectorToVectorRequest(BaseModel):
@@ -1033,6 +1037,11 @@ def _append_job_message(job_id: str, message: str) -> None:
10331037
)
10341038

10351039

1040+
def _count_in_flight_jobs_locked() -> int:
1041+
"""Return how many jobs are pending or running. Caller must hold ``_JOBS_LOCK``."""
1042+
return sum(1 for job in _JOBS.values() if job.status in {"pending", "running"})
1043+
1044+
10361045
def _evict_finished_jobs_locked() -> None:
10371046
"""Drop the oldest finished jobs once the retention cap is exceeded.
10381047
@@ -1122,7 +1131,15 @@ def _run_conversion_job(
11221131
outputs={output_name: {"path": output_path}} if output_path else {},
11231132
)
11241133
except Exception as exc:
1125-
_job_update(job_id, status="failed", error=str(exc))
1134+
# Mirror Whitebox: log the raw failure server-side and surface only a
1135+
# generic message. DuckDB/GDAL stderr often embeds absolute paths that
1136+
# must not reach the browser-proxied /jobs/{id} response.
1137+
logger.warning("Conversion job %s failed: %s", job_id, exc, exc_info=True)
1138+
_job_update(
1139+
job_id,
1140+
status="failed",
1141+
error="Conversion failed. See the sidecar logs for details.",
1142+
)
11261143
# Remove a partial output so a retry starts clean and stale bytes do not
11271144
# confuse downstream tools.
11281145
output_path = params.get("output_path")
@@ -1148,6 +1165,11 @@ def _start_job(
11481165
job_id = str(uuid.uuid4())
11491166
now = _utc_now()
11501167
with _JOBS_LOCK:
1168+
if _count_in_flight_jobs_locked() >= MAX_IN_FLIGHT_JOBS:
1169+
raise HTTPException(
1170+
status_code=429,
1171+
detail="Too many conversion jobs in progress; try again shortly.",
1172+
)
11511173
_JOBS[job_id] = JobState(
11521174
id=job_id,
11531175
status="pending",

backend/geolibre_server/geolibre_server/app/raster.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -929,6 +929,7 @@ def model(hh):
929929

930930

931931
_RASTER_CALC_SCRIPT = """
932+
import ast
932933
import json, re, sys
933934
934935
import numpy as np
@@ -1024,6 +1025,33 @@ def load_bands(path, letter, namespace, base):
10241025
"e": np.e,
10251026
}
10261027
namespace.update(safe_funcs)
1028+
# Band arrays are full ndarrays, so attribute access (``A.tofile(...)``,
1029+
# ``A.dump(...)``) would still write files even with an empty ``__builtins__``
1030+
# and no bare ``np``. Parse the expression and reject attribute access plus any
1031+
# call that is not one of the curated safe functions before ``eval``.
1032+
try:
1033+
tree = ast.parse(expression, mode="eval")
1034+
except SyntaxError as exc:
1035+
raise SystemExit(f"Failed to evaluate expression: {exc}") from exc
1036+
allowed_calls = set(safe_funcs)
1037+
for node in ast.walk(tree):
1038+
if isinstance(node, ast.Attribute):
1039+
raise SystemExit(
1040+
"Expression may not access attributes "
1041+
"(band math only allows curated functions and operators)"
1042+
)
1043+
if isinstance(node, ast.Call):
1044+
# ``np.where(...)`` / ``A.tofile(...)`` parse as Attribute-backed calls;
1045+
# reject them the same way as bare attribute access so ndarray file I/O
1046+
# and the withheld ``np`` module stay unreachable.
1047+
if isinstance(node.func, ast.Attribute):
1048+
raise SystemExit(
1049+
"Expression may not access attributes "
1050+
"(band math only allows curated functions and operators)"
1051+
)
1052+
if not isinstance(node.func, ast.Name) or node.func.id not in allowed_calls:
1053+
name = node.func.id if isinstance(node.func, ast.Name) else type(node.func).__name__
1054+
raise SystemExit(f"Call to '{name}' is not allowed in band math")
10271055
try:
10281056
with np.errstate(all="ignore"):
10291057
result = eval(expression, {"__builtins__": {}}, namespace)

backend/geolibre_server/geolibre_server/app/whitebox.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from pydantic import BaseModel
2121

2222
from . import conversion
23+
from geolibre_server.vector_ops import MAX_FEATURES as MAX_LAYER_FEATURES
2324
from .runtime import (
2425
RUNTIME_CATALOG_TIMEOUT_SECS,
2526
RUNTIME_DISCOVERY_TIMEOUT_SECS,
@@ -88,6 +89,10 @@ class WhiteboxRunRequest(BaseModel):
8889
_JOBS_LOCK = threading.Lock()
8990
_RUNTIME_SETUP_LOCK = threading.Lock()
9091
MAX_RETAINED_JOBS = 100
92+
# Concurrent pending/running Whitebox jobs. Finished jobs are retained up to
93+
# MAX_RETAINED_JOBS; in-flight work is refused with HTTP 429 once this cap is
94+
# hit so a burst of /run calls cannot spawn unbounded tool sessions.
95+
MAX_IN_FLIGHT_JOBS = 8
9196

9297

9398
def _check_python_import(python_executable: str) -> None:
@@ -701,10 +706,19 @@ def _write_layer_input(param_name: str, layer: dict[str, Any], temp_paths: list[
701706
702707
Returns:
703708
Path to the materialized input file.
709+
710+
Raises:
711+
ValueError: When the layer payload is not GeoJSON, or exceeds the
712+
shared feature cap used by vector/PostGIS/Sedona paths.
704713
"""
705714
geojson = layer.get("geojson")
706715
if not isinstance(geojson, dict):
707716
raise ValueError(f"Layer input for {param_name} does not contain GeoJSON.")
717+
features = geojson.get("features") or []
718+
if isinstance(features, list) and len(features) > MAX_LAYER_FEATURES:
719+
raise ValueError(
720+
f"Layer input for {param_name} exceeds the {MAX_LAYER_FEATURES}-feature limit"
721+
)
708722
folder = Path(tempfile.mkdtemp(prefix="geolibre-whitebox-input-"))
709723
temp_paths.append(folder)
710724
path = folder / f"{_safe_output_stem('input', param_name)}.geojson"
@@ -1041,15 +1055,41 @@ def _evict_finished_jobs_locked() -> None:
10411055
_JOBS.pop(job_id, None)
10421056

10431057

1058+
def _count_in_flight_jobs_locked() -> int:
1059+
"""Return how many jobs are pending or running. Caller must hold ``_JOBS_LOCK``."""
1060+
return sum(1 for job in _JOBS.values() if job.status in {"pending", "running"})
1061+
1062+
10441063
@router.post("/run")
10451064
def whitebox_run(request: WhiteboxRunRequest):
10461065
"""Start a background Whitebox tool run."""
10471066
tool_id = request.tool_id.strip()
10481067
if not tool_id:
10491068
raise HTTPException(status_code=400, detail="tool_id is required")
1069+
# Reject oversized embedded layers before enqueueing work, matching the
1070+
# 413 vector/PostGIS/Sedona feature cap (defense-in-depth also lives in
1071+
# ``_write_layer_input``).
1072+
for name, layer in request.layer_inputs.items():
1073+
geojson = layer.get("geojson") if isinstance(layer, dict) else None
1074+
if not isinstance(geojson, dict):
1075+
continue
1076+
features = geojson.get("features") or []
1077+
if isinstance(features, list) and len(features) > MAX_LAYER_FEATURES:
1078+
raise HTTPException(
1079+
status_code=413,
1080+
detail=(
1081+
f"Layer input for {name} exceeds the "
1082+
f"{MAX_LAYER_FEATURES}-feature limit"
1083+
),
1084+
)
10501085
job_id = str(uuid.uuid4())
10511086
now = _utc_now()
10521087
with _JOBS_LOCK:
1088+
if _count_in_flight_jobs_locked() >= MAX_IN_FLIGHT_JOBS:
1089+
raise HTTPException(
1090+
status_code=429,
1091+
detail="Too many Whitebox jobs in progress; try again shortly.",
1092+
)
10531093
_JOBS[job_id] = JobState(
10541094
id=job_id,
10551095
status="pending",

backend/geolibre_server/geolibre_server/sedona_ops.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def run_sql(sql: str, layers: Optional[list[dict]] = None) -> dict:
121121
``rows`` and as GeoJSON in ``geojson``.
122122
123123
Raises:
124-
SqlInputTooLarge: A layer exceeds :data:`MAX_FEATURES`.
124+
SqlInputTooLarge: A layer or the query result exceeds :data:`MAX_FEATURES`.
125125
ValueError: Invalid input.
126126
Exception: Whatever SedonaDB raises for an invalid SQL statement.
127127
"""
@@ -151,6 +151,13 @@ def run_sql(sql: str, layers: Optional[list[dict]] = None) -> dict:
151151
# to_pandas() returns a GeoDataFrame when the result has a geometry
152152
# column, otherwise a plain DataFrame.
153153
frame = result.to_pandas()
154+
if len(frame) > MAX_FEATURES:
155+
# Input registration already caps each layer, but a query can still
156+
# expand rows (cross joins, generate_series, etc.). Bound the
157+
# response the same way vector/PostGIS paths bound payloads.
158+
raise SqlInputTooLarge(
159+
f"Query result exceeds the {MAX_FEATURES}-feature limit"
160+
)
154161
columns = [str(column) for column in frame.columns]
155162

156163
geometry_column: Optional[str] = None

backend/geolibre_server/tests/test_conversion.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,3 +459,54 @@ def test_evict_finished_jobs_never_drops_running(monkeypatch) -> None:
459459
_evict_finished_jobs_locked()
460460
# Excess is 2, but only the one finished job is eligible for eviction.
461461
assert set(jobs) == {"old_running", "pending"}
462+
463+
464+
def test_start_job_rejects_when_in_flight_cap_reached(monkeypatch) -> None:
465+
"""A new conversion job is refused with 429 once in-flight work is at the cap."""
466+
monkeypatch.setattr(conversion, "MAX_IN_FLIGHT_JOBS", 1)
467+
monkeypatch.setattr(
468+
conversion,
469+
"_JOBS",
470+
{"busy": _job("busy", "running", "2026-01-01T00:00:00+00:00")},
471+
)
472+
473+
with pytest.raises(HTTPException) as exc:
474+
conversion._start_job("vector-to-geoparquet", "pass", {}, "output")
475+
assert exc.value.status_code == 429
476+
assert "Too many conversion jobs" in str(exc.value.detail)
477+
478+
479+
def test_conversion_job_does_not_leak_error(monkeypatch, tmp_path: Path) -> None:
480+
"""Failed conversion jobs store a generic error, never the raw exception text."""
481+
job_id = "test-conversion-leak"
482+
now = conversion._utc_now()
483+
out = tmp_path / "out.parquet"
484+
with conversion._JOBS_LOCK:
485+
conversion._JOBS[job_id] = conversion.JobState(
486+
id=job_id,
487+
status="pending",
488+
tool_id="vector-to-geoparquet",
489+
created_at=now,
490+
updated_at=now,
491+
)
492+
493+
secret = "/secret/path/to/duckdb: boom traceback leak"
494+
495+
def _boom(*_args, **_kwargs):
496+
raise RuntimeError(secret)
497+
498+
monkeypatch.setattr(conversion, "_runtime_python", _boom)
499+
try:
500+
conversion._run_conversion_job(
501+
job_id,
502+
"pass",
503+
{"output_path": str(out)},
504+
"output",
505+
)
506+
job = conversion._JOBS[job_id]
507+
assert job.status == "failed"
508+
assert job.error == "Conversion failed. See the sidecar logs for details."
509+
assert secret not in (job.error or "")
510+
finally:
511+
with conversion._JOBS_LOCK:
512+
conversion._JOBS.pop(job_id, None)

backend/geolibre_server/tests/test_raster.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,12 @@ def test_raster_calculator_blocks_numpy_io(tmp_path: Path) -> None:
634634
text=True,
635635
)
636636
assert completed.returncode != 0
637-
assert "Failed to evaluate expression" in (completed.stdout + completed.stderr)
637+
combined = completed.stdout + completed.stderr
638+
assert (
639+
"Failed to evaluate expression" in combined
640+
or "may not access attributes" in combined
641+
or "not allowed in band math" in combined
642+
)
638643
assert not out.exists()
639644

640645

@@ -1024,3 +1029,33 @@ def test_focal_std_zero_on_flat_input(tmp_path: Path) -> None:
10241029
with rasterio.open(out) as ds:
10251030
got = ds.read(1)
10261031
assert np.allclose(got, 0.0, atol=1e-4)
1032+
1033+
1034+
@requires_rasterio
1035+
def test_raster_calculator_blocks_ndarray_tofile(tmp_path: Path) -> None:
1036+
"""Ndarray file I/O via attribute access must not bypass the path allowlist."""
1037+
src = _write_dem(tmp_path / "dem.tif")
1038+
out = tmp_path / "calc.tif"
1039+
evil = tmp_path / "evil.bin"
1040+
completed = subprocess.run(
1041+
[
1042+
sys.executable,
1043+
"-c",
1044+
_RASTER_TOOL_SCRIPTS["raster-calc"],
1045+
json.dumps(
1046+
{
1047+
"input_path": str(src),
1048+
"output_path": str(out),
1049+
"expression": f"(A.tofile({str(evil)!r}), A)[1]",
1050+
}
1051+
),
1052+
],
1053+
check=False,
1054+
capture_output=True,
1055+
text=True,
1056+
)
1057+
assert completed.returncode != 0
1058+
combined = completed.stdout + completed.stderr
1059+
assert "may not access attributes" in combined
1060+
assert not evil.exists()
1061+
assert not out.exists()

backend/geolibre_server/tests/test_sql.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,44 @@ def test_run_rejects_oversized_layer(monkeypatch: pytest.MonkeyPatch) -> None:
121121
with pytest.raises(HTTPException) as exc:
122122
sql_run(SqlRunRequest(sql="SELECT 1", layers=[{"name": "big", "geojson": big}]))
123123
assert exc.value.status_code == 413
124+
125+
126+
@requires_sedona
127+
def test_run_rejects_oversized_result(monkeypatch: pytest.MonkeyPatch) -> None:
128+
"""Query results that expand past MAX_FEATURES are refused with 413."""
129+
monkeypatch.setattr(sedona_ops, "MAX_FEATURES", 1)
130+
131+
class _FakeFrame:
132+
def __len__(self) -> int:
133+
return 2
134+
135+
@property
136+
def columns(self):
137+
return ["n"]
138+
139+
def to_dict(self, orient: str): # noqa: ARG002
140+
return [{"n": 1}, {"n": 2}]
141+
142+
class _FakeResult:
143+
def to_pandas(self):
144+
return _FakeFrame()
145+
146+
class _FakeConnection:
147+
def create_data_frame(self, gdf): # noqa: ANN001, ARG002
148+
class _View:
149+
def to_view(self, name: str) -> None: # noqa: ARG002
150+
return None
151+
152+
return _View()
153+
154+
def sql(self, statement: str): # noqa: ARG002
155+
return _FakeResult()
156+
157+
def close(self) -> None:
158+
return None
159+
160+
monkeypatch.setattr(sedona_ops, "_import_sedona", lambda: type("M", (), {"connect": staticmethod(lambda: _FakeConnection())})())
161+
with pytest.raises(HTTPException) as exc:
162+
sql_run(SqlRunRequest(sql="SELECT 1 AS n UNION ALL SELECT 2 AS n"))
163+
assert exc.value.status_code == 413
164+
assert "Query result exceeds" in str(exc.value.detail)

backend/geolibre_server/tests/test_whitebox_endpoints.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,47 @@ def _boom(*args, **kwargs):
111111
finally:
112112
with whitebox._JOBS_LOCK:
113113
whitebox._JOBS.pop(job_id, None)
114+
115+
116+
def test_run_rejects_when_in_flight_cap_reached(monkeypatch):
117+
"""A new Whitebox job is refused with 429 once in-flight work is at the cap."""
118+
monkeypatch.setattr(whitebox, "MAX_IN_FLIGHT_JOBS", 1)
119+
now = whitebox._utc_now()
120+
with whitebox._JOBS_LOCK:
121+
whitebox._JOBS.clear()
122+
whitebox._JOBS["busy"] = whitebox.JobState(
123+
id="busy",
124+
status="running",
125+
tool_id="noop",
126+
created_at=now,
127+
updated_at=now,
128+
)
129+
try:
130+
with pytest.raises(HTTPException) as excinfo:
131+
whitebox.whitebox_run(whitebox.WhiteboxRunRequest(tool_id="noop"))
132+
assert excinfo.value.status_code == 429
133+
assert "Too many Whitebox jobs" in str(excinfo.value.detail)
134+
finally:
135+
with whitebox._JOBS_LOCK:
136+
whitebox._JOBS.clear()
137+
138+
139+
def test_run_rejects_oversized_layer_input(monkeypatch):
140+
"""Embedded GeoJSON layer inputs honor the shared MAX_FEATURES cap with 413."""
141+
monkeypatch.setattr(whitebox, "MAX_LAYER_FEATURES", 1)
142+
big = {
143+
"type": "FeatureCollection",
144+
"features": [
145+
{"type": "Feature", "properties": {}, "geometry": None},
146+
{"type": "Feature", "properties": {}, "geometry": None},
147+
],
148+
}
149+
with pytest.raises(HTTPException) as excinfo:
150+
whitebox.whitebox_run(
151+
whitebox.WhiteboxRunRequest(
152+
tool_id="noop",
153+
layer_inputs={"input": {"geojson": big}},
154+
)
155+
)
156+
assert excinfo.value.status_code == 413
157+
assert "feature limit" in str(excinfo.value.detail)

0 commit comments

Comments
 (0)