From f393f960e7607efa629937dd4212137d4ea458d2 Mon Sep 17 00:00:00 2001 From: eshulman2 Date: Thu, 6 Aug 2026 19:53:47 +0300 Subject: [PATCH 1/2] feat: add fail-closed repository output gate --- .env.example | 6 + docs/reference/config.md | 20 ++ src/forge/config.py | 21 ++ src/forge/workspace/git_ops.py | 14 ++ src/forge/workspace/output_validation.py | 181 ++++++++++++++++++ .../test_git_ops_output_validation.py | 34 ++++ .../unit/workspace/test_output_validation.py | 127 ++++++++++++ 7 files changed, 403 insertions(+) create mode 100644 src/forge/workspace/output_validation.py create mode 100644 tests/unit/workspace/test_git_ops_output_validation.py create mode 100644 tests/unit/workspace/test_output_validation.py diff --git a/.env.example b/.env.example index 4aecde790..68d43c73e 100644 --- a/.env.example +++ b/.env.example @@ -144,6 +144,12 @@ SKILLS_DIR=skills/ # setups where containers and the host worker must see the same checkout. # WORKSPACE_BASE_DIR=/var/lib/forge/workspaces +# Trusted output gate (checked immediately before every Git push) +# Agents cannot add, modify, or delete these comma-separated glob/path patterns. +# OUTPUT_PROTECTED_PATHS=.github/workflows/**,.github/CODEOWNERS,.gitlab-ci.yml,CODEOWNERS +# OUTPUT_MAX_FILE_BYTES=10485760 +# OUTPUT_MAX_TOTAL_BYTES=52428800 + # ============================================================================= # Prompt Configuration # ============================================================================= diff --git a/docs/reference/config.md b/docs/reference/config.md index f13fd5f29..ef0b5f2f1 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -2,6 +2,26 @@ All configuration is via environment variables in `.env`. See `.env.example` in the repository for the complete list with comments. +## Safe Output Gate + +Forge validates repository changes in the trusted worker immediately before +every Git push. Validation fails closed when Git metadata cannot be inspected, +and rejects protected paths, symbolic links, oversized files, and oversized +combined output. Deletions count as protected-path changes but do not count +toward size limits. + +| Variable | Default | Description | +|----------|---------|-------------| +| `OUTPUT_PROTECTED_PATHS` | `.github/workflows/**,.github/CODEOWNERS,.gitlab-ci.yml,CODEOWNERS` | Comma-separated exact paths or glob patterns agents cannot publish | +| `OUTPUT_MAX_FILE_BYTES` | `10485760` | Maximum size of an added or modified file | +| `OUTPUT_MAX_TOTAL_BYTES` | `52428800` | Maximum combined size of added and modified files | + +The gate evaluates changes from the merge base with `origin/HEAD` through the +current branch tip. A missing remote default-branch reference blocks the push +instead of silently skipping validation. Additional validators, such as secret +scanners, can consume the same `OutputValidationContext` and run in the same +trusted gate. + ## Required Variables ### Jira diff --git a/src/forge/config.py b/src/forge/config.py index d109ce92e..943044b5b 100644 --- a/src/forge/config.py +++ b/src/forge/config.py @@ -120,6 +120,27 @@ def atlassian_auth_base64(self) -> str: "Unset (default) uses a per-run system temp directory." ), ) + output_protected_paths: str = Field( + default=".github/workflows/**,.github/CODEOWNERS,.gitlab-ci.yml,CODEOWNERS", + description="Comma-separated path patterns agents may not publish", + ) + output_max_file_bytes: int = Field( + default=10 * 1024 * 1024, + ge=1, + description="Maximum size of one added or modified file published by an agent", + ) + output_max_total_bytes: int = Field( + default=50 * 1024 * 1024, + ge=1, + description="Maximum total size of added or modified agent output", + ) + + @property + def protected_output_paths(self) -> tuple[str, ...]: + """Return normalized configured protected path patterns.""" + return tuple( + value.strip() for value in self.output_protected_paths.split(",") if value.strip() + ) # PRD Approval Configuration (global fallbacks — per-project config via # Jira project property forge.prd_proposals_repo takes precedence) diff --git a/src/forge/workspace/git_ops.py b/src/forge/workspace/git_ops.py index 9afe3134a..a73159013 100644 --- a/src/forge/workspace/git_ops.py +++ b/src/forge/workspace/git_ops.py @@ -7,6 +7,7 @@ from forge.config import get_settings from forge.utils.redaction import redact_secrets from forge.workspace.manager import Workspace +from forge.workspace.output_validation import OutputValidationPolicy, validate_repository_output logger = logging.getLogger(__name__) @@ -176,6 +177,7 @@ def push_to_fork(self, force: bool = False) -> None: Args: force: Force push (use with caution). """ + self.validate_output_for_push() args = ["push", "-u", "fork", self.workspace.branch_name] if force: args.insert(1, "--force") @@ -357,6 +359,7 @@ def push(self, force: bool = False, check_conflicts: bool = True) -> None: Raises: GitError: If conflicts detected and check_conflicts is True. """ + self.validate_output_for_push() if check_conflicts and not force: has_conflicts, conflicting_files = self.check_for_conflicts() if has_conflicts: @@ -373,6 +376,17 @@ def push(self, force: bool = False, check_conflicts: bool = True) -> None: self._run_git(*args) logger.info(f"Pushed branch {self.workspace.branch_name}") + def validate_output_for_push(self) -> None: + """Validate branch output at the trusted boundary before any push.""" + validate_repository_output( + self.repo_path, + OutputValidationPolicy( + protected_paths=self.settings.protected_output_paths, + max_file_bytes=self.settings.output_max_file_bytes, + max_total_bytes=self.settings.output_max_total_bytes, + ), + ) + def get_current_sha(self) -> str: """Get the current commit SHA. diff --git a/src/forge/workspace/output_validation.py b/src/forge/workspace/output_validation.py new file mode 100644 index 000000000..b2e7d9c90 --- /dev/null +++ b/src/forge/workspace/output_validation.py @@ -0,0 +1,181 @@ +"""Fail-closed validation of agent-produced repository output.""" + +from __future__ import annotations + +import fnmatch +import logging +import subprocess +from dataclasses import dataclass, field +from pathlib import Path, PurePosixPath +from typing import Protocol + +from forge.utils.redaction import redact_secrets + +logger = logging.getLogger(__name__) + + +class OutputValidationError(RuntimeError): + """Raised when repository output is unsafe to publish.""" + + +@dataclass(frozen=True) +class OutputValidationPolicy: + """Policy applied immediately before an external Git write.""" + + protected_paths: tuple[str, ...] = () + max_file_bytes: int = 10 * 1024 * 1024 + max_total_bytes: int = 50 * 1024 * 1024 + reject_symlinks: bool = True + + +@dataclass +class OutputValidationContext: + """Stable input shared by all output validators.""" + + repo_path: Path + base_ref: str + head_ref: str = "HEAD" + changed_paths: tuple[str, ...] = () + metadata: dict[str, object] = field(default_factory=dict) + + +class OutputValidator(Protocol): + """Extension point for additional gates such as secret scanners.""" + + name: str + + def validate(self, context: OutputValidationContext) -> None: ... + + +class SafeRepositoryOutputValidator: + """Reject dangerous paths, links, and unexpectedly large output.""" + + name = "safe_repository_output" + + def __init__(self, policy: OutputValidationPolicy): + self.policy = policy + + def validate(self, context: OutputValidationContext) -> None: + entries = _changed_entries(context.repo_path, context.base_ref, context.head_ref) + context.changed_paths = tuple(path for _, _, path in entries) + violations: list[str] = [] + total_size = 0 + + for status, mode, path in entries: + if not _is_safe_relative_path(path): + violations.append(f"unsafe path: {path!r}") + continue + if _is_protected(path, self.policy.protected_paths): + violations.append(f"protected path changed: {path}") + if status == "D": + continue + if self.policy.reject_symlinks and mode == "120000": + violations.append(f"symbolic link output is not allowed: {path}") + continue + size = _blob_size(context.repo_path, context.head_ref, path) + total_size += size + if size > self.policy.max_file_bytes: + violations.append( + f"file exceeds {self.policy.max_file_bytes} bytes: {path} ({size} bytes)" + ) + + if total_size > self.policy.max_total_bytes: + violations.append( + f"changed output exceeds {self.policy.max_total_bytes} bytes ({total_size} bytes)" + ) + if violations: + raise OutputValidationError( + "Unsafe repository output; push blocked: " + "; ".join(violations) + ) + + +def validate_repository_output( + repo_path: Path, + policy: OutputValidationPolicy, + validators: tuple[OutputValidator, ...] = (), +) -> OutputValidationContext: + """Run every configured validator, failing closed on inspection errors.""" + + base_ref = _default_base_ref(repo_path) + context = OutputValidationContext(repo_path=repo_path, base_ref=base_ref) + configured: tuple[OutputValidator, ...] = (SafeRepositoryOutputValidator(policy), *validators) + for validator in configured: + try: + validator.validate(context) + except OutputValidationError: + raise + except Exception as exc: + raise OutputValidationError( + f"Output validator {validator.name!r} failed; push blocked: {redact_secrets(exc)}" + ) from exc + logger.info("Repository output passed %d validation gate(s)", len(configured)) + return context + + +def _git(repo_path: Path, *args: str) -> str: + result = subprocess.run( + ["git", *args], cwd=repo_path, capture_output=True, text=True, check=False + ) + if result.returncode != 0: + detail = result.stderr.strip() or result.stdout.strip() or "unknown Git error" + raise OutputValidationError( + f"Unable to inspect repository output; push blocked: {redact_secrets(detail)}" + ) + return result.stdout + + +def _default_base_ref(repo_path: Path) -> str: + result = subprocess.run( + ["git", "symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], + cwd=repo_path, + capture_output=True, + text=True, + check=False, + ) + symbolic = result.stdout.strip() + if not symbolic.startswith("refs/remotes/"): + raise OutputValidationError("Remote default branch is unavailable; push blocked") + remote_default = symbolic.removeprefix("refs/remotes/") + return _git(repo_path, "merge-base", "HEAD", remote_default).strip() + + +def _changed_entries(repo_path: Path, base_ref: str, head_ref: str) -> list[tuple[str, str, str]]: + output = _git( + repo_path, "diff", "--raw", "--no-renames", "--diff-filter=ACDMRTUXB", base_ref, head_ref + ) + entries: list[tuple[str, str, str]] = [] + for line in output.splitlines(): + header, separator, path = line.partition("\t") + if not separator: + raise OutputValidationError("Malformed Git diff metadata; push blocked") + fields = header.split() + if len(fields) != 5: + raise OutputValidationError("Malformed Git diff metadata; push blocked") + old_mode, new_mode, _old_sha, _new_sha, status = fields + entries.append((status[0], old_mode if status[0] == "D" else new_mode, path)) + return entries + + +def _blob_size(repo_path: Path, ref: str, path: str) -> int: + raw = _git(repo_path, "cat-file", "-s", f"{ref}:{path}").strip() + try: + return int(raw) + except ValueError as exc: + raise OutputValidationError(f"Invalid blob size for {path!r}; push blocked") from exc + + +def _is_safe_relative_path(path: str) -> bool: + if not path or "\x00" in path or "\n" in path or "\r" in path: + return False + pure = PurePosixPath(path) + return not pure.is_absolute() and ".." not in pure.parts and not path.startswith("-") + + +def _is_protected(path: str, patterns: tuple[str, ...]) -> bool: + normalized = path.removeprefix("./") + return any( + normalized == pattern.rstrip("/") + or normalized.startswith(pattern.rstrip("/") + "/") + or fnmatch.fnmatchcase(normalized, pattern) + for pattern in patterns + ) diff --git a/tests/unit/workspace/test_git_ops_output_validation.py b/tests/unit/workspace/test_git_ops_output_validation.py new file mode 100644 index 000000000..4517d8c4d --- /dev/null +++ b/tests/unit/workspace/test_git_ops_output_validation.py @@ -0,0 +1,34 @@ +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from forge.workspace.git_ops import GitOperations +from forge.workspace.manager import Workspace +from forge.workspace.output_validation import OutputValidationError + + +def _operations(tmp_path: Path) -> GitOperations: + settings = MagicMock() + settings.protected_output_paths = ("CODEOWNERS",) + settings.output_max_file_bytes = 100 + settings.output_max_total_bytes = 200 + workspace = Workspace(tmp_path, "org/repo", "forge/task-1", "TASK-1") + with patch("forge.workspace.git_ops.get_settings", return_value=settings): + return GitOperations(workspace) + + +@pytest.mark.parametrize("method", ["push_to_fork", "push"]) +def test_push_methods_validate_before_running_git(tmp_path: Path, method: str) -> None: + git = _operations(tmp_path) + git._run_git = MagicMock() + + with patch( + "forge.workspace.git_ops.validate_repository_output", + side_effect=OutputValidationError("blocked"), + ) as validate: + with pytest.raises(OutputValidationError, match="blocked"): + getattr(git, method)() + + validate.assert_called_once() + git._run_git.assert_not_called() diff --git a/tests/unit/workspace/test_output_validation.py b/tests/unit/workspace/test_output_validation.py new file mode 100644 index 000000000..c3f00ee94 --- /dev/null +++ b/tests/unit/workspace/test_output_validation.py @@ -0,0 +1,127 @@ +"""Tests for the trusted repository-output validation gate.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from forge.workspace.output_validation import ( + OutputValidationError, + OutputValidationPolicy, + validate_repository_output, +) + + +def _run(path: Path, *args: str) -> str: + return subprocess.run( + ["git", *args], cwd=path, capture_output=True, text=True, check=True + ).stdout.strip() + + +@pytest.fixture +def repository(tmp_path: Path) -> Path: + origin = tmp_path / "origin.git" + work = tmp_path / "work" + _run(tmp_path, "init", "--bare", "--initial-branch=main", str(origin)) + _run(tmp_path, "clone", str(origin), str(work)) + _run(work, "config", "user.email", "forge@example.com") + _run(work, "config", "user.name", "Forge") + (work / "README.md").write_text("initial\n") + _run(work, "add", "README.md") + _run(work, "commit", "-m", "initial") + _run(work, "push", "-u", "origin", "main") + _run(work, "remote", "set-head", "origin", "main") + _run(work, "switch", "-c", "forge/task-1") + return work + + +def _commit(repo: Path, path: str, content: str) -> None: + target = repo / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content) + _run(repo, "add", path) + _run(repo, "commit", "-m", f"change {path}") + + +def test_accepts_bounded_regular_output(repository: Path) -> None: + _commit(repository, "src/example.py", "print('safe')\n") + + context = validate_repository_output(repository, OutputValidationPolicy()) + + assert context.changed_paths == ("src/example.py",) + + +@pytest.mark.parametrize("path", [".github/workflows/release.yml", "CODEOWNERS"]) +def test_rejects_protected_path(repository: Path, path: str) -> None: + _commit(repository, path, "unsafe\n") + + with pytest.raises(OutputValidationError, match="protected path changed"): + validate_repository_output( + repository, + OutputValidationPolicy(protected_paths=(".github/workflows/**", "CODEOWNERS")), + ) + + +def test_rejects_symlink_output(repository: Path) -> None: + (repository / "escape").symlink_to("/etc/passwd") + _run(repository, "add", "escape") + _run(repository, "commit", "-m", "add link") + + with pytest.raises(OutputValidationError, match="symbolic link"): + validate_repository_output(repository, OutputValidationPolicy()) + + +def test_rejects_oversized_file(repository: Path) -> None: + _commit(repository, "large.txt", "12345") + + with pytest.raises(OutputValidationError, match="file exceeds 4 bytes"): + validate_repository_output(repository, OutputValidationPolicy(max_file_bytes=4)) + + +def test_rejects_oversized_combined_output(repository: Path) -> None: + _commit(repository, "one.txt", "123") + _commit(repository, "two.txt", "456") + + with pytest.raises(OutputValidationError, match="changed output exceeds 5 bytes"): + validate_repository_output(repository, OutputValidationPolicy(max_total_bytes=5)) + + +def test_fails_closed_when_remote_default_branch_is_unknown(repository: Path) -> None: + _commit(repository, "safe.txt", "safe") + _run(repository, "symbolic-ref", "--delete", "refs/remotes/origin/HEAD") + + with pytest.raises(OutputValidationError, match="default branch is unavailable"): + validate_repository_output(repository, OutputValidationPolicy()) + + +def test_runs_additional_validator_after_safe_path_checks(repository: Path) -> None: + _commit(repository, "src/example.py", "safe") + + class RecordingValidator: + name = "secret_scanner" + + def __init__(self) -> None: + self.paths: tuple[str, ...] = () + + def validate(self, context) -> None: + self.paths = context.changed_paths + + validator = RecordingValidator() + validate_repository_output(repository, OutputValidationPolicy(), (validator,)) + + assert validator.paths == ("src/example.py",) + + +def test_wraps_unexpected_validator_failure_as_fail_closed(repository: Path) -> None: + _commit(repository, "src/example.py", "safe") + + class BrokenValidator: + name = "broken" + + def validate(self, context) -> None: + raise RuntimeError("scanner unavailable") + + with pytest.raises(OutputValidationError, match="scanner unavailable"): + validate_repository_output(repository, OutputValidationPolicy(), (BrokenValidator(),)) From ca8b31acf811a427079fb21db1d627c1de2ef2e8 Mon Sep 17 00:00:00 2001 From: eshulman2 Date: Thu, 6 Aug 2026 20:09:56 +0300 Subject: [PATCH 2/2] fix: harden safe-output diff parsing --- src/forge/workspace/output_validation.py | 25 +++++++++++++------ .../test_git_ops_output_validation.py | 14 ++++++----- .../unit/workspace/test_output_validation.py | 10 +++++++- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/forge/workspace/output_validation.py b/src/forge/workspace/output_validation.py index b2e7d9c90..64ab99894 100644 --- a/src/forge/workspace/output_validation.py +++ b/src/forge/workspace/output_validation.py @@ -141,17 +141,26 @@ def _default_base_ref(repo_path: Path) -> str: def _changed_entries(repo_path: Path, base_ref: str, head_ref: str) -> list[tuple[str, str, str]]: output = _git( - repo_path, "diff", "--raw", "--no-renames", "--diff-filter=ACDMRTUXB", base_ref, head_ref + repo_path, + "diff", + "--raw", + "-z", + "--no-renames", + "--diff-filter=ACDMRTUXB", + base_ref, + head_ref, ) entries: list[tuple[str, str, str]] = [] - for line in output.splitlines(): - header, separator, path = line.partition("\t") - if not separator: + fields = output.split("\0") + if fields[-1:] == [""]: + fields.pop() + if len(fields) % 2: + raise OutputValidationError("Malformed Git diff metadata; push blocked") + for header, path in zip(fields[::2], fields[1::2], strict=True): + metadata = header.split() + if len(metadata) != 5: raise OutputValidationError("Malformed Git diff metadata; push blocked") - fields = header.split() - if len(fields) != 5: - raise OutputValidationError("Malformed Git diff metadata; push blocked") - old_mode, new_mode, _old_sha, _new_sha, status = fields + old_mode, new_mode, _old_sha, _new_sha, status = metadata entries.append((status[0], old_mode if status[0] == "D" else new_mode, path)) return entries diff --git a/tests/unit/workspace/test_git_ops_output_validation.py b/tests/unit/workspace/test_git_ops_output_validation.py index 4517d8c4d..fe6d031dd 100644 --- a/tests/unit/workspace/test_git_ops_output_validation.py +++ b/tests/unit/workspace/test_git_ops_output_validation.py @@ -23,12 +23,14 @@ def test_push_methods_validate_before_running_git(tmp_path: Path, method: str) - git = _operations(tmp_path) git._run_git = MagicMock() - with patch( - "forge.workspace.git_ops.validate_repository_output", - side_effect=OutputValidationError("blocked"), - ) as validate: - with pytest.raises(OutputValidationError, match="blocked"): - getattr(git, method)() + with ( + pytest.raises(OutputValidationError, match="blocked"), + patch( + "forge.workspace.git_ops.validate_repository_output", + side_effect=OutputValidationError("blocked"), + ) as validate, + ): + getattr(git, method)() validate.assert_called_once() git._run_git.assert_not_called() diff --git a/tests/unit/workspace/test_output_validation.py b/tests/unit/workspace/test_output_validation.py index c3f00ee94..7108200b6 100644 --- a/tests/unit/workspace/test_output_validation.py +++ b/tests/unit/workspace/test_output_validation.py @@ -53,6 +53,14 @@ def test_accepts_bounded_regular_output(repository: Path) -> None: assert context.changed_paths == ("src/example.py",) +def test_handles_tab_in_changed_filename_without_git_quoting(repository: Path) -> None: + _commit(repository, "src/tab\tname.py", "safe\n") + + context = validate_repository_output(repository, OutputValidationPolicy()) + + assert context.changed_paths == ("src/tab\tname.py",) + + @pytest.mark.parametrize("path", [".github/workflows/release.yml", "CODEOWNERS"]) def test_rejects_protected_path(repository: Path, path: str) -> None: _commit(repository, path, "unsafe\n") @@ -120,7 +128,7 @@ def test_wraps_unexpected_validator_failure_as_fail_closed(repository: Path) -> class BrokenValidator: name = "broken" - def validate(self, context) -> None: + def validate(self, _context) -> None: raise RuntimeError("scanner unavailable") with pytest.raises(OutputValidationError, match="scanner unavailable"):