mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:40:44 +00:00
fix: harden external auth fallback loading
This commit is contained in:
@@ -580,6 +580,28 @@ describe("loadPluginManifestRegistry", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves external auth provider contracts from plugin manifests", () => {
|
||||
const dir = makeTempDir();
|
||||
writeManifest(dir, {
|
||||
id: "acme-ai",
|
||||
providers: ["acme-ai"],
|
||||
contracts: {
|
||||
externalAuthProviders: ["acme-ai"],
|
||||
},
|
||||
configSchema: { type: "object" },
|
||||
});
|
||||
|
||||
const registry = loadSingleCandidateRegistry({
|
||||
idHint: "acme-ai",
|
||||
rootDir: dir,
|
||||
origin: "bundled",
|
||||
});
|
||||
|
||||
expect(registry.plugins[0]?.contracts).toEqual({
|
||||
externalAuthProviders: ["acme-ai"],
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves channel env metadata from plugin manifests", () => {
|
||||
const dir = makeTempDir();
|
||||
writeManifest(dir, {
|
||||
|
||||
@@ -316,6 +316,7 @@ describe("provider-runtime", () => {
|
||||
|
||||
beforeEach(() => {
|
||||
resetProviderRuntimeHookCacheForTest();
|
||||
providerRuntimeTesting.resetExternalAuthFallbackWarningCacheForTest();
|
||||
resolvePluginProvidersMock.mockReset();
|
||||
resolvePluginProvidersMock.mockReturnValue([]);
|
||||
isPluginProvidersLoadInFlightMock.mockReset();
|
||||
@@ -394,6 +395,52 @@ describe("provider-runtime", () => {
|
||||
expect(resolvePluginProvidersMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("warns once with a log-safe plugin id for undeclared external auth fallback plugins", () => {
|
||||
const unsafePluginId = "legacy-provider\nWARN forged";
|
||||
resolveExternalAuthProfileCompatFallbackPluginIdsMock.mockReturnValue([unsafePluginId]);
|
||||
resolvePluginProvidersMock.mockReturnValue([
|
||||
{
|
||||
id: "legacy-provider",
|
||||
pluginId: unsafePluginId,
|
||||
label: "Legacy Provider",
|
||||
auth: [],
|
||||
resolveExternalOAuthProfiles: () => [
|
||||
{
|
||||
profileId: "legacy-provider:external",
|
||||
credential: {
|
||||
type: "oauth",
|
||||
provider: "legacy-provider",
|
||||
access: "access",
|
||||
refresh: "refresh",
|
||||
expires: Date.now() + 60_000,
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
for (let i = 0; i < 2; i += 1) {
|
||||
expect(
|
||||
resolveExternalAuthProfilesWithPlugins({
|
||||
env: process.env,
|
||||
context: {
|
||||
env: process.env,
|
||||
store: { version: 1, profiles: {} },
|
||||
},
|
||||
}),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
profileId: "legacy-provider:external",
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
expect(providerRuntimeWarnMock).toHaveBeenCalledTimes(1);
|
||||
const warning = String(providerRuntimeWarnMock.mock.calls[0]?.[0] ?? "");
|
||||
expect(warning).toContain('Provider plugin "legacy-providerWARN forged"');
|
||||
expect(warning).not.toContain("\n");
|
||||
});
|
||||
|
||||
it("does not warn for declared external auth plugins with different provider ids", () => {
|
||||
resolveExternalAuthProfileProviderPluginIdsMock.mockReturnValue(["demo-plugin"]);
|
||||
resolvePluginProvidersMock.mockReturnValue([
|
||||
|
||||
@@ -9,6 +9,7 @@ import type { ModelProviderConfig } from "../config/types.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { normalizeOptionalString } from "../shared/string-coerce.js";
|
||||
import { sanitizeForLog } from "../terminal/ansi.js";
|
||||
import {
|
||||
__testing as providerHookRuntimeTesting,
|
||||
clearProviderRuntimeHookCache,
|
||||
@@ -78,6 +79,11 @@ import type {
|
||||
|
||||
const log = createSubsystemLogger("plugins/provider-runtime");
|
||||
const warnedExternalAuthFallbackPluginIds = new Set<string>();
|
||||
|
||||
function resetExternalAuthFallbackWarningCacheForTest(): void {
|
||||
warnedExternalAuthFallbackPluginIds.clear();
|
||||
}
|
||||
|
||||
export {
|
||||
clearProviderRuntimeHookCache,
|
||||
prepareProviderExtraParams,
|
||||
@@ -88,6 +94,7 @@ export {
|
||||
|
||||
export const __testing = {
|
||||
...providerHookRuntimeTesting,
|
||||
resetExternalAuthFallbackWarningCacheForTest,
|
||||
} as const;
|
||||
|
||||
function resolveProviderPluginsForCatalogHooks(params: {
|
||||
@@ -770,10 +777,12 @@ export function resolveExternalAuthProfilesWithPlugins(params: {
|
||||
workspaceDir,
|
||||
env,
|
||||
});
|
||||
const declaredPluginIds = new Set(externalAuthPluginIds);
|
||||
const fallbackPluginIds = resolveExternalAuthProfileCompatFallbackPluginIds({
|
||||
config: params.config,
|
||||
workspaceDir,
|
||||
env,
|
||||
declaredPluginIds,
|
||||
});
|
||||
const pluginIds = [...new Set([...externalAuthPluginIds, ...fallbackPluginIds])].toSorted(
|
||||
(left, right) => left.localeCompare(right),
|
||||
@@ -781,7 +790,6 @@ export function resolveExternalAuthProfilesWithPlugins(params: {
|
||||
if (pluginIds.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const declaredPluginIds = new Set(externalAuthPluginIds);
|
||||
const matches: ProviderExternalAuthProfile[] = [];
|
||||
for (const plugin of resolveProviderPluginsForHooks({
|
||||
...params,
|
||||
@@ -802,7 +810,7 @@ export function resolveExternalAuthProfilesWithPlugins(params: {
|
||||
// resolveExternalOAuthProfiles or omit contracts.externalAuthProviders.
|
||||
// Remove this warning with the fallback resolver after the migration window.
|
||||
log.warn(
|
||||
`Provider plugin "${pluginId}" uses external auth hooks without declaring contracts.externalAuthProviders. This compatibility fallback is deprecated and will be removed in a future release.`,
|
||||
`Provider plugin "${sanitizeForLog(pluginId)}" uses external auth hooks without declaring contracts.externalAuthProviders. This compatibility fallback is deprecated and will be removed in a future release.`,
|
||||
);
|
||||
}
|
||||
matches.push(...profiles);
|
||||
|
||||
@@ -42,6 +42,7 @@ let resolveOwningPluginIdsForProvider: typeof import("./providers.js").resolveOw
|
||||
let resolveOwningPluginIdsForModelRef: typeof import("./providers.js").resolveOwningPluginIdsForModelRef;
|
||||
let resolveActivatableProviderOwnerPluginIds: typeof import("./providers.js").resolveActivatableProviderOwnerPluginIds;
|
||||
let resolveEnabledProviderPluginIds: typeof import("./providers.js").resolveEnabledProviderPluginIds;
|
||||
let resolveExternalAuthProfileCompatFallbackPluginIds: typeof import("./providers.js").resolveExternalAuthProfileCompatFallbackPluginIds;
|
||||
let resolveExternalAuthProfileProviderPluginIds: typeof import("./providers.js").resolveExternalAuthProfileProviderPluginIds;
|
||||
let resolveDiscoveredProviderPluginIds: typeof import("./providers.js").resolveDiscoveredProviderPluginIds;
|
||||
let resolveDiscoverableProviderOwnerPluginIds: typeof import("./providers.js").resolveDiscoverableProviderOwnerPluginIds;
|
||||
@@ -315,6 +316,7 @@ describe("resolvePluginProviders", () => {
|
||||
resolveOwningPluginIdsForProvider,
|
||||
resolveOwningPluginIdsForModelRef,
|
||||
resolveEnabledProviderPluginIds,
|
||||
resolveExternalAuthProfileCompatFallbackPluginIds,
|
||||
resolveExternalAuthProfileProviderPluginIds,
|
||||
resolveDiscoveredProviderPluginIds,
|
||||
resolveDiscoverableProviderOwnerPluginIds,
|
||||
@@ -435,6 +437,39 @@ describe("resolvePluginProviders", () => {
|
||||
expect(resolveRuntimePluginRegistryMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reuses declared external auth plugin ids for compat fallback filtering", () => {
|
||||
setManifestPlugins([
|
||||
createManifestProviderPlugin({
|
||||
id: "declared-auth-owner",
|
||||
providerIds: ["declared"],
|
||||
origin: "workspace",
|
||||
contracts: { externalAuthProviders: ["declared"] },
|
||||
}),
|
||||
createManifestProviderPlugin({
|
||||
id: "legacy-auth-owner",
|
||||
providerIds: ["legacy"],
|
||||
origin: "workspace",
|
||||
}),
|
||||
]);
|
||||
const declaredPluginIds = new Set(["declared-auth-owner"]);
|
||||
|
||||
expect(
|
||||
resolveExternalAuthProfileCompatFallbackPluginIds({
|
||||
config: {
|
||||
plugins: {
|
||||
entries: {
|
||||
"declared-auth-owner": { enabled: true },
|
||||
"legacy-auth-owner": { enabled: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
env: {} as NodeJS.ProcessEnv,
|
||||
declaredPluginIds,
|
||||
}),
|
||||
).toEqual(["legacy-auth-owner"]);
|
||||
expect(resolveManifestContractPluginIdsMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats explicit empty provider scopes as scoped-empty in provider helpers", () => {
|
||||
expect(
|
||||
resolveEnabledProviderPluginIds({
|
||||
|
||||
@@ -136,11 +136,13 @@ export function resolveExternalAuthProfileCompatFallbackPluginIds(params: {
|
||||
config?: PluginLoadOptions["config"];
|
||||
workspaceDir?: string;
|
||||
env?: PluginLoadOptions["env"];
|
||||
declaredPluginIds?: ReadonlySet<string>;
|
||||
}): string[] {
|
||||
// Deprecated compatibility fallback for provider plugins that still implement
|
||||
// resolveExternalOAuthProfiles or omit contracts.externalAuthProviders. Remove
|
||||
// this with the warning path in provider-runtime after the migration window.
|
||||
const declaredPluginIds = new Set(resolveExternalAuthProfileProviderPluginIds(params));
|
||||
const declaredPluginIds =
|
||||
params.declaredPluginIds ?? new Set(resolveExternalAuthProfileProviderPluginIds(params));
|
||||
const registry = loadProviderManifestRegistry(params);
|
||||
const normalizedConfig = normalizePluginsConfig(params.config?.plugins);
|
||||
return listManifestPluginIds(
|
||||
|
||||
Reference in New Issue
Block a user