fix(config): strip legacy installs/plugins from channel configs before validation by giulio-leone · Pull Request #63474 · openclaw/openclaw

giulio-leone · 2026-04-09T01:33:01Z

Summary

After upgrading from v4.5 to v4.8, openclaw gateway restart fails with:

channels.feishu: invalid config: must NOT have additional properties

when legacy installs and plugins fields are present under channels.feishu.

Root Cause

The Feishu channel config schema (FeishuConfigSchema) uses Zod .strict() mode, which rejects any properties not explicitly declared in the schema. The installs and plugins fields were never valid channel-level properties but could appear in v4.5-era configs due to top-level key nesting.

Fix

Add a legacy config migration (channels.*.stale-top-level-keys) that auto-strips installs and plugins from every channels.* entry (including nested accounts.*) before schema validation runs. The migration:

Logs a clear change message so operators know the fields were removed
Applies generically to all channels (not just Feishu) since these fields are never valid at the channel level
Includes a legacy rule that triggers a warning for channels.feishu specifically

Tests

6 regression tests for the migration in src/config/legacy-migrate.test.ts
3 schema-level rejection tests in extensions/feishu/src/config-schema.test.ts

All 51 tests pass.

…e validation (openclaw#63101) After upgrading from v4.5 to v4.8, gateway restart fails with "channels.feishu: invalid config: must NOT have additional properties" when legacy `installs` and `plugins` fields are present under a channel config block. These fields were never valid channel-level properties but could appear in v4.5-era configs due to top-level key nesting. Add a legacy config migration that auto-strips `installs` and `plugins` from every `channels.*` entry (including nested `accounts.*`) before schema validation runs. The migration logs a clear change message so the operator knows the fields were removed. - Add migration `channels.*.stale-top-level-keys` with legacy rule for `channels.feishu` - Add 6 regression tests for the migration in legacy-migrate.test.ts - Add 3 schema-level rejection tests in config-schema.test.ts Closes openclaw#63101 Co-authored-by: Copilot <[email protected]>

greptile-apps · 2026-04-09T01:36:52Z

Greptile Summary

This PR adds a legacy config migration (channels.*.stale-top-level-keys) that strips installs and plugins from all channels.* entries (including nested accounts.*) before schema validation runs, fixing the v4.5 → v4.8 upgrade breakage reported in #63101. The migration is registered first in LEGACY_CONFIG_MIGRATIONS_CHANNELS, ensuring stale key removal precedes other channel migrations (streaming keys, thread bindings).

Confidence Score: 5/5

Safe to merge — the migration is correctly ordered, applies to all channels, uses established in-place mutation patterns, and is well-tested.

All findings are P2 style observations. The migration logic itself is correct: stale keys are deleted before schema validation runs, the function is registered first in the migrations list, prototype-safe key checks are used throughout, and the apply/rules split (all channels stripped, feishu-only warning) is intentional and documented in the PR description.

No files require special attention.

Vulnerabilities

No security concerns identified. The migration uses hasOwnKey (backed by Object.prototype.hasOwnProperty) for key checks, avoiding prototype pollution. The delete operations are scoped to known string keys, and structuredClone in applyLegacyMigrations prevents the original config from being mutated.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/config/legacy-migrate.test.ts
Line: 334-441

Comment:
**Test helper inconsistency with rest of file**

All other `describe` blocks in this file (`legacy migrate audio transcription`, `legacy migrate heartbeat config`, `legacy migrate controlUi.allowedOrigins seed`) call `migrateLegacyConfig` (which wraps `applyLegacyMigrations` with schema validation). The new stale-channel-keys suite calls `applyLegacyMigrations` directly instead.

Using `migrateLegacyConfig` here would also validate that the stripped config survives schema validation end-to-end — the most important invariant for the fix. The tests would need `appId`/`appSecret` values sufficient for the schema to accept the result, but most test fixtures already include them.

Not a blocking issue since the migration logic is still covered, but using the higher-level wrapper would add integration-level coverage consistent with the file's established pattern.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(config): strip legacy installs/plugi..." | Re-trigger Greptile}

greptile-apps · 2026-04-09T01:36:56Z

+describe("legacy migrate stale channel keys (issue #63101)", () => {
+  it("strips installs from channels.feishu", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        feishu: {
+          appId: "cli_test",
+          appSecret: "secret_test", // pragma: allowlist secret
+          installs: [{ source: "npm", spec: "@openclaw/[email protected]" }],
+        },
+      },
+    });
+
+    expect(next).not.toBeNull();
+    expect(changes.length).toBeGreaterThan(0);
+    expect(changes.some((c) => c.includes("channels.feishu.installs"))).toBe(true);
+    const feishu = (next?.channels as Record<string, Record<string, unknown>>)?.feishu;
+    expect(feishu).toBeDefined();
+    expect(Object.prototype.hasOwnProperty.call(feishu, "installs")).toBe(false);
+    expect(feishu?.appId).toBe("cli_test");
+  });
+
+  it("strips plugins from channels.feishu", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        feishu: {
+          appId: "cli_test",
+          appSecret: "secret_test", // pragma: allowlist secret
+          plugins: { allow: ["@openclaw/plugin-feishu"] },
+        },
+      },
+    });
+
+    expect(next).not.toBeNull();
+    expect(changes.some((c) => c.includes("channels.feishu.plugins"))).toBe(true);
+    const feishu = (next?.channels as Record<string, Record<string, unknown>>)?.feishu;
+    expect(Object.prototype.hasOwnProperty.call(feishu, "plugins")).toBe(false);
+  });
+
+  it("strips both installs and plugins from channels.feishu", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        feishu: {
+          appId: "cli_test",
+          appSecret: "secret_test", // pragma: allowlist secret
+          installs: [{ source: "npm", spec: "@openclaw/[email protected]" }],
+          plugins: { allow: ["@openclaw/plugin-feishu"] },
+        },
+      },
+    });
+
+    expect(next).not.toBeNull();
+    expect(changes.filter((c) => c.includes("channels.feishu")).length).toBe(2);
+    const feishu = (next?.channels as Record<string, Record<string, unknown>>)?.feishu;
+    expect(Object.prototype.hasOwnProperty.call(feishu, "installs")).toBe(false);
+    expect(Object.prototype.hasOwnProperty.call(feishu, "plugins")).toBe(false);
+  });
+
+  it("does not modify channel config without stale keys", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        feishu: {
+          appId: "cli_test",
+          appSecret: "secret_test", // pragma: allowlist secret
+        },
+      },
+    });
+
+    expect(next).toBeNull();
+    expect(changes).toHaveLength(0);
+  });
+
+  it("strips stale keys from other channels too", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        slack: {
+          botToken: "xoxb-test",
+          installs: [{ source: "npm", spec: "@openclaw/[email protected]" }],
+        },
+      },
+    });
+
+    expect(next).not.toBeNull();
+    expect(changes.some((c) => c.includes("channels.slack.installs"))).toBe(true);
+  });
+
+  it("strips stale keys from nested account configs", () => {
+    const { next, changes } = applyLegacyMigrations({
+      channels: {
+        feishu: {
+          appId: "cli_test",
+          appSecret: "secret_test", // pragma: allowlist secret
+          accounts: {
+            main: {
+              appId: "cli_main",
+              plugins: { allow: ["@openclaw/plugin-feishu"] },
+            },
+          },
+        },
+      },
+    });
+
+    expect(next).not.toBeNull();
+    expect(changes.some((c) => c.includes("channels.feishu.accounts.main.plugins"))).toBe(true);
+    const accounts = (next?.channels as Record<string, Record<string, unknown>>)?.feishu
+      ?.accounts as Record<string, Record<string, unknown>>;
+    expect(Object.prototype.hasOwnProperty.call(accounts?.main, "plugins")).toBe(false);
+  });
+});


Test helper inconsistency with rest of file

All other describe blocks in this file (legacy migrate audio transcription, legacy migrate heartbeat config, legacy migrate controlUi.allowedOrigins seed) call migrateLegacyConfig (which wraps applyLegacyMigrations with schema validation). The new stale-channel-keys suite calls applyLegacyMigrations directly instead.

Using migrateLegacyConfig here would also validate that the stripped config survives schema validation end-to-end — the most important invariant for the fix. The tests would need appId/appSecret values sufficient for the schema to accept the result, but most test fixtures already include them.

Not a blocking issue since the migration logic is still covered, but using the higher-level wrapper would add integration-level coverage consistent with the file's established pattern.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/config/legacy-migrate.test.ts Line: 334-441 Comment: **Test helper inconsistency with rest of file** All other `describe` blocks in this file (`legacy migrate audio transcription`, `legacy migrate heartbeat config`, `legacy migrate controlUi.allowedOrigins seed`) call `migrateLegacyConfig` (which wraps `applyLegacyMigrations` with schema validation). The new stale-channel-keys suite calls `applyLegacyMigrations` directly instead. Using `migrateLegacyConfig` here would also validate that the stripped config survives schema validation end-to-end — the most important invariant for the fix. The tests would need `appId`/`appSecret` values sufficient for the schema to accept the result, but most test fixtures already include them. Not a blocking issue since the migration logic is still covered, but using the higher-level wrapper would add integration-level coverage consistent with the file's established pattern. How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 886aa1abae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-09T01:37:02Z

+    path: ["channels", "feishu"],
+    message:
+      'channels.feishu contains legacy "installs" or "plugins" keys that are not valid channel properties (auto-removed on load).',
+    match: (value) => hasStaleChannelKeys(value),
+  },


Match stale-key legacy rule for all channel configs

The load path only applies migrations when findLegacyConfigIssues finds at least one matching legacy rule, but this new rule is limited to path: ["channels", "feishu"]. As a result, configs that still have channels.<other>.installs/plugins (for example channels.slack.installs) never trigger migration and will continue to fail strict channel validation, so the new channels.*.stale-top-level-keys migration is not reachable for the non-Feishu cases it is intended to cover.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-09T01:37:03Z

+function hasStaleChannelKeys(channelValue: unknown): boolean {
+  const entry = getRecord(channelValue);
+  return entry !== null && CHANNEL_LEVEL_STALE_KEYS.some((key) => hasOwnKey(entry, key));


Detect stale keys under feishu account entries

The legacy-rule matcher only checks installs/plugins on the top-level channels.feishu object and ignores channels.feishu.accounts.<id>.*. If stale keys exist only inside an account block, no legacy issue is reported, the read flow skips migrations, and strict plugin schema validation will still fail on those account-level extra properties even though the migration includes code to strip them.

Useful? React with 👍 / 👎.

clawsweeper · 2026-04-29T04:21:34Z

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open: the upgrade bug is valid and the PR has useful direction, but it is not merge-ready because current main moved the live doctor migration path, the rule/mutation scope is incomplete and compatibility-sensitive, and external real behavior proof is missing.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: Feishu channel schemas are strict and plugin-aware validation rejects additional channels.feishu keys. I did not run a live gateway because this was a read-only review.

Is this the best way to solve the issue?

No, not as written: the useful migration needs to move to the current doctor path and be scoped so it fixes the reported upgrade state without globally deleting possible plugin-owned channel fields.

Security review:

Security review cleared: No dependency, workflow, secret-handling, package, or code-execution surface was added; the remaining concern is compatibility, not supply chain security.

AGENTS.md: found and applied where relevant.

What I checked:

stale F-rated PR: PR was opened 2026-04-09T01:33:01Z, is older than 30 days, and the latest review rated it F.
proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

steipete: Recent commits maintain the current doctor migration path and moved channel doctor migrations to plugin-aware doctor surfaces. (role: recent area contributor; confidence: high; commits: 921a5416e4a6, d1b514af2e2c, d07f50802027; files: src/commands/doctor/shared/legacy-config-migrations.channels.ts, src/commands/doctor/shared/legacy-config-migrate.test.ts)
giodl73-repo: Authored recent Feishu account compatibility migration in the same current doctor migration file. (role: adjacent Feishu doctor migration contributor; confidence: medium; commits: 9e8cc7e07757; files: src/commands/doctor/shared/legacy-config-migrations.channels.ts)
vincentkoc: Authored recent doctor compatibility work for channel config downgrade behavior in the same migration area. (role: adjacent doctor compatibility contributor; confidence: medium; commits: 43cb94a39aa1; files: src/commands/doctor/shared/legacy-config-migrations.channels.ts)
glfruit: Recently changed Feishu config schema behavior, making them relevant for schema-level compatibility review. (role: recent Feishu schema contributor; confidence: medium; commits: 5cfb578cbaf6; files: extensions/feishu/src/config-schema.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 45144ce2e856.

clawsweeper · 2026-05-20T17:38:05Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

openclaw-barnacle · 2026-06-04T04:57:47Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

openclaw-barnacle · 2026-06-12T05:06:39Z

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

openclaw-barnacle Bot added channel: feishu Channel integration: feishu size: M labels Apr 9, 2026

greptile-apps Bot reviewed Apr 9, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 9, 2026

View reviewed changes

clawsweeper Bot mentioned this pull request Apr 27, 2026

[Bug] Feishu channel config validation fails after upgrading from v4.5 to v4.8 #63101

Closed

clawsweeper Bot mentioned this pull request May 12, 2026

[Bug]: openclaw 2026.4.11 also not resolve feishu configure issue. After upgrade, Config invalid File: ~/.openclaw/openclaw.json Problem: - channels.feishu: invalid config #65786

Closed

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 19, 2026

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 4, 2026

luoyanglang mentioned this pull request Jun 7, 2026

ci: check bundled channel config metadata #91134

Merged

openclaw-barnacle Bot closed this Jun 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(config): strip legacy installs/plugins from channel configs before validation#63474

fix(config): strip legacy installs/plugins from channel configs before validation#63474
giulio-leone wants to merge 1 commit into
openclaw:mainfrom
giulio-leone:fix/feishu-legacy-config-strict-63101

giulio-leone commented Apr 9, 2026

Uh oh!

greptile-apps Bot commented Apr 9, 2026

Vulnerabilities

Uh oh!

greptile-apps Bot Apr 9, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 9, 2026

Uh oh!

chatgpt-codex-connector Bot Apr 9, 2026

Uh oh!

clawsweeper Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 4, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

giulio-leone commented Apr 9, 2026

Summary

Root Cause

Fix

Tests

Uh oh!

greptile-apps Bot commented Apr 9, 2026

Greptile Summary

Confidence Score: 5/5

Vulnerabilities

Uh oh!

greptile-apps Bot Apr 9, 2026

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 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 4, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Apr 29, 2026 •

edited

Loading