-
Notifications
You must be signed in to change notification settings - Fork 5
feat(settings): export DB settings to the CLI's .env #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
pnewsam
wants to merge
12
commits into
staging
Choose a base branch
from
paul/eng-1127-settings-desync-collapse-the-onboardinglogin-write-path
base: staging
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
4074004
feat(settings): export DB settings to the CLI's .env (ENG-1127, Phase A)
pnewsam 04666cf
docs(settings): trim verbose comments on the ENG-1127 Phase A change
pnewsam 3537f13
refactor(settings): consolidate the .env<->DB boundary into env_bound…
pnewsam 435b0b7
fix(settings): export .env after minds-url backfill + serialize the e…
pnewsam 653c179
fix(settings): harden the .env export against Windows share-mode lock…
pnewsam a1a9c6b
fix(settings): reject dotenv injection via CR/LF in exported values (…
pnewsam de28960
fix(settings): export a runnable CLI config — resolved provider+model…
pnewsam b425358
fix(settings): don't export mixed-provider configs the CLI can't repr…
pnewsam 76eac50
test(settings): assert gemini base URL by equality, not prefix (CodeQL)
pnewsam dbe81f7
fix(settings): export the OpenAI slot for router-only Minds; don't wi…
pnewsam d39e661
fix(settings): stop stale .env round-tripping into the DB; drop basel…
pnewsam 89616e6
fix(settings): harden POST /raw's .env write via atomic_write_env (EN…
pnewsam File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,359 @@ | ||
| """The ``.env`` <-> DB settings boundary, both directions in one place. | ||
|
|
||
| The DB (``UserSettings``) is the source of truth; the standalone ``anton`` CLI | ||
| still reads its config from ``.env``, so ``.env`` is a trailing dependency the | ||
| server keeps in sync. Everything that knows an ``ANTON_*`` variable name lives | ||
| here — the alias map, provider-value normalization, and the two conversions — | ||
| so ``user_settings`` stays purely about the DB model. | ||
|
|
||
| - inbound (``.env`` -> DB): ``env_to_db_updates`` maps + normalizes; the caller | ||
| (SettingService / migration) validates, encrypts, and writes. | ||
| - outbound (DB -> ``.env``): ``db_to_env`` formats the stored values, then | ||
| ``merge_env_lines`` + ``atomic_write_env`` persist them, preserving unmanaged lines. | ||
|
|
||
| Model keys (planning_model / coding_model) are deliberately absent from the alias | ||
| map (ENG-739): a model is CLI-only and must never ride a bulk ``.env`` sync. | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import errno | ||
| import logging | ||
| import os | ||
| import tempfile | ||
| import time | ||
| from enum import Enum | ||
| from pathlib import Path | ||
|
|
||
| from pydantic import SecretStr | ||
|
|
||
| from cowork.common.settings.user_settings import ( | ||
| Provider, | ||
| UserSettings, | ||
| provider_api_key_str, | ||
| ) | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| # DB setting key -> its ANTON_* .env variable, for every field that overlaps | ||
| # between AntonSettings (.env) and UserSettings (DB). Single canonical map (was | ||
| # hand-maintained in two places that drifted — ENG-1125). | ||
| SETTING_ENV_ALIASES: dict[str, str] = { | ||
| "anthropic_api_key": "ANTON_ANTHROPIC_API_KEY", | ||
| "openai_api_key": "ANTON_OPENAI_API_KEY", | ||
| "openai_compatible_api_key": "ANTON_OPENAI_API_KEY_CUSTOM", | ||
| "gemini_api_key": "ANTON_GEMINI_API_KEY", | ||
| "minds_api_key": "ANTON_MINDS_API_KEY", | ||
| "planning_provider": "ANTON_PLANNING_PROVIDER", | ||
|
pnewsam marked this conversation as resolved.
|
||
| "coding_provider": "ANTON_CODING_PROVIDER", | ||
| "router_provider": "ANTON_ROUTER_PROVIDER", | ||
| "minds_url": "ANTON_MINDS_URL", | ||
| "openai_base_url": "ANTON_OPENAI_BASE_URL", | ||
| "memory_enabled": "ANTON_MEMORY_ENABLED", | ||
| "memory_mode": "ANTON_MEMORY_MODE", | ||
| "episodic_memory": "ANTON_EPISODIC_MEMORY", | ||
| "proactive_dashboards": "ANTON_PROACTIVE_DASHBOARDS", | ||
| "act_first": "ANTON_ACT_FIRST", | ||
| "publish_url": "ANTON_PUBLISH_URL", | ||
| } | ||
|
|
||
| # Inverse view (ANTON_* -> DB key) for the inbound (.env-first) callers. | ||
| ENV_ALIAS_TO_SETTING: dict[str, str] = {v: k for k, v in SETTING_ENV_ALIASES.items()} | ||
|
|
||
| # The per-role model vars the outbound export owns. Absent from SETTING_ENV_ALIASES | ||
| # because the INBOUND (.env->DB) direction must never map a model (ENG-739: a | ||
| # stale ``latest:`` line must not re-pin the picker). OUTBOUND they ARE managed — | ||
| # a provider is exported WITH its resolved model so the CLI can never end up with | ||
| # a provider/model mismatch (ENG-1127 review); being managed also means a stale | ||
| # model line is dropped when its provider is cleared. | ||
| _MODEL_ENV_VARS: tuple[str, ...] = ( | ||
| "ANTON_PLANNING_MODEL", | ||
| "ANTON_CODING_MODEL", | ||
| "ANTON_ROUTER_MODEL", | ||
| ) | ||
|
|
||
| # The ANTON_* vars the outbound export owns; every other .env line is preserved. | ||
| # Includes the two key vars the export no longer WRITES (ANTON_GEMINI_API_KEY, | ||
| # ANTON_OPENAI_API_KEY_CUSTOM — the pinned CLI has no field for them; gemini/oc | ||
| # creds ride the OpenAI slot) so a stale line from an older writer is still | ||
| # reconciled away. | ||
| MANAGED_ENV_VARS: tuple[str, ...] = tuple(SETTING_ENV_ALIASES.values()) + _MODEL_ENV_VARS | ||
|
|
||
|
|
||
| def normalize_provider_value(val: str, *, minds_key_present: bool) -> str: | ||
| """A .env / UI provider string -> the DB ``Provider`` enum value. | ||
|
|
||
| Hyphen->underscore canonicalization plus the "a Minds key is present, so | ||
| ``openai-compatible`` really means ``minds_cloud``" heuristic. The inverse | ||
| (DB -> UI/.env) is ``Provider.ui_value``. | ||
| """ | ||
| canonical = val.replace("-", "_") | ||
| if canonical == Provider.OPENAI_COMPATIBLE.value and minds_key_present: | ||
| return Provider.MINDS_CLOUD.value | ||
| return canonical | ||
|
|
||
|
|
||
| # ── inbound: .env -> DB ─────────────────────────────────────────────── | ||
|
|
||
| def env_to_db_updates(dotenv: dict[str, str]) -> dict[str, str]: | ||
| """A parsed ``.env`` dict -> ``{db_key: value}`` ready for the DB. | ||
|
|
||
| Maps ANTON_* names, skips absent/empty vars, normalizes provider fields. | ||
| Pure conversion — validation, encryption and the DB write stay in the caller. | ||
| """ | ||
| updates: dict[str, str] = {} | ||
| for env_var, setting_key in ENV_ALIAS_TO_SETTING.items(): | ||
| val = dotenv.get(env_var) | ||
| if not val: | ||
| continue | ||
| if setting_key.endswith("_provider"): | ||
| val = normalize_provider_value( | ||
| val, minds_key_present=bool(dotenv.get("ANTON_MINDS_API_KEY")) | ||
| ) | ||
| updates[setting_key] = val | ||
| return updates | ||
|
|
||
|
|
||
| # ── outbound: DB -> .env ────────────────────────────────────────────── | ||
|
|
||
| def _env_str(value: object) -> str: | ||
| """A loaded ``UserSettings`` value -> its ``.env`` string. | ||
|
|
||
| Providers use the dash form the CLI expects (``Provider.ui_value``); secrets | ||
| are the decrypted plaintext; booleans are lowercased. | ||
| """ | ||
| if isinstance(value, SecretStr): | ||
| return value.get_secret_value() | ||
| if isinstance(value, Provider): | ||
| return value.ui_value | ||
| if isinstance(value, bool): | ||
| return "true" if value else "false" | ||
| if isinstance(value, Enum): | ||
| return str(value.value) | ||
| return str(value) | ||
|
|
||
|
|
||
| def _is_dotenv_safe(value: str) -> bool: | ||
| """A value is safe to write as ``VAR=value`` only if it stays on one line. | ||
|
|
||
| A CR/LF in the value would terminate the assignment and turn the remainder | ||
| into a *new* line — e.g. ``minds_url = "https://x\\nDATABASE_URI=…"`` injects | ||
| an unmanaged ``DATABASE_URI`` that survives every later merge and is consumed | ||
| on the next CLI/server start. The exported fields (keys, URLs, providers, | ||
| booleans) never legitimately contain a newline, so we reject rather than | ||
| quote — a poisoned value is dropped, not smuggled into the file. | ||
| """ | ||
| return "\n" not in value and "\r" not in value | ||
|
|
||
|
|
||
| # The non-provider settings still export by a straight present-gated alias: | ||
| # booleans and the publish URL have no cross-field resolution. The provider / | ||
| # key / base-url / model cluster is rendered separately (see db_to_env) because | ||
| # it needs the SAME resolution the CLI and build_llm_client apply. | ||
| _OUTBOUND_FLAG_ALIASES: dict[str, str] = { | ||
| "memory_enabled": "ANTON_MEMORY_ENABLED", | ||
| "memory_mode": "ANTON_MEMORY_MODE", | ||
| "episodic_memory": "ANTON_EPISODIC_MEMORY", | ||
| "proactive_dashboards": "ANTON_PROACTIVE_DASHBOARDS", | ||
| "act_first": "ANTON_ACT_FIRST", | ||
| "publish_url": "ANTON_PUBLISH_URL", | ||
| } | ||
|
|
||
|
|
||
| def _anton_provider_name(p: Provider) -> str: | ||
| """A cowork ``Provider`` -> the provider name Anton understands ON DISK. | ||
|
|
||
| Anton has no first-class gemini provider: its own ``anton setup`` writes | ||
| gemini as ``openai-compatible`` + Google's base URL + the key in the OpenAI | ||
| slot, and ``LLMClient.from_settings`` maps ``minds-cloud`` -> openai-compatible | ||
| itself. So gemini is translated here; everything else is its kebab ui_value. | ||
| """ | ||
| return "openai-compatible" if p is Provider.GEMINI else p.ui_value | ||
|
|
||
|
|
||
| def _emit_provider_creds(out: dict[str, str], settings: UserSettings, p: Provider) -> None: | ||
| """Write ``p``'s API key (and base URL) into the ANTON_* slots the CLI reads. | ||
|
|
||
| openai / openai-compatible / gemini share the single ``ANTON_OPENAI_API_KEY`` / | ||
| ``ANTON_OPENAI_BASE_URL`` slots (as they do in AntonSettings); minds-cloud uses | ||
| the dedicated ``minds_*`` slots and derives its base from them. Uses | ||
| ``setdefault`` so the first (highest-priority, planning-first) provider wins | ||
| the shared OpenAI slot — Anton can't serve two different OpenAI-compatible | ||
| endpoints at once, so a mixed cross-role config resolves deterministically. | ||
| """ | ||
| from cowork.services.providers import provider_base_url # lazy: avoid import cycle | ||
|
|
||
| key = provider_api_key_str(settings, p) | ||
| if p is Provider.ANTHROPIC: | ||
| if key: | ||
| out.setdefault("ANTON_ANTHROPIC_API_KEY", key) | ||
| elif p is Provider.MINDS_CLOUD: | ||
| if key: | ||
| out.setdefault("ANTON_MINDS_API_KEY", key) | ||
| if settings.minds_url: | ||
| out.setdefault("ANTON_MINDS_URL", settings.minds_url) | ||
| else: # OPENAI / OPENAI_COMPATIBLE / GEMINI — all via the OpenAI slot | ||
| if key: | ||
| out.setdefault("ANTON_OPENAI_API_KEY", key) | ||
| base = provider_base_url(p.ui_value, openai_base_url=settings.openai_base_url or "") | ||
| if base: | ||
| out.setdefault("ANTON_OPENAI_BASE_URL", base) | ||
|
pnewsam marked this conversation as resolved.
Outdated
|
||
|
|
||
|
|
||
| def db_to_env(settings: UserSettings, present_keys: set[str]) -> dict[str, str]: | ||
| """A loaded ``UserSettings`` -> the ``{ANTON_*: value}`` the CLI can RUN. | ||
|
|
||
| Not a raw field dump: the provider / model / key / base cluster is rendered in | ||
| Anton's on-disk vocabulary via the SAME resolution the server's own | ||
| ``build_llm_client`` uses (``resolved_*`` + ``provider_base_url`` + | ||
| ``provider_api_key``). So a DB ``gemini`` provider exports as | ||
| ``openai-compatible`` + Google's base URL + the key in the OpenAI slot — the | ||
| shape the standalone CLI (and ``anton setup``) understands — instead of the | ||
| literal ``provider=gemini`` the pinned CLI rejects (ENG-1127 review). Each | ||
| role's provider is written WITH its resolved model so the pair is always | ||
| valid; a role whose resolved provider has no key exports nothing, so an | ||
| unconfigured or freshly-cleared install leaves the CLI on its own defaults. | ||
|
|
||
| The remaining settings (memory flags, publish URL) have no cross-field | ||
| resolution and export by a straight present-gated alias. | ||
| """ | ||
| out: dict[str, str] = {} | ||
|
|
||
| roles = ( | ||
| ("ANTON_PLANNING_PROVIDER", "ANTON_PLANNING_MODEL", | ||
| settings.resolved_planning_provider, settings.resolved_planning_model), | ||
| ("ANTON_CODING_PROVIDER", "ANTON_CODING_MODEL", | ||
| settings.resolved_coding_provider, settings.resolved_coding_model), | ||
| ("ANTON_ROUTER_PROVIDER", "ANTON_ROUTER_MODEL", | ||
| settings.resolved_router_provider, settings.resolved_router_model), | ||
|
pnewsam marked this conversation as resolved.
Outdated
|
||
| ) | ||
| creds_order: list[Provider] = [] # planning-first: wins the shared OpenAI slot | ||
| for prov_var, model_var, prov, model in roles: | ||
| if not provider_api_key_str(settings, prov): | ||
| continue # resolved provider has no key — nothing runnable for this role | ||
| out[prov_var] = _anton_provider_name(prov) | ||
| if model: | ||
| out[model_var] = model | ||
| if prov not in creds_order: | ||
| creds_order.append(prov) | ||
| for prov in creds_order: | ||
| _emit_provider_creds(out, settings, prov) | ||
|
|
||
| for db_key, env_var in _OUTBOUND_FLAG_ALIASES.items(): | ||
| if db_key not in present_keys: | ||
| continue | ||
| value = getattr(settings, db_key, None) | ||
| if value is not None: | ||
| text = _env_str(value) | ||
| if text: | ||
| out[env_var] = text | ||
|
|
||
| # One injection guard over everything emitted (keys, URLs, models, flags): a | ||
| # CR/LF-bearing value is a dotenv-injection vector, never a real setting. | ||
| safe: dict[str, str] = {} | ||
| for var, val in out.items(): | ||
| if _is_dotenv_safe(val): | ||
| safe[var] = val | ||
| else: | ||
| logger.warning("settings: refusing to export %s — value spans multiple lines", var) | ||
| return safe | ||
|
|
||
|
|
||
| def merge_env_lines(existing: str, managed: dict[str, str]) -> str: | ||
| """Rewrite the managed lines in ``existing``, preserving everything else. | ||
|
|
||
| Managed ANTON_* lines are dropped then re-appended in alias order (byte-stable | ||
| across identical states); unmanaged lines (auth token, CLI model pins, | ||
| comments) keep their place. A managed key absent from ``managed`` loses its | ||
| line — that is how a logout wipes credentials from the CLI's file too. | ||
|
|
||
| Newline-bearing managed values are skipped as a serialization invariant so a | ||
| single assignment can never expand into a second (injected) one, even if a | ||
| caller hands in an unsanitized dict. | ||
| """ | ||
| drop = tuple(f"{var}=" for var in MANAGED_ENV_VARS) | ||
| kept = [ln for ln in existing.split("\n") if ln and not ln.startswith(drop)] | ||
| kept.extend( | ||
| f"{var}={value}" for var, value in managed.items() if _is_dotenv_safe(value) | ||
| ) | ||
| return "\n".join(kept) + "\n" | ||
|
|
||
|
|
||
| # Transient Windows share-mode locks (the CLI or a version-skewed server holding | ||
| # ``.env`` open, an AV scan, a delete-pending handle still closing) abort the | ||
| # rename with one of these — the exact EPERM class that wedged onboarding on the | ||
| # CLIENT before it grew a retry (ENG-1209). Now that the server is the writer | ||
| # (ENG-1127), the same hardening has to live here. POSIX has no mandatory | ||
| # locking, so these are effectively Windows-only. | ||
| _TRANSIENT_LOCK_ERRNOS = frozenset({errno.EPERM, errno.EACCES, errno.EBUSY, errno.ENOTEMPTY}) | ||
| _REPLACE_ATTEMPTS = 6 | ||
| _REPLACE_BASE_DELAY_S = 0.06 | ||
|
|
||
| # Orphaned temps from a hard-kill / power-loss between the write and the rename | ||
| # hold the full plaintext key, so they must never linger; sweep only STALE ones | ||
| # so a concurrent writer's fresh in-flight temp is spared. | ||
| _STALE_TMP_S = 5 * 60 | ||
|
|
||
|
|
||
| def _is_transient_lock_error(exc: OSError) -> bool: | ||
| return exc.errno in _TRANSIENT_LOCK_ERRNOS | ||
|
|
||
|
|
||
| def _sweep_stale_temps(directory: Path) -> None: | ||
| """Remove orphaned ``.env.*.tmp`` files older than ``_STALE_TMP_S``. | ||
|
|
||
| Only stale temps go — a live writer's temp is fresh and spared, so this can't | ||
| yank one out from under a concurrent rename. | ||
| """ | ||
| try: | ||
| now = time.time() | ||
| for entry in directory.glob(".env.*.tmp"): | ||
| try: | ||
| if now - entry.stat().st_mtime > _STALE_TMP_S: | ||
| entry.unlink() | ||
| except OSError: | ||
| pass # gone already or unreadable — best-effort | ||
| except OSError: | ||
| pass # dir unreadable — nothing to sweep | ||
|
|
||
|
|
||
| def _replace_with_retry(tmp: str, dest: str) -> None: | ||
| """``os.replace`` the finished temp onto ``dest``, retrying transient locks. | ||
|
|
||
| The temp is already written to a fresh, unlocked path; only the rename | ||
| contends with a Windows share-mode lock, so that is all we retry — with a | ||
| widening backoff (~60ms..360ms, ~1.3s total) that mirrors the client's | ||
| ``retryOnTransientLock`` (ENG-1209). A non-lock error (ENOENT, ENOTDIR, a | ||
| genuinely unwritable target) rethrows at once. | ||
| """ | ||
| for attempt in range(_REPLACE_ATTEMPTS): | ||
| try: | ||
| os.replace(tmp, dest) | ||
| return | ||
| except OSError as exc: | ||
| if attempt < _REPLACE_ATTEMPTS - 1 and _is_transient_lock_error(exc): | ||
| time.sleep(_REPLACE_BASE_DELAY_S * (attempt + 1)) | ||
| continue | ||
| raise | ||
|
|
||
|
|
||
| def atomic_write_env(path: Path, content: str) -> None: | ||
| """Write ``content`` to ``path`` atomically, owner-only (0o600). | ||
|
|
||
| Temp file + ``os.replace`` so a crash or a concurrent CLI read never sees a | ||
| truncated ``.env``; 0o600 because the file holds plaintext API keys. The | ||
| rename is retried on transient Windows share-mode locks (ENG-1209/ENG-1127). | ||
| """ | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| _sweep_stale_temps(path.parent) | ||
| fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".env.", suffix=".tmp") | ||
| try: | ||
| with os.fdopen(fd, "w", encoding="utf-8") as fh: | ||
| fh.write(content) | ||
| os.chmod(tmp, 0o600) | ||
| _replace_with_retry(tmp, str(path)) | ||
| except BaseException: | ||
| try: | ||
| os.unlink(tmp) | ||
| except OSError: | ||
| pass | ||
| raise | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.