diff --git a/README.md b/README.md index ac7e350..3a4482b 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,13 @@ subprojects: - vendor/service-b/docs-project.yaml ``` +Configured paths are contained to the directory that owns each +`docs-project.yaml` by default. If a docs project lives in `./docs`, its +`docs_roots`, folders, indexes, and subproject references cannot escape +`./docs` with `../` paths. Use `subprojects` from a parent config to include +other docs roots, or set `security.allow_external_paths: true` only for a +trusted legacy layout that intentionally crosses that boundary. + Generated projects include `docs-project.schema.json` next to `docs-project.yaml` for editor validation and prompt-based config authoring. The same schema is published at diff --git a/docuchango/cli.py b/docuchango/cli.py index adb76db..f852740 100644 --- a/docuchango/cli.py +++ b/docuchango/cli.py @@ -7,6 +7,7 @@ from __future__ import annotations import sys +from collections import deque from pathlib import Path import click @@ -14,6 +15,7 @@ from rich.console import Console from docuchango import __version__ +from docuchango.config_paths import resolve_config_path from docuchango.schemas import DocsProjectConfig console = Console() @@ -21,7 +23,11 @@ def _load_docs_project_config(root: Path) -> tuple[DocsProjectConfig | None, Path | None]: """Load docs-project.yaml from repo root or docs-cms.""" - candidates = [root / "docs-project.yaml", root / "docs-cms" / "docs-project.yaml"] + candidates = [ + root / "docs-project.yaml", + root / "docs-cms" / "docs-project.yaml", + root / "docs" / "docs-project.yaml", + ] return _load_docs_project_config_from_candidates(candidates) @@ -47,13 +53,16 @@ def _iter_docs_project_configs(root: Path) -> list[tuple[DocsProjectConfig, Path configs = [(config, config_path)] seen = {config_path.resolve()} - pending = [(config, config_path)] + pending = deque([(config, config_path)]) while pending: - parent_config, parent_path = pending.pop(0) + parent_config, parent_path = pending.popleft() parent_base = parent_path.parent + allow_external_paths = parent_config.security.allow_external_paths for subproject in parent_config.subprojects: - sub_path = (parent_base / subproject.path).resolve() + sub_path = resolve_config_path(parent_base, subproject.path, parent_base, allow_external_paths) + if not sub_path: + continue if sub_path.is_dir(): sub_path = sub_path / "docs-project.yaml" if sub_path in seen: @@ -82,14 +91,19 @@ def _discover_doc_files(root: Path) -> list[Path]: if not config.structure: continue config_base = config_path.parent + allow_external_paths = config.security.allow_external_paths if config.structure.doc_types: roots = config.structure.docs_roots or ["."] for doc_type_cfg in config.structure.doc_types.values(): for root_rel in roots: - root_path = (config_base / root_rel).resolve() + root_path = resolve_config_path(config_base, root_rel, config_base, allow_external_paths) + if not root_path: + continue for folder in doc_type_cfg.folders: - folder_path = (root_path / folder).resolve() + folder_path = resolve_config_path(root_path, folder, root_path, allow_external_paths) + if not folder_path: + continue if not folder_path.exists(): continue for file_path in folder_path.rglob("*.md"): @@ -98,7 +112,9 @@ def _discover_doc_files(root: Path) -> list[Path]: all_files.append(file_path) else: for folder in config.structure.document_folders: - folder_path = (config_base / folder).resolve() + folder_path = resolve_config_path(config_base, folder, config_base, allow_external_paths) + if not folder_path: + continue if not folder_path.exists(): continue for file_path in folder_path.rglob("*.md"): @@ -673,15 +689,24 @@ def bulk_update( console.print("[red]Error: --set requires FIELD=VALUE format[/red]") sys.exit(1) field_name, value = set_field.split("=", 1) + if not field_name: + console.print("[red]Error: --set requires a non-empty field name[/red]") + sys.exit(1) operation = "set" elif add_field: if "=" not in add_field: console.print("[red]Error: --add requires FIELD=VALUE format[/red]") sys.exit(1) field_name, value = add_field.split("=", 1) + if not field_name: + console.print("[red]Error: --add requires a non-empty field name[/red]") + sys.exit(1) operation = "add" elif remove_field: field_name = remove_field + if not field_name: + console.print("[red]Error: --remove requires a non-empty field name[/red]") + sys.exit(1) value = None operation = "remove" elif rename_field: @@ -689,6 +714,9 @@ def bulk_update( console.print("[red]Error: --rename requires OLD=NEW format[/red]") sys.exit(1) field_name, value = rename_field.split("=", 1) + if not field_name or not value: + console.print("[red]Error: --rename requires non-empty OLD and NEW field names[/red]") + sys.exit(1) operation = "rename" # Find files to process diff --git a/docuchango/config_paths.py b/docuchango/config_paths.py new file mode 100644 index 0000000..17838df --- /dev/null +++ b/docuchango/config_paths.py @@ -0,0 +1,24 @@ +"""Path containment helpers for docs-project.yaml configuration.""" + +from __future__ import annotations + +from pathlib import Path + + +def is_within_path(path: Path, boundary: Path) -> bool: + """Return true when path resolves inside boundary or is boundary itself.""" + resolved_path = path.resolve() + resolved_boundary = boundary.resolve() + try: + resolved_path.relative_to(resolved_boundary) + return True + except ValueError: + return False + + +def resolve_config_path(base_dir: Path, path_value: str, boundary: Path, allow_external_paths: bool) -> Path | None: + """Resolve a config-relative path, enforcing containment unless explicitly allowed.""" + resolved = (base_dir / path_value).resolve() + if allow_external_paths or is_within_path(resolved, boundary): + return resolved + return None diff --git a/docuchango/fixes/bulk_update.py b/docuchango/fixes/bulk_update.py index 7683d4a..6c36612 100644 --- a/docuchango/fixes/bulk_update.py +++ b/docuchango/fixes/bulk_update.py @@ -37,7 +37,7 @@ def should_skip_file(file_path: Path) -> bool: return file_path.name == "index.md" -def serialize_frontmatter(metadata: dict) -> str: +def serialize_frontmatter(metadata: dict[str, object]) -> str: """Serialize frontmatter dict back to YAML with proper formatting. Args: @@ -76,6 +76,9 @@ def update_frontmatter_bulk( # Validate operation if operation not in VALID_OPERATIONS: raise ValueError(f"Invalid operation '{operation}'. Must be one of: {', '.join(VALID_OPERATIONS)}") + rename_target = new_value + if operation == "rename" and not rename_target: + raise ValueError("Rename operation requires a non-empty new field name") try: post = frontmatter.loads(content) @@ -128,9 +131,11 @@ def update_frontmatter_bulk( elif operation == "rename": # Rename field (field_name is old_name, new_value is new_name) if field_name in post.metadata: - post.metadata[new_value] = post.metadata.pop(field_name) + if rename_target is None: + raise ValueError("Rename operation requires a non-empty new field name") + post.metadata[rename_target] = post.metadata.pop(field_name) modified = True - message = f"Renamed {field_name} → {new_value}" + message = f"Renamed {field_name} → {rename_target}" else: message = f"Field {field_name} not found" diff --git a/docuchango/readability.py b/docuchango/readability.py index 0e60ace..d4b9485 100644 --- a/docuchango/readability.py +++ b/docuchango/readability.py @@ -169,6 +169,22 @@ def extract_paragraphs(self, markdown_content: str) -> list[tuple[str, int]]: in_html_comment = False frontmatter_count = 0 + def flush_current_paragraph() -> None: + nonlocal current_paragraph + if not current_paragraph: + return + para_text = " ".join(current_paragraph).strip() + if len(para_text) >= self.config.min_paragraph_length: + paragraphs.append((para_text, current_paragraph_start_line)) + current_paragraph = [] + + def is_void_html_tag_line(stripped_line: str) -> bool: + lower_line = stripped_line.lower() + return any( + lower_line == f"<{tag}>" or lower_line.startswith(f"<{tag} ") + for tag in ("br", "hr", "img", "input", "meta", "link") + ) + for i, line in enumerate(lines, start=1): stripped = line.strip() @@ -185,12 +201,8 @@ def extract_paragraphs(self, markdown_content: str) -> list[tuple[str, int]]: # Track code blocks if stripped.startswith("```"): in_code_block = not in_code_block - # Flush current paragraph when entering code block - if in_code_block and current_paragraph: - para_text = " ".join(current_paragraph).strip() - if len(para_text) >= self.config.min_paragraph_length: - paragraphs.append((para_text, current_paragraph_start_line)) - current_paragraph = [] + if in_code_block: + flush_current_paragraph() continue if in_code_block: @@ -199,38 +211,28 @@ def extract_paragraphs(self, markdown_content: str) -> list[tuple[str, int]]: # Track HTML comments (multi-line support) if "" in stripped: in_html_comment = False continue - # Track HTML/MDX blocks (opening and closing tags) - if stripped.startswith("<"): - # Flush current paragraph when entering HTML block - if current_paragraph: - para_text = " ".join(current_paragraph).strip() - if len(para_text) >= self.config.min_paragraph_length: - paragraphs.append((para_text, current_paragraph_start_line)) - current_paragraph = [] - - # Check if this is a self-closing tag or opening tag - if not stripped.endswith("/>"): - in_html_block = True - continue - # Check if we're closing an HTML block if in_html_block: if stripped.startswith(""): in_html_block = False continue + # Track HTML/MDX blocks (opening and closing tags) + if stripped.startswith("<"): + flush_current_paragraph() + + is_single_line_tag = stripped.endswith("/>") or " list[tuple[str, int]]: or stripped.startswith(">") # blockquotes or not stripped ): - # Flush current paragraph - if current_paragraph: - para_text = " ".join(current_paragraph).strip() - if len(para_text) >= self.config.min_paragraph_length: - paragraphs.append((para_text, current_paragraph_start_line)) - current_paragraph = [] + flush_current_paragraph() continue # Start new paragraph if this is the first line @@ -256,10 +253,7 @@ def extract_paragraphs(self, markdown_content: str) -> list[tuple[str, int]]: current_paragraph.append(stripped) # Flush final paragraph - if current_paragraph: - para_text = " ".join(current_paragraph).strip() - if len(para_text) >= self.config.min_paragraph_length: - paragraphs.append((para_text, current_paragraph_start_line)) + flush_current_paragraph() return paragraphs diff --git a/docuchango/schemas.py b/docuchango/schemas.py index bb3f1f3..90d4c02 100644 --- a/docuchango/schemas.py +++ b/docuchango/schemas.py @@ -220,6 +220,18 @@ def to_readability_config(self): # type: ignore[no-untyped-def] ) +class DocsProjectSecurity(BaseModel): + """Security controls for docs-project.yaml path resolution.""" + + allow_external_paths: bool = Field( + default=False, + description=( + "Allow configured document paths to resolve outside this docs-project.yaml directory. " + "Keep false for normal projects; use subprojects to include other documentation roots." + ), + ) + + class DocsProjectIndexTimeBucket(BaseModel): """Time or milestone bucket rules for a document index.""" @@ -328,6 +340,10 @@ class DocsProjectConfig(BaseModel): default_factory=DocsProjectReadability, description="Readability analysis configuration and thresholds", ) + security: DocsProjectSecurity = Field( + default_factory=DocsProjectSecurity, + description="Security controls for config-relative path resolution", + ) indexes: list[DocsProjectIndex] = Field( default_factory=list, description="Markdown index files with stricter content discipline", diff --git a/docuchango/templates/docs-project.schema.json b/docuchango/templates/docs-project.schema.json index 59e582a..36433c6 100644 --- a/docuchango/templates/docs-project.schema.json +++ b/docuchango/templates/docs-project.schema.json @@ -100,6 +100,18 @@ }, "default": [] }, + "security": { + "type": "object", + "additionalProperties": false, + "properties": { + "allow_external_paths": { + "type": "boolean", + "default": false, + "description": "Allow configured document paths to resolve outside this docs-project.yaml directory. Prefer subprojects for additional docs roots." + } + }, + "default": {"allow_external_paths": false} + }, "indexes": { "type": "array", "items": { diff --git a/docuchango/templates/docs-project.yaml b/docuchango/templates/docs-project.yaml index 3716ffd..7d5eaa6 100644 --- a/docuchango/templates/docs-project.yaml +++ b/docuchango/templates/docs-project.yaml @@ -64,6 +64,11 @@ structure: # my-custom-standard: "^custom-.+\\.md$" # date-prefix: "^\\d{6}-.+必$" +security: + # Keep configured paths inside this docs-project.yaml directory by default. + # Use subprojects to include other docs roots instead of ../ path escapes. + allow_external_paths: false + # Optional subproject docs configs. Paths are relative to this file and can point # to either a subproject directory or its docs-project.yaml file. # subprojects: diff --git a/docuchango/validator.py b/docuchango/validator.py index e6aa7a8..19524eb 100755 --- a/docuchango/validator.py +++ b/docuchango/validator.py @@ -25,6 +25,7 @@ import re import subprocess import sys +from collections import deque from dataclasses import dataclass, field from datetime import date, datetime from enum import Enum @@ -71,6 +72,9 @@ sys.exit(2) +from docuchango.config_paths import is_within_path, resolve_config_path + + class LinkType(Enum): """Types of links in markdown documents""" @@ -137,6 +141,10 @@ class ProjectConfigContext: def base_dir(self) -> Path: return self.path.parent + @property + def allow_external_paths(self) -> bool: + return self.config.security.allow_external_paths + class DocValidator: """Validates documentation""" @@ -160,6 +168,7 @@ def _load_project_config(self) -> DocsProjectConfig | None: candidate_paths = [ self.repo_root / "docs-project.yaml", self.repo_root / "docs-cms" / "docs-project.yaml", + self.repo_root / "docs" / "docs-project.yaml", ] for config_path in candidate_paths: @@ -220,12 +229,20 @@ def _load_project_config_contexts(self) -> list[ProjectConfigContext]: contexts = [ProjectConfigContext(self.project_config, self.project_config_path)] seen = {self.project_config_path.resolve()} - pending = list(contexts) + pending = deque(contexts) while pending: - parent = pending.pop(0) + parent = pending.popleft() for subproject in parent.config.subprojects: - sub_path = (parent.base_dir / subproject.path).resolve() + sub_path = self._resolve_config_path( + parent, + parent.base_dir, + subproject.path, + parent.base_dir, + "subproject", + ) + if not sub_path: + continue if sub_path.is_dir(): sub_path = sub_path / "docs-project.yaml" if sub_path in seen: @@ -249,6 +266,32 @@ def _get_config_base_dir(self) -> Path: # Legacy default return self.repo_root / "docs-cms" + def _add_path_escape_error( + self, context: ProjectConfigContext, path_value: str, purpose: str, boundary: Path + ) -> None: + """Record a blocked config path that resolved outside its allowed boundary.""" + message = ( + f"Blocked {purpose} path '{path_value}' in {context.path}: configured paths must stay within " + f"{boundary}. Use subprojects for additional docs roots or set security.allow_external_paths: true." + ) + if message not in self.errors: + self.errors.append(message) + self.log(f"⚠️ Warning: {message}", force=True) + + def _resolve_config_path( + self, + context: ProjectConfigContext, + base_dir: Path, + path_value: str, + boundary: Path, + purpose: str, + ) -> Path | None: + """Resolve a configured path and block escapes by default.""" + resolved = resolve_config_path(base_dir, path_value, boundary, context.allow_external_paths) + if resolved is None: + self._add_path_escape_error(context, path_value, purpose, boundary.resolve()) + return resolved + def log(self, message: str, force: bool = False): """Log if verbose or forced""" if self.verbose or force: @@ -278,10 +321,11 @@ def _get_document_folders(self) -> list[str]: # Default folders to scan (includes prd now!) return ["adr", "rfcs", "memos", "prd"] - def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: + def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path, Path, bool]]: """Build scan entries as tuples: - (doc_type, folder_relative, filename_pattern, enforce_filename_pattern, require_frontmatter, root_path) + (doc_type, folder_relative, filename_pattern, enforce_filename_pattern, + require_frontmatter, root_path, boundary, allow_external_paths) """ default_patterns = { "adr": r"^(adr)-(\d{3})-(.+)\.md$", @@ -293,7 +337,7 @@ def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: contexts = self.project_configs or [] if contexts: - entries: list[tuple[str, str, str, bool, bool, Path]] = [] + entries: list[tuple[str, str, str, bool, bool, Path, Path, bool]] = [] for context in contexts: config = context.config config_base = context.base_dir @@ -314,7 +358,15 @@ def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: pattern = default_patterns.get(doc_type_name, r"^(.+)\.md$") folders = cfg.folders or [] for root_rel in roots: - root_path = (config_base / root_rel).resolve() + root_path = self._resolve_config_path( + context, + config_base, + root_rel, + config_base, + "docs root", + ) + if not root_path: + continue for folder in folders: entries.append( ( @@ -324,6 +376,8 @@ def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: cfg.enforce_filename_pattern, cfg.require_frontmatter, root_path, + root_path, + context.allow_external_paths, ) ) continue @@ -338,7 +392,16 @@ def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: folder_name = folder_config[key] if folder_name in config.structure.document_folders: entries.append( - (schema_name, folder_name, default_patterns[schema_name], True, True, config_base) + ( + schema_name, + folder_name, + default_patterns[schema_name], + True, + True, + config_base, + config_base, + context.allow_external_paths, + ) ) return entries @@ -351,7 +414,18 @@ def _build_scan_entries(self) -> list[tuple[str, str, str, bool, bool, Path]]: for key, schema_name in [("adr", "adr"), ("rfc", "rfc"), ("memo", "memo"), ("prd", "prd")]: folder_name = folder_config[key] if folder_name in document_folders: - entries.append((schema_name, folder_name, default_patterns[schema_name], True, True, config_base)) + entries.append( + ( + schema_name, + folder_name, + default_patterns[schema_name], + True, + True, + config_base, + config_base, + False, + ) + ) return entries def _scan_document_folder( @@ -404,7 +478,16 @@ def scan_documents(self): if not entries: self.log(" ⊘ No configured scan entries found", force=True) - for doc_type, folder_name, pattern_text, enforce_pattern, require_frontmatter, root_path in entries: + for ( + doc_type, + folder_name, + pattern_text, + enforce_pattern, + require_frontmatter, + root_path, + boundary, + allow_external_paths, + ) in entries: try: pattern = re.compile(pattern_text) except re.error as e: @@ -412,6 +495,11 @@ def scan_documents(self): continue folder_path = (root_path / folder_name).resolve() + if not allow_external_paths and not is_within_path(folder_path, boundary): + self.errors.append( + f"Blocked document folder path '{folder_name}': configured paths must stay within {boundary}" + ) + continue self.log(f" Scanning {folder_path} ({doc_type} documents)...") self._scan_document_folder( folder_path, @@ -427,7 +515,12 @@ def scan_documents(self): if self.project_configs: roots_to_scan = [] for context in self.project_configs: - roots_to_scan.extend((context.base_dir / p).resolve() for p in context.config.structure.docs_roots) + for path_value in context.config.structure.docs_roots: + resolved = self._resolve_config_path( + context, context.base_dir, path_value, context.base_dir, "docs root" + ) + if resolved: + roots_to_scan.append(resolved) elif self.project_config and self.project_config.structure: roots_to_scan = [ (self._get_config_base_dir() / p).resolve() for p in self.project_config.structure.docs_roots @@ -920,6 +1013,25 @@ def _bucket_for_target(self, target_path: Path, bucket_config) -> str | None: return f"{target_date.year}" return None + def _glob_static_prefix(self, pattern: str) -> str: + """Return the non-glob path prefix used for containment checks.""" + parts = Path(pattern).parts + prefix_parts: list[str] = [] + for part in parts: + if any(char in part for char in "*?["): + break + prefix_parts.append(part) + if not prefix_parts: + return "." + return str(Path(*prefix_parts)) + + def _safe_index_target_paths(self, context: ProjectConfigContext, target_pattern: str) -> set[Path]: + """Expand an index target glob only when its static prefix stays in the config boundary.""" + prefix = self._glob_static_prefix(target_pattern) + if not self._resolve_config_path(context, context.base_dir, prefix, context.base_dir, "index target"): + return set() + return {path.resolve() for path in context.base_dir.glob(target_pattern) if path.is_file()} + def check_document_indexes(self): """Validate configured Markdown index files.""" contexts = self.project_configs or [] @@ -935,7 +1047,11 @@ def check_document_indexes(self): for context in contexts: config_base = context.base_dir for index_config in context.config.indexes: - index_path = (config_base / index_config.path).resolve() + index_path = self._resolve_config_path( + context, config_base, index_config.path, config_base, "document index" + ) + if not index_path: + continue if not index_path.exists(): self.errors.append(f"Document index '{index_config.name}' not found: {index_config.path}") continue @@ -945,9 +1061,7 @@ def check_document_indexes(self): links = self._extract_markdown_file_links(content, index_path) target_paths: set[Path] = set() for target_pattern in index_config.targets: - target_paths.update( - path.resolve() for path in config_base.glob(target_pattern) if path.is_file() - ) + target_paths.update(self._safe_index_target_paths(context, target_pattern)) target_paths.discard(index_path) if index_config.require_entries and target_paths and not links: diff --git a/tests/test_bulk_update_comprehensive.py b/tests/test_bulk_update_comprehensive.py index d03868d..ef5c745 100644 --- a/tests/test_bulk_update_comprehensive.py +++ b/tests/test_bulk_update_comprehensive.py @@ -195,6 +195,20 @@ def test_invalid_operation(self): with pytest.raises(ValueError, match="Invalid operation"): update_frontmatter_bulk(content, "field", "value", "invalid_op") + def test_rename_requires_new_field_name(self): + """Test rename operation rejects empty target names.""" + content = """--- +id: test +old_name: value +--- +# Test +""" + with pytest.raises(ValueError, match="non-empty new field name"): + update_frontmatter_bulk(content, "old_name", "", "rename") + + with pytest.raises(ValueError, match="non-empty new field name"): + update_frontmatter_bulk(content, "old_name", None, "rename") + def test_very_long_field_name(self): """Test with very long field name.""" content = """--- diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 1b353f0..2e99748 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -700,3 +700,23 @@ def test_validate_verbose_with_exception(self, tmp_path): ) # May succeed or fail, but should not crash assert result.exit_code in [0, 1, 2] + + +class TestBulkUpdateCommand: + """Test bulk update command validation.""" + + def test_bulk_update_rejects_empty_set_field_name(self): + runner = CliRunner() + + result = runner.invoke(main, ["bulk", "update", "--set", "=value"]) + + assert result.exit_code == 1 + assert "non-empty field name" in result.output + + def test_bulk_update_rejects_empty_rename_target(self): + runner = CliRunner() + + result = runner.invoke(main, ["bulk", "update", "--rename", "old_name="]) + + assert result.exit_code == 1 + assert "non-empty OLD and NEW" in result.output diff --git a/tests/test_config_loading.py b/tests/test_config_loading.py index a00e1fa..3cea749 100644 --- a/tests/test_config_loading.py +++ b/tests/test_config_loading.py @@ -10,6 +10,25 @@ from docuchango.validator import DocValidator +def write_valid_adr(path: Path, project_id: str = "secure-project", adr_id: str = "adr-001") -> None: + """Write a minimal valid ADR document.""" + path.write_text( + f"""--- +title: Secure Boundary Decision +status: Accepted +created: 2025-01-01 +deciders: Core Team +tags: [security] +id: {adr_id} +project_id: {project_id} +doc_uuid: 12345678-1234-4123-8123-123456789abc +--- + +# Decision +""" + ) + + class TestConfigLoading: """Test configuration file loading in DocValidator.""" @@ -22,6 +41,8 @@ def test_docs_project_json_schema_exists_for_editor_validation(self): assert schema["properties"]["version"]["default"] == "1" assert "subprojects" in schema["properties"] assert schema["properties"]["subprojects"]["items"]["oneOf"][0]["type"] == "string" + assert "security" in schema["properties"] + assert schema["properties"]["security"]["properties"]["allow_external_paths"]["default"] is False def test_load_valid_config(self): """Test that valid config file is loaded correctly.""" @@ -373,6 +394,27 @@ def test_load_config_from_repo_root(self): assert validator.project_config is not None assert validator.project_config.project.id == "root-config-project" + def test_load_config_from_docs_directory(self): + """Validator should load docs/docs-project.yaml when docs-cms config is absent.""" + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = Path(tmpdir) + docs_dir = repo_root / "docs" + docs_dir.mkdir() + (docs_dir / "docs-project.yaml").write_text( + yaml.dump( + { + "project": {"id": "docs-project", "name": "Docs Project"}, + "structure": {"document_folders": ["adr"]}, + } + ) + ) + + validator = DocValidator(repo_root, verbose=False) + + assert validator.project_config is not None + assert validator.project_config_path == (docs_dir / "docs-project.yaml").resolve() + assert validator.project_config.project.id == "docs-project" + def test_doc_types_custom_layout_mixed_schema(self): """Custom doc_types should support mixed folders and schema bindings.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -448,6 +490,116 @@ def test_doc_types_custom_layout_mixed_schema(self): assert "adr-001-test.md" in names assert "prfaq-why-now.md" in names + def test_docs_directory_config_blocks_folder_escape(self): + """A docs-local config should not process sibling repo folders via ../ paths.""" + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = Path(tmpdir) + docs_dir = repo_root / "docs" + in_scope_adr = docs_dir / "adr" + outside_adr = repo_root / "adr" + in_scope_adr.mkdir(parents=True) + outside_adr.mkdir() + + write_valid_adr(in_scope_adr / "adr-001-in-scope.md") + write_valid_adr(outside_adr / "adr-002-outside.md", adr_id="adr-002") + + (docs_dir / "docs-project.yaml").write_text( + yaml.dump( + { + "project": {"id": "secure-project", "name": "Secure Project"}, + "structure": { + "docs_roots": ["."], + "doc_types": { + "adr": { + "schema": "adr", + "folders": ["adr", "../adr"], + "filename_pattern": r"^(adr)-(\d{3})-(.+)\.md$", + } + }, + }, + } + ) + ) + + validator = DocValidator(repo_root, verbose=False) + validator.scan_documents() + + assert [doc.file_path.name for doc in validator.documents] == ["adr-001-in-scope.md"] + assert any("Blocked document folder path '../adr'" in error for error in validator.errors) + assert _discover_doc_files(repo_root) == [(in_scope_adr / "adr-001-in-scope.md").resolve()] + + def test_docs_directory_config_can_explicitly_allow_external_paths(self): + """The explicit security allow flag permits legacy external path layouts.""" + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = Path(tmpdir) + docs_dir = repo_root / "docs" + outside_adr = repo_root / "adr" + docs_dir.mkdir() + outside_adr.mkdir() + + write_valid_adr(outside_adr / "adr-001-outside.md") + + (docs_dir / "docs-project.yaml").write_text( + yaml.dump( + { + "project": {"id": "secure-project", "name": "Secure Project"}, + "security": {"allow_external_paths": True}, + "structure": { + "docs_roots": ["."], + "doc_types": { + "adr": { + "schema": "adr", + "folders": ["../adr"], + "filename_pattern": r"^(adr)-(\d{3})-(.+)\.md$", + } + }, + }, + } + ) + ) + + validator = DocValidator(repo_root, verbose=False) + validator.scan_documents() + + assert [doc.file_path.name for doc in validator.documents] == ["adr-001-outside.md"] + assert not any("Blocked" in error for error in validator.errors) + + def test_docs_directory_config_blocks_subproject_escape(self): + """A nested docs config should not load sibling subprojects through ../ by default.""" + with tempfile.TemporaryDirectory() as tmpdir: + repo_root = Path(tmpdir) + docs_dir = repo_root / "docs" + service_dir = repo_root / "service-a" + service_adr = service_dir / "adr" + docs_dir.mkdir() + service_adr.mkdir(parents=True) + + (docs_dir / "docs-project.yaml").write_text( + yaml.dump( + { + "project": {"id": "parent-project", "name": "Parent Project"}, + "subprojects": ["../service-a"], + } + ) + ) + (service_dir / "docs-project.yaml").write_text( + yaml.dump( + { + "project": {"id": "service-a", "name": "Service A"}, + "structure": {"document_folders": ["adr"]}, + } + ) + ) + write_valid_adr(service_adr / "adr-001-service.md", project_id="service-a") + + validator = DocValidator(repo_root, verbose=False) + validator.scan_documents() + + assert [context.config.project.id for context in validator.project_configs] == ["parent-project"] + assert validator.documents == [] + assert any("Blocked subproject path '../service-a'" in error for error in validator.errors) + assert _discover_doc_files(repo_root) == [] + def test_doc_types_can_disable_filename_enforcement(self): """Custom doc_types can disable strict filename enforcement.""" with tempfile.TemporaryDirectory() as tmpdir: diff --git a/tests/test_readability.py b/tests/test_readability.py index abd19b1..bc3d506 100644 --- a/tests/test_readability.py +++ b/tests/test_readability.py @@ -404,6 +404,24 @@ def test_html_mdx_tags_skipped(self): for para_text, _ in paragraphs: assert " + +Second regular paragraph with enough text. +""" + paragraphs = scorer.extract_paragraphs(content) + + assert len(paragraphs) == 2 + assert "First regular paragraph" in paragraphs[0][0] + assert "Second regular paragraph" in paragraphs[1][0] + def test_blockquotes_skipped(self): """Test that blockquotes are skipped.""" config = ReadabilityConfig(min_paragraph_length=20) diff --git a/tests/test_readability_optional.py b/tests/test_readability_optional.py index cb9475e..42a2359 100644 --- a/tests/test_readability_optional.py +++ b/tests/test_readability_optional.py @@ -208,12 +208,64 @@ def test_readability_metric_enum(self): class TestReadabilityParagraphExtraction: """Test paragraph extraction logic independently.""" - @pytest.mark.skipif(True, reason="Would need to mock textstat for ReadabilityScorer instantiation") def test_extract_paragraphs_logic(self): """Test paragraph extraction without requiring textstat calculations.""" - # This would require restructuring to separate extraction from scoring - # Keeping as placeholder for potential refactoring - pass + import docuchango.readability as readability_module + from docuchango.readability import ReadabilityConfig, ReadabilityScorer + + class UnexpectedTextstatUse: + def __getattr__(self, name: str) -> object: + raise AssertionError(f"extract_paragraphs should not call textstat.{name}") + + content = """--- +title: Test Document +--- + +# Heading + +This paragraph should be extracted because it is long enough to pass the configured minimum length. +It continues on a second line so extraction should join both lines with one space. + +- This list item should be ignored even though it has words. + +```python +print("code blocks should be ignored") +``` + + + + +MDX component content should be ignored. + + +This final paragraph should also be extracted because it is plain markdown text with sufficient length. +""" + + original_available = readability_module.TEXTSTAT_AVAILABLE + original_textstat = getattr(readability_module, "textstat", None) + + try: + readability_module.TEXTSTAT_AVAILABLE = True + vars(readability_module)["textstat"] = UnexpectedTextstatUse() + + scorer = ReadabilityScorer(ReadabilityConfig(min_paragraph_length=80)) + paragraphs = scorer.extract_paragraphs(content) + + assert paragraphs == [ + ( + "This paragraph should be extracted because it is long enough to pass the configured minimum " + "length. It continues on a second line so extraction should join both lines with one space.", + 7, + ), + ( + "This final paragraph should also be extracted because it is plain markdown text with " + "sufficient length.", + 22, + ), + ] + finally: + readability_module.TEXTSTAT_AVAILABLE = original_available + vars(readability_module)["textstat"] = original_textstat class TestReadabilityConfigSchema: diff --git a/tests/test_schemas.py b/tests/test_schemas.py index d91a6a1..246b8af 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -410,6 +410,21 @@ def test_valid_config_with_defaults(self): assert config.structure.template_dir == "templates" assert config.structure.document_folders == ["adr", "rfcs", "memos", "prd"] assert config.metadata is None + assert config.security.allow_external_paths is False + + def test_config_can_explicitly_allow_external_paths(self): + """Projects must opt in before config paths can escape their config directory.""" + config = DocsProjectConfig( + project={ + "id": "external-paths", + "name": "External Paths", + }, + security={ + "allow_external_paths": True, + }, + ) + + assert config.security.allow_external_paths is True def test_config_custom_document_folders(self): """Test that document_folders can be customized."""