diff --git a/.github/workflows/pr-review-autofix.yml b/.github/workflows/pr-review-autofix.yml index 786357722..acafaea91 100644 --- a/.github/workflows/pr-review-autofix.yml +++ b/.github/workflows/pr-review-autofix.yml @@ -463,6 +463,32 @@ jobs: --snapshot "$ordinary_scope_snapshot" \ --allowed-paths "$allowed_paths_zlist" + - name: Reject protected security-contract deletions and renames + run: | + set -euo pipefail + cd "$TARGET_WORKSPACE" + # Security-contract files may be edited only when a review explicitly + # names them, but an autofix must never delete or rename them. This + # keeps an unrelated optimization from removing origin validation, + # its regression evidence, or the standards record. + protected_security_paths=( + "backend/core/local_http.py" + "backend/core/url_validation.py" + "backend/tests/test_local_http.py" + "backend/tests/test_url_validation.py" + "docs/doctoring/local-http-origin-port-validation.md" + ) + for protected_path in "${protected_security_paths[@]}"; do + while IFS=$'\t' read -r status _; do + case "$status" in + D|R*) + echo "::error::Autofix cannot delete or rename protected security-contract path: $protected_path" + exit 1 + ;; + esac + done < <(git diff HEAD --name-status -- "$protected_path") + done + - name: Validate changed files if: env.RESOLVE_CONFLICT != 'true' run: | @@ -494,6 +520,7 @@ jobs: exit 1 fi done + changed_python_files=() changed_workflows=() for changed_file in "${changed_files[@]}"; do @@ -640,6 +667,27 @@ jobs: --allowed-paths "$conflicted_paths_file" fi + # Conflict resolution edits happen after the ordinary autofix guard; + # re-check the protected security contract immediately before staging + # so conflict-mode deletion and rename attempts also fail closed. + protected_security_paths=( + "backend/core/local_http.py" + "backend/core/url_validation.py" + "backend/tests/test_local_http.py" + "backend/tests/test_url_validation.py" + "docs/doctoring/local-http-origin-port-validation.md" + ) + for protected_path in "${protected_security_paths[@]}"; do + while IFS=$'\t' read -r status _; do + case "$status" in + D|R*) + echo "::error::Conflict resolution cannot delete or rename protected security-contract path: $protected_path" + exit 1 + ;; + esac + done < <(git diff HEAD --name-status -- "$protected_path") + done + # Fail closed: never push unresolved conflict markers. git add -A marker_report="$(git diff --cached --check 2>&1 || true)" diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 2360fdb26..f0b1af470 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1319,6 +1319,35 @@ def test_autofix_worker_resolves_merge_conflicts_fail_closed(): ) assert 'git push origin "HEAD:${PR_HEAD_REF}"' not in worker + for protected_path in ( + "backend/core/local_http.py", + "backend/core/url_validation.py", + "backend/tests/test_local_http.py", + "backend/tests/test_url_validation.py", + "docs/doctoring/local-http-origin-port-validation.md", + ): + assert protected_path in worker + assert "Autofix cannot delete or rename protected security-contract path" in worker + assert ( + "Conflict resolution cannot delete or rename protected security-contract path" + in worker + ) + assert "- name: Reject protected security-contract deletions and renames" in worker + protected_step = worker.split( + "- name: Reject protected security-contract deletions and renames", 1 + )[1].split("- name: Validate changed files", 1)[0] + assert "if: env.RESOLVE_CONFLICT" not in protected_step + assert "git diff HEAD --name-status -- \"$protected_path\"" in protected_step + assert "git diff --name-status -- \"$protected_path\"" not in protected_step + conflict_step = worker.split( + "- name: Merge base branch and resolve conflicts with OpenCode", 1 + )[1] + conflict_guard = conflict_step.split( + "# Fail closed: never push unresolved conflict markers.", 1 + )[0] + assert "Conflict resolution cannot delete or rename protected security-contract path" in conflict_guard + assert 'git diff HEAD --name-status -- "$protected_path"' in conflict_guard + # The fix scheduler dispatches the mode only for approved conflicting PRs. scheduler = Path("scripts/ci/pr_review_fix_scheduler.py").read_text( encoding="utf-8" diff --git a/tests/test_pr_review_conflict_scope.py b/tests/test_pr_review_conflict_scope.py index f770371ec..bfd2688de 100644 --- a/tests/test_pr_review_conflict_scope.py +++ b/tests/test_pr_review_conflict_scope.py @@ -338,5 +338,10 @@ def test_workflow_snapshots_after_merge_and_verifies_before_staging() -> None: conflict_add = conflict.index("# Fail closed: never push unresolved conflict markers.") assert merge < snapshot < model < verify < conflict_add + assert ( + "Conflict resolution cannot delete or rename protected security-contract path" + in conflict[:conflict_add] + ) + assert 'git diff HEAD --name-status -- "$protected_path"' in conflict[:conflict_add] assert 'git diff --name-only -z --diff-filter=U >"$conflicted_paths_file"' in conflict assert '--allowed-paths "$conflicted_paths_file"' in conflict