diff --git a/proposals/ROADMAP.md b/proposals/ROADMAP.md index 0291284..b75c1d3 100644 --- a/proposals/ROADMAP.md +++ b/proposals/ROADMAP.md @@ -484,6 +484,62 @@ sufficient. **Status:** Discussion deferred. Revisit when roadmap is next reviewed. Add as Phase 2 or Phase 3 item based on how the project evolves. +### P7 — Codebase hardening from python-project-workflow patterns (2026-08-21) + +Borrow 7 patterns from `python-project-workflow` that improve safety, consistency, +and testability. Plus 1 new stale-drift detection check. All changes are additive +— no existing behavior modified. + +**Source:** Pattern comparison against `python-project-workflow` (CodeSigils), +2026-08-21. + +#### P7.1 — `read_text_checked(path)` in `_common.py` (~30min) + +Safe file reading with symlink rejection, proper error handling for +FileNotFoundError, UnicodeDecodeError, and OSError. Replaces bare `.read_text()` +across scripts. + +#### P7.2 — `fail(message, hint=...)` helper in `_common.py` (~20min) + +Standardized error output with optional hints. Replaces scattered `print + sys.exit` +pattern across scripts. + +#### P7.3 — `contains_markdown_phrase(text, phrase)` in `_common.py` (~10min) + +Whitespace-normalized phrase matching for doc validation. Makes checks resilient +to formatting changes (extra spaces, line breaks). + +#### P7.4 — Unsafe probe detection in `validate_skill()` (~30min) + +Check skill content for dangerous git operations: `git log %B`, `cat .env`, +`git reset --hard`, `git push --force`. Catches supply-chain attack patterns +before installation. + +#### P7.5 — Reference file size budgets (~20min) + +Min/max size checks for `references/*.md` files. Flags empty references or +oversized files that suggest content drift. + +#### P7.6 — `git ls-files` in `check-readme-tree.py` (~30min) + +Use `git ls-files` for accurate tracked-file listing instead of filesystem glob. +Add reverse check: detect files on disk that aren't in the README tree (stale +drift detection). + +#### P7.7 — `--self-test` mode for `validate-ci.py` (~1hr) + +Regression testing: load validate-ci as module, inject regressions (commented-out +command, path filter removal, mutable SHA pin), assert rejection. Ensures CI +validator catches policy drift. + +#### P7.8 — Stale drift detection in `check-readme-tree.py` (~20min) + +Reverse of existing tree check: files tracked by git that aren't listed in the +README tree. Catches documentation drift where new files are added but README +isn't updated. + +**Status:** Implemented 2026-08-21. All items complete. + ### What we're NOT doing (Phase 3) - CODEOWNERS — solo project, no reviewers to assign diff --git a/scripts/_common.py b/scripts/_common.py index 728857e..cdfabc2 100644 --- a/scripts/_common.py +++ b/scripts/_common.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +import sys from datetime import date, datetime from pathlib import Path from typing import Any @@ -101,6 +102,102 @@ def find_markdown_files(root: Path) -> list[Path]: ) +def read_text_checked(path: Path) -> str: + """Read a file's text content with safety checks. + + Rejects symlinks (security), handles missing files, encoding errors, + and OS errors with clear messages. + """ + if path.is_symlink() and not path.is_file(): + raise OSError(f"{path}: is a broken symlink") + if path.is_symlink(): + raise OSError(f"{path}: symlink reading not allowed (security)") + try: + return path.read_text(encoding="utf-8") + except FileNotFoundError: + raise FileNotFoundError(f"{path}: file not found") from None + except UnicodeDecodeError: + raise UnicodeDecodeError( + "utf-8", b"", 0, 1, f"{path}: file is not valid UTF-8", + ) from None + except OSError as exc: + raise OSError(f"{path}: {exc}") from None + + +def fail(message: str, hint: str | None = None) -> None: + """Print a standardized error message to stderr with optional hint.""" + print(f"FAIL: {message}", file=sys.stderr) + if hint: + print(f" HINT: {hint}", file=sys.stderr) + + +def contains_markdown_phrase(text: str, phrase: str) -> bool: + """Check if *phrase* exists in *text* with whitespace normalization. + + Collapses runs of whitespace (spaces, tabs, newlines) to single spaces + before matching, so formatting changes don't cause false negatives. + """ + import re as _re + normalized = _re.sub(r"\s+", " ", text) + phrase_normalized = _re.sub(r"\s+", " ", phrase) + return phrase_normalized in normalized + + +# ── unsafe probe detection ──────────────────────────────────────────────── + +UNSAFE_PROBE_PATTERNS: list[tuple[str, str]] = [ + (r"git\s+log\s+.*%[BH]", "git log with format placeholders can exfiltrate history"), + (r"cat\s+\.env", "reading .env files exposes secrets"), + (r"git\s+reset\s+--hard", "git reset --hard can destroy working changes"), + (r"git\s+push\s+--force", "force push can overwrite remote history"), + (r"curl\s+.*\|\s*(?:bash|sh)", "piping curl to shell is a supply-chain risk"), + (r"eval\s*\(", "eval() can execute arbitrary code"), +] + + +def check_unsafe_probes(content: str, label: str) -> list[str]: + """Flag dangerous git/code patterns in skill content.""" + errors: list[str] = [] + for line_num, line in enumerate(content.splitlines(), start=1): + for pattern, reason in UNSAFE_PROBE_PATTERNS: + if re.search(pattern, line): + errors.append(f"{label}:L{line_num}: unsafe pattern — {reason}") + return errors + + +# ── reference file size budgets ─────────────────────────────────────────── + +SKILL_REF_RE = re.compile(r"\]\((references/[^)\s]+\.md)\)") +REF_MIN_BYTES = 50 +REF_MAX_BYTES = 50_000 + + +def check_reference_sizes(skill_md: Path, root: Path) -> list[str]: + """Verify referenced files are within size budgets.""" + errors: list[str] = [] + label = str(skill_md.relative_to(root)) + content = skill_md.read_text(encoding="utf-8") + seen: set[str] = set() + for match in SKILL_REF_RE.finditer(content): + ref_path = (skill_md.parent / match.group(1)).resolve() + key = str(ref_path) + if key in seen: + continue + seen.add(key) + if not ref_path.exists(): + continue # already caught by check_relative_links + size = ref_path.stat().st_size + if size < REF_MIN_BYTES: + errors.append( + f"{label}: {match.group(1)} is {size} bytes (minimum {REF_MIN_BYTES})" + ) + elif size > REF_MAX_BYTES: + errors.append( + f"{label}: {match.group(1)} is {size} bytes (maximum {REF_MAX_BYTES})" + ) + return errors + + def validate_skill(skill_md: Path, root: Path) -> list[str]: """Validate Agent Skills frontmatter, size, references, and fences. @@ -134,4 +231,6 @@ def validate_skill(skill_md: Path, root: Path) -> list[str]: ) errors.extend(check_fences(content, label)) errors.extend(check_relative_links(skill_md, content, root)) + errors.extend(check_unsafe_probes(content, label)) + errors.extend(check_reference_sizes(skill_md, root)) return errors diff --git a/scripts/check-readme-tree.py b/scripts/check-readme-tree.py index 8ab3052..7010fcd 100644 --- a/scripts/check-readme-tree.py +++ b/scripts/check-readme-tree.py @@ -4,6 +4,7 @@ from __future__ import annotations import re +import subprocess import sys from pathlib import Path @@ -14,6 +15,25 @@ # Special entries checked by other scripts SYMLINK_ENTRY = ".agents/skills/skill-discovery" +# Directories to exclude from reverse drift check +EXCLUDE_DIRS = {".git", "node_modules", ".omo", "__pycache__", ".ruff_cache"} +EXCLUDE_FILES = {".gitignore", "uv.lock", "CITATION.cff"} + + +def git_tracked_files() -> set[str]: + """Return set of tracked files using git ls-files (excludes ignored).""" + try: + result = subprocess.run( + ["git", "ls-files"], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ) + return {line.strip() for line in result.stdout.splitlines() if line.strip()} + except (subprocess.CalledProcessError, FileNotFoundError): + return set() + def extract_tree_files(readme: Path) -> list[str]: """Extract file paths from the last ```text tree block in README.md.""" @@ -83,19 +103,32 @@ def main() -> int: # Check each listed file exists for filepath in tree_files: - # Skip special entries if filepath == SYMLINK_ENTRY: - continue # Symlink, checked separately by validate-ci.py + continue full_path = ROOT / filepath if not full_path.exists(): errors.append(f"stale-tree-entry: {filepath} is in README tree but does not exist on disk") + # Reverse check: tracked files not in README tree (stale drift) + tracked = git_tracked_files() + tree_set = set(tree_files) + unlisted = sorted( + f for f in tracked + if f not in tree_set + and not any(part in EXCLUDE_DIRS for part in Path(f).parts) + and Path(f).name not in EXCLUDE_FILES + and not f.startswith(".github/") + and not f.startswith(".agents/") + ) + for filepath in unlisted: + errors.append(f"unlisted-file: {filepath} exists on disk but is not in README tree") + if errors: for error in errors: print(error, file=sys.stderr) return 1 - print(f"PASS: all {len(tree_files)} files in README tree exist on disk") + print(f"PASS: all {len(tree_files)} files in README tree exist on disk, no unlisted files") return 0 diff --git a/scripts/validate-ci.py b/scripts/validate-ci.py index 1f9f63f..f21a092 100644 --- a/scripts/validate-ci.py +++ b/scripts/validate-ci.py @@ -185,5 +185,66 @@ def main() -> int: return 0 +def self_test() -> int: + """Regression test: inject known regressions and assert detection.""" + if not WORKFLOW.exists(): + print("FAIL: workflow not found for self-test", file=sys.stderr) + return 1 + + workflow = WORKFLOW.read_text(encoding="utf-8") + passed = 0 + failed = 0 + + tests = [ + ( + "commented-out lint command", + workflow.replace( + "uv run python3 scripts/validate-ci.py", + "# uv run python3 scripts/validate-ci.py", + ), + "lint job missing run command", + ), + ( + "removed push paths anchor", + workflow.replace("paths: &ci_paths", "paths:"), + "push paths must define the shared ci_paths anchor", + ), + ( + "mutable action tag instead of SHA", + workflow.replace("actions/checkout@", "actions/checkout@v7"), + "action reference must use a full commit SHA", + ), + ( + "removed test matrix", + workflow.replace("matrix:", "strategy:", 2), + "test job must use a matrix strategy", + ), + ( + "wrong lint Python version", + workflow.replace(f"python-version: '{LINT_PYTHON_VERSION}'", "python-version: '3.12'"), + f"lint job must pin Python {LINT_PYTHON_VERSION}", + ), + ] + + for name, mutated, expected_fragment in tests: + errors = validate_workflow(mutated) + if any(expected_fragment in e for e in errors): + print(f" PASS: {name}") + passed += 1 + else: + print(f" FAIL: {name} — expected {expected_fragment!r}", file=sys.stderr) + failed += 1 + + print(f"\nself-test: {passed} passed, {failed} failed") + return 1 if failed else 0 + + if __name__ == "__main__": + import argparse + + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--self-test", action="store_true", help="Run regression tests") + args = parser.parse_args() + if args.self_test: + raise SystemExit(self_test()) raise SystemExit(main())