feat: support environment-based BYOK credentials

docs/byok-environment-credentials.md

@@ -0,0 +1,81 @@
+# BYOK environment-variable credentials
+
+Store a reference in the active profile's `config.yaml` to keep an API key out of
+that file. The TUI, `mcode exec`, and ACP resolve the reference from their own
+process environment when they use the credential.
+
+## Configure an existing provider
+
+Set `WORK_API_KEY` locally in the shell that launches `mcode`. For a POSIX shell,
+read the key without displaying it or putting its value in shell history:
+
+```bash
+read -s WORK_API_KEY
+export WORK_API_KEY
+```
+
+In PowerShell:
+
+```powershell
+$secureKey = Read-Host 'API Key' -AsSecureString
+$env:WORK_API_KEY = [System.Net.NetworkCredential]::new('', $secureKey).Password
+```
+
+Edit the existing provider entry in `config.yaml`, keeping its endpoint, API
+format, and model definitions:
+
+```yaml
+custom_provider:
+  work:
+    options:
+      apiKey: '${WORK_API_KEY}'
+```
+
+The official MiniMax API key supports the same syntax:
+
+```yaml
+minimax_api:
+  apiKey: '${MINIMAX_API_KEY}'
+```
+
+Launch `mcode` from the configured shell. Restart a running TUI, exec process, or
+ACP host after changing its launch environment; changes in another terminal do
+not update an existing process. A launcher or editor that starts ACP must pass
+the variable to the child process too.
+
+`provider add --api-key-env NAME` continues to read and save the variable's value.
+To retain a reference, edit the saved credential field as shown above.
+
+## Supported syntax and errors
+
+Only a complete `${NAME}` string expands. Names begin with a letter or underscore,
+followed by letters, digits, or underscores. `$NAME`, `Bearer ${NAME}`, and
+`prefix-${NAME}` remain literal values. A mapping such as `{env: NAME}` is rejected
+as the wrong credential type. This feature does not expand Base URLs or custom
+request headers.
+
+Missing or whitespace-only variables fail before the model request, with an error
+that identifies the provider, credential field, and variable name. Check whether
+the variable exists without printing its value. Saved-provider discovery and
+connection tests use the same credential resolution; cached connection status
+tracks the resolved key, so changing the key invalidates a previous result.
+
+## Configuration and logs
+
+Saving another setting preserves credential reference text and existing YAML
+comments. When a configuration value changes, YAML aliases and merge fields are
+expanded to independent values: editing one provider cannot change another
+provider through a shared anchor, and clearing an inherited key stays cleared
+when the file is read again. A no-op write keeps the original text. Changed
+files may have normalized whitespace or indentation.
+
+Numeric provider and model keys retain their identity during updates. YAML version
+directives do not change the application loader's scalar interpretation during
+alias expansion. Multiline plain strings retain their folded line breaks when
+unrelated settings are saved. Each changed document is checked with that loader
+before the configuration file is replaced.
+
+Malformed configuration is rejected without replacing the existing file. POSIX
+configuration permissions remain private. Runtime log fields and common
+credential text are redacted, while token usage counters remain readable. This
+protection does not remove credentials from older logs or files.

docs/examples.md

@@ -68,6 +68,11 @@ pnpm mcode provider list --json
 
 `--context-limit` and `--output-limit` each accept a positive safe integer (at most `9007199254740991`). Either flag can be used independently. The same limits apply to every repeated `--model`; only the first model is tested and selected by `--use`. The JSON list shows the configured values as `contextLimit` and `maxOutputTokens`. Without these flags, the existing defaults remain unchanged (unknown custom models currently fall back to 200,000 context tokens and 16,384 output tokens). Model discovery does not infer your local server's context size.
 
+To keep the key out of `config.yaml`, replace the saved `apiKey` with a quoted
+`${MCODE_PROVIDER_API_KEY}` reference and launch `mcode` from the shell where the
+variable is exported. See [environment-variable credentials](byok-environment-credentials.md)
+for both supported credential fields, restart behavior, and configuration writes.
+
 `--api-key-env` reads the current environment variable value and stores that value in the active profile's `config.yaml`; it does not save an environment-variable reference. The file still contains plaintext credentials. On POSIX systems, config writes and temporary copies use `0600`. When loading existing files, MCode removes group/other access while preserving the owner's permissions; already-private files such as `0400` or `0600` do not require a permission change. Loading fails if an unsafe main config cannot be restricted. Older migration backups are also checked, but inspection or repair failures produce a warning identifying the directory or backup that needs manual attention rather than preventing the main config from loading. Windows file modes do not provide equivalent ACL protection; restrict access to the profile directory using Windows permissions.
 
 ### Third-party relays and custom auth headers

packages/config/package.json

@@ -28,7 +28,8 @@
   "dependencies": {
     "@mavis/shared": "workspace:^",
     "js-yaml": "^4",
-    "proper-lockfile": "^4"
+    "proper-lockfile": "^4",
+    "yaml": "^2.9.0"
   },
   "devDependencies": {
     "@types/js-yaml": "^4",

packages/config/src/comment-preserving-config-write.ts

@@ -0,0 +1,237 @@
+import { isDeepStrictEqual } from 'node:util';
+
+import yaml from 'js-yaml';
+import {
+  isMap,
+  isNode,
+  isScalar,
+  parseDocument,
+  visit,
+  type Alias,
+  type Document,
+  type Node,
+  type YAMLMap,
+} from 'yaml';
+
+/**
+ * Runtime rewrites of config.yaml must not throw away what the user wrote.
+ *
+ * The previous path parsed the file, mutated a plain object, and dumped that
+ * object back, which dropped every comment and re-indented the whole file. Start
+ * from the original document — which keeps comments and original formatting on
+ * every node we do not touch — and write only the paths that actually changed.
+ *
+ * The diff is bounded: nested plain objects are walked key by key so comments
+ * survive inside them, while anything else — a changed node kind, or a list — is
+ * replaced as one value. That keeps the rewrite narrow instead of inventing a
+ * merge rule for shapes this file does not use.
+ */
+
+type ConfigEdit =
+  | { readonly kind: 'set'; readonly path: string[]; readonly value: unknown }
+  | { readonly kind: 'delete'; readonly path: string[] };
+
+/**
+ * Parses config text into the plain-object "before" state used for the diff.
+ * Callers keep this separate from the object they mutate, because several write
+ * paths hand that mutable object to a callback that aliases into it.
+ */
+export function parseConfigText(text: string): Record<string, unknown> {
+  try {
+    const parsed = yaml.load(text);
+    return isPlainRecord(parsed) ? parsed : {};
+  } catch {
+    return {};
+  }
+}
+
+/**
+ * Serializes `next` over `previous` while keeping comments and formatting from
+ * `originalText`. Falls back to a full dump when the original cannot be parsed
+ * into a document, so a hand-broken file still round-trips instead of vanishing.
+ */
+export function serializeConfigPreservingComments(
+  originalText: string,
+  previous: Record<string, unknown>,
+  next: Record<string, unknown>,
+): string {
+  const document = parseDocument(originalText, {
+    schema: 'core',
+    compat: 'yaml-1.1',
+    customTags: ['timestamp'],
+    merge: true,
+  });
+  if (document.errors.length > 0 || (!isMap(document.contents) && originalText.trim() !== '')) {
+    return dumpConfig(next);
+  }
+
+  try {
+    const edits = collectConfigEdits(previous, next);
+    if (edits.length === 0) return originalText;
+    useConfigLoaderScalarValues(document);
+    materializeConfigReferences(document);
+    for (const edit of edits) {
+      const path = existingConfigPath(document, edit.path);
+      if (edit.kind === 'delete') {
+        document.deleteIn(path);
+      } else {
+        document.setIn(path, edit.value);
+      }
+    }
+    const serialized = document.toString({ indent: 2, lineWidth: -1 });
+    // The application loader is the final authority. Reject an invalid or
+    // semantically different result before the atomic writer replaces the file.
+    if (!isDeepStrictEqual(yaml.load(serialized), yaml.load(dumpConfig(next)))) {
+      throw new Error('The serialized configuration does not match the intended values');
+    }
+    return serialized;
+  } catch (error) {
+    // A self-referential anchor (`&a { self: *a }`) makes the emitter recurse
+    // without bound. That input cannot be written back in any form, so report
+    // it as a config problem instead of letting a stack overflow escape.
+    throw new Error(
+      `config.yaml could not be rewritten: ${
+        error instanceof Error ? error.message : String(error)
+      }${error instanceof RangeError ? '. Remove the self-referential YAML anchor and retry.' : ''}`,
+    );
+  }
+}
+
+/** Keep alias expansion independent of the AST parser's implicit scalar rules. */
+function useConfigLoaderScalarValues(document: Document): void {
+  visit(document, {
+    Scalar(_key, node) {
+      if (node.addToJSMap || typeof node.source !== 'string') return;
+      if (node.type !== 'PLAIN' && !node.tag) return;
+      // A mapping wrapper keeps values such as "---" from becoming directives.
+      // node.source already contains YAML's folded multiline value. Quote it
+      // before reparsing so its line breaks keep their meaning and indentation.
+      // Explicit tags still apply to quoted values.
+      const source =
+        node.type === 'PLAIN' && !node.source.includes('\n')
+          ? node.source
+          : JSON.stringify(node.source);
+      const tag = node.tag ? `!<${node.tag}> ` : '';
+      const value = (yaml.load(`value: ${tag}${source}`) as { value: unknown }).value;
+      if (!isDeepStrictEqual(node.value, value)) {
+        node.value = value;
+        delete node.format;
+      }
+    },
+  });
+}
+
+/** JavaScript config keys are strings, while YAML keeps numeric/boolean keys typed. */
+function existingConfigKey(map: YAMLMap, key: string): unknown {
+  return map.items.find((pair) => isScalar(pair.key) && String(pair.key.value) === key)?.key ?? key;
+}
+
+function existingConfigPath(document: Document, path: readonly string[]): unknown[] {
+  const resolved: unknown[] = [];
+  let parent: unknown = document.contents;
+  for (const key of path) {
+    resolved.push(isMap(parent) ? existingConfigKey(parent, key) : key);
+    parent = document.getIn(resolved, true);
+  }
+  return resolved;
+}
+
+/**
+ * Provider updates can replace one options object while leaving its former
+ * aliases unchanged. Snapshot references before editing their sources, so YAML
+ * sharing cannot reintroduce coupling the application has already removed.
+ * Materialize merge fields too: deleting an inherited key must not expose the
+ * value again through `<<`. Explicit nodes retain their comments and styles.
+ */
+function materializeConfigReferences(document: Document): void {
+  const aliases = new Map<Alias, Node>();
+  visit(document, {
+    Alias(_key, alias) {
+      const value: unknown = alias.toJS(document);
+      const node = createConfigNode(document, value);
+      node.comment = alias.comment;
+      node.commentBefore = alias.commentBefore;
+      node.spaceBefore = alias.spaceBefore;
+      aliases.set(alias, node);
+    },
+  });
+  visit(document, { Alias: (_key, alias) => aliases.get(alias) });
+  visit(document, {
+    Map(_key, map) {
+      const merges = map.items.filter((pair) => isScalar(pair.key) && pair.key.addToJSMap);
+      if (merges.length === 0) return;
+      const values = map.toJS(document) as Record<string, unknown>;
+      const comments = merges.flatMap((pair) =>
+        [pair.key, pair.value].flatMap((node) =>
+          isNode(node) ? [node.commentBefore, node.comment] : [],
+        ),
+      );
+      map.commentBefore = [map.commentBefore, ...comments].filter(Boolean).join('\n') || undefined;
+      map.items = map.items.filter((pair) => !merges.includes(pair));
+      for (const [key, value] of Object.entries(values)) {
+        if (!map.has(existingConfigKey(map, key))) {
+          map.set(key, createConfigNode(document, value));
+        }
+      }
+    },
+  });
+  // All merge pairs are gone. Disable merge emission, and quote strings that
+  // either scalar schema could interpret (including the loader's 0b integers).
+  document.setSchema(document.directives?.yaml.version ?? '1.2', {
+    schema: 'core',
+    compat: 'yaml-1.1',
+    customTags: ['timestamp'],
+    merge: false,
+  });
+}
+
+function createConfigNode(document: Document, value: unknown): Node {
+  const node = document.createNode(value, { aliasDuplicateObjects: false });
+  // A resolved object's literal "<<" property has already lost its YAML quote
+  // metadata. Quote it again so materialization cannot turn it into a merge.
+  visit(node, {
+    Pair(_key, pair) {
+      if (isScalar(pair.key) && pair.key.value === '<<') pair.key.type = 'QUOTE_DOUBLE';
+    },
+  });
+  return node;
+}
+
+function collectConfigEdits(previous: unknown, next: unknown, path: string[] = []): ConfigEdit[] {
+  if (!isPlainRecord(previous) || !isPlainRecord(next)) {
+    return valuesEqual(previous, next) ? [] : [{ kind: 'set', path, value: next }];
+  }
+  const edits: ConfigEdit[] = [];
+  for (const key of new Set([...Object.keys(previous), ...Object.keys(next)])) {
+    const childPath = [...path, key];
+    // A caller that assigns undefined is removing the value; writing it back
+    // would emit `key: null`, which reads as a configured-but-empty field.
+    if (!(key in next) || next[key] === undefined) {
+      edits.push({ kind: 'delete', path: childPath });
+    } else if (!(key in previous)) {
+      edits.push({ kind: 'set', path: childPath, value: next[key] });
+    } else {
+      edits.push(...collectConfigEdits(previous[key], next[key], childPath));
+    }
+  }
+  return edits;
+}
+
+function valuesEqual(left: unknown, right: unknown): boolean {
+  if (left === right) return true;
+  if (left === null || right === null) return false;
+  if (typeof left !== 'object' || typeof right !== 'object') return false;
+  try {
+    return JSON.stringify(left) === JSON.stringify(right);
+  } catch {
+    return false;
+  }
+}
+
+function dumpConfig(next: Record<string, unknown>): string {
+  return yaml.dump(next, { indent: 2, lineWidth: -1, noRefs: true });
+}
+
+function isPlainRecord(value: unknown): value is Record<string, unknown> {
+  return Boolean(value) && typeof value === 'object' && !Array.isArray(value);
+}

packages/config/src/config.ts

@@ -1,3 +1,7 @@
+import {
+  parseConfigText,
+  serializeConfigPreservingComments,
+} from './comment-preserving-config-write.js';
 import {
   resolveRunawayGuardConfig,
   type RunawayGuardSettings,
@@ -1654,7 +1658,11 @@ function syncManagedPresetBaseUrl(configPath: string): void {
   try {
     writePrivateConfigFileSync(
       configPath,
-      yaml.dump(raw, { indent: 2, lineWidth: -1, noRefs: true }),
+      serializeConfigPreservingComments(
+        originalContent.toString('utf-8'),
+        parseConfigText(originalContent.toString('utf-8')),
+        raw,
+      ),
     );
   } catch (error) {
     // This on-disk sync is optional, but a failure after truncation is not safe