style(web): tighten the in-memory caveat and punctuate the footer
Reads "Memory (this session only): Secrets are not written anywhere and are lost on exit." — shorter tail than "lost when the Inspector exits", and a colon separating the store from what follows it. The colon is added at the footer's composition, not inside `secretStorageLabel`. The label is also consumed alone by `secretStorageSummary`, which appends " at <path>" for a file-backed store, so a colon baked into the label would print "File (encrypted): at /path" in the startup banner. It renders only when a detail actually follows. The caveat string is shared with the banner's warning line, so the terminal picks up the shorter wording too. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
0cb59cf3fd
commit
2d2c787e22
1 changed files with 123 additions and 14 deletions
|
|
@ -18,6 +18,7 @@
|
|||
* which is what makes them worth exporting.
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { existsSync } from "node:fs";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
|
|
@ -27,6 +28,10 @@ import {
|
|||
parseSecretStoreEnv,
|
||||
warnAboutSecretStorage,
|
||||
} from "@inspector/core/auth/node/secret-store-selection.js";
|
||||
import {
|
||||
InMemorySecretStore,
|
||||
type SecretStore,
|
||||
} from "@inspector/core/auth/node/secret-store.js";
|
||||
import type { SecretStorageInfo } from "@inspector/core/auth/secret-storage-info.js";
|
||||
|
||||
const ENV_KEYS = [
|
||||
|
|
@ -384,7 +389,7 @@ describe("warnAboutSecretStorage", () => {
|
|||
expect(output).toContain("MCP_INSPECTOR_SECRET_STORE");
|
||||
// The lossy-store caveat rides along — this is the "loud" half of
|
||||
// "automatic fallback with a loud banner".
|
||||
expect(output).toContain("lost when the Inspector exits");
|
||||
expect(output).toContain("lost on exit");
|
||||
});
|
||||
|
||||
it("warns about an unencrypted file even when it was explicitly chosen", () => {
|
||||
|
|
@ -540,6 +545,107 @@ describe("defaultSecretFilePath", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("absorbFileSecretsIntoKeyring", () => {
|
||||
// The transition: a box without libsecret falls back to a file, the user
|
||||
// saves secrets there, then installs libsecret. Without a hand-off the next
|
||||
// run selects the keychain and every saved value silently disappears —
|
||||
// still on disk, but read by nothing.
|
||||
async function seedFile(secrets: Record<string, string>): Promise<string> {
|
||||
const filePath = path.join(tmpDir, "secrets.json");
|
||||
await fs.writeFile(
|
||||
filePath,
|
||||
JSON.stringify({ version: 1, encryption: "none", secrets }),
|
||||
"utf-8",
|
||||
);
|
||||
return filePath;
|
||||
}
|
||||
|
||||
it("moves the file's secrets into the keychain and removes the file", async () => {
|
||||
const filePath = await seedFile({ "srv:oauthClientSecret": "from-file" });
|
||||
process.env.MCP_INSPECTOR_SECRET_FILE = filePath;
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const mod = await loadWithProbe(true);
|
||||
const keyring = new InMemorySecretStore();
|
||||
|
||||
await mod.absorbFileSecretsIntoKeyring(keyring);
|
||||
|
||||
expect(await keyring.get("srv", "oauthClientSecret")).toBe("from-file");
|
||||
expect(existsSync(filePath)).toBe(false);
|
||||
});
|
||||
|
||||
it("lets the keychain win on conflict", async () => {
|
||||
// The file's copy is the older one by construction — it was written while
|
||||
// the keychain was unreachable.
|
||||
const filePath = await seedFile({ "srv:oauthClientSecret": "from-file" });
|
||||
process.env.MCP_INSPECTOR_SECRET_FILE = filePath;
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const mod = await loadWithProbe(true);
|
||||
const keyring = new InMemorySecretStore();
|
||||
await keyring.set("srv", "oauthClientSecret", "already-here");
|
||||
|
||||
await mod.absorbFileSecretsIntoKeyring(keyring);
|
||||
|
||||
expect(await keyring.get("srv", "oauthClientSecret")).toBe("already-here");
|
||||
});
|
||||
|
||||
it("leaves the file alone when the keychain write fails", async () => {
|
||||
// A partial hand-off must not delete the source: the next run retries and
|
||||
// the keychain-wins rule absorbs whatever already made it across.
|
||||
const filePath = await seedFile({ "srv:oauthClientSecret": "from-file" });
|
||||
process.env.MCP_INSPECTOR_SECRET_FILE = filePath;
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const mod = await loadWithProbe(true);
|
||||
const failing: SecretStore = {
|
||||
get: async () => null,
|
||||
set: async () => {
|
||||
throw new Error("keychain exploded");
|
||||
},
|
||||
delete: async () => {},
|
||||
deleteAllForServer: async () => {},
|
||||
};
|
||||
|
||||
await mod.absorbFileSecretsIntoKeyring(failing);
|
||||
|
||||
expect(existsSync(filePath)).toBe(true);
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining("left in place"));
|
||||
});
|
||||
|
||||
it("does not delete a file it cannot read", async () => {
|
||||
// An undecryptable file is not an empty one. Deleting it here would lose
|
||||
// exactly the values this whole hand-off exists to preserve.
|
||||
const filePath = path.join(tmpDir, "secrets.json");
|
||||
process.env.MCP_INSPECTOR_SECRET_FILE = filePath;
|
||||
process.env.MCP_INSPECTOR_SECRET_KEY = "right-key";
|
||||
const seeded = await loadWithProbe(false);
|
||||
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
await new (
|
||||
await import("@inspector/core/auth/node/file-secret-store.js")
|
||||
).FileSecretStore({ filePath, passphrase: "right-key" }).set(
|
||||
"srv",
|
||||
"oauthClientSecret",
|
||||
"encrypted",
|
||||
);
|
||||
void seeded;
|
||||
|
||||
delete process.env.MCP_INSPECTOR_SECRET_KEY;
|
||||
const mod = await loadWithProbe(true);
|
||||
const keyring = new InMemorySecretStore();
|
||||
|
||||
await mod.absorbFileSecretsIntoKeyring(keyring);
|
||||
|
||||
expect(existsSync(filePath)).toBe(true);
|
||||
expect(await keyring.get("srv", "oauthClientSecret")).toBe(null);
|
||||
});
|
||||
|
||||
it("is a no-op when there is no file", async () => {
|
||||
process.env.MCP_INSPECTOR_SECRET_FILE = path.join(tmpDir, "absent.json");
|
||||
const mod = await loadWithProbe(true);
|
||||
const keyring = new InMemorySecretStore();
|
||||
await mod.absorbFileSecretsIntoKeyring(keyring);
|
||||
expect(existsSync(path.join(tmpDir, "absent.json"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the descriptor reports a mode it could not tighten", () => {
|
||||
it("carries looseMode into the descriptor and warns once at selection", async () => {
|
||||
// The warning alone was not enough: the caveat under the secret field
|
||||
|
|
|
|||
Loading…
Reference in a new issue