diff --git a/docs/security/credential-storage.mdx b/docs/security/credential-storage.mdx index 5d16eca48de..813be727b46 100644 --- a/docs/security/credential-storage.mdx +++ b/docs/security/credential-storage.mdx @@ -195,6 +195,9 @@ You see a one-line stderr notice the first time this happens. Credential lookup paths such as rebuild also stage allowlisted legacy values so interrupted upgrades can keep working, but those staging-only paths do not delete the plaintext file because they cannot prove every legacy value was registered with the gateway. If `~/.nemoclaw/credentials.json` remains after a rebuild or other credential lookup, run `$$nemoclaw onboard` to complete the verified gateway migration and cleanup. +Onboarding also sweeps a leftover `~/.nemoclaw/credentials.json` that holds nothing to migrate, such as an empty file, an empty `{}`, or entries whose values are all blank. +The sweep keeps any file that still holds a value, including a value under a key NemoClaw does not recognize, because NemoClaw never read or migrated it. + ## Rotate or Remove a Stored Credential To replace a stored value, rerun onboarding with the new value in your environment: diff --git a/src/lib/credentials/legacy-env-aliases.ts b/src/lib/credentials/legacy-env-aliases.ts new file mode 100644 index 00000000000..4652b4d82bc --- /dev/null +++ b/src/lib/credentials/legacy-env-aliases.ts @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Legacy credential env aliases. +// +// A pre-gateway `~/.nemoclaw/credentials.json` can name a credential under an +// older env key than the one the gateway registers today. Credential +// resolution accepts the alias, so migration accounting has to recognize the +// same relationship or a value that did reach the gateway looks unmigrated +// (#10373). The table lives here, outside the credential store, so both the +// store and onboarding provider registration can read it. + +const LEGACY_CREDENTIAL_ENV_ALIASES: Partial> = { + NVIDIA_INFERENCE_API_KEY: ["NVIDIA_API_KEY"], +}; + +/** Legacy env keys whose stored value can satisfy `envName`. */ +export function legacyCredentialAliases(envName: string): readonly string[] { + return LEGACY_CREDENTIAL_ENV_ALIASES[envName] ?? []; +} diff --git a/src/lib/credentials/store.ts b/src/lib/credentials/store.ts index d2010c75661..6ce7eeff735 100644 --- a/src/lib/credentials/store.ts +++ b/src/lib/credentials/store.ts @@ -19,6 +19,7 @@ import { createPromptActivityCleanup } from "../core/prompt-activity"; import { listMessagingCredentialMetadata } from "../messaging/channels"; import { rejectSymlinksOnPath } from "../state/config-io"; import { nemoclawStateRoot } from "../state/state-root"; +import { legacyCredentialAliases } from "./legacy-env-aliases"; import { getScopedCredentialOverride } from "./scoped-overrides"; export { withCredentialOverrides } from "./scoped-overrides"; @@ -55,10 +56,6 @@ export const KNOWN_CREDENTIAL_ENV_KEYS: readonly string[] = [ ...listMessagingCredentialMetadata().map((credential) => credential.providerEnvKey), ]; -const LEGACY_CREDENTIAL_ENV_ALIASES: Partial> = { - NVIDIA_INFERENCE_API_KEY: ["NVIDIA_API_KEY"], -}; - // Hard upper bound on the legacy credentials.json size we are willing to // read into memory. The largest realistic credential set NemoClaw has ever // shipped is well under 1 KiB; the cap exists purely so an attacker who @@ -195,7 +192,7 @@ export function getCredential(key: string): string | null { } function getLegacyCredentialAlias(envName: string): string | null { - for (const alias of LEGACY_CREDENTIAL_ENV_ALIASES[envName] ?? []) { + for (const alias of legacyCredentialAliases(envName)) { const value = getCredential(alias); if (value) return value; } @@ -441,9 +438,8 @@ export function removeLegacyCredentialsFile(): void { /** * Securely remove the legacy plaintext credentials.json *iff* it carries - * no migratable credential payload — i.e. it's an empty `{}`, contains - * only keys outside `KNOWN_CREDENTIAL_ENV_KEYS`, or every allowlisted key - * has a blank/non-string value. Used by the onboard completion path to + * no payload at all — i.e. it's empty, whitespace-only, an empty `{}`, or + * every value is a blank string. Used by the onboard completion path to * clean up the stale empty file left behind on upgrades from pre-gateway * NemoClaw versions (#3105). * @@ -499,11 +495,12 @@ export function removeLegacyCredentialsFileIfEmpty(): boolean { return false; } - const allowed = new Set(KNOWN_CREDENTIAL_ENV_KEYS); - for (const [key, value] of Object.entries(parsed as Record)) { - if (!allowed.has(key)) continue; - if (typeof value !== "string") continue; - if (normalizeCredentialValue(value)) { + // Any surviving value is payload this sweep did not migrate, whether or + // not NemoClaw recognizes its key. Keys outside KNOWN_CREDENTIAL_ENV_KEYS + // used to be treated as absent, so a file holding only unrecognized + // secrets was destroyed without ever being read (#10373). + for (const value of Object.values(parsed as Record)) { + if (typeof value !== "string" || normalizeCredentialValue(value)) { return false; } } diff --git a/src/lib/host-artifact-cleanup.ts b/src/lib/host-artifact-cleanup.ts index a5144c4e186..b81fc87e0c1 100644 --- a/src/lib/host-artifact-cleanup.ts +++ b/src/lib/host-artifact-cleanup.ts @@ -28,7 +28,7 @@ interface StaleHostFile { const STALE_FILES: readonly StaleHostFile[] = [ { - description: "~/.nemoclaw/credentials.json (no migratable credentials)", + description: "~/.nemoclaw/credentials.json (no stored values)", tryRemove: removeLegacyCredentialsFileIfEmpty, }, ]; diff --git a/src/lib/onboard/credential-provider-registration.test.ts b/src/lib/onboard/credential-provider-registration.test.ts index 2095e40c984..3d3bd2048d1 100644 --- a/src/lib/onboard/credential-provider-registration.test.ts +++ b/src/lib/onboard/credential-provider-registration.test.ts @@ -345,6 +345,66 @@ describe("credential provider registration", () => { }, ); + it("records migration for the legacy alias the canonical credential resolved from (#10373)", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const deps = registrationDeps(runOpenshell, session); + deps.stagedLegacyValues = new Map([["NVIDIA_API_KEY", "nvapi-legacy"]]); + const registration = createCredentialProviderRegistration(deps); + + const result = registration.upsertProvider( + "nvidia-prod", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + { NVIDIA_INFERENCE_API_KEY: "nvapi-legacy" }, + ); + + expect(result).toEqual({ ok: true }); + expect(deps.migratedLegacyKeys).toEqual(new Set(["NVIDIA_API_KEY"])); + expect(deps.persistMigratedLegacyKeys).toHaveBeenCalledOnce(); + }); + + it("drops the legacy alias when the provider receives a different value (#10373)", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const deps = registrationDeps(runOpenshell, session); + deps.stagedLegacyValues = new Map([["NVIDIA_API_KEY", "nvapi-legacy"]]); + deps.migratedLegacyKeys.add("NVIDIA_API_KEY"); + const registration = createCredentialProviderRegistration(deps); + + registration.upsertProvider( + "nvidia-prod", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + { NVIDIA_INFERENCE_API_KEY: "nvapi-replacement" }, + ); + + expect(deps.migratedLegacyKeys).toEqual(new Set()); + }); + + it("records only the key whose staged value the gateway received (#10373)", () => { + const session = { stagedCredentialProviders: [] } as unknown as Session; + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" })); + const deps = registrationDeps(runOpenshell, session); + deps.stagedLegacyValues = new Map([ + ["NVIDIA_INFERENCE_API_KEY", "nvapi-canonical"], + ["NVIDIA_API_KEY", "nvapi-stale-alias"], + ]); + const registration = createCredentialProviderRegistration(deps); + + registration.upsertProvider( + "nvidia-prod", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + { NVIDIA_INFERENCE_API_KEY: "nvapi-canonical" }, + ); + + expect(deps.migratedLegacyKeys).toEqual(new Set(["NVIDIA_INFERENCE_API_KEY"])); + }); + it("does not record migration when provider registration fails", () => { const session = { stagedCredentialProviders: [] } as unknown as Session; const runOpenshell = vi.fn((args: string[]) => ({ diff --git a/src/lib/onboard/credential-provider-registration.ts b/src/lib/onboard/credential-provider-registration.ts index 363d36ea112..17e4d4c233f 100644 --- a/src/lib/onboard/credential-provider-registration.ts +++ b/src/lib/onboard/credential-provider-registration.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { legacyCredentialAliases } from "../credentials/legacy-env-aliases"; import type { WebSearchConfig } from "../inference/web-search"; import type { CheckpointProviderBinding } from "../state/onboard-checkpoint-types"; import type { Session } from "../state/onboard-session"; @@ -235,16 +236,25 @@ export function createCredentialProviderRegistration(deps: CredentialProviderReg options, ); if (result.ok && credentialEnv) { - const stagedValue = deps.stagedLegacyValues.get(credentialEnv); - if (stagedValue !== undefined) { + // The legacy file can carry the value under an alias of the canonical + // credential env (NVIDIA_API_KEY for NVIDIA_INFERENCE_API_KEY), which + // resolveProviderCredential resolves transparently. Account the alias + // too, or the staged key never looks migrated and the plaintext file + // survives an onboard that used it (#10373). + const migrationKeys = [credentialEnv, ...legacyCredentialAliases(credentialEnv)].filter( + (key) => deps.stagedLegacyValues.has(key), + ); + if (migrationKeys.length > 0) { options.revalidateSandboxIdentity?.( `record migrated credential for provider ${JSON.stringify(name)}`, ); const upsertedValue = env[credentialEnv] ?? deps.getCredential(credentialEnv); - if (upsertedValue === stagedValue) { - deps.migratedLegacyKeys.add(credentialEnv); - } else { - deps.migratedLegacyKeys.delete(credentialEnv); + for (const key of migrationKeys) { + if (upsertedValue === deps.stagedLegacyValues.get(key)) { + deps.migratedLegacyKeys.add(key); + } else { + deps.migratedLegacyKeys.delete(key); + } } deps.persistMigratedLegacyKeys(); } diff --git a/test/credentials/credential-migration-reconciliation.test.ts b/test/credentials/credential-migration-reconciliation.test.ts index e9f38af80e2..c26698cfd37 100644 --- a/test/credentials/credential-migration-reconciliation.test.ts +++ b/test/credentials/credential-migration-reconciliation.test.ts @@ -8,6 +8,7 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { removeLegacyCredentialsFile, + resolveProviderCredential, stageLegacyCredentialsToEnv, } from "../../src/lib/credentials/store.js"; import { @@ -205,4 +206,74 @@ describe("legacy credential reconciliation", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + it("removes the plaintext file after an aliased legacy key reaches the gateway (#10373)", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-credential-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 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; + 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); + + // The build provider registers the canonical key; the legacy file named the alias. + const resolved = resolveProviderCredential("NVIDIA_INFERENCE_API_KEY"); + registration.upsertProvider( + "nvidia-prod", + "nvidia", + "NVIDIA_INFERENCE_API_KEY", + "https://integrate.api.nvidia.com/v1", + { NVIDIA_INFERENCE_API_KEY: resolved ?? "" }, + ); + + await finalizeMigration(stagedLegacyKeys, migratedLegacyKeys); + + expect(stagedLegacyKeys).toEqual(["NVIDIA_API_KEY"]); + expect(resolved).toBe(LEGACY_SECRET); + expect(migratedLegacyKeys.has("NVIDIA_API_KEY")).toBe(true); + expect( + fs.existsSync(legacyFile), + "a legacy credential the gateway accepted must not stay in plaintext", + ).toBe(false); + }, + ); + } finally { + error.mockRestore(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/test/credentials/credentials.test.ts b/test/credentials/credentials.test.ts index 561ac9d8b44..972e7aa8f75 100644 --- a/test/credentials/credentials.test.ts +++ b/test/credentials/credentials.test.ts @@ -572,18 +572,30 @@ describe("removeLegacyCredentialsFileIfEmpty post-upgrade cleanup (#3105)", () = expect(fs.existsSync(legacyFile)).toBe(false); }); - it("removes a file containing only unknown keys", async () => { + it("keeps a file whose only content is an unrecognized credential (#10373)", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); const credsDir = path.join(home, ".nemoclaw"); const legacyFile = path.join(credsDir, "credentials.json"); fs.mkdirSync(credsDir, { recursive: true }); - fs.writeFileSync(legacyFile, JSON.stringify({ FOO: "bar", PATH: "/etc/passwd" }), { - mode: 0o600, - }); + const payload = JSON.stringify({ FAKE_PROVIDER_TOKEN: "x" }); + fs.writeFileSync(legacyFile, payload, { mode: 0o600 }); const credentials = await importCredentialsModule(home); - expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(true); - expect(fs.existsSync(legacyFile)).toBe(false); + expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false); + expect(fs.readFileSync(legacyFile, "utf-8")).toBe(payload); + }); + + it("keeps a file holding a non-string value it cannot classify (#10373)", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + const payload = JSON.stringify({ OPENAI_API_KEY: { nested: "secret" } }); + fs.writeFileSync(legacyFile, payload, { mode: 0o600 }); + + const credentials = await importCredentialsModule(home); + expect(credentials.removeLegacyCredentialsFileIfEmpty()).toBe(false); + expect(fs.readFileSync(legacyFile, "utf-8")).toBe(payload); }); it("removes a file where every allowlisted value is blank/whitespace", async () => {