26d2f58784
No new inline comments this round, but two of the four suppressed ones were real defects rather than restatements — both in code I added, both silently wrong in the safe-looking direction. **A symlinked secrets directory read as unmounted.** `isOnMountPoint` walks to the nearest existing ancestor *lexically* and then string-prefixes it against mountinfo, so `~/.mcp-inspector -> /data/inspector` with `/data` mounted matched no mount point. The container then selected the session-only store on a box that had explicitly arranged for persistence, and nothing said so — the mount was there, the volume was there, the secrets evaporated on exit. Resolved through symlinks now, tolerantly: a path that cannot be resolved falls back to the lexical comparison, which is what this did before and is correct whenever no symlink is involved. The `statSync` fallback branch already followed links, so this also makes the two paths agree. **The bulk read did not fix the case it was written for.** `getMany` batched *one server's* fields, but both rehydration callers iterate servers — so a catalog of 20 servers holding one secret each still paid 20 serialized scrypt derivations. The same ~450ms stall the seam was introduced to remove, reached one server at a time instead of one field at a time. Measuring the wrong unit and then optimizing it is worse than not optimizing, because the number in the commit message says it is fixed. The seam is cross-server now — `getMany(requests: SecretBulkRequest[])` — and both callers pass the whole catalog in one call. `FileSecretStore` decrypts once for the entire rehydration; the keychain fallback still issues independent parallel `get`s, which is what it wants. A test asserts one derivation for four servers, next to the existing one asserting one for three fields, so the property is pinned at both axes. The other two suppressed comments are the cross-process guarantee — `mutate`'s residual and the hand-off's TOCTOU delete — which remain decided against for this PR and tracked in #2082. Full `npm run ci` green. Signed-off-by: cliffhall <cliff@futurescale.com>
35 lines
1.3 KiB
TypeScript
35 lines
1.3 KiB
TypeScript
import {
|
|
secretStoreGetMany,
|
|
type SecretStore,
|
|
} from "../../auth/node/secret-store.js";
|
|
import { expectedSecretFields, mergeSecretsIntoStored } from "../serverList.js";
|
|
import type { MCPConfig } from "../types.js";
|
|
|
|
/**
|
|
* Merge per-server secrets from the OS keychain into an on-disk MCP catalog
|
|
* shape. Mirrors the web `/api/servers` GET rehydration path so TUI/CLI see
|
|
* the same effective OAuth client secrets and stdio env values as the browser.
|
|
*/
|
|
export async function rehydrateMcpConfigFromKeychain(
|
|
config: MCPConfig,
|
|
secretStore: SecretStore,
|
|
): Promise<MCPConfig> {
|
|
const entries = Object.entries(config.mcpServers);
|
|
// One pass for the *whole catalog*, not one per server and not one per
|
|
// field. For the keychain this is the same parallel round-trips as before;
|
|
// for the file store it collapses the entire rehydration into a single
|
|
// read-and-decrypt (see `secretStoreGetMany`). Batching per server instead
|
|
// left a 20-server catalog paying 20 serialized scrypt derivations.
|
|
const secrets = await secretStoreGetMany(
|
|
secretStore,
|
|
entries.map(([serverId, stored]) => ({
|
|
serverId,
|
|
fields: expectedSecretFields(stored),
|
|
})),
|
|
);
|
|
const out: MCPConfig = { mcpServers: {} };
|
|
for (const [id, stored] of entries) {
|
|
out.mcpServers[id] = mergeSecretsIntoStored(stored, secrets[id] ?? {});
|
|
}
|
|
return out;
|
|
}
|