From 53d801c861767a8428187289f39c33c050d00d40 Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Mon, 25 May 2026 14:14:48 -0400 Subject: [PATCH 1/6] feat/file_input_output --- README.md | 16 + mcpb/manifest.json | 18 +- pyproject.toml | 4 + src/openstaad_mcp/connection.py | 8 +- src/openstaad_mcp/file_io.py | 707 ++++++++++++++++++++ src/openstaad_mcp/file_io/__init__.py | 47 ++ src/openstaad_mcp/file_io/const.py | 38 ++ src/openstaad_mcp/file_io/helpers.py | 107 +++ src/openstaad_mcp/file_io/models.py | 140 ++++ src/openstaad_mcp/file_io/path_validator.py | 150 +++++ src/openstaad_mcp/file_io/readers.py | 374 +++++++++++ src/openstaad_mcp/file_io/validation.py | 99 +++ src/openstaad_mcp/file_io/writers.py | 155 +++++ src/openstaad_mcp/main.py | 13 +- src/openstaad_mcp/sandbox/ast.py | 4 +- src/openstaad_mcp/sandbox/com_proxy.py | 5 + src/openstaad_mcp/sandbox/const.py | 12 + src/openstaad_mcp/sandbox/executor.py | 8 +- src/openstaad_mcp/sandbox/module_proxy.py | 7 + src/openstaad_mcp/sandbox/stdio_helpers.py | 7 + src/openstaad_mcp/server.py | 110 ++- tests/file_io/__init__.py | 0 tests/file_io/conftest.py | 30 + tests/file_io/test_path_validator.py | 213 ++++++ tests/file_io/test_readers.py | 315 +++++++++ tests/file_io/test_validation.py | 154 +++++ tests/file_io/test_writers.py | 124 ++++ tests/fixtures/basic.csv | 3 + tests/fixtures/cp1252.csv | 2 + tests/fixtures/dates.xlsx | Bin 0 -> 4887 bytes tests/fixtures/empty.csv | 0 tests/fixtures/empty_sheet.xlsx | Bin 0 -> 4787 bytes tests/fixtures/header_only.csv | 1 + tests/fixtures/header_only.xlsx | Bin 0 -> 4830 bytes tests/fixtures/multi_sheet.xlsx | Bin 0 -> 5354 bytes tests/fixtures/no_header.csv | 2 + tests/fixtures/no_header.xlsx | Bin 0 -> 4844 bytes tests/fixtures/semicolon.csv | 3 + tests/fixtures/single_sheet.xlsx | Bin 0 -> 4865 bytes tests/fixtures/utf8.csv | 2 + tests/sandbox/test_executor.py | 71 ++ tests/test_connection.py | 16 +- 42 files changed, 2928 insertions(+), 37 deletions(-) create mode 100644 src/openstaad_mcp/file_io.py create mode 100644 src/openstaad_mcp/file_io/__init__.py create mode 100644 src/openstaad_mcp/file_io/const.py create mode 100644 src/openstaad_mcp/file_io/helpers.py create mode 100644 src/openstaad_mcp/file_io/models.py create mode 100644 src/openstaad_mcp/file_io/path_validator.py create mode 100644 src/openstaad_mcp/file_io/readers.py create mode 100644 src/openstaad_mcp/file_io/validation.py create mode 100644 src/openstaad_mcp/file_io/writers.py create mode 100644 tests/file_io/__init__.py create mode 100644 tests/file_io/conftest.py create mode 100644 tests/file_io/test_path_validator.py create mode 100644 tests/file_io/test_readers.py create mode 100644 tests/file_io/test_validation.py create mode 100644 tests/file_io/test_writers.py create mode 100644 tests/fixtures/basic.csv create mode 100644 tests/fixtures/cp1252.csv create mode 100644 tests/fixtures/dates.xlsx create mode 100644 tests/fixtures/empty.csv create mode 100644 tests/fixtures/empty_sheet.xlsx create mode 100644 tests/fixtures/header_only.csv create mode 100644 tests/fixtures/header_only.xlsx create mode 100644 tests/fixtures/multi_sheet.xlsx create mode 100644 tests/fixtures/no_header.csv create mode 100644 tests/fixtures/no_header.xlsx create mode 100644 tests/fixtures/semicolon.csv create mode 100644 tests/fixtures/single_sheet.xlsx create mode 100644 tests/fixtures/utf8.csv diff --git a/README.md b/README.md index 4077eff..984b89e 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,22 @@ The server supports two transport modes: | `execute_code` | Runs validated Python code against the connected STAAD.Pro model | | `get_status` | Returns connection state, STAAD version, model path, analysis status | +### File I/O + +The `execute_code` tool supports optional **server-side file I/O** for bulk data workflows. +Instead of passing large datasets through the agent's context window, the server reads/writes +CSV and XLSX files directly and injects the data into the sandbox as the `__input__` variable. + +| Parameter | Description | +|-----------|-------------| +| `input_path` | Path to a `.csv` or `.xlsx` file. The server reads and parses it, then injects the data as the immutable `__input__` variable in the sandbox. | +| `output_path` | Path where the sandbox return value will be written. The return value must be a list-of-lists (CSV) or a `{sheet_name: {columns, rows}}` dict (multi-sheet XLSX). | +| `overwrite` | Allow overwriting an existing output file (default `false`). | + +**Path containment:** All file paths must resolve inside an MCP root configured by the client. +The server validates paths against the client-provided roots before any file access. + +**Limits:** Max file size 50 MB, max 100K rows, max 500 columns, max 50 input sheets. ## Security Notes diff --git a/mcpb/manifest.json b/mcpb/manifest.json index 468aa55..0616b34 100644 --- a/mcpb/manifest.json +++ b/mcpb/manifest.json @@ -12,12 +12,28 @@ "documentation": "https://github.com/BentleySystems/openstaad-mcp", "support": "https://www.bentley.com/support/", "icon": "assets/icon.png", + "user_config": { + "allowed_directories": { + "type": "directory", + "title": "Allowed Directories", + "description": "Select directories the openSTAAD server can access", + "multiple": true, + "required": true, + "default": [ + "${HOME}/Desktop", + "${HOME}/Documents" + ] + } + }, "server": { "type": "binary", "entry_point": "openstaad-mcp.exe", "mcp_config": { "command": "${__dirname}/openstaad-mcp.exe", - "args": [] + "args": [ + "--allowed-dirs", + "${user_config.allowed_directories}" + ] } }, "tools": [ diff --git a/pyproject.toml b/pyproject.toml index ff4c0be..cafcc7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,8 +25,12 @@ dependencies = [ "fastmcp>=3.2.3,<4", "packaging>=24", "starlette>=1.0.0,<2.0.0", + "pydantic>=2,<3", "pywin32>=311; platform_system == 'Windows'", "openstaadpy @ git+https://github.com/BentleySystems/openstaadpy.git@306b77ba6a4bed68fd91f21df19c9fd5fc9fb2e1", + "openpyxl>=3.1,<4", + "defusedxml>=0.7", + "chardet>=5,<8", ] [project.urls] diff --git a/src/openstaad_mcp/connection.py b/src/openstaad_mcp/connection.py index aec6d45..34eb78b 100644 --- a/src/openstaad_mcp/connection.py +++ b/src/openstaad_mcp/connection.py @@ -33,7 +33,7 @@ import threading from collections.abc import Callable from dataclasses import dataclass, field -from typing import Any +from typing import Any, TypeVar from openstaad_mcp.version import check_version_warning @@ -185,12 +185,14 @@ def _scan() -> None: # connect_and_run — per-execution STA thread # --------------------------------------------------------------------------- +T = TypeVar("T") + def connect_and_run( - fn: Callable[[Any], Any], + fn: Callable[[Any], T], file_path: str, timeout: float = 120.0, -) -> Any: +) -> T: """Connect to the STAAD.Pro instance that has *file_path* open and run *fn*. Spins a short-lived daemon thread that calls diff --git a/src/openstaad_mcp/file_io.py b/src/openstaad_mcp/file_io.py new file mode 100644 index 0000000..acff0bb --- /dev/null +++ b/src/openstaad_mcp/file_io.py @@ -0,0 +1,707 @@ +""" +Server-side file I/O for the ``execute_code`` tool. + +All operations run **outside** the sandbox. The sandbox never touches the +filesystem — it receives pre-parsed data via ``__input__`` and returns a +structured value that this module writes to disk. + +Architecture +------------ +Reading and writing are handled by format-specific subclasses: + +- :class:`CSVReader` / :class:`CSVWriter` +- :class:`XLSXReader` / :class:`XLSXWriter` + +Each inherits from :class:`BaseReader` or :class:`BaseWriter`, which enforce +file-size limits, column/row caps, and atomic writes. + +Public API +---------- +``read_input_file`` / ``write_output_file`` + Dispatch to the correct reader/writer based on file extension. + +``get_allowed_dirs`` / ``get_input_data`` + Server-level helpers called from ``execute_code``. + +``validate_return_value`` / ``deep_freeze`` + Data validation and summary helpers. +""" + +from __future__ import annotations + +import abc +import csv +import logging +import os +import time +import uuid +from datetime import date, datetime +from datetime import time as dt_time +from pathlib import Path +from typing import Any + +import chardet +import openpyxl +from fastmcp.server.context import Context +from mcp.shared.exceptions import McpError + +from openstaad_mcp.sandbox.const import ( + MAX_FILE_SIZE_BYTES, + MAX_INPUT_COLUMNS, + MAX_INPUT_ROWS, + MAX_INPUT_SHEETS, + MAX_OUTPUT_COLUMNS, + MAX_OUTPUT_ROWS, + MAX_OUTPUT_SHEETS, + MAX_SHEET_NAME_LENGTH, + SAMPLE_ROW_COUNT, + STALE_TEMP_AGE_SECONDS, + TEMP_FILE_PREFIX, +) +from openstaad_mcp.file_io.path_validator import FileIOError, parse_roots_to_dirs, validate_io_path + +logger = logging.getLogger(__name__) + +_JSON_PRIMITIVES = (str, int, float, bool, type(None)) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Server-level helpers (called from server.py) +# ═══════════════════════════════════════════════════════════════════════════ + + +def validate_args_allowed_dirs(allowed_dirs: list[str] | None) -> list[Path]: + """Validate and resolve ``--allowed-dir`` CLI arguments to real paths. + + Security: resolves symlinks so later checks compare against real paths. + """ + if not allowed_dirs: + return [] + + result: list[Path] = [] + for dir_str in allowed_dirs: + # Expand ~/… to the user's home directory + expanded = Path(dir_str).expanduser() + absolute = expanded.resolve(strict=False) + normalized_original = Path(os.path.normpath(absolute)) + + try: + # Security: resolve symlinks in allowed directories during startup + resolved = absolute.resolve(strict=True) + normalized_resolved = Path(os.path.normpath(resolved)) + result.append(normalized_resolved) + except OSError: + # If we can't resolve (doesn't exist), use the normalized absolute path + # This allows configuring allowed dirs that will be created later + result.append(normalized_original) + + return result + + +async def get_allowed_dirs( + ctx: Context, args_allowed_dirs: list[Path], input_path: str | None, output_path: str | None +) -> list[Path]: + """Resolve MCP roots into a list of allowed directories.""" + logger.debug("Args allowed dirs: %s", args_allowed_dirs) + allowed_dirs: list[Path] = [Path(el) for el in args_allowed_dirs] + if input_path is not None or output_path is not None: + try: + roots = await ctx.list_roots() + logger.debug(f"Received MCP roots: {roots}") + except McpError as exc: + logger.error(f"Error listing MCP roots: {exc}") + roots = [] + allowed_dirs += parse_roots_to_dirs(roots) + logger.debug(f"Allowed directories for file I/O: {allowed_dirs}") + return allowed_dirs + + +async def get_input_data(input_path: str | None, allowed_dirs: list[Path]) -> tuple[Any, dict[str, Any] | None]: + """Validate path, read file, freeze data. Returns ``(data, summary)``.""" + if input_path is None: + return None, None + resolved_input = validate_io_path(input_path, allowed_dirs, mode="read") + data, input_summary = read_input_file(resolved_input) + return deep_freeze(data), input_summary + + +# ═══════════════════════════════════════════════════════════════════════════ +# Base reader +# ═══════════════════════════════════════════════════════════════════════════ + + +class BaseReader(abc.ABC): + """Base class for file readers. Enforces file-size and limit checks.""" + + def __init__(self, path: Path) -> None: + self.path = path + self._check_file_size() + + def _check_file_size(self) -> None: + size = self.path.stat().st_size + if size > MAX_FILE_SIZE_BYTES: + raise FileIOError( + "FILE_TOO_LARGE", + f"File is {size:,} bytes; limit is {MAX_FILE_SIZE_BYTES:,} bytes", + ) + + @abc.abstractmethod + def read(self, *, start_row: int = 0, max_rows: int | None = None, **kwargs: Any) -> Any: + """Parse the file and return structured data.""" + + @abc.abstractmethod + def build_summary(self, data: Any) -> dict[str, Any]: + """Build a lightweight summary for the agent.""" + + +# ── Header detection ───────────────────────────────────────────────────── + + +def _cell_type(value: Any) -> str: + """Classify a cell value for header-detection comparison.""" + if value is None: + return "null" + if isinstance(value, bool): + return "bool" + if isinstance(value, (int, float)): + return "numeric" + if isinstance(value, (datetime, date, dt_time)): + return "numeric" + return "string" + + +def _detect_header(rows: list[list], has_header: bool | None) -> bool: + """Detect whether the first row is a header. + + When *has_header* is ``None`` (auto-detect), samples up to 5 rows and + compares the per-column type of row 0 against the majority type of + rows 1-4. If any column's first-row type differs from its data-row + majority, the first row is treated as a header. + """ + if has_header is not None: + return has_header + + if len(rows) <= 1: + return True # Too few rows to compare — conservative default + + first_row = rows[0] + data_rows = rows[1:5] # Up to 4 data rows for comparison + + if not first_row: + return True + + for col_idx in range(len(first_row)): + first_type = _cell_type(first_row[col_idx]) + if first_type == "null": + continue + + type_counts: dict[str, int] = {} + for row in data_rows: + if col_idx < len(row): + t = _cell_type(row[col_idx]) + if t != "null": + type_counts[t] = type_counts.get(t, 0) + 1 + + if not type_counts: + continue + + majority_type = max(type_counts, key=type_counts.get) + if first_type != majority_type: + return True # Type mismatch → first row is a header + + return False # All columns match → not a header + + +def _auto_columns(num_cols: int) -> list[str]: + """Generate column names ``col_1, col_2, …`` for headerless data.""" + return [f"col_{i + 1}" for i in range(num_cols)] + + +# ── CSV reader ─────────────────────────────────────────────────────────── + + +class CSVReader(BaseReader): + """Reads a CSV file into ``list[list]`` (array-of-arrays). + + Uses ``chardet`` for encoding detection and ``csv.Sniffer`` for + dialect detection. Values are coerced from strings to int/float + where possible. Streams from disk line-by-line. + """ + + _CHARDET_MIN_CONFIDENCE = 0.5 + + def read( + self, *, start_row: int = 0, max_rows: int | None = None, has_header: bool | None = None, **kwargs: Any + ) -> list[list]: + encoding = self._detect_encoding() + dialect = self._detect_dialect(encoding) + all_rows: list[list] = [] + + with open(self.path, newline="", encoding=encoding) as f: + reader = csv.reader(f, dialect) + for raw_row in reader: + coerced = [_coerce_csv_value(v) for v in raw_row] + if len(coerced) > MAX_INPUT_COLUMNS: + raise FileIOError( + "TOO_MANY_COLUMNS", + f"Row has {len(coerced)} columns; limit is {MAX_INPUT_COLUMNS}", + ) + all_rows.append(coerced) + + self._has_header = _detect_header(all_rows, has_header) + + if self._has_header: + if not all_rows: + return [] + data_rows = all_rows[1:] + if len(data_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"File has {len(data_rows)} data rows; limit is {MAX_INPUT_ROWS}", + ) + sliced = data_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + return [all_rows[0], *sliced] if start_row == 0 else sliced + else: + if len(all_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"File has {len(all_rows)} data rows; limit is {MAX_INPUT_ROWS}", + ) + sliced = all_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + return sliced + + def build_summary(self, data: list[list]) -> dict[str, Any]: + has_header = getattr(self, "_has_header", True) + if has_header: + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + else: + num_cols = len(data[0]) if data else 0 + header = _auto_columns(num_cols) + data_rows = data + return { + "total_rows": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + # -- helpers -- + + def _detect_encoding(self) -> str: + """Detect file encoding: try UTF-8 first, then chardet, then cp1252.""" + raw = self.path.read_bytes() + result = chardet.detect(raw) + encoding = result.get("encoding") + confidence = result.get("confidence", 0) + if encoding and confidence >= self._CHARDET_MIN_CONFIDENCE: + return encoding + # Low confidence or no result — fall back to cp1252 (Windows default) + return "cp1252" + + def _detect_dialect(self, encoding: str) -> type[csv.Dialect]: + """Detect CSV dialect using csv.Sniffer, fall back to ``excel``. + + Only trusts the sniffer when the detected delimiter is a common + separator character. Exotic delimiters (letters, digits, etc.) + indicate a false positive from a small or ambiguous sample. + """ + _COMMON_DELIMITERS = {",", ";", "\t", "|"} + try: + with open(self.path, newline="", encoding=encoding) as f: + sample = f.read(8192) + dialect = csv.Sniffer().sniff(sample) + if dialect.delimiter in _COMMON_DELIMITERS: + return dialect + except csv.Error: + pass + return csv.excel + + +def _coerce_csv_value(val: str) -> int | float | str: + """Attempt int → float → str coercion of a CSV string value.""" + try: + return int(val) + except ValueError: + pass + try: + return float(val) + except ValueError: + pass + return val + + +# ── XLSX reader ────────────────────────────────────────────────────────── + + +class XLSXReader(BaseReader): + """Reads an XLSX workbook into ``{sheet_name: {columns, rows}}``.""" + + def read( + self, + *, + start_row: int = 0, + max_rows: int | None = None, + sheet: str | None = None, + has_header: bool | None = None, + **kwargs: Any, + ) -> dict[str, dict[str, Any]]: + try: + wb = openpyxl.load_workbook(self.path, read_only=True, data_only=True) + except Exception as exc: + raise FileIOError("CORRUPTED_WORKBOOK", f"Cannot open workbook: {exc}") from None + + try: + self._validate_sheet_count(wb) + sheets_to_load = self._resolve_sheets(wb, sheet) + return {name: self._read_sheet(wb[name], name, start_row, max_rows, has_header) for name in sheets_to_load} + finally: + wb.close() + + def build_summary(self, data: dict[str, dict[str, Any]]) -> dict[str, Any]: + sheets = list(data.keys()) + first_sheet = sheets[0] if sheets else None + first = data[first_sheet] if first_sheet else {"columns": [], "rows": []} + return { + "sheets": sheets, + "loaded_sheet": first_sheet, + "total_rows": len(first["rows"]), + "columns": list(first["columns"]), + "sample_rows": [list(r) for r in first["rows"][:SAMPLE_ROW_COUNT]], + } + + # -- helpers -- + + @staticmethod + def _validate_sheet_count(wb: Any) -> None: + if len(wb.sheetnames) > MAX_INPUT_SHEETS: + raise FileIOError( + "TOO_MANY_ROWS", + f"Workbook has {len(wb.sheetnames)} sheets; limit is {MAX_INPUT_SHEETS}", + ) + + @staticmethod + def _resolve_sheets(wb: Any, sheet: str | None) -> list[str]: + if sheet is not None: + if sheet not in wb.sheetnames: + raise FileIOError("SHEET_NOT_FOUND", f"Sheet '{sheet}' not found in workbook") + return [sheet] + return list(wb.sheetnames) + + @staticmethod + def _read_sheet( + ws: Any, name: str, start_row: int, max_rows: int | None, has_header: bool | None + ) -> dict[str, Any]: + raw_rows: list[list] = [] + for row in ws.iter_rows(values_only=True): + raw_rows.append(list(row)) + + # Check column count on the first row + if raw_rows and len(raw_rows[0]) > MAX_INPUT_COLUMNS: + raise FileIOError( + "TOO_MANY_COLUMNS", + f"Sheet '{name}' has {len(raw_rows[0])} columns; limit is {MAX_INPUT_COLUMNS}", + ) + + # Detect header using raw types (before datetime → string conversion) + is_header = _detect_header(raw_rows, has_header) + + # Convert to JSON primitives + all_rows = [[_to_json_primitive(c) for c in row] for row in raw_rows] + + if is_header: + columns = all_rows[0] if all_rows else [] + data_rows = all_rows[1:] + else: + num_cols = len(all_rows[0]) if all_rows else 0 + columns = _auto_columns(num_cols) + data_rows = all_rows + + if len(data_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"Sheet '{name}' exceeds {MAX_INPUT_ROWS} rows", + ) + + sliced = data_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + + return {"columns": columns, "rows": sliced} + + +def _to_json_primitive(value: Any) -> str | int | float | bool | None: + """Convert an openpyxl cell value to a JSON-safe primitive.""" + if value is None: + return None + if isinstance(value, bool): + return value + if isinstance(value, (int, float)): + return value + if isinstance(value, str): + return value + if isinstance(value, datetime): + return value.isoformat() + if isinstance(value, date): + return value.isoformat() + if isinstance(value, dt_time): + return value.isoformat() + return str(value) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Base writer +# ═══════════════════════════════════════════════════════════════════════════ + + +class BaseWriter(abc.ABC): + """Base class for file writers. Handles atomic writes via temp file.""" + + def __init__(self, path: Path) -> None: + self.path = path + + def write(self, data: Any, *, overwrite: bool = False) -> dict[str, Any]: + """Validate, write atomically, and return a summary.""" + if self.path.exists() and not overwrite: + raise FileIOError("FILE_EXISTS", f"File already exists: {self.path}") + _clean_stale_temps(self.path.parent) + + tmp = _temp_path(self.path.parent) + try: + self._write_to(tmp, data) + os.replace(tmp, self.path) + except BaseException: + tmp.unlink(missing_ok=True) + raise + return self.build_summary(data) + + @abc.abstractmethod + def _write_to(self, tmp: Path, data: Any) -> None: + """Write *data* to the temporary file *tmp*.""" + + @abc.abstractmethod + def build_summary(self, data: Any) -> dict[str, Any]: + """Build a lightweight summary for the agent.""" + + +# ── CSV writer ─────────────────────────────────────────────────────────── + + +class CSVWriter(BaseWriter): + """Writes ``list[list]`` to a CSV file.""" + + def _write_to(self, tmp: Path, data: list[list]) -> None: + with open(tmp, "w", newline="", encoding="utf-8") as f: + csv.writer(f).writerows(data) + + def build_summary(self, data: list[list]) -> dict[str, Any]: + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + return { + "message": f"The `result` data has been written to `{self.path}`", + "rows_written": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + +# ── XLSX writer ────────────────────────────────────────────────────────── + + +class XLSXWriter(BaseWriter): + """Writes flat or multi-sheet data to an XLSX file.""" + + def _write_to(self, tmp: Path, data: Any) -> None: + wb = openpyxl.Workbook() + if isinstance(data, dict): + for i, (name, sheet_data) in enumerate(data.items()): + if i == 0: + ws = wb.active + assert ws is not None + ws.title = name + else: + ws = wb.create_sheet(title=name) + ws.append(sheet_data["columns"]) + for row in sheet_data["rows"]: + ws.append(row) + else: + ws = wb.active + assert ws is not None + for row in data: + ws.append(row) + wb.save(tmp) + + def build_summary(self, data: Any) -> dict[str, Any]: + if isinstance(data, dict): + return { + "message": f"The `result` data has been written to `{self.path}`", + "sheets": { + name: { + "columns": sheet["columns"], + "rows_written": len(sheet["rows"]), + "sample_rows": [list(r) for r in sheet["rows"][:SAMPLE_ROW_COUNT]], + } + for name, sheet in data.items() + }, + } + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + return { + "message": f"The `result` data has been written to `{self.path}`", + "rows_written": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + +# ═══════════════════════════════════════════════════════════════════════════ +# Reader/writer factory helpers +# ═══════════════════════════════════════════════════════════════════════════ + +_READERS: dict[str, type[BaseReader]] = {".csv": CSVReader, ".xlsx": XLSXReader} +_WRITERS: dict[str, type[BaseWriter]] = {".csv": CSVWriter, ".xlsx": XLSXWriter} + + +def _get_reader(path: Path) -> BaseReader: + ext = path.suffix.lower() + cls = _READERS.get(ext) + if cls is None: + raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot read '{ext}' files") + return cls(path) + + +def _get_writer(path: Path) -> BaseWriter: + ext = path.suffix.lower() + cls = _WRITERS.get(ext) + if cls is None: + raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot write '{ext}' files") + return cls(path) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Public dispatch functions (preserve existing API) +# ═══════════════════════════════════════════════════════════════════════════ + + +def read_input_file( + path: Path, + *, + sheet: str | None = None, + start_row: int = 0, + max_rows: int | None = None, + has_header: bool | None = None, +) -> tuple[Any, dict[str, Any]]: + """Read a CSV or XLSX file and return ``(data, summary)``.""" + reader = _get_reader(path) + data = reader.read(start_row=start_row, max_rows=max_rows, sheet=sheet, has_header=has_header) + summary = reader.build_summary(data) + return data, summary + + +def write_output_file(path: str, data: Any, allowed_dirs: list[Path], *, overwrite: bool = False) -> dict[str, Any]: + """Validate path, validate return value, write atomically, return summary.""" + resolved = validate_io_path(path, allowed_dirs, mode="write") + validate_return_value(data) + writer = _get_writer(resolved) + return writer.write(data, overwrite=overwrite) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Return value validation +# ═══════════════════════════════════════════════════════════════════════════ + + +def validate_return_value(value: Any) -> None: + """Raise :class:`FileIOError` if *value* is not a valid output structure. + + Accepts: + - ``list[list[primitive]]`` (flat / CSV / single-sheet) + - ``dict[str, {columns: list, rows: list[list[primitive]]}]`` (multi-sheet) + """ + if isinstance(value, list): + _validate_flat(value) + elif isinstance(value, dict): + _validate_multi_sheet(value) + else: + raise FileIOError( + "INVALID_RETURN_SHAPE", + "Return value must be a list of lists (flat) or a dict of sheets (multi-sheet)", + ) + + +def _validate_flat(rows: list) -> None: + if len(rows) > MAX_OUTPUT_ROWS + 1: # +1 header + raise FileIOError("INVALID_RETURN_SHAPE", f"Too many rows: {len(rows)}; limit {MAX_OUTPUT_ROWS}") + for row in rows: + if not isinstance(row, (list, tuple)): + raise FileIOError("INVALID_RETURN_SHAPE", f"Each row must be a list, got {type(row).__name__}") + if len(row) > MAX_OUTPUT_COLUMNS: + raise FileIOError("INVALID_RETURN_SHAPE", f"Too many columns: {len(row)}; limit {MAX_OUTPUT_COLUMNS}") + for cell in row: + if not isinstance(cell, _JSON_PRIMITIVES): + raise FileIOError( + "INVALID_RETURN_SHAPE", + f"Cell value must be a JSON primitive, got {type(cell).__name__}", + ) + + +def _validate_multi_sheet(sheets: dict) -> None: + if len(sheets) > MAX_OUTPUT_SHEETS: + raise FileIOError("INVALID_RETURN_SHAPE", f"Too many sheets: {len(sheets)}; limit {MAX_OUTPUT_SHEETS}") + for name, sheet_data in sheets.items(): + if len(name) > MAX_SHEET_NAME_LENGTH: + raise FileIOError( + "INVALID_RETURN_SHAPE", + f"Sheet name '{name}' exceeds {MAX_SHEET_NAME_LENGTH} characters", + ) + if not isinstance(sheet_data, dict) or "columns" not in sheet_data or "rows" not in sheet_data: + raise FileIOError( + "INVALID_RETURN_SHAPE", + "Each sheet must have 'columns' and 'rows' keys", + ) + _validate_flat([sheet_data["columns"], *list(sheet_data["rows"])]) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Deep freeze +# ═══════════════════════════════════════════════════════════════════════════ + + +def deep_freeze(data: Any) -> Any: + """Recursively convert mutable containers to immutable equivalents. + + - ``list`` → ``tuple`` + - ``dict`` values are recursively frozen (dict keys stay as-is since + strings are already immutable) + - Primitives (str, int, float, bool, None) pass through unchanged. + """ + if data is None or isinstance(data, (str, int, float, bool)): + return data + if isinstance(data, (list, tuple)): + return tuple(deep_freeze(item) for item in data) + if isinstance(data, dict): + return {k: deep_freeze(v) for k, v in data.items()} + return data + + +# ═══════════════════════════════════════════════════════════════════════════ +# Shared utilities +# ═══════════════════════════════════════════════════════════════════════════ + + +def _temp_path(directory: Path) -> Path: + return directory / f"{TEMP_FILE_PREFIX}{uuid.uuid4().hex}.tmp" + + +def _clean_stale_temps(directory: Path) -> None: + """Remove orphaned temp files older than ``STALE_TEMP_AGE_SECONDS``.""" + cutoff = time.time() - STALE_TEMP_AGE_SECONDS + for p in directory.glob(f"{TEMP_FILE_PREFIX}*.tmp"): + try: + if p.stat().st_mtime < cutoff: + p.unlink() + except OSError: + pass diff --git a/src/openstaad_mcp/file_io/__init__.py b/src/openstaad_mcp/file_io/__init__.py new file mode 100644 index 0000000..021238f --- /dev/null +++ b/src/openstaad_mcp/file_io/__init__.py @@ -0,0 +1,47 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +Server-side file I/O for the ``execute_code`` tool. + +All operations run **outside** the sandbox. The sandbox never touches the +filesystem -- it receives pre-parsed data via ``__input__`` and returns a +structured value that this module writes to disk. + +Sub-modules +----------- +``readers`` -- BaseReader, CSVReader, XLSXReader +``writers`` -- BaseWriter, CSVWriter, XLSXWriter +``models`` -- Pydantic models for return-value validation +``validation`` -- validate_return_value, validate_args_allowed_dirs, deep_freeze +``helpers`` -- get_allowed_dirs, get_input_data, dispatch functions +""" + +from openstaad_mcp.file_io.helpers import ( + get_allowed_dirs, + get_input_data, + read_input_file, + write_output_file, +) +from openstaad_mcp.file_io.readers import CSVReader, XLSXReader +from openstaad_mcp.file_io.validation import ( + deep_freeze, + validate_args_allowed_dirs, + validate_return_value, +) +from openstaad_mcp.file_io.writers import CSVWriter + +__all__ = [ + "CSVReader", + "CSVWriter", + "XLSXReader", + "deep_freeze", + "get_allowed_dirs", + "get_input_data", + "read_input_file", + "validate_args_allowed_dirs", + "validate_return_value", + "write_output_file", +] diff --git a/src/openstaad_mcp/file_io/const.py b/src/openstaad_mcp/file_io/const.py new file mode 100644 index 0000000..f857099 --- /dev/null +++ b/src/openstaad_mcp/file_io/const.py @@ -0,0 +1,38 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- +""" +# ── File I/O limits ────────────────────────────────────────────────────── + +# Maximum file size on disk (bytes) for read operations. +MAX_FILE_SIZE_BYTES = 50 * 1024 * 1024 # 50 MB + +# Row / column / sheet caps for input files. +MAX_INPUT_ROWS = 100_000 +MAX_INPUT_COLUMNS = 500 +MAX_INPUT_SHEETS = 50 + +# Row / column / sheet caps for output (return value → file). +MAX_OUTPUT_ROWS = 100_000 +MAX_OUTPUT_COLUMNS = 500 +MAX_OUTPUT_SHEETS = 20 + +# Excel's own limit on sheet-tab names. +MAX_SHEET_NAME_LENGTH = 31 + +# Allowed file extensions for file I/O paths. +ALLOWED_FILE_EXTENSIONS: frozenset[str] = frozenset({".csv", ".xlsx"}) + +# Temp-file naming for atomic writes. +TEMP_FILE_PREFIX = ".~omcp_" + +# Orphan temp files older than this (seconds) are cleaned up. +STALE_TEMP_AGE_SECONDS = 3600 + +# Number of sample rows included in summaries returned to the agent. +SAMPLE_ROW_COUNT = 5 + +# Maximum character length for a single cell value (matches Excel's limit). +MAX_CELL_SIZE = 32_768 diff --git a/src/openstaad_mcp/file_io/helpers.py b/src/openstaad_mcp/file_io/helpers.py new file mode 100644 index 0000000..ec35dcc --- /dev/null +++ b/src/openstaad_mcp/file_io/helpers.py @@ -0,0 +1,107 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +Server-level helpers and public dispatch functions for file I/O.""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import Any + +from fastmcp.server.context import Context +from mcp.shared.exceptions import McpError + +from openstaad_mcp.file_io.path_validator import FileIOError, parse_roots_to_dirs, validate_io_path +from openstaad_mcp.file_io.readers import BaseReader, CSVReader, XLSXReader +from openstaad_mcp.file_io.validation import deep_freeze, validate_return_value +from openstaad_mcp.file_io.writers import BaseWriter, CSVWriter, XLSXWriter + +logger = logging.getLogger(__name__) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Reader/writer factory helpers +# ═══════════════════════════════════════════════════════════════════════════ + +_READERS: dict[str, type[BaseReader]] = {".csv": CSVReader, ".xlsx": XLSXReader} +_WRITERS: dict[str, type[BaseWriter]] = {".csv": CSVWriter, ".xlsx": XLSXWriter} + + +def _get_reader(path: Path) -> BaseReader: + ext = path.suffix.lower() + cls = _READERS.get(ext) + if cls is None: + raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot read '{ext}' files") + return cls(path) + + +def _get_writer(path: Path) -> BaseWriter: + ext = path.suffix.lower() + cls = _WRITERS.get(ext) + if cls is None: + raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot write '{ext}' files") + return cls(path) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Public dispatch functions +# ═══════════════════════════════════════════════════════════════════════════ + + +def read_input_file( + path: Path, + *, + sheet: str | None = None, + start_row: int = 0, + max_rows: int | None = None, + has_header: bool | None = None, +) -> tuple[Any, dict[str, Any]]: + """Read a CSV or XLSX file and return ``(data, summary)``.""" + reader = _get_reader(path) + data = reader.read(start_row=start_row, max_rows=max_rows, sheet=sheet, has_header=has_header) + summary = reader.build_summary(data) + return data, summary + + +def write_output_file(path: str, data: Any, allowed_dirs: list[Path], *, overwrite: bool = False) -> dict[str, Any]: + """Validate path, validate return value, write atomically, return summary.""" + resolved = validate_io_path(path, allowed_dirs, mode="write") + validate_return_value(data) + writer = _get_writer(resolved) + return writer.write(data, overwrite=overwrite) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Server-level helpers (called from server.py) +# ═══════════════════════════════════════════════════════════════════════════ + + +async def get_allowed_dirs( + ctx: Context, args_allowed_dirs: list[Path], input_path: str | None, output_path: str | None +) -> list[Path]: + """Resolve MCP roots into a list of allowed directories.""" + logger.debug("Args allowed dirs: %s", args_allowed_dirs) + allowed_dirs: list[Path] = [Path(el) for el in args_allowed_dirs] + if input_path is not None or output_path is not None: + try: + roots = await ctx.list_roots() + logger.debug(f"Received MCP roots: {roots}") + except McpError as exc: + logger.error(f"Error listing MCP roots: {exc}") + roots = [] + allowed_dirs += parse_roots_to_dirs(roots) + logger.debug(f"Allowed directories for file I/O: {allowed_dirs}") + return allowed_dirs + + +async def get_input_data(input_path: str | None, allowed_dirs: list[Path]) -> tuple[Any, dict[str, Any] | None]: + """Validate path, read file, freeze data. Returns ``(data, summary)``.""" + if input_path is None: + return None, None + resolved_input = validate_io_path(input_path, allowed_dirs, mode="read") + data, input_summary = read_input_file(resolved_input) + return deep_freeze(data), input_summary diff --git a/src/openstaad_mcp/file_io/models.py b/src/openstaad_mcp/file_io/models.py new file mode 100644 index 0000000..04caacb --- /dev/null +++ b/src/openstaad_mcp/file_io/models.py @@ -0,0 +1,140 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +Pydantic models for file I/O return-value validation. +""" + +from __future__ import annotations + +from typing import Annotated, Any + +from pydantic import BaseModel, RootModel, field_validator, model_validator + +from openstaad_mcp.file_io.const import ( + MAX_CELL_SIZE, + MAX_OUTPUT_COLUMNS, + MAX_OUTPUT_ROWS, + MAX_OUTPUT_SHEETS, + MAX_SHEET_NAME_LENGTH, +) + +# --------------------------------------------------------------------------- +# Cell value type — JSON primitives with max string length +# --------------------------------------------------------------------------- + +CellValue = str | int | float | bool | None + + +def check_cell(v: Any) -> CellValue: + """Validate a single cell value: must be a JSON primitive with bounded string length.""" + + if isinstance(v, (bool, int, float)): + return v + if isinstance(v, str): + if len(v) > MAX_CELL_SIZE: + raise ValueError(f"String too long: {len(v)}; limit {MAX_CELL_SIZE}") + return v + if v is None: + return v + msg = f"Cell value must be a JSON primitive, got {type(v).__name__}" + raise ValueError(msg) + + +# --------------------------------------------------------------------------- +# Row type — a list (or tuple) of cell values +# --------------------------------------------------------------------------- + +Row = Annotated[list[CellValue], "A row of cell values"] + + +# --------------------------------------------------------------------------- +# Flat output (CSV / single-sheet) +# --------------------------------------------------------------------------- + + +class FlatOutput(RootModel[list[Row]]): + """Flat tabular output: list of rows, each row a list of JSON-primitive cells.""" + + @model_validator(mode="before") + @classmethod + def _coerce_sequences(cls, v: Any) -> Any: + """Accept tuples (from deep_freeze) as rows.""" + if isinstance(v, (list, tuple)): + return [list(row) if isinstance(row, (list, tuple)) else row for row in v] + return v + + @model_validator(mode="after") + def _check_limits(self) -> FlatOutput: + rows = self.root + if len(rows) > MAX_OUTPUT_ROWS + 1: # +1 header + msg = f"Too many rows: {len(rows)}; limit {MAX_OUTPUT_ROWS}" + raise ValueError(msg) + for row in rows: + if len(row) > MAX_OUTPUT_COLUMNS: + msg = f"Too many columns: {len(row)}; limit {MAX_OUTPUT_COLUMNS}" + raise ValueError(msg) + for cell in row: + check_cell(cell) + return self + + +# --------------------------------------------------------------------------- +# Multi-sheet output (XLSX) +# --------------------------------------------------------------------------- + + +class SheetData(BaseModel): + """A single sheet's data: column headers and rows of cells.""" + + columns: list[CellValue] + rows: list[Row] + + @model_validator(mode="before") + @classmethod + def _coerce_sequences(cls, v: Any) -> Any: + """Accept tuples (from deep_freeze) as columns/rows.""" + if isinstance(v, dict): + data = dict(v) + if "columns" in data and isinstance(data["columns"], tuple): + data["columns"] = list(data["columns"]) + if "rows" in data and isinstance(data["rows"], (list, tuple)): + data["rows"] = [list(r) if isinstance(r, (list, tuple)) else r for r in data["rows"]] + return data + return v + + @model_validator(mode="after") + def _check_limits(self) -> SheetData: + all_rows = [self.columns, *self.rows] + for row in all_rows: + if len(row) > MAX_OUTPUT_COLUMNS: + msg = f"Too many columns: {len(row)}; limit {MAX_OUTPUT_COLUMNS}" + raise ValueError(msg) + for cell in row: + check_cell(cell) + if len(self.rows) > MAX_OUTPUT_ROWS: + msg = f"Too many rows: {len(self.rows)}; limit {MAX_OUTPUT_ROWS}" + raise ValueError(msg) + return self + + +class MultiSheetOutput(RootModel[dict[str, SheetData]]): + """Multi-sheet output keyed by sheet name.""" + + @field_validator("root", mode="before") + @classmethod + def _check_sheet_count(cls, v: Any) -> Any: + if isinstance(v, dict) and len(v) > MAX_OUTPUT_SHEETS: + msg = f"Too many sheets: {len(v)}; limit {MAX_OUTPUT_SHEETS}" + raise ValueError(msg) + return v + + @model_validator(mode="after") + def _check_sheet_names(self) -> MultiSheetOutput: + for name in self.root: + if len(name) > MAX_SHEET_NAME_LENGTH: + msg = f"Sheet name '{name}' exceeds {MAX_SHEET_NAME_LENGTH} characters" + raise ValueError(msg) + return self diff --git a/src/openstaad_mcp/file_io/path_validator.py b/src/openstaad_mcp/file_io/path_validator.py new file mode 100644 index 0000000..f3c8590 --- /dev/null +++ b/src/openstaad_mcp/file_io/path_validator.py @@ -0,0 +1,150 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +Shared path validator for file I/O operations. + +Validates that a file path is safe to read from or write to, using +MCP roots as the containment boundary. Both ``input_path`` and +``output_path`` go through :func:`validate_io_path` — one function, +not duplicated logic. + +Validation order (do **not** reorder — see Research-file-io.md §5.2): +1. Roots guard — at least one allowed directory must be provided. +2. Resolve — canonicalise the path (collapse ``..``, follow symlinks). +3. UNC reject — resolved path must not be a network path. +4. Containment — resolved path must be inside at least one allowed dir. +5. Extension — must be ``.csv`` or ``.xlsx``. +6. Existence — read: file must exist; write: parent dir must exist. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from typing import Literal +from urllib.parse import unquote, urlparse + +from openstaad_mcp.file_io.const import ALLOWED_FILE_EXTENSIONS + +_UNC_RE = re.compile(r"^(?:\\\\|//)", re.ASCII) + + +class FileIOError(Exception): + """Structured error with a machine-readable ``code`` and human-readable ``message``.""" + + def __init__(self, code: str, message: str) -> None: + self.code = code + self.message = message + super().__init__(f"{code}: {message}") + + +def parse_roots_to_dirs(roots: list) -> list[Path]: + """Convert MCP ``Root`` objects (with ``file://`` URIs) to local ``Path`` instances. + + Non-``file://`` URIs are silently skipped. + """ + dirs: list[Path] = [] + for root in roots: + uri: str = root.uri if hasattr(root, "uri") else str(root) + parsed = urlparse(uri) + if parsed.scheme != "file": + continue + # RFC 8089: file:///C:/path → parsed.path = "/C:/path" + # The leading "/" is an artifact of the URI authority; strip it + # before the Windows drive letter so Path() resolves correctly. + local = unquote(parsed.path) + if local.startswith("/") and len(local) > 2 and local[2] == ":": + local = local[1:] # /C:/foo → C:/foo + dirs.append(Path(local)) + return dirs + + +def validate_io_path( + raw_path: str, + allowed_dirs: list[Path], + *, + mode: Literal["read", "write"], +) -> Path: + """Validate *raw_path* for a file I/O operation and return the resolved path. + + Parameters + ---------- + raw_path: + The user-supplied file path (may be relative or contain ``..``). + allowed_dirs: + Directories the server is allowed to access (from MCP roots). + mode: + ``"read"`` requires the file to exist; ``"write"`` requires the + parent directory to exist. + + Returns + ------- + Path + The resolved, canonical path. + + Raises + ------ + FileIOError + With a machine-readable ``code`` describing the failure. + """ + # ── 1. Roots guard ─────────────────────────────────────────────── + if not allowed_dirs: + raise FileIOError( + "NO_ROOTS", + "No allowed directories configured. Please instruct your user to go update their settings for that extension", + ) + + # Reject null bytes early (before Path() which may raise on some OSes). + if "\x00" in raw_path: + raise FileIOError("UNSUPPORTED_FORMAT", "Null bytes are not allowed in file paths") + + # ── 2. Resolve ─────────────────────────────────────────────────── + try: + resolved = Path(raw_path).resolve() + except (OSError, ValueError) as exc: + raise FileIOError("UNSUPPORTED_FORMAT", f"Invalid path: {exc}") from None + + # ── 3. UNC reject ──────────────────────────────────────────────── + resolved_str = str(resolved) + if _UNC_RE.match(resolved_str): + raise FileIOError("UNC_REJECTED", "Network paths (UNC) are not allowed") + + # ── 4. Containment ─────────────────────────────────────────────── + inside_any = False + for root_dir in allowed_dirs: + try: + resolved.relative_to(root_dir.resolve()) + inside_any = True + break + except ValueError: + continue + if not inside_any: + raise FileIOError( + "PATH_OUTSIDE_ROOTS", + f"Path is outside all allowed directories: {resolved}", + ) + + # ── 5. Extension ───────────────────────────────────────────────── + ext = resolved.suffix.lower() + if ext not in ALLOWED_FILE_EXTENSIONS: + allowed = ", ".join(sorted(ALLOWED_FILE_EXTENSIONS)) + raise FileIOError( + "UNSUPPORTED_FORMAT", + f"Extension '{ext}' is not supported. Allowed: {allowed}", + ) + + # ── 6. Existence ───────────────────────────────────────────────── + if mode == "read": + if not resolved.is_file(): + raise FileIOError("FILE_NOT_FOUND", f"File does not exist: {resolved}") + else: # write + if not resolved.parent.is_dir(): + raise FileIOError( + "PARENT_DIR_MISSING", + f"Parent directory does not exist: {resolved.parent}", + ) + + return resolved diff --git a/src/openstaad_mcp/file_io/readers.py b/src/openstaad_mcp/file_io/readers.py new file mode 100644 index 0000000..d3a90e7 --- /dev/null +++ b/src/openstaad_mcp/file_io/readers.py @@ -0,0 +1,374 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +File readers: CSV and XLSX. +""" + +from __future__ import annotations + +import abc +import csv +from datetime import date, datetime +from datetime import time as dt_time +from pathlib import Path +from typing import Any + +import chardet +import openpyxl + +from openstaad_mcp.file_io.const import ( + MAX_FILE_SIZE_BYTES, + MAX_INPUT_COLUMNS, + MAX_INPUT_ROWS, + MAX_INPUT_SHEETS, + SAMPLE_ROW_COUNT, +) +from openstaad_mcp.file_io.models import check_cell +from openstaad_mcp.file_io.path_validator import FileIOError + +# ── Header detection ───────────────────────────────────────────────────── + + +def _cell_type(value: Any) -> str: + """Classify a cell value for header-detection comparison.""" + if value is None: + return "null" + if isinstance(value, bool): + return "bool" + if isinstance(value, (int, float)): + return "numeric" + if isinstance(value, (datetime, date, dt_time)): + return "numeric" + return "string" + + +def _detect_header(rows: list[list], has_header: bool | None) -> bool: + """Detect whether the first row is a header. + + When *has_header* is ``None`` (auto-detect), samples up to 5 rows and + compares the per-column type of row 0 against the majority type of + rows 1-4. If any column's first-row type differs from its data-row + majority, the first row is treated as a header. + """ + if has_header is not None: + return has_header + + if len(rows) <= 1: + return True # Too few rows to compare — conservative default + + first_row = rows[0] + data_rows = rows[1:5] # Up to 4 data rows for comparison + + if not first_row: + return True + + for col_idx in range(len(first_row)): + first_type = _cell_type(first_row[col_idx]) + if first_type == "null": + continue + + type_counts: dict[str, int] = {} + for row in data_rows: + if col_idx < len(row): + t = _cell_type(row[col_idx]) + if t != "null": + type_counts[t] = type_counts.get(t, 0) + 1 + + if not type_counts: + continue + + majority_type = max(type_counts, key=type_counts.get) + if first_type != majority_type: + return True # Type mismatch -> first row is a header + + return False # All columns match -> not a header + + +def _auto_columns(num_cols: int) -> list[str]: + """Generate column names ``col_1, col_2, ...`` for headerless data.""" + return [f"col_{i + 1}" for i in range(num_cols)] + + +# ═══════════════════════════════════════════════════════════════════════════ +# Base reader +# ═══════════════════════════════════════════════════════════════════════════ + + +class BaseReader(abc.ABC): + """Base class for file readers. Enforces file-size and limit checks.""" + + def __init__(self, path: Path) -> None: + self.path = path + self._has_header: bool = True + self._check_file_size() + + def _check_file_size(self) -> None: + size = self.path.stat().st_size + if size > MAX_FILE_SIZE_BYTES: + raise FileIOError( + "FILE_TOO_LARGE", + f"File is {size:,} bytes; limit is {MAX_FILE_SIZE_BYTES:,} bytes", + ) + + @abc.abstractmethod + def read(self, *, start_row: int = 0, max_rows: int | None = None, **kwargs: Any) -> Any: + """Parse the file and return structured data.""" + + @abc.abstractmethod + def build_summary(self, data: Any) -> dict[str, Any]: + """Build a lightweight summary for the agent.""" + + +# ═══════════════════════════════════════════════════════════════════════════ +# CSV reader +# ═══════════════════════════════════════════════════════════════════════════ + + +class CSVReader(BaseReader): + """Reads a CSV file into ``list[list]`` (array-of-arrays). + + Uses ``chardet`` for encoding detection and ``csv.Sniffer`` for + dialect detection. Values are coerced from strings to int/float + where possible. + """ + + _CHARDET_MIN_CONFIDENCE = 0.5 + + def read( + self, *, start_row: int = 0, max_rows: int | None = None, has_header: bool | None = None, **kwargs: Any + ) -> list[list]: + encoding = self._detect_encoding() + dialect = self._detect_dialect(encoding) + all_rows: list[list] = [] + + with open(self.path, newline="", encoding=encoding) as f: + reader = csv.reader(f, dialect) + try: + for raw_row in reader: + coerced = [_coerce_csv_value(v) for v in raw_row] + if len(coerced) > MAX_INPUT_COLUMNS: + raise FileIOError( + "TOO_MANY_COLUMNS", + f"Row has {len(coerced)} columns; limit is {MAX_INPUT_COLUMNS}", + ) + for cell in coerced: + try: + check_cell(cell) + except ValueError as exc: + raise FileIOError("INVALID_CELL", f"Invalid cell value: {exc}") from exc + all_rows.append(coerced) + except csv.Error as exc: + raise FileIOError("CSV_PARSE_ERROR", str(exc)) from None + + self._has_header = _detect_header(all_rows, has_header) + + if self._has_header: + if not all_rows: + return [] + data_rows = all_rows[1:] + if len(data_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"File has {len(data_rows)} data rows; limit is {MAX_INPUT_ROWS}", + ) + sliced = data_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + return [all_rows[0], *sliced] if start_row == 0 else sliced + else: + if len(all_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"File has {len(all_rows)} data rows; limit is {MAX_INPUT_ROWS}", + ) + sliced = all_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + return sliced + + def build_summary(self, data: list[list]) -> dict[str, Any]: + if self._has_header: + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + else: + num_cols = len(data[0]) if data else 0 + header = _auto_columns(num_cols) + data_rows = data + return { + "total_rows": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + # -- helpers -- + + def _detect_encoding(self) -> str: + """Detect file encoding: try UTF-8 first, then chardet, then cp1252.""" + raw = self.path.read_bytes() + result = chardet.detect(raw) + encoding = result.get("encoding") + confidence = result.get("confidence", 0) + if encoding and confidence >= self._CHARDET_MIN_CONFIDENCE: + return encoding + return "cp1252" + + def _detect_dialect(self, encoding: str) -> type[csv.Dialect]: + """Detect CSV dialect using csv.Sniffer, fall back to ``excel``. + + Only trusts the sniffer when the detected delimiter is a common + separator character. Exotic delimiters (letters, digits, etc.) + indicate a false positive from a small or ambiguous sample. + """ + _COMMON_DELIMITERS = {",", ";", "\t", "|"} + try: + with open(self.path, newline="", encoding=encoding) as f: + sample = f.read(8192) + dialect = csv.Sniffer().sniff(sample) + if dialect.delimiter in _COMMON_DELIMITERS: + return dialect + except csv.Error: + pass + return csv.excel + + +def _coerce_csv_value(val: str) -> int | float | str: + """Attempt int -> float -> str coercion of a CSV string value.""" + try: + return int(val) + except ValueError: + pass + try: + return float(val) + except ValueError: + pass + return val + + +# ═══════════════════════════════════════════════════════════════════════════ +# XLSX reader +# ═══════════════════════════════════════════════════════════════════════════ + + +class XLSXReader(BaseReader): + """Reads an XLSX workbook into ``{sheet_name: {columns, rows}}``.""" + + def read( + self, + *, + start_row: int = 0, + max_rows: int | None = None, + sheet: str | None = None, + has_header: bool | None = None, + **kwargs: Any, + ) -> dict[str, dict[str, Any]]: + try: + wb = openpyxl.load_workbook(self.path, read_only=True, data_only=True) + except Exception as exc: + raise FileIOError("CORRUPTED_WORKBOOK", f"Cannot open workbook: {exc}") from None + + try: + self._validate_sheet_count(wb) + sheets_to_load = self._resolve_sheets(wb, sheet) + return {name: self._read_sheet(wb[name], name, start_row, max_rows, has_header) for name in sheets_to_load} + finally: + wb.close() + + def build_summary(self, data: dict[str, dict[str, Any]]) -> dict[str, Any]: + sheets = list(data.keys()) + first_sheet = sheets[0] if sheets else None + first = data[first_sheet] if first_sheet else {"columns": [], "rows": []} + return { + "sheets": sheets, + "loaded_sheet": first_sheet, + "total_rows": len(first["rows"]), + "columns": list(first["columns"]), + "sample_rows": [list(r) for r in first["rows"][:SAMPLE_ROW_COUNT]], + } + + # -- helpers -- + + @staticmethod + def _validate_sheet_count(wb: Any) -> None: + if len(wb.sheetnames) > MAX_INPUT_SHEETS: + raise FileIOError( + "TOO_MANY_ROWS", + f"Workbook has {len(wb.sheetnames)} sheets; limit is {MAX_INPUT_SHEETS}", + ) + + @staticmethod + def _resolve_sheets(wb: Any, sheet: str | None) -> list[str]: + if sheet is not None: + if sheet not in wb.sheetnames: + raise FileIOError("SHEET_NOT_FOUND", f"Sheet '{sheet}' not found in workbook") + return [sheet] + return list(wb.sheetnames) + + @staticmethod + def _read_sheet( + ws: Any, name: str, start_row: int, max_rows: int | None, has_header: bool | None + ) -> dict[str, Any]: + raw_rows: list[list] = [] + for row in ws.iter_rows(values_only=True): + raw_rows.append(list(row)) + + if raw_rows and len(raw_rows[0]) > MAX_INPUT_COLUMNS: + raise FileIOError( + "TOO_MANY_COLUMNS", + f"Sheet '{name}' has {len(raw_rows[0])} columns; limit is {MAX_INPUT_COLUMNS}", + ) + + # Detect header using raw types (before datetime -> string conversion) + is_header = _detect_header(raw_rows, has_header) + + # Convert to JSON primitives and validate cell sizes + all_rows: list[list] = [] + for row in raw_rows: + converted = [_to_json_primitive(c) for c in row] + for cell in converted: + try: + check_cell(cell) + except ValueError as exc: + raise FileIOError("INVALID_CELL", f"Invalid cell value: {exc}") from exc + all_rows.append(converted) + + if is_header: + columns = all_rows[0] if all_rows else [] + data_rows = all_rows[1:] + else: + num_cols = len(all_rows[0]) if all_rows else 0 + columns = _auto_columns(num_cols) + data_rows = all_rows + + if len(data_rows) > MAX_INPUT_ROWS: + raise FileIOError( + "TOO_MANY_ROWS", + f"Sheet '{name}' exceeds {MAX_INPUT_ROWS} rows", + ) + + sliced = data_rows[start_row:] + if max_rows is not None: + sliced = sliced[:max_rows] + + return {"columns": columns, "rows": sliced} + + +def _to_json_primitive(value: Any) -> str | int | float | bool | None: + """Convert an openpyxl cell value to a JSON-safe primitive.""" + if value is None: + return None + if isinstance(value, bool): + return value + if isinstance(value, (int, float)): + return value + if isinstance(value, str): + return value + if isinstance(value, datetime): + return value.isoformat() + if isinstance(value, date): + return value.isoformat() + if isinstance(value, dt_time): + return value.isoformat() + return str(value) diff --git a/src/openstaad_mcp/file_io/validation.py b/src/openstaad_mcp/file_io/validation.py new file mode 100644 index 0000000..83ce4d2 --- /dev/null +++ b/src/openstaad_mcp/file_io/validation.py @@ -0,0 +1,99 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +Validation and data-freezing utilities for file I/O. +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Any + +from pydantic import ValidationError + +from openstaad_mcp.file_io.models import FlatOutput, MultiSheetOutput +from openstaad_mcp.file_io.path_validator import FileIOError + + +def validate_return_value(value: Any) -> None: + """Raise :class:`FileIOError` if *value* is not a valid output structure. + + Accepts: + - ``list[list[primitive]]`` (flat / CSV / single-sheet) + - ``dict[str, {columns: list, rows: list[list[primitive]]}]`` (multi-sheet) + + Tuples are accepted interchangeably with lists (sandbox returns frozen data). + """ + if isinstance(value, (list, tuple)): + try: + FlatOutput.model_validate(value) + except ValidationError as exc: + raise FileIOError("INVALID_RETURN_SHAPE", _format_errors(exc)) from None + elif isinstance(value, dict): + try: + MultiSheetOutput.model_validate(value) + except ValidationError as exc: + raise FileIOError("INVALID_RETURN_SHAPE", _format_errors(exc)) from None + else: + raise FileIOError( + "INVALID_RETURN_SHAPE", + "Return value must be a list of lists (flat) or a dict of sheets (multi-sheet)", + ) + + +def _format_errors(exc: ValidationError) -> str: + """Format all validation errors into a single human-readable message.""" + errors = exc.errors() + if not errors: + return str(exc) + parts = [] + for err in errors: + loc = " -> ".join(str(part) for part in err.get("loc", ())) + msg = err.get("msg", "") + parts.append(f"{loc}: {msg}" if loc else msg) + return "; ".join(parts) + + +def validate_args_allowed_dirs(allowed_dirs: list[str] | None) -> list[Path]: + """Validate and resolve ``--allowed-dir`` CLI arguments to real paths. + + Security: resolves symlinks so later checks compare against real paths. + """ + if not allowed_dirs: + return [] + + result: list[Path] = [] + for dir_str in allowed_dirs: + expanded = Path(dir_str).expanduser() + absolute = expanded.resolve(strict=False) + normalized_original = Path(os.path.normpath(absolute)) + + try: + resolved = absolute.resolve(strict=True) + normalized_resolved = Path(os.path.normpath(resolved)) + result.append(normalized_resolved) + except OSError: + result.append(normalized_original) + + return result + + +def deep_freeze(data: Any) -> Any: + """Recursively convert mutable containers to immutable equivalents. + + - ``list`` -> ``tuple`` + - ``dict`` values are recursively frozen (dict keys stay as-is since + strings are already immutable) + - Primitives (str, int, float, bool, None) pass through unchanged. + """ + if data is None or isinstance(data, (str, int, float, bool)): + return data + if isinstance(data, (list, tuple)): + return tuple(deep_freeze(item) for item in data) + if isinstance(data, dict): + return {k: deep_freeze(v) for k, v in data.items()} + return data diff --git a/src/openstaad_mcp/file_io/writers.py b/src/openstaad_mcp/file_io/writers.py new file mode 100644 index 0000000..354ab03 --- /dev/null +++ b/src/openstaad_mcp/file_io/writers.py @@ -0,0 +1,155 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + +File writers: CSV and XLSX. +""" + +from __future__ import annotations + +import abc +import csv +import os +import time +import uuid +from pathlib import Path +from typing import Any + +import openpyxl + +from openstaad_mcp.file_io.const import ( + SAMPLE_ROW_COUNT, + STALE_TEMP_AGE_SECONDS, + TEMP_FILE_PREFIX, +) +from openstaad_mcp.file_io.path_validator import FileIOError + +# ═══════════════════════════════════════════════════════════════════════════ +# Base writer +# ═══════════════════════════════════════════════════════════════════════════ + + +class BaseWriter(abc.ABC): + """Base class for file writers. Handles atomic writes via temp file.""" + + def __init__(self, path: Path) -> None: + self.path = path + + def write(self, data: Any, *, overwrite: bool = False) -> dict[str, Any]: + """Validate, write atomically, and return a summary.""" + if self.path.exists() and not overwrite: + raise FileIOError("FILE_EXISTS", f"File already exists: {self.path}") + _clean_stale_temps(self.path.parent) + + tmp = _temp_path(self.path.parent) + try: + self._write_to(tmp, data) + os.replace(tmp, self.path) + except BaseException: + tmp.unlink(missing_ok=True) + raise + return self.build_summary(data) + + @abc.abstractmethod + def _write_to(self, tmp: Path, data: Any) -> None: + """Write *data* to the temporary file *tmp*.""" + + @abc.abstractmethod + def build_summary(self, data: Any) -> dict[str, Any]: + """Build a lightweight summary for the agent.""" + + +# ═══════════════════════════════════════════════════════════════════════════ +# CSV writer +# ═══════════════════════════════════════════════════════════════════════════ + + +class CSVWriter(BaseWriter): + """Writes ``list[list]`` to a CSV file.""" + + def _write_to(self, tmp: Path, data: list[list]) -> None: + with open(tmp, "w", newline="", encoding="utf-8") as f: + csv.writer(f).writerows(data) + + def build_summary(self, data: list[list]) -> dict[str, Any]: + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + return { + "message": f"The `result` data has been written to `{self.path}`", + "rows_written": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + +# ═══════════════════════════════════════════════════════════════════════════ +# XLSX writer +# ═══════════════════════════════════════════════════════════════════════════ + + +class XLSXWriter(BaseWriter): + """Writes flat or multi-sheet data to an XLSX file.""" + + def _write_to(self, tmp: Path, data: Any) -> None: + wb = openpyxl.Workbook() + if isinstance(data, dict): + for i, (name, sheet_data) in enumerate(data.items()): + if i == 0: + ws = wb.active + assert ws is not None + ws.title = name + else: + ws = wb.create_sheet(title=name) + ws.append(sheet_data["columns"]) + for row in sheet_data["rows"]: + ws.append(row) + else: + ws = wb.active + assert ws is not None + for row in data: + ws.append(row) + wb.save(tmp) + + def build_summary(self, data: Any) -> dict[str, Any]: + if isinstance(data, dict): + return { + "message": f"The `result` data has been written to `{self.path}`", + "sheets": { + name: { + "columns": sheet["columns"], + "rows_written": len(sheet["rows"]), + "sample_rows": [list(r) for r in sheet["rows"][:SAMPLE_ROW_COUNT]], + } + for name, sheet in data.items() + }, + } + header = data[0] if data else [] + data_rows = data[1:] if len(data) > 1 else [] + return { + "message": f"The `result` data has been written to `{self.path}`", + "rows_written": len(data_rows), + "columns": list(header), + "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], + } + + +# ═══════════════════════════════════════════════════════════════════════════ +# Shared utilities +# ═══════════════════════════════════════════════════════════════════════════ + + +def _temp_path(directory: Path) -> Path: + return directory / f"{TEMP_FILE_PREFIX}{uuid.uuid4().hex}.tmp" + + +def _clean_stale_temps(directory: Path) -> None: + """Remove orphaned temp files older than ``STALE_TEMP_AGE_SECONDS``.""" + cutoff = time.time() - STALE_TEMP_AGE_SECONDS + for p in directory.glob(f"{TEMP_FILE_PREFIX}*.tmp"): + try: + if p.stat().st_mtime < cutoff: + p.unlink() + except OSError: + pass diff --git a/src/openstaad_mcp/main.py b/src/openstaad_mcp/main.py index 238d4f7..ee1f18b 100644 --- a/src/openstaad_mcp/main.py +++ b/src/openstaad_mcp/main.py @@ -20,6 +20,7 @@ from starlette.middleware import Middleware from starlette.middleware.trustedhost import TrustedHostMiddleware +from openstaad_mcp.file_io import validate_args_allowed_dirs from openstaad_mcp.http_middleware import SecFetchMiddleware from openstaad_mcp.server import create_mcp_server @@ -51,6 +52,13 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: choices=["DEBUG", "INFO", "WARNING", "ERROR"], help="Logging verbosity (default: INFO)", ) + parser.add_argument( + "--allowed-dirs", + type=str, + nargs="+", + default=None, + help="Directories the openSTAAD server can access (for MCP clients that don't support roots; space separated list)", + ) # ── HTTP-only options ───────────────────────────────────────── parser.add_argument( @@ -96,10 +104,11 @@ def main(argv: list[str] | None = None) -> None: args = parse_args(argv) setup_logging(args.log_level) + allowed_dirs = validate_args_allowed_dirs(args.allowed_dirs) if args.transport == "stdio": # Run FastMCP server in the main thread, the COM thread will be started by the lifespan. - mcp = create_mcp_server() + mcp = create_mcp_server(allowed_dirs) try: mcp.run(transport="stdio", show_banner=False) except KeyboardInterrupt: @@ -119,7 +128,7 @@ def main(argv: list[str] | None = None) -> None: required_scopes=["read:data"], ) } - mcp = create_mcp_server(fastmcp_kwargs=fastmcp_kwargs) + mcp = create_mcp_server(allowed_dirs, fastmcp_kwargs=fastmcp_kwargs) try: mcp.run( transport="http", diff --git a/src/openstaad_mcp/sandbox/ast.py b/src/openstaad_mcp/sandbox/ast.py index 5ea159e..1691a4f 100644 --- a/src/openstaad_mcp/sandbox/ast.py +++ b/src/openstaad_mcp/sandbox/ast.py @@ -15,7 +15,7 @@ import re from dataclasses import dataclass, field -from openstaad_mcp.sandbox.const import ALLOWED_MODULE_ATTRS, BLOCKED_ATTRS, BLOCKED_BUILTINS +from openstaad_mcp.sandbox.const import ALLOWED_DUNDER_NAMES, ALLOWED_MODULE_ATTRS, BLOCKED_ATTRS, BLOCKED_BUILTINS _FORMAT_DUNDER_RE = re.compile(r"\{[^}]*\.__[a-zA-Z_][a-zA-Z0-9_]*__") @@ -109,7 +109,7 @@ def visit_Attribute(self, node: ast.Attribute) -> None: def visit_Name(self, node: ast.Name) -> None: if node.id in BLOCKED_BUILTINS: self._err(node, f"reference to '{node.id}' is not allowed") - if node.id.startswith("__") and node.id.endswith("__"): + if node.id.startswith("__") and node.id.endswith("__") and node.id not in ALLOWED_DUNDER_NAMES: self._err(node, f"reference to dunder name '{node.id}' is not allowed") self.generic_visit(node) diff --git a/src/openstaad_mcp/sandbox/com_proxy.py b/src/openstaad_mcp/sandbox/com_proxy.py index d9818fd..6a28a77 100644 --- a/src/openstaad_mcp/sandbox/com_proxy.py +++ b/src/openstaad_mcp/sandbox/com_proxy.py @@ -1,4 +1,9 @@ """ +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- + COM object proxy for the sandbox. Wraps pywin32 CDispatch objects to block access to internal attributes diff --git a/src/openstaad_mcp/sandbox/const.py b/src/openstaad_mcp/sandbox/const.py index 62104fe..b930828 100644 --- a/src/openstaad_mcp/sandbox/const.py +++ b/src/openstaad_mcp/sandbox/const.py @@ -1,3 +1,10 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- +""" + # Allowed builtins exceptions. # Excludes BaseException and its direct subclasses (SystemExit, # KeyboardInterrupt, GeneratorExit) to prevent interference with executor @@ -200,6 +207,11 @@ } ) +INPUT_DATA_VARIABLE_NAME = "__input__" + +# Sandbox-injected variables that are exempt from the dunder-name ban. +ALLOWED_DUNDER_NAMES: frozenset[str] = frozenset({INPUT_DATA_VARIABLE_NAME}) + # Per-module attribute whitelists for modules injected into the sandbox. # Used both here (AST-level static check) and in executor.py (_ModuleProxy # runtime enforcement). Only attributes listed here may be accessed on the diff --git a/src/openstaad_mcp/sandbox/executor.py b/src/openstaad_mcp/sandbox/executor.py index 38bdf3d..c111f77 100644 --- a/src/openstaad_mcp/sandbox/executor.py +++ b/src/openstaad_mcp/sandbox/executor.py @@ -26,7 +26,7 @@ from openstaad_mcp.sandbox.ast import capture_last_expr, validate_code from openstaad_mcp.sandbox.com_proxy import COMProxy -from openstaad_mcp.sandbox.const import ALLOWED_BUILTINS, ALLOWED_MODULE_ATTRS +from openstaad_mcp.sandbox.const import ALLOWED_BUILTINS, ALLOWED_MODULE_ATTRS, INPUT_DATA_VARIABLE_NAME from openstaad_mcp.sandbox.module_proxy import ModuleProxy from openstaad_mcp.sandbox.stdio_helpers import LimitedStringIO, sanitize_output, sanitize_traceback @@ -82,6 +82,8 @@ def execute( self, code: str, staad_object: Any, + *, + input_data: Any = None, ) -> ExecutionResult: """Validate and execute *code* in the sandbox. @@ -91,6 +93,9 @@ def execute( Python source code to execute. staad_object: The connected OpenSTAAD root object (or a mock for testing). + input_data: + Optional pre-parsed, deep-frozen data injected as ``__input__`` + in the sandbox globals. ``None`` when no input file is provided. Returns ------- @@ -111,6 +116,7 @@ def execute( sandbox_globals: dict[str, Any] = {"__builtins__": self.safe_builtins.copy()} sandbox_globals.update(self.injected_modules) sandbox_globals["staad"] = COMProxy(staad_object) + sandbox_globals[INPUT_DATA_VARIABLE_NAME] = input_data # ── 4. Execute with stdout/stderr capture ─────────────────── captured_out, captured_err = LimitedStringIO(), LimitedStringIO() diff --git a/src/openstaad_mcp/sandbox/module_proxy.py b/src/openstaad_mcp/sandbox/module_proxy.py index 9298e5a..814e209 100644 --- a/src/openstaad_mcp/sandbox/module_proxy.py +++ b/src/openstaad_mcp/sandbox/module_proxy.py @@ -1,3 +1,10 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- +""" + from types import ModuleType from typing import Any diff --git a/src/openstaad_mcp/sandbox/stdio_helpers.py b/src/openstaad_mcp/sandbox/stdio_helpers.py index 0ace623..e0772e9 100644 --- a/src/openstaad_mcp/sandbox/stdio_helpers.py +++ b/src/openstaad_mcp/sandbox/stdio_helpers.py @@ -1,3 +1,10 @@ +""" +--------------------------------------------------------------------------------------------- +Copyright (c) Bentley Systems, Incorporated. All rights reserved. +See LICENSE.md in the project root for license terms and full copyright notice. +--------------------------------------------------------------------------------------------- +""" + from __future__ import annotations import io diff --git a/src/openstaad_mcp/server.py b/src/openstaad_mcp/server.py index 94ff480..35b123a 100644 --- a/src/openstaad_mcp/server.py +++ b/src/openstaad_mcp/server.py @@ -17,14 +17,18 @@ import logging from collections.abc import AsyncIterator +from pathlib import Path from typing import Any from fastmcp import FastMCP +from fastmcp.server.context import Context from fastmcp.server.lifespan import lifespan from mcp.types import ToolAnnotations from openstaad_mcp.connection import InstanceRegistry, StaadInstance, connect_and_run +from openstaad_mcp.file_io import get_allowed_dirs, get_input_data, write_output_file from openstaad_mcp.sandbox.executor import Executor +from openstaad_mcp.file_io.path_validator import FileIOError from openstaad_mcp.skills import SkillsManager from openstaad_mcp.version import check_version_warning @@ -34,7 +38,13 @@ # ── Tool registrations ──────────────────────────────────────────── -def _register_tools(mcp: FastMCP, registry: InstanceRegistry, exc: Executor, skills_mgr: SkillsManager) -> None: +def _register_tools( + mcp: FastMCP, + registry: InstanceRegistry, + exc: Executor, + skills_mgr: SkillsManager, + args_allowed_dirs: list[Path], +) -> None: """Register MCP tools on *mcp*, closing over the *InstanceRegistry*.""" def _resolve_target(instance: str | None) -> StaadInstance: @@ -178,19 +188,50 @@ def _read_status(staad: Any) -> dict[str, Any]: openWorldHint=False, # Only internal data ) ) - def execute_code(code: str, instance: str | None = None) -> dict[str, Any]: - """Execute Python code against the OpenSTAAD API. - - The sandbox provides a pre-connected ``staad`` variable (the - OpenSTAAD root object) plus ``json`` and ``math`` modules. - Imports and filesystem access are blocked for security. - - The last expression value or an explicit ``result = ...`` - assignment is returned as the result. - - Pass ``instance`` (alias from ``list_instances``, e.g. ``staadPro1``) - to target a specific STAAD instance. Omit it when only one instance - is running — it will be selected automatically. + async def execute_code( + code: str, + ctx: Context, + instance: str | None = None, + input_path: str | None = None, + output_path: str | None = None, + overwrite: bool = False, + ) -> dict[str, Any]: + """Execute Python code in a sandbox against the OpenSTAAD API. + + The sandbox provides a pre-connected ``staad`` variable (the OpenSTAAD root object) plus ``json`` + and ``math`` modules. Imports and regular filesystem access are blocked for security; all data exchange + happens through `result`, `__input__`, and the file I/O params below. + + The last expression value or an explicit ``result = ...`` assignment is returned as the result. + + Pass ``instance`` (alias from ``list_instances``, e.g. ``staadPro1``) to target a specific + STAAD instance. Omit it when only one instance is running — it will be selected automatically. + + **File I/O** (optional): + + - ``input_path``: path to a ``.csv`` or ``.xlsx`` file. The server reads the file and injects its contents + as the immutable `__input__` variable inside the sandbox. Use this to feed large datasets + (e.g. node loads, section properties) into your code without hardcoding them. + + - `output_path`: path where the sandbox return value will be written as a file. Use this whenever + the result is tabular data destined for a file (node lists, member forces, design results, etc.) — + it avoids flooding the context window with large arrays. The `result` variable must be formatted as one of: + - List-of-lists → written as CSV or single-sheet xlsx: + result = [["Node ID", "X", "Y", "Z"], [1, 0.0, 0.0, 0.0], ...] + - Dict of sheet dicts → written as multi-sheet xlsx: + result = { + "Nodes": {"columns": ["Node ID", "X", "Y", "Z"], + "rows": [[1, 0.0, 0.0, 0.0], ...]}, + "Members": {"columns": ["Member ID", "Start", "End"], + "rows": [[1, 1, 2], ...]} + } + + - ``overwrite``: allow overwriting an existing output file. + + Paths must be inside MCP roots or `allowed_dirs`configured by the client. + On Claude Desktop, users can configure allowed directories in the extension settings. + If no roots are configured, omit both file I/O params and handle the returned + `result` value in the agent instead (e.g. write the file via a separate tool). """ try: target = _resolve_target(instance) @@ -204,8 +245,25 @@ def execute_code(code: str, instance: str | None = None) -> dict[str, Any]: "duration_seconds": 0.0, } + # ── Resolve allowed dirs for path validation ── + allowed_dirs = await get_allowed_dirs(ctx, args_allowed_dirs, input_path, output_path) + + # ── Input file handling (server-side, outside sandbox) ─────── + try: + input_data, input_summary = await get_input_data(input_path, allowed_dirs) + except FileIOError as e: + return { + "success": False, + "result": None, + "stdout": "", + "stderr": "", + "error": f"{e.code}: {e.message}", + "duration_seconds": 0.0, + } + + # ── Execute code in sandbox ────────────────────────────────── def _run(staad: Any) -> dict[str, Any]: - return exc.execute(code, staad).to_dict() + return exc.execute(code, staad, input_data=input_data).to_dict() try: result = connect_and_run(_run, target.file_path) @@ -227,12 +285,30 @@ def _run(staad: Any) -> dict[str, Any]: "error": str(e), "duration_seconds": 0.0, } + + # ── Output file handling (server-side, outside sandbox) ────── + if output_path is not None and result.get("success"): + try: + result["result"] = write_output_file(output_path, result["result"], allowed_dirs, overwrite=overwrite) + except FileIOError as e: + return { + "success": False, + "result": None, + "stdout": result.get("stdout", ""), + "stderr": result.get("stderr", ""), + "error": f"{e.code}: {e.message}", + "duration_seconds": result.get("duration_seconds", 0.0), + } + + # ── Attach summaries ───────────────────────────────────────── + if input_summary is not None: + result["input_summary"] = input_summary if target.warning: result["warning"] = target.warning return result -def create_mcp_server(fastmcp_kwargs: dict | None = None) -> FastMCP: +def create_mcp_server(allowed_dirs: list[Path], fastmcp_kwargs: dict | None = None) -> FastMCP: """Create an MCP server instance with tools registered""" fastmcp_kwargs = fastmcp_kwargs or {} @@ -256,5 +332,5 @@ async def mcp_lifespan(server: Any) -> AsyncIterator[None]: lifespan=mcp_lifespan, **fastmcp_kwargs, ) - _register_tools(mcp, registry, Executor(), SkillsManager()) + _register_tools(mcp, registry, Executor(), SkillsManager(), args_allowed_dirs=allowed_dirs) return mcp diff --git a/tests/file_io/__init__.py b/tests/file_io/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/file_io/conftest.py b/tests/file_io/conftest.py new file mode 100644 index 0000000..1407b3b --- /dev/null +++ b/tests/file_io/conftest.py @@ -0,0 +1,30 @@ +"""Shared helpers and fixtures for file I/O tests.""" + +import csv +from pathlib import Path + +import openpyxl +from openpyxl.worksheet.worksheet import Worksheet + +FIXTURES_DIR = Path(__file__).parent.parent / "fixtures" + + +def _write_csv(path: Path, rows: list[list], encoding: str = "utf-8") -> None: + with open(path, "w", newline="", encoding=encoding) as f: + writer = csv.writer(f) + writer.writerows(rows) + + +def _write_xlsx(path: Path, sheets: dict[str, list[list]]) -> None: + wb = openpyxl.Workbook() + first = True + for name, rows in sheets.items(): + ws = wb.active if first else wb.create_sheet(title=name) + if not isinstance(ws, Worksheet): + raise ValueError("Expected openpyxl Worksheet") + if first: + ws.title = name + first = False + for row in rows: + ws.append(row) + wb.save(path) diff --git a/tests/file_io/test_path_validator.py b/tests/file_io/test_path_validator.py new file mode 100644 index 0000000..f4659d8 --- /dev/null +++ b/tests/file_io/test_path_validator.py @@ -0,0 +1,213 @@ +""" +Tests for the file I/O path validator. + +RED phase — these tests define the expected behavior of +``validate_io_path`` before the implementation exists. +""" + +from pathlib import Path + +import pytest + +from openstaad_mcp.file_io.path_validator import FileIOError, validate_io_path + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +@pytest.fixture +def model_dir(tmp_path: Path) -> Path: + """Create a temporary 'model directory' with a sample CSV.""" + csv = tmp_path / "data.csv" + csv.write_text("a,b\n1,2\n", encoding="utf-8") + xlsx = tmp_path / "data.xlsx" + xlsx.write_bytes(b"fake-xlsx") # existence check only + sub = tmp_path / "sub" + sub.mkdir() + (sub / "nested.csv").write_text("x\n1\n", encoding="utf-8") + return tmp_path + + +# --------------------------------------------------------------------------- +# Roots guard +# --------------------------------------------------------------------------- + + +class TestRootsGuard: + """Rejects when no allowed directories are provided.""" + + def test_empty_allowed_dirs_read(self): + with pytest.raises(FileIOError) as exc_info: + validate_io_path("anything.csv", [], mode="read") + assert exc_info.value.code == "NO_ROOTS" + + def test_empty_allowed_dirs_write(self): + with pytest.raises(FileIOError) as exc_info: + validate_io_path("anything.csv", [], mode="write") + assert exc_info.value.code == "NO_ROOTS" + + +# --------------------------------------------------------------------------- +# Resolution & containment +# --------------------------------------------------------------------------- + + +class TestResolutionAndContainment: + """Resolve before containment check — critical ordering.""" + + def test_path_traversal_rejected(self, model_dir: Path): + """Path that starts with model_dir but resolves outside it.""" + evil = str(model_dir / ".." / ".." / "Windows" / "evil.csv") + with pytest.raises(FileIOError) as exc_info: + validate_io_path(evil, [model_dir], mode="read") + assert exc_info.value.code == "PATH_OUTSIDE_ROOTS" + + def test_path_inside_root_accepted(self, model_dir: Path): + path = str(model_dir / "data.csv") + result = validate_io_path(path, [model_dir], mode="read") + assert result == (model_dir / "data.csv").resolve() + + def test_subdirectory_accepted(self, model_dir: Path): + path = str(model_dir / "sub" / "nested.csv") + result = validate_io_path(path, [model_dir], mode="read") + assert result == (model_dir / "sub" / "nested.csv").resolve() + + def test_multiple_roots_any_match(self, model_dir: Path, tmp_path: Path): + """Path inside any of the provided roots passes.""" + other_root = tmp_path / "other" + other_root.mkdir() + (other_root / "file.csv").write_text("x\n", encoding="utf-8") + + result = validate_io_path( + str(other_root / "file.csv"), + [model_dir, other_root], + mode="read", + ) + assert result == (other_root / "file.csv").resolve() + + def test_outside_all_roots_rejected(self, model_dir: Path): + outside = model_dir.parent / "outside_root" + outside.mkdir(exist_ok=True) + (outside / "bad.csv").write_text("x\n", encoding="utf-8") + + with pytest.raises(FileIOError) as exc_info: + validate_io_path(str(outside / "bad.csv"), [model_dir], mode="read") + assert exc_info.value.code == "PATH_OUTSIDE_ROOTS" + + +# --------------------------------------------------------------------------- +# UNC rejection +# --------------------------------------------------------------------------- + + +class TestUNCRejection: + """Blocks network paths (UNC) to prevent NTLM relay.""" + + def test_backslash_unc(self, model_dir: Path): + with pytest.raises(FileIOError) as exc_info: + validate_io_path("\\\\server\\share\\file.csv", [model_dir], mode="read") + assert exc_info.value.code == "UNC_REJECTED" + + def test_forward_slash_unc(self, model_dir: Path): + with pytest.raises(FileIOError) as exc_info: + validate_io_path("//server/share/file.csv", [model_dir], mode="read") + assert exc_info.value.code == "UNC_REJECTED" + + +# --------------------------------------------------------------------------- +# Extension check +# --------------------------------------------------------------------------- + + +class TestExtensionCheck: + """Only .csv and .xlsx are allowed.""" + + @pytest.mark.parametrize("ext", [".xls", ".xlsm", ".tsv", ".txt", ".exe", ".py"]) + def test_unsupported_extension_rejected(self, model_dir: Path, ext: str): + path = str(model_dir / f"file{ext}") + with pytest.raises(FileIOError) as exc_info: + validate_io_path(path, [model_dir], mode="read") + assert exc_info.value.code == "UNSUPPORTED_FORMAT" + + def test_csv_allowed(self, model_dir: Path): + result = validate_io_path(str(model_dir / "data.csv"), [model_dir], mode="read") + assert result.suffix == ".csv" + + def test_xlsx_allowed(self, model_dir: Path): + result = validate_io_path(str(model_dir / "data.xlsx"), [model_dir], mode="read") + assert result.suffix == ".xlsx" + + def test_case_insensitive(self, model_dir: Path): + upper = model_dir / "DATA.CSV" + upper.write_text("a\n", encoding="utf-8") + result = validate_io_path(str(upper), [model_dir], mode="read") + assert result.suffix.lower() == ".csv" + + +# --------------------------------------------------------------------------- +# Existence checks +# --------------------------------------------------------------------------- + + +class TestExistenceChecks: + """Read mode: file must exist. Write mode: parent must exist.""" + + def test_read_nonexistent_file(self, model_dir: Path): + with pytest.raises(FileIOError) as exc_info: + validate_io_path(str(model_dir / "missing.csv"), [model_dir], mode="read") + assert exc_info.value.code == "FILE_NOT_FOUND" + + def test_write_nonexistent_parent(self, model_dir: Path): + with pytest.raises(FileIOError) as exc_info: + validate_io_path( + str(model_dir / "no_such_dir" / "out.csv"), + [model_dir], + mode="write", + ) + assert exc_info.value.code == "PARENT_DIR_MISSING" + + def test_write_existing_parent_ok(self, model_dir: Path): + """Write to a file whose parent exists — should succeed.""" + result = validate_io_path( + str(model_dir / "new_output.csv"), + [model_dir], + mode="write", + ) + assert result.parent == model_dir.resolve() + + def test_write_to_subdirectory(self, model_dir: Path): + result = validate_io_path( + str(model_dir / "sub" / "new.xlsx"), + [model_dir], + mode="write", + ) + assert result.parent == (model_dir / "sub").resolve() + + +# --------------------------------------------------------------------------- +# Null bytes +# --------------------------------------------------------------------------- + + +class TestNullBytes: + def test_null_byte_rejected(self, model_dir: Path): + with pytest.raises(FileIOError): + validate_io_path( + str(model_dir / "file\x00.csv"), + [model_dir], + mode="read", + ) + + +# --------------------------------------------------------------------------- +# FileIOError structure +# --------------------------------------------------------------------------- + + +class TestFileIOError: + def test_has_code_and_message(self): + err = FileIOError("TEST_CODE", "some detail") + assert err.code == "TEST_CODE" + assert err.message == "some detail" + assert "TEST_CODE" in str(err) diff --git a/tests/file_io/test_readers.py b/tests/file_io/test_readers.py new file mode 100644 index 0000000..db24ea5 --- /dev/null +++ b/tests/file_io/test_readers.py @@ -0,0 +1,315 @@ +"""Tests for CSV and XLSX readers: encoding, dialect, limits, header detection, summaries.""" + +import csv +from pathlib import Path + +import pytest + +from openstaad_mcp.file_io import CSVReader, XLSXReader, read_input_file +from openstaad_mcp.file_io.path_validator import FileIOError +from openstaad_mcp.file_io.readers import _detect_header + +from .conftest import FIXTURES_DIR, _write_csv, _write_xlsx + +# ═══════════════════════════════════════════════════════════════════════════ +# _detect_header unit tests +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestDetectHeader: + """Direct tests for the header-detection heuristic.""" + + def test_explicit_true_overrides(self): + assert _detect_header([[1, 2], [3, 4]], has_header=True) is True + + def test_explicit_false_overrides(self): + assert _detect_header([["a", "b"], [1, 2]], has_header=False) is False + + def test_single_row_defaults_to_header(self): + assert _detect_header([["a", "b"]], has_header=None) is True + + def test_empty_rows_defaults_to_header(self): + assert _detect_header([], has_header=None) is True + + def test_string_header_numeric_data(self): + rows = [["name", "value"], [1, 2], [3, 4]] + assert _detect_header(rows, has_header=None) is True + + def test_all_numeric_no_header(self): + rows = [[1, 2], [3, 4], [5, 6]] + assert _detect_header(rows, has_header=None) is False + + def test_all_string_no_header(self): + rows = [["a", "b"], ["c", "d"], ["e", "f"]] + assert _detect_header(rows, has_header=None) is False + + def test_matching_mixed_types_no_header(self): + """When header types match data types per-column, heuristic says no header.""" + rows = [["Name", 0, "Type"], ["Alice", 1, "A"], ["Bob", 2, "B"]] + assert _detect_header(rows, has_header=None) is False + + def test_null_values_ignored(self): + rows = [[None, "b"], [None, 1], [None, 2]] + assert _detect_header(rows, has_header=None) is True + + def test_all_null_column_skipped(self): + rows = [[None, 1], [None, 2], [None, 3]] + assert _detect_header(rows, has_header=None) is False + + def test_bool_vs_string_is_header(self): + rows = [["flag", "val"], [True, 1], [False, 2]] + assert _detect_header(rows, has_header=None) is True + + def test_uses_up_to_4_data_rows(self): + rows = [["h"], [1], [2], [3], [4], ["outlier"]] + assert _detect_header(rows, has_header=None) is True + + +# ═══════════════════════════════════════════════════════════════════════════ +# CSV Read +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestCSVRead: + def test_basic_csv(self): + data, _summary = read_input_file(FIXTURES_DIR / "basic.csv") + assert data == [["name", "value"], ["A", 1], ["B", 2.5]] + + def test_utf8_encoding(self): + data, _ = read_input_file(FIXTURES_DIR / "utf8.csv") + assert data[1][0] == "café" + + def test_cp1252_fallback(self): + data, _ = read_input_file(FIXTURES_DIR / "cp1252.csv") + assert data[1][0] == "naïve" + + def test_semicolon_dialect(self): + data, _ = read_input_file(FIXTURES_DIR / "semicolon.csv") + assert data == [["x", "y"], [1, 2], [3, 4]] + + def test_single_row_csv_as_header_only(self): + data, summary = read_input_file(FIXTURES_DIR / "header_only.csv") + assert data == [["a", "b", "c"]] + assert summary["total_rows"] == 0 + assert summary["columns"] == ["a", "b", "c"] + + def test_empty_csv(self): + data, summary = read_input_file(FIXTURES_DIR / "empty.csv") + assert data == [] + assert summary["total_rows"] == 0 + assert summary["columns"] == [] + + def test_start_row_and_max_rows(self, tmp_path: Path): + rows = [["h"]] + [[i] for i in range(100)] + p = tmp_path / "big.csv" + _write_csv(p, rows) + data, _ = read_input_file(p, start_row=10, max_rows=5) + assert len(data) == 5 + assert data[0] == [10] + + def test_too_large_file(self, tmp_path: Path): + p = tmp_path / "huge.csv" + p.write_bytes(b"a\n" + b"x" * (50 * 1024 * 1024 + 1)) + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "FILE_TOO_LARGE" + + def test_too_many_columns(self, tmp_path: Path): + p = tmp_path / "wide.csv" + _write_csv(p, [[f"c{i}" for i in range(501)]]) + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "TOO_MANY_COLUMNS" + + def test_oversized_cell_csv(self, tmp_path: Path): + """CSV cell exceeding MAX_CELL_SIZE is rejected on read.""" + p = tmp_path / "big_cell.csv" + _write_csv(p, [["header"], ["x" * 32_769]]) + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "INVALID_CELL" + + def test_malformed_csv_raises_file_io_error(self, tmp_path: Path): + """CSV field exceeding csv.field_size_limit raises CSV_PARSE_ERROR.""" + p = tmp_path / "big_field.csv" + p.write_text("a,b\n" + "x" * 200 + ",c\n", encoding="utf-8") + old_limit = csv.field_size_limit(10) + try: + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "CSV_PARSE_ERROR" + finally: + csv.field_size_limit(old_limit) + + def test_no_header_csv(self): + """CSV with all-numeric rows: auto-detect no header.""" + data, summary = read_input_file(FIXTURES_DIR / "no_header.csv") + assert data == [[1, 2, 3], [4, 5, 6]] + assert summary["total_rows"] == 2 + assert summary["columns"] == ["col_1", "col_2", "col_3"] + + def test_mixed_first_row_still_header(self, tmp_path: Path): + """First row with different types from data rows is detected as header.""" + p = tmp_path / "mixed.csv" + _write_csv(p, [["Name", "X", "Y"], ["Alice", 3, 4], ["Bob", 5, 6]]) + data, summary = read_input_file(p) + assert data[0] == ["Name", "X", "Y"] + assert summary["total_rows"] == 2 + assert summary["columns"] == ["Name", "X", "Y"] + + def test_has_header_true_override(self): + """Force has_header=True on all-numeric data.""" + data, summary = read_input_file(FIXTURES_DIR / "no_header.csv", has_header=True) + assert data == [[1, 2, 3], [4, 5, 6]] + assert summary["total_rows"] == 1 + assert summary["columns"] == [1, 2, 3] + + def test_has_header_false_override(self, tmp_path: Path): + """Force has_header=False on data with string header.""" + p = tmp_path / "with_header.csv" + _write_csv(p, [["Name", "Value"], ["A", 1], ["B", 2]]) + data, summary = read_input_file(p, has_header=False) + assert data == [["Name", "Value"], ["A", 1], ["B", 2]] + assert summary["total_rows"] == 3 + assert summary["columns"] == ["col_1", "col_2"] + + def test_all_string_data_no_header(self, tmp_path: Path): + """All-string rows with uniform types -- auto-detect no header.""" + p = tmp_path / "strings.csv" + _write_csv(p, [["A", "B"], ["C", "D"], ["E", "F"]]) + data, summary = read_input_file(p) + assert data == [["A", "B"], ["C", "D"], ["E", "F"]] + assert summary["total_rows"] == 3 + assert summary["columns"] == ["col_1", "col_2"] + + +# ═══════════════════════════════════════════════════════════════════════════ +# XLSX Read +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestXLSXRead: + def test_single_sheet(self): + data, _summary = read_input_file(FIXTURES_DIR / "single_sheet.xlsx") + assert isinstance(data, dict) + assert "Sheet1" in data + assert data["Sheet1"]["columns"] == ["a", "b"] + assert data["Sheet1"]["rows"] == [[1, 2], [3, 4]] + + def test_multi_sheet(self): + data, _ = read_input_file(FIXTURES_DIR / "multi_sheet.xlsx") + assert set(data.keys()) == {"Beams", "Loads"} + + def test_select_sheet(self): + data, _ = read_input_file(FIXTURES_DIR / "multi_sheet.xlsx", sheet="Loads") + assert "Loads" in data + assert len(data) == 1 + + def test_sheet_not_found(self): + with pytest.raises(FileIOError) as exc_info: + read_input_file(FIXTURES_DIR / "single_sheet.xlsx", sheet="Missing") + assert exc_info.value.code == "SHEET_NOT_FOUND" + + def test_datetime_to_iso(self): + data, _ = read_input_file(FIXTURES_DIR / "dates.xlsx") + assert data["Dates"]["rows"][0][0] == "2026-05-01T00:00:00" + + def test_header_only_xlsx(self): + data, summary = read_input_file(FIXTURES_DIR / "header_only.xlsx") + assert data["Data"]["columns"] == ["x", "y"] + assert data["Data"]["rows"] == [] + assert summary["total_rows"] == 0 + + def test_empty_sheet_xlsx(self): + data, summary = read_input_file(FIXTURES_DIR / "empty_sheet.xlsx") + assert data["Empty"]["columns"] == [] + assert data["Empty"]["rows"] == [] + assert summary["total_rows"] == 0 + + def test_corrupted_xlsx(self, tmp_path: Path): + p = tmp_path / "bad.xlsx" + p.write_bytes(b"not a zip file") + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "CORRUPTED_WORKBOOK" + + def test_oversized_cell_xlsx(self, tmp_path: Path, monkeypatch): + """XLSX cell exceeding MAX_CELL_SIZE is rejected on read.""" + + # openpyxl truncates strings to 32767 chars, so lower the limit + def check_cell_override(cell): + if isinstance(cell, str) and len(cell) > 100: + raise ValueError("Cell too large") + + monkeypatch.setattr("openstaad_mcp.file_io.readers.check_cell", check_cell_override) + _write_xlsx(tmp_path / "big_cell.xlsx", {"Sheet1": [["h"], ["x" * 200]]}) + with pytest.raises(FileIOError) as exc_info: + read_input_file(tmp_path / "big_cell.xlsx") + assert exc_info.value.code == "INVALID_CELL" + + def test_no_header_xlsx(self): + """XLSX with all-numeric rows: auto-detect no header.""" + data, summary = read_input_file(FIXTURES_DIR / "no_header.xlsx") + assert data["Data"]["columns"] == ["col_1", "col_2", "col_3"] + assert data["Data"]["rows"] == [[1, 2, 3], [4, 5, 6]] + assert summary["total_rows"] == 2 + + def test_has_header_true_override_xlsx(self, tmp_path: Path): + """Force has_header=True on all-numeric XLSX.""" + _write_xlsx(tmp_path / "num.xlsx", {"Sheet1": [[1, 2], [3, 4]]}) + data, summary = read_input_file(tmp_path / "num.xlsx", has_header=True) + assert data["Sheet1"]["columns"] == [1, 2] + assert data["Sheet1"]["rows"] == [[3, 4]] + assert summary["total_rows"] == 1 + + def test_has_header_false_override_xlsx(self, tmp_path: Path): + """Force has_header=False on XLSX with string header.""" + _write_xlsx(tmp_path / "hdr.xlsx", {"Sheet1": [["Name", "Val"], ["A", 1]]}) + data, summary = read_input_file(tmp_path / "hdr.xlsx", has_header=False) + assert data["Sheet1"]["columns"] == ["col_1", "col_2"] + assert data["Sheet1"]["rows"] == [["Name", "Val"], ["A", 1]] + assert summary["total_rows"] == 2 + + +# ═══════════════════════════════════════════════════════════════════════════ +# Summary generation +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestSummaries: + def test_input_summary_csv(self): + data, _ = read_input_file(FIXTURES_DIR / "basic.csv") + summary = CSVReader(FIXTURES_DIR / "basic.csv").build_summary(data) + assert summary["total_rows"] == 2 + assert summary["columns"] == ["name", "value"] + + def test_input_summary_xlsx(self): + data, _ = read_input_file(FIXTURES_DIR / "multi_sheet.xlsx") + summary = XLSXReader(FIXTURES_DIR / "multi_sheet.xlsx").build_summary(data) + assert set(summary["sheets"]) == {"Beams", "Loads"} + assert summary["total_rows"] == 1 + + +# ═══════════════════════════════════════════════════════════════════════════ +# defusedxml verification +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestDefusedXML: + """Verify that defusedxml is active alongside openpyxl.""" + + def test_defusedxml_installed(self): + import defusedxml + + assert defusedxml is not None + + def test_corrupted_xml_rejected(self, tmp_path: Path): + """An xlsx with invalid/malicious XML content is rejected.""" + import zipfile + + p = tmp_path / "bomb.xlsx" + with zipfile.ZipFile(p, "w") as zf: + zf.writestr("[Content_Types].xml", "") + with pytest.raises(FileIOError) as exc_info: + read_input_file(p) + assert exc_info.value.code == "CORRUPTED_WORKBOOK" diff --git a/tests/file_io/test_validation.py b/tests/file_io/test_validation.py new file mode 100644 index 0000000..ed56fb7 --- /dev/null +++ b/tests/file_io/test_validation.py @@ -0,0 +1,154 @@ +"""Tests for return-value validation (Pydantic models), deep freeze, and allowed dirs.""" + +import os +from pathlib import Path + +import pytest + +from openstaad_mcp.file_io import deep_freeze, validate_args_allowed_dirs, validate_return_value +from openstaad_mcp.file_io.path_validator import FileIOError + +# ═══════════════════════════════════════════════════════════════════════════ +# Return value validation +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestValidateReturnValue: + def test_valid_flat(self): + validate_return_value([["a", "b"], [1, 2]]) + + def test_valid_multi_sheet(self): + validate_return_value( + { + "S1": {"columns": ["a"], "rows": [[1]]}, + } + ) + + def test_rejects_non_primitive_leaf(self): + with pytest.raises(FileIOError) as exc_info: + validate_return_value([["a"], [object()]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_too_many_sheets(self): + data = {f"S{i}": {"columns": ["a"], "rows": [[1]]} for i in range(21)} + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_too_many_rows(self): + data = [["a"]] + [[i] for i in range(100_001)] + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_too_many_columns(self): + data = [[i for i in range(501)]] + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_long_sheet_name(self): + data = {"A" * 32: {"columns": ["a"], "rows": [[1]]}} + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_non_list_non_dict(self): + with pytest.raises(FileIOError) as exc_info: + validate_return_value("not valid") + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_oversized_cell_in_flat(self): + """String cell exceeding MAX_CELL_SIZE is rejected.""" + data = [["a"], ["x" * 32_769]] + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_rejects_oversized_cell_in_multi_sheet(self): + """Oversized cell in multi-sheet output is rejected.""" + data = {"S1": {"columns": ["a"], "rows": [["x" * 32_769]]}} + with pytest.raises(FileIOError) as exc_info: + validate_return_value(data) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + def test_accepts_tuples_as_rows(self): + """Tuples (from deep_freeze) should be accepted as rows.""" + validate_return_value((("a", "b"), (1, 2))) + + def test_accepts_tuples_in_multi_sheet(self): + """Frozen multi-sheet data should be accepted.""" + validate_return_value( + { + "S1": {"columns": ("a",), "rows": ((1,),)}, + } + ) + + +# ═══════════════════════════════════════════════════════════════════════════ +# Deep freeze +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestDeepFreeze: + def test_lists_become_tuples(self): + data = [[1, 2], [3, 4]] + frozen = deep_freeze(data) + assert isinstance(frozen, tuple) + assert isinstance(frozen[0], tuple) + + def test_dict_values_frozen(self): + data = {"S": {"columns": ["a"], "rows": [[1]]}} + frozen = deep_freeze(data) + assert isinstance(frozen["S"]["rows"], tuple) + + def test_none_passthrough(self): + assert deep_freeze(None) is None + + def test_primitives_unchanged(self): + assert deep_freeze(42) == 42 + assert deep_freeze("hello") == "hello" + assert deep_freeze(True) is True + + +# ═══════════════════════════════════════════════════════════════════════════ +# validate_args_allowed_dirs +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestValidateArgsAllowedDirs: + def test_none_returns_empty(self): + assert validate_args_allowed_dirs(None) == [] + + def test_empty_list_returns_empty(self): + assert validate_args_allowed_dirs([]) == [] + + def test_existing_dir_resolved(self, tmp_path: Path): + result = validate_args_allowed_dirs([str(tmp_path)]) + assert len(result) == 1 + assert result[0] == Path(os.path.normpath(tmp_path.resolve(strict=True))) + + def test_nonexistent_dir_still_accepted(self, tmp_path: Path): + fake = tmp_path / "does_not_exist" + result = validate_args_allowed_dirs([str(fake)]) + assert len(result) == 1 + assert result[0] == Path(os.path.normpath(fake.resolve(strict=False))) + + def test_multiple_dirs(self, tmp_path: Path): + d1 = tmp_path / "a" + d1.mkdir() + d2 = tmp_path / "b" + d2.mkdir() + result = validate_args_allowed_dirs([str(d1), str(d2)]) + assert len(result) == 2 + + def test_tilde_expanded(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + monkeypatch.setenv("HOME", str(tmp_path)) + result = validate_args_allowed_dirs(["~/mydir"]) + assert len(result) == 1 + assert "~" not in str(result[0]) + + def test_returns_path_objects(self, tmp_path: Path): + result = validate_args_allowed_dirs([str(tmp_path)]) + assert all(isinstance(p, Path) for p in result) diff --git a/tests/file_io/test_writers.py b/tests/file_io/test_writers.py new file mode 100644 index 0000000..350a830 --- /dev/null +++ b/tests/file_io/test_writers.py @@ -0,0 +1,124 @@ +"""Tests for CSV and XLSX writers: basic writes, overwrite, atomic cleanup, summaries.""" + +import csv +import os +import time +from pathlib import Path + +import openpyxl +import pytest +from openpyxl.worksheet.worksheet import Worksheet + +from openstaad_mcp.file_io import CSVWriter, write_output_file +from openstaad_mcp.file_io.path_validator import FileIOError + +# ═══════════════════════════════════════════════════════════════════════════ +# CSV Write +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestCSVWrite: + def test_basic_write(self, tmp_path: Path): + p = tmp_path / "out.csv" + data = [["Member", "Fx"], [1, -12.4], [2, -8.1]] + write_output_file(str(p), data, allowed_dirs=[tmp_path]) + assert p.exists() + with open(p, encoding="utf-8") as f: + rows = list(csv.reader(f)) + assert rows[0] == ["Member", "Fx"] + + def test_file_exists_no_overwrite(self, tmp_path: Path): + p = tmp_path / "exists.csv" + p.write_text("old", encoding="utf-8") + with pytest.raises(FileIOError) as exc_info: + write_output_file(str(p), [["a"], [1]], allowed_dirs=[tmp_path], overwrite=False) + assert exc_info.value.code == "FILE_EXISTS" + + def test_overwrite_true(self, tmp_path: Path): + p = tmp_path / "exists.csv" + p.write_text("old", encoding="utf-8") + write_output_file(str(p), [["a"], [1]], allowed_dirs=[tmp_path], overwrite=True) + assert "a" in p.read_text(encoding="utf-8") + + def test_atomic_write_cleanup(self, tmp_path: Path): + """No temp files left after successful write.""" + p = tmp_path / "out.csv" + write_output_file(str(p), [["x"], [1]], allowed_dirs=[tmp_path]) + temps = list(tmp_path.glob(".~omcp_*")) + assert temps == [] + + def test_stale_temp_cleanup(self, tmp_path: Path): + """Old temp files are cleaned up before a new write.""" + stale = tmp_path / ".~omcp_stale.tmp" + stale.write_text("orphan", encoding="utf-8") + old_time = time.time() - 7200 + os.utime(stale, (old_time, old_time)) + + p = tmp_path / "out.csv" + write_output_file(str(p), [["x"], [1]], allowed_dirs=[tmp_path]) + assert not stale.exists() + + +# ═══════════════════════════════════════════════════════════════════════════ +# XLSX Write +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestXLSXWrite: + def test_flat_write(self, tmp_path: Path): + p = tmp_path / "out.xlsx" + data = [["Member", "Fx"], [1, -12.4]] + write_output_file(str(p), data, allowed_dirs=[tmp_path]) + wb = openpyxl.load_workbook(p) + ws = wb.active + assert isinstance(ws, Worksheet) + assert ws.cell(1, 1).value == "Member" + assert ws.cell(2, 2).value == -12.4 + + def test_multi_sheet_write(self, tmp_path: Path): + p = tmp_path / "multi.xlsx" + data = { + "Summary": {"columns": ["Member", "Status"], "rows": [[1, "OK"]]}, + "Details": {"columns": ["Member", "Fx"], "rows": [[1, -12.4]]}, + } + write_output_file(str(p), data, allowed_dirs=[tmp_path]) + wb = openpyxl.load_workbook(p) + assert "Summary" in wb.sheetnames + assert "Details" in wb.sheetnames + + def test_overwrite_existing_xlsx(self, tmp_path: Path): + """Overwriting an existing XLSX file replaces the content entirely.""" + p = tmp_path / "overwrite.xlsx" + write_output_file(str(p), [["old_col"], ["old_val"]], allowed_dirs=[tmp_path]) + write_output_file(str(p), [["new_col"], ["new_val"]], allowed_dirs=[tmp_path], overwrite=True) + wb = openpyxl.load_workbook(p) + ws = wb.active + assert isinstance(ws, Worksheet) + assert ws.cell(1, 1).value == "new_col" + assert ws.cell(2, 1).value == "new_val" + + def test_multi_sheet_overwrite_preserves_new_sheets(self, tmp_path: Path): + """Overwriting with different sheet names produces only the new sheets.""" + p = tmp_path / "sheets.xlsx" + old_data = {"OldSheet": {"columns": ["a"], "rows": [[1]]}} + new_data = {"NewSheet": {"columns": ["b"], "rows": [[2]]}} + write_output_file(str(p), old_data, allowed_dirs=[tmp_path]) + write_output_file(str(p), new_data, allowed_dirs=[tmp_path], overwrite=True) + wb = openpyxl.load_workbook(p) + assert "NewSheet" in wb.sheetnames + assert "OldSheet" not in wb.sheetnames + + +# ═══════════════════════════════════════════════════════════════════════════ +# Output summary +# ═══════════════════════════════════════════════════════════════════════════ + + +class TestOutputSummary: + def test_output_summary_csv(self, tmp_path: Path): + p = tmp_path / "out.csv" + data = [["Member", "Fx"], [1, -12.4], [2, -8.1]] + summary = CSVWriter(p).build_summary(data) + assert summary["rows_written"] == 2 + assert summary["columns"] == ["Member", "Fx"] + assert len(summary["sample_rows"]) == 2 diff --git a/tests/fixtures/basic.csv b/tests/fixtures/basic.csv new file mode 100644 index 0000000..a75c49a --- /dev/null +++ b/tests/fixtures/basic.csv @@ -0,0 +1,3 @@ +name,value +A,1 +B,2.5 diff --git a/tests/fixtures/cp1252.csv b/tests/fixtures/cp1252.csv new file mode 100644 index 0000000..0036ca2 --- /dev/null +++ b/tests/fixtures/cp1252.csv @@ -0,0 +1,2 @@ +col +nave diff --git a/tests/fixtures/dates.xlsx b/tests/fixtures/dates.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..3599f11c187a5d1b00752404b323ba151cda73ca GIT binary patch literal 4887 zcmZ`-1ymGk8(vZxmIaX|rMYw|Eg>Kcg1E4hz)HA;EGZyQ>>Qk;MiAS>OwYFg0_6;@t`m< z3hCa3+N#wNVRd`Y{!|t3*RqB@Z3_@d*;l-5rPIu0G|PV6iZLKoy<;V8-&knKILfFl{xOc!`9OZnL_FRAuo*ebqaSfA}P@S=^IE>qZ6=DH4+ zNJ-M(L-t~ngOSgA$Z&GbM?%X??*J7(Mrg3=kV#UwVcO9|nnlW3uhtnbL(`hl`Vkd0 zx`mv|J5{u9-@Yxm7fZ92{`shBr8G*L>Ap|E^fi6VvL};fEwKc5IgsaBvGDUPjzC5* z>0I-#tN;&mTbZXm z68Y@_-f=uRs|f2~m1PkRb`fQQum|?JmSFYFB0n0pwzoLhUo*MaH8^x5DCAv0GEsx( zUcoRui8!U4Y&N9N%YUzZBR95Z(nHc2JioZ*BVB_Tig9BmoHuODY0ch&Hy0|CA`2V` z>^|&GRwNX^j5E?7{vI&5mg$g176;WIzUMtN{5Yz!e87jYX-hw%ts<(kbiiZHk%#xB z;$GqEMF$o2#Wg=#deoBAFgpO^<~7zkM38F9f9RPEo?vO5Q8xby`63eqJnift9CI9E zOqEo3&yFXe0$gn$)G_d{RIZ7hZ%rIBk00yjM;U-gGP;9zH9{_TOjn-lK5cwgDB+hK zAlr8@Inc2buVkY!vp?w=v4YJlNgCHx>2rG{NV;{z3B41+>mmnomu0qB@$cW;ODN(N z@83KiTho)bXERByvgq%}=ES;lHqPup0`~MZTsX&Hv{ZA9C#!R*<#r1-V1d#oO-gal zHInS`G$#XQ{6bQ()V%P0WM>(09u!d9MAAJM7x}%^HACTc9z=68be~2YypV>k$ zqUBZGyL~b7z-HEPX|0Tn{QD5XlN`%7P4r3#=I%+|!|+iFxZvaO8Y2u^SZbbKkW_rmDo#eURnMxp-Cf3&N&FvVa$D9<929f5HV!Aa zLMt;&g*Y$9YX?^=bJZQk^nCrRopi+x%1w$f9)?|wrp-}da#=On)k+y(s;uX$lZRqg zEAu=Q1^W-=t(KAR$t6^4-L*g0BsZaWq_uWl^}6Y?VkqS7ZsjGZbs(Z!;P-EemM2-;!=tlc=G6dg;yLr>^^YMs>7a zHPrax^^Gzw{9t1C#oy+HC`bjNk*2f@x)vLw%b8gfejHwhPrcFB-%i}#X;3f8EnDqa zs8&4On^`<`Ns_yvljYk1#Pd+Y=S*QHzQ`bsHTaSkPYl9)pZxCnSNssWV&RQ#x&LCr~ z2jX)wZd0f4CIa6*}f!NTQ!XvK$bn*J+hnK}2Bl8@ab_8-Q{f|iq51twE z6I|i5m!S4Un+z{GrCwU?d>Wz{`&m%9+WQL)`oXO#%^gCpRxE9zB`@UBm|;}bvHA50 zX9v5-Yo*8cCUe&}U_q$ixtFaq*7|~_&r&twpySPGSV=@?2Se0N?Cfs(6HN4*ST$x% zhLpI=8v{mGgIshe*SlpBA4U3VQs2K2Ji)HSADYbit{tmy)0Eb?iFgPe-tSiJ-B&$S zpNMyS9(A?;0awW>{E%Y6Q`2*kCR-DxJqPxGqqK}dNC#b-{CX4_cy1TM_lW&*JFrRy z08swg1%!t$9ESLjz-LUfpJa;CoC-u9rE($p%Ge_~)LxF52yC$WdD?&k50v9fxleX? ztwn-oC`wHf=;?g#Dh6(UgA}f;t5tP{i!o&v8CWo09jp1SImE)mqxoIAR!63AOr3Vk z7@^fLCaNOGu9*NgDHTizE%mwm-11A&Mt;GU6rsgkhv?&PVY0S+VvpZ6Y8jbyGJK!b z^wn@2nb!85^SqzD9?c~^MUOH;=yUZpakyrX$3$7&(*xx(d`1-ow8 z+XFiNOKl6S%*edn-k#lJi^~w_;5+*-7zsbab)f9!JF=ZeWiolG}|j66GAN5QDWM~~%KmJc&p zdIK(%27=j1rF3Plu1z__SzpBL%juxf_N8~sAr&EGZd&PnQ;N%TiH{n?s{t+yFsaqB z05Dp-sb}ue(%arH3u*(%2hWt|*0QqdH8nSMUH*En1HCaqmcj;0$Z$oTU{MOm#!9GS z%6gq0FbHt~^I^6JAZNA~&^N-=`HU{Es?aP{hiQ&C=w&!VTv(O+cXsY&-;BGAld~@y zvem=(L^1Ob($^&jslept{}g|Dj-D5BR3*pVt=wU7goyBuH9Jw; z1z(Ipsgf3ErSqa4U##pym1-n;UDR?xdV7Lx`_5k6hZc4Rp!Qi#j!-4FdLq^ExDq>wAnBm_D;bZRzMCCsK4eu})X6?6rg}vH3DMH?6f&N3Sdh-R&FY zAa4OrU_`~fdW!mnh{~H}4=8}~W*ver6j!KuUlP5n?w4Q->G0sugLdEYUn~e?cP<@# zOt`_pEFvc90}TN2>JUlEKF=ZO>QX1U6&}}V%l^L7^=>WN-@eykz;YV2`nhWyIkJ1M zos(EwY$(TB^$2HS@GtBBV$wft{F7Cv1O=)`q7-43v`c_X&qwm`i-pVR86ratfWg;a zm9}m$p#e8o8bnR|Q=7lr*^5XcnFAL|$jQJr0$EMApTEPwb!dW%}rzl`m$`tK_&b{x5WA4&gTf$yhdj%6;RXVmXrO}`yNH|Fjr{@hl+Oy*9$eSql z3kwhH{b~O4!Ph6+1t0jKg3Z1N!TMQT3rL%KK#<<*60ukIx9VdD!Q4@c3-i0wT3c#- z_3qv1&glh1Cc%;y-Nw80fJHk3|%^e zqM-CDFchU_q{)lA=6B?O@7{IJS$D1b?R)pP_Wt(S$HahylooT-=#aG;AqP2^Kg@N z7P?P{MM}}i4=&YLZM+rNfjz{fY5KobG!^VzvXWJNCB%cC;iP6<3ldO`1sN!$i;$hi znhmg+<$df&oH~)UQZ%xe{Nh4)A>{rh;nxR<007{>U2sHrL%;ceN@{oQl%z#MP%C-8 zU}xd(LsU^IJ-G;c>jAu$E*9kI#vgK`a4*{fUe8r=J}yLMupGHTtQtA&_?&(DzNZmK z0$EX-GtHQL!MVC%_vO5P%=eYSsy=C#W<;aX!B$wHJ<3TjG~(=YS-J#7C6X^C`-7S^0-*Rvaqj}PV} zE2|G*eSNwsXu|aPEc=-BS7tJ8bNCp^0e~DK002Z_CIBws>kM^={+Ok{akFi0<1;1C z{P+`QBmjnYr|G^CURMM1aq*k3$&A9gH3YFyHG4Xo`s!-g9BA0Zyha`v9mC#{C=?d67YSFx-tIZXrX5rQ|I;I!) zq%S0etnoI!Xq<&onodbE*J{}B5x#3XKdyhuSJs_}MwyGua5dp1E~b)7D?om!=`IH@fI{k9D0}DQqy9!CvQ#V!tZq+5d|}jK zU&`_M=ramGp+M=3;_o)k6s23S5T#$5mwhQG`eq^^wQaW|N0|%6G?YR^u7OPF7i4;& z@YqNX^i>2k^)+W){d0;0|FM43A1>8=rM`kg>oV-^Iv9?JPo2k}(u$3Drub;JD=w(HEmS^92yM6u1z=N}f$H;dwaM6Unb>Fh- zuP_Lfs%=QiQWRy~qxv(#!ZrWpQ(4tAZfR<=T`afiYES_6&9v@jlgcN-MCYSZH#^W@ zp0p{1iihA(pRfli1!MI$`9fgrs_J6+KMYZl^BS1A^J`TMZ!<47V;jv+MsGmBy ziosrMP_|jwz!dnZisFuxA!{{5G%{NC-UffwrL@!v%IhDz8uSEn56~*Xl%H|c`9J59 zJHgxet?;2F&C<0R*fm>7+#7Re6y@_WlCad{cM=huM;mVf*q0POkYYnJEZx`MkB*+K zwVPHIeNs;2>WSJb`E3zzF+acE5T@p~EEu1Vi`FL0`4s z<-*;y*{f=CRjFYC~*p^c+*zO7si&FPpWjiQMkGE8nUuQ^%Bksd@Qh zj~Cdifyu9qfmj;br0^mLDg`Uqn-`;{619oC$FyW*xBYG{JEt<}s^4+Cf4jrpnP;Ca z!>ZC6V8!Jsq9BUeo5eA@o4N)4Ne&VEtm0(bROpX{K9 z4(;-Ds|oHS>3IduX~j7b`;Q<|t>VY+Au%ywK=l2VMvU>1K#o#P@NW2(?9++qV*Km^ z?W4D7Ym}1+P|bki5zjMn$2gkIvT$_Ruvd}!fEwGe)Me4*ze&<-wRwy3F zT=SbMWR@Z_y9>lyv*GT_ubJz>GskJmNqNSa#LCnuF>iH7@+*^LWV~Z?sh}tWE0!95=`Ylm&ydr!QdYb(xzUc@F-tpaBC#w#cTlWU8DmFh>{rgh zLSPDRL05${#U4~&H8WiItseV*>iVRRl8kdb80PZK-J)W2azr_vs5MPEuUQ%k77^5T z*R~x@Ni4ktiRc~Ha|Eru^DHzgJ7-}V)J-o=z^06JBb`|XB6L52XoMxMv%1j2Ex3FV zZ2mk1^GB`+!)qYmT+3JWTe6TxhKNgxivw@FZ_Lcqa%g3zu2k?LSx&Shb2ipZIo9H? zqJeOPY^?rp&FE*WzimHE5yP`>7w(wd6@NY!p;DTFE3uTzk{Vh!37__rI%-|p?m~Oz zQ_LsQf5o-Se{~8JNoZHALy+3KQV963N{mM6Kyx3L0#j}({H38Mbg<5KYuxj!MJa#& z)fB^#9Z~DcMKO-zU~Jgtfote;{3Y|idxy8;`uUs>T;X0Bh&@8@kcCy{D zd2t}Y(xExBZjqSLNt;ofw8Pqj+)10e{7?;F+ZCeCc->YNnU+!8Ro)v+d3O<63qT*R zIb%ilo3Ab4E#5g^W|`?Me9ogs$-+c2QRo`lv=1VYl)4d`K^M@U>_}phcgghqUe_>) z6rU&$r6LDRT%}iLmN(@mep_ftQm$O;gQQ+`Qzc3jh;Y_xKVT)BDtqGLY5!TEfB!Wp zW4O9bo>r9odn(HQE4+;T`(JoN_EiRk=doMIlV1BAzXo5u?q*CIA?R2M0Z2nwA>L59 zkA(QQIro`?2eA~bI)(`fF?`WQELHx#N-K({A$qMSvn$E5>i{4BzU_)L(BM2TPYlB# z``nMyXBq|Xmh7_qU?EolcnSGy^ zSB#R3=BVW4v4ITlF6lK(MB)2N<%V$zmhpN@#%7Qk4z;a?thO2Nx-1Nw1Jm!=IZ}yS zSE2!rxugx=!ocwJGC8lf3#-{Q8tc&ZR zZ1=M{M;yb+S5wem-PHwHxvZ77QqhJ2Ed|RUsl2}E8`;nLGHi{I?W|T+r+kRQAqKlI z_CUMlo?!bn~2!w-nX^ueky`pdJ^6TrI*~6Y9eiC zr!Byvnu!zd9CV4-u5#j}Xanp*K)Qxxa*Ad4#63M4ly{M_-DAlmlQ)y69a>~QiaTR= zGJOx1-IV{@qxqFzna$}3DxT;N3q5qvgHHK*waH;HV0da|V zISX6VlOtgGtyk!dZH{(;9#^B}JzQGr-%c)r%IQ!)B;-l>ueIn|2Cf5`7LNIoj(DbsQizP@U+ zV%q`7-)Ogfb}bQgZPWJnHMlrzfC(r;?(#?VMsjEnO6{rL%(f*~4yW5t!Mkv5zRw?OKKp#FO3DkJZyz9}9jveiZt z(PdZkG5hHpmt&o&?d=K@& zOy?!JQFm&+DV82%A2D7JAT}mDWIqb34Y%GAgW|XlMN}>QAgjUZQmI$4yV$rS zrG-NV{ax*ASnuBU?pwB=nt9RHQM?Bti{@W~!cyXpa1qd-dOa?M)x;alL<0ltutwU4 z?#%1Y@d*a{aa6_Kjgwm%3(%z4orTgV%K;Vbt0O!}s3kZ0dNU+Bz+J&>W`>{IXr29?}xOc%ANf>9%E zAc%;W0ss4efeXIFNtMhKt?#P005{6w<YoV G0RI8AU@ngU literal 0 HcmV?d00001 diff --git a/tests/fixtures/header_only.csv b/tests/fixtures/header_only.csv new file mode 100644 index 0000000..b2ffb02 --- /dev/null +++ b/tests/fixtures/header_only.csv @@ -0,0 +1 @@ +a,b,c diff --git a/tests/fixtures/header_only.xlsx b/tests/fixtures/header_only.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..978976f2b268ddf62b44b9183c6d6b40dca834d6 GIT binary patch literal 4830 zcmZ`-2T&7R+YY^#5Q>DRRFU44UJYFYsR{xYhye^8sZxbVm)?t%07?l;4JcheP(qOo z(t?11fIzM^`J>Jqul)Dhopa9Y%s%h#`^?)7^@xZW0RR9wz}N$ArP+H!I}QKz9>3`E z%fZdw(9_M`OYARqcTqnV_zlbrs!nlg(uKBLK8;y1oF%l^m16Q=VMHH9HVC>z9BpqA ztGN36_pud;MsxEwWI}dDu`W`Qy)>%hJV(j{{e>}w*uZLuxv;nwV7d}6PNJ?Nmx<6w z32NDQON}+_A4D}_4>0NKzAxpC_`8-Yq~%`-a8=B*Q_`;n@F~ZF^yD&xNX}x72bhfW zKlQtvIFPiFHM1E0p+ZlQjok+R*9Qmy0N}q>uy^x>{?GxL)ZyGEPK~fZV)Oe990hw0 zki})Rq(bn`d+>UiSdjgF-k@W-+c~cAMvkhpQ2`3Q<;ZnH<;X$%r{HBf_Zw`HyE&Ws zSQ1rfn72H0w4<@#X834o{vxq;j&G3qWmd~DjS;gHgi+@4ROa2Z@prmsz-%2`8rw&- zmND&Aw7%)$P2XT$sqe7#8(E)^Th}V0_1HZ8f@W224y-<4(`h7?;;RDkKP?q4-{lTr z1yjt&e3d`02=z(gCvWzfb!fyQrH__>OZ?zw>yD+@v@B!rHfqQZ zhI66nxf<3`2l8_AnW@W)*fQXX1X{$y{T-4bLfW*h(hsE5Y$e}l&QT;5UOyg$Xsxcv z`tN-awOd0(rGE9Zof4ZFZ)p_!xL_{H3se(lEko_JQzG@P>+d)Lx@3dif*87H!(++y z{1^wutrjf7Vm#xY>t^Q6BAnob7qlE~gRF%eRfkNB?zq`ptXtud`o=V15)UmV0GX@t zqoeA+9>U+8On+2{V%Ft3B*R?A*(@%G47!zJ4emrOnsj!zJHu4W?)MIlXorTs4N4_x z*1;8zGLcKtD9Gno4Ej98b#E2K^-p_CyMPy#cl~4=21a7t*@+j7TJk&d_Tgtn%~HA%%zOCyg7Q`cWyKxx~FQ$kEV6^=HssF=$?up?+qt@ zfz#^yCF{(N>YB?N0Srv_E2^Ux0T%8)0BpX6ZF|F&J!qoF`Aq9L^#LcT>xiXX06VE%!vKCNHF*Pox1DuD~2pvG46zWMm5zbo9bmCOq*bfzP|(QAU2a=kuz z4a1Mgkeb6d(^s>_85T?g85icIpUVip8TU(T->u43(7R0Cd)?GRsYvBX;PMd-eV^VP8guk^uu;^mx_{&t* zC8f!Vvu{(pjxcjBc>Y*gxtvpyl4KXnsk{>4M|my1=ape~cp$;q@btCLif`fVav`EY z800p3U#W1U@fuGMtYb9R@}s7!W%qHl#Po!;=w*9z&-Z=3Ca4s_Nx6dd%~MC^{N1gi zDc%TF_H7X!=0v@)S~cFLli2>9E8P@p4wifr116);YcUM@>TIs-#s|7-6Duh6^iE@TUi)$sA#iV_nt~hqtR3Eb30+2Al)b3NXcFjt1YS95TnayFOSR zq_}a0MiW@QmB^(zZ&qG5KQjqUIeIJ6t?O|8jURYPZi*Nklxgm=IypRiyxL(@Q@pL1 z&e0q7wdm59LPRE}OXIPBpiL$$t;bNB(FI<~E3DaQ+cdE&%(&oSEAo`=5z2`0>n&4T z3CkWLp~Rwx`f-po_tgw5o8{r;<(X1wy(>!4m(_*)mRzS(CJ0v1{}KA4!;}MKYWcOM z)mf>I$pG{Hc2!>iHPC>Wib2~xm8Dd-XphynbyY~G+f=pO<0b~5 z20B7Xbc@`x0H_44^y|C`HHENc)NO{P8+W(HR&#Qz1Gs&TGJHF1zn;3k)uLNbShd-| zt5F?{n_E6|O;OO+&-H%;B=Ck1@}#koGP6ix4ZkEOl7a}{r@pIZsJHd%q7A^(qW9HUn9ChNy<}W z(u8lXdYYdqa%RcoeG9mi-&U_Hz%@@q;B3bFgHj!flIC7<)&ghkW3QZmrcOxZnZ-Eg zq2#=r`;7cYBI{2eVU3dDj-Z&BP+)~!YcuNRAz!XSUEprm#hjDznG)RG0`NgvWHyBr1o)^`_eW~811S`UsUx>OHliVTI^95-jkhw49Nydjm1{V#Z$Vejj67v z%zU76#L-BzCy`i7WHnqZ#u`v6&VJvfsp;%aSlo(jBlt6c$dW5WsNvJ95+ML^;a39j zb@ROMg^}NYsy>$18SgY^?Hh&%ubw zBwweai&Y~Aj8j9FN8DW;tXsD0DK)ibhh6N{ICN%=u{|;;9f1&jX&_TWdbH^mTBx{T zG969@zPnU7_-4=hDwhChPX2CgDi2g3wpR|vV>}uY^Q?5k`0MPuR-OJk_4)ywxvFFTY zVrInYPnnkK%2P74n|S@pFBv)xbbisWS+Ou6Zf<({iud%$nRm4zXZhBVS^y4)6QdC8ZF^$zqG zd(i5bdy#SZ857Gpt&EZcbm~wK!jX9(LTejDB`9`<*@+r%#^IG<`TD?sH*zfyUT0&F zXa1sbQ`+XCzT1Vx#eok!S7$%gvuWg{VXJr$Ovf7Hx$A32Y^!nH6+pO~bgb@R-S7_D z*Qy^TkK)?88)l!=9e*|wp;VTDDK(eLmUzEr7&hZAaoD!H)m`CHKsKL9`yJCE``y7` zD4|28!Hw9;nT*eOMQk`i6Pg!X228ys_gPI_V1JF{`l$P9t3tv2i)p&Uy98~|7e&}g z0@0xx`_3WD@fS?|Zy#KbkFTi4nalV4d*m70=&vET{Eg$Q=O#MzUo8$KnA^UJY*-|u zcYx5V5_jG)Bz1srRvxI}>brv=^jEBu5$T!r-IaZTv45 zfB0E^;;;d}Yk-6x`^kn}!*%pGtTRJ($mul#Z(91+r+1q!Ro9?;odorlBFnlb2H(E6 zKEOU98J0M#>4`*QOuJ@9N2OC~EI6vmNd#VJzZw%ps+5>aSac%#2g8@W^P#aH9u3D~ z)gQSS@@%P84h0OEIszuMflwLwdg~j5){lFL`){?BZZ|oiJXFiq*|D^^CWzWUN;Ghj6JDxD%%kIlU=Ee z<>o3ptH*eT!N1u3Wl8_$_@`CXCn?cB5~q$tF{}VClz%8BEETO}Vu=br1cqIIQPH`@ z)&bDwXcoVXNpJhx0VXCL6R2P`p}?@`$lnj`-Q)z6D6lg@#upj+;}43LE(TEORDUX&gHVrP!z(yJ^sB+L z)Nph4hPry&nE1Oxz3%=nqpB3l4>RhqL4JPVEG6r%x*-}TwAoG-(S5i0Q-PhEx7(AM zm%2}1(4^@V_u&5aI^nBmv?qQi4^uMp*I?v|R2>UVu|SaL^E}WtU^pwyi@H_sNwyS> zen@}CkMJhR0r)VWKJ3mH5h#Ykt(c;{Gjk>fiv}p1zAT%`&0T%+j*6wvGswG@^Bp4U~!*V8mSKCCiHui%HC2*>qIAk>szFraAr=HknC5Mf0*qRcE_~=1% zWF~v_8&8SP4)L3scU*hE1p=p*V1K&8)9t`<6f&jfCQa0$5=sSMYY-4J0{-_b0&g`x zuTVV0|9>)Z9({fc@Cyq7gc5Z98~x8v;5__%ulyT6hyVP4bk64moUfJs5zr9EN&Zv7 ze^yTCrJPUw|40cXNTc{E<#!=)9(q1-|AsP9{}IN|1J4J--@s!$5U={bUG_Zqyr2ID t-=_Tu{vTd{Ue0+}`YlHpADsWgu?+P{@S_a?fC7Ij<3o7NkMR}Y{{RDBRp06S4(TU^>!hB2PBl#$?Nz@dOINcXws=Zn z9MwWd;+-t;_SmL9v3Ci$p7!;uX%!KvLGS7Vo07ZRv*JmwR*NglUP{LCqKL0#m-Pu9 z2!A$ePx1^A(k z@S*G(HJLL8@w1J7MNUrZhuS&j(yCZlz|44T2oe#(l&ZFuh)iZvA2=acX&TIq#6|R0hWkU zuXdC_Oob;zEG3y`*yj~+gxbt`-aX+V;slyo-1QN!=^2W4VZff#X~=HNI&f?*xQ&m@ zw;Qk;Kbk0m7rlzny*vC3HoKl-n@JF3e0SKvdwMuFva@u+hq!6?ZbW-oWG7<4W8IE} z^PVL3wfAkDRJ{S$_CBz|h5Ow08(A*^KMdGZFNev0?hQ$&TTcL?qew&JOHR zyCJ${5oNcmI2;ndE!3bUB%u6uP1Ial{E$i9Xg^mq1caB~6?~`?!qPESesP%6@S#B1 zFAFBw=aBfs4uMg;*^tqnaPGdklvA8Arlrv5(#V6qd&Xw`FoF{*Md2pNV51Pwe{=*d zWEbk+J|S4wmbPKmPp&ZS?_bJ(?#9+IePjqbx~l?Z8++MO#X6R#3|7kN;;mmI6R%by z7NU^D%L-4mhcI9k;0q?_h94t4Uz5#Iz-pWDx@KdZeM2~>%NXPus!fC*1C>DwneJcQ z2OZC@6)F!N-EW-APcfvzNTHn-c_qrzG3t}lvRj%d%}7E1F_94Wc6bVygZ!mLpq47d z_nU7{LCHx*Vx*F2+_~LC~_$OgUp4e|HBQ)wuq#VU^ zYeFK|^3xySzYEp3&v_LgB3r^FM1Zqf$0WP#=Rc7UGX_IXH2v4RIXOBJqR| z)?c_?cOa8HRC|jJX45(xZ9Jn4HAbD42~LcO@ZGR9aQoQxUavfXYg{6yW#htDHhXvT z^fNfLJpBPL8+DvUaMf+_+w z#!GAExgN4S{U_4qE69(8!iu$S8eeM?o2og))ekCrU9_2ch@@j;ML#W}a1tabQ+79>7n*TGT^oL0sM2hUbobt>|= zrIQ)oKikV=*~$%1?Qd6%@bx!OwMpuPh)_B?z6SFs*IK+C+vTC0v$EiQaqVfj4(8sy z`xb)6ome;H@&YuUE3LY$q?nj54#qD|7FkzA%elPioLKKkwArOn*dV%RtSejZGxpy% z-m7S`mqAi#_kVv-+Lc2@rcF&qu431^Pp8|oa#Ia;U%XjKxQ0B1wsG;xdsl7U zx8zNl95^F$Le*O;@xVocj59ug>+eJ z#r(iju`KXtdhryRAf>9A>Dxhu;h}`dmc)QdO(V1f*@}%wkf^7 zG#q6*5t@~7nUtKtGW$ZoqgW8w3X6&gB11fCsxQBL!k#IG^xqA>o^d`pS#UHxM-(uE zFevS6GkQM7 znYj}eFQPH*|LW~PfkkD_8ysP&M1}Tl4rnoDtsJ|m@E0rXzOd?5zvnnc6~W#W80WOo zEyW%ScA=P?p@WfOVw`vPmC}^%Y#dVx)FZ_4A9GvQu=qN-#fPWKE5rncoLQ1;#0d$U zs5RX!SYidmjqRp=XFvV6cz>#OsEdElA}2v5USwb|3sp^LKEJfKCA1){VkY3wiNdYI zK_~*ju>65}xR@=>)Q6o1TrMvZ6&MP6aflC@!DRR@rduxZ!8*aj3(K_ca#+$U0=5;M zICvo{`p%82=BUq!6R814G9~meHG2C8gj3tjJG`u|_takdsY; zYQaNli(EDCP5T{cQpjrtSdfFlBv(2_WV*x()ch$mG=!M;iEiGfkO2FeTOh)`)ePi< z8pEtBZ^1*~iLp_!ySu9DAj8Qn#%k&#e++_#=USf&E8zFFV8JVPY~16R??jtgiv#Ak z3UC*}>0JtGmG9RA;0KH{3#JMy%C3~X8$ePPUoIi)h&1`uY|OA76+#UNH(C(c{(=o| zkmUQ_u)t4e-xfDRpR&IqeE`X-yhcq0iOSC-_Ud;z5^0D%2CFW;?BILT>X^NS7DNV; zzN2dVrit$z`Ldkt_2nf_Jc-$y-Du-u{gP&vtwP*)Rs$N)gYu%wmstZ3bDgp^A9Fds zUkb3|;n})Bt`4H%x&bqguKRYhxyG)I|8SV!FXB$gS;Y}Eo+IG+j2s>gc6@Z4aKitm zBttm6%JNi!ZrnheXqPZKje^RHY}ACY>ln8)@|t~A2wha zyuP%MERz`mAyW6trfpz05)OW>Txq)@s|>#XISj4vq~- z$6z!kb7eOQ)oI`4P;yc-=}n9)rZo@k>Ql9(So!3Vr(1GKMboR2QV?60IM5kxOWhNy zvQ0tA#m`M`N93r_=pJkQ?ob;Xw(9SQG}q2DsI1)(F%Qsmrk$Vf8R-<8nyIE&%t%@) zWecY|QxwQtTh*aodCrO;b95GoR_{X&?$mjkblXUlGjEy(TV|kQE{8&8ievkW3`Elf zKdwT8Cp`pDnpZYa2-lozvvDNf`&-4oTlwCMZI!EW#x}9P#_qk$KNzZPofTM2mUv6z z>upuegH^^m!!8$1QaQ7g6Qn1m7|pNddFc!M>w?w~?4K;g(CYauxJdcna2cLDFNlN+l(N%R!U5-59N-kQ7|PSw~1_c za%wf|b(cTh{rT`}0OEkgwvK1N@y6Vd{wGU1s;Tz87tE@7ROHu2^X#8A>{DO~2#P&P zCHCn~u*5RXrqvnWL-kQ$AH|7V*GCh5>bsMK+uKVFNQl0hyIDKB^Yi_C_W>68@ZC7hJst7 z4vu2RTdv!ZXN$ac*$l!q9{W$o!~>WW66U4w)*EfJxQ9+#)0cFS!QD+%6|Gs!)CffxW(h+-vuS{?aluXpq{ z!6Thak?xj~2&+`6?y=o7J2zAi4rr<06G*! z`A@$-@KNCDQd2+qi`I{EzX$QT+Hm`M9mPgRhv{rtCx6`u=AX_+n7Q#G(N2y*I~(-N z*}nqopDzCyWF7HZ1WyHk%H`y402QaTyb=U-ikctjao&mw?M?i;d9xmp8x1W$>}YkJ zF=@tg+IM6zmTWRa4z32@`z+KT1xVrrDFu6q_ui^LRNiHPVQDA5vj;8+GzQNKTP1I# z<)t!wS3r7BCmmNT_sq4Jx9rq36;s0tZzMX$>Eh?lbYl0A;Rrv7YE&=mN2n4U*Xjfa zMD>fXW{V6JRee#sT<#6bo?XyyMtS;}^mxL9+P8aT|FpGmp5Bld9at9Fzdn(T6`i3T z)=&>~Jzp1Vcheui)uGpe4ldw%Qr8%_*IF=^y!^vM z!@5({r%qJJOcfi$@2~nnFV4v$&t{XhND>i8LJcwdBEBdzQq7f1u-xYwjY^d_7ze~v zxq-7x;T%jt&u1Gx;Nqr7M-fdI0O7hCE)6q+$xTv!X<|qACMjlv)BN}FbMH2We63xO zp8pm%!9tMQJzt-y*zFEhVYm;PQVbA=0T7`G%zqJ0slQwLI=># zBM9x~|D7*gMPD7}{K5hNK^X1-ME^b7xeC8J@%Rm&M*sc4%s;LYTlk$%;@jt*T ziS{?JnB@Og{&(8F3ci{+euF8|vqkiR|09iD<+-{kfAjnp@cqjcg=pZQ$6Npa0s0q; L&RP)iA5Z@W>FGZ@ literal 0 HcmV?d00001 diff --git a/tests/fixtures/no_header.csv b/tests/fixtures/no_header.csv new file mode 100644 index 0000000..da813b6 --- /dev/null +++ b/tests/fixtures/no_header.csv @@ -0,0 +1,2 @@ +1,2,3 +4,5,6 diff --git a/tests/fixtures/no_header.xlsx b/tests/fixtures/no_header.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..8b4f925f56db427eb7e8a575ffafd0b70ccb2980 GIT binary patch literal 4844 zcmZ`-1yodB*B-iaK$MgQ=~B9fmM$GiU>rb(p(F(XVWg#`LmDLpkWL2>5QL$TP6b2+ z1V%xSf7JCZU-{qf-gVAdcdh&Ed-rp8oo%R#hffCp0EhwQ9?e!7eK)n9VxNYvgBm*= z;SPqLaCa|}JMQknelXXY=$m9+qU40joyI zfB$}_0^ukQo`!VLzVJFsLcEVcd5ZHyad03%I)6Q|N^BuCwuX_ah>aDmy8t#F5+Oz| z{dTpndTT;h19~5ws_I)SYsAyNYAGpO!^>7a&rC|a5x}h&1JspC6CgN`F&U&c$^9?@ zKXoMNBx+$W{7Z#9E+45sVt;)Q2LPb?w+aq$PsmRliW5=#-J;|OTgb=u`^hfA=oTyG zY-Lq@+DP!a9~d;Q5oOfc$SVXU3S;W~c78fz?s7=_UD+W2D_v856QlJUC?YtEC8HfT z(vJ&jpBO0|@m*26d+Q42GDSxgUZ~(8g|xi`1yg!>vu*6mEoTp-lP4p?xG~B?G9RVG z`0%Z6l`OC3QFzrRIJjPNxs0$cyDJRiT0Yue@wZWgxAd2&4S2(h?p*B-8W{)ukZh28 zATYHtV@pt5E~&ErX8d|9m&oMK!e_%JVjzCN8@RD__nWLd;R-9(uzuRa`xm#*vIj$ASA>Ure?RWiTVH#(*j680u2g`hn#amTwdGvlWrIsi*aYhUp8vZ?aDcH?JQCusVj6I zaeRlFtx7C;5ocmB{xxW6JIg7XG!ASqe$RJdJU*(oa>S3KW8dI$cU4qx`H1(nGY{`s z)xDyvOHQg9Yuf>|^!4wR$FBk`-F>Fsj^U-+@SogIVVq%UT~M*wv-~U*Mf0QgHU54u+2S}BM=;f}YF{^|rG=IGzBXMSb=#gd^`|Ek+*-={Sn<9yT zoFLiZdnpf{%W+C~TC>o}r(T=ud8NtIH&ut-+XPAWzjK1`Jmz(i1A59bLscK3F_^?+ zZgKSPG3mCxJe18mwb~kuUeAs7W@wapi%S<>UAU0#xWAtm^F%J1o`{mN{479j0V3K^jaB8`iO_PI>(O)cOL zf#TU8+2hK4DI#1aI*Cu5Uvidu^9*fDFrqY&O!prHH$=oUMu*5&hwdvMIZH5irJ!=X^OiH*4#(&kEV9;5o;we?y z5|<<@%)CX?9&YZE_u{dnVi~JADZzd-tK$0rKhhhiy={h7VSza3qq8@<%D;uZl6fc` zgf8A~K2*pbYrMf31VxRGXSKGhdTD zczp|*@2x0^K9;xLs2d`aP;2zm{n(JwQO_f-b6E4%U7vN3Tn;Lq#M0pVj791@#@c6{ zvslz5O|6MhwF3{!M9F%SW+juuR`h5dsR9(0$ zpUTn~`K5sUb3P&+-L3Z6KhQQE`n1Kg_lvQa zofxG0qQtyW3_q_t+FqY|ZP6tBE2NoeUC^Htc8Td5k- zG*ID+H@C_>4}ge4OTH`#k&_65BX7~J-n8DC+{ntV3gGZLN%KY7eK~V~qe=B%Zo~G_ zTCFM=v#@sJmMo{Gm+k+W2FDwO%lVX<@DhXgy5Z-f1VSLrP|6$iJ=`$I66TQf6SDR% z144!MdyU8xTvC}7lp|EgMamPu90ktTUf`q5q zJ5%n1s#zYgh`CkMp;pEY9y{Ic0Jj_=-t#$|357Za1&xEkjAhoBkG--2nz}$0=ay5f z$Kp#e?sKw>cs3t^f@(#5^X&+NFRPd192}y!C zlL$<#4(s|=VU;oDU(KUgr!v#qjjhhBlvmTouP88FFW7PAtL9+xBipr8BD}F|e0_gH zRRxZZQCCYI@0+RB>sH_VMkkud$FunMlE+5!v}|eA5($^-l!?19^D}&Yh2**Q@9H`t z1;OI+=tDHB6AT}uNa5>LhqY!a8XBv0e;spke-N5ndJK8Z>*k;6%p8Te8&mNmG|fBi z#{lJPK?fq#;=OLXu`)ljxpLIU%8ozlaP@w$;1FwQqP>$SaP9)PYAm>+ zuDW(NDJyWpnhm*28=2^=bGO`lTBC+8YzPxYpsIptv%ZffvZ=sq$o1`AK;#pLj+sRfbj4_s{ zA5<+sgP=0^0yy|HgzmoNFwxueemVAN_S!VBoP<-OKGZo0W?nfuJt7~6)0xVj+a}(u zFTewWfvn!9B$QsZ4euY;asX~jx)+#~ozpYC)l4gjZ%!HMML1m=4AOKDPW$b3}M;yv7CQ6G0d z>yXP^s+pxaw#Mmvu_DA&6xbZHedzLVE$*_Z|E(kSxVZ8vjD_rgzekRVt==Yr&EF)h zYGE3s*S0bkZ(-LK(XfI`?Fgb)#_zgqNazS+tvFJ`)b|8|sIOTmB2v@qdn)<^iOp6J z^?>q22B&7hgEqcpjQOO)75e$^f@f@6#Pqa8Qw1&$TMvMEqGHz{rc?M0Bs<`Nb1xgc z`_eNE#K$B^Lr4e#Qyi4?bkas#xDy3Ncop)cUI@~adx|(uxx<~bP=}WYX3N6d+->%_ z2M%iSsY8`Da@8Vj-jNUwTxF*oIQYgMbf7Reywv>ZWZL6^>2Kp})ZC7Kg*CcMSOX-( zuC|^KS1%FapL0%R)%wBWFuDzl?Rx{>Om4MoN{C2q>1(w&Obd`njB@;%D-cbvFINUNuI z)bl#kJ|s=tZt<#v_>$5c;e4y=&g9flyFKF(BB6UeG>;~o$jm`jd^#YLD4! zRW-8DnO|=;qO@mv_)WXb(fSjDQL*Fd-iTtfS@*o~xMT{2B}-Kq0dIR|+oWKzQjzJj zWfx*#IBd;37ZNk^Xf$?R^%2aFb4RISBw)nM3GgnH1|lU}Z*z0lCMe0qyvMLNuYmEX zf+|}uk$pzCs9h-Gv4lnULWqu_Y@HwCnni|=s@fT+zkIR-%%_;w!BdKfDR3|LJ)1uR ztYe#loOF9t=CWVpZ6043NG3GsI&^?ds&p0v%O!e!d@0w zDzvV%xVfpE0+VXFgR*iw&`m)snJ>MHGFFMBd;vv;ze~_77L;Vga}ExYXrb_jwDf26yv@xOv-}`nyBCtbdwOWit9_rtGyX{&?R-LfTzH`_>;L>oo6)^Pjw4>F<-q-xyu_J@ZZWp70mtB=-48ZCqt4g5yZKi z_qFtC&r5P5jq5#$R)d=#P+#-IH6S=*JPxQ2z5Q7Tf@Xmil5`9JE#JN@6{~@oHOG!J za!0MLE+0|p?1T84J^N9;>Q?Tmxlu18*$)L)Ouq$$q{Jdz1%T~Zea;0hakrds1_v3T zE#&uMbeqpG@wz&(Bt^Z1A=yHjqp#^D_Y>}%E)KcbB{XZTPryiNX6o@DZsfRbmB;j} zCiGj$tcN@9%)Jif@S#s_bD!-FE#~!V*!8=obG?4|7j6igkL;5|AEhAKmQ*G z>5Bp`mQMc&Xb5E?{w3f)3#p4zE~fr}q=euUlKhhLrx3UZy%@OvK(CVj6~-?DF9yOt zKoV^AfmQwAE_)Gt(a-;Yp_ISC|HJDq%DL!Df8^+4gY$nlmZ2^Iw$lLskYH~`YzR;P I*~|j`AK4gV2><{9 literal 0 HcmV?d00001 diff --git a/tests/fixtures/semicolon.csv b/tests/fixtures/semicolon.csv new file mode 100644 index 0000000..ba4c545 --- /dev/null +++ b/tests/fixtures/semicolon.csv @@ -0,0 +1,3 @@ +x;y +1;2 +3;4 diff --git a/tests/fixtures/single_sheet.xlsx b/tests/fixtures/single_sheet.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..1c9bab166578e6d7fa40b185d39f0c18681bf6ba GIT binary patch literal 4865 zcmZ`-1yodB*B-h%9ZID{Iz+lg8b=VM1q4RIp^=nyKw6NLltyYmasX)%siCBsK|rKo zl#+kc^)6re-|yaa&RKV@`|Nx7b9SArt%-w63jhEJ0lF>-QQ)e^ z?qSLi2*1u%o1(lYfN~HO`9P*H&VC}-*P9)Yjq)uQo(qbq1XAR&vEp>(I7|eF2$M?m zFV|IUj0vdNJVhocdA^g<;p$j66qBmtW-FOxCZ=5T;gpM{(UeT)#k+{q?W5Dp{M_qw zW{uZE(7>SmmkQlE=9Zh7U+=>L0I2@0f|ZjitkpQStO0s6Ar&qNuxAJXdO%0#V9wC=1Ry0wp_@d)f?c7rSn~;kiE38S|~gP zF&lR&`fyuaudl(Q37Jc{W@(;&N;j^U1c65M<3n{)PNz~HCXV-OTu`N|TaZ~iCpU>` zB_a1r5~}}h(~;1RqTGD_<+ORdBwUlp#mjG2{$AhOQzrE~d{NFaDy}yL0!4e*1Fisx z79#egPD=tk;FkvF06;o706>Fb#tSOwZVPdM{GNq>a%RjZ^0GlY zkaUX&)mGEE*?G)VzlLpr*+QrcqoLl`aj<|ERdLF`B(kkncN@}VaQQY)2bERV)+M|T zwgoKLL&FmGy)32#XT}@r1m_nGL^!Avg{~CBcUy_`wajXa?EoDTejjP5J7$9;2{pZF z`^HTdO@M;z<6o+0=Ja89&}>FkYYRU!K3jz$Jso2w^Q+aXY@*-k`t+h9xmYv?id=}W z>iuJA*V**E90a)`#Uc{qD8yuVHDJ)G5Ct|4TheRqY_+$M*MIV1ctkBQ_@iF}UV}P1 zca)A$giKm0!*I~!8M<>TE2?+eUCaTvxU%OZQQJ2X>CB9~sMDC)o^c3m$x|e%&b1q| zo)spv=C`n!W}8kNZE|n)iRavCOn7(MkQZ6=-n|zc<>B2WL++b)T-@j7 zPx3bCZIx73Hhrk+YE~6SuL2C6J;wV-aFWb;PM#(Jr&t>26is&xwrBicJC@P5>butke zdBTj)6nkxE>^vgjr0me|)!n63i!^?9&4fJ*(P7_8oKj^CvJKUzVc#iLfXnG_pWTL` zrv&*b!{`T1v$@HJbXduZ3u48$_`1fu5?lAm(q&j^s0R{A@D)RoIk>3bO8Vbbqxr#` zKm8+pRADzkkmE!n?xkH7d!akmz`7`~O$E;M^o8w+jL#3e}Dxs0OmE&TFd!A|5q29J0-1qpr-aqt{D3mYeo9Y;Y28x2KP<}&1 zj377Fi0C~`-#)AOg_v9ss|YdP9)eYF)yIqYc2akfc6qQb*2VDj?e>!I!L5=30)EK+ z9mJt*_DJ1rc0Zf8(MXed6-SfK({kbI2{C~iRtByEJs5sZRIlewoaxv zVDMBUes=m;&7dkpj{396-rbv>MC;ZjoJ4(kqmbJX)R{_5jvKm18i^CDaKw#z857h7 zJlkE44|yzOzE(XzA_}T=)%;SM&|JeMp>bH*?+j+`BbBz1dBsxe`G)1zDf*!YialRQ zHyKn9RBFa?XvmnAlE_SnLlBSNk9F!e-stiIE=x|~BK%Se9M&d>hfmkqbSiRpWRh4u zgze{C+s+P6L3V&%c>9{C*d%sqi_tnjOF8&d>MZIf_V{QQtu6T95Il$LVDI05U?FVM zjl&z8^GqvBdEI#}+0=Yx_|?ix0i?ze&f|H-;re~acDocBo08snNM+jt7UTny{fcIL z*=jm4@`q7bPZlW^n4W}M%_@PRKqKeiif;27uX^VLi56wiTIyuRrj^ehT)?{Z)E>1I z*dmBV$+tcbVVi>e1%6T@K9evb>gBr+w?3_TcxM+o*t&o@Fy(~g{XQvz zhiQ*;t8jzBMnE zye7djwpaL8Q6`6C<}Yb2%L*#Xx~GLny#oyoNV6YHEWA)o1&V%(ER~EVc2fMLuqr<@ zMrMm96Jw6UGZTJYE1ru26U)s0*deQE?~Ga6ifqC9JAs@Ei^M@NX%&eL05JYeAf8UH zPuy%F5O+7hU(2s7^7Q^k$LB(n=X5BFA|iKPT(!H!#j<)4)HSk9jI9w2_V<)dztv>G zV%225lWy+}EJKGLKamDR`R8eibF)~=7mn$(s6BRar|5+M9XLR1sY%oGfD%v~tyuVq z@69U)c|A%{NQBmowlf|t%~VE0|CxZo`ZaD*SK?CdnMZ^}*Efwwm|9=X(@7-9$lOYP z!#8nLWC(?_6F1MxzJ66slGMXe)j#F_W|`;wm-NnNZc6R!LW}LG4r~oB*POMzqG`M{m0r;FWzJ*~9H^C&7!x6maMCjqTZGQINj13Ps#r2ffiMnczmC z=`p9nP`!kwhG9iiPW(6WvY5rLfW>`kt&H9qiBb2}i_jBO3{_Fvh5e(ZBA*4qM&fQ{ z1dj6N*lnXD!oc)P6z79leTmVyV&z=T-p-pMF9ukc$NauC=Btc}q-2~rTgbGoGL^Zw zpRIPwo1R_Tni0G`mHU`&Q6&Zd$bN5)o4Yp@;`TGE&*{%Z%>3Ms6^f$xl+=0-@1kqk zwtel}AoEp2Fm6NryC#nF6MK%e+O(DXC;DlNuO+2KZSsT%i#@qNW>KU!qKfjiCpOv; z+qy|djW`CS7Zy_GvLnEfl!Nj)8$TP#CqCDCUh_Y$x~{9W?p`(WeEQ}Dx3s8j9oWY1 zm4klS@WhZzG*(LzPiB({0?f;$?4WGgpAcKfXb$TcRI{R4`{bOXTXaFk(65@D7lTL` z>JGJ~?}MrC(2(#5-lVr9h3d1o#hAQ50&|3{`$DVD!5IdXbzjBIpJ_QUE-m$qb&Jo= z*D!(75>aLBp>(Gpq4bS)9j3LY>m^iBC$UJ4!Rp~%gr{k*jTD@1>tT>pT4(gd2u!vx z23cTmD^+-4T{~#TUHG_VZL71yC5vDomiz~@P2z{OH*ZXve616%sXYOw=c?c^Oa+qR zUr3d3Tk?yd8u#Hk%bii@^JeL+h01A)RbFKAIHgDbN^0|pNt)`{E7z2x@klH0|N^50G1>AOHZG3BG*3u(+bWNw9 zGUZKExzMDPn$FT5U&04Vp*4V#Lk3#}-$B!jMYR4Wt1EP~9XW5<)ClRQ3C45m0~!x# zaD;@#15(JmdgHBdOfngDCigoBX>if85)dMM!1#4?8CnS)4(zcU9h_2`Lbp)jr6+P& ziJUN7wYEcgyy>D~M`yEL&fbG}xRgN(Dw&`#vq>Vt-mBLrdk?-}^E;638(ctqJ(+Mh zVEQ{Q)TwSpv|@~o9%FzcnAO}B0(BD<_&I01(saZYCRKn_L(H|_c47;cPF8@zNb1Aa za$k4GS#=(wqbFOh+EQs=WM=ZiDa77*FuTpy#8;jzec$uYcnOH+YA zTP$|yXp)le^8sfMe~06EotE2~`XO=LW`kQb#FLoz2j;gXV-(g&yR-{J)TrP92Iqx9f%Cg#PVg2G@sEAhfxFtg< zKYV0DnTX4&;u!C5MbS^OiiI!%kO!kE#cwbB82~MvoSyvjv*_4iFy>^T9FX5jFyt7l zuC-~F5~xl{3Gzi6d)6d%CaySEz`50Vw3b7-KTM2&d~cQ>Mj@R8LO+T=a1(gIJ2G%E zYrt=~1uw#3cuZFLiQ0I6ZA{FtEuwfnIR2S2ZB=@kt)gpl*I-Eynk{LE(S6GVQ2AKF zC>qSqkXvOixZf9pTl`&DFZZ3Y#PLGN$o4Sd zt_F#W&=E_6UptKPSUJW$qVh4=*>E{g2(&9;}gL&{Q_R*bt1KU8AUfk1a{d4kxN z&tr>;xuZGb3MOvH&n!0I{w!cCxE=0(ZUt@nvQP3ixuTmZ4P_WoPcZ}of0O&$kp4~a zFQcl7lO=yHL>dC8UIj1~jb&pO2$a$>gasc{1>LDEY2RXM1E{ey2pJ)hTE1D^2#Q7c zO6yEWQy`rEEnYYo42{CMeT(C|bPD_|=Wr{2QC5C-1;{fqm3eg7|hXt2W{T zeDdXAJe=J!K-@yA)|XvdJltHqq-0vC5{wC>-~`U^Gnfg;$WeZXG6c}$9N|?$1R1HjhQIHXKzs8kd*&p$Y@}?K|J-j2}u}@8q@ispM z$>Nkt)Z9yB*k4sBTxs(F5j70t3}0DZJfhIpQ@(+4?P=@2W9qDw8D153?T~j#@4HW6 zLR2V}m*&0N2fLgq>z~?ZZ|Bu7`^$L&8*aniq{_+{YbIQR$1hABQUs{h+%FM}`p`5*8z@?YTp;q{m0Ty~{Davot~ b^M5#&wk95?`vCwDVQx801TXv9;R5_0c0F%h literal 0 HcmV?d00001 diff --git a/tests/fixtures/utf8.csv b/tests/fixtures/utf8.csv new file mode 100644 index 0000000..918ee9a --- /dev/null +++ b/tests/fixtures/utf8.csv @@ -0,0 +1,2 @@ +col +café diff --git a/tests/sandbox/test_executor.py b/tests/sandbox/test_executor.py index 5d153cc..d29576b 100644 --- a/tests/sandbox/test_executor.py +++ b/tests/sandbox/test_executor.py @@ -665,3 +665,74 @@ def test_keyboard_interrupt_not_available(self, staad, executor): code = "result = KeyboardInterrupt" r = executor.execute(code, staad) assert not r.success, "KeyboardInterrupt should not be available in sandbox" + + +class TestInputInjection: + """Tests for ``__input`` data injection into the sandbox.""" + + def test_input_none_when_no_data(self, staad, executor): + """``__input`` is None when no input_data is provided (backward compat).""" + r = executor.execute("result = __input__", staad) + assert r.success + assert r.result is None + + def test_input_contains_provided_data(self, staad, executor): + data = (("a", "b"), (1, 2), (3, 4)) + r = executor.execute("result = [list(row) for row in __input__]", staad, input_data=data) + assert r.success + assert r.result == [["a", "b"], [1, 2], [3, 4]] + + def test_input_deeply_immutable(self, staad, executor): + """Sandbox code cannot mutate ``__input__`` (tuples are immutable).""" + data = (("a", "b"), (1, 2)) + r = executor.execute( + dedent( + """ + try: + __input__[0] = "mutated" + result = "mutation succeeded" + except TypeError: + result = "immutable" + """ + ), + staad, + input_data=data, + ) + assert r.success + assert r.result == "immutable" + + def test_input_iterable(self, staad, executor): + """Sandbox code can iterate over ``__input__``.""" + data = ((10,), (20,), (30,)) + r = executor.execute( + "result = sum(row[0] for row in __input__)", + staad, + input_data=data, + ) + assert r.success + assert r.result == 60 + + def test_input_indexable(self, staad, executor): + """Sandbox code can index into ``__input__``.""" + data = (("x",), (42,)) + r = executor.execute("result = __input__[1][0]", staad, input_data=data) + assert r.success + assert r.result == 42 + + def test_input_no_carryover(self, staad, executor): + """``__input__`` does not persist across executions.""" + executor.execute("x = __input__", staad, input_data=((1,),)) + r = executor.execute("result = __input__", staad) + assert r.success + assert r.result is None + + def test_input_dict_shape(self, staad, executor): + """Dict-shaped __input__ (XLSX multi-sheet) works.""" + data = {"Sheet1": {"columns": ("a",), "rows": ((1,), (2,))}} + r = executor.execute( + "result = len(__input__['Sheet1']['rows'])", + staad, + input_data=data, + ) + assert r.success + assert r.result == 2 diff --git a/tests/test_connection.py b/tests/test_connection.py index 94723b3..f20b205 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -10,6 +10,7 @@ from __future__ import annotations import asyncio +import sys import threading from typing import Any from unittest.mock import MagicMock, patch @@ -17,6 +18,7 @@ import pytest from openstaad_mcp.connection import InstanceRegistry, StaadInstance, connect_and_run +from openstaad_mcp.server import create_mcp_server # --------------------------------------------------------------------------- # get_active_instances — ROT enumeration (mocked COM layer) @@ -131,7 +133,6 @@ def _mock_get_active_instances(instances: list[StaadInstance]): class TestInstanceSelection: def test_auto_select_single_instance(self): """With one instance and no 'instance' param, it is selected automatically.""" - from openstaad_mcp.server import create_mcp_server single = [StaadInstance(alias="staadPro1", pid=1234, file_path="C:\\A.std", version="22.12")] expected = {"success": True, "result": 42, "stdout": "", "stderr": "", "error": None, "duration_seconds": 0.1} @@ -140,7 +141,7 @@ def test_auto_select_single_instance(self): _mock_get_active_instances(single), patch("openstaad_mcp.server.connect_and_run", return_value=expected) as mock_run, ): - mcp = create_mcp_server() + mcp = create_mcp_server(allowed_dirs=[]) asyncio.run(mcp.call_tool("execute_code", {"code": "result = 42"})) assert mock_run.called called_path = mock_run.call_args[0][1] @@ -148,9 +149,7 @@ def test_auto_select_single_instance(self): def test_auto_select_errors_on_zero_instances(self): with _mock_get_active_instances([]): - from openstaad_mcp.server import create_mcp_server - - mcp = create_mcp_server() + mcp = create_mcp_server(allowed_dirs=[]) result = asyncio.run(mcp.call_tool("execute_code", {"code": "result = 1"})) text = result.content[0].text assert "No STAAD.Pro instances found" in text @@ -161,9 +160,7 @@ def test_auto_select_errors_on_multiple_instances(self): StaadInstance(alias="staadPro2", pid=5678, file_path="C:\\B.std", version="22.12"), ] with _mock_get_active_instances(two): - from openstaad_mcp.server import create_mcp_server - - mcp = create_mcp_server() + mcp = create_mcp_server(allowed_dirs=[]) result = asyncio.run(mcp.call_tool("execute_code", {"code": "result = 1"})) text = result.content[0].text assert "staadPro1" in text @@ -186,8 +183,6 @@ def _hang(staad: Any) -> None: mock_os = MagicMock() mock_os.connect.return_value = MagicMock() - import sys - with ( patch.dict(sys.modules, {"openstaadpy": MagicMock(), "openstaadpy.os_analytical": mock_os}), pytest.raises(TimeoutError), @@ -197,7 +192,6 @@ def _hang(staad: Any) -> None: def test_non_windows_raises_import_error(self): """On non-Windows, openstaadpy is unavailable — ImportError propagates as exception.""" - import sys def _fn(staad: Any) -> str: return "ok" From 5875dc46e317348ca500c4e3e146948e3be28f72 Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Mon, 25 May 2026 18:10:25 -0400 Subject: [PATCH 2/6] Copilot Review Feedback --- README.md | 5 +- src/openstaad_mcp/file_io.py | 707 ------------------------ src/openstaad_mcp/file_io/const.py | 4 +- src/openstaad_mcp/file_io/helpers.py | 2 +- src/openstaad_mcp/file_io/readers.py | 4 +- src/openstaad_mcp/file_io/validation.py | 14 +- src/openstaad_mcp/server.py | 4 +- tests/file_io/test_validation.py | 36 +- tests/file_io/test_writers.py | 8 + 9 files changed, 52 insertions(+), 732 deletions(-) delete mode 100644 src/openstaad_mcp/file_io.py diff --git a/README.md b/README.md index 984b89e..1d0825f 100644 --- a/README.md +++ b/README.md @@ -198,8 +198,9 @@ CSV and XLSX files directly and injects the data into the sandbox as the `__inpu | `output_path` | Path where the sandbox return value will be written. The return value must be a list-of-lists (CSV) or a `{sheet_name: {columns, rows}}` dict (multi-sheet XLSX). | | `overwrite` | Allow overwriting an existing output file (default `false`). | -**Path containment:** All file paths must resolve inside an MCP root configured by the client. -The server validates paths against the client-provided roots before any file access. +**Path containment:** File paths must resolve inside a configured allowed boundary before any read/write occurs. +The server supports both **client-configured MCP roots** and **server-configured allowed directories** (via `--allowed-dirs` or `user_config.allowed_directories` in the manifest). +The server validates paths against these boundaries before any file access. **Limits:** Max file size 50 MB, max 100K rows, max 500 columns, max 50 input sheets. diff --git a/src/openstaad_mcp/file_io.py b/src/openstaad_mcp/file_io.py deleted file mode 100644 index acff0bb..0000000 --- a/src/openstaad_mcp/file_io.py +++ /dev/null @@ -1,707 +0,0 @@ -""" -Server-side file I/O for the ``execute_code`` tool. - -All operations run **outside** the sandbox. The sandbox never touches the -filesystem — it receives pre-parsed data via ``__input__`` and returns a -structured value that this module writes to disk. - -Architecture ------------- -Reading and writing are handled by format-specific subclasses: - -- :class:`CSVReader` / :class:`CSVWriter` -- :class:`XLSXReader` / :class:`XLSXWriter` - -Each inherits from :class:`BaseReader` or :class:`BaseWriter`, which enforce -file-size limits, column/row caps, and atomic writes. - -Public API ----------- -``read_input_file`` / ``write_output_file`` - Dispatch to the correct reader/writer based on file extension. - -``get_allowed_dirs`` / ``get_input_data`` - Server-level helpers called from ``execute_code``. - -``validate_return_value`` / ``deep_freeze`` - Data validation and summary helpers. -""" - -from __future__ import annotations - -import abc -import csv -import logging -import os -import time -import uuid -from datetime import date, datetime -from datetime import time as dt_time -from pathlib import Path -from typing import Any - -import chardet -import openpyxl -from fastmcp.server.context import Context -from mcp.shared.exceptions import McpError - -from openstaad_mcp.sandbox.const import ( - MAX_FILE_SIZE_BYTES, - MAX_INPUT_COLUMNS, - MAX_INPUT_ROWS, - MAX_INPUT_SHEETS, - MAX_OUTPUT_COLUMNS, - MAX_OUTPUT_ROWS, - MAX_OUTPUT_SHEETS, - MAX_SHEET_NAME_LENGTH, - SAMPLE_ROW_COUNT, - STALE_TEMP_AGE_SECONDS, - TEMP_FILE_PREFIX, -) -from openstaad_mcp.file_io.path_validator import FileIOError, parse_roots_to_dirs, validate_io_path - -logger = logging.getLogger(__name__) - -_JSON_PRIMITIVES = (str, int, float, bool, type(None)) - - -# ═══════════════════════════════════════════════════════════════════════════ -# Server-level helpers (called from server.py) -# ═══════════════════════════════════════════════════════════════════════════ - - -def validate_args_allowed_dirs(allowed_dirs: list[str] | None) -> list[Path]: - """Validate and resolve ``--allowed-dir`` CLI arguments to real paths. - - Security: resolves symlinks so later checks compare against real paths. - """ - if not allowed_dirs: - return [] - - result: list[Path] = [] - for dir_str in allowed_dirs: - # Expand ~/… to the user's home directory - expanded = Path(dir_str).expanduser() - absolute = expanded.resolve(strict=False) - normalized_original = Path(os.path.normpath(absolute)) - - try: - # Security: resolve symlinks in allowed directories during startup - resolved = absolute.resolve(strict=True) - normalized_resolved = Path(os.path.normpath(resolved)) - result.append(normalized_resolved) - except OSError: - # If we can't resolve (doesn't exist), use the normalized absolute path - # This allows configuring allowed dirs that will be created later - result.append(normalized_original) - - return result - - -async def get_allowed_dirs( - ctx: Context, args_allowed_dirs: list[Path], input_path: str | None, output_path: str | None -) -> list[Path]: - """Resolve MCP roots into a list of allowed directories.""" - logger.debug("Args allowed dirs: %s", args_allowed_dirs) - allowed_dirs: list[Path] = [Path(el) for el in args_allowed_dirs] - if input_path is not None or output_path is not None: - try: - roots = await ctx.list_roots() - logger.debug(f"Received MCP roots: {roots}") - except McpError as exc: - logger.error(f"Error listing MCP roots: {exc}") - roots = [] - allowed_dirs += parse_roots_to_dirs(roots) - logger.debug(f"Allowed directories for file I/O: {allowed_dirs}") - return allowed_dirs - - -async def get_input_data(input_path: str | None, allowed_dirs: list[Path]) -> tuple[Any, dict[str, Any] | None]: - """Validate path, read file, freeze data. Returns ``(data, summary)``.""" - if input_path is None: - return None, None - resolved_input = validate_io_path(input_path, allowed_dirs, mode="read") - data, input_summary = read_input_file(resolved_input) - return deep_freeze(data), input_summary - - -# ═══════════════════════════════════════════════════════════════════════════ -# Base reader -# ═══════════════════════════════════════════════════════════════════════════ - - -class BaseReader(abc.ABC): - """Base class for file readers. Enforces file-size and limit checks.""" - - def __init__(self, path: Path) -> None: - self.path = path - self._check_file_size() - - def _check_file_size(self) -> None: - size = self.path.stat().st_size - if size > MAX_FILE_SIZE_BYTES: - raise FileIOError( - "FILE_TOO_LARGE", - f"File is {size:,} bytes; limit is {MAX_FILE_SIZE_BYTES:,} bytes", - ) - - @abc.abstractmethod - def read(self, *, start_row: int = 0, max_rows: int | None = None, **kwargs: Any) -> Any: - """Parse the file and return structured data.""" - - @abc.abstractmethod - def build_summary(self, data: Any) -> dict[str, Any]: - """Build a lightweight summary for the agent.""" - - -# ── Header detection ───────────────────────────────────────────────────── - - -def _cell_type(value: Any) -> str: - """Classify a cell value for header-detection comparison.""" - if value is None: - return "null" - if isinstance(value, bool): - return "bool" - if isinstance(value, (int, float)): - return "numeric" - if isinstance(value, (datetime, date, dt_time)): - return "numeric" - return "string" - - -def _detect_header(rows: list[list], has_header: bool | None) -> bool: - """Detect whether the first row is a header. - - When *has_header* is ``None`` (auto-detect), samples up to 5 rows and - compares the per-column type of row 0 against the majority type of - rows 1-4. If any column's first-row type differs from its data-row - majority, the first row is treated as a header. - """ - if has_header is not None: - return has_header - - if len(rows) <= 1: - return True # Too few rows to compare — conservative default - - first_row = rows[0] - data_rows = rows[1:5] # Up to 4 data rows for comparison - - if not first_row: - return True - - for col_idx in range(len(first_row)): - first_type = _cell_type(first_row[col_idx]) - if first_type == "null": - continue - - type_counts: dict[str, int] = {} - for row in data_rows: - if col_idx < len(row): - t = _cell_type(row[col_idx]) - if t != "null": - type_counts[t] = type_counts.get(t, 0) + 1 - - if not type_counts: - continue - - majority_type = max(type_counts, key=type_counts.get) - if first_type != majority_type: - return True # Type mismatch → first row is a header - - return False # All columns match → not a header - - -def _auto_columns(num_cols: int) -> list[str]: - """Generate column names ``col_1, col_2, …`` for headerless data.""" - return [f"col_{i + 1}" for i in range(num_cols)] - - -# ── CSV reader ─────────────────────────────────────────────────────────── - - -class CSVReader(BaseReader): - """Reads a CSV file into ``list[list]`` (array-of-arrays). - - Uses ``chardet`` for encoding detection and ``csv.Sniffer`` for - dialect detection. Values are coerced from strings to int/float - where possible. Streams from disk line-by-line. - """ - - _CHARDET_MIN_CONFIDENCE = 0.5 - - def read( - self, *, start_row: int = 0, max_rows: int | None = None, has_header: bool | None = None, **kwargs: Any - ) -> list[list]: - encoding = self._detect_encoding() - dialect = self._detect_dialect(encoding) - all_rows: list[list] = [] - - with open(self.path, newline="", encoding=encoding) as f: - reader = csv.reader(f, dialect) - for raw_row in reader: - coerced = [_coerce_csv_value(v) for v in raw_row] - if len(coerced) > MAX_INPUT_COLUMNS: - raise FileIOError( - "TOO_MANY_COLUMNS", - f"Row has {len(coerced)} columns; limit is {MAX_INPUT_COLUMNS}", - ) - all_rows.append(coerced) - - self._has_header = _detect_header(all_rows, has_header) - - if self._has_header: - if not all_rows: - return [] - data_rows = all_rows[1:] - if len(data_rows) > MAX_INPUT_ROWS: - raise FileIOError( - "TOO_MANY_ROWS", - f"File has {len(data_rows)} data rows; limit is {MAX_INPUT_ROWS}", - ) - sliced = data_rows[start_row:] - if max_rows is not None: - sliced = sliced[:max_rows] - return [all_rows[0], *sliced] if start_row == 0 else sliced - else: - if len(all_rows) > MAX_INPUT_ROWS: - raise FileIOError( - "TOO_MANY_ROWS", - f"File has {len(all_rows)} data rows; limit is {MAX_INPUT_ROWS}", - ) - sliced = all_rows[start_row:] - if max_rows is not None: - sliced = sliced[:max_rows] - return sliced - - def build_summary(self, data: list[list]) -> dict[str, Any]: - has_header = getattr(self, "_has_header", True) - if has_header: - header = data[0] if data else [] - data_rows = data[1:] if len(data) > 1 else [] - else: - num_cols = len(data[0]) if data else 0 - header = _auto_columns(num_cols) - data_rows = data - return { - "total_rows": len(data_rows), - "columns": list(header), - "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], - } - - # -- helpers -- - - def _detect_encoding(self) -> str: - """Detect file encoding: try UTF-8 first, then chardet, then cp1252.""" - raw = self.path.read_bytes() - result = chardet.detect(raw) - encoding = result.get("encoding") - confidence = result.get("confidence", 0) - if encoding and confidence >= self._CHARDET_MIN_CONFIDENCE: - return encoding - # Low confidence or no result — fall back to cp1252 (Windows default) - return "cp1252" - - def _detect_dialect(self, encoding: str) -> type[csv.Dialect]: - """Detect CSV dialect using csv.Sniffer, fall back to ``excel``. - - Only trusts the sniffer when the detected delimiter is a common - separator character. Exotic delimiters (letters, digits, etc.) - indicate a false positive from a small or ambiguous sample. - """ - _COMMON_DELIMITERS = {",", ";", "\t", "|"} - try: - with open(self.path, newline="", encoding=encoding) as f: - sample = f.read(8192) - dialect = csv.Sniffer().sniff(sample) - if dialect.delimiter in _COMMON_DELIMITERS: - return dialect - except csv.Error: - pass - return csv.excel - - -def _coerce_csv_value(val: str) -> int | float | str: - """Attempt int → float → str coercion of a CSV string value.""" - try: - return int(val) - except ValueError: - pass - try: - return float(val) - except ValueError: - pass - return val - - -# ── XLSX reader ────────────────────────────────────────────────────────── - - -class XLSXReader(BaseReader): - """Reads an XLSX workbook into ``{sheet_name: {columns, rows}}``.""" - - def read( - self, - *, - start_row: int = 0, - max_rows: int | None = None, - sheet: str | None = None, - has_header: bool | None = None, - **kwargs: Any, - ) -> dict[str, dict[str, Any]]: - try: - wb = openpyxl.load_workbook(self.path, read_only=True, data_only=True) - except Exception as exc: - raise FileIOError("CORRUPTED_WORKBOOK", f"Cannot open workbook: {exc}") from None - - try: - self._validate_sheet_count(wb) - sheets_to_load = self._resolve_sheets(wb, sheet) - return {name: self._read_sheet(wb[name], name, start_row, max_rows, has_header) for name in sheets_to_load} - finally: - wb.close() - - def build_summary(self, data: dict[str, dict[str, Any]]) -> dict[str, Any]: - sheets = list(data.keys()) - first_sheet = sheets[0] if sheets else None - first = data[first_sheet] if first_sheet else {"columns": [], "rows": []} - return { - "sheets": sheets, - "loaded_sheet": first_sheet, - "total_rows": len(first["rows"]), - "columns": list(first["columns"]), - "sample_rows": [list(r) for r in first["rows"][:SAMPLE_ROW_COUNT]], - } - - # -- helpers -- - - @staticmethod - def _validate_sheet_count(wb: Any) -> None: - if len(wb.sheetnames) > MAX_INPUT_SHEETS: - raise FileIOError( - "TOO_MANY_ROWS", - f"Workbook has {len(wb.sheetnames)} sheets; limit is {MAX_INPUT_SHEETS}", - ) - - @staticmethod - def _resolve_sheets(wb: Any, sheet: str | None) -> list[str]: - if sheet is not None: - if sheet not in wb.sheetnames: - raise FileIOError("SHEET_NOT_FOUND", f"Sheet '{sheet}' not found in workbook") - return [sheet] - return list(wb.sheetnames) - - @staticmethod - def _read_sheet( - ws: Any, name: str, start_row: int, max_rows: int | None, has_header: bool | None - ) -> dict[str, Any]: - raw_rows: list[list] = [] - for row in ws.iter_rows(values_only=True): - raw_rows.append(list(row)) - - # Check column count on the first row - if raw_rows and len(raw_rows[0]) > MAX_INPUT_COLUMNS: - raise FileIOError( - "TOO_MANY_COLUMNS", - f"Sheet '{name}' has {len(raw_rows[0])} columns; limit is {MAX_INPUT_COLUMNS}", - ) - - # Detect header using raw types (before datetime → string conversion) - is_header = _detect_header(raw_rows, has_header) - - # Convert to JSON primitives - all_rows = [[_to_json_primitive(c) for c in row] for row in raw_rows] - - if is_header: - columns = all_rows[0] if all_rows else [] - data_rows = all_rows[1:] - else: - num_cols = len(all_rows[0]) if all_rows else 0 - columns = _auto_columns(num_cols) - data_rows = all_rows - - if len(data_rows) > MAX_INPUT_ROWS: - raise FileIOError( - "TOO_MANY_ROWS", - f"Sheet '{name}' exceeds {MAX_INPUT_ROWS} rows", - ) - - sliced = data_rows[start_row:] - if max_rows is not None: - sliced = sliced[:max_rows] - - return {"columns": columns, "rows": sliced} - - -def _to_json_primitive(value: Any) -> str | int | float | bool | None: - """Convert an openpyxl cell value to a JSON-safe primitive.""" - if value is None: - return None - if isinstance(value, bool): - return value - if isinstance(value, (int, float)): - return value - if isinstance(value, str): - return value - if isinstance(value, datetime): - return value.isoformat() - if isinstance(value, date): - return value.isoformat() - if isinstance(value, dt_time): - return value.isoformat() - return str(value) - - -# ═══════════════════════════════════════════════════════════════════════════ -# Base writer -# ═══════════════════════════════════════════════════════════════════════════ - - -class BaseWriter(abc.ABC): - """Base class for file writers. Handles atomic writes via temp file.""" - - def __init__(self, path: Path) -> None: - self.path = path - - def write(self, data: Any, *, overwrite: bool = False) -> dict[str, Any]: - """Validate, write atomically, and return a summary.""" - if self.path.exists() and not overwrite: - raise FileIOError("FILE_EXISTS", f"File already exists: {self.path}") - _clean_stale_temps(self.path.parent) - - tmp = _temp_path(self.path.parent) - try: - self._write_to(tmp, data) - os.replace(tmp, self.path) - except BaseException: - tmp.unlink(missing_ok=True) - raise - return self.build_summary(data) - - @abc.abstractmethod - def _write_to(self, tmp: Path, data: Any) -> None: - """Write *data* to the temporary file *tmp*.""" - - @abc.abstractmethod - def build_summary(self, data: Any) -> dict[str, Any]: - """Build a lightweight summary for the agent.""" - - -# ── CSV writer ─────────────────────────────────────────────────────────── - - -class CSVWriter(BaseWriter): - """Writes ``list[list]`` to a CSV file.""" - - def _write_to(self, tmp: Path, data: list[list]) -> None: - with open(tmp, "w", newline="", encoding="utf-8") as f: - csv.writer(f).writerows(data) - - def build_summary(self, data: list[list]) -> dict[str, Any]: - header = data[0] if data else [] - data_rows = data[1:] if len(data) > 1 else [] - return { - "message": f"The `result` data has been written to `{self.path}`", - "rows_written": len(data_rows), - "columns": list(header), - "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], - } - - -# ── XLSX writer ────────────────────────────────────────────────────────── - - -class XLSXWriter(BaseWriter): - """Writes flat or multi-sheet data to an XLSX file.""" - - def _write_to(self, tmp: Path, data: Any) -> None: - wb = openpyxl.Workbook() - if isinstance(data, dict): - for i, (name, sheet_data) in enumerate(data.items()): - if i == 0: - ws = wb.active - assert ws is not None - ws.title = name - else: - ws = wb.create_sheet(title=name) - ws.append(sheet_data["columns"]) - for row in sheet_data["rows"]: - ws.append(row) - else: - ws = wb.active - assert ws is not None - for row in data: - ws.append(row) - wb.save(tmp) - - def build_summary(self, data: Any) -> dict[str, Any]: - if isinstance(data, dict): - return { - "message": f"The `result` data has been written to `{self.path}`", - "sheets": { - name: { - "columns": sheet["columns"], - "rows_written": len(sheet["rows"]), - "sample_rows": [list(r) for r in sheet["rows"][:SAMPLE_ROW_COUNT]], - } - for name, sheet in data.items() - }, - } - header = data[0] if data else [] - data_rows = data[1:] if len(data) > 1 else [] - return { - "message": f"The `result` data has been written to `{self.path}`", - "rows_written": len(data_rows), - "columns": list(header), - "sample_rows": [list(r) for r in data_rows[:SAMPLE_ROW_COUNT]], - } - - -# ═══════════════════════════════════════════════════════════════════════════ -# Reader/writer factory helpers -# ═══════════════════════════════════════════════════════════════════════════ - -_READERS: dict[str, type[BaseReader]] = {".csv": CSVReader, ".xlsx": XLSXReader} -_WRITERS: dict[str, type[BaseWriter]] = {".csv": CSVWriter, ".xlsx": XLSXWriter} - - -def _get_reader(path: Path) -> BaseReader: - ext = path.suffix.lower() - cls = _READERS.get(ext) - if cls is None: - raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot read '{ext}' files") - return cls(path) - - -def _get_writer(path: Path) -> BaseWriter: - ext = path.suffix.lower() - cls = _WRITERS.get(ext) - if cls is None: - raise FileIOError("UNSUPPORTED_FORMAT", f"Cannot write '{ext}' files") - return cls(path) - - -# ═══════════════════════════════════════════════════════════════════════════ -# Public dispatch functions (preserve existing API) -# ═══════════════════════════════════════════════════════════════════════════ - - -def read_input_file( - path: Path, - *, - sheet: str | None = None, - start_row: int = 0, - max_rows: int | None = None, - has_header: bool | None = None, -) -> tuple[Any, dict[str, Any]]: - """Read a CSV or XLSX file and return ``(data, summary)``.""" - reader = _get_reader(path) - data = reader.read(start_row=start_row, max_rows=max_rows, sheet=sheet, has_header=has_header) - summary = reader.build_summary(data) - return data, summary - - -def write_output_file(path: str, data: Any, allowed_dirs: list[Path], *, overwrite: bool = False) -> dict[str, Any]: - """Validate path, validate return value, write atomically, return summary.""" - resolved = validate_io_path(path, allowed_dirs, mode="write") - validate_return_value(data) - writer = _get_writer(resolved) - return writer.write(data, overwrite=overwrite) - - -# ═══════════════════════════════════════════════════════════════════════════ -# Return value validation -# ═══════════════════════════════════════════════════════════════════════════ - - -def validate_return_value(value: Any) -> None: - """Raise :class:`FileIOError` if *value* is not a valid output structure. - - Accepts: - - ``list[list[primitive]]`` (flat / CSV / single-sheet) - - ``dict[str, {columns: list, rows: list[list[primitive]]}]`` (multi-sheet) - """ - if isinstance(value, list): - _validate_flat(value) - elif isinstance(value, dict): - _validate_multi_sheet(value) - else: - raise FileIOError( - "INVALID_RETURN_SHAPE", - "Return value must be a list of lists (flat) or a dict of sheets (multi-sheet)", - ) - - -def _validate_flat(rows: list) -> None: - if len(rows) > MAX_OUTPUT_ROWS + 1: # +1 header - raise FileIOError("INVALID_RETURN_SHAPE", f"Too many rows: {len(rows)}; limit {MAX_OUTPUT_ROWS}") - for row in rows: - if not isinstance(row, (list, tuple)): - raise FileIOError("INVALID_RETURN_SHAPE", f"Each row must be a list, got {type(row).__name__}") - if len(row) > MAX_OUTPUT_COLUMNS: - raise FileIOError("INVALID_RETURN_SHAPE", f"Too many columns: {len(row)}; limit {MAX_OUTPUT_COLUMNS}") - for cell in row: - if not isinstance(cell, _JSON_PRIMITIVES): - raise FileIOError( - "INVALID_RETURN_SHAPE", - f"Cell value must be a JSON primitive, got {type(cell).__name__}", - ) - - -def _validate_multi_sheet(sheets: dict) -> None: - if len(sheets) > MAX_OUTPUT_SHEETS: - raise FileIOError("INVALID_RETURN_SHAPE", f"Too many sheets: {len(sheets)}; limit {MAX_OUTPUT_SHEETS}") - for name, sheet_data in sheets.items(): - if len(name) > MAX_SHEET_NAME_LENGTH: - raise FileIOError( - "INVALID_RETURN_SHAPE", - f"Sheet name '{name}' exceeds {MAX_SHEET_NAME_LENGTH} characters", - ) - if not isinstance(sheet_data, dict) or "columns" not in sheet_data or "rows" not in sheet_data: - raise FileIOError( - "INVALID_RETURN_SHAPE", - "Each sheet must have 'columns' and 'rows' keys", - ) - _validate_flat([sheet_data["columns"], *list(sheet_data["rows"])]) - - -# ═══════════════════════════════════════════════════════════════════════════ -# Deep freeze -# ═══════════════════════════════════════════════════════════════════════════ - - -def deep_freeze(data: Any) -> Any: - """Recursively convert mutable containers to immutable equivalents. - - - ``list`` → ``tuple`` - - ``dict`` values are recursively frozen (dict keys stay as-is since - strings are already immutable) - - Primitives (str, int, float, bool, None) pass through unchanged. - """ - if data is None or isinstance(data, (str, int, float, bool)): - return data - if isinstance(data, (list, tuple)): - return tuple(deep_freeze(item) for item in data) - if isinstance(data, dict): - return {k: deep_freeze(v) for k, v in data.items()} - return data - - -# ═══════════════════════════════════════════════════════════════════════════ -# Shared utilities -# ═══════════════════════════════════════════════════════════════════════════ - - -def _temp_path(directory: Path) -> Path: - return directory / f"{TEMP_FILE_PREFIX}{uuid.uuid4().hex}.tmp" - - -def _clean_stale_temps(directory: Path) -> None: - """Remove orphaned temp files older than ``STALE_TEMP_AGE_SECONDS``.""" - cutoff = time.time() - STALE_TEMP_AGE_SECONDS - for p in directory.glob(f"{TEMP_FILE_PREFIX}*.tmp"): - try: - if p.stat().st_mtime < cutoff: - p.unlink() - except OSError: - pass diff --git a/src/openstaad_mcp/file_io/const.py b/src/openstaad_mcp/file_io/const.py index f857099..b5d9e4b 100644 --- a/src/openstaad_mcp/file_io/const.py +++ b/src/openstaad_mcp/file_io/const.py @@ -34,5 +34,5 @@ # Number of sample rows included in summaries returned to the agent. SAMPLE_ROW_COUNT = 5 -# Maximum character length for a single cell value (matches Excel's limit). -MAX_CELL_SIZE = 32_768 +# Maximum character length for a single cell value (Excel limit: 32,767 characters). +MAX_CELL_SIZE = 32_767 diff --git a/src/openstaad_mcp/file_io/helpers.py b/src/openstaad_mcp/file_io/helpers.py index ec35dcc..b5a3cfd 100644 --- a/src/openstaad_mcp/file_io/helpers.py +++ b/src/openstaad_mcp/file_io/helpers.py @@ -70,7 +70,7 @@ def read_input_file( def write_output_file(path: str, data: Any, allowed_dirs: list[Path], *, overwrite: bool = False) -> dict[str, Any]: """Validate path, validate return value, write atomically, return summary.""" resolved = validate_io_path(path, allowed_dirs, mode="write") - validate_return_value(data) + validate_return_value(resolved, data) writer = _get_writer(resolved) return writer.write(data, overwrite=overwrite) diff --git a/src/openstaad_mcp/file_io/readers.py b/src/openstaad_mcp/file_io/readers.py index d3a90e7..dec1cef 100644 --- a/src/openstaad_mcp/file_io/readers.py +++ b/src/openstaad_mcp/file_io/readers.py @@ -80,7 +80,7 @@ def _detect_header(rows: list[list], has_header: bool | None) -> bool: if not type_counts: continue - majority_type = max(type_counts, key=type_counts.get) + majority_type = max(type_counts, key=type_counts.get) # type: ignore if first_type != majority_type: return True # Type mismatch -> first row is a header @@ -294,7 +294,7 @@ def build_summary(self, data: dict[str, dict[str, Any]]) -> dict[str, Any]: def _validate_sheet_count(wb: Any) -> None: if len(wb.sheetnames) > MAX_INPUT_SHEETS: raise FileIOError( - "TOO_MANY_ROWS", + "TOO_MANY_SHEETS", f"Workbook has {len(wb.sheetnames)} sheets; limit is {MAX_INPUT_SHEETS}", ) diff --git a/src/openstaad_mcp/file_io/validation.py b/src/openstaad_mcp/file_io/validation.py index 83ce4d2..6e120c1 100644 --- a/src/openstaad_mcp/file_io/validation.py +++ b/src/openstaad_mcp/file_io/validation.py @@ -11,6 +11,7 @@ import os from pathlib import Path +from types import MappingProxyType from typing import Any from pydantic import ValidationError @@ -19,7 +20,7 @@ from openstaad_mcp.file_io.path_validator import FileIOError -def validate_return_value(value: Any) -> None: +def validate_return_value(path: Path, value: Any) -> None: """Raise :class:`FileIOError` if *value* is not a valid output structure. Accepts: @@ -28,12 +29,18 @@ def validate_return_value(value: Any) -> None: Tuples are accepted interchangeably with lists (sandbox returns frozen data). """ + ext = path.suffix.lower() if isinstance(value, (list, tuple)): try: FlatOutput.model_validate(value) except ValidationError as exc: raise FileIOError("INVALID_RETURN_SHAPE", _format_errors(exc)) from None elif isinstance(value, dict): + if ext == ".csv": + raise FileIOError( + "SHAPE_EXTENSION_MISMATCH", + "Multi-sheet data cannot be written to a CSV file; use .xlsx or provide a flat list of rows", + ) try: MultiSheetOutput.model_validate(value) except ValidationError as exc: @@ -86,8 +93,7 @@ def deep_freeze(data: Any) -> Any: """Recursively convert mutable containers to immutable equivalents. - ``list`` -> ``tuple`` - - ``dict`` values are recursively frozen (dict keys stay as-is since - strings are already immutable) + - ``dict`` -> ``MappingProxyType`` (with recursively frozen values) - Primitives (str, int, float, bool, None) pass through unchanged. """ if data is None or isinstance(data, (str, int, float, bool)): @@ -95,5 +101,5 @@ def deep_freeze(data: Any) -> Any: if isinstance(data, (list, tuple)): return tuple(deep_freeze(item) for item in data) if isinstance(data, dict): - return {k: deep_freeze(v) for k, v in data.items()} + return MappingProxyType({k: deep_freeze(v) for k, v in data.items()}) return data diff --git a/src/openstaad_mcp/server.py b/src/openstaad_mcp/server.py index 35b123a..63e9286 100644 --- a/src/openstaad_mcp/server.py +++ b/src/openstaad_mcp/server.py @@ -27,8 +27,8 @@ from openstaad_mcp.connection import InstanceRegistry, StaadInstance, connect_and_run from openstaad_mcp.file_io import get_allowed_dirs, get_input_data, write_output_file -from openstaad_mcp.sandbox.executor import Executor from openstaad_mcp.file_io.path_validator import FileIOError +from openstaad_mcp.sandbox.executor import Executor from openstaad_mcp.skills import SkillsManager from openstaad_mcp.version import check_version_warning @@ -228,7 +228,7 @@ async def execute_code( - ``overwrite``: allow overwriting an existing output file. - Paths must be inside MCP roots or `allowed_dirs`configured by the client. + Paths must be inside MCP roots or `allowed_dirs` configured by the client. On Claude Desktop, users can configure allowed directories in the extension settings. If no roots are configured, omit both file I/O params and handle the returned `result` value in the agent instead (e.g. write the file via a separate tool). diff --git a/tests/file_io/test_validation.py b/tests/file_io/test_validation.py index ed56fb7..83e7f8b 100644 --- a/tests/file_io/test_validation.py +++ b/tests/file_io/test_validation.py @@ -2,6 +2,7 @@ import os from pathlib import Path +from types import MappingProxyType import pytest @@ -15,73 +16,75 @@ class TestValidateReturnValue: def test_valid_flat(self): - validate_return_value([["a", "b"], [1, 2]]) + validate_return_value(Path("test.csv"), [["a", "b"], [1, 2]]) def test_valid_multi_sheet(self): validate_return_value( + Path("test.xlsx"), { "S1": {"columns": ["a"], "rows": [[1]]}, - } + }, ) def test_rejects_non_primitive_leaf(self): with pytest.raises(FileIOError) as exc_info: - validate_return_value([["a"], [object()]]) + validate_return_value(Path("test.csv"), [["a"], [object()]]) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_too_many_sheets(self): data = {f"S{i}": {"columns": ["a"], "rows": [[1]]} for i in range(21)} with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.xlsx"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_too_many_rows(self): data = [["a"]] + [[i] for i in range(100_001)] with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.csv"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_too_many_columns(self): data = [[i for i in range(501)]] with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.csv"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_long_sheet_name(self): data = {"A" * 32: {"columns": ["a"], "rows": [[1]]}} with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.xlsx"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_non_list_non_dict(self): with pytest.raises(FileIOError) as exc_info: - validate_return_value("not valid") + validate_return_value(Path("test.csv"), "not valid") assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_oversized_cell_in_flat(self): """String cell exceeding MAX_CELL_SIZE is rejected.""" data = [["a"], ["x" * 32_769]] with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.csv"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_oversized_cell_in_multi_sheet(self): """Oversized cell in multi-sheet output is rejected.""" data = {"S1": {"columns": ["a"], "rows": [["x" * 32_769]]}} with pytest.raises(FileIOError) as exc_info: - validate_return_value(data) + validate_return_value(Path("test.xlsx"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_accepts_tuples_as_rows(self): """Tuples (from deep_freeze) should be accepted as rows.""" - validate_return_value((("a", "b"), (1, 2))) + validate_return_value(Path("test.csv"), (("a", "b"), (1, 2))) def test_accepts_tuples_in_multi_sheet(self): """Frozen multi-sheet data should be accepted.""" validate_return_value( + Path("test.xlsx"), { "S1": {"columns": ("a",), "rows": ((1,),)}, - } + }, ) @@ -100,8 +103,17 @@ def test_lists_become_tuples(self): def test_dict_values_frozen(self): data = {"S": {"columns": ["a"], "rows": [[1]]}} frozen = deep_freeze(data) + assert isinstance(frozen, MappingProxyType) + assert isinstance(frozen["S"], MappingProxyType) assert isinstance(frozen["S"]["rows"], tuple) + def test_dict_is_immutable(self): + frozen = deep_freeze({"a": 1}) + with pytest.raises(TypeError): + frozen["a"] = 2 + with pytest.raises(TypeError): + frozen["b"] = 3 + def test_none_passthrough(self): assert deep_freeze(None) is None diff --git a/tests/file_io/test_writers.py b/tests/file_io/test_writers.py index 350a830..137b31a 100644 --- a/tests/file_io/test_writers.py +++ b/tests/file_io/test_writers.py @@ -58,6 +58,14 @@ def test_stale_temp_cleanup(self, tmp_path: Path): write_output_file(str(p), [["x"], [1]], allowed_dirs=[tmp_path]) assert not stale.exists() + def test_multi_sheet_to_csv_rejected(self, tmp_path: Path): + """Multi-sheet dict cannot be written to a CSV file.""" + p = tmp_path / "out.csv" + data = {"Sheet1": {"columns": ["a"], "rows": [[1]]}} + with pytest.raises(FileIOError) as exc_info: + write_output_file(str(p), data, allowed_dirs=[tmp_path]) + assert exc_info.value.code == "SHAPE_EXTENSION_MISMATCH" + # ═══════════════════════════════════════════════════════════════════════════ # XLSX Write From e36ba4c9a9650afa80e7e10456c62b13ec204b78 Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Wed, 27 May 2026 11:15:52 -0400 Subject: [PATCH 3/6] improvements after internal feedback --- README.md | 4 +- mcpb/manifest.json | 4 +- src/openstaad_mcp/file_io/__init__.py | 2 +- src/openstaad_mcp/file_io/path_validator.py | 4 +- src/openstaad_mcp/sandbox/ast.py | 4 +- src/openstaad_mcp/sandbox/const.py | 5 -- src/openstaad_mcp/sandbox/executor.py | 6 +- src/openstaad_mcp/server.py | 79 ++++++++++--------- .../staad_skills/staad-core/SKILL.md | 12 ++- tests/sandbox/test_executor.py | 28 +++---- 10 files changed, 76 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 1d0825f..6eaca2b 100644 --- a/README.md +++ b/README.md @@ -190,11 +190,11 @@ The server supports two transport modes: The `execute_code` tool supports optional **server-side file I/O** for bulk data workflows. Instead of passing large datasets through the agent's context window, the server reads/writes -CSV and XLSX files directly and injects the data into the sandbox as the `__input__` variable. +CSV and XLSX files directly and injects the data into the sandbox as the `input_data` variable. | Parameter | Description | |-----------|-------------| -| `input_path` | Path to a `.csv` or `.xlsx` file. The server reads and parses it, then injects the data as the immutable `__input__` variable in the sandbox. | +| `input_path` | Path to a `.csv` or `.xlsx` file. The server reads and parses it, then injects the data as the immutable `input_data` variable in the sandbox. | | `output_path` | Path where the sandbox return value will be written. The return value must be a list-of-lists (CSV) or a `{sheet_name: {columns, rows}}` dict (multi-sheet XLSX). | | `overwrite` | Allow overwriting an existing output file (default `false`). | diff --git a/mcpb/manifest.json b/mcpb/manifest.json index 0616b34..66da7ee 100644 --- a/mcpb/manifest.json +++ b/mcpb/manifest.json @@ -47,11 +47,11 @@ }, { "name": "list_instances", - "description": "List currently active STAAD instances. Returns instance IDs, model paths, and STAAD versions. Use instance IDs to target specific models in other tools." + "description": "List currently active STAAD instances." }, { "name": "execute_code", - "description": "Execute Python code against the OpenSTAAD API. The sandbox provides a pre-connected 'staad' variable (the OpenSTAAD root object) plus ``json`` and ``math`` modules. Imports and filesystem access are blocked for security. The last expression value or an explicit ``result = ...`` assignment is returned as the result." + "description": "Execute Python code against the OpenSTAAD API in a sandboxed environment." }, { "name": "get_status", diff --git a/src/openstaad_mcp/file_io/__init__.py b/src/openstaad_mcp/file_io/__init__.py index 021238f..29ad6d5 100644 --- a/src/openstaad_mcp/file_io/__init__.py +++ b/src/openstaad_mcp/file_io/__init__.py @@ -7,7 +7,7 @@ Server-side file I/O for the ``execute_code`` tool. All operations run **outside** the sandbox. The sandbox never touches the -filesystem -- it receives pre-parsed data via ``__input__`` and returns a +filesystem -- it receives pre-parsed data via ``input_data`` and returns a structured value that this module writes to disk. Sub-modules diff --git a/src/openstaad_mcp/file_io/path_validator.py b/src/openstaad_mcp/file_io/path_validator.py index f3c8590..dffc59a 100644 --- a/src/openstaad_mcp/file_io/path_validator.py +++ b/src/openstaad_mcp/file_io/path_validator.py @@ -7,8 +7,8 @@ Shared path validator for file I/O operations. Validates that a file path is safe to read from or write to, using -MCP roots as the containment boundary. Both ``input_path`` and -``output_path`` go through :func:`validate_io_path` — one function, +MCP roots as the containment boundary. Both ``input_data_path`` and +``output_data_path`` go through :func:`validate_io_path` — one function, not duplicated logic. Validation order (do **not** reorder — see Research-file-io.md §5.2): diff --git a/src/openstaad_mcp/sandbox/ast.py b/src/openstaad_mcp/sandbox/ast.py index 1691a4f..5ea159e 100644 --- a/src/openstaad_mcp/sandbox/ast.py +++ b/src/openstaad_mcp/sandbox/ast.py @@ -15,7 +15,7 @@ import re from dataclasses import dataclass, field -from openstaad_mcp.sandbox.const import ALLOWED_DUNDER_NAMES, ALLOWED_MODULE_ATTRS, BLOCKED_ATTRS, BLOCKED_BUILTINS +from openstaad_mcp.sandbox.const import ALLOWED_MODULE_ATTRS, BLOCKED_ATTRS, BLOCKED_BUILTINS _FORMAT_DUNDER_RE = re.compile(r"\{[^}]*\.__[a-zA-Z_][a-zA-Z0-9_]*__") @@ -109,7 +109,7 @@ def visit_Attribute(self, node: ast.Attribute) -> None: def visit_Name(self, node: ast.Name) -> None: if node.id in BLOCKED_BUILTINS: self._err(node, f"reference to '{node.id}' is not allowed") - if node.id.startswith("__") and node.id.endswith("__") and node.id not in ALLOWED_DUNDER_NAMES: + if node.id.startswith("__") and node.id.endswith("__"): self._err(node, f"reference to dunder name '{node.id}' is not allowed") self.generic_visit(node) diff --git a/src/openstaad_mcp/sandbox/const.py b/src/openstaad_mcp/sandbox/const.py index b930828..1564be8 100644 --- a/src/openstaad_mcp/sandbox/const.py +++ b/src/openstaad_mcp/sandbox/const.py @@ -207,11 +207,6 @@ } ) -INPUT_DATA_VARIABLE_NAME = "__input__" - -# Sandbox-injected variables that are exempt from the dunder-name ban. -ALLOWED_DUNDER_NAMES: frozenset[str] = frozenset({INPUT_DATA_VARIABLE_NAME}) - # Per-module attribute whitelists for modules injected into the sandbox. # Used both here (AST-level static check) and in executor.py (_ModuleProxy # runtime enforcement). Only attributes listed here may be accessed on the diff --git a/src/openstaad_mcp/sandbox/executor.py b/src/openstaad_mcp/sandbox/executor.py index c111f77..e862cbc 100644 --- a/src/openstaad_mcp/sandbox/executor.py +++ b/src/openstaad_mcp/sandbox/executor.py @@ -26,7 +26,7 @@ from openstaad_mcp.sandbox.ast import capture_last_expr, validate_code from openstaad_mcp.sandbox.com_proxy import COMProxy -from openstaad_mcp.sandbox.const import ALLOWED_BUILTINS, ALLOWED_MODULE_ATTRS, INPUT_DATA_VARIABLE_NAME +from openstaad_mcp.sandbox.const import ALLOWED_BUILTINS, ALLOWED_MODULE_ATTRS from openstaad_mcp.sandbox.module_proxy import ModuleProxy from openstaad_mcp.sandbox.stdio_helpers import LimitedStringIO, sanitize_output, sanitize_traceback @@ -94,7 +94,7 @@ def execute( staad_object: The connected OpenSTAAD root object (or a mock for testing). input_data: - Optional pre-parsed, deep-frozen data injected as ``__input__`` + Optional pre-parsed, deep-frozen data injected as ``input_data`` in the sandbox globals. ``None`` when no input file is provided. Returns @@ -116,7 +116,7 @@ def execute( sandbox_globals: dict[str, Any] = {"__builtins__": self.safe_builtins.copy()} sandbox_globals.update(self.injected_modules) sandbox_globals["staad"] = COMProxy(staad_object) - sandbox_globals[INPUT_DATA_VARIABLE_NAME] = input_data + sandbox_globals["input_data"] = input_data # ── 4. Execute with stdout/stderr capture ─────────────────── captured_out, captured_err = LimitedStringIO(), LimitedStringIO() diff --git a/src/openstaad_mcp/server.py b/src/openstaad_mcp/server.py index 63e9286..2eb5f5d 100644 --- a/src/openstaad_mcp/server.py +++ b/src/openstaad_mcp/server.py @@ -78,9 +78,8 @@ def _resolve_target(instance: str | None) -> StaadInstance: def discover_api() -> str: """Discover available API guidance and skills. - Call this before using other openstaad-mcp tools to understand the API surface - and see what skills are available. Then use ``read_skills`` with one or more - specific skill names to load full guidance. + Call this FIRST before using other openstaad-mcp tools. + Then use ``read_skills`` with one or more specific skill names to load full guidance. """ return skills_mgr.discover_api() @@ -101,6 +100,11 @@ def read_skills(skills: list[str]) -> str: Pass skill names like ``["staad-analysis"]`` or sub-paths like ``["staad-steel-design/assets/DESIGN_CODES"]`` to read reference files within a skill. + + Parameters + ---------- + skills: list[str] + List of skill names or sub-paths to read. Use ``discover_api`` to see available skills. """ return skills_mgr.read_skills(skills) @@ -121,7 +125,7 @@ def list_instances() -> list[dict[str, Any]]: right one. The ``alias`` (e.g. ``staadPro1``) is stable for the server session even if the model file changes. - If a version is below the minimum supported (26.0.1), a ``warning`` + If a version is below the minimum supported (25.0.1), a ``warning`` field is included with details about potential data inaccuracies. """ results = [] @@ -189,33 +193,38 @@ def _read_status(staad: Any) -> dict[str, Any]: ) ) async def execute_code( - code: str, ctx: Context, + code: str, instance: str | None = None, - input_path: str | None = None, - output_path: str | None = None, + input_data_path: str | None = None, + output_data_path: str | None = None, overwrite: bool = False, ) -> dict[str, Any]: - """Execute Python code in a sandbox against the OpenSTAAD API. + """Execute Python code in a sandbox against the OpenSTAAD API (don't forget to call discover_api and read_skills for API guidance). - The sandbox provides a pre-connected ``staad`` variable (the OpenSTAAD root object) plus ``json`` - and ``math`` modules. Imports and regular filesystem access are blocked for security; all data exchange - happens through `result`, `__input__`, and the file I/O params below. + The sandbox provides pre-connected ``staad`` (the OpenSTAAD root object) and ``input_data`` (if input_data_path is provided) variables (plus ``json`` + and ``math`` modules). `import` statements, `dir()`, `getattr()`, ... are **BLOCKED**. The last expression value or an explicit ``result = ...`` assignment is returned as the result. - - Pass ``instance`` (alias from ``list_instances``, e.g. ``staadPro1``) to target a specific - STAAD instance. Omit it when only one instance is running — it will be selected automatically. - - **File I/O** (optional): - - - ``input_path``: path to a ``.csv`` or ``.xlsx`` file. The server reads the file and injects its contents - as the immutable `__input__` variable inside the sandbox. Use this to feed large datasets - (e.g. node loads, section properties) into your code without hardcoding them. - - - `output_path`: path where the sandbox return value will be written as a file. Use this whenever - the result is tabular data destined for a file (node lists, member forces, design results, etc.) — - it avoids flooding the context window with large arrays. The `result` variable must be formatted as one of: + If ``output_data_path`` is provided, the sandbox will write the result to the specified file. + + Paths must be on the user LOCAL filesystem and inside MCP roots or configured `allowed_dirs`. + On Claude Desktop, users can configure allowed directories in the extension settings and Claude can use the filesystem ``copy_file_to_claude`` + tool to move files to Claude's filesystem. + + Parameters + ---------- + code: str + Python source code to execute. Use the pre-injected ``staad`` variable to interact with the API. + (don't forget to call discover_api and read_skills for API guidance) + instance: str + Alias (from ``list_instances``, e.g. ``staadPro1``) of the STAAD instance to target. If omitted, last opened instance is selected. + input_data_path: str, optional + Path on user LOCAL filesystem to a ``.csv`` or ``.xlsx`` file. Its content is injected as the immutable `input_data` variable inside the sandbox. + Use this to feed large datasets (e.g. node loads, section properties) into your code without hardcoding them. + output_data_path: str, optional + Path on user LOCAL filesystem to a ``.csv`` or ``.xlsx`` file where to write the ``result`` value. + Use this to avoid flooding the context window with large amount of data. The ``result`` variable must be formatted as one of: - List-of-lists → written as CSV or single-sheet xlsx: result = [["Node ID", "X", "Y", "Z"], [1, 0.0, 0.0, 0.0], ...] - Dict of sheet dicts → written as multi-sheet xlsx: @@ -225,13 +234,8 @@ async def execute_code( "Members": {"columns": ["Member ID", "Start", "End"], "rows": [[1, 1, 2], ...]} } - - - ``overwrite``: allow overwriting an existing output file. - - Paths must be inside MCP roots or `allowed_dirs` configured by the client. - On Claude Desktop, users can configure allowed directories in the extension settings. - If no roots are configured, omit both file I/O params and handle the returned - `result` value in the agent instead (e.g. write the file via a separate tool). + overwrite: bool, optional + Allow overwriting an existing output file. """ try: target = _resolve_target(instance) @@ -246,11 +250,11 @@ async def execute_code( } # ── Resolve allowed dirs for path validation ── - allowed_dirs = await get_allowed_dirs(ctx, args_allowed_dirs, input_path, output_path) + allowed_dirs = await get_allowed_dirs(ctx, args_allowed_dirs, input_data_path, output_data_path) # ── Input file handling (server-side, outside sandbox) ─────── try: - input_data, input_summary = await get_input_data(input_path, allowed_dirs) + input_data, _ = await get_input_data(input_data_path, allowed_dirs) except FileIOError as e: return { "success": False, @@ -287,9 +291,11 @@ def _run(staad: Any) -> dict[str, Any]: } # ── Output file handling (server-side, outside sandbox) ────── - if output_path is not None and result.get("success"): + if output_data_path is not None and result.get("success"): try: - result["result"] = write_output_file(output_path, result["result"], allowed_dirs, overwrite=overwrite) + result["result"] = write_output_file( + output_data_path, result["result"], allowed_dirs, overwrite=overwrite + ) except FileIOError as e: return { "success": False, @@ -300,9 +306,6 @@ def _run(staad: Any) -> dict[str, Any]: "duration_seconds": result.get("duration_seconds", 0.0), } - # ── Attach summaries ───────────────────────────────────────── - if input_summary is not None: - result["input_summary"] = input_summary if target.warning: result["warning"] = target.warning return result diff --git a/src/openstaad_mcp/staad_skills/staad-core/SKILL.md b/src/openstaad_mcp/staad_skills/staad-core/SKILL.md index 76b15ef..85aad27 100644 --- a/src/openstaad_mcp/staad_skills/staad-core/SKILL.md +++ b/src/openstaad_mcp/staad_skills/staad-core/SKILL.md @@ -9,10 +9,13 @@ description: "ALWAYS load first for any STAAD.Pro automation. Covers: Python san ### Sandbox -- Pre-injected names (do NOT import): `staad`, `json`, `math` -- `import` statements are **BLOCKED** — only use pre-injected names +- Pre-injected names (do NOT import): `staad`, `input_data`, `json`, `math` +- `import` statements, `dir()`, `getattr()`, ... are **BLOCKED** — use skills for discovery, only use pre-injected names in code - `staad` is already connected and ready — do NOT call any initialization function +- `input_data` is injected if `input_data_path` is provided in `execute_code` params — use it to feed large datasets into the sandbox without hardcoding - Sub-modules: `geo = staad.Geometry`, `prop = staad.Property`, `sup = staad.Support`, `load = staad.Load`, `cmd = staad.Command`, `out = staad.Output`, `design = staad.Design` +- If `output_data_path` is provided, write the `result` variable to that file path instead of returning it in the context (use for large/tabular data). The `execute_code` return value will contain a summary of the `result` content instead (e.g. number of rows, columns and a sample of rows). +- Both `input_data_path` and `output_data_path` must be on the user LOCAL filesystem and inside MCP roots or configured `allowed_dirs`. On Claude Desktop, users can configure allowed directories in the extension settings and Claude can use the filesystem `copy_file_to_claude` tool to move files to Claude's filesystem. ### Discovery @@ -120,7 +123,10 @@ staad.CloseSTAADFile() ## Gotchas -- `import` is blocked — only `staad`, `json`, `math` are available +- `import`, `dir()`, `getattr()`, ... are blocked — only `staad`, `input_data`, `json`, `math` are available +- If `input_data_path` is provided, `input_data` is injected as an immutable variable — use it to feed large datasets into the sandbox without hardcoding +- If `output_data_path` is provided, write the `result` variable to that file path instead of returning it in the context (use for large/tabular data). The `execute_code` return value will contain a summary of the `result` content instead (e.g. number of rows, columns and a sample of rows). +- Both `input_data_path` and `output_data_path` must be on the user LOCAL filesystem and inside MCP roots or configured `allowed_dirs`. On Claude Desktop, users can configure allowed directories in the extension settings and Claude can use the filesystem `copy_file_to_claude` tool to move files to Claude's filesystem. - Use `staad.GetSTAADFile()` to get the current model path after a file switch - Always wrap `UpdateStructure`/`AnalyzeModel`/`AnalyzeEx`/`SaveModel` inside `SetSilentMode(True/False)` - **Never** call `SaveModel` without explicit user instruction diff --git a/tests/sandbox/test_executor.py b/tests/sandbox/test_executor.py index d29576b..f6e2f76 100644 --- a/tests/sandbox/test_executor.py +++ b/tests/sandbox/test_executor.py @@ -671,25 +671,25 @@ class TestInputInjection: """Tests for ``__input`` data injection into the sandbox.""" def test_input_none_when_no_data(self, staad, executor): - """``__input`` is None when no input_data is provided (backward compat).""" - r = executor.execute("result = __input__", staad) + """``input_data`` is None when no input_data is provided (backward compat).""" + r = executor.execute("result = input_data", staad) assert r.success assert r.result is None def test_input_contains_provided_data(self, staad, executor): data = (("a", "b"), (1, 2), (3, 4)) - r = executor.execute("result = [list(row) for row in __input__]", staad, input_data=data) + r = executor.execute("result = [list(row) for row in input_data]", staad, input_data=data) assert r.success assert r.result == [["a", "b"], [1, 2], [3, 4]] def test_input_deeply_immutable(self, staad, executor): - """Sandbox code cannot mutate ``__input__`` (tuples are immutable).""" + """Sandbox code cannot mutate ``input_data`` (tuples are immutable).""" data = (("a", "b"), (1, 2)) r = executor.execute( dedent( """ try: - __input__[0] = "mutated" + input_data[0] = "mutated" result = "mutation succeeded" except TypeError: result = "immutable" @@ -702,10 +702,10 @@ def test_input_deeply_immutable(self, staad, executor): assert r.result == "immutable" def test_input_iterable(self, staad, executor): - """Sandbox code can iterate over ``__input__``.""" + """Sandbox code can iterate over ``input_data``.""" data = ((10,), (20,), (30,)) r = executor.execute( - "result = sum(row[0] for row in __input__)", + "result = sum(row[0] for row in input_data)", staad, input_data=data, ) @@ -713,24 +713,24 @@ def test_input_iterable(self, staad, executor): assert r.result == 60 def test_input_indexable(self, staad, executor): - """Sandbox code can index into ``__input__``.""" + """Sandbox code can index into ``input_data``.""" data = (("x",), (42,)) - r = executor.execute("result = __input__[1][0]", staad, input_data=data) + r = executor.execute("result = input_data[1][0]", staad, input_data=data) assert r.success assert r.result == 42 def test_input_no_carryover(self, staad, executor): - """``__input__`` does not persist across executions.""" - executor.execute("x = __input__", staad, input_data=((1,),)) - r = executor.execute("result = __input__", staad) + """``input_data`` does not persist across executions.""" + executor.execute("x = input_data", staad, input_data=((1,),)) + r = executor.execute("result = input_data", staad) assert r.success assert r.result is None def test_input_dict_shape(self, staad, executor): - """Dict-shaped __input__ (XLSX multi-sheet) works.""" + """Dict-shaped input_data (XLSX multi-sheet) works.""" data = {"Sheet1": {"columns": ("a",), "rows": ((1,), (2,))}} r = executor.execute( - "result = len(__input__['Sheet1']['rows'])", + "result = len(input_data['Sheet1']['rows'])", staad, input_data=data, ) From 526499def0b21e30248e8b6a1ba65924fdb85126 Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Wed, 3 Jun 2026 08:56:06 -0400 Subject: [PATCH 4/6] fix after pr feedback (formula detection + max_output_sheet) --- src/openstaad_mcp/file_io/const.py | 2 +- src/openstaad_mcp/file_io/models.py | 44 ++++++++++- src/openstaad_mcp/file_io/readers.py | 4 +- tests/file_io/test_validation.py | 113 ++++++++++++++++++++++++++- 4 files changed, 158 insertions(+), 5 deletions(-) diff --git a/src/openstaad_mcp/file_io/const.py b/src/openstaad_mcp/file_io/const.py index b5d9e4b..50d6520 100644 --- a/src/openstaad_mcp/file_io/const.py +++ b/src/openstaad_mcp/file_io/const.py @@ -17,7 +17,7 @@ # Row / column / sheet caps for output (return value → file). MAX_OUTPUT_ROWS = 100_000 MAX_OUTPUT_COLUMNS = 500 -MAX_OUTPUT_SHEETS = 20 +MAX_OUTPUT_SHEETS = 50 # Excel's own limit on sheet-tab names. MAX_SHEET_NAME_LENGTH = 31 diff --git a/src/openstaad_mcp/file_io/models.py b/src/openstaad_mcp/file_io/models.py index 04caacb..87972d1 100644 --- a/src/openstaad_mcp/file_io/models.py +++ b/src/openstaad_mcp/file_io/models.py @@ -9,6 +9,7 @@ from __future__ import annotations +import unicodedata from typing import Annotated, Any from pydantic import BaseModel, RootModel, field_validator, model_validator @@ -27,8 +28,47 @@ CellValue = str | int | float | bool | None +# Characters that, when leading a spreadsheet cell, cause the value to be +# interpreted as a formula (CSV/XLSX injection). +_FORMULA_PREFIXES = ("=", "+", "-", "@") -def check_cell(v: Any) -> CellValue: +# Line separators that would split a single cell into multiple logical CSV +# rows. A later segment could itself begin with a formula prefix, so any cell +# containing one of these is rejected outright. +_LINE_SEPARATORS = frozenset("\n\r\x0b\x0c\u2028\u2029\u0085") + + +def _check_formula_injection(v: str) -> None: + """Reject strings that could be interpreted as a spreadsheet formula. + + Defends against three bypasses of a naive ``v.strip()[0] in (...)`` check: + + 1. Fullwidth / lookalike Unicode (e.g. U+FF1D fullwidth equals) that + NFKC-normalises to a formula character. We normalise first, then inspect. + 2. Zero-width / format characters (Unicode category ``Cf``) that survive + ``str.strip()`` and hide the real leading formula character. + 3. Embedded newlines that split the cell into extra CSV rows. + """ + + normalized = unicodedata.normalize("NFKC", v) + + if any(ch in _LINE_SEPARATORS for ch in normalized): + raise ValueError("Cell values cannot contain line separators to prevent CSV row splitting") + + # Drop leading whitespace and zero-width/format characters so the first + # *visible* character is the one that gets checked. + idx = 0 + for ch in normalized: + if ch.isspace() or unicodedata.category(ch) == "Cf": + idx += 1 + else: + break + + if normalized[idx : idx + 1] in _FORMULA_PREFIXES: + raise ValueError("Cell values cannot start with '=', '+', '-', or '@' to prevent formula injection") + + +def check_cell(v: Any, reject_formula: bool = True) -> CellValue: """Validate a single cell value: must be a JSON primitive with bounded string length.""" if isinstance(v, (bool, int, float)): @@ -36,6 +76,8 @@ def check_cell(v: Any) -> CellValue: if isinstance(v, str): if len(v) > MAX_CELL_SIZE: raise ValueError(f"String too long: {len(v)}; limit {MAX_CELL_SIZE}") + if reject_formula: + _check_formula_injection(v) return v if v is None: return v diff --git a/src/openstaad_mcp/file_io/readers.py b/src/openstaad_mcp/file_io/readers.py index dec1cef..abeea08 100644 --- a/src/openstaad_mcp/file_io/readers.py +++ b/src/openstaad_mcp/file_io/readers.py @@ -156,7 +156,7 @@ def read( ) for cell in coerced: try: - check_cell(cell) + check_cell(cell, reject_formula=False) # CSV reader doesn't need formula-injection checks except ValueError as exc: raise FileIOError("INVALID_CELL", f"Invalid cell value: {exc}") from exc all_rows.append(coerced) @@ -329,7 +329,7 @@ def _read_sheet( converted = [_to_json_primitive(c) for c in row] for cell in converted: try: - check_cell(cell) + check_cell(cell, reject_formula=False) # XLSX reader doesn't need formula-injection checks except ValueError as exc: raise FileIOError("INVALID_CELL", f"Invalid cell value: {exc}") from exc all_rows.append(converted) diff --git a/tests/file_io/test_validation.py b/tests/file_io/test_validation.py index 83e7f8b..c0fdcb8 100644 --- a/tests/file_io/test_validation.py +++ b/tests/file_io/test_validation.py @@ -32,7 +32,7 @@ def test_rejects_non_primitive_leaf(self): assert exc_info.value.code == "INVALID_RETURN_SHAPE" def test_rejects_too_many_sheets(self): - data = {f"S{i}": {"columns": ["a"], "rows": [[1]]} for i in range(21)} + data = {f"S{i}": {"columns": ["a"], "rows": [[1]]} for i in range(51)} with pytest.raises(FileIOError) as exc_info: validate_return_value(Path("test.xlsx"), data) assert exc_info.value.code == "INVALID_RETURN_SHAPE" @@ -78,6 +78,117 @@ def test_accepts_tuples_as_rows(self): """Tuples (from deep_freeze) should be accepted as rows.""" validate_return_value(Path("test.csv"), (("a", "b"), (1, 2))) + @pytest.mark.parametrize("cell", ["=SUM(A1)", "+cmd|'/C calc'!A0", "-2+3", "@SUM(1,2)"]) + def test_rejects_formula_injection_in_flat(self, cell: str): + """Cell values starting with formula-injection characters are rejected.""" + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.csv"), [["header"], [cell]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize("cell", ["=A1", "+1", "-1", "@test"]) + def test_rejects_formula_injection_in_multi_sheet(self, cell: str): + """Formula injection is rejected in multi-sheet output cells.""" + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.xlsx"), {"S1": {"columns": ["a"], "rows": [[cell]]}}) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize("cell", ["=SUM(A1)", "+cmd", "-2", "@foo"]) + def test_rejects_formula_injection_in_column_headers(self, cell: str): + """Formula injection is also rejected in column header cells.""" + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.xlsx"), {"S1": {"columns": [cell], "rows": []}}) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize("cell", [" =not-injected", "\t+ok"]) + def test_rejects_formula_injection_after_stripping_whitespace(self, cell: str): + """Leading whitespace is stripped before the formula-char check, so these are also rejected.""" + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.csv"), [["header"], [cell]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize("cell", ["hello=world", "1+1", "text-here", "price: -5", "note@domain.com"]) + def test_accepts_strings_not_starting_with_formula_chars(self, cell: str): + """Strings where formula chars appear only in the middle are accepted.""" + validate_return_value(Path("test.csv"), [["header"], [cell]]) + + # ----------------------------------------------------------------------- + # Formula-injection bypass tests — prove the intern's check is incomplete + # ----------------------------------------------------------------------- + + @pytest.mark.parametrize( + "cell", + [ + "＝SUM(A1)", # U+FF1D FULLWIDTH EQUALS SIGN — NFKC-normalises to '=' + "＋1+2", # U+FF0B FULLWIDTH PLUS SIGN + "－1+2", # U+FF0D FULLWIDTH HYPHEN-MINUS + "＠SUM(1)", # U+FF20 FULLWIDTH COMMERCIAL AT + ], + ) + def test_bypass_fullwidth_unicode_lookalikes(self, cell: str): + """Fullwidth Unicode lookalikes bypass the ASCII-only set check. + + The guard checks against the ASCII characters '=', '+', '-', '@'. + Fullwidth variants (U+FF0B, U+FF0D, U+FF1D, U+FF20) are distinct + code points and are not in that set, so the check silently passes them + through. Yet many spreadsheet applications apply NFKC Unicode + normalisation when loading a CSV/XLSX file, which maps every fullwidth + character back to its ASCII equivalent and then evaluates the result as + a formula. + """ + # Should raise ValueError / INVALID_RETURN_SHAPE — currently does NOT. + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.csv"), [["header"], [cell]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize( + "cell", + [ + "\u200b=SUM(1)", # U+200B ZERO WIDTH SPACE (category Cf, not stripped by str.strip()) + "\u200c+cmd", # U+200C ZERO WIDTH NON-JOINER + "\u200d-1", # U+200D ZERO WIDTH JOINER + "\u2060=A1", # U+2060 WORD JOINER + ], + ) + def test_bypass_zero_width_character_prefix(self, cell: str): + """Zero-width characters prefix the injection character and evade the guard. + + Python's str.strip() only removes characters where str.isspace() is + True. Zero-width format characters (Unicode category Cf) do *not* + satisfy isspace(), so they survive the strip() call. After stripping, + the first character is the zero-width character, which is not in + ('=', '+', '-', '@'), and the check silently passes. + + Some spreadsheet applications ignore invisible characters when parsing + cell content, effectively treating '\\u200b=SUM(1)' as '=SUM(1)'. + """ + # Should raise ValueError / INVALID_RETURN_SHAPE — currently does NOT. + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.csv"), [["header"], [cell]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + + @pytest.mark.parametrize( + "cell", + [ + 'safe text\n=HYPERLINK("http://evil.com","click")', + "normal value\r\n=cmd|'/c calc'!A0", + "header\r=SUM(1+1)", + ], + ) + def test_bypass_embedded_newline_row_splitting(self, cell: str): + """Embedded newlines bypass the first-character check and split CSV rows. + + The guard inspects v.strip()[0:1] of the *entire* cell value. A cell + whose content begins with benign text but contains an embedded newline + starts with a safe character and is accepted. When this value is then + written to a CSV file, the newline breaks the logical row: the text + after '\\n' becomes the first token of a new CSV row. If that token + starts with '=' the spreadsheet application evaluates it as a formula. + """ + # Should raise ValueError / INVALID_RETURN_SHAPE — currently does NOT. + with pytest.raises(FileIOError) as exc_info: + validate_return_value(Path("test.csv"), [["header"], [cell]]) + assert exc_info.value.code == "INVALID_RETURN_SHAPE" + def test_accepts_tuples_in_multi_sheet(self): """Frozen multi-sheet data should be accepted.""" validate_return_value( From 69fbd626fa0e14bd3da50fc7c9a8bc7a382280fc Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Wed, 3 Jun 2026 14:57:29 -0400 Subject: [PATCH 5/6] add input_output_collision detection --- src/openstaad_mcp/file_io/helpers.py | 14 ++++++++++++++ src/openstaad_mcp/server.py | 8 +++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/openstaad_mcp/file_io/helpers.py b/src/openstaad_mcp/file_io/helpers.py index b5a3cfd..c852dd7 100644 --- a/src/openstaad_mcp/file_io/helpers.py +++ b/src/openstaad_mcp/file_io/helpers.py @@ -105,3 +105,17 @@ async def get_input_data(input_path: str | None, allowed_dirs: list[Path]) -> tu resolved_input = validate_io_path(input_path, allowed_dirs, mode="read") data, input_summary = read_input_file(resolved_input) return deep_freeze(data), input_summary + + +async def detect_input_output_collision( + input_path: str | None, output_path: str | None, allowed_dirs: list[Path] +) -> None: + """Detect if input and output paths resolve to the same file, which would cause a collision.""" + if input_path is None or output_path is None: + return + resolved_input = validate_io_path(input_path, allowed_dirs, mode="read") + resolved_output = validate_io_path(output_path, allowed_dirs, mode="write") + if resolved_input == resolved_output: + raise FileIOError( + "INPUT_OUTPUT_COLLISION", "Input and output paths resolve to the same file, which is not allowed" + ) diff --git a/src/openstaad_mcp/server.py b/src/openstaad_mcp/server.py index 2eb5f5d..9a370ae 100644 --- a/src/openstaad_mcp/server.py +++ b/src/openstaad_mcp/server.py @@ -26,7 +26,12 @@ from mcp.types import ToolAnnotations from openstaad_mcp.connection import InstanceRegistry, StaadInstance, connect_and_run -from openstaad_mcp.file_io import get_allowed_dirs, get_input_data, write_output_file +from openstaad_mcp.file_io.helpers import ( + detect_input_output_collision, + get_allowed_dirs, + get_input_data, + write_output_file, +) from openstaad_mcp.file_io.path_validator import FileIOError from openstaad_mcp.sandbox.executor import Executor from openstaad_mcp.skills import SkillsManager @@ -254,6 +259,7 @@ async def execute_code( # ── Input file handling (server-side, outside sandbox) ─────── try: + await detect_input_output_collision(input_data_path, output_data_path, allowed_dirs) input_data, _ = await get_input_data(input_data_path, allowed_dirs) except FileIOError as e: return { From 6e6918281c3c2ec526f92ecbf1f63d91fbc6d724 Mon Sep 17 00:00:00 2001 From: Silvestre Perret Date: Wed, 3 Jun 2026 15:08:47 -0400 Subject: [PATCH 6/6] fix tests and linting --- tests/file_io/test_readers.py | 2 +- tests/file_io/test_validation.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/file_io/test_readers.py b/tests/file_io/test_readers.py index db24ea5..e3b2285 100644 --- a/tests/file_io/test_readers.py +++ b/tests/file_io/test_readers.py @@ -237,7 +237,7 @@ def test_oversized_cell_xlsx(self, tmp_path: Path, monkeypatch): """XLSX cell exceeding MAX_CELL_SIZE is rejected on read.""" # openpyxl truncates strings to 32767 chars, so lower the limit - def check_cell_override(cell): + def check_cell_override(cell, reject_formula: bool = True) -> None: if isinstance(cell, str) and len(cell) > 100: raise ValueError("Cell too large") diff --git a/tests/file_io/test_validation.py b/tests/file_io/test_validation.py index c0fdcb8..c05e98d 100644 --- a/tests/file_io/test_validation.py +++ b/tests/file_io/test_validation.py @@ -118,10 +118,10 @@ def test_accepts_strings_not_starting_with_formula_chars(self, cell: str): @pytest.mark.parametrize( "cell", [ - "＝SUM(A1)", # U+FF1D FULLWIDTH EQUALS SIGN — NFKC-normalises to '=' - "＋1+2", # U+FF0B FULLWIDTH PLUS SIGN - "－1+2", # U+FF0D FULLWIDTH HYPHEN-MINUS - "＠SUM(1)", # U+FF20 FULLWIDTH COMMERCIAL AT + "＝SUM(A1)", # U+FF1D FULLWIDTH EQUALS SIGN — NFKC-normalises to '=' # noqa: RUF001 + "＋1+2", # U+FF0B FULLWIDTH PLUS SIGN # noqa: RUF001 + "－1+2", # U+FF0D FULLWIDTH HYPHEN-MINUS # noqa: RUF001 + "＠SUM(1)", # U+FF20 FULLWIDTH COMMERCIAL AT # noqa: RUF001 ], ) def test_bypass_fullwidth_unicode_lookalikes(self, cell: str):