consume only artifacts the runtime produced
tile_manifest_path arrives in the detection and segmentation request and was read straight off disk, and a manifest entry may name an absolute tile path. That makes an API field an unbounded reference to the host filesystem, and it contradicts the rule the persistence model rests on: only a governed, runtime-produced artifact may be consumed, and a file outside the storage root is not one. Both the manifest and every tile it names now resolve under STORAGE_ROOT. Resolution happens before the comparison, so ".." cannot climb out and a sibling that merely shares a name prefix does not pass. GEOINTEL_ALLOW_EXTERNAL_ARTIFACT_PATHS opts out for provisioning workflows that stage tiles before ingest. The check honours the Settings the caller is operating under rather than the process-wide ones, because every analysis path already threads its own. The affected tests write manifests into tmp_path, so they now declare tmp_path as the storage root — which is what a deployment does, and makes the fixtures more honest than they were. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -287,7 +287,10 @@ def test_model_assets_api_returns_canonical_envelope(monkeypatch, tmp_path: Path
|
||||
assert payload["data"]["items"][0]["will_download_models"] is False
|
||||
|
||||
|
||||
def test_detection_run_persists_selected_model_asset_parameters(tmp_path: Path) -> None:
|
||||
def test_detection_run_persists_selected_model_asset_parameters(tmp_path, monkeypatch: Path) -> None:
|
||||
# A manifest written into tmp_path is only a governed artifact if
|
||||
# tmp_path is the storage root.
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
model_file = tmp_path / "building-detector.pt"
|
||||
model_file.write_bytes(b"local model")
|
||||
db, project_id, dataset_id = _project_and_raster_dataset()
|
||||
|
||||
@@ -140,6 +140,9 @@ def _project_and_dataset(dataset_type: str = "raster"):
|
||||
|
||||
def _settings(tmp_path: Path, **overrides) -> Settings:
|
||||
values = {
|
||||
# The runtime only consumes artifacts under the storage root, so a
|
||||
# test that writes tiles into tmp_path must say that is the root.
|
||||
"storage_root": str(tmp_path),
|
||||
"yolo_seg_enabled": True,
|
||||
"yolo_seg_model_path": str(tmp_path / "seg.pt"),
|
||||
"sam_enabled": True,
|
||||
|
||||
@@ -114,7 +114,10 @@ def _session(tmp_path: Path, *, with_manifest: bool):
|
||||
return db, analysis_run_id, reference_dataset_id
|
||||
|
||||
|
||||
def test_segmentation_qa_scores_only_inside_persisted_tile_coverage(tmp_path: Path) -> None:
|
||||
def test_segmentation_qa_scores_only_inside_persisted_tile_coverage(tmp_path: Path, monkeypatch) -> None:
|
||||
# A manifest written into tmp_path is only a governed artifact if
|
||||
# tmp_path is the storage root.
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
db, analysis_run_id, reference_dataset_id = _session(tmp_path, with_manifest=True)
|
||||
|
||||
result = SegmentationService.compare_segmentations_with_reference(
|
||||
@@ -151,7 +154,10 @@ def test_segmentation_qa_without_manifest_reports_unbounded_coverage(tmp_path: P
|
||||
assert result["coverage"]["mode"] == "unbounded_no_manifest"
|
||||
|
||||
|
||||
def test_segmentation_qa_rejects_reference_entirely_outside_coverage(tmp_path: Path) -> None:
|
||||
def test_segmentation_qa_rejects_reference_entirely_outside_coverage(tmp_path: Path, monkeypatch) -> None:
|
||||
# A manifest written into tmp_path is only a governed artifact if
|
||||
# tmp_path is the storage root.
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
db, analysis_run_id, reference_dataset_id = _session(tmp_path, with_manifest=True)
|
||||
db.query_rows[VectorFeature] = [
|
||||
_reference(reference_dataset_id, box(8.0, 8.0, 8.1, 8.1)),
|
||||
|
||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
||||
|
||||
from hashlib import sha256
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
@@ -138,7 +139,12 @@ def test_yolo_preflight_validates_model_and_manifest_without_importing_yolo(tmp_
|
||||
model_path = tmp_path / "model.pt"
|
||||
model_path.write_bytes(b"weights")
|
||||
manifest_path = _manifest(tmp_path, tile_count=2)
|
||||
settings = Settings(yolo_enabled=True, yolo_model_path=str(model_path), yolo_max_tiles=4)
|
||||
settings = Settings(
|
||||
storage_root=str(tmp_path),
|
||||
yolo_enabled=True,
|
||||
yolo_model_path=str(model_path),
|
||||
yolo_max_tiles=4,
|
||||
)
|
||||
_write_model_sidecar(model_path, settings)
|
||||
|
||||
result = YoloPreflightService.run(
|
||||
@@ -162,7 +168,12 @@ def test_yolo_preflight_marks_assumed_dependencies_in_runtime_details(tmp_path:
|
||||
model_path = tmp_path / "model.pt"
|
||||
model_path.write_bytes(b"weights")
|
||||
manifest_path = _manifest(tmp_path, tile_count=1)
|
||||
settings = Settings(yolo_enabled=True, yolo_model_path=str(model_path), yolo_max_tiles=4)
|
||||
settings = Settings(
|
||||
storage_root=str(tmp_path),
|
||||
yolo_enabled=True,
|
||||
yolo_model_path=str(model_path),
|
||||
yolo_max_tiles=4,
|
||||
)
|
||||
_write_model_sidecar(model_path, settings)
|
||||
|
||||
result = YoloPreflightService.run(
|
||||
@@ -182,7 +193,12 @@ def test_yolo_preflight_can_explicitly_smoke_load_local_model(tmp_path: Path) ->
|
||||
model_path = tmp_path / "model.pt"
|
||||
model_path.write_bytes(b"weights")
|
||||
manifest_path = _manifest(tmp_path)
|
||||
settings = Settings(yolo_enabled=True, yolo_model_path=str(model_path), yolo_max_tiles=4)
|
||||
settings = Settings(
|
||||
storage_root=str(tmp_path),
|
||||
yolo_enabled=True,
|
||||
yolo_model_path=str(model_path),
|
||||
yolo_max_tiles=4,
|
||||
)
|
||||
_write_model_sidecar(model_path, settings)
|
||||
|
||||
result = YoloPreflightService.run(
|
||||
@@ -202,7 +218,12 @@ def test_yolo_preflight_can_explicitly_smoke_load_local_model(tmp_path: Path) ->
|
||||
def test_yolo_preflight_reports_explicit_model_load_failure(tmp_path: Path) -> None:
|
||||
model_path = tmp_path / "model.pt"
|
||||
model_path.write_bytes(b"weights")
|
||||
settings = Settings(yolo_enabled=True, yolo_model_path=str(model_path), yolo_max_tiles=4)
|
||||
settings = Settings(
|
||||
storage_root=str(tmp_path),
|
||||
yolo_enabled=True,
|
||||
yolo_model_path=str(model_path),
|
||||
yolo_max_tiles=4,
|
||||
)
|
||||
_write_model_sidecar(model_path, settings)
|
||||
|
||||
result = YoloPreflightService.run(
|
||||
@@ -238,6 +259,9 @@ def test_yolo_preflight_script_outputs_json(tmp_path: Path) -> None:
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
# The script reads process settings; the manifest it is asked to
|
||||
# validate lives here, so this is the storage root for that run.
|
||||
env={**os.environ, "STORAGE_ROOT": str(tmp_path)},
|
||||
)
|
||||
payload = json.loads(result.stdout)
|
||||
|
||||
@@ -250,7 +274,13 @@ def test_yolo_preflight_script_uses_environment_configuration(tmp_path: Path, mo
|
||||
model_path = tmp_path / "model.pt"
|
||||
model_path.write_bytes(b"weights")
|
||||
manifest_path = _manifest(tmp_path)
|
||||
_write_model_sidecar(model_path, Settings(yolo_enabled=True, yolo_model_path=str(model_path), yolo_max_tiles=4))
|
||||
_write_model_sidecar(model_path, Settings(
|
||||
storage_root=str(tmp_path),
|
||||
yolo_enabled=True,
|
||||
yolo_model_path=str(model_path),
|
||||
yolo_max_tiles=4,
|
||||
))
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
monkeypatch.setenv("YOLO_ENABLED", "true")
|
||||
monkeypatch.setenv("YOLO_MODEL_PATH", str(model_path))
|
||||
monkeypatch.setenv("YOLO_MAX_TILES", "4")
|
||||
@@ -314,6 +344,7 @@ def test_yolo_preflight_script_rejects_assumed_dependencies_for_model_load(tmp_p
|
||||
|
||||
|
||||
def test_yolo_preflight_api_returns_canonical_envelope(monkeypatch, tmp_path: Path) -> None:
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
monkeypatch.setenv("YOLO_ENABLED", "false")
|
||||
monkeypatch.setenv("YOLO_MODEL_PATH", str(tmp_path / "missing.pt"))
|
||||
monkeypatch.setenv("YOLO_CONFIG_DIR", str(tmp_path / "ultralytics"))
|
||||
|
||||
@@ -208,6 +208,9 @@ def _project_and_dataset(dataset_type: str = "raster"):
|
||||
def _settings(tmp_path: Path, **overrides) -> Settings:
|
||||
model_path = tmp_path / "model.pt"
|
||||
values = {
|
||||
# The runtime only consumes artifacts under the storage root, so a
|
||||
# test that writes tiles into tmp_path must say that is the root.
|
||||
"storage_root": str(tmp_path),
|
||||
"yolo_enabled": True,
|
||||
"yolo_model_path": str(model_path),
|
||||
"yolo_max_tiles": 4,
|
||||
|
||||
@@ -460,7 +460,10 @@ def _coverage_manifest(tmp_path, dataset_id, bounds=(-1.0, -1.0, 3.0, 3.0)):
|
||||
return manifest_path
|
||||
|
||||
|
||||
def test_detection_qa_excludes_references_outside_persisted_tile_coverage(tmp_path) -> None:
|
||||
def test_detection_qa_excludes_references_outside_persisted_tile_coverage(tmp_path, monkeypatch) -> None:
|
||||
# QA reads process settings; a manifest written into tmp_path is only a
|
||||
# governed artifact if tmp_path is the storage root.
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
project_id = uuid4()
|
||||
dataset_id = uuid4()
|
||||
reference_dataset_id = uuid4()
|
||||
@@ -525,7 +528,10 @@ def test_detection_qa_excludes_references_outside_persisted_tile_coverage(tmp_pa
|
||||
assert quality_check.findings_json["coverage"] == result["coverage"]
|
||||
|
||||
|
||||
def test_detection_qa_reports_box_to_footprint_diagnostic_without_changing_strict_metrics(tmp_path) -> None:
|
||||
def test_detection_qa_reports_box_to_footprint_diagnostic_without_changing_strict_metrics(tmp_path, monkeypatch) -> None:
|
||||
# QA reads process settings; a manifest written into tmp_path is only a
|
||||
# governed artifact if tmp_path is the storage root.
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
project_id = uuid4()
|
||||
dataset_id = uuid4()
|
||||
reference_dataset_id = uuid4()
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
"""Analysis may only read artifacts the runtime itself produced.
|
||||
|
||||
``tile_manifest_path`` arrives in the detection and segmentation request and was
|
||||
read straight off disk, and a manifest entry may name an absolute tile path. A
|
||||
manifest outside the storage root is by definition not a governed artifact, so
|
||||
consuming one contradicts the rule the whole persistence model rests on — and
|
||||
it turns an API field into an unbounded reference to the host filesystem.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from app.core.errors import AppError
|
||||
from app.services.storage_service import StorageService
|
||||
|
||||
|
||||
def test_a_path_inside_the_root_is_returned_resolved(tmp_path: Path, monkeypatch) -> None:
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
target = tmp_path / "tiles" / "manifest.json"
|
||||
target.parent.mkdir(parents=True)
|
||||
target.write_text("{}", encoding="utf-8")
|
||||
|
||||
resolved = StorageService.assert_within_storage_root(str(target), label="tile manifest")
|
||||
|
||||
assert resolved == target.resolve()
|
||||
|
||||
|
||||
def test_a_path_outside_the_root_is_refused(tmp_path: Path, monkeypatch) -> None:
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path / "storage"))
|
||||
(tmp_path / "storage").mkdir()
|
||||
outside = tmp_path / "elsewhere.json"
|
||||
outside.write_text("{}", encoding="utf-8")
|
||||
|
||||
with pytest.raises(AppError) as exc_info:
|
||||
StorageService.assert_within_storage_root(str(outside), label="tile manifest")
|
||||
|
||||
assert exc_info.value.code == "STORAGE_PATH_OUTSIDE_ROOT"
|
||||
assert "tile manifest" in exc_info.value.message
|
||||
|
||||
|
||||
def test_a_traversal_sequence_cannot_climb_out(tmp_path: Path, monkeypatch) -> None:
|
||||
root = tmp_path / "storage"
|
||||
root.mkdir()
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(root))
|
||||
secret = tmp_path / "secret.json"
|
||||
secret.write_text("{}", encoding="utf-8")
|
||||
|
||||
with pytest.raises(AppError) as exc_info:
|
||||
StorageService.assert_within_storage_root(str(root / ".." / "secret.json"), label="tile")
|
||||
|
||||
assert exc_info.value.code == "STORAGE_PATH_OUTSIDE_ROOT"
|
||||
|
||||
|
||||
def test_a_sibling_directory_sharing_a_name_prefix_is_refused(tmp_path: Path, monkeypatch) -> None:
|
||||
"""``/data/storage-old`` is not inside ``/data/storage``."""
|
||||
|
||||
root = tmp_path / "storage"
|
||||
root.mkdir()
|
||||
sibling = tmp_path / "storage-old"
|
||||
sibling.mkdir()
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(root))
|
||||
target = sibling / "manifest.json"
|
||||
target.write_text("{}", encoding="utf-8")
|
||||
|
||||
with pytest.raises(AppError):
|
||||
StorageService.assert_within_storage_root(str(target), label="tile manifest")
|
||||
|
||||
|
||||
def test_an_empty_path_is_refused(tmp_path: Path, monkeypatch) -> None:
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path))
|
||||
|
||||
with pytest.raises(AppError):
|
||||
StorageService.assert_within_storage_root("", label="tile manifest")
|
||||
|
||||
|
||||
def test_the_check_can_be_disabled_for_an_operator_provisioning_workflow(
|
||||
tmp_path: Path, monkeypatch
|
||||
) -> None:
|
||||
"""Provisioning scripts stage tiles outside the root before ingest."""
|
||||
|
||||
monkeypatch.setenv("STORAGE_ROOT", str(tmp_path / "storage"))
|
||||
monkeypatch.setenv("GEOINTEL_ALLOW_EXTERNAL_ARTIFACT_PATHS", "true")
|
||||
(tmp_path / "storage").mkdir()
|
||||
outside = tmp_path / "elsewhere.json"
|
||||
outside.write_text("{}", encoding="utf-8")
|
||||
|
||||
assert StorageService.assert_within_storage_root(str(outside), label="tile") == outside.resolve()
|
||||
Reference in New Issue
Block a user