Skip to content

Commit c1c1e7e

Browse files
Pigbibicodex
andcommitted
fix: fail closed automation policy loading
Co-Authored-By: Codex <noreply@openai.com>
1 parent f5fe332 commit c1c1e7e

7 files changed

Lines changed: 124 additions & 15 deletions

docs/async_service_deployment.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ bash scripts/deploy_codex_audit_service.sh deploy
7272

7373
The job directory should be owned by the service user and mode `0700`.
7474
The deploy script points `CODEX_AUDIT_SERVICE_EXECUTION_POLICY_PATH` to
75-
`${CODEX_AUDIT_SERVICE_JOB_DIR}/execution_policy.json`. If present, this
76-
service-owned file can cap repo autonomy without trusting the reviewed checkout:
75+
`${CODEX_AUDIT_SERVICE_JOB_DIR}/execution_policy.json` and creates a conservative
76+
default file if it is missing. This service-owned file can cap repo autonomy
77+
without trusting the reviewed checkout:
7778

7879
```json
7980
{
@@ -91,6 +92,9 @@ service-owned file can cap repo autonomy without trusting the reviewed checkout:
9192
}
9293
```
9394

95+
If this configured file later becomes unreadable or malformed, the service
96+
fails closed for execution decisions until the file is repaired.
97+
9498
The service should rely on an authenticated Codex CLI session and must not
9599
inject OpenAI/Codex API keys into the Codex subprocess.
96100
With `CODEX_AUDIT_SERVICE_CODEX_ACCOUNT_USAGE=1`, `/v1/ai/quota` includes a

scripts/deploy_codex_audit_service.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,28 @@ write_admin_env_file_if_needed() {
214214
trap - RETURN
215215
}
216216

217+
write_default_execution_policy_if_missing() {
218+
local owner="$1"
219+
local policy_path="${JOB_DIR}/execution_policy.json"
220+
if [ -e "$policy_path" ]; then
221+
return
222+
fi
223+
local tmp
224+
tmp="$(mktemp)"
225+
cat >"$tmp" <<'EOF_POLICY'
226+
{
227+
"default": {
228+
"max_autonomy": "auto_pr",
229+
"max_consecutive_failures": 3,
230+
"low_cost_model": "gpt-5.4-mini"
231+
},
232+
"repositories": {}
233+
}
234+
EOF_POLICY
235+
sudo install -m 0600 -o "$owner" -g "$owner" "$tmp" "$policy_path"
236+
rm -f "$tmp"
237+
}
238+
217239
write_audit_service_unit() {
218240
local runner_user runner_home
219241
runner_user="$(id -un)"
@@ -510,6 +532,7 @@ deploy() {
510532
install_file "scripts/codex_audit_service.py" "${DEPLOY_DIR}/scripts/codex_audit_service.py" "0755"
511533
install_service_package
512534
sudo install -d -m 0700 -o "$runner_user" -g "$runner_user" "$JOB_DIR"
535+
write_default_execution_policy_if_missing "$runner_user"
513536
write_admin_env_file_if_needed
514537
write_audit_service_unit
515538
write_managed_audit_service_dropin

service/ai_gateway_service.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
from service.auth import authenticate
3737
from service.contracts import (
38+
MODE_REVIEW_AND_FIX,
3839
MODE_REVIEW_ONLY,
3940
TASK_ANALYZE,
4041
TASK_EXECUTE,
@@ -508,7 +509,7 @@ def _public_job_payload(job: dict[str, Any]) -> dict[str, object]:
508509
return payload
509510

510511

511-
def _automation_control_snapshot(repo: str, *, task_name: str = "", requested_mode: str = MODE_REVIEW_ONLY) -> dict[str, Any]:
512+
def _automation_control_snapshot(repo: str, *, task_name: str = "", requested_mode: str = MODE_REVIEW_AND_FIX) -> dict[str, Any]:
512513
try:
513514
org_health = read_org_health()
514515
except Exception:
@@ -520,8 +521,10 @@ def _automation_control_snapshot(repo: str, *, task_name: str = "", requested_mo
520521
control = suggest_control_action(get_health_monitor().status, quota_status, org_health)
521522
try:
522523
recent_runs = get_automation_run_ledger().snapshot(limit=None)["runs"]
524+
ledger_unavailable = False
523525
except Exception:
524526
recent_runs = []
527+
ledger_unavailable = True
525528
execution = decide_automation_execution(
526529
repo=repo or "unknown",
527530
task_name=task_name,
@@ -533,6 +536,14 @@ def _automation_control_snapshot(repo: str, *, task_name: str = "", requested_mo
533536
recent_runs=recent_runs,
534537
policy=load_execution_policy(),
535538
)
539+
if ledger_unavailable:
540+
execution["action"] = EXECUTION_HUMAN_REVIEW
541+
execution["effective_mode"] = MODE_REVIEW_ONLY
542+
execution["human_review_required"] = True
543+
execution["auto_fix_allowed"] = False
544+
reasons = execution.get("reasons") if isinstance(execution.get("reasons"), list) else []
545+
reasons.append("automation ledger unavailable; forcing human review")
546+
execution["reasons"] = reasons
536547
original_action = str(control.get("action") or CONTROL_REVIEW_ONLY)
537548
strict_action = original_action
538549
if execution.get("action") == EXECUTION_HUMAN_REVIEW:
@@ -1459,7 +1470,7 @@ def _handle_automation_control(self) -> None:
14591470
else:
14601471
repo = claims_repo
14611472
repo = repo or "unknown"
1462-
mode = str(params.get("mode", [MODE_REVIEW_ONLY])[0] or MODE_REVIEW_ONLY)
1473+
mode = str(params.get("mode", [MODE_REVIEW_AND_FIX])[0] or MODE_REVIEW_AND_FIX)
14631474
_json_response(self, HTTPStatus.OK, {"status": "ok", "control": _automation_control_snapshot(repo, requested_mode=mode)})
14641475

14651476
def _handle_automation_triage(self, claims: dict[str, Any], payload: dict[str, Any]) -> None:
@@ -1541,7 +1552,7 @@ def _handle_record_automation_run(self, claims: dict[str, Any], payload: dict[st
15411552
control = _automation_control_snapshot(
15421553
repo,
15431554
task_name=str(payload.get("task") or payload.get("task_name") or ""),
1544-
requested_mode=str(payload.get("mode") or MODE_REVIEW_ONLY),
1555+
requested_mode=str(payload.get("mode") or MODE_REVIEW_AND_FIX),
15451556
)
15461557
metadata = payload.get("metadata") if isinstance(payload.get("metadata"), dict) else {}
15471558
ledger = get_automation_run_ledger()

service/automation_decision.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
DEFAULT_MAX_CONSECUTIVE_FAILURES = 3
3030
DEFAULT_LOW_COST_MODEL = "gpt-5.4-mini"
3131
EXECUTION_POLICY_PATH_ENV = "CODEX_AUDIT_SERVICE_EXECUTION_POLICY_PATH"
32+
POLICY_LOAD_ERROR_KEY = "_load_error"
3233
QUOTA_STATUS_SEVERITY = {
3334
"ok": 0,
3435
"healthy": 0,
@@ -83,18 +84,32 @@ def _repo_from_run(run: dict[str, Any]) -> str:
8384
return str(metadata.get("source_repository") or metadata.get("repository") or "")
8485

8586

87+
def _fail_closed_policy(reason: str) -> dict[str, Any]:
88+
return {
89+
POLICY_LOAD_ERROR_KEY: reason,
90+
"default": {
91+
"max_autonomy": AUTONOMY_MANUAL,
92+
"max_consecutive_failures": 1,
93+
},
94+
}
95+
96+
8697
def load_execution_policy(path: Path | None = None) -> dict[str, Any]:
8798
"""Load service-owned execution policy for repo autonomy thresholds."""
8899
if path is None:
89100
configured = os.environ.get(EXECUTION_POLICY_PATH_ENV, "").strip()
90101
if not configured:
91102
return {}
92103
path = Path(configured).expanduser()
104+
if not path.exists():
105+
return _fail_closed_policy("execution policy file is unavailable")
93106
try:
94107
payload = json.loads(path.read_text(encoding="utf-8"))
95108
except (OSError, json.JSONDecodeError):
96-
return {}
97-
return payload if isinstance(payload, dict) else {}
109+
return _fail_closed_policy("execution policy file is unreadable")
110+
if not isinstance(payload, dict):
111+
return _fail_closed_policy("execution policy file is invalid")
112+
return payload
98113

99114

100115
def repo_execution_policy(repo: str, policy: dict[str, Any] | None = None) -> dict[str, Any]:
@@ -146,6 +161,7 @@ def decide_automation_execution(
146161
) -> dict[str, Any]:
147162
"""Produce a safe execution decision from health, quota, failures, and repo policy."""
148163
repo_policy = repo_execution_policy(repo, policy)
164+
policy_load_error = str((policy or {}).get(POLICY_LOAD_ERROR_KEY) or "") if isinstance(policy, dict) else ""
149165
max_autonomy, autonomy_config_error = _parse_autonomy(repo_policy.get("max_autonomy"), AUTONOMY_AUTO_PR)
150166
max_failures = _safe_positive_int(repo_policy.get("max_consecutive_failures"), DEFAULT_MAX_CONSECUTIVE_FAILURES)
151167
low_cost_model = str(repo_policy.get("low_cost_model") or DEFAULT_LOW_COST_MODEL)
@@ -170,6 +186,8 @@ def decide_automation_execution(
170186
reasons.append(f"repo max autonomy is {max_autonomy}")
171187
if autonomy_config_error:
172188
reasons.append(autonomy_config_error)
189+
if policy_load_error:
190+
reasons.append(policy_load_error)
173191
if max_autonomy == AUTONOMY_MANUAL:
174192
action = EXECUTION_HUMAN_REVIEW
175193

@@ -199,16 +217,18 @@ def decide_automation_execution(
199217
if quota in {"low", "constrained"}:
200218
effective_model = effective_model or low_cost_model or recommend_model(0.0)
201219
if quota_low_behavior == "defer":
202-
action = EXECUTION_DEFER
203-
defer = True
220+
if action != EXECUTION_HUMAN_REVIEW:
221+
action = EXECUTION_DEFER
222+
defer = True
204223
effective_mode = MODE_REVIEW_ONLY
205224
human_review_required = True
206225
reasons.append(f"quota status is {quota}; deferring automation")
207226
else:
208227
reasons.append(f"quota status is {quota}; recommending low-cost model")
209228
elif quota in {"exhausted", "blocked"}:
210-
action = EXECUTION_DEFER
211-
defer = True
229+
if action != EXECUTION_HUMAN_REVIEW:
230+
action = EXECUTION_DEFER
231+
defer = True
212232
effective_mode = MODE_REVIEW_ONLY
213233
human_review_required = True
214234
reasons.append(f"quota status is {quota}; deferring automation")

tests/test_ai_gateway_automation_control.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,24 @@
1313

1414

1515
class TestAutomationControlSnapshot(unittest.TestCase):
16+
def test_control_snapshot_preserves_continue_for_healthy_default_mode(self) -> None:
17+
health = type("Health", (), {"status": "healthy"})()
18+
quota = type("Quota", (), {"runtime_status": lambda self, repo: {"status": "ok"}})()
19+
ledger = type("Ledger", (), {"snapshot": lambda self, limit=None: {"runs": []}})()
20+
21+
with (
22+
patch("service.ai_gateway_service.read_org_health", return_value={"status": "ok"}),
23+
patch("service.ai_gateway_service.get_health_monitor", return_value=health),
24+
patch("service.ai_gateway_service.get_quota_manager", return_value=quota),
25+
patch("service.ai_gateway_service.get_automation_run_ledger", return_value=ledger),
26+
patch("service.ai_gateway_service.load_execution_policy", return_value={}),
27+
):
28+
control = _automation_control_snapshot("QuantStrategyLab/TargetRepo")
29+
30+
self.assertEqual(control["action"], "continue")
31+
self.assertEqual(control["execution"]["effective_mode"], "review_and_fix")
32+
self.assertTrue(control["execution"]["auto_fix_allowed"])
33+
1634
def test_control_snapshot_applies_service_owned_execution_policy(self) -> None:
1735
with TemporaryDirectory() as tmp:
1836
policy_path = Path(tmp) / "execution_policy.json"
@@ -95,6 +113,23 @@ def snapshot(self, limit=100):
95113
self.assertEqual(control["execution"]["action"], "human_review")
96114
self.assertEqual(control["execution"]["consecutive_failures"], 2)
97115

116+
def test_control_snapshot_fails_closed_when_ledger_is_unavailable(self) -> None:
117+
health = type("Health", (), {"status": "healthy"})()
118+
quota = type("Quota", (), {"runtime_status": lambda self, repo: {"status": "ok"}})()
119+
ledger = type("Ledger", (), {"snapshot": lambda self, limit=None: (_ for _ in ()).throw(RuntimeError("boom"))})()
120+
121+
with (
122+
patch("service.ai_gateway_service.read_org_health", return_value={"status": "ok"}),
123+
patch("service.ai_gateway_service.get_health_monitor", return_value=health),
124+
patch("service.ai_gateway_service.get_quota_manager", return_value=quota),
125+
patch("service.ai_gateway_service.get_automation_run_ledger", return_value=ledger),
126+
patch("service.ai_gateway_service.load_execution_policy", return_value={}),
127+
):
128+
control = _automation_control_snapshot("QuantStrategyLab/TargetRepo")
129+
130+
self.assertEqual(control["action"], "escalate")
131+
self.assertEqual(control["execution"]["action"], "human_review")
132+
98133

99134
if __name__ == "__main__":
100135
unittest.main()

tests/test_automation_decision.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ def test_degraded_health_forces_review_only(self) -> None:
4949
self.assertFalse(result["auto_fix_allowed"])
5050
self.assertTrue(result["human_review_required"])
5151

52-
def test_exhausted_quota_defers_execution(self) -> None:
52+
def test_exhausted_quota_defers_execution_without_stronger_guard(self) -> None:
5353
result = decide_automation_execution(
5454
repo="QuantStrategyLab/AIAuditBridge",
5555
requested_mode=MODE_REVIEW_AND_FIX,
56-
control_action=CONTROL_ESCALATE,
56+
control_action=CONTROL_CONTINUE,
5757
service_health="healthy",
5858
quota_status={"status": "exhausted"},
5959
org_health_status="ok",
@@ -63,6 +63,20 @@ def test_exhausted_quota_defers_execution(self) -> None:
6363
self.assertTrue(result["defer"])
6464
self.assertEqual(result["effective_mode"], MODE_REVIEW_ONLY)
6565

66+
def test_human_review_dominates_exhausted_quota_defer(self) -> None:
67+
result = decide_automation_execution(
68+
repo="QuantStrategyLab/AIAuditBridge",
69+
requested_mode=MODE_REVIEW_AND_FIX,
70+
control_action=CONTROL_ESCALATE,
71+
service_health="healthy",
72+
quota_status={"status": "exhausted"},
73+
org_health_status="ok",
74+
)
75+
76+
self.assertEqual(result["action"], EXECUTION_HUMAN_REVIEW)
77+
self.assertFalse(result["defer"])
78+
self.assertEqual(result["effective_mode"], MODE_REVIEW_ONLY)
79+
6680
def test_low_quota_recommends_low_cost_model(self) -> None:
6781
result = decide_automation_execution(
6882
repo="QuantStrategyLab/AIAuditBridge",
@@ -168,12 +182,12 @@ def test_invalid_failure_threshold_falls_back_safely(self) -> None:
168182
self.assertEqual(result["max_consecutive_failures"], 3)
169183
self.assertEqual(result["action"], EXECUTION_RUN)
170184

171-
def test_load_execution_policy_ignores_malformed_files(self) -> None:
185+
def test_load_execution_policy_fails_closed_for_malformed_files(self) -> None:
172186
with TemporaryDirectory() as tmp:
173187
path = Path(tmp) / "policy.json"
174188
path.write_text("not-json", encoding="utf-8")
175189

176-
self.assertEqual(load_execution_policy(path), {})
190+
self.assertEqual(load_execution_policy(path)["default"]["max_autonomy"], "manual")
177191

178192
path.write_text(json.dumps({"default": {"max_autonomy": "review_only"}}), encoding="utf-8")
179193
self.assertEqual(load_execution_policy(path)["default"]["max_autonomy"], "review_only")

tests/test_run_monthly_codex_audit.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,6 +2343,8 @@ def test_vps_deploy_adds_nginx_audit_route_without_router_service(self) -> None:
23432343
self.assertIn("location ^~ /v1/codex-audit/", deploy_script)
23442344
self.assertIn("CODEX_AUDIT_SERVICE_JOB_DIR", deploy_script)
23452345
self.assertIn("CODEX_AUDIT_SERVICE_EXECUTION_POLICY_PATH", deploy_script)
2346+
self.assertIn("write_default_execution_policy_if_missing", deploy_script)
2347+
self.assertIn('"max_consecutive_failures": 3', deploy_script)
23462348
self.assertIn("codex_pr_review.yml@refs/pull/*/merge", deploy_script)
23472349
self.assertIn("refs/pull/*/merge", deploy_script)
23482350
self.assertIn("QuantStrategyLab/AIAuditBridge,QuantStrategyLab/CryptoLivePoolPipelines", deploy_script)

0 commit comments

Comments
 (0)