Skip to content

Commit 15eb5f3

Browse files
doctor: Enforce Codex project-hook discovery policy
Prevent structurally current Codex project registrations from appearing healthy when the effective managed-only policy excludes project hooks. Probe Codex's composed policy once per doctor invocation, classify blocked and unknown policy separately from trust, reuse the result across registrations and reports, and keep repairs limited to structural drift. Plan: codex-cli-integration.md — T22 (follow-up correctness fix #3) Co-authored-by: SCE <sce@crocoder.dev>
1 parent e82fb2d commit 15eb5f3

13 files changed

Lines changed: 1435 additions & 71 deletions

File tree

cli/src/services/codex_hook_policy.rs

Lines changed: 614 additions & 0 deletions
Large diffs are not rendered by default.

cli/src/services/codex_hook_trust.rs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
1-
//! Read-only diagnosis of Codex's own hook-trust bookkeeping for SCE-owned
2-
//! `.codex/hooks.json` registrations.
1+
//! Read-only diagnosis of Codex's own per-handler hook-*trust* bookkeeping
2+
//! (enabled / `trusted_hash`) for SCE-owned `.codex/hooks.json`
3+
//! registrations.
4+
//!
5+
//! This is deliberately only one of two independent dimensions Codex
6+
//! requires before it will actually execute a project hook handler. This
7+
//! module answers "given an eligible hook *source*, has this handler been
8+
//! enabled and durably trusted?" — it says nothing about whether Codex
9+
//! considers the *source* (SCE's project `.codex/hooks.json`) eligible at
10+
//! all. That second dimension is effective hook-discovery *policy*
11+
//! (`allow_managed_hooks_only`), owned entirely by `codex_hook_policy`. A
12+
//! project registration is only executable when both are satisfied:
13+
//! structurally current, policy-eligible, *and* trusted. See
14+
//! `codex_hook_policy`'s module documentation for why policy cannot be
15+
//! determined by reading any file this module could read, and for the
16+
//! upstream discovery-policy source references.
317
//!
418
//! Mirrors current upstream `openai/codex` (commit
519
//! `8e649e3afa5cdddfb09a1b85a090b94775045d9b`):
@@ -45,8 +59,13 @@ const EVENTS_SUPPORTING_ADDITIONAL_CONTEXT: [&str; 4] = [
4559
/// project-owned (non-managed) handlers in `.codex/hooks.json`.
4660
#[derive(Clone, Debug, Eq, PartialEq)]
4761
pub(crate) enum TrustReadiness {
48-
/// Enabled and `trusted_hash` matches the handler's current hash: Codex
49-
/// will execute this handler.
62+
/// Enabled and `trusted_hash` matches the handler's current hash: this
63+
/// handler is durably trusted. This alone does **not** mean Codex will
64+
/// execute it — `Trusted` says nothing about whether Codex's
65+
/// hook-discovery policy considers this project handler's *source*
66+
/// eligible at all (see `codex_hook_policy`). The accurate reading is
67+
/// "the current non-managed handler is enabled and durably trusted,
68+
/// assuming Codex policy permits this hook source."
5069
Trusted,
5170
/// Enabled but no `trusted_hash` is recorded for this handler yet.
5271
Untrusted,
@@ -208,7 +227,7 @@ fn state_key(
208227
/// [<one normalized handler>]}` shape, canonicalize (recursively sort object
209228
/// keys, matching `fingerprint::canonical_json`), and SHA-256 the compact
210229
/// JSON encoding.
211-
fn hash_command_handler(
230+
pub(crate) fn hash_command_handler(
212231
event: &str,
213232
matcher: Option<&str>,
214233
handler: &Value,

cli/src/services/doctor/inspect.rs

Lines changed: 596 additions & 50 deletions
Large diffs are not rendered by default.

cli/src/services/doctor/mod.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::process::Command;
55
use anyhow::{Context, Result};
66

77
use crate::app::{ContextWithRepoRoot, HasRepoRoot};
8+
use crate::services::codex_hook_policy;
89
use crate::services::default_paths::{resolve_sce_default_locations, resolve_state_data_root};
910
use crate::services::lifecycle::{
1011
lifecycle_providers, FixOutcome, HealthCategory, HealthFixability, HealthProblem,
@@ -52,6 +53,13 @@ struct DoctorDependencies<'a> {
5253
resolve_state_root: &'a dyn Fn() -> Result<PathBuf>,
5354
resolve_global_config_path: &'a dyn Fn() -> Result<PathBuf>,
5455
validate_config_file: &'a dyn Fn(&Path) -> Result<()>,
56+
/// Probes Codex's effective hook-discovery policy
57+
/// (`allow_managed_hooks_only`). Invoked exactly once per doctor
58+
/// invocation (see `execute_doctor_with_lifecycle_providers`) and reused
59+
/// for every Codex integration inspection within that invocation —
60+
/// initial report, `--fix`, and final report alike — never once per
61+
/// registration.
62+
probe_codex_hook_policy: &'a dyn Fn() -> codex_hook_policy::CodexHookPolicyReadiness,
5563
}
5664

5765
struct DoctorExecution {
@@ -98,6 +106,7 @@ fn execute_doctor_with_context(
98106
Ok(resolve_sce_default_locations()?.global_config_file())
99107
},
100108
validate_config_file: &crate::services::config::validate_config_file,
109+
probe_codex_hook_policy: &codex_hook_policy::probe_default,
101110
},
102111
)
103112
}
@@ -108,6 +117,11 @@ fn execute_doctor_with_lifecycle_providers(
108117
context: &impl HasRepoRoot,
109118
dependencies: &DoctorDependencies<'_>,
110119
) -> DoctorExecution {
120+
// Probed exactly once per doctor invocation, then reused for every
121+
// Codex integration inspection below (initial report, `--fix`, and final
122+
// report alike) instead of once per registration or once per report.
123+
let policy_readiness = (dependencies.probe_codex_hook_policy)();
124+
111125
let providers = lifecycle_providers(true);
112126
let initial_problems = diagnose_lifecycle_providers(context, &providers);
113127
let initial_doctor_problems = initial_problems
@@ -119,6 +133,7 @@ fn execute_doctor_with_lifecycle_providers(
119133
repository_root,
120134
dependencies,
121135
initial_doctor_problems,
136+
&policy_readiness,
122137
);
123138

124139
if request.mode != DoctorMode::Fix {
@@ -129,7 +144,10 @@ fn execute_doctor_with_lifecycle_providers(
129144
}
130145

131146
let mut fix_results = fix_lifecycle_providers(context, &providers, &initial_problems);
132-
fix_results.extend(repair_merge_target_configs(repository_root));
147+
fix_results.extend(repair_merge_target_configs(
148+
repository_root,
149+
&policy_readiness,
150+
));
133151
let final_problems = diagnose_lifecycle_providers(context, &providers);
134152
let final_doctor_problems = final_problems
135153
.into_iter()
@@ -140,6 +158,7 @@ fn execute_doctor_with_lifecycle_providers(
140158
repository_root,
141159
dependencies,
142160
final_doctor_problems,
161+
&policy_readiness,
143162
);
144163
fix_results.extend(build_manual_fix_results(&final_report));
145164

@@ -335,6 +354,12 @@ fn doctor_problem_kind(kind: HealthProblemKind) -> ProblemKind {
335354
HealthProblemKind::CodexHookRegistrationNotTrusted => {
336355
ProblemKind::CodexHookRegistrationNotTrusted
337356
}
357+
HealthProblemKind::CodexHookRegistrationPolicyBlocked => {
358+
ProblemKind::CodexHookRegistrationPolicyBlocked
359+
}
360+
HealthProblemKind::CodexHookRegistrationPolicyUnknown => {
361+
ProblemKind::CodexHookRegistrationPolicyUnknown
362+
}
338363
HealthProblemKind::AgentTraceDbConnectionFailed => {
339364
ProblemKind::AgentTraceDbConnectionFailed
340365
}
@@ -403,6 +428,12 @@ fn health_problem_kind(kind: ProblemKind) -> HealthProblemKind {
403428
ProblemKind::CodexHookRegistrationNotTrusted => {
404429
HealthProblemKind::CodexHookRegistrationNotTrusted
405430
}
431+
ProblemKind::CodexHookRegistrationPolicyBlocked => {
432+
HealthProblemKind::CodexHookRegistrationPolicyBlocked
433+
}
434+
ProblemKind::CodexHookRegistrationPolicyUnknown => {
435+
HealthProblemKind::CodexHookRegistrationPolicyUnknown
436+
}
406437
ProblemKind::AgentTraceDbConnectionFailed => {
407438
HealthProblemKind::AgentTraceDbConnectionFailed
408439
}

cli/src/services/doctor/render.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,10 @@ fn integration_group_status(
436436
| IntegrationContentState::Mismatch
437437
| IntegrationContentState::Stale
438438
| IntegrationContentState::Malformed(_)
439-
| IntegrationContentState::ReadFailed(_) => DoctorDisplayStatus::Fail,
440-
IntegrationContentState::NotTrusted(_) => DoctorDisplayStatus::Warn,
439+
| IntegrationContentState::ReadFailed(_)
440+
| IntegrationContentState::PolicyBlocked(_) => DoctorDisplayStatus::Fail,
441+
IntegrationContentState::NotTrusted(_)
442+
| IntegrationContentState::PolicyUnknown(_) => DoctorDisplayStatus::Warn,
441443
})
442444
});
443445
let problem_status = report
@@ -606,6 +608,16 @@ fn render_display_detail(lines: &mut Vec<String>, detail: &DoctorDisplayDetail,
606608
lines.push(format!("{prefix}Path: {}", path.display()));
607609
lines.push(format!("{prefix}Not yet executable by Codex: {reason}"));
608610
}
611+
DoctorDisplayDetail::PolicyBlocked { path, reason } => {
612+
lines.push(format!("{prefix}Path: {}", path.display()));
613+
lines.push(format!("{prefix}Blocked by Codex policy: {reason}"));
614+
}
615+
DoctorDisplayDetail::PolicyUnknown { path, reason } => {
616+
lines.push(format!("{prefix}Path: {}", path.display()));
617+
lines.push(format!(
618+
"{prefix}Codex hook-discovery policy could not be determined: {reason}"
619+
));
620+
}
609621
DoctorDisplayDetail::Problem {
610622
summary,
611623
remediation,

cli/src/services/doctor/types.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@ pub(super) enum IntegrationContentState {
226226
/// Carries a short machine-readable reason (`"untrusted"`, `"modified"`,
227227
/// `"disabled"`, or `"unknown"`).
228228
NotTrusted(String),
229+
/// The registration is structurally current, but Codex's effective
230+
/// hook-discovery policy (`allow_managed_hooks_only = true`) discards
231+
/// every project-owned hook source, so Codex will never even consider
232+
/// this handler — independent of, and checked before, trust readiness.
233+
/// Carries a human-readable explanation.
234+
PolicyBlocked(String),
235+
/// Whether Codex's effective hook-discovery policy allows project hooks
236+
/// could not be determined (no Codex executable, probe failure, timeout,
237+
/// malformed response, etc.). Never treated as healthy: AC28 requires
238+
/// proof Codex will actually execute the registration, and an unknown
239+
/// policy is not proof. Carries a human-readable reason.
240+
PolicyUnknown(String),
229241
}
230242

231243
#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
@@ -271,6 +283,14 @@ pub(super) enum DoctorDisplayDetail {
271283
path: PathBuf,
272284
reason: String,
273285
},
286+
PolicyBlocked {
287+
path: PathBuf,
288+
reason: String,
289+
},
290+
PolicyUnknown {
291+
path: PathBuf,
292+
reason: String,
293+
},
274294
Problem {
275295
summary: String,
276296
remediation: String,
@@ -398,6 +418,20 @@ impl IntegrationChildHealth {
398418
reason: reason.clone(),
399419
}),
400420
),
421+
IntegrationContentState::PolicyBlocked(reason) => (
422+
DoctorDisplayStatus::Fail,
423+
Some(DoctorDisplayDetail::PolicyBlocked {
424+
path: self.path.clone(),
425+
reason: reason.clone(),
426+
}),
427+
),
428+
IntegrationContentState::PolicyUnknown(reason) => (
429+
DoctorDisplayStatus::Warn,
430+
Some(DoctorDisplayDetail::PolicyUnknown {
431+
path: self.path.clone(),
432+
reason: reason.clone(),
433+
}),
434+
),
401435
};
402436
DoctorDisplayNode::asset(self.relative_path.clone(), status, detail)
403437
}
@@ -457,6 +491,8 @@ pub(crate) enum ProblemKind {
457491
CodexAssetReadFailed,
458492
CodexHookRegistrationMalformed,
459493
CodexHookRegistrationNotTrusted,
494+
CodexHookRegistrationPolicyBlocked,
495+
CodexHookRegistrationPolicyUnknown,
460496
AgentTraceDbConnectionFailed,
461497
AgentTraceDbSchemaNotReady,
462498
}

cli/src/services/lifecycle.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ pub enum HealthProblemKind {
6767
CodexAssetReadFailed,
6868
CodexHookRegistrationMalformed,
6969
CodexHookRegistrationNotTrusted,
70+
CodexHookRegistrationPolicyBlocked,
71+
CodexHookRegistrationPolicyUnknown,
7072
AgentTraceDbConnectionFailed,
7173
AgentTraceDbSchemaNotReady,
7274
}

cli/src/services/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub mod bash_policy;
1515
pub mod capabilities;
1616
pub mod checkout;
1717
pub(crate) mod codex_hook_config;
18+
pub(crate) mod codex_hook_policy;
1819
pub(crate) mod codex_hook_trust;
1920
pub mod command_registry;
2021
pub mod completion;

0 commit comments

Comments
 (0)