feat(agent-hosts): auto-trust Codex managed hooks on install and update - #566
Conversation
|
983f261 to
763d105
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 983f2618b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "trusted_hash".to_string(), | ||
| toml::Value::String(entry.hash.clone()), | ||
| ); | ||
| state.insert(entry.trust_key.clone(), toml::Value::Table(record)); |
There was a problem hiding this comment.
Remove managed trust records during Codex uninstall
When an install has inserted these trust records, codex plugin remove deliberately preserves [hooks], but codex_registration_residue counts every remaining tracedecay@… trust key as residue. Consequently uninstall verification sees Repairable instead of its required Missing state, rejects the operation, and rolls it back. Prune the managed trust entries transactionally during deactivation.
AGENTS.md reference: AGENTS.md:L84-L86
Useful? React with 👍 / 👎.
| /// `toml` serializer otherwise emits only `[hooks.state."..."]` child tables, | ||
| /// which parses equivalently but still triggers Codex's hook-review prompt. | ||
| fn write_codex_hook_trust_config(config_path: &Path, config: &toml::Value) -> Result<()> { | ||
| let backup = super::backup_config_file(config_path)?; |
There was a problem hiding this comment.
Preserve restrictive permissions on the config backup
On a multi-user Unix host with the common 022 umask, syncing an existing mode-0600 Codex config calls backup_config_file, whose plain std::fs::write creates .bak.new as mode 0644 and never reapplies the source mode before publishing config.toml.bak. Because Codex MCP configuration can contain credential environment values, the new trust sync can expose secrets to other local users; copy the original metadata onto the backup staging file.
Useful? React with 👍 / 👎.
| if codex_plugin_manifest_path(&ctx.home).exists() | ||
| || !codex_plugin_cached_install_dirs(&ctx.home).is_empty() | ||
| { | ||
| announce_codex_hook_trust(&ctx.home, &ctx.tracedecay_bin); |
There was a problem hiding this comment.
Wire trust sync into the canonical update path
For an already-current Codex installation whose hook trust is missing, preflight_non_interactive_install returns Ready, so prepare_non_interactive_install is skipped, while the production handle_update_plugin_command uses the canonical component transaction and never calls this update_plugin override; src/update_cmd.rs also explicitly skips canonical hosts. Thus tracedecay update-plugin leaves existing hooks untrusted and Codex continues skipping them, despite this method claiming to auto-trust updates.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
| // payload into its cache without writing `[hooks.state]`, so recording | ||
| // the content hashes here is what lets the hooks run without a manual | ||
| // `/hooks` approval once activation completes. | ||
| announce_codex_hook_trust(&ctx.home, &ctx.tracedecay_bin); |
There was a problem hiding this comment.
Move trust mutation inside the registration transaction
When preflight requires preparation, this writes config.toml before apply_default_canonical_component_set starts and snapshots registration state. If codex plugin add, artifact verification, or a later transaction step fails, rollback therefore restores the already-modified trust state rather than the user's pre-command config, leaving a failed install with trusted TraceDecay entries. Perform the sync after activation within the transaction's rollback boundary.
AGENTS.md reference: AGENTS.md:L108-L110
Useful? React with 👍 / 👎.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
763d105 to
21fdaa0
Compare
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
dbed6d3
into
codex/tracedecay-total-redesign-plan
Ports the origin/master Codex hook auto-trust write path onto #421 (base
codex/tracedecay-total-redesign-plan, remounted onto a709de6 / 0.1.0-beta.9), then addresses all four Codex review findings.What
codex plugin addnever writes[hooks.state]and/hooksis interactive-only, so on this branch freshly installed hooks were silently skipped by Codex until a manual/hooksapproval. master already solved this by recording trust hashes in~/.codex/config.toml; this PR reimplements that write path against #421's architecture.crates/tracedecay-agent-hosts/src/agents/codex.rs):sync_codex_hook_trust,write_codex_hook_trust_config,announce_codex_hook_trust,codex_installed_hook_trust_entries,codex_runtime_hooks_path, and thecodex_hook_command_invokes_tracedecaysafety valve, adapted to this branch'sResult-returning hash helpers (canonical_sha256) and its durable write authority (backup_config_file+safe_write_text_file, preserving permissions and write intents) instead of master's rawfs::write./hooksreview. Sync prunes stale active/legacy-personalentries while preserving foreign plugins' records and all unrelated user config.activate_deployed_host_registration— after a successfulcodex plugin add, and equally for an already-natively-active install. That single point is reached by the canonical component transaction for Install, Update, and Repair (activate_deployed_host_component_registrationfor Core sets) and by the traitupdate_plugin, and it runs insidewith_host_config_write_intents, so a rejected transaction rolls the trust write back with the rest ofconfig.toml.deactivate_deployed_host_registrationprunes the managedtracedecay@…[hooks.state]records (foreign records preserved, emptied tables dropped), so post-uninstall registration state reachesMissinginstead of being heldRepairableby trust residue.agents/mod.rs):backup_config_filenow copies the original file's permission identity (mode/readonly/POSIX ACLs) onto the.bak.newstaging file before publishing.bak, so a 0600 Codex config (which can carry credential env values) no longer leaks a umask-default 0644 backup.codex_hook_trust_followupstill returns/hooksguidance until explicit, current trust exists; it clears after a successful auto-trust and persists when the safety valve skipped a hook or the config was unwritable.plugin_registryregion guard is untouched — it still refuses acodex plugincommand that mutates[hooks]; TraceDecay's own trust writes run outside any CLI observation window. Module docs updated (they previously stated TraceDecay never writesconfig.tomlhook trust).Review findings addressed
prune_codex_hook_trust_records), covered bydeactivation_prunes_managed_hook_trust_and_preserves_foreign_records.config.toml.bak) insync_codex_hook_trust_records_entries_and_preserves_unrelated_config.tracedecay update-pluginskipsupdate_pluginfor canonical hosts and drivesapply_default_canonical_component_set(Update); trust sync moved intoactivate_deployed_host_registration, which that transaction reaches even for already-current installs (covered byactivation_records_hook_trust_for_already_active_install).prepare_non_interactive_install(asserted untouched-config inprepare_stages_the_source_and_returns_ready_for_cli_activation); it now happens during activation inside the registration transaction's write-intent scope, so failed installs roll it back.Tests
All six master sync tests ported (
sync_codex_hook_trust_*, incl. the tampered-command rejection) pluscodex_hook_command_invokes_tracedecay_is_a_safety_valve; master's agent_suite update-plugin hook-trust assertion carried as a crate test driving the realupdate_plugin(codex_update_plugin_refreshes_bundle_and_records_hook_trust: trust keys recorded,model = "gpt-5"and the[plugins]activation record preserved). This branch has notests/agent_suiteinstall/update-plugin Codex tests to extend. Existing #421 followup tests kept.Verification (on a709de6)
cargo test -p tracedecay-agent-hosts --lib: 718 passed, 0 failedcargo clippy -p tracedecay-agent-hosts --lib --all-features: clean