diff --git a/src/lib/onboard/credential-provider-registration.test.ts b/src/lib/onboard/credential-provider-registration.test.ts index 2095e40c984..f835183f718 100644 --- a/src/lib/onboard/credential-provider-registration.test.ts +++ b/src/lib/onboard/credential-provider-registration.test.ts @@ -345,6 +345,27 @@ describe("credential provider registration", () => { }, ); + it("records migration under the staged legacy alias key when the provider registers with the canonical env name (#10388)", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const deps = registrationDeps(runOpenshell, session); + deps.getCredential = vi.fn(() => "legacy-key"); + deps.stagedLegacyValues = new Map([["NVIDIA_API_KEY", "legacy-key"]]); + const registration = createCredentialProviderRegistration(deps); + + const result = registration.upsertProvider( + "nvidia-build", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + ); + + expect(result).toEqual({ ok: true }); + expect(deps.migratedLegacyKeys.has("NVIDIA_API_KEY")).toBe(true); + expect(deps.migratedLegacyKeys.has("NVIDIA_INFERENCE_API_KEY")).toBe(false); + expect(deps.persistMigratedLegacyKeys).toHaveBeenCalledOnce(); + }); + it("does not record migration when provider registration fails", () => { const session = { stagedCredentialProviders: [] } as unknown as Session; const runOpenshell = vi.fn((args: string[]) => ({ @@ -940,4 +961,28 @@ describe("credential provider registration", () => { expect(deps.migratedLegacyKeys).toEqual(new Set()); expect(deps.persistMigratedLegacyKeys).not.toHaveBeenCalled(); }); + + it("records messaging migration under the staged legacy alias key when the token def uses the canonical env name (#10388)", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const missing = { status: 1, stdout: "", stderr: "not found" }; + const success = { status: 0, stdout: "", stderr: "" }; + const runOpenshell = vi.fn((args: string[]) => + args[1] === "get" ? missing : success, + ); + const deps = registrationDeps(runOpenshell, session); + deps.stagedLegacyValues = new Map([["NVIDIA_API_KEY", "legacy-key"]]); + const registration = createCredentialProviderRegistration(deps); + + registration.upsertMessagingProviders([ + { + name: "alpha-nvidia-bridge", + envKey: "NVIDIA_INFERENCE_API_KEY", + token: "legacy-key", + }, + ]); + + expect(deps.migratedLegacyKeys.has("NVIDIA_API_KEY")).toBe(true); + expect(deps.migratedLegacyKeys.has("NVIDIA_INFERENCE_API_KEY")).toBe(false); + expect(deps.persistMigratedLegacyKeys).toHaveBeenCalledOnce(); + }); }); diff --git a/src/lib/onboard/credential-provider-registration.ts b/src/lib/onboard/credential-provider-registration.ts index 363d36ea112..780494c407a 100644 --- a/src/lib/onboard/credential-provider-registration.ts +++ b/src/lib/onboard/credential-provider-registration.ts @@ -125,6 +125,25 @@ export interface CredentialProviderRegistrationDeps { persistMigratedLegacyKeys(): void; } +// stagedLegacyValues is keyed by the literal env key found in the legacy +// credentials.json (e.g. NVIDIA_API_KEY), but a provider can consume that +// value under a different env key instead (e.g. NVIDIA_INFERENCE_API_KEY, +// its canonical alias). Match the exact key first, then fall back to any +// staged key whose value equals what the provider actually registered, so +// migration is still recorded against whichever key was actually staged. +function findStagedLegacyKey( + envKey: string, + value: string | undefined, + stagedLegacyValues: ReadonlyMap, +): string | undefined { + if (stagedLegacyValues.has(envKey)) return envKey; + if (value === undefined) return undefined; + for (const [key, stagedValue] of stagedLegacyValues) { + if (stagedValue === value) return key; + } + return undefined; +} + function recordMigratedLegacyMessagingCredentials( tokenDefs: readonly MessagingTokenDef[], registeredProviderNames: readonly string[], @@ -132,18 +151,19 @@ function recordMigratedLegacyMessagingCredentials( revalidateSandboxIdentity?: (operation: string) => void, ): void { const registeredProviders = new Set(registeredProviderNames); - const migrations: Array<{ envKey: string; migrated: boolean }> = []; + const migrations: Array<{ stagedKey: string; migrated: boolean }> = []; for (const def of tokenDefs) { if (!registeredProviders.has(def.name) || !def.token || !def.envKey) continue; - const stagedValue = deps.stagedLegacyValues.get(def.envKey); - if (stagedValue === undefined) continue; - migrations.push({ envKey: def.envKey, migrated: def.token === stagedValue }); + const stagedKey = findStagedLegacyKey(def.envKey, def.token, deps.stagedLegacyValues); + if (stagedKey === undefined) continue; + const stagedValue = deps.stagedLegacyValues.get(stagedKey); + migrations.push({ stagedKey, migrated: def.token === stagedValue }); } if (migrations.length === 0) return; revalidateSandboxIdentity?.("record migrated messaging provider credentials"); for (const migration of migrations) { - if (migration.migrated) deps.migratedLegacyKeys.add(migration.envKey); - else deps.migratedLegacyKeys.delete(migration.envKey); + if (migration.migrated) deps.migratedLegacyKeys.add(migration.stagedKey); + else deps.migratedLegacyKeys.delete(migration.stagedKey); } deps.persistMigratedLegacyKeys(); } @@ -235,16 +255,17 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg options, ); if (result.ok && credentialEnv) { - const stagedValue = deps.stagedLegacyValues.get(credentialEnv); - if (stagedValue !== undefined) { + const upsertedValue = env[credentialEnv] ?? deps.getCredential(credentialEnv) ?? undefined; + const stagedKey = findStagedLegacyKey(credentialEnv, upsertedValue, deps.stagedLegacyValues); + if (stagedKey !== undefined) { options.revalidateSandboxIdentity?.( `record migrated credential for provider ${JSON.stringify(name)}`, ); - const upsertedValue = env[credentialEnv] ?? deps.getCredential(credentialEnv); + const stagedValue = deps.stagedLegacyValues.get(stagedKey); if (upsertedValue === stagedValue) { - deps.migratedLegacyKeys.add(credentialEnv); + deps.migratedLegacyKeys.add(stagedKey); } else { - deps.migratedLegacyKeys.delete(credentialEnv); + deps.migratedLegacyKeys.delete(stagedKey); } deps.persistMigratedLegacyKeys(); } diff --git a/test/credentials/credential-migration-reconciliation.test.ts b/test/credentials/credential-migration-reconciliation.test.ts index e9f38af80e2..dc6fd3b5d9f 100644 --- a/test/credentials/credential-migration-reconciliation.test.ts +++ b/test/credentials/credential-migration-reconciliation.test.ts @@ -205,4 +205,76 @@ describe("legacy credential reconciliation", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + it("removes plaintext when the provider registers the canonical env name for a legacy alias key (#10388)", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-credential-migration-alias-")); + const legacyDir = path.join(tmpDir, ".nemoclaw"); + const legacyFile = path.join(legacyDir, "credentials.json"); + fs.mkdirSync(legacyDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync(legacyFile, JSON.stringify({ NVIDIA_API_KEY: LEGACY_SECRET }), { + mode: 0o600, + }); + const exit = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new Error(`gateway registration exited ${String(code)}`); + }); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + try { + await withProcessEnv( + { HOME: tmpDir, NVIDIA_API_KEY: undefined, NVIDIA_INFERENCE_API_KEY: undefined }, + async () => { + const stagedLegacyKeys = stageLegacyCredentialsToEnv(); + const stagedLegacyValues = new Map( + stagedLegacyKeys.map((key) => [key, process.env[key] ?? ""]), + ); + const migratedLegacyKeys = new Set(); + const session = { stagedCredentialProviders: [] } as unknown as Session; + // Onboarding resolves a staged legacy alias into the canonical env var + // before registering the provider (see credentials/store.ts ensureApiKey()). + process.env.NVIDIA_INFERENCE_API_KEY = process.env.NVIDIA_API_KEY; + const runOpenshell = vi.fn((args: string[]) => ({ + status: args.slice(0, 2).join(" ") === "provider get" ? 1 : 0, + stdout: "", + stderr: "", + })); + const deps: CredentialProviderRegistrationDeps = { + root: path.join(import.meta.dirname, "../.."), + runOpenshell: + runOpenshell as unknown as CredentialProviderRegistrationDeps["runOpenshell"], + redact: (input) => input, + getGatewayName: () => "nemoclaw", + getCredential: (name) => process.env[name] ?? null, + normalizeCredentialValue: (value) => (typeof value === "string" ? value.trim() : ""), + updateSession: (mutator) => mutator(session) ?? session, + stagedLegacyValues, + migratedLegacyKeys, + persistMigratedLegacyKeys: () => undefined, + }; + const registration = createCredentialProviderRegistration(deps); + + const result = registration.upsertProvider( + "nvidia-build", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + { NVIDIA_INFERENCE_API_KEY: process.env.NVIDIA_INFERENCE_API_KEY ?? "" }, + ); + expect(result).toEqual({ ok: true }); + + await finalizeMigration(stagedLegacyKeys, migratedLegacyKeys); + + expect(stagedLegacyKeys).toEqual(["NVIDIA_API_KEY"]); + expect(migratedLegacyKeys.has("NVIDIA_API_KEY")).toBe(true); + expect(exit).not.toHaveBeenCalled(); + expect( + fs.existsSync(legacyFile), + "successful alias-based registration must remove the legacy file", + ).toBe(false); + }, + ); + } finally { + error.mockRestore(); + exit.mockRestore(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); });