Skip to content

Commit a4ab14f

Browse files
Pigbibicodex
andcommitted
feat(gate): require base-approved exact deletions
Co-Authored-By: Codex <noreply@openai.com>
1 parent 5eeea47 commit a4ab14f

3 files changed

Lines changed: 77 additions & 3 deletions

File tree

.github/codex_auto_merge_policy.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@
2323
}
2424
},
2525
"max_changed_files": 30,
26-
"max_changed_lines": 2000,
26+
"max_changed_lines": 3000,
27+
"approved_deleted_paths": [
28+
".github/workflows/codex_pr_review.yml",
29+
"prompts/pr_review.md",
30+
"scripts/run_codex_pr_review.py",
31+
"tests/test_run_codex_pr_review.py"
32+
],
2733
"pr_review": {
2834
"enabled": true,
2935
"block_severities": [

scripts/gate_codex_app_review_static.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import json
99
import re
10-
from pathlib import Path
10+
from pathlib import Path, PurePosixPath
1111
from typing import Any
1212

1313
DEFAULT_POLICY_PATH = Path(".github/codex_auto_merge_policy.json")
@@ -72,14 +72,32 @@ def scan_diff(diff_text: str, path_patterns: list[re.Pattern[str]]) -> list[str]
7272

7373
def check_metadata(files: list[dict[str, Any]], policy: dict[str, Any]) -> list[str]:
7474
issues: list[str] = []
75+
approved_deleted_paths: set[str] = set()
76+
configured_approvals = policy.get("approved_deleted_paths", [])
77+
if isinstance(configured_approvals, list):
78+
for value in configured_approvals:
79+
if not isinstance(value, str) or value != value.strip():
80+
continue
81+
path = PurePosixPath(value)
82+
if (
83+
not value
84+
or path.is_absolute()
85+
or path.as_posix() != value
86+
or ".." in path.parts
87+
or "\\" in value
88+
or any(character in value for character in "*?[]")
89+
):
90+
continue
91+
approved_deleted_paths.add(value)
92+
7593
max_files = policy.get("max_changed_files", 50)
7694
max_lines = policy.get("max_changed_lines", 5000)
7795
total_added = sum(f.get("additions", 0) or 0 for f in files)
7896
total_deleted = sum(f.get("deletions", 0) or 0 for f in files)
7997
for f in files:
8098
filename = f.get("filename", "?")
8199
status = (f.get("status") or "").lower().strip()
82-
if status == "removed":
100+
if status == "removed" and filename not in approved_deleted_paths:
83101
issues.append(f"**File deleted**: `{filename}` — verify intentional")
84102
elif status == "renamed":
85103
issues.append(f"**File renamed**: `{f.get('previous_filename', '?')}` → `{filename}`")

tests/test_gate_codex_app_review.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,56 @@ def test_collect_static_gate_issues_aggregates_metadata_and_diff(self) -> None:
107107
self.assertTrue(any("Hardcoded secret" in issue for issue in issues))
108108
self.assertTrue(any("Too many lines" in issue for issue in issues))
109109

110+
def test_check_metadata_allows_only_exact_base_approved_deletions(self) -> None:
111+
files = [
112+
{
113+
"filename": "scripts/retired.py",
114+
"status": "removed",
115+
"additions": 0,
116+
"deletions": 10,
117+
},
118+
{
119+
"filename": "scripts/unapproved.py",
120+
"status": "removed",
121+
"additions": 0,
122+
"deletions": 10,
123+
},
124+
]
125+
policy = {
126+
"approved_deleted_paths": ["scripts/retired.py"],
127+
"max_changed_files": 10,
128+
"max_changed_lines": 100,
129+
}
130+
131+
issues = gate_codex_app_review_static.check_metadata(files, policy)
132+
133+
self.assertFalse(any("scripts/retired.py" in issue for issue in issues))
134+
self.assertTrue(any("scripts/unapproved.py" in issue for issue in issues))
135+
136+
def test_check_metadata_rejects_unsafe_delete_approvals(self) -> None:
137+
files = [
138+
{
139+
"filename": "scripts/retired.py",
140+
"status": "removed",
141+
"additions": 0,
142+
"deletions": 10,
143+
},
144+
]
145+
for approval in (
146+
"scripts/*.py",
147+
"../scripts/retired.py",
148+
"/scripts/retired.py",
149+
"scripts\\retired.py",
150+
):
151+
with self.subTest(approval=approval):
152+
policy = {
153+
"approved_deleted_paths": [approval],
154+
"max_changed_files": 10,
155+
"max_changed_lines": 100,
156+
}
157+
issues = gate_codex_app_review_static.check_metadata(files, policy)
158+
self.assertTrue(any("scripts/retired.py" in issue for issue in issues))
159+
110160

111161
if __name__ == "__main__":
112162
unittest.main()

0 commit comments

Comments
 (0)