[repo-health] Medium: LocalProvider.fetch() accepts arbitrary filesystem paths — path traversal risk when /api/analyze is wired up

[Liohtml]

Summary

LocalProvider.fetch() resolves the source argument directly to Path(target) and scans any directory or ZIP file on the host filesystem. When the /api/analyze endpoint is connected to the pipeline (currently returns not_implemented), an authenticated user could pass source=/etc or source=../../secrets to read arbitrary files.

Category

Security

Severity

Medium

Location

• File: src/medcheck/providers/local.py
• Lines: 23–34 (fetch() method)

Details

The ZIP-traversal check in _scan_zip() is correctly implemented. The gap is in the directory path branch: Path(target).is_dir() accepts any absolute or relative path. There is no sandboxing to a safe base directory.

Current mitigations (lower severity):

• /api/analyze currently returns {"status": "not_implemented"} — the code is never reached
• Server defaults to 127.0.0.1 (localhost-only) and supports API key auth
• Operators must explicitly set MEDCHECK_HOST=0.0.0.0 to expose over the network

Suggested Fix

Before connecting the analyze pipeline to the web endpoint, add a sandbox check:

import os

SAFE_BASE = Path(os.environ.get("MEDCHECK_DATA_DIR", "/data/dicom"))

def _sandbox(path: Path) -> Path:
    resolved = path.resolve()
    if not str(resolved).startswith(str(SAFE_BASE.resolve())):
        raise ValueError(f"Path outside allowed data directory: {path}")
    return resolved

And in fetch():

target_path = self._sandbox(Path(target))

Effort Estimate

15 min

---

Automated finding by repo-health-agent v1.0

[Liohtml]

Analysis

Confirmed (latent — not exploitable today, but will be the moment the pipeline is wired into the web API).

Root cause: LocalProvider.fetch() (src/medcheck/providers/local.py) accepts any target string and resolves it as a filesystem path:

def fetch(self, target: str, credentials: dict[str, str]) -> list[DicomSeries]:
    target_path = Path(target)
    if target_path.is_dir():
        return self._scan_directory(target_path)
    if target_path.suffix.lower() == ".zip" and target_path.exists():
        return self._scan_zip(target_path)
    raise ValueError(...)

When the /api/analyze endpoint is wired up and passes req.source directly as target, an API caller can supply /etc/passwd, ../secrets.env, /var/run/docker.sock, or any other path accessible to the process. _scan_directory will recursively read every file under it and attempt to parse each as DICOM.

Current exposure: The endpoint returns {"status": "not_implemented"} today, so no real-world risk yet. But if the pipeline is wired in before this is fixed, the risk is immediate.

How I would fix this

The web API needs an input allowlist. The cleanest approach is a configurable data root:

In Settings:

data_root: Path = field(
    default_factory=lambda: Path(os.environ.get("MEDCHECK_DATA_ROOT", "/data"))
)

In LocalProvider.fetch() (or the analyze endpoint before calling the provider):

def _validate_source_path(target: str, data_root: Path) -> Path:
    resolved = (data_root / target).resolve()
    if not str(resolved).startswith(str(data_root.resolve())):
        raise ValueError(f"Source path escapes the allowed data root: {target!r}")
    return resolved

Alternatively, if source from the API is expected to be a user-uploaded file reference (not an arbitrary path), the endpoint should accept a file ID or file upload rather than a raw path string.

Effort estimate

30 minutes (settings addition + path validation in the endpoint)

Suggesting: fix this before wiring the pipeline into /api/analyze.

[Liohtml]

One nuance worth adding to the analysis above: even with pydicom.dcmread(force=True), the current implementation is less dangerous than it looks for non-DICOM files, because _try_read returns None for anything without PixelData. So the traversal won't silently read /etc/passwd into a patient report — it'll just silently skip non-DICOM files.

However, two risks remain:

1. Directory enumeration: _scan_directory does directory.rglob("*"), which walks the entire provided path recursively. Passing source=/ wouldn't return files but would generate stat calls on every filesystem object, which is a latency/resource oracle even if it returns no DICOM data.

2. pydicom with force=True is aggressive: It will attempt to parse any binary file as DICOM. A crafted file at a known path (e.g., /var/log/something.log) could potentially trigger a pydicom parser path that leaks metadata about file content via error messages — this is speculative but worth noting for a tool that will be wired to an HTTP endpoint.

The right fix is a path allowlist in the web API handler (not in LocalProvider.fetch itself, which should remain general-purpose for the CLI use case). When /api/analyze is wired, restrict source to paths under a configured MEDCHECK_DATA_ROOT before passing to LocalProvider.