Skip to content

fix: preserve bundled channel plugin compat#58873

Merged
obviyus merged 3 commits intomainfrom
fix/58848-channel-plugin-compat
Apr 1, 2026
Merged

fix: preserve bundled channel plugin compat#58873
obviyus merged 3 commits intomainfrom
fix/58848-channel-plugin-compat

Conversation

@obviyus
Copy link
Copy Markdown
Contributor

@obviyus obviyus commented Apr 1, 2026

Summary

  • keep bundled channel plugins loadable when legacy channels.<id>.enabled=true config is used under a restrictive plugins.allow
  • add doctor warnings for real channel-plugin blockers and suppress misleading setup guidance when the plugin is explicitly blocked
  • add regression coverage for loader, enable-state, and doctor flows

Fixes #58848

@openclaw-barnacle openclaw-barnacle bot added commands Command implementations size: M maintainer Maintainer-authored PR labels Apr 1, 2026
@obviyus obviyus marked this pull request as ready for review April 1, 2026 08:35
@obviyus obviyus self-assigned this Apr 1, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 1, 2026

Greptile Summary

This PR fixes a compatibility bug where bundled channel plugins (e.g. telegram) were not loadable when the legacy channels.<id>.enabled=true config was used alongside a restrictive plugins.allow list. It also adds a new doctor subsystem (channel-plugin-blockers.ts) to surface real blockers — explicit plugin disable and global plugins.enabled=false — and suppress misleading first-time-setup guidance when the channel's plugin is definitively blocked.

Key changes:

  • src/plugins/config-state.ts: Extends the resolveEffectiveEnableState bypass to include "not in allowlist" alongside "bundled (disabled by default)", so a bundled channel plugin is promoted to enabled when channels.<id>.enabled=true is set, even under a restrictive plugins.allow.
  • src/commands/doctor/shared/channel-plugin-blockers.ts (new file): Scans for channel plugins that are explicitly disabled (plugins.entries.<id>.enabled=false) or globally blocked (plugins.enabled=false) and emits actionable warnings.
  • src/commands/doctor/shared/preview-warnings.ts: Integrates the new scanner and filters out misleading empty-allowlist setup warnings when a channel's plugin is definitively blocked.
  • Tests cover loader behaviour, config-state resolution, and doctor warning flows for both new scenarios.

Confidence Score: 5/5

Safe to merge — targeted fix with good test coverage and correct precedence handling for all disable/bypass combinations.

All changes are focused and well-reasoned. The bypass added to resolveEffectiveEnableState correctly positions "not in allowlist" only after the explicit-disable and denylist checks, preserving their authority. The new blocker scanner correctly re-uses resolveEffectiveEnableState (including the new bypass) so it won't flag plugins that are now loadable via channels.<id>.enabled=true. Warning suppression is scoped only to the two genuine blockers ("disabled in config" and "plugins disabled"), leaving denylist and workspace-plugin cases untouched. Tests cover the new loader path, config-state resolution, and doctor warning flows. No P0 or P1 findings.

No files require special attention.

Reviews (1): Last reviewed commit: "fix: preserve bundled channel plugin com..." | Re-trigger Greptile

Copy link
Copy Markdown

@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: e41b20edf6

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

@obviyus obviyus force-pushed the fix/58848-channel-plugin-compat branch from e41b20e to 7fc3770 Compare April 1, 2026 08:44
@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Apr 1, 2026

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🟡 Medium Bundled channel plugins can be enabled even when not in plugins.allow allowlist
1. 🟡 Bundled channel plugins can be enabled even when not in plugins.allow allowlist
Property Value
Severity Medium
CWE CWE-284
Location src/plugins/config-state.ts:303-310

Description

resolveEffectiveEnableState overrides a disabled enable-state when the plugin was disabled for "not in allowlist" (i.e., plugins.allow is restrictive) if channels.<id>.enabled === true is set in the root config.

This creates an allowlist bypass for bundled channel plugins:

  • Input: rootConfig.channels[<channelId>].enabled (from OpenClawConfig)
  • Policy check: resolveEnableState returns { enabled: false, reason: "not in allowlist" } when plugins.allow.length > 0 and the plugin ID is not explicitly allowed
  • Bypass: resolveEffectiveEnableState re-enables the plugin anyway when it matches a bundled channel ID
  • Impact: plugin loading proceeds in loadOpenClawPlugins based on resolveEffectiveEnableState, enabling/disabling which code is registered and executed. This weakens the expectation that plugins.allow is a strict boundary controlling which plugins are allowed to run.

Vulnerable logic:

if (
  !base.enabled &&
  (base.reason === "bundled (disabled by default)" || base.reason === "not in allowlist") &&
  isBundledChannelEnabledByChannelConfig(params.rootConfig, params.id)
) {
  return { enabled: true };
}

If an operator relies on plugins.allow to prevent certain bundled channel plugins from being enabled (e.g., to disable integration code paths), setting channels.<id>.enabled=true will now override that restriction and load the channel plugin.

Recommendation

Do not allow channels.<id>.enabled to override an explicit plugins.allow policy.

Options:

  1. Keep auto-enable only for the default-bundled case, but not when the plugin was blocked by the allowlist:
if (
  !base.enabled &&
  base.reason === "bundled (disabled by default)" &&
  isBundledChannelEnabledByChannelConfig(params.rootConfig, params.id)
) {
  return { enabled: true };
}
  1. If channel enablement must work under allowlist, require an explicit opt-in that preserves the security boundary, e.g. a dedicated config flag like plugins.allowBundledChannelsFromChannelsConfig: true, and default it to false.

  2. Alternatively, when plugins.allow is set, require the channel plugin ID to also be included in plugins.allow before enabling it, even if channels.<id>.enabled is true.


Analyzed PR: #58873 at commit 7fc3770

Last updated on: 2026-04-01T08:46:46Z

@SonicBotMan

This comment was marked as spam.

@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Apr 1, 2026

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🟡 Medium Bundled channel plugins can bypass plugins.allow allowlist via channels..enabled=true
1. 🟡 Bundled channel plugins can bypass plugins.allow allowlist via channels..enabled=true
Property Value
Severity Medium
CWE CWE-284
Location src/plugins/config-state.ts:302-312

Description

resolveEffectiveEnableState() now re-enables bundled plugins even when they were disabled due to the plugin allowlist, as long as channels.<id>.enabled === true.

  • Input: channels.<id>.enabled from the root config
  • Security control bypassed: plugins.allow (allowlist) enforcement in resolveEnableState()
  • Effect: a restrictive allowlist can be unintentionally bypassed for bundled channel plugins, loading additional code and capabilities that the operator may have intended to block via the allowlist.

Vulnerable logic:

if (
  params.origin === "bundled" &&
  !base.enabled &&
  (base.reason === "bundled (disabled by default)" || base.reason === "not in allowlist") &&
  isBundledChannelEnabledByChannelConfig(params.rootConfig, params.id)
) {
  return { enabled: true };
}

This effectively turns channels.<id>.enabled=true into an override for the allowlist decision for bundled channel plugins.

Recommendation

Do not allow channels.<id>.enabled to override the allowlist decision, or gate it behind an explicit opt-in.

Safer options:

  1. Keep allowlist authoritative (recommended if plugins.allow is a security boundary):
if (
  params.origin === "bundled" &&
  !base.enabled &&
  base.reason === "bundled (disabled by default)" &&
  isBundledChannelEnabledByChannelConfig(params.rootConfig, params.id)
) {
  return { enabled: true };
}// do not override when base.reason === "not in allowlist"
  1. If legacy behavior is required, introduce an explicit setting such as plugins.allowBundledChannelsFromLegacyChannelConfig: true and default it to false, and only then override the allowlist.

Also consider emitting a warning when channels.<id>.enabled=true would load a plugin that is not in the allowlist, so operators notice the policy conflict.


Analyzed PR: #58873 at commit bfe67f9

Last updated on: 2026-04-01T08:56:32Z

@obviyus obviyus force-pushed the fix/58848-channel-plugin-compat branch from bfe67f9 to 1530a9d Compare April 1, 2026 09:11
@obviyus obviyus merged commit fb28b02 into main Apr 1, 2026
23 of 24 checks passed
@obviyus obviyus deleted the fix/58848-channel-plugin-compat branch April 1, 2026 09:12
@obviyus
Copy link
Copy Markdown
Contributor Author

obviyus commented Apr 1, 2026

Landed on main.

Copy link
Copy Markdown

@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: 1530a9d43e

ℹ️ 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 +92 to +94
return hits.some((hit) => {
const prefix = `channels.${sanitizeForLog(hit.channelId)}`;
return warning.includes(`${prefix}:`) || warning.includes(`${prefix}.`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Suppress channel guidance only when the channel is truly blocked

isWarningBlockedByChannelPlugin suppresses all channels.<id> empty-allowlist guidance as soon as any blocker hit exists for that channel, even if another channel plugin for the same channel is still enabled. For example, a disabled alternate plugin (e.g. plugins.entries.alt-telegram.enabled=false) will add a blocker hit for telegram, and this check then hides first-time/setup warnings for channels.telegram.* despite the bundled Telegram plugin remaining loadable. That makes openclaw doctor omit actionable channel guidance in mixed-plugin setups.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade from 3.24 to 3.31 silently breaks Telegram — channel moved to plugin but existing config not migrated

2 participants