fix: harden plugin feature gating by xl-openai · Pull Request #15020 · openai/codex

xl-openai · 2026-03-18T04:21:20Z

BAD PR
I messed up the commits in this PR and accidentally merged incorrect changes.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65980a9847

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

chatgpt-codex-connector · 2026-03-18T04:26:02Z

codex-rs/core/src/plugins/manager.rs

        force_reload: bool,
    ) -> PluginLoadOutcome {
-        if !plugins_feature_enabled_from_stack(config_layer_stack) {
+        if !config.features.enabled(Feature::Plugins) {


Restrict plugin feature checks to trusted user config

plugins_for_layer_stack now gates on config.features.enabled(Feature::Plugins), which is computed from merged layers. A project .codex/config.toml can set [features].plugins = true and override a user-disabled flag, re-enabling persisted user plugins for that checkout. This breaks the prior user-opt-in boundary and lets untrusted project config influence plugin activation.

Useful? React with 👍 / 👎.

that‘s ok.

chatgpt-codex-connector · 2026-03-18T04:26:02Z

codex-rs/core/src/skills/manager.rs

-        let loaded_plugins =
-            self.plugins_manager
-                .plugins_for_layer_stack(cwd, &config_layer_stack, force_reload);
+        let config_toml: ConfigToml = match config_layer_stack.effective_config().try_into() {


Deserialize merged config with base path before skill load

skills_for_cwd_with_extra_user_roots uses effective_config().try_into::<ConfigToml>() directly. This skips deserialize_config_toml_with_base, so any relative AbsolutePathBuf field fails deserialization (AbsolutePathBuf deserialized without a base path). The method then returns an error and skips loading skills, even though normal config loading accepts those configs.

Useful? React with 👍 / 👎.

xl-openai · 2026-03-18T04:55:31Z

@codex review again

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36c76cbc66

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

chatgpt-codex-connector · 2026-03-18T05:05:30Z

codex-rs/core/src/plugins/manager.rs


 impl PluginsManager {
-    pub fn new(codex_home: PathBuf) -> Self {
+    pub fn new(codex_home: PathBuf, plugins_enabled: bool) -> Self {


Update all PluginsManager::new call sites

Changing PluginsManager::new to require plugins_enabled breaks callers that still use the old 1-arg API. In this commit, chatgpt/src/connectors.rs:119, cli/src/mcp_cmd.rs:392, and tui_app_server/src/chatwidget.rs:8851 still call PluginsManager::new(config.codex_home.clone()), which causes a compile-time arity error (E0061) for those crates.

Useful? React with 👍 / 👎.

sayan-oai · 2026-03-18T05:15:58Z

codex-rs/core/src/thread_manager.rs

+        let plugins_manager = Arc::new(PluginsManager::new(
+            codex_home.clone(),
+            config.features.enabled(crate::features::Feature::Plugins),
+        ));


this instance is reused for all threads/sessions over the lifetime of the process. so for app-server, changing config after the process is started wouldn't update the value, and passing thread-level config that changes the value wouldn't be respected.

lmk if im misunderstanding, but this would basically make it static until you restart app-server

Good call rollback to original approach.

sayan-oai · 2026-03-18T07:40:00Z

@codex review this

sayan-oai · 2026-03-18T07:41:51Z

codex-rs/core/src/plugins/manager.rs

-        force_reload: bool,
-    ) -> PluginLoadOutcome {
-        if !plugins_feature_enabled_from_stack(config_layer_stack) {
+        if !config.features.enabled(Feature::Plugins) {


this reads cloud requirements, yes?

sayan-oai · 2026-03-18T07:46:40Z

codex-rs/core/src/plugins/manager.rs

    let Some(plugins_value) = user_layer.config.get("plugins") else {
        return HashMap::new();
    };


as we discussed offline, this just reads the current enablement state of the plugins directly from config, without considering whether features.plugins is enabled. do we not want to gate this as well?

looked a bit more, configured_plugins_from_stack seems like the main way existing functionality loads plugins. gating here seems like it would allow us to avoid gating on many of the higher level functions you added the gating to, as long as those work correctly when no plugins are returned (empty HashMap).

it seems that the only logic that needs to keep its own gate is the marketplace-reading logic in list_marketplaces_for_config.

im not sure if code-organization-wise pushing down the gate check is better (less random gate checks in higher-level functions, dont have to remember to add those checks yourself) or worse (less clear that those functions will return empty when features.plugin = false).

LMK what you think.

I think configured_plugins_from_stack is a low-level help function and it makes sense to parse the config stack directly (no depends on config). But I do cleanup the configured_plugin_states more to get rid of the plugins_for_config call.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ee78106b04

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

chatgpt-codex-connector · 2026-03-18T07:47:19Z

codex-rs/core/src/plugins/manager.rs

+        if let Some(outcome) = self.cached_enabled_outcome() {
            return outcome;


Restore force_reload semantics for plugin skill loading

skills_for_cwd*_ still accepts force_reload, but plugin resolution now calls plugins_for_config, which unconditionally returns cached_enabled_outcome when present. After plugin config/state changes, force_reload=true no longer refreshes plugin-provided roots, so stale plugin skills can persist until an unrelated clear_cache() runs.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-03-18T07:47:19Z

codex-rs/core/src/plugins/marketplace.rs

+        match load_marketplace(&marketplace_path) {
+            Ok(marketplace) => marketplaces.push(marketplace),
+            Err(err) => {
+                warn!(
+                    path = %marketplace_path,
+                    error = %err,
+                    "skipping marketplace that failed to load"
+                );
+            }


Narrow plugin/list fail-open handling to malformed files only

list_marketplaces_with_home now suppresses every load_marketplace failure and only logs a warning. This causes plugin/list to return partial/empty success responses for IO/permission/other hard failures, masking actionable errors. The intended behavior was fail-open for malformed marketplace data, not all marketplace load errors.

Useful? React with 👍 / 👎.

This is OK.

sayan-oai

approving to unblock as some of these fixes are urgent, but I think we should think about this comment to see which option is cleaner (it was already hard to debug this because of messy code 🙃).

and i know this is tedious/a nit, but it would be great if we could split this pr based on the 4 points in the description. the changes are all small but high impact on functionality, and I'm worried about rolling back 3 good fixes because they were bundled with one bug.

xl-openai · 2026-03-18T09:11:15Z

Removing plugins_for_layer_stack means a cwd-scoped plugin cache no longer makes sense, so I update it to a global cache. Also to fully respect the feature flag, we also need to add gating at the remaining callsites.

The fail-open change for marketplace.json is a bit orthogonal, but it is small, so I’m keeping it in this PR.

xl-openai requested review from gverma-openai and sayan-oai March 18, 2026 04:21

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

xl-openai force-pushed the xl/plugins2 branch from 65980a9 to 36c76cb Compare March 18, 2026 04:51

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

sayan-oai reviewed Mar 18, 2026

View reviewed changes

xl-openai force-pushed the xl/plugins2 branch 2 times, most recently from 5334dff to 07c90b2 Compare March 18, 2026 06:44

sayan-oai reviewed Mar 18, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

sayan-oai approved these changes Mar 18, 2026

View reviewed changes

xl-openai force-pushed the xl/plugins2 branch from df32a1d to 8d12e41 Compare March 18, 2026 08:44

xl-openai changed the title ~~fix: read plugin feature flag from requirement-resolved config.~~ fix: harden plugin feature gating Mar 18, 2026

feat: support product-scoped plugins.

5ca279b

xl-openai force-pushed the xl/plugins2 branch from 8d12e41 to 5ca279b Compare March 18, 2026 09:13

xl-openai merged commit 580f32a into main Mar 18, 2026
54 of 56 checks passed

xl-openai deleted the xl/plugins2 branch March 18, 2026 17:11

github-actions bot locked and limited conversation to collaborators Mar 18, 2026

		if let Some(outcome) = self.cached_enabled_outcome() {
		return outcome;

Conversation

xl-openai commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

xl-openai commented Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayan-oai commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xl-openai Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayan-oai left a comment

Choose a reason for hiding this comment

Uh oh!

xl-openai commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xl-openai commented Mar 18, 2026 •

edited

Loading

xl-openai Mar 18, 2026 •

edited

Loading