Skip to content

fix(config): resolve masked apiKey from env on load - #2235

Open
kiwipaulrob wants to merge 1 commit into
MemTensor:mainfrom
kiwipaulrob:fix/config-apikey-env-fallback
Open

fix(config): resolve masked apiKey from env on load#2235
kiwipaulrob wants to merge 1 commit into
MemTensor:mainfrom
kiwipaulrob:fix/config-apikey-env-fallback

Conversation

@kiwipaulrob

Copy link
Copy Markdown

Summary

The bridge persists config.yaml with API keys masked to __memos_secret__ (maskSecrets() in core/pipeline/memory-core.ts) and strips empty secrets from patches (stripEmptySecrets()). But nothing ever re-reads the real value back: when the daemon restarts, loadConfig() parses the mask as the literal API key, every LLM call fails auth, and the bridge restart-loops with lastOkAt: null while skill crystallize stays stuck. This was observed live on 2026-08-11 with 290 candidate skills backlogged and skill.crystallize.failed ... openai_compatible timed out after 120000 ms spam in the journal.

This PR makes resolveConfig() (the single choke point for both disk-loaded and in-memory patched configs) resolve masked/placeholder secret values from the environment, read-side only — the on-disk write stays masked, so the security posture of maskSecrets() is preserved.

Change

apps/memos-local-plugin/core/config/index.tsresolveConfig(raw) now walks SECRET_FIELD_PATHS before pruneUnknown/deepMerge:

  • ${ENV_VAR} references in any secret field resolve from process.env[ENV_VAR]
  • __memos_secret__ / empty-string apiKey fields fall back to LLM_API_KEY, EMBEDDING_API_KEY, then OPENCODE_GO_API_KEY / OPENCODE_ZEN_API_KEY
  • Hub tokens (hub.teamToken, hub.userToken) have no env convention and are left untouched unless the user writes an explicit ${VAR}
  • Real (non-placeholder) values pass through unchanged

Tests

New tests/unit/config/resolve-secret-env.test.ts (6 tests):

  1. ${ENV_VAR} expansion
  2. __memos_secret__ mask resolution
  3. empty-string fallback
  4. all apiKey SECRET_FIELD_PATHS leaves resolve (hub tokens stay masked)
  5. real values untouched
  6. placeholder stays when no env var is set
Test Files  4 passed (4)
     Tests  54 passed (54)   # incl. full existing config suite (load/paths/writer)

tsc -p tsconfig.json --noEmit passes clean.

Related

  • MemOS feat(l3): dedicated l3Llm config slot for abstraction pass #1959 feat(l3): dedicated l3Llm config slot for abstraction pass — added the l3Llm.apiKey secret path this PR also covers; without the env fallback, L3 abstraction fails with l3.abstraction: 'inference' must be an array when the key is masked.
  • MemOS fix(llm): preserve all vLLM stream chunks #2234 fix(llm): preserve all vLLM stream chunks — same class of LLM-call robustness: this PR removes the auth-failure half of the timed out after 120000 ms burst seen on capture.reflect.scoring (16-step) runs that overload single-slot local models.
  • Hermes #56569 / #56581 / #58055 — the compound-channel-ID email bug follows the same "masked write without read path → silent failure" pattern; listed as the cross-project archetype.
  • Hermes #44370 MemOS skillInjectionMode full but memos_skill_get never called — downstream symptom class where MemOS config/LLM wiring fails silently; this PR removes one root cause.

Environment

  • Bridge: @memtensor/memos-local-plugin 2.0.12-beta.1 (source matches apps/memos-local-plugin)
  • Trigger: migration to the opencode-go provider (https://opencode.ai/zen/go/v1, deepseek-v4-flash). Config written via the UI/viewer masks the key, restart loops, curl with the env key works (GO_DEEPSEEK_OK), bridge with masked key fails.
  • Platform: Debian 12 LXC (Hermes CT100), node 22, systemd memos-bridge.service

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Unit Test (vitest run tests/unit/config — 54 passed)
  • Test Script Or Test Steps (live bridge: after patching dist the same way, lastOkAt populates and crystallize drains; verified 2026-08-11 on Hermes CT100)

Checklist

  • My code follows the style guidelines of this project (no new deps, existing patterns)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (PR body + code comments; doc note pending maintainer preference)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally

@Memtensor-AI Memtensor-AI added area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 11, 2026
@Memtensor-AI

Memtensor-AI commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2235
Task: 9394e6bfaccb1327
Base: main
Head: fix/config-apikey-env-fallback

🔍 OpenCodeReview found 2 issue(s) in this PR.


1. apps/memos-local-plugin/core/config/index.ts (L173-L178)

Treating val === "" the same as the __memos_secret__ sentinel is unsafe. pruneUnknown passes through any key that the user explicitly wrote in their YAML, so apiKey: "" in the config file means the user has intentionally cleared the key (e.g., to disable the provider or switch to a no-auth endpoint). Silently replacing it with whatever LLM_API_KEY or the fallback chain resolves to can cause API calls to be authenticated against a completely unintended account/service, with no indication to the user.

Only __memos_secret__ is a known system sentinel. Empty-string should be left as-is (or separately documented as an expansion trigger, which would need a distinct opt-in syntax).

Suggestion: remove || val === "" from the condition, or introduce a separate, distinct placeholder (e.g. __memos_env__) for the "infer from env" intent, so it cannot be confused with an intentional blank.

💡 Suggested Change

Before:

    } else if (val === "__memos_secret__" || val === "") {
      // Masked/empty API key — infer the env var from the field path.
      // Only apiKey fields have a convention (OPENAI_API_KEY, etc.);
      // hub tokens (teamToken/userToken) have no env convention, so
      // they must be set explicitly via ${VAR} or the UI.
      if (leaf !== "apiKey") continue;

After:

    } else if (val === "__memos_secret__") {
      // Masked API key sentinel — infer the env var from the field path.
      // Only apiKey fields have a convention (OPENAI_API_KEY, etc.);
      // hub tokens (teamToken/userToken) have no env convention, so
      // they must be set explicitly via ${VAR} or the UI.
      if (leaf !== "apiKey") continue;

2. apps/memos-local-plugin/core/config/index.ts (L187-L191)

genericFallbacks is true for every non-embedding apiKey field — including llm.apiKey, l3Llm.apiKey, and skillEvolver.apiKey — regardless of which LLM provider is configured. This means that when LLM_API_KEY is not set, OPENCODE_GO_API_KEY or OPENCODE_ZEN_API_KEY will be silently injected into the credentials of whichever provider is active, even if it has nothing to do with opencode-go or opencode-zen. This can leak a scoped key to a third-party API (e.g. Anthropic, OpenAI) without the user's knowledge.

Suggestion: Apply these named fallbacks only when the configured provider field matches the specific service they belong to, or remove the cross-provider fallback chain entirely and rely on explicit ${VAR} references for non-generic keys.

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Executor error: Command failed: git merge --no-edit base/main
Branch: fix/config-apikey-env-fallback

Problem:
The bridge persists config.yaml with apiKey masked to __memos_secret__
(maskSecrets in memory-core.ts) and strips empty secrets from patches,
but nothing re-reads the real value back. On restart, loadConfig parses
the mask as the literal API key, so every LLM call fails auth and the
bridge restart-loops with lastOkAt: null while crystallize stays stuck
(observed with 290 candidate skills backlogged on 2026-08-11).

Solution:
resolveConfig now walks SECRET_FIELD_PATHS before merging and expands
placeholder values from the environment, read-side only:
- ${ENV_VAR} references are resolved from process.env
- __memos_secret__ / empty apiKey fields fall back to LLM_API_KEY,
  EMBEDDING_API_KEY, then OPENCODE_GO_API_KEY / OPENCODE_ZEN_API_KEY
- hub tokens (teamToken/userToken) have no env convention and are
  left untouched unless an explicit ${VAR} reference is used
- real values pass through unchanged; on-disk masking is preserved

Tests: 6 new unit tests in tests/unit/config/resolve-secret-env.test.ts
(env expansion, mask resolution, empty-string, all apiKey paths,
real-value passthrough, unset-env fallback). Full config suite 54/54.
@kiwipaulrob
kiwipaulrob force-pushed the fix/config-apikey-env-fallback branch from cfeb5b7 to 3f33609 Compare August 11, 2026 09:02
@kiwipaulrob

Copy link
Copy Markdown
Author

Thanks for the review — all four findings are fair, and I've folded them into the revised branch (now rebased onto the current main, 8d310a7).

1. Provider fallbacks leaking across secret fields (L126–L130) — agreed, good catch. The OPENCODE_GO/ZEN fallbacks were meant to spare opencode-go/zen users from defining a second env var, but applying them to embedding.apiKey, and to explicit ${VAR} references whose named variable is unset, is wrong: it can hand an LLM key to an embedding provider's calls, or silently substitute a different key than the one the user named. Fix: an explicit ${VAR} now resolves exactly that variable or stays untouched, and the OPENCODE_* fallbacks apply only on the inferred sentinel/empty path, only for LLM-class fields (parent != embedding). Tests cover both cases: embedding.apiKey resolves from EMBEDDING_API_KEY and never from an LLM key, and an unset ${VAR} stays literal even when generic keys are present.

2. Unrestricted ${VAR} expansion (L110–L112) — agreed that expansion should be bounded, with one adjustment to the proposed allowlist: restricting to *_API_KEY names would break a designed use case of this PR, namely hub tokens (hub.teamToken / hub.userToken) set via explicit ${VAR} — they have no other env convention. The allowlist is now ^[A-Z][A-Z0-9_]*_(API_KEY|TOKEN)$; names outside it warn and are left untouched. I'd rate the practical severity here as modest for a local single-user plugin (user-owned config file, auth-gated viewer patch path), but the same change has a robustness payoff beyond the threat model: it stops accidental expansion of unrelated variables (a typo'd ${HOME} or ${PATH}) from becoming a bogus credential — so I'm happy to take it.

3. In-place mutation of raw (L130) — fair. Both current call sites are safe (loadConfig parses fresh YAML; writer.ts passes doc.toJS()), but the signature didn't promise non-mutation. Rather than just documenting the contract I made the pass non-mutating: pruneUnknown already returns a fresh copy, so resolution now runs on that cleaned object — identical semantics (user-provided leaves only), no writes to the caller's object. The JSDoc states the input is never mutated, and a test asserts the raw object is untouched after resolution.

4. Shadowed leaf declaration (L113–L118) — agreed, a leftover from an earlier edit. Removed the inner declaration; the outer one is in scope.

On the automated test run ("ENV ISSUE": git merge --no-edit base/main failed) — that was a genuine merge conflict rather than an environment problem: main had since changed this same file (the viewer-port migration, #2230, gives loadConfig/resolveConfig an agent parameter and effectiveViewerPort handling), and GitHub reported the branch as not mergeable (mergeable_state: dirty). I've merged the latest main into the branch, keeping both sets of changes (agent/viewer-port handling + env resolution), and pushed the rebased branch. The executor should pick the update up on the next run; if a manual re-run is needed, that would be appreciated.

Validation on the revised branch: config suite 71/71 (5 files — includes main's new hermes-migration tests) and tsc --noEmit clean.

Thanks again — the embedding-key leak in particular was a sharp catch.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (10/10 executed). memos_local_plugin/unit: 10/10. Duration: 3s

Branch: fix/config-apikey-env-fallback

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 11, 2026
@kiwipaulrob

Copy link
Copy Markdown
Author

Deployment note (from the production deployment that hit this)

The fix resolves masked keys from the daemon's process environment — which only works if the daemon actually has them. For systemd users:

# /etc/systemd/system/memos-bridge.service.d/env.conf
[Service]
EnvironmentFile=/path/to/durable/bridge.env

where bridge.env is a filtered copy of your secrets file, e.g. grep -v '^CUSTOM:' .env > <plugin-home>/daemon/bridge.env && chmod 600 <plugin-home>/daemon/bridge.env.

⚠️ Do NOT place the EnvironmentFile in /tmp. It is wiped on reboot; the unit then crash-loops with Failed to load environment files: No such file or directory (result 'resources', NRestarts +1 per 5s), and because the daemon is down a session host's ensure_viewer_daemon() spawns a rogue bridge.mjs --daemon that binds the viewer port — so /api/v1/health keeps answering while the systemd service is dead (observed live 2026-08-12 after a container reboot; the daemon must be restarted so systemd binds :18800 first).

Env var contract implemented by this PR:

  • generic: LLM_API_KEY (llm/skillEvolver/l3Llm apiKey), EMBEDDING_API_KEY (embedding.apiKey only — never an LLM key)
  • OpenCode fallbacks (LLM-class only): OPENCODE_GO_API_KEYOPENCODE_ZEN_API_KEY
  • explicit ${VAR}: allowlisted names matching ^[A-Z][A-Z0-9_]*_(API_KEY|TOKEN)$ (hub tokens use this — they have no other convention)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:plugin OpenClaw & Hermes status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants