fix(cli): keep channel status checks off plugin runtimes#69479
fix(cli): keep channel status checks off plugin runtimes#69479gumadeiras merged 31 commits intomainfrom
Conversation
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟠 Secret target scoping bypass via wildcard/dot characters in channel/plugin IDs
Description
Because
This can cause status/audit/secret discovery flows that rely on these “scoped” ids to enumerate and resolve SecretRefs outside the intended channel boundary, leading to unintended secret access or disclosure. Vulnerable code: const allowedPrefix = `channels.${channelId}.`;
return (
params.entry.id.startsWith(allowedPrefix) &&
params.entry.configFile === "openclaw.json" &&
typeof params.entry.pathPattern === "string" &&
params.entry.pathPattern.startsWith(allowedPrefix) &&
(params.entry.refPathPattern === undefined ||
params.entry.refPathPattern.startsWith(allowedPrefix))
);RecommendationTreat the channel/plugin id as a single literal path segment and prevent it from being interpreted as pattern syntax. Recommended defenses (do both):
import { parseDotPath } from "../secrets/shared.js";
function isScopedToChannel(pathPattern: string, channelId: string): boolean {
const segs = parseDotPath(pathPattern);
// Require exact literal segments: ["channels", channelId, ...]
return segs.length >= 3 && segs[0] === "channels" && segs[1] === channelId;
}
function isScopedChannelSecretTargetEntry({ entry, pluginChannelId }: { entry: ...; pluginChannelId: string }) {
const channelId = normalizeOptionalString(pluginChannelId);
if (!channelId) return false;
if (entry.configFile !== "openclaw.json") return false;
if (typeof entry.pathPattern !== "string") return false;
if (!isScopedToChannel(entry.id, channelId)) return false;
if (!isScopedToChannel(entry.pathPattern, channelId)) return false;
if (entry.refPathPattern !== undefined && !isScopedToChannel(entry.refPathPattern, channelId)) return false;
return true;
}This prevents 2. 🟡 Ambient environment variable names can mark external channels as "configured" and trigger plugin loading on read-only paths
DescriptionExternal channel plugins can declare Impact:
This is risky because a malicious/buggy but explicitly trusted/allowed plugin can select overly-generic names (e.g., Vulnerable code: function hasNonEmptyEnvValue(env: NodeJS.ProcessEnv, key: string): boolean {
if (!isSafeChannelEnvVarTriggerName(key)) {
return false;
}
const trimmed = key.trim();
const value = env[trimmed] ?? env[trimmed.toUpperCase()];
return typeof value === "string" && value.trim().length > 0;
}and the safety check is blacklist-based: return /^[A-Z][A-Z0-9_]*$/.test(normalized) && !UNSAFE_CHANNEL_ENV_VAR_TRIGGER_NAMES.has(normalized)which still permits many common ambient names. RecommendationMake env-var-based configuration triggers opt-in and strongly namespaced, rather than relying on a small blacklist. Suggested hardening options (combine as appropriate):
export function isSafeChannelEnvVarTriggerName(key: string): boolean {
const normalized = key.trim().toUpperCase();
if (!/^OPENCLAW_[A-Z0-9_]+$/.test(normalized)) return false;
return true;
}
Also consider setting Analyzed PR: #69479 at commit Last updated on: 2026-04-21T17:56:50Z |
Greptile SummaryThis PR fixes a cold-start latency regression (#69042) where read-only CLI commands ( Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/edge-case observations that do not affect correctness on the targeted paths. The architectural split (runtime vs. read-only security adapters) is sound, the latency numbers are well-evidenced, and the three inline findings are either a minor type-safety style gap (missing satisfies on Telegram) or unlikely edge-case observations with no impact on the common single-provider case. No P0/P1 issues found. extensions/telegram/src/security.ts (missing satisfies guard); src/security/audit.ts (sourceConfig vs autoEnabled.config mismatch in channel ID deletion); src/infra/provider-usage.auth.ts (per-provider vs. global allowAuthProfileStore gate) Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/infra/provider-usage.auth.ts
Line: 240-260
Comment:
**`hasDirectCredentialSource` is evaluated across all providers jointly, not per-provider**
The check gates the single `allowAuthProfileStore` flag for every provider in the loop. If `providers = ["openai", "anthropic"]` and only `"openai"` has a direct env/config credential, `hasDirectCredentialSource` is `true`, `allowAuthProfileStore` becomes `true`, and plugin auth is attempted for `"anthropic"` even though it has no direct credential source. The PR description says "skip plugin auth when there is no direct credential source", but the skip only applies when *none* of the providers have a direct source.
This is benign for the targeted use case (a single usage provider is most common), but if two usage providers are configured and only one has credentials, the read-only path will still load the plugin runtime for the second provider.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/security/audit.ts
Line: 358-365
Comment:
**Channel ID removal uses `sourceConfig`; `requestedPluginIds` is built from `autoEnabled.config`**
`requestedPluginIds` is populated from `autoEnabled.config` (which may diverge from `sourceConfig` after `applyPluginAutoEnable` merges auto-enable overrides), but the IDs to delete are derived from `listPotentialConfiguredChannelIds(context.sourceConfig, …)`. In theory, a channel that is auto-enabled at the `autoEnabled.config` level but absent from `sourceConfig` would survive the deletion and still trigger a full plugin metadata registry load for that channel.
Practically this is an unlikely edge case, but since the entire PR targets latency on read-only paths, it's worth verifying that `listPotentialConfiguredChannelIds` is called against the same config object that seeded `requestedPluginIds` (i.e., `autoEnabled.config`).
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/telegram/src/security.ts
Line: 28-36
Comment:
**Missing `satisfies` type guard on Telegram security adapter**
`telegramSecurityAdapter` is exported without the `satisfies NonNullable<ChannelPlugin<ResolvedTelegramAccount>["security"]>` constraint that the Slack (`extensions/slack/src/security.ts`) and Discord (`extensions/discord/src/security.ts`) adapters both use. If the `ChannelPlugin["security"]` contract gains a required field, this adapter will silently pass TypeScript checking while the others immediately fail, hiding the regression from the read-only path.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: add channel read-only security adap..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a45d1d06f
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR fixes a CLI cold-start regression by keeping read-only status/doctor/health/channels paths on config-only + bundled metadata surfaces, avoiding full bundled channel/provider runtime imports when users have Telegram/Slack/Discord configured.
Changes:
- Introduces read-only channel plugin listing and routes multiple read-only commands through config/metadata-based helpers instead of runtime-heavy plugin entrypoints.
- Adds a snapshot-based plugin compatibility notice path for status/config validation/doctor workspace outputs.
- Narrows provider-usage auth so
channels listcan skip plugin-based usage auth when no direct credential source exists, plus adds guardrail tests to lock in “no plugin preload” behavior.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/security/audit.ts | Avoids loading plugin audit collectors for configured channels when channel security plugins are supplied directly. |
| src/plugins/status.ts | Adds compatibility notices based on snapshot report (no runtime module loading). |
| src/infra/provider-usage.load.ts | Plumbs option to skip plugin usage auth without credentials. |
| src/infra/provider-usage.auth.ts | Implements “skip plugin auth without credential source” behavior and config/env key checks. |
| src/infra/provider-usage.auth.plugin.test.ts | Tests plugin-boundary behavior for usage auth skipping. |
| src/infra/provider-usage.auth.normalizes-keys.test.ts | Updates mocks to support auth store source detection. |
| src/infra/channel-summary.ts | Switches to read-only channel plugin listing for summaries. |
| src/flows/doctor-health-contributions.ts | Reduces runtime repair/recall work in non-deep / non-fix doctor flows. |
| src/commands/status.summary.ts | Adds flag to disable channel summary imports/processing. |
| src/commands/status.summary.test.ts | Tests “includeChannelSummary: false” behavior. |
| src/commands/status.scan.ts | Uses snapshot-based plugin compatibility notices. |
| src/commands/status.scan.test.ts | Updates expectations to ensure no plugin registry preload for status JSON. |
| src/commands/status.scan.test-helpers.ts | Extends status plugin status module mock surface. |
| src/commands/status.scan.fast-json.ts | Removes configured-channel plugin preload; disables channel summary on lean JSON path. |
| src/commands/status.scan.fast-json.test.ts | Updates tests to assert no plugin preload and no channel summary on fast JSON. |
| src/commands/status.scan-overview.ts | Limits secret target discovery to configured channels for status. |
| src/commands/status.scan-execute.ts | Threads summary policy (includeChannelSummary) into execution. |
| src/commands/status.scan-execute.test.ts | Updates expectations for summary policy plumbing. |
| src/commands/status.link-channel.ts | Uses read-only channel plugin listing. |
| src/commands/status.link-channel.test.ts | Updates mocks for read-only channel plugin listing. |
| src/commands/status-runtime-shared.ts | Passes read-only channel plugins into security audit to avoid runtime loads. |
| src/commands/status-runtime-shared.test.ts | Asserts plugins passed into security audit call. |
| src/commands/status-json.test.ts | Updates security audit invocation expectations. |
| src/commands/status-all/channels.ts | Switches channels table building to read-only plugin listing. |
| src/commands/health.ts | Uses read-only channel plugin listing; avoids plugin preload dependencies. |
| src/commands/health.command.coverage.test.ts | Updates coverage expectation for web self-id logging. |
| src/commands/doctor-workspace-status.ts | Uses snapshot plugin report instead of diagnostics report. |
| src/commands/doctor-workspace-status.test.ts | Updates mocks for snapshot plugin report usage. |
| src/commands/doctor-memory-search.ts | Moves some memory checks to config-only metadata; reduces runtime dependence in checks. |
| src/commands/doctor-memory-search.test.ts | Updates tests for new config-only/provider probing behavior. |
| src/commands/doctor-claude-cli.ts | Avoids loading auth profile store when no config/store signals exist. |
| src/commands/doctor-auth.ts | Skips external auth profile resolution when no auth sources exist. |
| src/commands/doctor-auth.profile-health.test.ts | New test covering early-return when no auth sources exist. |
| src/commands/doctor-auth.deprecated-cli-profiles.test.ts | Adds test ensuring provider loading is skipped when no legacy OAuth profiles exist. |
| src/commands/doctor-auth-legacy-oauth.ts | Avoids provider loading when config/store contain no relevant OAuth entries. |
| src/commands/config-validation.ts | Uses snapshot-based plugin compatibility notices during config validation. |
| src/commands/config-validation.test.ts | Updates mocks to use snapshot compatibility notices. |
| src/commands/channels/status.ts | Keeps JSON fallback config-only; narrows secret targets to configured channels. |
| src/commands/channels/status-config-format.ts | Uses read-only channel plugin listing for config-only formatting. |
| src/commands/channels/shared.ts | Uses loaded/setup plugin surfaces to label channels without runtime loads where possible. |
| src/commands/channels/list.ts | Uses read-only channel plugin listing; loads auth store without external profiles; skips plugin usage auth when no creds. |
| src/commands/channels.status.command-flow.test.ts | Adds mocks/tests for config-only JSON fallback behavior. |
| src/commands/channels.list.auth-profiles.test.ts | Updates to assert local-only auth profile listing without external profile loading. |
| src/commands/channels.config-only-status-output.test.ts | Updates mocks to support read-only channel plugin listing. |
| src/cli/program/routes.test.ts | Adds routed coverage for channels list/status and asserts no plugin preload. |
| src/cli/program/routed-command-definitions.ts | Adds routed command definitions for channels list/status. |
| src/cli/program/route-args.ts | Adds argv parsers for channels list/status routed commands. |
| src/cli/program/preaction.test.ts | Updates preaction expectations: no plugin registry preload for status. |
| src/cli/command-startup-policy.test.ts | Updates policy expectations: status/health/channels list/status should not load plugins. |
| src/cli/command-secret-targets.ts | Adds configured-channel secret target selection via bundled channel contract metadata. |
| src/cli/command-secret-targets.import.test.ts | Adds test ensuring configured-channel status targets resolve without touching full registry. |
| src/cli/command-path-policy.test.ts | Updates CLI command path policy: status never loads plugins; adds channels list/status policies. |
| src/cli/command-catalog.ts | Adds channels list/status to CLI catalog; changes status/health to never preload plugins. |
| src/cli/channels-cli.ts | Switches channels list/status actions to direct imports (avoids heavier loader). |
| src/channels/plugins/read-only.ts | New helper to list channel plugins for read-only paths based on config/configured channels. |
| extensions/telegram/src/shared.ts | Exposes security adapter on setup/base surface. |
| extensions/telegram/src/security.ts | New shared Telegram security adapter module. |
| extensions/telegram/src/channel.ts | Uses shared Telegram security adapter; removes inline security collectors. |
| extensions/slack/src/shared.ts | Exposes security adapter on setup/base surface. |
| extensions/slack/src/shared.test.ts | Adds test ensuring security surface is exposed. |
| extensions/slack/src/security.ts | New shared Slack security adapter module. |
| extensions/slack/src/channel.ts | Uses shared Slack security adapter; removes inline security collectors. |
| extensions/discord/src/shared.ts | Exposes security adapter on setup/base surface. |
| extensions/discord/src/shared.test.ts | Adds test ensuring security surface is exposed. |
| extensions/discord/src/security.ts | New shared Discord security adapter module with lazy audit import. |
| extensions/discord/src/channel.ts | Uses shared Discord security adapter; removes inline security collectors. |
| CHANGELOG.md | Documents the CLI cold-path performance fix for configured Telegram/Slack/Discord. |
8f57b21 to
fe7f3aa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe7f3aa834
ℹ️ 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".
|
Check triage update:
Local verification run:
Also attempted the repo changed-scope pre-commit gate. Typecheck/lint/import-cycle/guards passed, but the broad infra test lane failed in unrelated git-env tests ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18e04abb0a
ℹ️ 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".
|
Check triage update for the latest bot comments:
Verification:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4f38f140
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0f3e6867e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26f89ecf59
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5448c88f39
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fe0e45697
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8259a34239
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b31ab2cad
ℹ️ 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".
e8cce7a to
63f6e41
Compare
|
Merged via squash.
Thanks @gumadeiras! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63f6e416a9
ℹ️ 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".
) Merged via squash. Prepared head SHA: 63f6e41 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
) Merged via squash. Prepared head SHA: 63f6e41 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
) Merged via squash. Prepared head SHA: 63f6e41 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
) Merged via squash. Prepared head SHA: 63f6e41 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
) Merged via squash. Prepared head SHA: 63f6e41 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
c727388f937ffc9128ba4e4d2ae37d4c72487bc1moved bundled runtime dependencies under extension runtime entrypoints. Several read-only commands still reached APIs that resolve the runtime plugin registry, so they paid the full extension import cost even when only config/security/status metadata was needed.channels listalso fetched usage by default; with no usage credentials, that path still tried plugin-owned usage auth and loaded provider runtimes before returning no usage rows.Regression Test Plan (if applicable)
User-visible / Behavior Changes
channels listno longer loads external plugin-synced auth profiles on the no-runtime read-only path; local auth profiles are still listed.channels listnow returns an empty usage summary without loading usage provider plugins.Diagram (if applicable)
Security Impact (required)
Repro + Verification
Environment
Steps
pnpm buildOPENCLAW_PLUGIN_LOAD_PROFILE=1 ./openclaw.mjs <command>against temp Telegram config./tmp/openclaw-issue-bench.sh /Users/gumadeiras/openclaw <telegram|slack|discord> 1Expected
Actual
status,doctor --non-interactive,health --verbose,channels list, andchannels statusemittedplugin_load_lines=0with Telegram config.health --verboseexits 1 in the temp benchmark because the gateway is not configured/running in that temp HOME; it still stays off plugin runtimes.Evidence
First-bad commit re-measured for this PR update:
c727388f937ffc9128ba4e4d2ae37d4c72487bc1.Current PR head:
26f89ecf59ccd3c5ab2cc2252befec71f0023a64.Benchmark command:
/tmp/openclaw-issue-bench.sh <repo> <telegram|slack|discord> 1; single cold sample per channel/command afterpnpm buildon both worktrees.statusdoctor --non-interactivehealth --verbosechannels listchannels statusstatusdoctor --non-interactivehealth --verbosechannels listchannels statusstatusdoctor --non-interactivehealth --verbosechannels listchannels statusNotes:
status,health --verbose,channels list, andchannels statusacross Telegram, Slack, and Discord.doctor --non-interactiveno longer loads full plugin runtimes, but this run is slower because remaining non-plugin doctor work dominates.health --verboseexits 1 in the temp benchmark because the gateway is not configured/running in that temp HOME; it still stays off plugin runtimes.Human Verification (required)
pnpm buildpnpm check:changedReview Conversations
Compatibility / Migration
Risks and Mitigations