Skip to content

fix: enforce trusted-before-project ordering for hooks#15936

Open
viyatb-oai wants to merge 7 commits intocodex/viyatb/trusted-project-config-gatingfrom
codex/viyatb/hooks-trust-precedence
Open

fix: enforce trusted-before-project ordering for hooks#15936
viyatb-oai wants to merge 7 commits intocodex/viyatb/trusted-project-config-gatingfrom
codex/viyatb/hooks-trust-precedence

Conversation

@viyatb-oai
Copy link
Copy Markdown
Collaborator

@viyatb-oai viyatb-oai commented Mar 27, 2026

Summary

  • execute trust-sensitive hook events in tiers so non-project hooks run before project hooks for PreToolUse, SessionStart, and UserPromptSubmit
  • preserve parallel execution within each trust tier, but skip lower-precedence project hooks once a higher-precedence hook blocks or stops processing
  • keep trusted-project lookup stable on Windows by matching both raw and canonicalized absolute path keys when resolving project trust and active project config

Security

  • closes the PreToolUse race where a project hook could observe or exfiltrate a tool invocation before a higher-precedence user or managed hook denied it
  • closes the analogous prompt/session leak where project UserPromptSubmit or SessionStart hooks could observe prompt or startup data before a higher-precedence hook stopped processing
  • makes the trust-gating regressions portable on Windows, where temp-path aliases and canonicalized paths can differ for the same trusted project

Stack

Test

  • cargo test -p codex-hooks
  • targeted codex-core regressions covering project hooks without config.toml
  • cargo clippy -p codex-core -p codex-hooks --tests -- -D warnings
  • cargo clippy -p codex-core --tests -- -D warnings

@viyatb-oai viyatb-oai force-pushed the codex/viyatb/hooks-trust-precedence branch from 8dfdefd to a5e69ff Compare March 27, 2026 18:36
@viyatb-oai viyatb-oai marked this pull request as ready for review March 30, 2026 17:22
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9401e5ff84

ℹ️ 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".

Comment on lines 664 to 673
let projects_trust = projects
.into_iter()
.filter_map(|(key, project)| project.trust_level.map(|trust_level| (key, trust_level)))
.flat_map(|(key, project)| {
project.trust_level.into_iter().flat_map(move |trust_level| {
normalized_project_trust_key_strs(&key)
.into_iter()
.map(move |normalized_key| (normalized_key, trust_level))
})
})
.collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Resolve canonical trust-key collisions deterministically

project_trust_context canonicalizes each project key and then collects into a HashMap. If config contains two different keys that resolve to the same canonical path (e.g. symlink path vs real path) with conflicting trust_level, the later insert wins. Because HashMap iteration order is not stable, trust can flip between trusted/untrusted across runs, enabling or disabling project hooks/config unpredictably.

Useful? React with 👍 / 👎.

@viyatb-oai viyatb-oai changed the title fix: run project pre-tool hooks after trusted hooks fix: enforce trusted-before-project ordering for hooks Mar 30, 2026
@viyatb-oai viyatb-oai force-pushed the codex/viyatb/hooks-trust-precedence branch from 9401e5f to f0f222a Compare March 31, 2026 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant