From abd7b21dd77fbb43efb7249665fb1c194ebc4d12 Mon Sep 17 00:00:00 2001 From: Ming-Jer Lee Date: Mon, 20 Jul 2026 15:21:20 -0700 Subject: [PATCH 1/4] feat: validate paths in from_json_file (Item 7) --- src/clgraph/pipeline.py | 13 +++++++-- src/clgraph/pipeline_factory.py | 21 +++++++++++---- tests/test_path_validation_integration.py | 32 +++++++++++++++++++++++ 3 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 tests/test_path_validation_integration.py diff --git a/src/clgraph/pipeline.py b/src/clgraph/pipeline.py index 7f25a85..a56b9e8 100644 --- a/src/clgraph/pipeline.py +++ b/src/clgraph/pipeline.py @@ -494,13 +494,20 @@ def from_json( return create_from_json(data, apply_metadata=apply_metadata) @classmethod - def from_json_file(cls, file_path: str, apply_metadata: bool = True) -> "Pipeline": + def from_json_file( + cls, + file_path: str, + apply_metadata: bool = True, + *, + allow_symlinks: bool = False, + ) -> "Pipeline": """ Create pipeline from JSON file exported by JSONExporter. Args: file_path: Path to JSON file apply_metadata: Whether to apply metadata from the JSON + allow_symlinks: If True, follow symbolic links (logs a security warning). Returns: Pipeline instance @@ -514,7 +521,9 @@ def from_json_file(cls, file_path: str, apply_metadata: bool = True) -> "Pipelin """ from .pipeline_factory import create_from_json_file - return create_from_json_file(file_path, apply_metadata=apply_metadata) + return create_from_json_file( + file_path, apply_metadata=apply_metadata, allow_symlinks=allow_symlinks + ) @classmethod def _create_empty(cls, table_graph: "TableDependencyGraph") -> "Pipeline": diff --git a/src/clgraph/pipeline_factory.py b/src/clgraph/pipeline_factory.py index 5993c73..b57133c 100644 --- a/src/clgraph/pipeline_factory.py +++ b/src/clgraph/pipeline_factory.py @@ -276,6 +276,7 @@ def create_from_json( def create_from_json_file( file_path: str, apply_metadata: bool = True, + allow_symlinks: bool = False, ) -> "Pipeline": """ Create Pipeline from JSON file exported by JSONExporter. @@ -283,18 +284,28 @@ def create_from_json_file( Args: file_path: Path to JSON file apply_metadata: Whether to apply metadata from the JSON + allow_symlinks: If True, follow symbolic links (logs a security warning). Returns: Pipeline instance """ import json - from pathlib import Path + import logging + + from .path_validation import PathValidator + + if allow_symlinks: + logging.getLogger(__name__).warning( + "SECURITY: allow_symlinks=True enables following symbolic links. " + "This may expose sensitive files outside the intended location." + ) - path = Path(file_path) - if not path.exists(): - raise FileNotFoundError(f"JSON file not found: {file_path}") + validator = PathValidator() + resolved = validator.validate_file( + file_path, allowed_extensions=[".json"], allow_symlinks=allow_symlinks + ) - with open(path) as f: + with open(resolved) as f: data = json.load(f) return create_from_json(data, apply_metadata=apply_metadata) diff --git a/tests/test_path_validation_integration.py b/tests/test_path_validation_integration.py new file mode 100644 index 0000000..ba72066 --- /dev/null +++ b/tests/test_path_validation_integration.py @@ -0,0 +1,32 @@ +"""Integration tests: path validation reached through the public Pipeline API. + +These exercise the factory entry points a caller actually uses. The unit tests +in test_path_validation.py cover PathValidator internals directly. +""" + +import json +from pathlib import Path + +import pytest + +from clgraph import Pipeline + + +class TestFromJsonFilePathValidation: + def test_non_json_extension_rejected(self, tmp_path: Path): + bad = tmp_path / "data.txt" + bad.write_text("{}") + with pytest.raises(ValueError): + Pipeline.from_json_file(str(bad)) + + def test_symlink_rejected_by_default(self, tmp_path: Path): + real = tmp_path / "real.json" + real.write_text(json.dumps({"columns": [], "edges": []})) + link = tmp_path / "link.json" + link.symlink_to(real) + with pytest.raises(ValueError): + Pipeline.from_json_file(str(link)) + + def test_missing_file_raises_filenotfound(self, tmp_path: Path): + with pytest.raises(FileNotFoundError): + Pipeline.from_json_file(str(tmp_path / "nope.json")) From f5ef66a436132a71770b83f149cb2e82da8b83b5 Mon Sep 17 00:00:00 2001 From: Ming-Jer Lee Date: Mon, 20 Jul 2026 15:26:06 -0700 Subject: [PATCH 2/4] test: tie path-validation tests to validator error messages --- tests/test_path_validation_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_path_validation_integration.py b/tests/test_path_validation_integration.py index ba72066..aab3894 100644 --- a/tests/test_path_validation_integration.py +++ b/tests/test_path_validation_integration.py @@ -16,7 +16,7 @@ class TestFromJsonFilePathValidation: def test_non_json_extension_rejected(self, tmp_path: Path): bad = tmp_path / "data.txt" bad.write_text("{}") - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="Invalid file extension"): Pipeline.from_json_file(str(bad)) def test_symlink_rejected_by_default(self, tmp_path: Path): @@ -24,7 +24,7 @@ def test_symlink_rejected_by_default(self, tmp_path: Path): real.write_text(json.dumps({"columns": [], "edges": []})) link = tmp_path / "link.json" link.symlink_to(real) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="Symbolic links are not allowed"): Pipeline.from_json_file(str(link)) def test_missing_file_raises_filenotfound(self, tmp_path: Path): From a8d4f7ba5f0d347f7aa673303b07bc35478cf5f7 Mon Sep 17 00:00:00 2001 From: Ming-Jer Lee Date: Mon, 20 Jul 2026 15:29:38 -0700 Subject: [PATCH 3/4] feat: validate paths and read TOCTOU-safe in from_sql_files (Item 7) --- src/clgraph/pipeline.py | 4 +++ src/clgraph/pipeline_factory.py | 24 +++++++++++++--- tests/test_path_validation_integration.py | 35 +++++++++++++++++++++++ tests/test_pipeline_factories.py | 7 +++-- 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/clgraph/pipeline.py b/src/clgraph/pipeline.py index a56b9e8..b094a5b 100644 --- a/src/clgraph/pipeline.py +++ b/src/clgraph/pipeline.py @@ -890,6 +890,8 @@ def from_sql_files( pattern: str = "*.sql", query_id_from: str = "filename", template_context: Optional[Dict[str, Any]] = None, + *, + allow_symlinks: bool = False, ) -> "Pipeline": """ Create pipeline from SQL files in a directory. @@ -902,6 +904,7 @@ def from_sql_files( - "filename": Use filename without extension (default) - "comment": Extract from first line comment (-- query_id: name) template_context: Optional dictionary of template variables + allow_symlinks: If True, follow symbolic links (logs a security warning). Returns: Pipeline instance @@ -930,6 +933,7 @@ def from_sql_files( pattern=pattern, query_id_from=query_id_from, template_context=template_context, + allow_symlinks=allow_symlinks, ) @classmethod diff --git a/src/clgraph/pipeline_factory.py b/src/clgraph/pipeline_factory.py index b57133c..a71cfeb 100644 --- a/src/clgraph/pipeline_factory.py +++ b/src/clgraph/pipeline_factory.py @@ -317,6 +317,7 @@ def create_from_sql_files( pattern: str = "*.sql", query_id_from: str = "filename", template_context: Optional[Dict[str, Any]] = None, + allow_symlinks: bool = False, ) -> "Pipeline": """ Create Pipeline from SQL files in a directory. @@ -329,22 +330,37 @@ def create_from_sql_files( - "filename": Use filename without extension (default) - "comment": Extract from first line comment (-- query_id: name) template_context: Optional dictionary of template variables + allow_symlinks: If True, follow symbolic links (logs a security warning). Returns: Pipeline instance """ import re - from pathlib import Path - sql_path = Path(sql_dir) - sql_files = sorted(sql_path.glob(pattern)) + from .path_validation import PathValidator, _safe_read_sql_file + + if allow_symlinks: + logging.getLogger(__name__).warning( + "SECURITY: allow_symlinks=True enables following symbolic links. " + "This may expose sensitive files outside the SQL directory." + ) + + validator = PathValidator() + resolved_dir = validator.validate_directory(sql_dir, allow_symlinks=allow_symlinks) + safe_pattern = validator.validate_glob_pattern(pattern, allowed_extensions=[".sql"]) + + sql_files = sorted(resolved_dir.glob(safe_pattern)) if not sql_files: raise ValueError(f"No SQL files found in {sql_dir} matching {pattern}") queries = [] for sql_file in sql_files: - sql_content = sql_file.read_text() + # Validate and read atomically to prevent TOCTOU (a validated file being + # swapped for a symlink before the read). + sql_content = _safe_read_sql_file( + sql_file, base_dir=resolved_dir, allow_symlinks=allow_symlinks + ) if query_id_from == "filename": query_id = sql_file.stem diff --git a/tests/test_path_validation_integration.py b/tests/test_path_validation_integration.py index aab3894..cff318f 100644 --- a/tests/test_path_validation_integration.py +++ b/tests/test_path_validation_integration.py @@ -30,3 +30,38 @@ def test_symlink_rejected_by_default(self, tmp_path: Path): def test_missing_file_raises_filenotfound(self, tmp_path: Path): with pytest.raises(FileNotFoundError): Pipeline.from_json_file(str(tmp_path / "nope.json")) + + +class TestFromSqlFilesPathValidation: + def _write_sql(self, d: Path, name: str = "q.sql") -> None: + (d / name).write_text("SELECT 1 AS a") + + def test_valid_directory_loads(self, tmp_path: Path): + self._write_sql(tmp_path) + pipeline = Pipeline.from_sql_files(str(tmp_path), dialect="bigquery") + assert pipeline is not None + + def test_traversal_pattern_rejected(self, tmp_path: Path): + self._write_sql(tmp_path) + with pytest.raises( + ValueError, match="Glob pattern must not contain directory traversal components" + ): + Pipeline.from_sql_files(str(tmp_path), pattern="../*.sql") + + def test_symlinked_dir_rejected_by_default(self, tmp_path: Path): + real_dir = tmp_path / "real" + real_dir.mkdir() + self._write_sql(real_dir) + link_dir = tmp_path / "link" + link_dir.symlink_to(real_dir, target_is_directory=True) + with pytest.raises(ValueError, match="Symbolic links are not allowed"): + Pipeline.from_sql_files(str(link_dir)) + + def test_symlinked_dir_allowed_with_optin(self, tmp_path: Path): + real_dir = tmp_path / "real" + real_dir.mkdir() + self._write_sql(real_dir) + link_dir = tmp_path / "link" + link_dir.symlink_to(real_dir, target_is_directory=True) + pipeline = Pipeline.from_sql_files(str(link_dir), allow_symlinks=True) + assert pipeline is not None diff --git a/tests/test_pipeline_factories.py b/tests/test_pipeline_factories.py index 82d4804..2cb7032 100644 --- a/tests/test_pipeline_factories.py +++ b/tests/test_pipeline_factories.py @@ -215,9 +215,10 @@ def test_custom_pattern(self, tmp_path): pipeline = Pipeline.from_sql_files(str(tmp_path), pattern="*.sql") assert len(pipeline.table_graph.queries) == 1 - # Load .txt files - pipeline = Pipeline.from_sql_files(str(tmp_path), pattern="*.txt") - assert len(pipeline.table_graph.queries) == 1 + # Patterns for non-.sql extensions are rejected by the path validator: + # from_sql_files is restricted to .sql files by design. + with pytest.raises(ValueError, match="Invalid extension in pattern"): + Pipeline.from_sql_files(str(tmp_path), pattern="*.txt") class TestQueryIdGeneration: From e8e685cdc0ec387dd27c0fb8779dca901010ee82 Mon Sep 17 00:00:00 2001 From: Ming-Jer Lee Date: Mon, 20 Jul 2026 15:34:20 -0700 Subject: [PATCH 4/4] chore: changelog and version bump for path validation (Item 7) --- CHANGELOG.md | 14 ++++++++++++++ pyproject.toml | 2 +- uv.lock | 4 ++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 776e047..8c051f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Security + +- `Pipeline.from_sql_files()` and `Pipeline.from_json_file()` now validate paths: + directory traversal, disallowed extensions, and symbolic links are rejected. + +### Changed + +- **BREAKING:** `Pipeline.from_sql_files()` and `Pipeline.from_json_file()` reject + symlinked paths, glob patterns that escape the directory, and files whose + extension is not `.sql`/`.json`. Pass `allow_symlinks=True` to opt back into + following symbolic links. + ## [0.0.3] - 2025-12-29 ### Added diff --git a/pyproject.toml b/pyproject.toml index 0cd7446..c53e7d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "clgraph" -version = "0.0.3" +version = "0.0.4" description = "Column lineage and pipeline dependency analysis for SQL" readme = "README.md" requires-python = ">=3.10" diff --git a/uv.lock b/uv.lock index 9952ffd..c971624 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.10" resolution-markers = [ "python_full_version >= '3.14'", @@ -811,7 +811,7 @@ wheels = [ [[package]] name = "clgraph" -version = "0.0.3" +version = "0.0.4" source = { editable = "." } dependencies = [ { name = "graphviz" },