feat(plugins): surface manifest provider setup choices#71240
Conversation
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟡 Terminal escape injection via manifest-controlled auth choice labels/hints rendered in CLI prompts
DescriptionUntrusted plugin manifest fields (
This enables ANSI escape/control sequence injection (terminal UI redressing), allowing a malicious plugin manifest to manipulate the user's terminal display (e.g., hiding options, spoofing prompts, rewriting lines), potentially leading to phishing-style credential capture or unsafe selections. Vulnerable code (manifest to flow contribution): label: choice.choiceLabel,
...(choice.choiceHint ? { hint: choice.choiceHint } : {}),
...
label: groupLabel,
...(choice.groupHint ? { hint: choice.groupHint } : {}),Rendering sink (no escaping): const base = { value: opt.value, label: opt.label };
return opt.hint === undefined ? base : { ...base, hint: stylePromptHint(opt.hint) };
...
await select({ message: stylePromptMessage(params.message), options })RecommendationSanitize/neutralize terminal control sequences for any untrusted strings before passing them to terminal prompt renderers. A robust fix is to strip all ANSI escapes and other C0 control characters (except Example (centralized, applied in import stripAnsi from "strip-ansi";
function sanitizeForTerminal(s: string): string {
// Remove ANSI escapes
const noAnsi = stripAnsi(s);
// Remove other control characters (C0/C1) to prevent terminal control injection
return noAnsi.replace(/[\u0000-\u001F\u007F-\u009F]/g, "");
}
const options = params.options.map((opt) => {
const base = { value: opt.value, label: sanitizeForTerminal(opt.label) };
return opt.hint === undefined
? base
: { ...base, hint: stylePromptHint(sanitizeForTerminal(opt.hint)) };
});Also consider applying the same sanitization to group labels/hints wherever groups are rendered, and add tests ensuring escape sequences are removed. 2. 🟡 Provider setup option spoofing via choiceId collision (manifest suppresses runtime/install-catalog options)
Description
Impact:
Vulnerable code: const seenOptionValues = new Set(
manifestContributions.map((contribution) => contribution.option.value),
);
...
.filter((option) => !seenOptionValues.has(option.value))
...
.filter((contribution) => !seenOptionValues.has(contribution.option.value));RecommendationDo not allow manifest choices to suppress runtime/install-catalog choices based on a global Safer approaches:
type SetupKey = string;
const keyFor = (c: { source: string; providerId?: string; pluginId?: string; option: { value: string } }) =>
[c.source, c.pluginId ?? "", c.providerId ?? "", c.option.value].join("::");
const seen = new Set(manifestContributions.map(keyFor));
const runtime = resolveProviderWizardOptions(...)
.filter((o) => !seen.has(["runtime", "", o.groupId, o.value].join("::")));
Analyzed PR: #71240 at commit Last updated on: 2026-04-24T20:16:26Z |
Greptile SummaryThis PR surfaces plugin manifest
Confidence Score: 3/5Not safe to merge until the manifest path applies the same plugin enable-state filter that the install-catalog path already enforces. A P1 logic gap exists: manifest contributions bypass the resolveEffectiveEnableState check that install-catalog contributions go through, meaning explicitly-disabled plugins can still appear in the setup flow via their manifest metadata. The rest of the change (dedup ordering, scope filtering, type extension, tests, docs) is correct. src/flows/provider-flow.ts — resolveManifestProviderSetupFlowContributions needs resolveEffectiveEnableState filtering mirroring resolveInstallCatalogProviderSetupFlowContributions; src/flows/provider-flow.test.ts — needs tests for manifest contributions respecting plugins.enabled: false and allowlist config. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/flows/provider-flow.ts
Line: 125-168
Comment:
**Missing enable-state gate for manifest contributions**
`resolveManifestProviderSetupFlowContributions` never calls `resolveEffectiveEnableState`, so manifest choices from explicitly-disabled or non-allowlisted plugins will still appear in the setup flow. By contrast, `resolveInstallCatalogProviderSetupFlowContributions` filters every entry through `resolveEffectiveEnableState({ ..., enabledByDefault: true })`.
Concretely: with `plugins: { enabled: false }` or `plugins: { allow: ["openai"] }` in config, install-catalog contributions are correctly hidden (existing tests at lines 247–299 verify this), but the equivalent manifest contributions are not. `resolveManifestProviderAuthChoiceCandidates` only gates workspace-origin, untrusted plugins — bundled and global plugins bypass that check entirely.
The fix mirrors what the install-catalog path does: compute `normalizedPluginsConfig` at the top of the function and add a `.filter` that calls `resolveEffectiveEnableState` with `enabledByDefault: true` after the scope filter.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(plugins): surface manifest provider..." | Re-trigger Greptile |
| function resolveManifestProviderSetupFlowContributions(params?: { | ||
| config?: OpenClawConfig; | ||
| workspaceDir?: string; | ||
| env?: NodeJS.ProcessEnv; | ||
| scope?: ProviderFlowScope; | ||
| }): ProviderSetupFlowContribution[] { | ||
| const scope = params?.scope ?? DEFAULT_PROVIDER_FLOW_SCOPE; | ||
| return resolveManifestProviderAuthChoices({ | ||
| ...params, | ||
| includeUntrustedWorkspacePlugins: false, | ||
| }) | ||
| .filter((choice) => includesProviderFlowScope(choice.onboardingScopes, scope)) | ||
| .map((choice) => { | ||
| const groupId = choice.groupId ?? choice.providerId; | ||
| const groupLabel = choice.groupLabel ?? choice.choiceLabel; | ||
| return Object.assign( | ||
| { | ||
| id: `provider:setup:${choice.choiceId}`, | ||
| kind: `provider` as const, | ||
| surface: `setup` as const, | ||
| providerId: choice.providerId, | ||
| pluginId: choice.pluginId, | ||
| option: { | ||
| value: choice.choiceId, | ||
| label: choice.choiceLabel, | ||
| ...(choice.choiceHint ? { hint: choice.choiceHint } : {}), | ||
| ...(choice.assistantPriority !== undefined | ||
| ? { assistantPriority: choice.assistantPriority } | ||
| : {}), | ||
| ...(choice.assistantVisibility | ||
| ? { assistantVisibility: choice.assistantVisibility } | ||
| : {}), | ||
| group: { | ||
| id: groupId, | ||
| label: groupLabel, | ||
| ...(choice.groupHint ? { hint: choice.groupHint } : {}), | ||
| }, | ||
| }, | ||
| }, | ||
| choice.onboardingScopes ? { onboardingScopes: [...choice.onboardingScopes] } : {}, | ||
| { source: `manifest` as const }, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
resolveManifestProviderSetupFlowContributions never calls resolveEffectiveEnableState, so manifest choices from explicitly-disabled or non-allowlisted plugins will still appear in the setup flow. By contrast, resolveInstallCatalogProviderSetupFlowContributions filters every entry through resolveEffectiveEnableState({ ..., enabledByDefault: true }).
Concretely: with plugins: { enabled: false } or plugins: { allow: ["openai"] } in config, install-catalog contributions are correctly hidden (existing tests at lines 247–299 verify this), but the equivalent manifest contributions are not. resolveManifestProviderAuthChoiceCandidates only gates workspace-origin, untrusted plugins — bundled and global plugins bypass that check entirely.
The fix mirrors what the install-catalog path does: compute normalizedPluginsConfig at the top of the function and add a .filter that calls resolveEffectiveEnableState with enabledByDefault: true after the scope filter.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/flows/provider-flow.ts
Line: 125-168
Comment:
**Missing enable-state gate for manifest contributions**
`resolveManifestProviderSetupFlowContributions` never calls `resolveEffectiveEnableState`, so manifest choices from explicitly-disabled or non-allowlisted plugins will still appear in the setup flow. By contrast, `resolveInstallCatalogProviderSetupFlowContributions` filters every entry through `resolveEffectiveEnableState({ ..., enabledByDefault: true })`.
Concretely: with `plugins: { enabled: false }` or `plugins: { allow: ["openai"] }` in config, install-catalog contributions are correctly hidden (existing tests at lines 247–299 verify this), but the equivalent manifest contributions are not. `resolveManifestProviderAuthChoiceCandidates` only gates workspace-origin, untrusted plugins — bundled and global plugins bypass that check entirely.
The fix mirrors what the install-catalog path does: compute `normalizedPluginsConfig` at the top of the function and add a `.filter` that calls `resolveEffectiveEnableState` with `enabledByDefault: true` after the scope filter.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc40d517c7
ℹ️ 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".
| return resolveManifestProviderAuthChoices({ | ||
| ...params, | ||
| includeUntrustedWorkspacePlugins: false, | ||
| }) | ||
| .filter((choice) => includesProviderFlowScope(choice.onboardingScopes, scope)) |
There was a problem hiding this comment.
Respect plugin enable policy for manifest setup choices
Manifest-derived setup options are added with only a scope filter, so they bypass the enable/allowlist gating that install-catalog options use. In practice this makes resolveProviderSetupFlowContributions surface blocked providers (for example with plugins.enabled=false or a narrow plugins.allow list), and those options later fail when selection tries to enable the plugin. Please apply the same effective-enable policy to manifest choices before adding them (and before using them to suppress runtime/install entries).
Useful? React with 👍 / 👎.
Summary
providerAuthChoiceswere not a direct setup-flow source.source: "manifest", prefers them over duplicate runtime/install-catalog entries, and keeps runtime/install-catalog paths as compatibility fallback.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
src/flows/provider-flow.test.tsUser-visible / Behavior Changes
Provider setup choices declared in plugin manifests can appear in setup/onboarding flows without loading provider runtime, when no duplicate higher-priority option suppresses them.
Diagram (if applicable)
N/A
Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation: N/ARepro + Verification
Environment
Steps
pnpm test src/flows/provider-flow.test.ts.pnpm check:changed.Expected
Actual
pnpm check:changedpassed: 6 files, 30 tests.Evidence
Human Verification (required)
Review Conversations
Compatibility / Migration
Yes)No)No)Runtime wizard setup and install-catalog setup choices remain wired as compatibility fallback.
Risks and Mitigations