Fix #2245: bug: masked apiKey written to config.yaml is never resolved on daemon restart —

apps/memos-local-plugin/core/config/index.ts

@@ -18,7 +18,7 @@ import { MemosError } from "../../agent-contract/errors.js";
 import type { ResolvedHome } from "./paths.js";
 import { resolveHome } from "./paths.js";
 import { ConfigSchema, type ResolvedConfig } from "./schema.js";
-import { DEFAULT_CONFIG, effectiveViewerPort } from "./defaults.js";
+import { DEFAULT_CONFIG, SECRET_FIELD_PATHS, effectiveViewerPort } from "./defaults.js";
 import { migrateHermesViewerPort } from "./migrations.js";
 import { parseYaml } from "./yaml.js";
 
@@ -72,9 +72,47 @@ export async function loadConfig(home: ResolvedHome, agent?: string): Promise<Lo
 /**
  * Merge an arbitrary raw object over `DEFAULT_CONFIG` and validate. Used in
  * tests and by `writer.ts`. `warnings` is mutated in place if provided.
+ *
+ * The `raw` argument is never mutated — the secret resolution below runs on a
+ * freshly built copy, so callers may pass shared or cached objects safely.
  */
 export function resolveConfig(raw: unknown, warnings?: string[], agent?: string): ResolvedConfig {
   const cleaned = pruneUnknown(raw, DEFAULT_CONFIG, "", warnings);
+  // Resolve masked/placeholder secret values from the environment before
+  // merging. `maskSecrets()` (pipeline/memory-core.ts) rewrites every
+  // SECRET_FIELD_PATHS leaf to `__memos_secret__` before the config is
+  // persisted or surfaced, and `stripEmptySecrets()` drops empty leaves
+  // from patches. But nothing re-reads the real value back: when the
+  // daemon restarts and `loadConfig()` parses the YAML, the placeholder
+  // is treated as the literal API key, so every LLM call fails auth and
+  // the bridge loops on restart with `lastOkAt: null` and crystallize
+  // stuck. Same problem if a user writes `apiKey: ""` or an explicit
+  // `${ENV_VAR}` reference and expects expansion (the writer's
+  // `resolveConfig` is the single choke point for both disk and
+  // in-memory patch paths, so resolving here covers both).
+  //
+  // Resolution rules (first match wins):
+  //   1. Value is `${NAME}`  -> use process.env[NAME]. Only allowlisted
+  //      names are expanded (`^[A-Z][A-Z0-9_]*_(API_KEY|TOKEN)$`);
+  //      anything else emits a warning and is left untouched.
+  //   2. Value is the mask sentinel `__memos_secret__` or empty string
+  //      -> derive the env var from the field path itself
+  //      (llm.apiKey -> LLM_API_KEY, hub.teamToken -> HUB_TEAM_TOKEN,
+  //      skillEvolver.apiKey -> SKILL_EVOLVER_API_KEY, …). The generic
+  //      OPENCODE_GO/ZEN fallback is applied ONLY to the primary
+  //      `llm.apiKey`; per-component overrides (l3Llm, skillEvolver)
+  //      and non-LLM secrets (embedding, hub tokens) never borrow an
+  //      unrelated provider's key — that would cause cross-provider
+  //      auth failures or unexpected billing on the wrong account.
+  //   3. Otherwise leave the value untouched.
+  //
+  // Any secret leaf that references an env var that is not set emits a
+  // warning so the operator gets an actionable log message instead of
+  // silent auth failures on the next LLM call.
+  //
+  // The mask itself is never used as a credential, and the on-disk write
+  // stays masked (security preserved); this is read-side only.
+  resolveSecretEnv(cleaned, warnings);
   const merged = deepMerge(DEFAULT_CONFIG as Record<string, unknown>, cleaned);
   stripUnsupportedEmbeddingDimensions(merged);
   const viewerPort = effectiveViewerPort(agent);
@@ -104,6 +142,106 @@ export function resolveConfig(raw: unknown, warnings?: string[], agent?: string)
 
 // ─── helpers ────────────────────────────────────────────────────────────────
 
+/** Env var names accepted in `${NAME}` config references. */
+const ENV_REF_ALLOWLIST = /^[A-Z][A-Z0-9_]*_(API_KEY|TOKEN)$/;
+
+/**
+ * Replace masked / empty / `${VAR}` secret leaves in `cleaned` (a freshly
+ * built, non-shared object — see `pruneUnknown`) with values from the
+ * environment. The caller's raw config object is never written to.
+ */
+function resolveSecretEnv(cleaned: Record<string, unknown>, warnings?: string[]): void {
+  for (const dotted of SECRET_FIELD_PATHS) {
+    const keys = dotted.split(".");
+    let cursor: unknown = cleaned;
+    // Explicit flag: `break` alone leaves `cursor` pointing at the last
+    // valid value, which for paths deeper than 2 levels could accidentally
+    // pass the `isPlainObject(cursor)` check below and index `leaf` on the
+    // wrong node. Today every SECRET_FIELD_PATHS entry is only 2 levels
+    // deep, but keeping the flag makes the intent explicit and future-
+    // proofs against deeper paths being added.
+    let traversalOk = true;
+    for (let i = 0; i < keys.length - 1; i++) {
+      if (!isPlainObject(cursor)) {
+        traversalOk = false;
+        break;
+      }
+      cursor = (cursor as Record<string, unknown>)[keys[i]!];
+    }
+    if (!traversalOk || !isPlainObject(cursor)) continue;
+    const leaf = keys[keys.length - 1]!;
+    const val = (cursor as Record<string, unknown>)[leaf];
+    if (typeof val !== "string") continue;
+
+    let envName: string | null = null;
+    let genericFallbacks = false;
+    if (val.startsWith("${") && val.endsWith("}")) {
+      const name = val.slice(2, -1);
+      if (!ENV_REF_ALLOWLIST.test(name)) {
+        warnings?.push(
+          `config: leaving '${dotted}' as '${val}' — env name '${name}' is not allowlisted ` +
+            `(expected ^[A-Z][A-Z0-9_]*_(API_KEY|TOKEN)$)`
+        );
+        continue;
+      }
+      envName = name;
+    } else if (val === "__memos_secret__" || val === "") {
+      // Derive env var name from the field path itself so every entry
+      // in SECRET_FIELD_PATHS is resolvable, not just the ones whose
+      // leaf is `apiKey`:
+      //   embedding.apiKey    → EMBEDDING_API_KEY
+      //   llm.apiKey          → LLM_API_KEY
+      //   l3Llm.apiKey        → L3_LLM_API_KEY
+      //   skillEvolver.apiKey → SKILL_EVOLVER_API_KEY
+      //   hub.teamToken       → HUB_TEAM_TOKEN
+      //   hub.userToken       → HUB_USER_TOKEN
+      const parent = keys[keys.length - 2] ?? "";
+      envName = `${camelToUpperSnake(parent)}_${camelToUpperSnake(leaf)}`;
+      // OPENCODE_GO_API_KEY / OPENCODE_ZEN_API_KEY are only meaningful
+      // for the primary `llm.apiKey`. Per-component overrides
+      // (l3Llm.apiKey, skillEvolver.apiKey) and non-LLM secrets
+      // (embedding.apiKey, hub.*Token) must not silently borrow an
+      // unrelated provider's key — doing so causes cross-provider auth
+      // failures and unexpected billing on the wrong account when the
+      // component is configured for a different provider entirely.
+      genericFallbacks = parent === "llm" && leaf === "apiKey";
+    }
+    if (!envName) continue;
+
+    const envVal =
+      process.env[envName] ??
+      (genericFallbacks
+        ? (process.env.OPENCODE_GO_API_KEY ?? process.env.OPENCODE_ZEN_API_KEY)
+        : undefined);
+    if (envVal) {
+      (cursor as Record<string, unknown>)[leaf] = envVal;
+    } else {
+      // Explicit-reference case: the user asked for env expansion but
+      // the target is unset. Mask/empty case: we walked the path-based
+      // convention and nothing was set. Both perpetuate the original
+      // bug (silent auth failure on next LLM call) unless we log it.
+      warnings?.push(
+        `config: '${dotted}' references env var '${envName}' but it is not set — ` +
+          `field left as placeholder and auth will fail on the next call`
+      );
+    }
+  }
+}
+
+/**
+ * camelCase → UPPER_SNAKE_CASE for deriving env var names from config
+ * field paths. Only inserts an underscore at a lowercase/digit → uppercase
+ * boundary so acronyms and digit runs stay intact:
+ *   apiKey        → API_KEY
+ *   teamToken     → TEAM_TOKEN
+ *   userToken     → USER_TOKEN
+ *   l3Llm         → L3_LLM
+ *   skillEvolver  → SKILL_EVOLVER
+ */
+function camelToUpperSnake(s: string): string {
+  return s.replace(/([a-z0-9])([A-Z])/g, "$1_$2").toUpperCase();
+}
+
 function formatErr(e: ValueError): string {
   return `${e.path || "<root>"}: ${e.message}`;
 }

apps/memos-local-plugin/tests/unit/config/resolve-secret-env.test.ts

@@ -0,0 +1,168 @@
+import { afterEach, describe, expect, it } from "vitest";
+
+import { resolveConfig } from "../../../core/config/index.js";
+import { SECRET_FIELD_PATHS } from "../../../core/config/defaults.js";
+
+const ORIGINAL_ENV = { ...process.env };
+
+afterEach(() => {
+  process.env = { ...ORIGINAL_ENV };
+});
+
+describe("resolveConfig secret env fallback", () => {
+  it("expands allowlisted ${ENV_VAR} references in secret fields", () => {
+    process.env.MY_LLM_API_KEY = "sk-env-expanded";
+    const cfg = resolveConfig({ llm: { apiKey: "${MY_LLM_API_KEY}" } });
+    expect(cfg.llm.apiKey).toBe("sk-env-expanded");
+  });
+
+  it("resolves the __memos_secret__ mask sentinel from env", () => {
+    process.env.OPENCODE_GO_API_KEY = "sk-mask-resolved";
+    const cfg = resolveConfig({ llm: { apiKey: "__memos_secret__" } });
+    expect(cfg.llm.apiKey).toBe("sk-mask-resolved");
+  });
+
+  it("resolves empty string secret fields from env", () => {
+    process.env.OPENCODE_ZEN_API_KEY = "sk-empty-resolved";
+    const cfg = resolveConfig({ llm: { apiKey: "" } });
+    expect(cfg.llm.apiKey).toBe("sk-empty-resolved");
+  });
+
+  it("uses per-path env conventions — every secret path resolves from its own env var", () => {
+    // OPENCODE_GO_API_KEY is only the generic fallback for the *primary*
+    // llm.apiKey — l3Llm / skillEvolver / hub / embedding all get their
+    // own path-derived env var and never silently borrow the LLM key.
+    process.env.OPENCODE_GO_API_KEY = "sk-llm";
+    process.env.EMBEDDING_API_KEY = "sk-embed";
+    process.env.L3_LLM_API_KEY = "sk-l3";
+    process.env.SKILL_EVOLVER_API_KEY = "sk-skill";
+    process.env.HUB_TEAM_TOKEN = "sk-team";
+    process.env.HUB_USER_TOKEN = "sk-user";
+    const raw: Record<string, unknown> = {};
+    for (const dotted of SECRET_FIELD_PATHS) {
+      const keys = dotted.split(".");
+      let cursor = raw;
+      for (let i = 0; i < keys.length - 1; i++) {
+        cursor[keys[i]!] = cursor[keys[i]!] ?? {};
+        cursor = cursor[keys[i]!] as Record<string, unknown>;
+      }
+      cursor[keys[keys.length - 1]!] = "__memos_secret__";
+    }
+    const cfg = resolveConfig(raw);
+    const expected: Record<string, string> = {
+      "embedding.apiKey": "sk-embed",
+      "llm.apiKey": "sk-llm",
+      "l3Llm.apiKey": "sk-l3",
+      "skillEvolver.apiKey": "sk-skill",
+      "hub.teamToken": "sk-team",
+      "hub.userToken": "sk-user",
+    };
+    for (const dotted of SECRET_FIELD_PATHS) {
+      const keys = dotted.split(".");
+      let cursor: unknown = cfg;
+      for (const k of keys) {
+        cursor = (cursor as Record<string, unknown>)[k];
+      }
+      expect(cursor).toBe(expected[dotted]);
+    }
+  });
+
+  it("resolves masked hub tokens from HUB_TEAM_TOKEN / HUB_USER_TOKEN", () => {
+    // Regression: previously the mask/empty path only ran when `leaf ===
+    // 'apiKey'`, so hub.teamToken / hub.userToken masked by
+    // maskSecrets() were silently left unresolved and hub auth failed
+    // exactly like the LLM auth bug in #2245.
+    process.env.HUB_TEAM_TOKEN = "sk-team-mask";
+    process.env.HUB_USER_TOKEN = "sk-user-empty";
+    const cfg = resolveConfig({
+      hub: { teamToken: "__memos_secret__", userToken: "" },
+    });
+    expect(cfg.hub.teamToken).toBe("sk-team-mask");
+    expect(cfg.hub.userToken).toBe("sk-user-empty");
+  });
+
+  it("does not fall back to OPENCODE_GO/ZEN for l3Llm.apiKey", () => {
+    // Per-component overrides must not silently inherit the primary
+    // provider's key: l3-llm and skill-evolver are frequently pointed
+    // at a different provider than the shared llm settings.
+    process.env.OPENCODE_GO_API_KEY = "sk-llm";
+    process.env.OPENCODE_ZEN_API_KEY = "sk-zen";
+    delete process.env.L3_LLM_API_KEY;
+    const cfg = resolveConfig({ l3Llm: { apiKey: "__memos_secret__" } });
+    expect(cfg.l3Llm.apiKey).toBe("__memos_secret__");
+  });
+
+  it("does not fall back to OPENCODE_GO/ZEN for skillEvolver.apiKey", () => {
+    process.env.OPENCODE_GO_API_KEY = "sk-llm";
+    process.env.OPENCODE_ZEN_API_KEY = "sk-zen";
+    delete process.env.SKILL_EVOLVER_API_KEY;
+    const cfg = resolveConfig({ skillEvolver: { apiKey: "__memos_secret__" } });
+    expect(cfg.skillEvolver.apiKey).toBe("__memos_secret__");
+  });
+
+  it("warns when an explicit ${VAR} reference cannot be resolved", () => {
+    // Without a warning the user sees auth failures with no actionable
+    // hint; the whole point of the read-side resolver is to make config
+    // → env misconfiguration debuggable.
+    delete process.env.MISSING_LLM_API_KEY;
+    const warnings: string[] = [];
+    const cfg = resolveConfig({ llm: { apiKey: "${MISSING_LLM_API_KEY}" } }, warnings);
+    expect(cfg.llm.apiKey).toBe("${MISSING_LLM_API_KEY}");
+    expect(warnings.some((w) => w.includes("MISSING_LLM_API_KEY") && w.includes("not set"))).toBe(
+      true,
+    );
+  });
+
+  it("warns when a masked apiKey has no backing env var", () => {
+    delete process.env.LLM_API_KEY;
+    delete process.env.OPENCODE_GO_API_KEY;
+    delete process.env.OPENCODE_ZEN_API_KEY;
+    const warnings: string[] = [];
+    const cfg = resolveConfig({ llm: { apiKey: "__memos_secret__" } }, warnings);
+    expect(cfg.llm.apiKey).toBe("__memos_secret__");
+    expect(warnings.some((w) => w.includes("llm.apiKey") && w.includes("not set"))).toBe(true);
+  });
+
+  it("resolves hub tokens via explicit ${VAR} references", () => {
+    process.env.HUB_TEAM_TOKEN = "sk-hub-token";
+    const cfg = resolveConfig({ hub: { teamToken: "${HUB_TEAM_TOKEN}" } });
+    expect(cfg.hub.teamToken).toBe("sk-hub-token");
+  });
+
+  it("does not fall back to generic keys when an explicit ${VAR} is unset", () => {
+    process.env.OPENCODE_GO_API_KEY = "sk-llm";
+    process.env.OPENCODE_ZEN_API_KEY = "sk-zen";
+    const cfg = resolveConfig({ llm: { apiKey: "${MY_LLM_API_KEY}" } });
+    expect(cfg.llm.apiKey).toBe("${MY_LLM_API_KEY}");
+  });
+
+  it("warns and skips expansion for non-allowlisted ${VAR} names", () => {
+    process.env.HOME = "/home/test";
+    const warnings: string[] = [];
+    const cfg = resolveConfig({ llm: { apiKey: "${HOME}" } }, warnings);
+    expect(cfg.llm.apiKey).toBe("${HOME}");
+    expect(warnings.length).toBe(1);
+    expect(warnings[0]).toContain("not allowlisted");
+  });
+
+  it("leaves real (non-placeholder) values untouched", () => {
+    const cfg = resolveConfig({ llm: { apiKey: "sk-real-value" } });
+    expect(cfg.llm.apiKey).toBe("sk-real-value");
+  });
+
+  it("leaves placeholders untouched when no env var is set", () => {
+    delete process.env.LLM_API_KEY;
+    delete process.env.OPENCODE_GO_API_KEY;
+    delete process.env.OPENCODE_ZEN_API_KEY;
+    const cfg = resolveConfig({ llm: { apiKey: "__memos_secret__" } });
+    expect(cfg.llm.apiKey).toBe("__memos_secret__");
+  });
+
+  it("never mutates the caller's raw config object", () => {
+    process.env.OPENCODE_GO_API_KEY = "sk-llm";
+    const raw = { llm: { apiKey: "__memos_secret__" } };
+    const cfg = resolveConfig(raw);
+    expect(cfg.llm.apiKey).toBe("sk-llm");
+    expect(raw.llm.apiKey).toBe("__memos_secret__");
+  });
+});