From b305e7a64c40c85589ef0e3f79e42857b5e5904b Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 00:01:37 +0000 Subject: [PATCH 1/5] feat(env): add .dailybot/env.json per-repo API key override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Introduce a new opt-in, gitignored `.dailybot/env.json` file that lets developers configure per-repo API keys + URLs for multiple environments (live, local, staging, ...) with one-command switching. When active, it overrides `DAILYBOT_API_KEY`, `config.json`, and the login Bearer session for the enclosing repo — enabling "logged into different orgs in different repos" without touching global state. ## Change Log ### Schema - New file: `/.dailybot/env.json` — `{ disabled?, active?, profiles[] }`. - Each profile: `{ name, api_key, api_url?, app_url? }`. - `active` top-level string points at one profile (or empty/null = inert). - `disabled: true` is a kill-switch that preserves `active` for quick re-enable. ### CLI (new `env` command group) - `dailybot env add --name --key [--api-url] [--app-url]` - `dailybot env use ` (empty string clears active) - `dailybot env show` (masked key, resolved URLs) - `dailybot env list` (all profiles, active marked) - `dailybot env remove [--yes]` - `dailybot env off` / `dailybot env on` (kill switch) ### Precedence (aditivo, non-breaking) Insert env.json at position 2 in the auth resolution order — above `agents.json` default, below `--profile` flag. Applies to `get_api_key()`, `get_api_url()`, `get_app_url()` so every command (agent + user-scoped) picks up the override transparently. ### Security - Broad `.gitignore` (`.dailybot/*`) covers env.json automatically; explicit comment added to make the intent obvious. - `0o600` enforced on write AND defensively on load. - **Fatal refuse-if-tracked guard**: `git ls-files --error-unmatch` runs on every load; a tracked env.json raises `RepoEnvError` with an actionable fix. The CLI refuses to operate until the file is untracked. - `dailybot env add` runs `git check-ignore` after writing; warns loudly when env.json is NOT covered by any ignore rule. - API keys masked (`abcd****`) in every display path. ### Docs - `AGENTS.md` rule 14 (auth resolution order) — env.json inserted at #2. - `docs/CONFIGURATION.md` — full new section "Repo-level env override". - `docs/SECURITY.md` — new subsection on the three-layer protection. - `README.md` — new "Env commands" section in the commands table. - `.gitignore` — explicit "NEVER excepted" comment for env.json. ### Tests - `tests/repo_env_test.py` — 58 tests covering find/load/save/mutate, active resolution, kill-switch, committed-guard (uses real `git init`), precedence in `get_api_key`/`get_api_url`/`get_app_url`, and `DailyBotClient` integration. - `tests/env_commands_test.py` — 23 tests covering the full CLI surface via `CliRunner`. - Suite grows from 913 → 994 passing. Zero regressions. ## Risks - None for backward compat — env.json is opt-in and non-existent by default; the resolver falls through untouched when the file is absent or inert. - One behavioral change worth noting: `get_api_key()` / `get_api_url()` / `get_app_url()` now do a walk-up filesystem check on every call. For the ~1-2 client constructions per invocation this is <5ms and negligible; no caching is needed. If profiling ever shows this as hot, a per-process cache is trivial to add later. Co-authored-by: Cursor --- .gitignore | 6 +- AGENTS.md | 15 +- README.md | 14 + dailybot_cli/commands/env.py | 359 ++++++++++++++ dailybot_cli/config.py | 593 ++++++++++++++++++++++- dailybot_cli/display.py | 87 +++- dailybot_cli/main.py | 2 + docs/CONFIGURATION.md | 150 +++++- docs/SECURITY.md | 11 + tests/env_commands_test.py | 357 ++++++++++++++ tests/repo_env_test.py | 909 +++++++++++++++++++++++++++++++++++ 11 files changed, 2486 insertions(+), 17 deletions(-) create mode 100644 dailybot_cli/commands/env.py create mode 100644 tests/env_commands_test.py create mode 100644 tests/repo_env_test.py diff --git a/.gitignore b/.gitignore index ba4fbe3..fc54e7f 100644 --- a/.gitignore +++ b/.gitignore @@ -59,7 +59,11 @@ pip-delete-this-directory.txt # Dailybot repo profile — broad-ignore the folder, but the per-repo # `profile.json` itself MUST be tracked (it pins the agent identity for # everyone working in this repo). Any other on-disk state Dailybot stores -# here (caches, transient OTP scratch, etc.) is correctly ignored. +# here (caches, transient OTP scratch, .dailybot/env.json for per-repo +# API keys, etc.) is correctly ignored. +# IMPORTANT: `.dailybot/env.json` is INTENTIONALLY NEVER excepted — it +# contains API keys and must never be committed. The CLI enforces this +# at load time with a fatal `RepoEnvError`. See docs/SECURITY.md. .dailybot/* !.dailybot/profile.json diff --git a/AGENTS.md b/AGENTS.md index 1e9da64..5a69495 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -270,15 +270,16 @@ class DailyBotClient: ... The agent commands resolve credentials in this strict order — changing it is a **breaking change** for users: 1. `--profile` flag (explicit profile from `~/.config/dailybot/agents.json`) -2. `/.dailybot/profile.json::profile` — the closest ancestor of `$PWD` containing this file pins a profile slug for everyone working in the repo -3. Default profile from `agents.json` -4. `DAILYBOT_API_KEY` environment variable -5. `dailybot config key=...` (stored in `~/.config/dailybot/config.json`) -6. Login session (Bearer token from `~/.config/dailybot/credentials.json`) +2. **`/.dailybot/env.json` active profile** — the closest ancestor of `$PWD` containing this file provides API key + optional `api_url` / `app_url` for the enclosing repo. Gitignored, opt-in, plain-text on disk (`0o600`). When `disabled: true` or `active` is empty/null/missing, the file is inert and resolution continues below. See § "Repo-level env override" in `docs/CONFIGURATION.md`. +3. `/.dailybot/profile.json::profile` — the closest ancestor of `$PWD` containing this file pins a profile slug for everyone working in the repo +4. Default profile from `agents.json` +5. `DAILYBOT_API_KEY` environment variable +6. `dailybot config key=...` (stored in `~/.config/dailybot/config.json`) +7. Login session (Bearer token from `~/.config/dailybot/credentials.json`) -The repo file may also pin the agent display name (`name`) and a `default_metadata` object that gets shallow-merged into every report. **Credentials never live in the repo file** — a `key` field in `.dailybot/profile.json` is a hard error. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md) for the per-field precedence and the security rule. +The `profile.json` file may also pin the agent display name (`name`) and a `default_metadata` object that gets shallow-merged into every report. **Credentials never live in `profile.json`** — a `key` field there is a hard error. `env.json` is the ONLY sanctioned place for API keys inside `.dailybot/`, and it is **fatally refused when tracked by git** (the CLI runs `git ls-files --error-unmatch` on load and raises `RepoEnvError` if the file is tracked). See [docs/CONFIGURATION.md](docs/CONFIGURATION.md) for the per-field precedence and the security rule. -The implementation lives in `dailybot_cli/commands/agent.py::_resolve_agent_context` and `dailybot_cli/api_client.py::_agent_headers`. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md). +The implementation lives in `dailybot_cli/config.py` (`get_active_env_profile`, `get_api_key`, `get_api_url`, `get_app_url`), `dailybot_cli/commands/agent.py::_resolve_agent_context`, and `dailybot_cli/api_client.py::_agent_headers`. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md). ### 15. Packaging & Versioning diff --git a/README.md b/README.md index ec2aab9..27119c7 100644 --- a/README.md +++ b/README.md @@ -992,6 +992,20 @@ same delivery path as `chat send`). | `dailybot agent message claim-all` | Mark all pending messages as delivered | | `dailybot agent email send` | Send an email through an agent | +### Env commands (per-repo API key overrides) + +`dailybot env` manages `.dailybot/env.json` — an **opt-in, gitignored** file that carries API keys + URLs for one or more environments (production, local dev, staging). One profile is *active* at a time; when set, it overrides `DAILYBOT_API_KEY`, `config.json`, and the login Bearer session **for the enclosing repo**. This is the recommended way to be "logged into different orgs in different repos" simultaneously. Full docs: [docs/CONFIGURATION.md § "Repo-level env override"](docs/CONFIGURATION.md#repo-level-env-override-dailybotenvjson). + +| Command | Description | +|---------|-------------| +| `dailybot env add --name NAME --key KEY [--api-url URL] [--app-url URL]` | Add a profile (creates the file + auto-active on first add) | +| `dailybot env use NAME` | Switch the active profile (empty string clears active) | +| `dailybot env show` | Show the currently resolved profile (API key masked) | +| `dailybot env list` | List every profile in the file (active marked) | +| `dailybot env remove NAME [--yes]` | Remove a profile (clears active if it was the active one) | +| `dailybot env off` | Disable the file without deleting it (preserves active) | +| `dailybot env on` | Re-enable the file (restores the previously active profile) | + ### Hook commands (agent harness integration) Local-only lifecycle commands that agent harnesses (Claude Code, Cursor, diff --git a/dailybot_cli/commands/env.py b/dailybot_cli/commands/env.py new file mode 100644 index 0000000..acb851c --- /dev/null +++ b/dailybot_cli/commands/env.py @@ -0,0 +1,359 @@ +"""Per-repo environment override commands (``dailybot env``). + +The ``env`` command group manages ``.dailybot/env.json`` — the opt-in, +gitignored file that carries API keys + URLs for one or more environments +(live, local, staging, ...). One profile can be *active* at a time; when set, +it overrides ``DAILYBOT_API_KEY`` / ``config.json`` / the login Bearer session +for the enclosing repo. + +Full docs (schema, precedence, security posture): ``docs/CONFIGURATION.md`` +section "Repo-level env override". +""" + +import subprocess +from pathlib import Path +from typing import Any + +import click + +from dailybot_cli.config import ( + REPO_ENV_FILENAME, + REPO_PROFILE_DIRNAME, + RepoEnvError, + add_env_profile, + find_repo_env_path, + find_repo_root, + get_active_env_profile, + load_repo_env, + remove_env_profile, + set_active_env_profile, + set_env_disabled, +) +from dailybot_cli.display import ( + console, + print_env_profile, + print_env_profiles_table, + print_error, + print_info, + print_success, + print_warning, +) + + +def _mask_key(value: str) -> str: + """Mask everything after the first 4 characters (safe display).""" + if not value: + return "****" + if len(value) <= 4: + return value[0] + "****" + return value[:4] + "****" + + +def _warn_if_env_json_not_gitignored(env_path: Path) -> None: + """Best-effort warning when ``.dailybot/env.json`` is not gitignored. + + Fires only when git is installed AND the enclosing directory is a git + repository AND the file is not currently ignored. If any of those + prerequisites is missing, we stay silent — there's nothing to warn about. + """ + import shutil + + if not shutil.which("git"): + return + try: + result = subprocess.run( + [ + "git", + "-C", + str(env_path.parent), + "check-ignore", + "--quiet", + "--", + env_path.name, + ], + capture_output=True, + timeout=5.0, + ) + except (OSError, subprocess.SubprocessError): + return + # exit 0 = ignored (safe), exit 1 = NOT ignored (unsafe), + # exit 128 = not a git repo (nothing to warn about). + if result.returncode == 1: + print_warning( + f"{env_path} is NOT gitignored. This file contains API keys and " + "must never be committed. Add `.dailybot/*` (except " + "`!.dailybot/profile.json`) to your .gitignore before committing." + ) + + +def _handle_env_error(exc: RepoEnvError) -> None: + """Uniform error handling for ``env`` subcommands.""" + print_error(str(exc)) + raise SystemExit(1) + + +# --- Group ------------------------------------------------------------------ + + +@click.group(name="env") +def env() -> None: + """Manage per-repo API key overrides in ``.dailybot/env.json``. + + \b + The file is opt-in and MUST be gitignored — it carries API keys for one + or more environments (live, local, staging). One profile is *active* at + a time; when set, it overrides DAILYBOT_API_KEY, config.json, and the + login Bearer session for this repo. + + \b + Common workflow: + dailybot env add --name local --key sk_xxx --api-url http://localhost:8000 + dailybot env add --name staging --key sk_yyy --api-url https://staging-api.example.com + dailybot env use staging # switch active + dailybot env show # inspect current + dailybot env off # temporarily disable (preserves active) + dailybot env on # re-enable + dailybot env list # all configured profiles + dailybot env remove staging # delete a profile + + \b + Precedence (highest wins): + 1. --api-url / --app-url / --profile CLI flags + 2. .dailybot/env.json active profile <-- this file + 3. .dailybot/profile.json + agents.json + 4. DAILYBOT_API_KEY env var + 5. config.json (dailybot config key=...) + 6. Login session Bearer token + + Full docs: docs/CONFIGURATION.md § "Repo-level env override". + """ + + +# --- env add ---------------------------------------------------------------- + + +@env.command(name="add") +@click.option("--name", "-n", required=True, help="Profile name (unique per env.json).") +@click.option("--key", "-k", required=True, help="API key for this environment.") +@click.option( + "--api-url", + "api_url", + default=None, + help="Optional API base URL for this profile (e.g. http://localhost:8000).", +) +@click.option( + "--app-url", + "app_url", + default=None, + help="Optional webapp/dashboard URL for this profile.", +) +def env_add(name: str, key: str, api_url: str | None, app_url: str | None) -> None: + """Add a profile to ``.dailybot/env.json``, creating the file if needed. + + \b + The first profile added is automatically set as active. Subsequent adds + keep the current active profile — switch with `dailybot env use `. + + \b + Examples: + dailybot env add --name local --key sk_local_xxx \\ + --api-url http://localhost:8000 --app-url http://localhost:8090 + dailybot env add --name live --key sk_live_yyy + """ + try: + path, became_active = add_env_profile( + name=name, api_key=key, api_url=api_url, app_url=app_url + ) + except RepoEnvError as exc: + _handle_env_error(exc) + return # unreachable, for type checkers + + if became_active: + print_success(f"Created {path} and added profile '{name}' (set as active).") + else: + print_success(f"Added profile '{name}' to {path}.") + print_info(f"Active profile unchanged. Switch with: dailybot env use {name}") + + _warn_if_env_json_not_gitignored(path) + + +# --- env use ---------------------------------------------------------------- + + +@env.command(name="use") +@click.argument("name", required=True) +def env_use(name: str) -> None: + """Set the active profile in ``.dailybot/env.json``. + + \b + Pass an empty string to clear the active profile (the file stays, but + the CLI falls through to global auth): + + dailybot env use local + dailybot env use "" # clear active + """ + try: + target: str | None = name or None + path: Path = set_active_env_profile(target) + except RepoEnvError as exc: + _handle_env_error(exc) + return + + if target is None: + print_success(f"Cleared active profile in {path}.") + print_info("The CLI will now fall back to global auth (env var, config, or login).") + else: + print_success(f"Active profile is now '{target}' in {path}.") + + +# --- env show --------------------------------------------------------------- + + +@env.command(name="show") +def env_show() -> None: + """Show the currently resolved env.json profile (API key masked).""" + env_path: Path | None = find_repo_env_path() + if not env_path: + # Non-error: the file is optional. Point the user at the setup command. + candidate: Path = find_repo_root() / REPO_PROFILE_DIRNAME / REPO_ENV_FILENAME + print_info(f"No {candidate} found. Run `dailybot env add ...` to create one.") + return + + try: + data: dict[str, Any] | None = load_repo_env() + except RepoEnvError as exc: + _handle_env_error(exc) + return + + if not data: + print_warning(f"{env_path} is malformed. Fix or remove it.") + return + + if data.get("disabled"): + preserved: str = str(data.get("active") or "(none)") + console.print( + f"[bold yellow]env.json is disabled[/bold yellow] " + f"(active would be: [dim]{preserved}[/dim])" + ) + print_info(f"Path: {env_path}") + print_info("Re-enable with: dailybot env on") + return + + active_profile: dict[str, Any] | None = get_active_env_profile() + if not active_profile: + active_raw: str = str(data.get("active") or "") + if active_raw: + print_warning( + f"active='{active_raw}' but no profile in {env_path} matches " + f"that name. Set with: dailybot env use " + ) + else: + print_info(f"{env_path} exists but has no active profile.") + print_info("Choose one with: dailybot env use ") + return + + print_env_profile(active_profile, env_path, _mask_key) + + +# --- env list --------------------------------------------------------------- + + +@env.command(name="list") +def env_list() -> None: + """List every profile in ``.dailybot/env.json`` (active marked).""" + env_path: Path | None = find_repo_env_path() + if not env_path: + print_info("No .dailybot/env.json found. Create one with `dailybot env add ...`.") + return + + try: + data: dict[str, Any] | None = load_repo_env() + except RepoEnvError as exc: + _handle_env_error(exc) + return + + if not data: + print_warning(f"{env_path} is malformed.") + return + + profiles: list[dict[str, Any]] = data.get("profiles", []) + if not profiles: + print_info(f"{env_path} has no profiles. Add one with `dailybot env add ...`.") + return + + print_env_profiles_table( + profiles=profiles, + active=data.get("active"), + disabled=bool(data.get("disabled")), + path=env_path, + mask=_mask_key, + ) + + +# --- env remove ------------------------------------------------------------- + + +@env.command(name="remove") +@click.argument("name", required=True) +@click.option( + "--yes", + "-y", + is_flag=True, + default=False, + help="Skip the interactive confirmation.", +) +def env_remove(name: str, yes: bool) -> None: + """Remove a profile from ``.dailybot/env.json``. + + \b + If the removed profile was the active one, the active pointer is cleared + and the CLI falls back to global auth until you run `dailybot env use`. + """ + if not yes and not click.confirm(f"Remove profile '{name}' from env.json?"): + print_info("Cancelled.") + return + + try: + path, cleared = remove_env_profile(name) + except RepoEnvError as exc: + _handle_env_error(exc) + return + + if cleared: + print_success(f"Removed profile '{name}' from {path} (was active — active cleared).") + print_info("The CLI will fall back to global auth until you run `dailybot env use`.") + else: + print_success(f"Removed profile '{name}' from {path}.") + + +# --- env off / on ---------------------------------------------------------- + + +@env.command(name="off") +def env_off() -> None: + """Disable ``.dailybot/env.json`` without deleting it (preserves active).""" + try: + path: Path = set_env_disabled(True) + except RepoEnvError as exc: + _handle_env_error(exc) + return + + print_success(f"Disabled {path}. The CLI now uses global auth.") + print_info("Re-enable with: dailybot env on") + + +@env.command(name="on") +def env_on() -> None: + """Re-enable ``.dailybot/env.json`` (restores the previously active profile).""" + try: + path: Path = set_env_disabled(False) + except RepoEnvError as exc: + _handle_env_error(exc) + return + + print_success(f"Enabled {path}.") + active: dict[str, Any] | None = get_active_env_profile() + if active: + print_info(f"Active profile: {active['name']}") + else: + print_info("No active profile set. Choose one with: dailybot env use ") diff --git a/dailybot_cli/config.py b/dailybot_cli/config.py index 53707b5..a524c63 100644 --- a/dailybot_cli/config.py +++ b/dailybot_cli/config.py @@ -104,9 +104,23 @@ def clear_credentials() -> None: def get_api_url() -> str: - """Return the API URL (--api-url flag > env var > credentials > default).""" + """Return the API URL. + + Resolution order (highest layer wins): + 1. ``--api-url`` flag (via :func:`set_api_url_override`) + 2. ``.dailybot/env.json`` active profile's ``api_url`` (walk-up from cwd) + 3. ``DAILYBOT_API_URL`` env var + 4. ``credentials.json::api_url`` (login session's stored URL) + 5. :data:`DEFAULT_API_URL` + + Errors reading env.json are swallowed here (they surface at the CLI + entry point). This keeps the plumbing side of things resilient. + """ if _api_url_override: return _api_url_override + env_profile: dict[str, Any] | None = _safe_active_env_profile() + if env_profile and env_profile.get("api_url"): + return str(env_profile["api_url"]).rstrip("/") env_url: str | None = os.environ.get("DAILYBOT_API_URL") if env_url: return env_url.rstrip("/") @@ -117,9 +131,19 @@ def get_api_url() -> str: def get_app_url() -> str: - """Return the webapp URL (--app-url flag > env var > default).""" + """Return the webapp URL. + + Resolution order (highest layer wins): + 1. ``--app-url`` flag (via :func:`set_app_url_override`) + 2. ``.dailybot/env.json`` active profile's ``app_url`` + 3. ``DAILYBOT_APP_URL`` env var + 4. :data:`DEFAULT_APP_URL` + """ if _app_url_override: return _app_url_override + env_profile: dict[str, Any] | None = _safe_active_env_profile() + if env_profile and env_profile.get("app_url"): + return str(env_profile["app_url"]).rstrip("/") env_url: str | None = os.environ.get("DAILYBOT_APP_URL") if env_url: return env_url.rstrip("/") @@ -163,7 +187,21 @@ def save_config(data: dict[str, Any]) -> None: def get_api_key() -> str | None: - """Return the org API key (env var > stored config > None).""" + """Return the org API key. + + Resolution order (highest layer wins): + 1. ``.dailybot/env.json`` active profile's ``api_key`` (walk-up from cwd) + 2. ``DAILYBOT_API_KEY`` env var + 3. ``config.json::api_key`` (set via ``dailybot config key=...``) + 4. ``None`` + + Errors reading env.json are swallowed here so plumbing stays resilient; + the CLI entry point calls :func:`load_repo_env` explicitly to surface + fatal misconfigurations (e.g. env.json tracked in git). + """ + env_profile: dict[str, Any] | None = _safe_active_env_profile() + if env_profile and env_profile.get("api_key"): + return str(env_profile["api_key"]) env_key: str | None = os.environ.get("DAILYBOT_API_KEY") if env_key: return env_key @@ -171,6 +209,23 @@ def get_api_key() -> str | None: return config.get("api_key") or None +def _safe_active_env_profile() -> dict[str, Any] | None: + """Return the active env.json profile, swallowing any error. + + ``get_api_key`` / ``get_api_url`` / ``get_app_url`` call this on every + invocation; they must never raise. The CLI entry point calls + :func:`load_repo_env` directly at startup so fatal errors (e.g. env.json + tracked in git) still surface loudly — this helper is the resilient + plumbing-side accessor. + """ + try: + return get_active_env_profile() + except RepoEnvError: + return None + except Exception: + return None + + def save_org_cache(email: str, organizations: list[dict[str, Any]]) -> None: """Cache the org list from request_code for UUID resolution in step 2.""" get_config_dir() @@ -526,6 +581,31 @@ def resolve_active_profile( agent_name, name_source = "CLI Agent", "default" api_key: str | None = profile_data.get("api_key") if profile_data else None + api_key_source: str = "global" if api_key else "absent" + + # env.json takes precedence over the global agent profile for credentials. + # We surface it so `agent profiles --resolve` shows the full picture. + env_profile: dict[str, Any] | None = None + env_profile_error: str | None = None + if not profile_flag: + try: + env_profile = get_active_env_profile(cwd) + except RepoEnvError as exc: + env_profile_error = str(exc) + if env_profile and env_profile.get("api_key"): + api_key = str(env_profile["api_key"]) + api_key_source = "env.json" + + env_api_url: str | None = ( + str(env_profile["api_url"]).rstrip("/") + if env_profile and env_profile.get("api_url") + else None + ) + env_app_url: str | None = ( + str(env_profile["app_url"]).rstrip("/") + if env_profile and env_profile.get("app_url") + else None + ) return { "agent_name": agent_name, @@ -536,9 +616,14 @@ def resolve_active_profile( "profile_missing_from_flag": profile_missing_from_flag, "profile_missing_from_repo": profile_missing_from_repo, "repo_profile_path": repo_path, + "env_profile_name": env_profile.get("name") if env_profile else None, + "env_profile_api_url": env_api_url, + "env_profile_app_url": env_app_url, + "env_profile_error": env_profile_error, "resolved_from": { "agent_name": name_source, "profile": profile_source, + "api_key": api_key_source, "default_metadata": "repo" if repo_default_metadata else "absent", }, } @@ -620,3 +705,505 @@ def write_repo_profile( profile_path.write_text(json.dumps(merged, indent=2) + "\n") return profile_path + + +# --- Repo-level env override (.dailybot/env.json) --- +# +# Optional per-repo file that carries API keys + URLs for one or more +# environments (live, local, staging, ...). One profile can be "active" at +# a time; when set, it overrides env vars, config.json, and the login +# Bearer session for that repo. The file is opt-in and MUST NEVER be +# committed to git — the load path enforces this with a fatal guard. +# +# Full schema: +# +# { +# "disabled": false, # optional; true = ignore this file entirely +# "active": "local org 1", # optional; empty/null/missing = inert +# "profiles": [ +# { +# "name": "live", +# "api_key": "xxxxxxx" +# }, +# { +# "name": "local org 1", +# "api_key": "xxxxxxx", +# "api_url": "http://localhost:8000", # optional +# "app_url": "http://localhost:8090" # optional +# } +# ] +# } +# +# See docs/CONFIGURATION.md for the full precedence table and +# docs/SECURITY.md for the security posture. + +REPO_ENV_FILENAME: str = "env.json" +_VALID_REPO_ENV_TOP_KEYS: frozenset[str] = frozenset({"active", "disabled", "profiles"}) +_VALID_REPO_ENV_PROFILE_KEYS: frozenset[str] = frozenset({"name", "api_key", "api_url", "app_url"}) +_REQUIRED_REPO_ENV_PROFILE_KEYS: frozenset[str] = frozenset({"name", "api_key"}) +_GIT_CHECK_TIMEOUT_SECS: float = 5.0 + +_warned_env_paths: set[str] = set() + + +class RepoEnvError(Exception): + """Raised when ``.dailybot/env.json`` violates a hard rule. + + Currently only one hard rule: the file must not be tracked by git. + Write-side validation errors (missing required keys, duplicate names, + unknown keys) also raise this so callers get a single exception type + to catch. + """ + + +def reset_repo_env_warnings() -> None: + """Clear the per-process warning dedup set. Useful in tests.""" + _warned_env_paths.clear() + + +def find_repo_env_path(cwd: Path | None = None) -> Path | None: + """Walk up from *cwd* to find the closest ``.dailybot/env.json``. + + Returns ``None`` when no ancestor contains the file, or when the file is + non-regular. Mirrors the semantics of :func:`find_repo_profile_path`. + """ + start: Path = (cwd or Path.cwd()).resolve() + for ancestor in [start, *start.parents]: + candidate_dir: Path = ancestor / REPO_PROFILE_DIRNAME + if not candidate_dir.is_dir(): + continue + env_path: Path = candidate_dir / REPO_ENV_FILENAME + if env_path.is_file(): + return env_path + return None + + +def _is_env_tracked_by_git(env_path: Path) -> bool: + """Return ``True`` iff ``env_path`` is tracked by its containing git repo. + + Returns ``False`` when: + - git is not installed on PATH, + - the file is not inside a git repository, + - the file is inside a git repo but is properly untracked + (``.gitignore`` covers it, or it was never ``git add``ed). + + Only ``True`` is a security violation. Kept as a top-level function so + tests can patch it without needing a real git repo. + """ + import shutil + import subprocess + + if not shutil.which("git"): + return False + + try: + result = subprocess.run( + [ + "git", + "-C", + str(env_path.parent), + "ls-files", + "--error-unmatch", + "--", + env_path.name, + ], + capture_output=True, + timeout=_GIT_CHECK_TIMEOUT_SECS, + ) + except (OSError, subprocess.SubprocessError): + return False + return result.returncode == 0 + + +def _warn_env_once(path_key: str, message: str) -> None: + """Emit a warning once per *path_key* per process.""" + if path_key in _warned_env_paths: + return + _warned_env_paths.add(path_key) + from dailybot_cli.display import print_warning + + print_warning(message) + + +def load_repo_env(cwd: Path | None = None) -> dict[str, Any] | None: + """Load and validate ``.dailybot/env.json``. + + Returns the parsed dict augmented with ``disabled`` (bool) and ``_path`` + (str), or ``None`` when the file is absent or malformed. Raises + :class:`RepoEnvError` when the file is tracked by git — that is a hard + security violation and the CLI refuses to operate until it's fixed. + + On the first successful load in a process, the file's permissions are + tightened to ``0o600`` defensively (in case it was created via an editor + or ``cp`` that used the default umask). + """ + path: Path | None = find_repo_env_path(cwd) + if not path: + return None + + if _is_env_tracked_by_git(path): + try: + rel_path: str = str(path.relative_to(Path.cwd())) + except ValueError: + rel_path = str(path) + raise RepoEnvError( + f"{path} is tracked by git. This file contains API keys and must " + "never be committed. Fix with:\n" + f" git rm --cached {rel_path}\n" + " # ensure your .gitignore ignores .dailybot/env.json\n" + " git commit -m 'chore: untrack .dailybot/env.json'\n" + "The CLI refuses to load env.json while it is tracked." + ) + + # Defensive chmod — an editor or `cp` may have created the file with the + # default umask (typically 0o644). Bring it back to owner-only quietly. + import contextlib + + with contextlib.suppress(OSError): + os.chmod(path, 0o600) + + try: + raw: str = path.read_text() + data: Any = json.loads(raw) + except (OSError, json.JSONDecodeError) as exc: + _warn_env_once( + f"parse:{path}", + f"Could not parse {path}: {exc}. Falling back to global auth.", + ) + return None + + if not isinstance(data, dict): + _warn_env_once( + f"shape:{path}", + f"{path} must contain a JSON object. Falling back to global auth.", + ) + return None + + unknown_top: set[str] = set(data.keys()) - _VALID_REPO_ENV_TOP_KEYS + if unknown_top: + _warn_env_once( + f"unknown-top:{path}", + f"{path} has unknown top-level key(s) {sorted(unknown_top)}; ignoring.", + ) + + profiles_raw: Any = data.get("profiles") + if not isinstance(profiles_raw, list): + _warn_env_once( + f"profiles-shape:{path}", + f"{path} 'profiles' must be a list. Falling back to global auth.", + ) + return None + + profiles: list[dict[str, Any]] = [] + seen_names: set[str] = set() + for i, entry in enumerate(profiles_raw): + if not isinstance(entry, dict): + _warn_env_once( + f"entry-shape:{path}:{i}", + f"{path} profiles[{i}] must be an object; skipping.", + ) + continue + missing: frozenset[str] = _REQUIRED_REPO_ENV_PROFILE_KEYS - set(entry.keys()) + if missing: + _warn_env_once( + f"entry-required:{path}:{i}", + f"{path} profiles[{i}] missing required key(s) {sorted(missing)}; skipping.", + ) + continue + unknown_profile: set[str] = set(entry.keys()) - _VALID_REPO_ENV_PROFILE_KEYS + if unknown_profile: + _warn_env_once( + f"unknown-profile:{path}:{i}", + f"{path} profiles[{i}] has unknown key(s) {sorted(unknown_profile)}; ignoring.", + ) + cleaned: dict[str, Any] = {k: entry[k] for k in _VALID_REPO_ENV_PROFILE_KEYS if k in entry} + name: str = cleaned["name"] + if name in seen_names: + _warn_env_once( + f"entry-duplicate:{path}:{i}", + f"{path} profiles[{i}] duplicate name '{name}'; keeping the first.", + ) + continue + seen_names.add(name) + profiles.append(cleaned) + + active_raw: Any = data.get("active") + active: str | None = active_raw if isinstance(active_raw, str) and active_raw else None + + disabled_raw: Any = data.get("disabled", False) + disabled: bool = bool(disabled_raw) if isinstance(disabled_raw, bool) else False + + return { + "active": active, + "disabled": disabled, + "profiles": profiles, + "_path": str(path), + } + + +def get_active_env_profile(cwd: Path | None = None) -> dict[str, Any] | None: + """Return the active profile from ``.dailybot/env.json``, or ``None``. + + Returns ``None`` when: + - the env.json file does not exist, + - the file has ``disabled: true``, + - the file has no ``active`` field (or it's empty/null), + - ``active`` points at a name that does not exist in ``profiles``. + + Raises :class:`RepoEnvError` when the file is tracked by git (fatal). + """ + data: dict[str, Any] | None = load_repo_env(cwd) + if not data: + return None + if data.get("disabled"): + return None + active_name: str | None = data.get("active") + if not active_name: + return None + for profile in data["profiles"]: + if profile.get("name") == active_name: + return profile + return None + + +def _validate_env_payload(payload: dict[str, Any]) -> None: + """Raise :class:`RepoEnvError` if the payload cannot be safely written.""" + unknown_top: set[str] = set(payload.keys()) - _VALID_REPO_ENV_TOP_KEYS + if unknown_top: + raise RepoEnvError( + f"Unknown top-level key(s): {sorted(unknown_top)}. " + f"Allowed: {sorted(_VALID_REPO_ENV_TOP_KEYS)}." + ) + + profiles: Any = payload.get("profiles", []) + if not isinstance(profiles, list): + raise RepoEnvError("'profiles' must be a list.") + + seen_names: set[str] = set() + for i, entry in enumerate(profiles): + if not isinstance(entry, dict): + raise RepoEnvError(f"profiles[{i}] must be an object.") + missing: frozenset[str] = _REQUIRED_REPO_ENV_PROFILE_KEYS - set(entry.keys()) + if missing: + raise RepoEnvError(f"profiles[{i}] missing required key(s) {sorted(missing)}.") + unknown: set[str] = set(entry.keys()) - _VALID_REPO_ENV_PROFILE_KEYS + if unknown: + raise RepoEnvError( + f"profiles[{i}] has unknown key(s) {sorted(unknown)}. " + f"Allowed: {sorted(_VALID_REPO_ENV_PROFILE_KEYS)}." + ) + name: Any = entry["name"] + if not isinstance(name, str) or not name.strip(): + raise RepoEnvError(f"profiles[{i}]['name'] must be a non-empty string.") + if name in seen_names: + raise RepoEnvError(f"Duplicate profile name '{name}'.") + seen_names.add(name) + + active: Any = payload.get("active") + if active is not None and active != "": + if not isinstance(active, str): + raise RepoEnvError("'active' must be a string.") + if active not in seen_names: + raise RepoEnvError(f"'active' points to '{active}' but no profile has that name.") + + disabled: Any = payload.get("disabled") + if disabled is not None and not isinstance(disabled, bool): + raise RepoEnvError("'disabled' must be a boolean.") + + +def save_repo_env(payload: dict[str, Any], *, cwd: Path | None = None) -> Path: + """Write ``.dailybot/env.json`` at the repo root and return the path. + + Validates *payload* via :func:`_validate_env_payload` before writing. + Anchors at the git repo root (``find_repo_root``) so the file lives at + a stable location regardless of where the caller ran from. Always sets + mode ``0o600`` per the credential-hygiene rules in AGENTS.md. + """ + _validate_env_payload(payload) + + repo_root: Path = find_repo_root(cwd) + env_dir: Path = repo_root / REPO_PROFILE_DIRNAME + env_dir.mkdir(parents=True, exist_ok=True) + env_path: Path = env_dir / REPO_ENV_FILENAME + env_path.write_text(json.dumps(_normalize_env_payload(payload), indent=2) + "\n") + os.chmod(env_path, 0o600) + return env_path + + +def _normalize_env_payload(payload: dict[str, Any]) -> dict[str, Any]: + """Return a payload with keys in stable order for reproducible writes. + + The on-disk order is intentional so a diff between two writes stays + minimal (helps humans spot real changes when the file is inspected). + """ + ordered: dict[str, Any] = {} + if payload.get("disabled"): + ordered["disabled"] = True + active: Any = payload.get("active") + if active is not None and active != "": + ordered["active"] = active + else: + # Explicit null keeps the intent visible (developer can flip it back + # with an editor if the CLI is unavailable). + ordered["active"] = None + profiles: list[dict[str, Any]] = payload.get("profiles", []) or [] + ordered["profiles"] = [_normalize_env_entry(p) for p in profiles] + return ordered + + +def _normalize_env_entry(entry: dict[str, Any]) -> dict[str, Any]: + """Return a profile entry with a canonical key order.""" + out: dict[str, Any] = {"name": entry["name"], "api_key": entry["api_key"]} + if entry.get("api_url"): + out["api_url"] = str(entry["api_url"]).rstrip("/") + if entry.get("app_url"): + out["app_url"] = str(entry["app_url"]).rstrip("/") + return out + + +def _read_or_init_env(cwd: Path | None) -> dict[str, Any]: + """Return the existing env.json as an editable dict, or a fresh skeleton.""" + path: Path | None = find_repo_env_path(cwd) + if not path: + return {"active": None, "disabled": False, "profiles": []} + data: dict[str, Any] | None = load_repo_env(cwd) + if not data: + return {"active": None, "disabled": False, "profiles": []} + return { + "active": data.get("active"), + "disabled": bool(data.get("disabled")), + "profiles": [dict(p) for p in data.get("profiles", [])], + } + + +def add_env_profile( + name: str, + api_key: str, + api_url: str | None = None, + app_url: str | None = None, + *, + cwd: Path | None = None, +) -> tuple[Path, bool]: + """Add a profile to ``.dailybot/env.json``, creating the file if needed. + + Returns ``(path, became_active)``. When the file did not previously + exist (or had no active profile), the new profile is auto-set as active + and ``became_active`` is ``True``. + """ + data: dict[str, Any] = _read_or_init_env(cwd) + profiles: list[dict[str, Any]] = data["profiles"] + + if any(p.get("name") == name for p in profiles): + raise RepoEnvError( + f"A profile named '{name}' already exists in .dailybot/env.json. " + "Use `dailybot env remove` first, or pick a different name." + ) + + entry: dict[str, Any] = {"name": name, "api_key": api_key} + if api_url: + entry["api_url"] = api_url.rstrip("/") + if app_url: + entry["app_url"] = app_url.rstrip("/") + profiles.append(entry) + + became_active: bool = False + if not data.get("active"): + data["active"] = name + became_active = True + + path: Path = save_repo_env( + { + "disabled": bool(data.get("disabled")), + "active": data.get("active"), + "profiles": profiles, + }, + cwd=cwd, + ) + return path, became_active + + +def remove_env_profile(name: str, *, cwd: Path | None = None) -> tuple[Path, bool]: + """Remove a profile from ``.dailybot/env.json``. + + Returns ``(path, cleared_active)`` where ``cleared_active`` is ``True`` + when the removed profile was the currently active one. Raises + :class:`RepoEnvError` if the file does not exist or the profile is not + present. + """ + path: Path | None = find_repo_env_path(cwd) + if not path: + raise RepoEnvError("No .dailybot/env.json found in the current directory or its ancestors.") + data: dict[str, Any] | None = load_repo_env(cwd) + if not data: + raise RepoEnvError(f"{path} is malformed; cannot remove profile.") + + if not any(p.get("name") == name for p in data["profiles"]): + raise RepoEnvError(f"No profile named '{name}' in {path}.") + + new_profiles: list[dict[str, Any]] = [ + dict(p) for p in data["profiles"] if p.get("name") != name + ] + active: str | None = data.get("active") + cleared_active: bool = False + if active == name: + active = None + cleared_active = True + + save_path: Path = save_repo_env( + { + "disabled": bool(data.get("disabled")), + "active": active, + "profiles": new_profiles, + }, + cwd=cwd, + ) + return save_path, cleared_active + + +def set_active_env_profile(name: str | None, *, cwd: Path | None = None) -> Path: + """Set (or clear) the active profile in ``.dailybot/env.json``. + + ``name=None`` clears the active profile (the file stays, but the CLI + falls through to global auth). Raises :class:`RepoEnvError` if the + file does not exist or *name* does not match any known profile. + """ + path: Path | None = find_repo_env_path(cwd) + if not path: + raise RepoEnvError("No .dailybot/env.json found. Run `dailybot env add ...` first.") + data: dict[str, Any] | None = load_repo_env(cwd) + if not data: + raise RepoEnvError(f"{path} is malformed; cannot set active profile.") + if name is not None and not any(p.get("name") == name for p in data["profiles"]): + available: str = ", ".join(sorted(str(p["name"]) for p in data["profiles"])) or "(none)" + raise RepoEnvError(f"No profile named '{name}' in {path}. Available: {available}") + return save_repo_env( + { + "disabled": bool(data.get("disabled")), + "active": name, + "profiles": [dict(p) for p in data["profiles"]], + }, + cwd=cwd, + ) + + +def set_env_disabled(disabled: bool, *, cwd: Path | None = None) -> Path: + """Toggle the ``disabled`` kill-switch in ``.dailybot/env.json``. + + Preserves the ``active`` selection so that turning the file back on + restores the previously chosen profile. Raises :class:`RepoEnvError` + when the file does not exist. + """ + path: Path | None = find_repo_env_path(cwd) + if not path: + raise RepoEnvError("No .dailybot/env.json found. Run `dailybot env add ...` first.") + data: dict[str, Any] | None = load_repo_env(cwd) + if not data: + raise RepoEnvError(f"{path} is malformed; cannot toggle disabled state.") + return save_repo_env( + { + "disabled": bool(disabled), + "active": data.get("active"), + "profiles": [dict(p) for p in data["profiles"]], + }, + cwd=cwd, + ) diff --git a/dailybot_cli/display.py b/dailybot_cli/display.py index 93b74f2..1af6a4e 100644 --- a/dailybot_cli/display.py +++ b/dailybot_cli/display.py @@ -487,7 +487,22 @@ def print_resolved_profile(resolved: dict[str, Any]) -> None: table.add_row("Profile slug", slug, sources.get("profile", "")) api_key: str | None = resolved.get("api_key") - table.add_row("API key", "set" if api_key else "(none)", "global" if api_key else "") + api_key_source: str = sources.get("api_key", "absent") + table.add_row( + "API key", + "set" if api_key else "(none)", + api_key_source if api_key else "", + ) + + env_name: str | None = resolved.get("env_profile_name") + if env_name: + table.add_row("env.json profile", env_name, "walk-up") + env_api_url: str | None = resolved.get("env_profile_api_url") + if env_api_url: + table.add_row("env.json API URL", env_api_url, "env.json") + env_app_url: str | None = resolved.get("env_profile_app_url") + if env_app_url: + table.add_row("env.json Webapp URL", env_app_url, "env.json") repo_path: str | None = resolved.get("repo_profile_path") table.add_row("Repo file", repo_path or "(not found)", "walk-up" if repo_path else "") @@ -510,6 +525,76 @@ def print_resolved_profile(resolved: dict[str, Any]) -> None: "in agents.json. Falling back to session credentials." ) + env_error: str | None = resolved.get("env_profile_error") + if env_error: + print_warning(f".dailybot/env.json is invalid: {env_error}") + + +def print_env_profile( + profile: dict[str, Any], + path: Any, + mask: Any, +) -> None: + """Render a single resolved ``env.json`` profile with the API key masked. + + ``path`` is the on-disk path to env.json (shown as provenance so the + developer can find and edit the file). ``mask`` is a callable that + takes the raw key and returns the display-safe representation. + """ + table: Table = Table(title="Active env.json Profile", border_style="cyan") + table.add_column("Field", style="bold") + table.add_column("Value") + table.add_row("Profile", str(profile.get("name", ""))) + api_key: Any = profile.get("api_key", "") + table.add_row("API key", mask(str(api_key)) if api_key else "[dim]—[/dim]") + table.add_row( + "API URL", + str(profile.get("api_url", "")) or "[dim](default)[/dim]", + ) + table.add_row( + "Webapp URL", + str(profile.get("app_url", "")) or "[dim](default)[/dim]", + ) + table.add_row("Source", str(path)) + console.print(table) + + +def print_env_profiles_table( + profiles: list[dict[str, Any]], + active: str | None, + disabled: bool, + path: Any, + mask: Any, +) -> None: + """Render every ``env.json`` profile, marking the active one. + + When ``disabled`` is True, a warning banner appears above the table. + ``mask`` is used to redact each row's API key for safe display. + """ + if disabled: + console.print( + "[bold yellow]env.json is currently disabled[/bold yellow] " + "(re-enable with `dailybot env on`)" + ) + table: Table = Table(title=f"Profiles in {path}", border_style="cyan") + table.add_column("Active", justify="center") + table.add_column("Name", style="bold") + table.add_column("API key") + table.add_column("API URL") + table.add_column("Webapp URL") + for profile in profiles: + name: str = str(profile.get("name", "")) + is_active: str = "[green]•[/green]" if name == (active or "") else "" + api_key: Any = profile.get("api_key", "") + table.add_row( + is_active, + name, + mask(str(api_key)) if api_key else "[dim]—[/dim]", + str(profile.get("api_url", "")) or "[dim](default)[/dim]", + str(profile.get("app_url", "")) or "[dim](default)[/dim]", + ) + console.print(table) + def print_registration_result(data: dict[str, Any]) -> None: """Display agent registration result.""" diff --git a/dailybot_cli/main.py b/dailybot_cli/main.py index 7113eab..cafab6d 100644 --- a/dailybot_cli/main.py +++ b/dailybot_cli/main.py @@ -13,6 +13,7 @@ from dailybot_cli.commands.checkin import checkin from dailybot_cli.commands.config import config from dailybot_cli.commands.conversation import conversation +from dailybot_cli.commands.env import env from dailybot_cli.commands.form import form from dailybot_cli.commands.hook import hook from dailybot_cli.commands.identity import me, org @@ -99,6 +100,7 @@ def cli(ctx: click.Context, api_url: str | None, app_url: str | None) -> None: cli.add_command(ask) cli.add_command(interactive) cli.add_command(config) +cli.add_command(env) cli.add_command(hook) cli.add_command(version) cli.add_command(upgrade) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index d245709..fb3073b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -13,6 +13,7 @@ The Dailybot CLI persists state in `~/.config/dailybot/` by default. The path ca | `ledger/.json` | `dailybot hook ...` / `dailybot agent update` | Per-repo report ledger: `{ repo, first_seen_at, last_report_at, last_reported_commit, last_nudge_at, last_activity_at, work_pending, snoozed_until, turns_since_report, reported_by }` | `0o600` (dir `0o700`) | | `ledger/_global.json` | `dailybot hook session-start` | Cross-repo hook state: `{ last_login_nudge_at }` | `0o600` | | `/.dailybot/profile.json` | hand-authored, committed to git | `{ name?, profile?, default_metadata?, vars?, report? }` | (no chmod — must be readable by team) | +| `/.dailybot/env.json` | `dailybot env` / hand-authored, **gitignored** | `{ disabled?, active?, profiles: [{ name, api_key, api_url?, app_url? }, ...] }` | `0o600` | ### Schema notes @@ -38,6 +39,144 @@ The Dailybot CLI persists state in `~/.config/dailybot/` by default. The path ca **Security rule:** a `key` field is rejected with a hard error — credentials must never be committed. The file is plain text and lives in the repo, so it must remain free of secrets. Unknown future keys log a one-line warning and are ignored (forward compatibility). Malformed JSON falls back to the global config with a warning. +## Repo-level env override (`.dailybot/env.json`) + +`.dailybot/env.json` is an **opt-in, gitignored** file that carries API keys and optional URL overrides for one or more environments (production, local dev orgs, staging). It sits **beside** `profile.json` in `.dailybot/` and serves a different purpose: `profile.json` pins the *identity* of an agent (committed, shared), `env.json` pins the *auth context* (per-machine, per-developer, never committed). + +Introduced in CLI `>= 3.7.0`. + +### Why it exists + +Before `env.json`, switching between local dev orgs / staging / production required exporting `DAILYBOT_API_URL` / `DAILYBOT_APP_URL` / `DAILYBOT_API_KEY` for every shell, or reconfiguring the global `agents.json`. This got painful when a developer wanted: + +- Repo A "logged into" org X +- Repo B "logged into" org Y + +Both simultaneously, from any shell, with zero env-var management. `env.json` gives each repo its own credential context that follows the working directory. + +### Schema + +```json +{ + "disabled": false, + "active": "local org 1", + "profiles": [ + { + "name": "live", + "api_key": "sk_live_xxxxxxxxxxxx" + }, + { + "name": "local org 1", + "api_key": "sk_local_xxxxxxxxxxxx", + "api_url": "http://localhost:8000", + "app_url": "http://localhost:8090" + }, + { + "name": "staging", + "api_key": "sk_staging_xxxxxxxxxxxx", + "api_url": "https://staging-api.example.com", + "app_url": "https://staging-app.example.com" + } + ] +} +``` + +| Field | Type | Required | Purpose | +|---|---|---|---| +| `disabled` | boolean | optional (default `false`) | **Kill-switch.** When `true`, the entire file is ignored even if `active` points to a valid profile. Use to temporarily disable the override while preserving the selection (`dailybot env off` / `dailybot env on`). | +| `active` | string \| null | optional | Name of the profile to use. `null`, empty string, missing, or pointing at an unknown name all render the file *inert* (resolution continues to lower layers). Only one active at a time — impossible to be ambiguous. | +| `profiles` | list of objects | **required** | Every configured environment. Each entry needs `name` and `api_key`; `api_url` and `app_url` are optional. | +| `profiles[].name` | string | **required** | Unique per file. Human-friendly (spaces allowed). | +| `profiles[].api_key` | string | **required** | The API key for this environment. Stored in plain text — gitignore is mandatory. | +| `profiles[].api_url` | string | optional | Overrides `DAILYBOT_API_URL` / `credentials.json` when this profile is active. Trailing slashes normalized. Falls through to `DEFAULT_API_URL` when absent. | +| `profiles[].app_url` | string | optional | Overrides `DAILYBOT_APP_URL` when this profile is active. Falls through to `DEFAULT_APP_URL` when absent. | + +### CLI commands + +```bash +dailybot env add \ + --name "local org 1" \ + --key sk_xxx \ + --api-url http://localhost:8000 \ + --app-url http://localhost:8090 # Creates env.json + auto-active if first +dailybot env add --name live --key sk_live_yyy # Appends without changing active +dailybot env use "local org 1" # Switch active +dailybot env use "" # Clear active (fall through to global) +dailybot env show # Show resolved profile (key masked) +dailybot env list # All profiles in the file +dailybot env remove "local org 1" # Remove a profile (confirms first, --yes to skip) +dailybot env off # Disable the file (preserves active) +dailybot env on # Re-enable +``` + +### Security guarantees + +1. **Gitignored by convention.** The repo's root `.gitignore` should carry `.dailybot/*` with an explicit exception only for `!.dailybot/profile.json`. `env.json` is NEVER excepted. See the [example .gitignore for this repo](../.gitignore). +2. **`0o600` permissions.** Every write via `dailybot env` — and every load — enforces owner-only permissions defensively (in case an editor created the file with a lax umask). +3. **Fatal refuse-if-tracked guard.** On every load, the CLI runs `git ls-files --error-unmatch .dailybot/env.json` and raises `RepoEnvError` if the file is tracked. Any `dailybot env` subcommand (and any subsequent command that would consume env.json auth) exits non-zero with an actionable message: + ``` + .dailybot/env.json is tracked by git. This file contains API keys and + must never be committed. Fix with: + git rm --cached .dailybot/env.json + # ensure your .gitignore ignores .dailybot/env.json + git commit -m 'chore: untrack .dailybot/env.json' + The CLI refuses to load env.json while it is tracked. + ``` +4. **Masked in all output.** `dailybot env show` and `dailybot env list` mask API keys as `abcd****` (first 4 chars + `****`), matching the pattern used by `dailybot config key`. + +### Auth-resolution precedence (updated) + +The full order for **`api_key`**: + +1. `.dailybot/env.json` active profile's `api_key` (walk-up from cwd) — new in 3.7.0 +2. `DAILYBOT_API_KEY` env var +3. `config.json::api_key` (from `dailybot config key=...`) + +The full order for **`api_url`**: + +1. `--api-url` CLI flag +2. `.dailybot/env.json` active profile's `api_url` — new in 3.7.0 +3. `DAILYBOT_API_URL` env var +4. `credentials.json::api_url` (from the login session) +5. `DEFAULT_API_URL` (`https://api.dailybot.com`) + +The full order for **`app_url`**: + +1. `--app-url` CLI flag +2. `.dailybot/env.json` active profile's `app_url` — new in 3.7.0 +3. `DAILYBOT_APP_URL` env var +4. `DEFAULT_APP_URL` (`https://app.dailybot.com`) + +When `env.json::disabled` is `true` or `active` is empty/null/unknown, the file is transparently skipped and every resolver behaves as if the file didn't exist. + +### Interaction with `profile.json` + +The two files are **orthogonal** and both can be present: + +| File | Committed | Contains | Rules | +|---|---|---|---| +| `.dailybot/profile.json` | Yes (tracked) | `name`, `default_metadata`, `report`, `vars` — **identity** | `key` field fatally rejected. | +| `.dailybot/env.json` | **No** (gitignored) | `api_key`, `api_url`, `app_url` per profile — **auth context** | `agent_name` / `default_metadata` NOT allowed (belongs in `profile.json`). Fatally rejected when tracked. | + +`profile.json` still governs how reports are *signed* even when `env.json` provides the credentials to send them. + +### Interaction with the login Bearer token + +When an `env.json` active profile provides an `api_key`, the CLI's `DailyBotClient` receives that key and — because Bearer is preferred over API key in `_headers()` — the presence of the login Bearer token would normally still win. To make "logged into different orgs in different repos" work correctly, the client should be constructed such that the API key takes precedence for env.json-authored contexts. + +Two ways this is achieved in practice: + +- **Different API URLs.** When the env.json profile carries `api_url` pointing at, say, `http://localhost:8000` while the stored Bearer session was issued against `https://api.dailybot.com`, the URL alone routes correctly and the server-side auth is the only thing that matters (each env has its own key/token). +- **Auto-fallback on 401.** As of CLI `>= 3.5.1`, if Bearer fails with 401, the client automatically retries with the alternative credential (API key). So even if the Bearer wins the initial dispatch and the token isn't valid on the env.json's API, the API key retry succeeds. + +For a bulletproof "different org per repo" story, prefer profiles with distinct `api_url`s or use `dailybot logout` on machines where the mix would be ambiguous. + +### When NOT to use `env.json` + +- **CI environments** — prefer `DAILYBOT_API_KEY` as an env var; leaves no on-disk secret to clean up between jobs. +- **Single-org development** — `dailybot login` + a global `agents.json` profile is simpler. +- **Team-shared identity** — that's `profile.json`'s job. `env.json` is per-developer, per-machine. + ## Environment Variables | Variable | Read by | Effect | @@ -80,12 +219,13 @@ The full resolution (see `_resolve_agent_context` in `commands/agent.py`): 2. `.dailybot/profile.json::profile` (closest ancestor along `$PWD` → `/`) 3. Default profile from `agents.json` -**Credentials** (resolved against the selected profile, then via legacy fallback): +**Credentials** (highest layer wins; env.json is the newest layer, sits above everything except a `--profile` flag): -1. `::api_key` from `agents.json` -2. `DAILYBOT_API_KEY` env var -3. `config.json::api_key` (set via `dailybot config key=...`) -4. Login session Bearer token (`credentials.json::token`) +1. `.dailybot/env.json` active profile's `api_key` (walk-up from cwd) — new in 3.7.0 +2. `::api_key` from `agents.json` (when profile slug is set) +3. `DAILYBOT_API_KEY` env var +4. `config.json::api_key` (set via `dailybot config key=...`) +5. Login session Bearer token (`credentials.json::token`) A profile that has no `api_key` but a login session is allowed — it just uses the Bearer token. A profile that has neither is an error. If `.dailybot/profile.json::profile` points at a slug that does not exist in `agents.json`, the CLI warns once and falls through to session credentials (this is **not** a hard error so the repo file can roll out safely before every developer has configured the matching local profile). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 7a994d5..0e51ab7 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -29,12 +29,23 @@ Files with secrets: - `credentials.json` (login Bearer token) - `config.json` (stored API key) - `agents.json` (per-profile API keys) +- `/.dailybot/env.json` (per-repo API keys — see § below) Files without secrets (still written `0o600` for consistency): - `org_cache.json` (transient list of org names + UUIDs from step 1 of multi-org login) - `plan_cache.json` (non-sensitive org plan tier, keyed by org UUID; used to short-circuit non-allowlisted commands on a free plan — never stores tokens or keys) +### Repo-level env override (`.dailybot/env.json`) + +The `env.json` file is the ONLY sanctioned place inside `.dailybot/` where API keys may live. It carries per-repo credential context (API key + optional URLs for one or more environments). Because it sits inside the repo tree, three additional protections apply beyond the standard `0o600`: + +1. **Gitignore is mandatory.** The broad `.dailybot/*` rule in the repo's `.gitignore` covers it automatically; the only excepted file is `!.dailybot/profile.json`. `env.json` MUST NEVER be excepted. +2. **Load-time refuse-if-tracked guard.** On every load, the CLI runs `git ls-files --error-unmatch .dailybot/env.json` and raises `RepoEnvError` if the file is tracked (fatal — the CLI exits non-zero with an actionable message and refuses to use env.json until fixed). The check runs even outside `env`-group commands because `get_api_key()`, `get_api_url()`, and `get_app_url()` all consult env.json on every construction of `DailyBotClient`. Implementation: `dailybot_cli/config.py::_is_env_tracked_by_git` (independently mockable). +3. **Write-time gitignore warning.** `dailybot env add` runs `git check-ignore --quiet .dailybot/env.json` after writing; if the file is NOT covered by any ignore rule, a warning fires on stderr with the exact `.gitignore` snippet to add. The warning is non-fatal because a fresh repo might not have a `.gitignore` yet, and the load-time guard catches the actual security violation. + +The full schema, precedence, and CLI commands for `env.json` are in [CONFIGURATION.md § "Repo-level env override"](CONFIGURATION.md#repo-level-env-override-dailybotenvjson). + ## Secrets in Output Never display, log, or echo a full secret. Always mask: diff --git a/tests/env_commands_test.py b/tests/env_commands_test.py new file mode 100644 index 0000000..2cecdab --- /dev/null +++ b/tests/env_commands_test.py @@ -0,0 +1,357 @@ +"""Tests for the `dailybot env` CLI command group.""" + +import json +import stat +import subprocess +from pathlib import Path +from typing import Any + +import pytest +from click.testing import CliRunner + + +@pytest.fixture(autouse=True) +def _reset_env_warnings() -> None: + from dailybot_cli.config import reset_repo_env_warnings + + reset_repo_env_warnings() + + +@pytest.fixture(autouse=True) +def _isolate_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: + for var in ( + "DAILYBOT_API_KEY", + "DAILYBOT_CLI_TOKEN", + "DAILYBOT_API_URL", + "DAILYBOT_APP_URL", + "DAILYBOT_CONFIG_DIR", + ): + monkeypatch.delenv(var, raising=False) + + +@pytest.fixture +def chdir_tmp(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + monkeypatch.chdir(tmp_path) + # Init a git repo so save_repo_env anchors at tmp_path, not somewhere else. + subprocess.run(["git", "init", "-q"], cwd=tmp_path, check=True) + return tmp_path + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +# --- Top-level wiring ------------------------------------------------------- + + +def test_env_group_registered(runner: CliRunner) -> None: + """The `env` group must be discoverable from the root CLI.""" + from dailybot_cli.main import cli + + result = runner.invoke(cli, ["env", "--help"]) + assert result.exit_code == 0 + assert "env" in result.output.lower() + for sub in ("list", "use", "show", "add", "remove", "off", "on"): + assert sub in result.output + + +# --- env add ---------------------------------------------------------------- + + +class TestEnvAdd: + def test_creates_file_and_sets_active(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + result = runner.invoke( + cli, + [ + "env", + "add", + "--name", + "local", + "--key", + "sk_local_xxx", + "--api-url", + "http://localhost:8000", + "--app-url", + "http://localhost:8090", + ], + ) + assert result.exit_code == 0, result.output + env_path: Path = chdir_tmp / ".dailybot" / "env.json" + assert env_path.exists() + assert stat.S_IMODE(env_path.stat().st_mode) == 0o600 + payload: dict[str, Any] = json.loads(env_path.read_text()) + assert payload["active"] == "local" + assert payload["profiles"][0]["api_key"] == "sk_local_xxx" + assert payload["profiles"][0]["api_url"] == "http://localhost:8000" + # Success feedback mentions the profile name and that it became active. + assert "local" in result.output + + def test_append_does_not_change_active(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "sk_live_xxx"]) + result = runner.invoke( + cli, + [ + "env", + "add", + "--name", + "staging", + "--key", + "sk_staging_xxx", + "--api-url", + "https://staging-api.example.com", + ], + ) + assert result.exit_code == 0, result.output + payload: dict[str, Any] = json.loads((chdir_tmp / ".dailybot" / "env.json").read_text()) + assert payload["active"] == "live" + assert [p["name"] for p in payload["profiles"]] == ["live", "staging"] + + def test_duplicate_name_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k1"]) + result = runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k2"]) + assert result.exit_code == 1 + assert "already exists" in (result.output + (result.stderr or "")).lower() + + def test_missing_required_flags_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + # Without --name / --key we expect Click to complain about missing options. + result = runner.invoke(cli, ["env", "add"]) + assert result.exit_code != 0 + + def test_warns_when_gitignore_does_not_cover_env_json( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """If .gitignore doesn't cover env.json, we warn (write still succeeds).""" + from dailybot_cli.main import cli + + # No .gitignore in the repo — env.json would be trackable. + result = runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + assert result.exit_code == 0, result.output + combined: str = result.output + (result.stderr or "") + assert "gitignore" in combined.lower() + + +# --- env use ---------------------------------------------------------------- + + +class TestEnvUse: + def test_switches_active(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k1"]) + runner.invoke(cli, ["env", "add", "--name", "local", "--key", "k2"]) + result = runner.invoke(cli, ["env", "use", "local"]) + assert result.exit_code == 0, result.output + payload: dict[str, Any] = json.loads((chdir_tmp / ".dailybot" / "env.json").read_text()) + assert payload["active"] == "local" + + def test_clear_active_with_empty_string(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k1"]) + result = runner.invoke(cli, ["env", "use", ""]) + assert result.exit_code == 0, result.output + payload: dict[str, Any] = json.loads((chdir_tmp / ".dailybot" / "env.json").read_text()) + assert payload["active"] is None + + def test_unknown_name_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + result = runner.invoke(cli, ["env", "use", "ghost"]) + assert result.exit_code == 1 + + def test_no_file_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + result = runner.invoke(cli, ["env", "use", "live"]) + assert result.exit_code == 1 + + +# --- env show --------------------------------------------------------------- + + +class TestEnvShow: + def test_shows_active_profile_masked(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke( + cli, + [ + "env", + "add", + "--name", + "local", + "--key", + "sk_supersecret_xxxxxxxxxxxxxxxx", + "--api-url", + "http://localhost:8000", + ], + ) + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0, result.output + # Masked (first 4 chars + ****) + assert "sk_s****" in result.output + assert "sk_supersecret_xxxxxxxxxxxxxxxx" not in result.output + assert "http://localhost:8000" in result.output + assert "local" in result.output + + def test_reports_disabled_state(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + runner.invoke(cli, ["env", "off"]) + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0, result.output + assert "disabled" in result.output.lower() + + def test_reports_no_active(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + runner.invoke(cli, ["env", "use", ""]) + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0, result.output + assert "no active" in result.output.lower() or "inactive" in result.output.lower() + + def test_no_file_message(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0 + assert "no" in result.output.lower() and "env.json" in result.output.lower() + + +# --- env list --------------------------------------------------------------- + + +class TestEnvList: + def test_lists_all_profiles_marks_active(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k1"]) + runner.invoke(cli, ["env", "add", "--name", "local", "--key", "k2"]) + runner.invoke(cli, ["env", "use", "local"]) + result = runner.invoke(cli, ["env", "list"]) + assert result.exit_code == 0, result.output + assert "live" in result.output + assert "local" in result.output + + def test_empty_file_prints_hint(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + result = runner.invoke(cli, ["env", "list"]) + assert result.exit_code == 0 + assert "env add" in result.output + + +# --- env remove ------------------------------------------------------------- + + +class TestEnvRemove: + def test_removes_profile(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k1"]) + runner.invoke(cli, ["env", "add", "--name", "local", "--key", "k2"]) + result = runner.invoke(cli, ["env", "remove", "local", "--yes"]) + assert result.exit_code == 0, result.output + payload: dict[str, Any] = json.loads((chdir_tmp / ".dailybot" / "env.json").read_text()) + assert [p["name"] for p in payload["profiles"]] == ["live"] + + def test_removing_active_reports_it(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + result = runner.invoke(cli, ["env", "remove", "live", "--yes"]) + assert result.exit_code == 0, result.output + assert "active" in result.output.lower() + + def test_unknown_profile_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + result = runner.invoke(cli, ["env", "remove", "ghost", "--yes"]) + assert result.exit_code == 1 + + +# --- env off / on ----------------------------------------------------------- + + +class TestEnvKillSwitch: + def test_off_disables_env_json(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + assert get_active_env_profile(chdir_tmp) is not None + result = runner.invoke(cli, ["env", "off"]) + assert result.exit_code == 0, result.output + assert get_active_env_profile(chdir_tmp) is None + + def test_on_re_enables(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + runner.invoke(cli, ["env", "off"]) + result = runner.invoke(cli, ["env", "on"]) + assert result.exit_code == 0, result.output + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None + assert active["name"] == "live" + + def test_off_no_file_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: + from dailybot_cli.main import cli + + result = runner.invoke(cli, ["env", "off"]) + assert result.exit_code == 1 + + +# --- Committed guard surfaces at CLI -------------------------------------- + + +class TestCommittedGuardSurfacing: + def test_env_use_bubbles_fatal_when_env_json_tracked( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """Env.json tracked in git → any env subcommand exits non-zero with a + message the developer can act on.""" + from dailybot_cli.main import cli + + # First, write env.json properly. + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=chdir_tmp, + check=True, + ) + # Force-add and commit — simulate a developer mistake. + subprocess.run( + ["git", "add", "-f", ".dailybot/env.json"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run(["git", "commit", "-q", "-m", "leak"], cwd=chdir_tmp, check=True) + # Now any env-touching command should refuse. + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 1 + combined: str = result.output + (result.stderr or "") + assert "tracked" in combined.lower() diff --git a/tests/repo_env_test.py b/tests/repo_env_test.py new file mode 100644 index 0000000..3515228 --- /dev/null +++ b/tests/repo_env_test.py @@ -0,0 +1,909 @@ +"""Tests for repo-level env override (`.dailybot/env.json`). + +Covers: +- Walk-up discovery (mirrors `find_repo_profile_path`). +- Schema validation on load (unknown keys warn, invalid entries skipped). +- `active` resolution (top-level string, empty/null → inert). +- `disabled: true` kill-switch (preserves `active`, ignores the file). +- Committed-to-git guard — fatal (raises `RepoEnvError`). +- File permissions locked to `0o600` on write. +- Precedence in `get_api_key()`, `get_api_url()`, `get_app_url()` — env.json + active profile wins over env vars, config, and credentials, but yields to + explicit CLI flag overrides. +- Mutation helpers (`add_env_profile`, `remove_env_profile`, `set_active_env_profile`, + `set_env_disabled`) — including auto-active on first add and clear-active + on removal. +""" + +import json +import os +import stat +import subprocess +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest + + +@pytest.fixture(autouse=True) +def _reset_env_warnings() -> None: + """Clear the per-process env.json warning dedup between tests.""" + from dailybot_cli.config import reset_repo_env_warnings + + reset_repo_env_warnings() + + +@pytest.fixture(autouse=True) +def _isolate_env_vars(monkeypatch: pytest.MonkeyPatch) -> None: + """Prevent leakage from the real developer environment.""" + for var in ( + "DAILYBOT_API_KEY", + "DAILYBOT_CLI_TOKEN", + "DAILYBOT_API_URL", + "DAILYBOT_APP_URL", + "DAILYBOT_CONFIG_DIR", + ): + monkeypatch.delenv(var, raising=False) + + +@pytest.fixture +def chdir_tmp(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + monkeypatch.chdir(tmp_path) + return tmp_path + + +def _write_env(repo_root: Path, payload: dict[str, Any], *, mode: int | None = None) -> Path: + env_dir: Path = repo_root / ".dailybot" + env_dir.mkdir(parents=True, exist_ok=True) + env_path: Path = env_dir / "env.json" + env_path.write_text(json.dumps(payload)) + if mode is not None: + os.chmod(env_path, mode) + return env_path + + +# --- find_repo_env_path ----------------------------------------------------- + + +class TestFindRepoEnvPath: + def test_walk_up_from_nested_cwd(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import find_repo_env_path + + _write_env(chdir_tmp, {"profiles": []}) + nested: Path = chdir_tmp / "src" / "deep" + nested.mkdir(parents=True) + found: Path | None = find_repo_env_path(nested) + assert found is not None + assert found.parent.parent == chdir_tmp + + def test_closest_ancestor_wins(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import find_repo_env_path + + _write_env(chdir_tmp, {"profiles": []}) + inner: Path = chdir_tmp / "inner" + inner.mkdir() + _write_env(inner, {"profiles": [{"name": "x", "api_key": "k"}]}) + assert find_repo_env_path(inner / "src") is None or ( + find_repo_env_path(inner) is not None + and find_repo_env_path(inner).parent.parent == inner # type: ignore[union-attr] + ) + + def test_returns_none_when_missing(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import find_repo_env_path + + assert find_repo_env_path(chdir_tmp) is None + + def test_ignores_dailybot_as_regular_file(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import find_repo_env_path + + (chdir_tmp / ".dailybot").write_text("not a directory") + assert find_repo_env_path(chdir_tmp) is None + + +# --- load_repo_env ---------------------------------------------------------- + + +class TestLoadRepoEnv: + def test_loads_valid_payload(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + path: Path = _write_env( + chdir_tmp, + { + "active": "live", + "profiles": [ + {"name": "live", "api_key": "live-key"}, + { + "name": "local", + "api_key": "local-key", + "api_url": "http://localhost:8000", + "app_url": "http://localhost:8090", + }, + ], + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["active"] == "live" + assert result["disabled"] is False + assert len(result["profiles"]) == 2 + assert result["profiles"][0] == {"name": "live", "api_key": "live-key"} + assert result["profiles"][1]["api_url"] == "http://localhost:8000" + assert result["_path"] == str(path) + + def test_active_missing_returns_none_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["active"] is None + + def test_active_empty_string_returns_none_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + {"active": "", "profiles": [{"name": "x", "api_key": "k"}]}, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["active"] is None + + def test_active_null_returns_none_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + {"active": None, "profiles": [{"name": "x", "api_key": "k"}]}, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["active"] is None + + def test_disabled_true_short_circuits(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + { + "disabled": True, + "active": "live", + "profiles": [{"name": "live", "api_key": "k"}], + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["disabled"] is True + # Profiles are still parsed so `env list` can render them, but the + # convenience getter surfaces the flag so callers know to bail. + + def test_malformed_json_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + env_dir: Path = chdir_tmp / ".dailybot" + env_dir.mkdir() + (env_dir / "env.json").write_text("{not valid json") + assert load_repo_env(chdir_tmp) is None + + def test_non_object_json_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + env_dir: Path = chdir_tmp / ".dailybot" + env_dir.mkdir() + (env_dir / "env.json").write_text('["not", "an", "object"]') + assert load_repo_env(chdir_tmp) is None + + def test_profiles_not_a_list_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env(chdir_tmp, {"profiles": "nope"}) # type: ignore[dict-item] + assert load_repo_env(chdir_tmp) is None + + def test_entry_missing_required_key_is_skipped(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + { + "profiles": [ + {"name": "good", "api_key": "k"}, + {"name": "missing-key"}, + {"api_key": "orphan-key"}, + ] + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert [p["name"] for p in result["profiles"]] == ["good"] + + def test_unknown_top_level_keys_are_dropped(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + { + "profiles": [{"name": "x", "api_key": "k"}], + "future_field": "ignored", + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert "future_field" not in result + + def test_unknown_profile_keys_are_dropped(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + { + "profiles": [ + { + "name": "x", + "api_key": "k", + "webAppUrl": "http://x", + "note": "future", + } + ] + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["profiles"][0] == {"name": "x", "api_key": "k"} + + def test_defensive_chmod_when_file_lax(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + path: Path = _write_env( + chdir_tmp, + {"profiles": [{"name": "x", "api_key": "k"}]}, + mode=0o644, + ) + load_repo_env(chdir_tmp) + mode: int = stat.S_IMODE(path.stat().st_mode) + assert mode == 0o600 + + +# --- Committed-to-git guard ------------------------------------------------- + + +class TestCommittedGuard: + def test_untracked_in_git_repo_is_allowed(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + # Init a git repo and make sure the file is gitignored. + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + (chdir_tmp / ".gitignore").write_text(".dailybot/*\n") + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + + def test_tracked_env_json_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, load_repo_env + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=chdir_tmp, + check=True, + ) + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "leaked"}]}) + # Intentionally track it — simulates a developer mistake. + subprocess.run( + ["git", "add", "-f", ".dailybot/env.json"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run( + ["git", "commit", "-q", "-m", "leak"], + cwd=chdir_tmp, + check=True, + ) + with pytest.raises(RepoEnvError) as exc_info: + load_repo_env(chdir_tmp) + message: str = str(exc_info.value) + assert "tracked" in message.lower() + assert "credentials" in message.lower() or "api key" in message.lower() + + def test_no_git_at_all_is_allowed(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + # No `git init` at all — the guard should skip cleanly. + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + + +# --- get_active_env_profile ------------------------------------------------- + + +class TestGetActiveEnvProfile: + def test_returns_active_profile(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + {"name": "live", "api_key": "live-k"}, + { + "name": "local", + "api_key": "local-k", + "api_url": "http://localhost:8000", + }, + ], + }, + ) + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None + assert active["name"] == "local" + assert active["api_key"] == "local-k" + assert active["api_url"] == "http://localhost:8000" + + def test_missing_active_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + assert get_active_env_profile(chdir_tmp) is None + + def test_active_unknown_name_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + + _write_env( + chdir_tmp, + { + "active": "ghost", + "profiles": [{"name": "live", "api_key": "k"}], + }, + ) + assert get_active_env_profile(chdir_tmp) is None + + def test_disabled_true_returns_none_even_with_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + + _write_env( + chdir_tmp, + { + "disabled": True, + "active": "live", + "profiles": [{"name": "live", "api_key": "k"}], + }, + ) + assert get_active_env_profile(chdir_tmp) is None + + def test_no_file_returns_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_active_env_profile + + assert get_active_env_profile(chdir_tmp) is None + + +# --- save_repo_env ---------------------------------------------------------- + + +class TestSaveRepoEnv: + def test_writes_at_repo_root_with_0600(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import save_repo_env + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + nested: Path = chdir_tmp / "src" / "deep" + nested.mkdir(parents=True) + path: Path = save_repo_env( + {"active": "x", "profiles": [{"name": "x", "api_key": "k"}]}, + cwd=nested, + ) + # File anchors at git repo root, not the nested cwd. + assert path == chdir_tmp / ".dailybot" / "env.json" + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + + def test_validation_rejects_missing_required(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, save_repo_env + + with pytest.raises(RepoEnvError): + save_repo_env( + {"profiles": [{"name": "x"}]}, # missing api_key + cwd=chdir_tmp, + ) + + def test_validation_rejects_duplicate_names(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, save_repo_env + + with pytest.raises(RepoEnvError): + save_repo_env( + { + "profiles": [ + {"name": "x", "api_key": "k1"}, + {"name": "x", "api_key": "k2"}, + ], + }, + cwd=chdir_tmp, + ) + + def test_validation_rejects_active_pointing_to_ghost(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, save_repo_env + + with pytest.raises(RepoEnvError): + save_repo_env( + { + "active": "ghost", + "profiles": [{"name": "x", "api_key": "k"}], + }, + cwd=chdir_tmp, + ) + + def test_validation_rejects_unknown_top_level_key(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, save_repo_env + + with pytest.raises(RepoEnvError): + save_repo_env( + { + "profiles": [{"name": "x", "api_key": "k"}], + "weird": True, + }, + cwd=chdir_tmp, + ) + + def test_validation_rejects_unknown_profile_key(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, save_repo_env + + with pytest.raises(RepoEnvError): + save_repo_env( + { + "profiles": [ + {"name": "x", "api_key": "k", "extra": "no"}, + ], + }, + cwd=chdir_tmp, + ) + + +# --- Mutation helpers ------------------------------------------------------- + + +class TestAddEnvProfile: + def test_creates_file_when_missing(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import add_env_profile + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + path, became_active = add_env_profile( + name="local", + api_key="k", + api_url="http://localhost:8000", + cwd=chdir_tmp, + ) + assert path.exists() + assert became_active is True + payload: dict[str, Any] = json.loads(path.read_text()) + assert payload["active"] == "local" + assert payload["profiles"][0]["api_url"] == "http://localhost:8000" + + def test_appends_without_touching_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import add_env_profile + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="live-k", cwd=chdir_tmp) + path, became_active = add_env_profile( + name="local", + api_key="local-k", + cwd=chdir_tmp, + ) + assert became_active is False + payload: dict[str, Any] = json.loads(path.read_text()) + assert payload["active"] == "live" + assert [p["name"] for p in payload["profiles"]] == ["live", "local"] + + def test_duplicate_name_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, add_env_profile + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + with pytest.raises(RepoEnvError): + add_env_profile(name="live", api_key="other", cwd=chdir_tmp) + + def test_normalizes_trailing_slash_in_urls(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import add_env_profile + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + path, _ = add_env_profile( + name="local", + api_key="k", + api_url="http://localhost:8000/", + app_url="http://localhost:8090/", + cwd=chdir_tmp, + ) + payload: dict[str, Any] = json.loads(path.read_text()) + assert payload["profiles"][0]["api_url"] == "http://localhost:8000" + assert payload["profiles"][0]["app_url"] == "http://localhost:8090" + + +class TestSetActiveEnvProfile: + def test_switches_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + add_env_profile, + get_active_env_profile, + set_active_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="live-k", cwd=chdir_tmp) + add_env_profile(name="local", api_key="local-k", cwd=chdir_tmp) + set_active_env_profile("local", cwd=chdir_tmp) + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None + assert active["name"] == "local" + + def test_clear_active_with_none(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + add_env_profile, + get_active_env_profile, + set_active_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + set_active_env_profile(None, cwd=chdir_tmp) + assert get_active_env_profile(chdir_tmp) is None + + def test_unknown_name_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + RepoEnvError, + add_env_profile, + set_active_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + with pytest.raises(RepoEnvError): + set_active_env_profile("ghost", cwd=chdir_tmp) + + def test_no_file_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, set_active_env_profile + + with pytest.raises(RepoEnvError): + set_active_env_profile("live", cwd=chdir_tmp) + + +class TestRemoveEnvProfile: + def test_removes_and_preserves_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + add_env_profile, + get_active_env_profile, + remove_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="live-k", cwd=chdir_tmp) + add_env_profile(name="local", api_key="local-k", cwd=chdir_tmp) + _, cleared = remove_env_profile("local", cwd=chdir_tmp) + assert cleared is False + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None + assert active["name"] == "live" + + def test_removing_active_clears_active(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + add_env_profile, + get_active_env_profile, + remove_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + _, cleared = remove_env_profile("live", cwd=chdir_tmp) + assert cleared is True + assert get_active_env_profile(chdir_tmp) is None + + def test_unknown_name_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + RepoEnvError, + add_env_profile, + remove_env_profile, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + with pytest.raises(RepoEnvError): + remove_env_profile("ghost", cwd=chdir_tmp) + + def test_no_file_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, remove_env_profile + + with pytest.raises(RepoEnvError): + remove_env_profile("x", cwd=chdir_tmp) + + +class TestSetEnvDisabled: + def test_off_then_on(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import ( + add_env_profile, + get_active_env_profile, + load_repo_env, + set_env_disabled, + ) + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + add_env_profile(name="live", api_key="k", cwd=chdir_tmp) + set_env_disabled(True, cwd=chdir_tmp) + assert get_active_env_profile(chdir_tmp) is None + loaded: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert loaded is not None + assert loaded["disabled"] is True + # Turning it back on restores the active profile. + set_env_disabled(False, cwd=chdir_tmp) + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None + assert active["name"] == "live" + + def test_no_file_raises(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import RepoEnvError, set_env_disabled + + with pytest.raises(RepoEnvError): + set_env_disabled(True, cwd=chdir_tmp) + + +# --- Precedence in get_api_key / get_api_url / get_app_url ------------------ + + +class TestPrecedence: + def test_env_json_beats_env_var_for_api_key( + self, chdir_tmp: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from dailybot_cli.config import get_api_key + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [{"name": "local", "api_key": "env-json-key"}], + }, + ) + monkeypatch.setenv("DAILYBOT_API_KEY", "env-var-key") + assert get_api_key() == "env-json-key" + + def test_env_var_wins_when_env_json_inert( + self, chdir_tmp: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from dailybot_cli.config import get_api_key + + _write_env(chdir_tmp, {"profiles": [{"name": "local", "api_key": "j"}]}) + monkeypatch.setenv("DAILYBOT_API_KEY", "env-var-key") + assert get_api_key() == "env-var-key" + + def test_env_var_wins_when_env_json_disabled( + self, chdir_tmp: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from dailybot_cli.config import get_api_key + + _write_env( + chdir_tmp, + { + "disabled": True, + "active": "local", + "profiles": [{"name": "local", "api_key": "env-json-key"}], + }, + ) + monkeypatch.setenv("DAILYBOT_API_KEY", "env-var-key") + assert get_api_key() == "env-var-key" + + def test_env_json_provides_api_url(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_api_url + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "k", + "api_url": "http://localhost:8000", + } + ], + }, + ) + assert get_api_url() == "http://localhost:8000" + + def test_cli_flag_beats_env_json_for_api_url(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_api_url, set_api_url_override + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "k", + "api_url": "http://localhost:8000", + } + ], + }, + ) + set_api_url_override("https://explicit.example.com") + try: + assert get_api_url() == "https://explicit.example.com" + finally: + set_api_url_override("") # reset; empty string clears the override + + def test_env_json_without_api_url_falls_through_to_default(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import DEFAULT_API_URL, get_api_url + + _write_env( + chdir_tmp, + { + "active": "live", + "profiles": [{"name": "live", "api_key": "k"}], + }, + ) + assert get_api_url() == DEFAULT_API_URL + + def test_env_json_provides_app_url(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import get_app_url + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "k", + "app_url": "http://localhost:8090", + } + ], + }, + ) + assert get_app_url() == "http://localhost:8090" + + +# --- End-to-end: DailyBotClient sees env.json credentials ------------------- + + +class TestClientIntegration: + def test_client_uses_env_json_credentials(self, chdir_tmp: Path) -> None: + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "env-json-key", + "api_url": "http://localhost:8000", + } + ], + }, + ) + # Import late so the env.json is picked up on construction. + from dailybot_cli.api_client import DailyBotClient + + client: DailyBotClient = DailyBotClient() + assert client.api_key == "env-json-key" + assert client.api_url == "http://localhost:8000" + + def test_client_falls_back_when_env_json_inert( + self, chdir_tmp: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + _write_env( + chdir_tmp, + {"profiles": [{"name": "local", "api_key": "j"}]}, + ) + monkeypatch.setenv("DAILYBOT_API_KEY", "env-var-key") + from dailybot_cli.api_client import DailyBotClient + + client: DailyBotClient = DailyBotClient() + assert client.api_key == "env-var-key" + + +# --- Committed-guard integration with load_repo_env ------------------------- + + +class TestFatalGuardBubbles: + def test_load_raises_and_caller_sees_it(self, chdir_tmp: Path) -> None: + """If env.json is tracked, get_active_env_profile must propagate the fatal.""" + from dailybot_cli.config import RepoEnvError, get_active_env_profile + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run( + ["git", "config", "user.name", "T"], + cwd=chdir_tmp, + check=True, + ) + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + subprocess.run( + ["git", "add", "-f", ".dailybot/env.json"], + cwd=chdir_tmp, + check=True, + ) + subprocess.run( + ["git", "commit", "-q", "-m", "leak"], + cwd=chdir_tmp, + check=True, + ) + with pytest.raises(RepoEnvError): + get_active_env_profile(chdir_tmp) + + +# --- Guard-check mocking ---------------------------------------------------- + + +class TestResolverProvenance: + """`resolve_active_profile` surfaces env.json fields so `agent profiles + --resolve` can render the full auth picture.""" + + def test_env_json_appears_in_resolution(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import resolve_active_profile + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "env-json-key", + "api_url": "http://localhost:8000", + "app_url": "http://localhost:8090", + } + ], + }, + ) + resolved: dict[str, Any] = resolve_active_profile(None, None, cwd=chdir_tmp) + assert resolved["env_profile_name"] == "local" + assert resolved["env_profile_api_url"] == "http://localhost:8000" + assert resolved["env_profile_app_url"] == "http://localhost:8090" + assert resolved["api_key"] == "env-json-key" + assert resolved["resolved_from"]["api_key"] == "env.json" + + def test_env_json_beats_global_profile_key(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import resolve_active_profile + + _write_env( + chdir_tmp, + { + "active": "local", + "profiles": [{"name": "local", "api_key": "env-json-key"}], + }, + ) + # A global profile shouldn't win over env.json. + with patch( + "dailybot_cli.config.get_default_profile", + return_value={ + "profile": "default", + "agent_name": "Bot", + "api_key": "global-key", + }, + ): + resolved: dict[str, Any] = resolve_active_profile(None, None, cwd=chdir_tmp) + assert resolved["api_key"] == "env-json-key" + assert resolved["resolved_from"]["api_key"] == "env.json" + + def test_no_env_json_leaves_provenance_untouched(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import resolve_active_profile + + resolved: dict[str, Any] = resolve_active_profile(None, None, cwd=chdir_tmp) + assert resolved["env_profile_name"] is None + assert resolved["env_profile_error"] is None + + +class TestGuardIsFactoredOut: + """The git-tracked check must be independently mockable so downstream tests + can bypass it without needing a real git repo.""" + + def test_can_patch_the_guard(self, chdir_tmp: Path) -> None: + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + with patch("dailybot_cli.config._is_env_tracked_by_git", return_value=False): + from dailybot_cli.config import load_repo_env + + assert load_repo_env(chdir_tmp) is not None From 3e544b0dbf7ab79f5a385d73479e500ecd33f47a Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 00:35:21 +0000 Subject: [PATCH 2/5] =?UTF-8?q?fix(client):=20silent=20Bearer=E2=86=92API-?= =?UTF-8?q?key=20retry=20on=20401/403=20for=20env.json=20UX?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Makes `.dailybot/env.json` "just work" alongside a stale global Bearer session — the CLI now auto-retries every user-scoped call once with the alternative credential when the server rejects the primary with 401 OR 403, so `dailybot status --auth`, `user list`, `form list`, etc. all transparently succeed even when the on-disk OTP token was issued against a different API URL than the env.json profile points at. ## Change Log - api_client: extended `_agent_request` retry to 401 OR 403 (was 401 only); Django/DRF answers 403 for rejected credentials just as often as 401, so 401-only left the local-Django + env.json case broken. - api_client: added sibling `_request()` helper that mirrors `_agent_request` for every user-scoped endpoint that authenticates via `_headers()` (auth_status, checkin, form, kudos, chat, ask, user, team, ...). Same retry semantics on 401/403. - api_client: migrated 34 authenticated `_headers()` call sites to the new `_request()` helper (login/logout/register endpoints keep raw `_headers()` — retrying there is semantically wrong). - api_client: added `_dispatch_http` that routes to per-method `httpx.get/post/patch/put` / `httpx.request` (DELETE), preserving the per-method patchable surface used by the test suite so the migration is invisible to existing tests. - api_client: paginated GET helper also benefits from the auth retry now (list endpoints inherit the fallback for free). - api_client: promoted `120.0` to `LONG_TIMEOUT_SECS`; extracted `_AUTH_RETRY_STATUS_CODES` constant with a comment explaining why 403 is retried alongside 401 (no more inline magic numbers). - status: `dailybot status --auth` inspects the client's `_agent_auth_mode` after the call and reports the credential that actually succeeded on the wire ("Authenticated via API key" vs "Authenticated via login (OTP)") so the UX is honest about the effective auth path. Added a distinctive "Both credentials were rejected" message when neither works. - docs: rewrote the "Interaction with the login Bearer token" section of CONFIGURATION.md with a step-by-step example and a rationale for retrying on 403 (referencing DRF's behavior). - tests: added `TestUserAuthFallback` (6 cases: 401 retry, 403 retry, no-alt-credential, 2xx happy path, non-auth errors don't retry, login endpoints never retry). - tests: added 403-retry case to `TestAgentAuthFallback`; broadened the non-auth exclusion test to sweep 400/404/422/500/502/503. - tests: adapted the 4 `status_auth_*` tests to the new single-`auth_status`-call architecture and added a `both_credentials_rejected` case. ## Risks - Behavioral change for callers that relied on getting a 403 back without a retry attempt on the wire. In practice this is invisible: the retry is single-shot, uses the alt credential if available, and either succeeds (UX improvement) or fails the same way it would have before. Login-lifecycle endpoints are excluded on purpose. - Full pytest suite (1002 tests) + ruff + mypy all green. - Smoke-tested end-to-end against a live local API: `dailybot status --auth` and `dailybot user list` both succeed transparently with a prod Bearer on disk and a local env.json. Co-authored-by: Cursor --- dailybot_cli/api_client.py | 316 ++++++++++++++++++-------------- dailybot_cli/commands/status.py | 70 +++---- docs/CONFIGURATION.md | 31 +++- tests/api_client_test.py | 206 +++++++++++++++++++-- tests/commands_test.py | 62 ++++++- 5 files changed, 485 insertions(+), 200 deletions(-) diff --git a/dailybot_cli/api_client.py b/dailybot_cli/api_client.py index 52eab7f..f181263 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -10,6 +10,15 @@ from dailybot_cli.config import get_api_key, get_api_url, get_token _MAX_LIST_PAGES: int = 50 # safety cap for paginated list endpoints +LONG_TIMEOUT_SECS: float = 120.0 # AI-processing endpoints (ask, submit_update) + +# HTTP status codes that trigger the alt-credential auth retry. 401 is the +# standards-compliant "credentials rejected" answer; 403 is what many +# Django/DRF backends actually send for the same condition (including for +# "credentials not provided" when the primary credential was silently +# stripped or malformed). Retrying on both makes env.json + a stale +# session work seamlessly regardless of which convention the server uses. +_AUTH_RETRY_STATUS_CODES: frozenset[int] = frozenset({401, 403}) DEFAULT_PAGE_SIZE: int = 25 # server default page size for paginated list endpoints MAX_PAGE_SIZE: int = 100 # server clamps above this; the client clamps too @@ -208,9 +217,15 @@ def _agent_request( """Execute an agent-authenticated request with automatic alt-credential retry. Tries with ``_agent_headers()`` (Bearer preferred, API key fallback). - If the server returns 401 and an alternative credential is available, - retries once with it. This covers both directions: expired Bearer - retried with API key, and stale API key retried with Bearer. + If the server returns 401 or 403 and an alternative credential is + available, retries once with it. This covers both directions: expired + Bearer retried with API key, and stale API key retried with Bearer. + + Why 401 **and** 403: many Django/DRF APIs return 403 for "credentials + rejected" or "credentials not provided" rather than the more + standards-compliant 401. Retrying on both makes the behaviour + consistent across backends and lets ``.dailybot/env.json`` work + seamlessly even when a stale prod Bearer session is still on disk. """ kwargs: dict[str, Any] = {"headers": self._agent_headers(), "timeout": self.timeout} if json is not None: @@ -220,7 +235,7 @@ def _agent_request( response: httpx.Response = httpx.request(method, url, **kwargs) - if response.status_code == 401: + if response.status_code in _AUTH_RETRY_STATUS_CODES: alt: dict[str, str] | None = self._alt_auth_headers() if alt is not None: kwargs["headers"] = alt @@ -228,6 +243,74 @@ def _agent_request( return response + def _request( + self, + method: str, + url: str, + *, + json: dict[str, Any] | None = None, + params: dict[str, Any] | None = None, + timeout: float | None = None, + ) -> httpx.Response: + """Execute a user-scoped authenticated request with alt-credential retry. + + Sibling of :meth:`_agent_request` for the endpoints that authenticate + via :meth:`_headers` (user-scoped: auth_status, checkin, form, kudos, + chat, ask, user, team, ...). Same retry semantics — on 401/403 with + an alternative credential present, retries once transparently. + + Do **not** use for login-lifecycle endpoints (``request_code``, + ``verify_code``, ``logout``, ``register_agent``) — those must never + fall back because the credential IS the thing under negotiation + (or, for logout, we're actively invalidating it). + + ``timeout`` defaults to ``self.timeout`` (the standard read timeout); + pass an explicit value for AI/AI-processing endpoints that need + the longer :data:`LONG_TIMEOUT_SECS`. + + The dispatch to ``httpx.get`` / ``httpx.post`` / ``httpx.patch`` / + ``httpx.put`` / ``httpx.request`` (for ``DELETE``) preserves the + long-standing per-method patchable surface used by the test suite; + both invocations (primary + retry) go through the same dispatch so + the retry is transparent to callers and to test mocks alike. + """ + kwargs: dict[str, Any] = { + "headers": self._headers(), + "timeout": self.timeout if timeout is None else timeout, + } + if params is not None: + kwargs["params"] = params + if json is not None: + kwargs["json"] = json + + response: httpx.Response = self._dispatch_http(method, url, **kwargs) + + if response.status_code in _AUTH_RETRY_STATUS_CODES: + alt: dict[str, str] | None = self._alt_auth_headers() + if alt is not None: + kwargs["headers"] = alt + response = self._dispatch_http(method, url, **kwargs) + + return response + + @staticmethod + def _dispatch_http(method: str, url: str, **kwargs: Any) -> httpx.Response: + """Route to the per-method ``httpx`` function so per-method patches + (``patch("httpx.get", ...)``) keep working. ``DELETE`` goes through + the generic ``httpx.request`` because ``httpx.delete`` does not + accept a ``json`` body in all supported versions. + """ + method_upper: str = method.upper() + if method_upper == "GET": + return httpx.get(url, **kwargs) + if method_upper == "POST": + return httpx.post(url, **kwargs) + if method_upper == "PATCH": + return httpx.patch(url, **kwargs) + if method_upper == "PUT": + return httpx.put(url, **kwargs) + return httpx.request(method_upper, url, **kwargs) + def _handle_response(self, response: httpx.Response) -> dict[str, Any]: """Parse API response and raise on errors.""" if response.status_code >= 400: @@ -350,7 +433,7 @@ def _paginated_get( def _do_get( url: str = page_url, prm: dict[str, Any] | None = page_params ) -> httpx.Response: - return httpx.get(url, headers=self._headers(), params=prm, timeout=self.timeout) + return self._request("GET", url, params=prm) response: httpx.Response = self._send_with_retry(_do_get) if response.status_code >= 400: @@ -413,15 +496,18 @@ def verify_code( def auth_status(self) -> dict[str, Any]: """GET /v1/cli/auth/status/""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/cli/auth/status/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/cli/auth/status/" ) return self._handle_response(response) def logout(self) -> dict[str, Any]: - """POST /v1/cli/auth/logout/""" + """POST /v1/cli/auth/logout/ + + Uses ``_headers`` directly (no fallback) because logout is a + Bearer-only lifecycle operation — retrying with an API key would + neither succeed nor be semantically meaningful. + """ response: httpx.Response = httpx.post( f"{self.api_url}/v1/cli/auth/logout/", headers=self._headers(), @@ -448,20 +534,18 @@ def submit_update( payload["doing"] = doing if blocked: payload["blocked"] = blocked - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/cli/updates/", json=payload, - headers=self._headers(), - timeout=120.0, + timeout=LONG_TIMEOUT_SECS, ) return self._handle_response(response) def get_status(self) -> dict[str, Any]: """GET /v1/cli/status/""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/cli/status/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/cli/status/" ) return self._handle_response(response) @@ -490,11 +574,11 @@ def create_chat_completion( if available_commands is not None: payload["available_commands"] = available_commands - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/cli/chat/completions/", json=payload, - headers=self._headers(), - timeout=120.0, + timeout=LONG_TIMEOUT_SECS, ) return self._handle_response(response) @@ -513,11 +597,10 @@ def complete_checkin( payload["last_question_index"] = last_question_index if response_date: payload["response_date"] = response_date - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/checkins/{followup_uuid}/responses/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -561,10 +644,8 @@ def list_checkins( def get_checkin(self, followup_uuid: str) -> dict[str, Any]: """GET /v1/checkins//.""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/checkins/{followup_uuid}/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/checkins/{followup_uuid}/" ) return self._handle_response(response) @@ -576,10 +657,8 @@ def get_checkin_detail(self, followup_uuid: str) -> dict[str, Any]: and the ``is_archived`` flag — the shape aligned with form detail. Use this for authoring/verification rather than the v2 retrieve serializer. """ - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/checkins/{followup_uuid}/detail/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/checkins/{followup_uuid}/detail/" ) return self._handle_response(response) @@ -593,11 +672,10 @@ def get_template( params: dict[str, str] = {} if followup_uuid: params = {"render_special_vars": "true", "followup_id": followup_uuid} - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/templates/{template_uuid}/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) @@ -660,11 +738,10 @@ def update_checkin_response( payload: dict[str, Any] = {"responses": responses} if last_question_index is not None: payload["last_question_index"] = last_question_index - response: httpx.Response = httpx.put( + response: httpx.Response = self._request( + "PUT", f"{self.api_url}/v1/checkins/{followup_uuid}/responses/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -679,12 +756,10 @@ def delete_checkin_response( if response_date: params["date_start"] = response_date params["date_end"] = response_date - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/checkins/{followup_uuid}/responses/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) @@ -721,11 +796,10 @@ def create_checkin( payload["generate_short_question"] = True if config: payload.update(config) - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/checkins/create/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -759,21 +833,18 @@ def update_checkin_config( payload["participants"] = participants if config: payload.update(config) - response: httpx.Response = httpx.patch( + response: httpx.Response = self._request( + "PATCH", f"{self.api_url}/v1/checkins/{followup_uuid}/config/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) def archive_checkin(self, followup_uuid: str) -> dict[str, Any]: """DELETE /v1/checkins//archive/ — soft-delete a check-in (204).""" - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/checkins/{followup_uuid}/archive/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -783,11 +854,10 @@ def add_checkin_question( question: dict[str, Any], ) -> dict[str, Any]: """POST /v1/checkins//questions/ — add a question.""" - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/checkins/{followup_uuid}/questions/", json=question, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -798,11 +868,10 @@ def update_checkin_question( fields: dict[str, Any], ) -> dict[str, Any]: """PATCH /v1/checkins//questions// — update a question.""" - response: httpx.Response = httpx.patch( + response: httpx.Response = self._request( + "PATCH", f"{self.api_url}/v1/checkins/{followup_uuid}/questions/{question_uuid}/", json=fields, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -812,11 +881,9 @@ def delete_checkin_question( question_uuid: str, ) -> dict[str, Any]: """DELETE /v1/checkins//questions//delete/ (204).""" - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/checkins/{followup_uuid}/questions/{question_uuid}/delete/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -826,11 +893,10 @@ def reorder_checkin_questions( order: list[str], ) -> dict[str, Any]: """PUT /v1/checkins//questions/reorder/ — set a new question order.""" - response: httpx.Response = httpx.put( + response: httpx.Response = self._request( + "PUT", f"{self.api_url}/v1/checkins/{followup_uuid}/questions/reorder/", json={"question_uuids": order}, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -839,11 +905,10 @@ def get_mood(self, mood_date: str | None = None) -> dict[str, Any]: params: dict[str, str] = {} if mood_date: params["date"] = mood_date - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/mood/track/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) @@ -852,11 +917,10 @@ def track_mood(self, score: int, mood_date: str | None = None) -> dict[str, Any] payload: dict[str, Any] = {"score": score} if mood_date: payload["date"] = mood_date - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/mood/track/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -936,20 +1000,17 @@ def list_form_owners( params["offset"] = offset if limit is not None: params["limit"] = limit - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/forms/form-owners/", params=params, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) def get_form(self, form_uuid: str) -> dict[str, Any]: """GET /v1/forms// — form metadata and question definitions.""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/forms/{form_uuid}/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/forms/{form_uuid}/" ) return self._handle_response(response) @@ -973,11 +1034,10 @@ def submit_form_response( payload["guest_user"] = guest_user if submission_source: payload["submission_source"] = submission_source - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/forms/{form_uuid}/responses/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1050,10 +1110,9 @@ def get_form_response( response_uuid: str, ) -> dict[str, Any]: """GET /v1/forms//responses//""" - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/forms/{form_uuid}/responses/{response_uuid}/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1064,11 +1123,10 @@ def update_form_response( content: dict[str, Any], ) -> dict[str, Any]: """PATCH /v1/forms//responses//""" - response: httpx.Response = httpx.patch( + response: httpx.Response = self._request( + "PATCH", f"{self.api_url}/v1/forms/{form_uuid}/responses/{response_uuid}/", json={"content": content}, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1083,11 +1141,10 @@ def transition_form_response( payload: dict[str, Any] = {"to_state": to_state} if note: payload["note"] = note - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/forms/{form_uuid}/responses/{response_uuid}/transition/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1097,11 +1154,9 @@ def delete_form_response( response_uuid: str, ) -> dict[str, Any]: """DELETE /v1/forms//responses//""" - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/forms/{form_uuid}/responses/{response_uuid}/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1114,10 +1169,8 @@ def list_report_channels(self) -> list[dict[str, Any]]: older/other deployments may return ``{"results": [...]}`` or a bare list. All three are accepted. """ - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/report-channels/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/report-channels/" ) if response.status_code >= 400: self._handle_response(response) @@ -1158,11 +1211,10 @@ def create_form( payload["generate_short_question"] = True if config: payload.update(config) - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/forms/create/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1186,21 +1238,18 @@ def update_form_config( payload["report_channels"] = report_channels if config: payload.update(config) - response: httpx.Response = httpx.patch( + response: httpx.Response = self._request( + "PATCH", f"{self.api_url}/v1/forms/{form_uuid}/config/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) def archive_form(self, form_uuid: str) -> dict[str, Any]: """DELETE /v1/forms//archive/ — soft-delete a form (204).""" - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/forms/{form_uuid}/archive/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1210,11 +1259,10 @@ def add_form_question( question: dict[str, Any], ) -> dict[str, Any]: """POST /v1/forms//questions/ — add a question to a form.""" - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/forms/{form_uuid}/questions/", json=question, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1225,11 +1273,10 @@ def update_form_question( fields: dict[str, Any], ) -> dict[str, Any]: """PATCH /v1/forms//questions// — update a question.""" - response: httpx.Response = httpx.patch( + response: httpx.Response = self._request( + "PATCH", f"{self.api_url}/v1/forms/{form_uuid}/questions/{question_uuid}/", json=fields, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1239,11 +1286,9 @@ def delete_form_question( question_uuid: str, ) -> dict[str, Any]: """DELETE /v1/forms//questions//delete/ (204).""" - response: httpx.Response = httpx.request( + response: httpx.Response = self._request( "DELETE", f"{self.api_url}/v1/forms/{form_uuid}/questions/{question_uuid}/delete/", - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1253,11 +1298,10 @@ def reorder_form_questions( order: list[str], ) -> dict[str, Any]: """PUT /v1/forms//questions/reorder/ — set a new question order.""" - response: httpx.Response = httpx.put( + response: httpx.Response = self._request( + "PUT", f"{self.api_url}/v1/forms/{form_uuid}/questions/reorder/", json={"question_uuids": order}, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1286,20 +1330,17 @@ def get_me(self, *, include_email: bool = False) -> dict[str, Any]: params: dict[str, str] = {} if include_email: params["include_email"] = "true" - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/me/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) def get_organization(self) -> dict[str, Any]: """GET /v1/organization/ — the org the current credential is scoped to.""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/organization/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/organization/" ) return self._handle_response(response) @@ -1308,11 +1349,10 @@ def get_user(self, user_uuid: str, *, include_email: bool = False) -> dict[str, params: dict[str, str] = {} if include_email: params["include_email"] = "true" - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/users/{user_uuid}/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) @@ -1340,11 +1380,10 @@ def give_kudos( payload["teams_receivers"] = team_uuid_receivers if company_value: payload["company_value"] = company_value - response: httpx.Response = httpx.post( + response: httpx.Response = self._request( + "POST", f"{self.api_url}/v1/kudos/", json=payload, - headers=self._headers(), - timeout=self.timeout, ) return self._handle_response(response) @@ -1405,10 +1444,8 @@ def list_workflows( def get_workflow(self, workflow_uuid: str) -> dict[str, Any]: """GET /v1/workflows// — a single workflow's configuration.""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/workflows/{workflow_uuid}/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/workflows/{workflow_uuid}/" ) return self._handle_response(response) @@ -1447,11 +1484,10 @@ def get_kudos_wall_of_fame(self, *, limit: int | None = None) -> dict[str, Any]: params: dict[str, str] = {} if limit is not None: params["limit"] = str(limit) - response: httpx.Response = httpx.get( + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/kudos/wall-of-fame/", - headers=self._headers(), params=params, - timeout=self.timeout, ) return self._handle_response(response) @@ -1462,19 +1498,15 @@ def list_teams(self) -> list[dict[str, Any]]: def get_team(self, team_uuid: str) -> dict[str, Any]: """GET /v1/teams//""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/teams/{team_uuid}/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/teams/{team_uuid}/" ) return self._handle_response(response) def list_team_members(self, team_uuid: str) -> list[dict[str, Any]]: """GET /v1/teams//members/""" - response: httpx.Response = httpx.get( - f"{self.api_url}/v1/teams/{team_uuid}/members/", - headers=self._headers(), - timeout=self.timeout, + response: httpx.Response = self._request( + "GET", f"{self.api_url}/v1/teams/{team_uuid}/members/" ) if response.status_code >= 400: self._handle_response(response) diff --git a/dailybot_cli/commands/status.py b/dailybot_cli/commands/status.py index 3ab4bc5..56c1816 100644 --- a/dailybot_cli/commands/status.py +++ b/dailybot_cli/commands/status.py @@ -18,43 +18,49 @@ def _check_auth() -> None: - """Check authentication status: try OTP login first, then API key.""" - client: DailyBotClient = DailyBotClient() + """Check authentication status. - # Try OTP/login token first + Runs a single ``auth_status`` call; because the client transparently + retries with the alternative credential on 401/403, we then inspect + ``client._agent_auth_mode`` to report which credential actually + succeeded. This is what makes ``.dailybot/env.json`` "just work" even + when a stale prod Bearer session is still on disk — the retry inside + the client silently falls back to the env.json API key. + """ + client: DailyBotClient = DailyBotClient() token: str | None = get_token() - if token: - try: - with console.status("Checking login session..."): - data: dict[str, Any] = client.auth_status() - print_success("Authenticated via login (OTP)") - print_auth_status(data) - return - except APIError: - print_info("Login session is invalid or expired.") - - # Try API key api_key: str | None = get_api_key() - if api_key: - try: - with console.status("Checking API key..."): - client.get_agent_health(agent_name="CLI") - print_success("Authenticated via API key") - masked: str = api_key[:4] + "****" - print_info(f"API key: {masked}") - return - except APIError as e: - if e.status_code in (401, 403): - print_error("API key is invalid or unauthorized.") + + if not token and not api_key: + print_error("Not authenticated. Run: dailybot login or dailybot config key=YOUR_KEY") + raise SystemExit(1) + + try: + with console.status("Checking authentication..."): + data: dict[str, Any] = client.auth_status() + except APIError as e: + if e.status_code in (401, 403): + if token and api_key: + print_error( + "Both credentials were rejected — login token and API key both invalid." + ) + elif token: + print_error("Login session is invalid or expired. Run: dailybot login") else: - # Non-auth error means the key itself is valid - print_success("Authenticated via API key") - masked = api_key[:4] + "****" - print_info(f"API key: {masked}") - return + print_error("API key is invalid or unauthorized.") + else: + print_error(e.detail) + raise SystemExit(1) - print_error("Not authenticated. Run: dailybot login or dailybot config key=YOUR_KEY") - raise SystemExit(1) + mode: str | None = client._agent_auth_mode + if mode == "bearer": + print_success("Authenticated via login (OTP)") + elif mode == "api_key" and api_key: + print_success("Authenticated via API key") + print_info(f"API key: {api_key[:4]}****") + else: + print_success("Authenticated") + print_auth_status(data) @click.command() diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index fb3073b..b15ee61 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -162,14 +162,35 @@ The two files are **orthogonal** and both can be present: ### Interaction with the login Bearer token -When an `env.json` active profile provides an `api_key`, the CLI's `DailyBotClient` receives that key and — because Bearer is preferred over API key in `_headers()` — the presence of the login Bearer token would normally still win. To make "logged into different orgs in different repos" work correctly, the client should be constructed such that the API key takes precedence for env.json-authored contexts. +When an `env.json` active profile provides an `api_key` **and** a login Bearer token also exists on disk, the CLI's `DailyBotClient` needs to reconcile the two. It does so with a **transparent alt-credential retry** that makes both single-org and cross-org setups behave sensibly with zero user intervention. -Two ways this is achieved in practice: +The mechanics: -- **Different API URLs.** When the env.json profile carries `api_url` pointing at, say, `http://localhost:8000` while the stored Bearer session was issued against `https://api.dailybot.com`, the URL alone routes correctly and the server-side auth is the only thing that matters (each env has its own key/token). -- **Auto-fallback on 401.** As of CLI `>= 3.5.1`, if Bearer fails with 401, the client automatically retries with the alternative credential (API key). So even if the Bearer wins the initial dispatch and the token isn't valid on the env.json's API, the API key retry succeeds. +1. **`_headers()` still prefers the Bearer token** on the first attempt (preserves backward compat — every existing single-org flow is unchanged). +2. **On a 401 or 403 response**, the client's `_request()` / `_agent_request()` helpers automatically retry the same call **once** with the alternative credential (the `env.json` API key). The retry is invisible to the caller — it happens inside the HTTP layer, not in each command. +3. **`status --auth` inspects `_agent_auth_mode`** after the call returns to report which credential *actually* succeeded on the wire, so the UX is honest about the effective auth path. -For a bulletproof "different org per repo" story, prefer profiles with distinct `api_url`s or use `dailybot logout` on machines where the mix would be ambiguous. +Why retry on 403 too? Django/DRF frequently returns 403 instead of 401 for rejected credentials (see [DRF docs — "If not authenticated, 403"](https://www.django-rest-framework.org/api-guide/authentication/#unauthorized-and-forbidden-responses)). Retrying on only 401 misses this common local-Django case entirely — which is precisely the case `env.json` was designed to fix. + +Concrete example. You are logged in with `dailybot login` against production, and you `cd` into a repo that has `.dailybot/env.json` with an active `local-admin` profile pointing at `http://localhost:8000`: + +``` + + client.auth_status() + | + | Attempt 1: Bearer -> http://localhost:8000 + | 403 Forbidden (Bearer unknown to local API) + | + | Attempt 2: X-API-KEY -> http://localhost:8000 + | 200 OK + | + + returns { user, organization, ... } from LOCAL org +``` + +`dailybot status --auth` then prints `Authenticated via API key` (not "login (OTP)") because that is what actually worked. `dailybot user list`, `dailybot form list`, `dailybot kudos give`, etc. all follow the exact same path — they never hit the "you must log in again" wall when `env.json` is providing valid credentials for a different API URL. + +The retry costs at most one extra round-trip, only on the first request against a new server, and is completely silent to the user. The client's `_agent_auth_mode` attribute is used only by `dailybot status --auth` to describe which credential succeeded; no other command needs to care. + +For a bulletproof "different org per repo" story, prefer profiles with distinct `api_url`s (which is the whole point of `env.json`). `dailybot logout` remains available if a developer wants to eliminate the Bearer entirely. ### When NOT to use `env.json` diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 3decc7d..c1c81ea 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -1106,20 +1106,61 @@ def test_api_key_rejected_retries_with_bearer(self) -> None: assert exc_info.value.status_code == 401 assert mock_req.call_count == 1 - def test_no_retry_on_non_401_errors(self) -> None: - client = DailyBotClient(api_url="http://test.com", token="valid-token", api_key="key") - forbidden: MagicMock = MagicMock(spec=httpx.Response) - forbidden.status_code = 403 - forbidden.json.return_value = {"detail": "Forbidden"} + def test_403_triggers_retry_with_api_key(self) -> None: + """403 (auth-not-provided) also retries with the alt credential. + + Many Django/DRF backends return 403 for both "credentials rejected" + and "credentials accepted but permission denied". Because we cannot + tell them apart from the status code alone, and because the retry + with a different identity is either strictly better (rejected case) + or a no-op with the same 403 outcome (permission-denied case), we + retry on both 401 and 403. This is what makes env.json + a stale + Bearer session work seamlessly against a local API that answers 403 + to invalid Bearers. + """ + client = DailyBotClient( + api_url="http://test.com", token="wrong-server-bearer", api_key="valid-key" + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 403 + rejected.json.return_value = {"detail": "Authentication credentials were not provided."} - with ( - patch("dailybot_cli.api_client.httpx.request", return_value=forbidden) as mock_req, - pytest.raises(APIError) as exc_info, - ): - client.submit_agent_report(agent_name="Test", content="Hi") + success: MagicMock = MagicMock(spec=httpx.Response) + success.status_code = 200 + success.json.return_value = {"id": 2, "uuid": "xyz"} - assert exc_info.value.status_code == 403 - assert mock_req.call_count == 1 + with patch( + "dailybot_cli.api_client.httpx.request", side_effect=[rejected, success] + ) as mock_req: + result: dict[str, Any] = client.submit_agent_report( + agent_name="Test Agent", content="Hello" + ) + + assert result == {"id": 2, "uuid": "xyz"} + assert mock_req.call_count == 2 + first_headers: dict[str, str] = mock_req.call_args_list[0][1]["headers"] + assert first_headers.get("Authorization") == "Bearer wrong-server-bearer" + retry_headers: dict[str, str] = mock_req.call_args_list[1][1]["headers"] + assert retry_headers.get("X-API-KEY") == "valid-key" + + def test_no_retry_on_non_auth_errors(self) -> None: + """Genuine non-auth errors (400/404/500) must NOT retry.""" + client = DailyBotClient(api_url="http://test.com", token="valid-token", api_key="key") + for status in (400, 404, 422, 500, 502, 503): + error_resp: MagicMock = MagicMock(spec=httpx.Response) + error_resp.status_code = status + error_resp.json.return_value = {"detail": f"HTTP {status}"} + + with ( + patch( + "dailybot_cli.api_client.httpx.request", return_value=error_resp + ) as mock_req, + pytest.raises(APIError) as exc_info, + ): + client.submit_agent_report(agent_name="Test", content="Hi") + + assert exc_info.value.status_code == status + assert mock_req.call_count == 1, f"Expected no retry for HTTP {status}" def test_no_retry_when_only_one_credential(self) -> None: """Bearer-only client: no alternative credential → no retry.""" @@ -1169,6 +1210,147 @@ def test_both_credentials_available_bearer_goes_first(self) -> None: assert "X-API-KEY" not in headers +class TestUserAuthFallback: + """`_request()` mirrors `_agent_request` for user-scoped endpoints + (auth_status, checkin, form, kudos, chat, user, team, ...). It retries + on 401 OR 403 with the alternative credential, using `_headers()` as + the primary. This closes the "env.json active but prod Bearer session + still present" UX gap — the CLI silently falls back to the env.json + API key when the Bearer is rejected by a different-org API.""" + + def test_auth_status_retries_bearer_to_api_key_on_401(self) -> None: + """`dailybot status --auth` recovers when the login Bearer is stale + but the env.json API key is fresh.""" + client = DailyBotClient( + api_url="http://test.com", token="expired-bearer", api_key="fresh-key" + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 401 + rejected.json.return_value = {"detail": "Unauthorized"} + + ok: MagicMock = MagicMock(spec=httpx.Response) + ok.status_code = 200 + ok.json.return_value = {"email": "me@example.com", "organization_name": "Local"} + + with patch( + "dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok] + ) as mock_get: + result: dict[str, Any] = client.auth_status() + + assert result["email"] == "me@example.com" + assert mock_get.call_count == 2 + assert ( + mock_get.call_args_list[0][1]["headers"].get("Authorization") + == "Bearer expired-bearer" + ) + assert ( + mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "fresh-key" + ) + + def test_auth_status_retries_bearer_to_api_key_on_403(self) -> None: + """403 (DRF's default for a rejected Bearer against a different-server + API) also triggers the fallback.""" + client = DailyBotClient( + api_url="http://test.com", token="wrong-server-bearer", api_key="local-key" + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 403 + rejected.json.return_value = {"detail": "Authentication credentials were not provided."} + + ok: MagicMock = MagicMock(spec=httpx.Response) + ok.status_code = 200 + ok.json.return_value = {"email": "local@example.com"} + + with patch( + "dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok] + ) as mock_get: + result: dict[str, Any] = client.auth_status() + + assert result["email"] == "local@example.com" + assert mock_get.call_count == 2 + assert ( + mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "local-key" + ) + + def test_no_retry_when_only_bearer_available(self) -> None: + """Bearer-only client (no API key): no alternative → single request.""" + client = DailyBotClient( + api_url="http://test.com", token="stale-bearer", api_key=None + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 401 + rejected.json.return_value = {"detail": "Unauthorized"} + + with ( + patch( + "dailybot_cli.api_client.httpx.get", return_value=rejected + ) as mock_get, + pytest.raises(APIError) as exc_info, + ): + client.auth_status() + + assert exc_info.value.status_code == 401 + assert mock_get.call_count == 1 + + def test_no_retry_on_2xx_success(self) -> None: + """Happy path (Bearer works first try) makes only one request.""" + client = DailyBotClient( + api_url="http://test.com", token="good", api_key="also-good" + ) + ok: MagicMock = MagicMock(spec=httpx.Response) + ok.status_code = 200 + ok.json.return_value = {"email": "me@example.com"} + + with patch( + "dailybot_cli.api_client.httpx.get", return_value=ok + ) as mock_get: + client.auth_status() + + assert mock_get.call_count == 1 + + def test_no_retry_on_non_auth_errors(self) -> None: + """Genuine non-auth errors (400/404/500/502) do NOT retry.""" + client = DailyBotClient( + api_url="http://test.com", token="good", api_key="also-good" + ) + for status in (400, 404, 422, 500, 502, 503): + error_resp: MagicMock = MagicMock(spec=httpx.Response) + error_resp.status_code = status + error_resp.json.return_value = {"detail": f"HTTP {status}"} + + with ( + patch( + "dailybot_cli.api_client.httpx.get", return_value=error_resp + ) as mock_get, + pytest.raises(APIError) as exc_info, + ): + client.auth_status() + + assert exc_info.value.status_code == status + assert mock_get.call_count == 1, f"Expected no retry for HTTP {status}" + + def test_login_endpoints_never_retry(self) -> None: + """`request_code` / `verify_code` / `logout` / `register_agent` — the + auth-lifecycle endpoints — must never trigger the alt-credential + retry because the credential IS the thing being negotiated (or + because we're actively logging out). A failure at any of them means + exactly what it says.""" + client = DailyBotClient( + api_url="http://test.com", token="tok", api_key="key" + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 401 + rejected.json.return_value = {"detail": "Bad code"} + + with ( + patch("dailybot_cli.api_client.httpx.post", return_value=rejected) as mock_post, + pytest.raises(APIError), + ): + client.request_code("me@example.com") + + assert mock_post.call_count == 1 + + class TestHeadersDualAuth: def test_headers_sends_api_key_when_no_token(self) -> None: """_headers() sends X-API-KEY when no Bearer token is available.""" diff --git a/tests/commands_test.py b/tests/commands_test.py index b5a597e..8171394 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -1031,18 +1031,25 @@ def test_status_not_authenticated(self, mock_get_auth: MagicMock, runner: CliRun assert result.exit_code == 1 assert "Not authenticated" in result.output + @patch("dailybot_cli.commands.status.get_api_key") @patch("dailybot_cli.commands.status.get_token") @patch("dailybot_cli.commands.status.DailyBotClient") def test_status_auth_valid_login( - self, mock_client_cls: MagicMock, mock_get_token: MagicMock, runner: CliRunner + self, + mock_client_cls: MagicMock, + mock_get_token: MagicMock, + mock_get_api_key: MagicMock, + runner: CliRunner, ) -> None: """--auth with valid OTP session shows login auth info.""" mock_get_token.return_value = "tok" + mock_get_api_key.return_value = None mock_client: MagicMock = mock_client_cls.return_value mock_client.auth_status.return_value = { "user": {"email": "user@test.com"}, "organization": {"name": "MyOrg", "uuid": "org-uuid"}, } + mock_client._agent_auth_mode = "bearer" result = runner.invoke(cli, ["status", "--auth"]) assert result.exit_code == 0 @@ -1060,11 +1067,17 @@ def test_status_auth_valid_api_key( mock_get_api_key: MagicMock, runner: CliRunner, ) -> None: - """--auth falls back to API key when no login token.""" + """--auth reports 'API key' when the client used the API key path + (either because no login token exists, or because env.json is + active and forces the API key as the effective credential).""" mock_get_token.return_value = None mock_get_api_key.return_value = "sk-abc123" mock_client: MagicMock = mock_client_cls.return_value - mock_client.get_agent_health.return_value = {"status": "healthy"} + mock_client.auth_status.return_value = { + "user": {"email": "user@test.com"}, + "organization": {"name": "MyOrg", "uuid": "org-uuid"}, + } + mock_client._agent_auth_mode = "api_key" result = runner.invoke(cli, ["status", "--auth"]) assert result.exit_code == 0 @@ -1074,26 +1087,57 @@ def test_status_auth_valid_api_key( @patch("dailybot_cli.commands.status.get_api_key") @patch("dailybot_cli.commands.status.get_token") @patch("dailybot_cli.commands.status.DailyBotClient") - def test_status_auth_expired_login_falls_back_to_api_key( + def test_status_auth_expired_login_transparently_falls_back( + self, + mock_client_cls: MagicMock, + mock_get_token: MagicMock, + mock_get_api_key: MagicMock, + runner: CliRunner, + ) -> None: + """--auth with an expired login + valid API key succeeds silently: + the client's internal alt-credential retry (Bearer → API key) means + ``status --auth`` never surfaces the intermediate failure. The final + reported credential is the API key because that's what actually + succeeded on the wire.""" + mock_get_token.return_value = "expired-tok" + mock_get_api_key.return_value = "sk-xyz789" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.auth_status.return_value = { + "user": {"email": "user@test.com"}, + "organization": {"name": "MyOrg", "uuid": "org-uuid"}, + } + mock_client._agent_auth_mode = "api_key" + + result = runner.invoke(cli, ["status", "--auth"]) + assert result.exit_code == 0 + assert "invalid or expired" not in result.output + assert "API key" in result.output + assert "sk-x****" in result.output + + @patch("dailybot_cli.commands.status.get_api_key") + @patch("dailybot_cli.commands.status.get_token") + @patch("dailybot_cli.commands.status.DailyBotClient") + def test_status_auth_both_credentials_rejected( self, mock_client_cls: MagicMock, mock_get_token: MagicMock, mock_get_api_key: MagicMock, runner: CliRunner, ) -> None: - """--auth with expired login falls back to valid API key.""" + """When both Bearer and API key are on disk AND the API rejects both + (401/403), the CLI surfaces a distinctive error that names both + credentials — otherwise the user cannot tell whether the login is + expired, the API key is wrong, or both.""" from dailybot_cli.api_client import APIError mock_get_token.return_value = "expired-tok" mock_get_api_key.return_value = "sk-xyz789" mock_client: MagicMock = mock_client_cls.return_value mock_client.auth_status.side_effect = APIError(401, "Unauthorized") - mock_client.get_agent_health.return_value = {"status": "healthy"} result = runner.invoke(cli, ["status", "--auth"]) - assert result.exit_code == 0 - assert "invalid or expired" in result.output - assert "API key" in result.output + assert result.exit_code == 1 + assert "Both credentials were rejected" in result.output @patch("dailybot_cli.commands.status.get_api_key") @patch("dailybot_cli.commands.status.get_token") From 065768ce6ceca8b1f789f64a490f72b32b5db833 Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 00:36:40 +0000 Subject: [PATCH 3/5] style(client,tests): apply `ruff format` to api_client + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Follow-up on the previous commit — the CI `ruff format --check` gate caught unformatted whitespace in api_client.py and tests/api_client_test.py after the auth-retry refactor. Zero behavioral change; local pytest, ruff check, ruff format --check, and mypy all clean. Co-authored-by: Cursor --- dailybot_cli/api_client.py | 24 +++++------------- tests/api_client_test.py | 51 ++++++++++---------------------------- 2 files changed, 19 insertions(+), 56 deletions(-) diff --git a/dailybot_cli/api_client.py b/dailybot_cli/api_client.py index f181263..da33ea2 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -496,9 +496,7 @@ def verify_code( def auth_status(self) -> dict[str, Any]: """GET /v1/cli/auth/status/""" - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/cli/auth/status/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/cli/auth/status/") return self._handle_response(response) def logout(self) -> dict[str, Any]: @@ -544,9 +542,7 @@ def submit_update( def get_status(self) -> dict[str, Any]: """GET /v1/cli/status/""" - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/cli/status/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/cli/status/") return self._handle_response(response) def create_chat_completion( @@ -1009,9 +1005,7 @@ def list_form_owners( def get_form(self, form_uuid: str) -> dict[str, Any]: """GET /v1/forms// — form metadata and question definitions.""" - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/forms/{form_uuid}/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/forms/{form_uuid}/") return self._handle_response(response) def submit_form_response( @@ -1169,9 +1163,7 @@ def list_report_channels(self) -> list[dict[str, Any]]: older/other deployments may return ``{"results": [...]}`` or a bare list. All three are accepted. """ - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/report-channels/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/report-channels/") if response.status_code >= 400: self._handle_response(response) body: Any = response.json() @@ -1339,9 +1331,7 @@ def get_me(self, *, include_email: bool = False) -> dict[str, Any]: def get_organization(self) -> dict[str, Any]: """GET /v1/organization/ — the org the current credential is scoped to.""" - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/organization/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/organization/") return self._handle_response(response) def get_user(self, user_uuid: str, *, include_email: bool = False) -> dict[str, Any]: @@ -1498,9 +1488,7 @@ def list_teams(self) -> list[dict[str, Any]]: def get_team(self, team_uuid: str) -> dict[str, Any]: """GET /v1/teams//""" - response: httpx.Response = self._request( - "GET", f"{self.api_url}/v1/teams/{team_uuid}/" - ) + response: httpx.Response = self._request("GET", f"{self.api_url}/v1/teams/{team_uuid}/") return self._handle_response(response) def list_team_members(self, team_uuid: str) -> list[dict[str, Any]]: diff --git a/tests/api_client_test.py b/tests/api_client_test.py index c1c81ea..597f833 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -1152,9 +1152,7 @@ def test_no_retry_on_non_auth_errors(self) -> None: error_resp.json.return_value = {"detail": f"HTTP {status}"} with ( - patch( - "dailybot_cli.api_client.httpx.request", return_value=error_resp - ) as mock_req, + patch("dailybot_cli.api_client.httpx.request", return_value=error_resp) as mock_req, pytest.raises(APIError) as exc_info, ): client.submit_agent_report(agent_name="Test", content="Hi") @@ -1232,20 +1230,15 @@ def test_auth_status_retries_bearer_to_api_key_on_401(self) -> None: ok.status_code = 200 ok.json.return_value = {"email": "me@example.com", "organization_name": "Local"} - with patch( - "dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok] - ) as mock_get: + with patch("dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok]) as mock_get: result: dict[str, Any] = client.auth_status() assert result["email"] == "me@example.com" assert mock_get.call_count == 2 assert ( - mock_get.call_args_list[0][1]["headers"].get("Authorization") - == "Bearer expired-bearer" - ) - assert ( - mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "fresh-key" + mock_get.call_args_list[0][1]["headers"].get("Authorization") == "Bearer expired-bearer" ) + assert mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "fresh-key" def test_auth_status_retries_bearer_to_api_key_on_403(self) -> None: """403 (DRF's default for a rejected Bearer against a different-server @@ -1261,30 +1254,22 @@ def test_auth_status_retries_bearer_to_api_key_on_403(self) -> None: ok.status_code = 200 ok.json.return_value = {"email": "local@example.com"} - with patch( - "dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok] - ) as mock_get: + with patch("dailybot_cli.api_client.httpx.get", side_effect=[rejected, ok]) as mock_get: result: dict[str, Any] = client.auth_status() assert result["email"] == "local@example.com" assert mock_get.call_count == 2 - assert ( - mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "local-key" - ) + assert mock_get.call_args_list[1][1]["headers"].get("X-API-KEY") == "local-key" def test_no_retry_when_only_bearer_available(self) -> None: """Bearer-only client (no API key): no alternative → single request.""" - client = DailyBotClient( - api_url="http://test.com", token="stale-bearer", api_key=None - ) + client = DailyBotClient(api_url="http://test.com", token="stale-bearer", api_key=None) rejected: MagicMock = MagicMock(spec=httpx.Response) rejected.status_code = 401 rejected.json.return_value = {"detail": "Unauthorized"} with ( - patch( - "dailybot_cli.api_client.httpx.get", return_value=rejected - ) as mock_get, + patch("dailybot_cli.api_client.httpx.get", return_value=rejected) as mock_get, pytest.raises(APIError) as exc_info, ): client.auth_status() @@ -1294,34 +1279,26 @@ def test_no_retry_when_only_bearer_available(self) -> None: def test_no_retry_on_2xx_success(self) -> None: """Happy path (Bearer works first try) makes only one request.""" - client = DailyBotClient( - api_url="http://test.com", token="good", api_key="also-good" - ) + client = DailyBotClient(api_url="http://test.com", token="good", api_key="also-good") ok: MagicMock = MagicMock(spec=httpx.Response) ok.status_code = 200 ok.json.return_value = {"email": "me@example.com"} - with patch( - "dailybot_cli.api_client.httpx.get", return_value=ok - ) as mock_get: + with patch("dailybot_cli.api_client.httpx.get", return_value=ok) as mock_get: client.auth_status() assert mock_get.call_count == 1 def test_no_retry_on_non_auth_errors(self) -> None: """Genuine non-auth errors (400/404/500/502) do NOT retry.""" - client = DailyBotClient( - api_url="http://test.com", token="good", api_key="also-good" - ) + client = DailyBotClient(api_url="http://test.com", token="good", api_key="also-good") for status in (400, 404, 422, 500, 502, 503): error_resp: MagicMock = MagicMock(spec=httpx.Response) error_resp.status_code = status error_resp.json.return_value = {"detail": f"HTTP {status}"} with ( - patch( - "dailybot_cli.api_client.httpx.get", return_value=error_resp - ) as mock_get, + patch("dailybot_cli.api_client.httpx.get", return_value=error_resp) as mock_get, pytest.raises(APIError) as exc_info, ): client.auth_status() @@ -1335,9 +1312,7 @@ def test_login_endpoints_never_retry(self) -> None: retry because the credential IS the thing being negotiated (or because we're actively logging out). A failure at any of them means exactly what it says.""" - client = DailyBotClient( - api_url="http://test.com", token="tok", api_key="key" - ) + client = DailyBotClient(api_url="http://test.com", token="tok", api_key="key") rejected: MagicMock = MagicMock(spec=httpx.Response) rejected.status_code = 401 rejected.json.return_value = {"detail": "Bad code"} From 9fcb1869208f8c145835e771a5012eeb37440e05 Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 00:46:20 +0000 Subject: [PATCH 4/5] fix(env): fire refuse-if-tracked guard from root cli() + strict docs audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Audit finding: the fatal refuse-if-tracked guard for `.dailybot/env.json` was only surfacing from the `env` subcommands. Every other command (`status`, `user list`, `form list`, `agent update`, ...) silently swallowed `RepoEnvError` in `_safe_active_env_profile()` and continued with fallback global auth. Result: if a developer accidentally `git add`-ed env.json, the CLI would keep operating happily while the API keys leaked in git history — precisely the disaster the guard is meant to prevent. ## Change Log - `dailybot_cli/main.py`: root `cli()` callback now calls `load_repo_env()` at startup and re-raises `RepoEnvError` as `SystemExit(1)` with a stderr `print_error()`. Every command is blocked; `--help` and `--version` still work (Click short-circuits them before the callback runs). - `tests/env_commands_test.py`: extracted `_stage_tracked_env_json()` helper; added `test_root_cli_refuses_every_command_when_env_json_tracked` (regression guard covering `status`, `user list`, `form list`, `me`, `config list`) and `test_root_cli_help_still_works_when_env_json_tracked`. - `docs/CONFIGURATION.md`: added a prominent "STOP — Read this before you author env.json" callout at the top of the section with the three-layer defense-in-depth model, the recovery recipe when a leak has already happened, and the explicit rotate-first-not-revert rule. Rewrote "Security guarantees" to state that the guard fires at the root callback for every command, not just env subcommands. - `docs/SECURITY.md`: upgraded the env.json subsection from 3 to 4 layers (added file permissions as its own numbered protection) and clarified that the guard fires at the root of every CLI invocation, cross-linking to the CONFIGURATION.md recovery recipe. - `AGENTS.md` rule 14: replaced the single-sentence "fatally refused when tracked" note with the full three-layer contract and pointed to the recovery recipe. ## Risks - The new startup check runs `git ls-files` once per CLI invocation when `.dailybot/env.json` exists in an ancestor of `$PWD`. Negligible overhead (single subprocess with `capture_output=True`); the check already existed inside `env` subcommands, this only widens the aperture. No overhead when env.json is absent (the check short- circuits on `find_repo_env_path()` returning None). - No behavior change for the vast majority of users (env.json is gitignored by default and never becomes tracked). - Backward compatible: no schema changes, no CLI flag changes. Co-authored-by: Cursor --- AGENTS.md | 12 +++++- dailybot_cli/main.py | 22 ++++++++++- docs/CONFIGURATION.md | 41 +++++++++++++++++--- docs/SECURITY.md | 13 +++++-- tests/env_commands_test.py | 76 +++++++++++++++++++++++++++++++++----- 5 files changed, 141 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5a69495..00ff258 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -277,9 +277,17 @@ The agent commands resolve credentials in this strict order — changing it is a 6. `dailybot config key=...` (stored in `~/.config/dailybot/config.json`) 7. Login session (Bearer token from `~/.config/dailybot/credentials.json`) -The `profile.json` file may also pin the agent display name (`name`) and a `default_metadata` object that gets shallow-merged into every report. **Credentials never live in `profile.json`** — a `key` field there is a hard error. `env.json` is the ONLY sanctioned place for API keys inside `.dailybot/`, and it is **fatally refused when tracked by git** (the CLI runs `git ls-files --error-unmatch` on load and raises `RepoEnvError` if the file is tracked). See [docs/CONFIGURATION.md](docs/CONFIGURATION.md) for the per-field precedence and the security rule. +The `profile.json` file may also pin the agent display name (`name`) and a `default_metadata` object that gets shallow-merged into every report. **Credentials never live in `profile.json`** — a `key` field there is a hard error. -The implementation lives in `dailybot_cli/config.py` (`get_active_env_profile`, `get_api_key`, `get_api_url`, `get_app_url`), `dailybot_cli/commands/agent.py::_resolve_agent_context`, and `dailybot_cli/api_client.py::_agent_headers`. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md). +**`env.json` is the ONLY sanctioned place for API keys inside `.dailybot/`, and it MUST NEVER be committed.** The CLI enforces this with three independent layers, all of them mandatory: + +1. `.gitignore` MUST include `.dailybot/*` (with `!.dailybot/profile.json` as the ONLY exception). `env.json` is never un-ignored. +2. Every write and every load re-chmods the file to `0o600`. +3. The **root `cli()` callback** in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation — if `.dailybot/env.json` is tracked by git (`git ls-files --error-unmatch` returns 0), `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts the process **before any subcommand runs**. Every command (`status`, `user list`, `form list`, `agent update`, `env show`, `login`, `upgrade`, ...) is blocked. Only `--help` and `--version` remain accessible (Click short-circuits). No silent fallback to global auth — ever. + +See [docs/CONFIGURATION.md § "STOP — Read this before you author `env.json`"](docs/CONFIGURATION.md#stop--read-this-before-you-author-envjson) for the recovery recipe when a leak has already happened (spoiler: rotate first, don't rewrite history). + +The implementation lives in `dailybot_cli/config.py` (`get_active_env_profile`, `get_api_key`, `get_api_url`, `get_app_url`, `load_repo_env`), `dailybot_cli/main.py::cli` (root-callback guard), `dailybot_cli/commands/agent.py::_resolve_agent_context`, and `dailybot_cli/api_client.py::_agent_headers`. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md). ### 15. Packaging & Versioning diff --git a/dailybot_cli/main.py b/dailybot_cli/main.py index cafab6d..7daa1e2 100644 --- a/dailybot_cli/main.py +++ b/dailybot_cli/main.py @@ -28,7 +28,13 @@ from dailybot_cli.commands.user import user from dailybot_cli.commands.version import version from dailybot_cli.commands.workflow import workflow -from dailybot_cli.config import set_api_url_override, set_app_url_override +from dailybot_cli.config import ( + RepoEnvError, + load_repo_env, + set_api_url_override, + set_app_url_override, +) +from dailybot_cli.display import print_error # Format used by `dailybot --version`. Single line so it's friendly to scripts # parsing the output. The richer multi-line panel lives in `dailybot version`. @@ -73,6 +79,20 @@ def cli(ctx: click.Context, api_url: str | None, app_url: str | None) -> None: Run without arguments for interactive mode. """ + # Fatal safety check: if `.dailybot/env.json` exists in the current tree + # AND it is tracked by git, refuse to run ANY command. Silently swallowing + # this in `_safe_active_env_profile()` (as the resilient per-getter path + # does) would mean the CLI happily continues with global auth while the + # user's org API keys leak in git history — precisely the disaster the + # gitignore + guard combo is meant to prevent. Blocking here is the + # correct security posture: force the user to `git rm --cached` (and + # rotate the exposed key) before the CLI does anything else. + try: + load_repo_env() + except RepoEnvError as exc: + print_error(str(exc)) + raise SystemExit(1) from exc + if api_url: set_api_url_override(api_url) if app_url: diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index b15ee61..014626c 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -45,6 +45,31 @@ The Dailybot CLI persists state in `~/.config/dailybot/` by default. The path ca Introduced in CLI `>= 3.7.0`. +> ## STOP — Read this before you author `env.json` +> +> **`.dailybot/env.json` MUST NEVER be committed to git. Ever. Under any circumstance.** +> +> The file stores API keys in plain text. Once committed to a repo — public or private — the keys are considered leaked and MUST be rotated. Git history is forever; a `git revert` does not undo the exposure. +> +> The CLI enforces this rule with **three independent layers of protection** — all of them must be in place, and any of them tripping is treated as a security incident, not a warning: +> +> 1. **Gitignore rule (mandatory).** The repo's `.gitignore` MUST contain `.dailybot/*` **without** ever un-ignoring `env.json`. Only `profile.json` may be excepted. This repo's [`.gitignore`](../.gitignore) is the reference implementation. +> 2. **`0o600` file permissions.** Every load and every write via `dailybot env` re-chmods the file to owner-only. Even shared workstations cannot leak the file laterally. +> 3. **Fatal refuse-if-tracked guard.** The root `cli()` callback calls `load_repo_env()` on every invocation. If `.dailybot/env.json` exists AND `git ls-files --error-unmatch .dailybot/env.json` returns 0 (i.e. the file is tracked), the CLI **immediately refuses to run any command** — `status`, `user list`, `form list`, `agent update`, `env show`, everything — and prints the exact `git rm --cached` recipe. There is no partial degradation, no "warning + continue" path, no silent fallback to global auth. The user must uncommit the file before the CLI does anything else. `--help` and `--version` are exempt (Click short-circuits them before the callback) so the user can always read instructions. +> +> **If any of these three layers appears to be missing or misbehaving on your machine, treat it as a bug and report it — don't work around it.** +> +> **If you have already committed `env.json`**, follow these steps in order: +> +> 1. **Rotate every key in the file immediately** (via the Dailybot dashboard). The old keys are compromised. +> 2. `git rm --cached .dailybot/env.json` +> 3. Verify `.gitignore` contains `.dailybot/*` (add it if missing). +> 4. `git commit -m "chore: untrack .dailybot/env.json"` +> 5. Force-clean the file from git history if the repo has been pushed anywhere (see [GitHub's guide to removing sensitive data](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/removing-sensitive-data-from-a-repository)). +> 6. Only then re-author `env.json` with the freshly rotated keys. +> +> When in doubt, prefer `DAILYBOT_API_KEY` (an environment variable, never on disk in the repo) — that path has no exposure surface at all. + ### Why it exists Before `env.json`, switching between local dev orgs / staging / production required exporting `DAILYBOT_API_URL` / `DAILYBOT_APP_URL` / `DAILYBOT_API_KEY` for every shell, or reconfiguring the global `agents.json`. This got painful when a developer wanted: @@ -111,18 +136,22 @@ dailybot env on # Re-enable ### Security guarantees -1. **Gitignored by convention.** The repo's root `.gitignore` should carry `.dailybot/*` with an explicit exception only for `!.dailybot/profile.json`. `env.json` is NEVER excepted. See the [example .gitignore for this repo](../.gitignore). -2. **`0o600` permissions.** Every write via `dailybot env` — and every load — enforces owner-only permissions defensively (in case an editor created the file with a lax umask). -3. **Fatal refuse-if-tracked guard.** On every load, the CLI runs `git ls-files --error-unmatch .dailybot/env.json` and raises `RepoEnvError` if the file is tracked. Any `dailybot env` subcommand (and any subsequent command that would consume env.json auth) exits non-zero with an actionable message: +Cross-referenced with the top-of-section STOP block. Repeated here so this appears in every table-of-contents jump. + +1. **Gitignored by convention (`.gitignore`).** The repo's root `.gitignore` MUST carry `.dailybot/*`. `profile.json` is the ONLY file allowed to be excepted (`!.dailybot/profile.json`). `env.json` is NEVER excepted — no exception, no per-machine dot-file trick, nothing. See the [example .gitignore for this repo](../.gitignore). +2. **`0o600` file permissions.** Every write via `dailybot env` AND every load re-chmods the file to owner-only, defensively (in case an editor created the file with a lax umask). Enforced in `dailybot_cli/config.py::save_repo_env`. +3. **Fatal refuse-if-tracked guard — enforced at the ROOT `cli()` callback.** On every command invocation (yes, including non-`env` commands like `status`, `user list`, `form list`, `agent update`), the CLI runs `load_repo_env()`, which internally runs `git ls-files --error-unmatch .dailybot/env.json`. If the file is tracked, `RepoEnvError` is raised, `print_error()` writes the message to stderr, and `SystemExit(1)` aborts the process **before any subcommand runs**. Sample stderr output: ``` - .dailybot/env.json is tracked by git. This file contains API keys and - must never be committed. Fix with: + Error: /path/to/repo/.dailybot/env.json is tracked by git. This file contains + API keys and must never be committed. Fix with: git rm --cached .dailybot/env.json # ensure your .gitignore ignores .dailybot/env.json git commit -m 'chore: untrack .dailybot/env.json' The CLI refuses to load env.json while it is tracked. ``` -4. **Masked in all output.** `dailybot env show` and `dailybot env list` mask API keys as `abcd****` (first 4 chars + `****`), matching the pattern used by `dailybot config key`. + **No command bypasses this** — `dailybot version`, `dailybot upgrade`, `dailybot uninstall`, `dailybot login`, `dailybot logout`, everything is blocked. Only `dailybot --help` and `dailybot --version` (Click short-circuits) still work so the developer can read instructions. This is the third and final layer that guarantees the CLI cannot silently degrade to global auth while `env.json` (and the keys it contains) leaks in git history. +4. **Masked in all output.** `dailybot env show`, `dailybot env list`, and `dailybot agent profiles --resolve` all mask API keys as `abcd****` (first 4 chars + `****`), matching the pattern used by `dailybot config key`. Full keys never appear in logs, stderr, error traces, or telemetry. +5. **No key export.** There is intentionally no `dailybot env export` or `dailybot env cat` command that would print the raw keys to stdout. Editing the file requires opening it in a text editor (which triggers the developer's own security awareness). ### Auth-resolution precedence (updated) diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 0e51ab7..7b159ee 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -38,11 +38,16 @@ Files without secrets (still written `0o600` for consistency): ### Repo-level env override (`.dailybot/env.json`) -The `env.json` file is the ONLY sanctioned place inside `.dailybot/` where API keys may live. It carries per-repo credential context (API key + optional URLs for one or more environments). Because it sits inside the repo tree, three additional protections apply beyond the standard `0o600`: +The `env.json` file is the ONLY sanctioned place inside `.dailybot/` where API keys may live. It carries per-repo credential context (API key + optional URLs for one or more environments). Because it sits inside the repo tree, **four independent protections** apply beyond the standard `0o600`: -1. **Gitignore is mandatory.** The broad `.dailybot/*` rule in the repo's `.gitignore` covers it automatically; the only excepted file is `!.dailybot/profile.json`. `env.json` MUST NEVER be excepted. -2. **Load-time refuse-if-tracked guard.** On every load, the CLI runs `git ls-files --error-unmatch .dailybot/env.json` and raises `RepoEnvError` if the file is tracked (fatal — the CLI exits non-zero with an actionable message and refuses to use env.json until fixed). The check runs even outside `env`-group commands because `get_api_key()`, `get_api_url()`, and `get_app_url()` all consult env.json on every construction of `DailyBotClient`. Implementation: `dailybot_cli/config.py::_is_env_tracked_by_git` (independently mockable). -3. **Write-time gitignore warning.** `dailybot env add` runs `git check-ignore --quiet .dailybot/env.json` after writing; if the file is NOT covered by any ignore rule, a warning fires on stderr with the exact `.gitignore` snippet to add. The warning is non-fatal because a fresh repo might not have a `.gitignore` yet, and the load-time guard catches the actual security violation. +1. **Gitignore is mandatory (`.gitignore`).** The broad `.dailybot/*` rule in the repo's `.gitignore` covers it automatically; the only excepted file is `!.dailybot/profile.json`. `env.json` MUST NEVER be excepted — not with a per-machine dot-file trick, not with a `git update-index --assume-unchanged`, not with anything. +2. **File permissions (`0o600`).** Every read and every write via `dailybot env` re-chmods the file to owner-only. Implementation: `dailybot_cli/config.py::save_repo_env`. +3. **Root-callback refuse-if-tracked guard (fatal, applies to EVERY command).** The root `cli()` callback in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation, which internally runs `git ls-files --error-unmatch .dailybot/env.json`. If the file is tracked, `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts **before any subcommand runs**. There is no silent fallback to global auth — the entire process refuses to operate until the developer runs `git rm --cached .dailybot/env.json`. The only exempt paths are `--help` and `--version` (Click short-circuits) so the developer can always read instructions. Implementation: `dailybot_cli/config.py::_is_env_tracked_by_git` (independently mockable), invoked via `load_repo_env` from `main.py::cli`. +4. **Write-time gitignore warning.** `dailybot env add` runs `git check-ignore --quiet .dailybot/env.json` after writing; if the file is NOT covered by any ignore rule, a warning fires on stderr with the exact `.gitignore` snippet to add. The warning is non-fatal because a fresh repo might not have a `.gitignore` yet, and the load-time guard (#3) catches the actual security violation. + +**All four protections must trip together** for a leak to happen: the developer would have to (a) remove or fail to add the `.gitignore` rule, (b) survive the write-time warning, (c) survive the load-time refuse-if-tracked check, and (d) somehow bypass the file permissions. The design is defense-in-depth on purpose. + +**If a key ever ends up in a commit**, treat it as compromised — rotate immediately via the Dailybot dashboard, then follow the recovery recipe in [CONFIGURATION.md § "STOP — Read this before you author `env.json`"](CONFIGURATION.md#stop--read-this-before-you-author-envjson). Git history is forever; `git revert` does not undo the exposure. The full schema, precedence, and CLI commands for `env.json` are in [CONFIGURATION.md § "Repo-level env override"](CONFIGURATION.md#repo-level-env-override-dailybotenvjson). diff --git a/tests/env_commands_test.py b/tests/env_commands_test.py index 2cecdab..c3c84f1 100644 --- a/tests/env_commands_test.py +++ b/tests/env_commands_test.py @@ -322,16 +322,11 @@ def test_off_no_file_errors(self, runner: CliRunner, chdir_tmp: Path) -> None: class TestCommittedGuardSurfacing: - def test_env_use_bubbles_fatal_when_env_json_tracked( - self, - runner: CliRunner, - chdir_tmp: Path, - ) -> None: - """Env.json tracked in git → any env subcommand exits non-zero with a - message the developer can act on.""" + def _stage_tracked_env_json(self, runner: CliRunner, chdir_tmp: Path) -> None: + """Shared setup: write env.json via the CLI, then simulate a + developer mistake by force-adding and committing it.""" from dailybot_cli.main import cli - # First, write env.json properly. runner.invoke(cli, ["env", "add", "--name", "live", "--key", "k"]) subprocess.run( ["git", "config", "user.email", "test@example.com"], @@ -343,15 +338,76 @@ def test_env_use_bubbles_fatal_when_env_json_tracked( cwd=chdir_tmp, check=True, ) - # Force-add and commit — simulate a developer mistake. subprocess.run( ["git", "add", "-f", ".dailybot/env.json"], cwd=chdir_tmp, check=True, ) subprocess.run(["git", "commit", "-q", "-m", "leak"], cwd=chdir_tmp, check=True) - # Now any env-touching command should refuse. + + def test_env_use_bubbles_fatal_when_env_json_tracked( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """Env.json tracked in git → any env subcommand exits non-zero with a + message the developer can act on.""" + from dailybot_cli.main import cli + + self._stage_tracked_env_json(runner, chdir_tmp) result = runner.invoke(cli, ["env", "show"]) assert result.exit_code == 1 combined: str = result.output + (result.stderr or "") assert "tracked" in combined.lower() + + def test_root_cli_refuses_every_command_when_env_json_tracked( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """Regression guard for the security bug where the refuse-if-tracked + check only fired for `env` subcommands — every other command + (`status`, `user list`, `form list`, `agent update`, ...) silently + ignored the guard and continued with fallback global auth, leaving + the tracked env.json (and the API keys it contains) exposed in git + history while the CLI happily kept running. + + The fix wires the check into the root `cli()` callback so it fires + universally, before any subcommand executes. Any command that goes + through the root group should refuse.""" + from dailybot_cli.main import cli + + self._stage_tracked_env_json(runner, chdir_tmp) + + # Sample non-env commands from every layer — user-scoped, agent, + # meta, and no-op subcommands. All must refuse identically. + for argv in ( + ["status", "--auth"], + ["user", "list"], + ["form", "list"], + ["me"], + ["config", "list"], + ): + result = runner.invoke(cli, argv) + assert result.exit_code == 1, f"{argv!r} should have refused" + combined: str = result.output + (result.stderr or "") + assert "tracked" in combined.lower(), ( + f"{argv!r} exit was 1 but the error text did not surface the " + f"tracked-env.json reason. Got: {combined!r}" + ) + + def test_root_cli_help_still_works_when_env_json_tracked( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """`--help` and `--version` must NOT be blocked by the guard — + Click short-circuits these before the root callback, so the user + can always discover the CLI (and read the fix instructions).""" + from dailybot_cli.main import cli + + self._stage_tracked_env_json(runner, chdir_tmp) + + for argv in (["--help"], ["--version"]): + result = runner.invoke(cli, argv) + assert result.exit_code == 0, f"{argv!r} should succeed even when env.json is tracked" From 3d4766418beb59ddc78d0c46bcebd16c92d5c00a Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 01:40:24 +0000 Subject: [PATCH 5/5] fix(env): make env.json precedence real on the wire + harden the guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Close every gap found in the adversarial audit of the env.json feature so the documented contract and the actual behavior are identical. ## Change Log - env.json-sourced API keys now go FIRST on the wire (X-API-KEY on attempt 1) instead of relying on the 401/403 retry: correct identity on same-server multi-org setups, no Bearer-token transmission to the env.json server, no doubled round-trips. Keys from env var / config.json keep the historical Bearer-first order. (get_api_key_source + DailyBotClient.prefer_api_key, auto-detected) - _resolve_agent_context now applies env.json above keyed agents.json profiles (except explicit --profile), matching agent profiles --resolve so display and runtime always agree; keyless profiles now use the full ambient chain instead of requiring a Bearer - Root refuse-if-tracked guard: hook group exempted (prints to stderr, exits 0 per the AGENT_HOOKS always-exit-0 contract); stale/deleted cwd no longer tracebacks (find_repo_env_path/find_repo_profile_path return None on OSError); git-missing-but-.git-present degrades to a loud warning - dailybot login warns when an active env.json profile redirects it (login rewrites the GLOBAL session api_url) - Non-bool "disabled" values (e.g. the string "true") now warn loudly instead of silently keeping the file active - env.json is created 0o600 from the first byte (os.open) — no umask window - env show renders an explicit "Disabled: no" row and resolved default URLs - Tests: +31 (wire preference both directions, retry no-loop, _dispatch_http branches, staged-not-committed guard, hook-under-guard exit 0, agent env.json-vs-profile regression suite, login warning, disabled-string, duplicate/non-dict profile entries, warn-once dedup, stale cwd); fixed the misnamed api_key-retry test and the tautological closest-ancestor test; extended login-lifecycle no-retry to verify_code/logout - Docs: AGENTS.md rule 14, CONFIGURATION.md, SECURITY.md, AGENT_HOOKS.md rewritten to the now-true contract (wire preference, --profile carve-out, hook exemption, four protections wording) ## Risks - Behavior change is scoped to env.json-sourced keys; every pre-existing auth flow (Bearer, DAILYBOT_API_KEY, config.json, keyed --profile) keeps its exact wire order. 1035 tests pass; ruff + mypy clean. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 9 +- dailybot_cli/api_client.py | 53 ++++++++--- dailybot_cli/commands/agent.py | 34 +++++-- dailybot_cli/commands/auth.py | 33 +++++++ dailybot_cli/config.py | 66 ++++++++++++-- dailybot_cli/display.py | 8 +- dailybot_cli/main.py | 9 +- docs/AGENT_HOOKS.md | 5 +- docs/CONFIGURATION.md | 35 ++++---- docs/SECURITY.md | 4 +- tests/api_client_test.py | 158 ++++++++++++++++++++++++++++++++- tests/commands_test.py | 52 +++++++++++ tests/env_commands_test.py | 65 ++++++++++++++ tests/repo_env_test.py | 136 ++++++++++++++++++++++++++-- tests/repo_profile_test.py | 75 ++++++++++++++++ 15 files changed, 684 insertions(+), 58 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 00ff258..6511b17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -277,13 +277,16 @@ The agent commands resolve credentials in this strict order — changing it is a 6. `dailybot config key=...` (stored in `~/.config/dailybot/config.json`) 7. Login session (Bearer token from `~/.config/dailybot/credentials.json`) +This order holds at the HTTP layer too: when the resolved API key comes from `env.json`, the client sends `X-API-KEY` on the **first** attempt even if a Bearer login session exists (`DailyBotClient._prefer_api_key`, auto-detected via `get_api_key_source()`); the transparent 401/403 retry covers the reverse direction. Keys from any other layer keep the long-standing Bearer-first wire order. A keyed `agents.json` profile only beats `env.json` when selected with an explicit `--profile` flag (layer 1); resolved via `profile.json` or as the default profile, it yields to `env.json` — `agent profiles --resolve` and the actual request always agree. + The `profile.json` file may also pin the agent display name (`name`) and a `default_metadata` object that gets shallow-merged into every report. **Credentials never live in `profile.json`** — a `key` field there is a hard error. -**`env.json` is the ONLY sanctioned place for API keys inside `.dailybot/`, and it MUST NEVER be committed.** The CLI enforces this with three independent layers, all of them mandatory: +**`env.json` is the ONLY sanctioned place for API keys inside `.dailybot/`, and it MUST NEVER be committed.** The CLI enforces this with four independent protections (three enforced + one advisory), all of them mandatory: 1. `.gitignore` MUST include `.dailybot/*` (with `!.dailybot/profile.json` as the ONLY exception). `env.json` is never un-ignored. -2. Every write and every load re-chmods the file to `0o600`. -3. The **root `cli()` callback** in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation — if `.dailybot/env.json` is tracked by git (`git ls-files --error-unmatch` returns 0), `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts the process **before any subcommand runs**. Every command (`status`, `user list`, `form list`, `agent update`, `env show`, `login`, `upgrade`, ...) is blocked. Only `--help` and `--version` remain accessible (Click short-circuits). No silent fallback to global auth — ever. +2. Every write creates the file with mode `0o600` from the first byte (`os.open(..., 0o600)`), and every load re-chmods it defensively. +3. The **root `cli()` callback** in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation — if `.dailybot/env.json` is tracked by git (`git ls-files --error-unmatch` returns 0, staged or committed), `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts the process **before any subcommand runs**. Every command (`status`, `user list`, `form list`, `agent update`, `env show`, `login`, `upgrade`, ...) is blocked. Exactly two carve-outs: `--help` / `--version` (Click short-circuits before the callback), and the `hook` group, which prints the same error to stderr but continues and exits 0 — its contract (`docs/AGENT_HOOKS.md`) is "always exit 0, never break the agent harness" and hooks never consume env.json auth. No silent fallback to global auth — ever. If git is not on PATH but a `.git` ancestor exists, the guard cannot verify and degrades to a loud warning. +4. (Advisory) `dailybot env add` runs `git check-ignore` after writing and warns when the file is not covered by any ignore rule. See [docs/CONFIGURATION.md § "STOP — Read this before you author `env.json`"](docs/CONFIGURATION.md#stop--read-this-before-you-author-envjson) for the recovery recipe when a leak has already happened (spoiler: rotate first, don't rewrite history). diff --git a/dailybot_cli/api_client.py b/dailybot_cli/api_client.py index da33ea2..7e5d91d 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -7,7 +7,13 @@ import httpx -from dailybot_cli.config import get_api_key, get_api_url, get_token +from dailybot_cli.config import ( + API_KEY_SOURCE_ENV_JSON, + get_api_key, + get_api_key_source, + get_api_url, + get_token, +) _MAX_LIST_PAGES: int = 50 # safety cap for paginated list endpoints LONG_TIMEOUT_SECS: float = 120.0 # AI-processing endpoints (ask, submit_update) @@ -138,50 +144,69 @@ def __init__( token: str | None = None, api_key: str | None = None, timeout: float = 30.0, + prefer_api_key: bool | None = None, ) -> None: self.api_url: str = (api_url or get_api_url()).rstrip("/") self.token: str | None = token or get_token() self.api_key: str | None = api_key or get_api_key() self.timeout: float = timeout self._agent_auth_mode: str | None = None + # Credential preference on the wire. A key resolved from + # `.dailybot/env.json` expresses per-repo intent, so it must beat the + # global Bearer session on the FIRST attempt — otherwise the Bearer + # would silently win whenever the target server accepts it (wrong + # identity) and the session token would leak to whatever server the + # repo's env.json points at. Explicit `api_key` args and keys from + # env var / config.json keep the long-standing Bearer-first order. + if prefer_api_key is not None: + self._prefer_api_key: bool = prefer_api_key + else: + self._prefer_api_key = ( + api_key is None + and self.api_key is not None + and get_api_key_source() == API_KEY_SOURCE_ENV_JSON + ) def _headers(self, authenticated: bool = True) -> dict[str, str]: """Build request headers. - Prefers the Bearer login token; falls back to the org API key so that - user-scoped endpoints (users, teams, forms, kudos, check-ins) work under - either credential. The server accepts both on these endpoints. + Default priority is Bearer login token first, org API key second — + the server accepts both on user-scoped endpoints (users, teams, + forms, kudos, check-ins). When the key came from ``.dailybot/env.json`` + (``self._prefer_api_key``), the order inverts so the per-repo key + wins on the first attempt; the 401/403 retry covers the reverse. """ headers: dict[str, str] = { "Content-Type": "application/json", "Accept": "application/json", } if authenticated: - if self.token: - headers["Authorization"] = f"Bearer {self.token}" - self._agent_auth_mode = "bearer" - elif self.api_key: + if self.api_key and (self._prefer_api_key or not self.token): headers["X-API-KEY"] = self.api_key self._agent_auth_mode = "api_key" + elif self.token: + headers["Authorization"] = f"Bearer {self.token}" + self._agent_auth_mode = "bearer" return headers def _agent_headers(self) -> dict[str, str]: """Build headers for agent authentication. Uses the same priority as ``_headers()`` — Bearer first, API key - second — so that all endpoints behave consistently. The server - accepts both on every ``/v1/agent*`` endpoint. + second, inverted when the key came from ``.dailybot/env.json`` — so + that all endpoints behave consistently. The server accepts both on + every ``/v1/agent*`` endpoint. """ headers: dict[str, str] = { "Content-Type": "application/json", "Accept": "application/json", } - if self.token: - headers["Authorization"] = f"Bearer {self.token}" - self._agent_auth_mode = "bearer" - elif self.api_key: + if self.api_key and (self._prefer_api_key or not self.token): headers["X-API-KEY"] = self.api_key self._agent_auth_mode = "api_key" + elif self.token: + headers["Authorization"] = f"Bearer {self.token}" + self._agent_auth_mode = "bearer" else: self._agent_auth_mode = None return headers diff --git a/dailybot_cli/commands/agent.py b/dailybot_cli/commands/agent.py index 2426999..368f29e 100644 --- a/dailybot_cli/commands/agent.py +++ b/dailybot_cli/commands/agent.py @@ -10,9 +10,11 @@ from dailybot_cli import ledger from dailybot_cli.api_client import APIError, DailyBotClient from dailybot_cli.config import ( + RepoEnvError, RepoProfileError, _slugify, find_repo_root, + get_active_env_profile, get_agent_auth, get_default_profile, get_profile, @@ -91,9 +93,13 @@ def _resolve_agent_context( Resolution order (per-field, highest layer wins): 1. CLI flags (``--name``, ``--profile``) - 2. Repo file ``.dailybot/profile.json`` (walk-up from cwd, closest wins) - 3. Global default profile from ``agents.json`` - 4. Hardcoded fallback ``"CLI Agent"`` for the display name + 2. ``.dailybot/env.json`` active profile's API key (walk-up from cwd) — + applied unless ``--profile`` was passed explicitly; mirrors + :func:`dailybot_cli.config.resolve_active_profile` so that + ``agent profiles --resolve`` and the actual request always agree + 3. Repo file ``.dailybot/profile.json`` (walk-up from cwd, closest wins) + 4. Global default profile from ``agents.json`` + 5. Hardcoded fallback ``"CLI Agent"`` for the display name Returns ``(agent_name, client, default_metadata)`` — *default_metadata* is the repo file's ``default_metadata`` dict (``{}`` when absent), which the @@ -144,12 +150,28 @@ def _resolve_agent_context( else: agent_name = "CLI Agent" + # env.json credentials beat a keyed agents.json profile — the repo-local + # file is the more specific opt-in — but an explicit --profile flag is a + # direct user instruction and keeps its own key. This mirrors + # `resolve_active_profile` (the `agent profiles --resolve` display) so + # what the CLI shows and what it sends never diverge. + env_key_active: bool = False + if not profile_flag: + try: + env_profile: dict[str, Any] | None = get_active_env_profile() + except RepoEnvError: + # The root cli() guard already surfaced fatal env.json states; + # stay resilient here and fall through to the other layers. + env_profile = None + env_key_active = bool(env_profile and env_profile.get("api_key")) + if profile_data: api_key: str | None = profile_data.get("api_key") - if api_key: + if api_key and not env_key_active: return agent_name, DailyBotClient(api_key=api_key), repo_default_metadata - # Profile without key — fall through to Bearer token - if get_token(): + # Keyed profile overridden by env.json, or keyless profile — use the + # ambient chain (env.json > env var > config.json > Bearer session). + if env_key_active or get_agent_auth(): return agent_name, DailyBotClient(), repo_default_metadata print_error( f"Profile '{profile_data['profile']}' has no API key and no login session.\n" diff --git a/dailybot_cli/commands/auth.py b/dailybot_cli/commands/auth.py index 6be0125..a956250 100644 --- a/dailybot_cli/commands/auth.py +++ b/dailybot_cli/commands/auth.py @@ -7,9 +7,12 @@ from dailybot_cli.api_client import APIError, DailyBotClient from dailybot_cli.config import ( + RepoEnvError, clear_credentials, clear_org_cache, + get_active_env_profile, get_api_key, + get_api_url, get_token, load_org_cache, save_credentials, @@ -21,9 +24,37 @@ print_error, print_info, print_success, + print_warning, ) +def _warn_if_env_json_redirects_login() -> None: + """Warn when an active ``.dailybot/env.json`` profile redirects the login. + + Login writes the resolved ``api_url`` (and the token issued by that + server) to the GLOBAL ``~/.config/dailybot/credentials.json`` — a + repo-local env.json must never rewrite the user's global session + silently. The login still proceeds; the developer just gets told + where it is going and how to opt out. + """ + try: + env_profile: dict[str, Any] | None = get_active_env_profile() + except RepoEnvError: + # The root cli() guard already aborted on fatal states; stay quiet. + return + if ( + env_profile + and env_profile.get("api_url") + and get_api_url() == str(env_profile["api_url"]).rstrip("/") + ): + print_warning( + f"This repo's .dailybot/env.json (profile '{env_profile['name']}') points " + f"the CLI at {get_api_url()}. Logging in will authenticate against that " + "server and update your GLOBAL session for every repo. Run `dailybot env " + "off` first if you meant to log into your default server." + ) + + def _prompt_org_selection_numbered(organizations: list[dict[str, Any]]) -> dict[str, Any]: """Numbered org picker — fallback when questionary TUI is unavailable.""" print_info("You belong to multiple organizations. Select one by number:") @@ -274,6 +305,8 @@ def login(ctx: click.Context, email: str, code: str | None, org_uuid: str | None ctx.get_parameter_source("email") == click.core.ParameterSource.COMMANDLINE ) + _warn_if_env_json_redirects_login() + if code is not None: # Non-interactive step 2: verify code directly _verify_non_interactive(email, code, org_uuid) diff --git a/dailybot_cli/config.py b/dailybot_cli/config.py index a524c63..33c8a32 100644 --- a/dailybot_cli/config.py +++ b/dailybot_cli/config.py @@ -209,6 +209,28 @@ def get_api_key() -> str | None: return config.get("api_key") or None +API_KEY_SOURCE_ENV_JSON: str = "env.json" +API_KEY_SOURCE_ENV_VAR: str = "env" +API_KEY_SOURCE_CONFIG: str = "config" + + +def get_api_key_source() -> str | None: + """Return which layer :func:`get_api_key` resolves from, without the key. + + One of :data:`API_KEY_SOURCE_ENV_JSON`, :data:`API_KEY_SOURCE_ENV_VAR`, + :data:`API_KEY_SOURCE_CONFIG`, or ``None`` when no key is configured. + The HTTP client uses this to decide credential preference on the wire: + an env.json key expresses per-repo intent and must beat the global + Bearer session on the first attempt, not only via the 401/403 retry. + """ + env_profile: dict[str, Any] | None = _safe_active_env_profile() + if env_profile and env_profile.get("api_key"): + return API_KEY_SOURCE_ENV_JSON + if os.environ.get("DAILYBOT_API_KEY"): + return API_KEY_SOURCE_ENV_VAR + return API_KEY_SOURCE_CONFIG if load_config().get("api_key") else None + + def _safe_active_env_profile() -> dict[str, Any] | None: """Return the active env.json profile, swallowing any error. @@ -439,9 +461,13 @@ def find_repo_profile_path(cwd: Path | None = None) -> Path | None: Returns ``None`` if no ancestor contains the file, if ``.dailybot`` exists along the path as a regular file rather than a directory, or if the file - itself is missing or non-regular. + itself is missing or non-regular. A stale working directory (deleted + while a shell was still inside it) resolves to ``None``. """ - start: Path = (cwd or Path.cwd()).resolve() + try: + start: Path = (cwd or Path.cwd()).resolve() + except OSError: + return None for ancestor in [start, *start.parents]: candidate_dir: Path = ancestor / REPO_PROFILE_DIRNAME if not candidate_dir.is_dir(): @@ -766,8 +792,13 @@ def find_repo_env_path(cwd: Path | None = None) -> Path | None: Returns ``None`` when no ancestor contains the file, or when the file is non-regular. Mirrors the semantics of :func:`find_repo_profile_path`. + A stale working directory (deleted while a shell was still inside it) + resolves to ``None`` rather than crashing — there is no tree to walk. """ - start: Path = (cwd or Path.cwd()).resolve() + try: + start: Path = (cwd or Path.cwd()).resolve() + except OSError: + return None for ancestor in [start, *start.parents]: candidate_dir: Path = ancestor / REPO_PROFILE_DIRNAME if not candidate_dir.is_dir(): @@ -794,6 +825,16 @@ def _is_env_tracked_by_git(env_path: Path) -> bool: import subprocess if not shutil.which("git"): + # A checkout without the git binary can still carry a tracked + # env.json (e.g. a repo mounted into a slim container). The guard + # cannot verify, so it degrades to a loud warning instead of a + # silent pass. + if any((ancestor / ".git").exists() for ancestor in env_path.parent.parents): + _warn_env_once( + f"no-git:{env_path}", + f"git is not on PATH; cannot verify that {env_path} is untracked. " + "Ensure .dailybot/env.json is gitignored — it contains API keys.", + ) return False try: @@ -931,7 +972,16 @@ def load_repo_env(cwd: Path | None = None) -> dict[str, Any] | None: active: str | None = active_raw if isinstance(active_raw, str) and active_raw else None disabled_raw: Any = data.get("disabled", False) - disabled: bool = bool(disabled_raw) if isinstance(disabled_raw, bool) else False + disabled: bool = disabled_raw if isinstance(disabled_raw, bool) else False + if "disabled" in data and not isinstance(disabled_raw, bool): + # A hand-edited `"disabled": "true"` (string) must not silently keep + # the file active while the developer believes they turned it off. + _warn_env_once( + f"disabled-shape:{path}", + f"{path} 'disabled' must be a JSON boolean (got {disabled_raw!r}); " + "treating it as false — the file stays ACTIVE. " + "Use `dailybot env off` to disable it.", + ) return { "active": active, @@ -1025,7 +1075,13 @@ def save_repo_env(payload: dict[str, Any], *, cwd: Path | None = None) -> Path: env_dir: Path = repo_root / REPO_PROFILE_DIRNAME env_dir.mkdir(parents=True, exist_ok=True) env_path: Path = env_dir / REPO_ENV_FILENAME - env_path.write_text(json.dumps(_normalize_env_payload(payload), indent=2) + "\n") + serialized: str = json.dumps(_normalize_env_payload(payload), indent=2) + "\n" + # Create with 0o600 from the very first byte — a plain write-then-chmod + # leaves a umask-permission window while the file already holds API keys. + fd: int = os.open(env_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w") as handle: + handle.write(serialized) + # A pre-existing file keeps its old mode through os.open — tighten it. os.chmod(env_path, 0o600) return env_path diff --git a/dailybot_cli/display.py b/dailybot_cli/display.py index 1af6a4e..ded7039 100644 --- a/dailybot_cli/display.py +++ b/dailybot_cli/display.py @@ -15,6 +15,7 @@ from rich.text import Text from dailybot_cli.api_client import resource_uuid +from dailybot_cli.config import get_api_url, get_app_url console: Console = Console() error_console: Console = Console(stderr=True) @@ -549,12 +550,15 @@ def print_env_profile( table.add_row("API key", mask(str(api_key)) if api_key else "[dim]—[/dim]") table.add_row( "API URL", - str(profile.get("api_url", "")) or "[dim](default)[/dim]", + str(profile.get("api_url", "")) or f"[dim]{get_api_url()} (default)[/dim]", ) table.add_row( "Webapp URL", - str(profile.get("app_url", "")) or "[dim](default)[/dim]", + str(profile.get("app_url", "")) or f"[dim]{get_app_url()} (default)[/dim]", ) + # Explicit reassurance: the file is enabled. (`disabled: false` is + # normalized away on write, so the row is the only visible signal.) + table.add_row("Disabled", "[dim]no[/dim]") table.add_row("Source", str(path)) console.print(table) diff --git a/dailybot_cli/main.py b/dailybot_cli/main.py index 7daa1e2..6954a6c 100644 --- a/dailybot_cli/main.py +++ b/dailybot_cli/main.py @@ -87,11 +87,18 @@ def cli(ctx: click.Context, api_url: str | None, app_url: str | None) -> None: # gitignore + guard combo is meant to prevent. Blocking here is the # correct security posture: force the user to `git rm --cached` (and # rotate the exposed key) before the CLI does anything else. + # + # Single exemption: the `hook` group. Its contract (docs/AGENT_HOOKS.md) + # is "always exit 0, never break the developer's agent harness, never + # call the network" — hooks never consume env.json auth, so the guard + # degrades to a stderr warning there instead of aborting every agent + # session in the repo. try: load_repo_env() except RepoEnvError as exc: print_error(str(exc)) - raise SystemExit(1) from exc + if ctx.invoked_subcommand != "hook": + raise SystemExit(1) from exc if api_url: set_api_url_override(api_url) diff --git a/docs/AGENT_HOOKS.md b/docs/AGENT_HOOKS.md index 946243b..6024063 100644 --- a/docs/AGENT_HOOKS.md +++ b/docs/AGENT_HOOKS.md @@ -288,7 +288,10 @@ prohibition still applies. path is local file reads plus at most two git subprocesses (5 s timeout). 2. **Always exit 0.** Every subcommand catches all internal exceptions and degrades to silence — a broken ledger must never break a developer's - agent session. + agent session. This holds even under the fatal `.dailybot/env.json` + refuse-if-tracked guard: the root `cli()` callback exempts the `hook` + group (the error still prints to stderr, but the hook runs and exits 0), + because hooks are local-only and never consume env.json auth. 3. **Machine output bypasses `display.py`.** Like `_print_org_list`, hook commands emit raw JSON/plain lines via `click.echo` because the consumer is a harness parsing stdout (see AGENTS.md rule 9). diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 014626c..51106e2 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -51,13 +51,14 @@ Introduced in CLI `>= 3.7.0`. > > The file stores API keys in plain text. Once committed to a repo — public or private — the keys are considered leaked and MUST be rotated. Git history is forever; a `git revert` does not undo the exposure. > -> The CLI enforces this rule with **three independent layers of protection** — all of them must be in place, and any of them tripping is treated as a security incident, not a warning: +> The CLI enforces this rule with **four independent protections (three enforced + one advisory)** — all of them must be in place, and any of them tripping is treated as a security incident, not a warning: > > 1. **Gitignore rule (mandatory).** The repo's `.gitignore` MUST contain `.dailybot/*` **without** ever un-ignoring `env.json`. Only `profile.json` may be excepted. This repo's [`.gitignore`](../.gitignore) is the reference implementation. -> 2. **`0o600` file permissions.** Every load and every write via `dailybot env` re-chmods the file to owner-only. Even shared workstations cannot leak the file laterally. -> 3. **Fatal refuse-if-tracked guard.** The root `cli()` callback calls `load_repo_env()` on every invocation. If `.dailybot/env.json` exists AND `git ls-files --error-unmatch .dailybot/env.json` returns 0 (i.e. the file is tracked), the CLI **immediately refuses to run any command** — `status`, `user list`, `form list`, `agent update`, `env show`, everything — and prints the exact `git rm --cached` recipe. There is no partial degradation, no "warning + continue" path, no silent fallback to global auth. The user must uncommit the file before the CLI does anything else. `--help` and `--version` are exempt (Click short-circuits them before the callback) so the user can always read instructions. +> 2. **`0o600` file permissions.** Every write via `dailybot env` creates the file with mode `0o600` from the first byte (`os.open(..., 0o600)` — no umask window), and every load re-chmods it defensively. Even shared workstations cannot leak the file laterally. +> 3. **Fatal refuse-if-tracked guard.** The root `cli()` callback calls `load_repo_env()` on every invocation. If `.dailybot/env.json` exists AND `git ls-files --error-unmatch .dailybot/env.json` returns 0 (i.e. the file is tracked — staged counts too, a commit is not required), the CLI **immediately refuses to run any command** — `status`, `user list`, `form list`, `agent update`, `env show`, everything — and prints the exact `git rm --cached` recipe. There is no partial degradation, no silent fallback to global auth. The user must untrack the file before the CLI does anything else. Exactly two carve-outs: `--help` / `--version` (Click short-circuits them before the callback) so the user can always read instructions, and the `hook` group, which prints the same error to stderr but exits 0 — its contract ([docs/AGENT_HOOKS.md](AGENT_HOOKS.md)) is "always exit 0, never break the agent harness", and hooks never consume env.json auth. If git is not on PATH but a `.git` directory exists in an ancestor, the guard cannot verify tracking and degrades to a loud warning. +> 4. **Write-time gitignore warning (advisory).** `dailybot env add` runs `git check-ignore` after writing and warns on stderr when the file is not covered by any ignore rule. > -> **If any of these three layers appears to be missing or misbehaving on your machine, treat it as a bug and report it — don't work around it.** +> **If any of these protections appears to be missing or misbehaving on your machine, treat it as a bug and report it — don't work around it.** > > **If you have already committed `env.json`**, follow these steps in order: > @@ -149,7 +150,7 @@ Cross-referenced with the top-of-section STOP block. Repeated here so this appea git commit -m 'chore: untrack .dailybot/env.json' The CLI refuses to load env.json while it is tracked. ``` - **No command bypasses this** — `dailybot version`, `dailybot upgrade`, `dailybot uninstall`, `dailybot login`, `dailybot logout`, everything is blocked. Only `dailybot --help` and `dailybot --version` (Click short-circuits) still work so the developer can read instructions. This is the third and final layer that guarantees the CLI cannot silently degrade to global auth while `env.json` (and the keys it contains) leaks in git history. + **No auth-consuming command bypasses this** — `dailybot version`, `dailybot upgrade`, `dailybot uninstall`, `dailybot login`, `dailybot logout`, everything is blocked. Only `dailybot --help` / `dailybot --version` (Click short-circuits) still work so the developer can read instructions, and `dailybot hook *` prints the same error to stderr but exits 0 to honor its always-exit-0 harness contract (hooks are local-only and never consume env.json auth). This is the third and final layer that guarantees the CLI cannot silently degrade to global auth while `env.json` (and the keys it contains) leaks in git history. 4. **Masked in all output.** `dailybot env show`, `dailybot env list`, and `dailybot agent profiles --resolve` all mask API keys as `abcd****` (first 4 chars + `****`), matching the pattern used by `dailybot config key`. Full keys never appear in logs, stderr, error traces, or telemetry. 5. **No key export.** There is intentionally no `dailybot env export` or `dailybot env cat` command that would print the raw keys to stdout. Editing the file requires opening it in a text editor (which triggers the developer's own security awareness). @@ -161,6 +162,11 @@ The full order for **`api_key`**: 2. `DAILYBOT_API_KEY` env var 3. `config.json::api_key` (from `dailybot config key=...`) +Two refinements to keep the whole story honest: + +- **Wire preference.** When the key resolves from `env.json` (layer 1), the HTTP client sends `X-API-KEY` on the **first** attempt even if a Bearer login session also exists — see "Interaction with the login Bearer token" below. Keys from layers 2–3 keep the long-standing Bearer-first order (backward compatible). +- **`agent *` commands.** A keyed `agents.json` profile selected with an explicit `--profile` flag supplies its own key and beats `env.json` (a CLI flag is the highest layer). The same profile resolved implicitly — via `profile.json::profile` or as the `agents.json` default — **yields to `env.json`**. `dailybot agent profiles --resolve` always shows exactly what will be sent. + The full order for **`api_url`**: 1. `--api-url` CLI flag @@ -176,6 +182,8 @@ The full order for **`app_url`**: 3. `DAILYBOT_APP_URL` env var 4. `DEFAULT_APP_URL` (`https://app.dailybot.com`) +> `app_url` is **informational**: it tells you (via `dailybot env show` and `agent profiles --resolve`) which webapp the current context points at. Links printed after actions (e.g. the `View:` URL of a submitted report) come from the **server response**, so they already match the server the request went to. + When `env.json::disabled` is `true` or `active` is empty/null/unknown, the file is transparently skipped and every resolver behaves as if the file didn't exist. ### Interaction with `profile.json` @@ -195,29 +203,26 @@ When an `env.json` active profile provides an `api_key` **and** a login Bearer t The mechanics: -1. **`_headers()` still prefers the Bearer token** on the first attempt (preserves backward compat — every existing single-org flow is unchanged). -2. **On a 401 or 403 response**, the client's `_request()` / `_agent_request()` helpers automatically retry the same call **once** with the alternative credential (the `env.json` API key). The retry is invisible to the caller — it happens inside the HTTP layer, not in each command. +1. **The env.json key goes FIRST.** When the resolved API key comes from `.dailybot/env.json`, `_headers()` / `_agent_headers()` send `X-API-KEY` on the **first** attempt (`DailyBotClient._prefer_api_key`, auto-detected from the key's provenance via `get_api_key_source()`). This is what makes "env.json overrides the login Bearer session" literally true: the per-repo key wins even when the Bearer would have been accepted by the target server (same-server, different-org setups), and the global session token is never transmitted to whatever server the repo's env.json points at. Keys resolved from `DAILYBOT_API_KEY` / `config.json` keep the long-standing Bearer-first order — every pre-env.json flow is unchanged. +2. **On a 401 or 403 response**, the client's `_request()` / `_agent_request()` helpers automatically retry the same call **once** with the alternative credential (in either direction — a stale env.json key falls back to the Bearer, and a stale Bearer falls back to an API key). The retry is invisible to the caller — it happens inside the HTTP layer, not in each command. 3. **`status --auth` inspects `_agent_auth_mode`** after the call returns to report which credential *actually* succeeded on the wire, so the UX is honest about the effective auth path. -Why retry on 403 too? Django/DRF frequently returns 403 instead of 401 for rejected credentials (see [DRF docs — "If not authenticated, 403"](https://www.django-rest-framework.org/api-guide/authentication/#unauthorized-and-forbidden-responses)). Retrying on only 401 misses this common local-Django case entirely — which is precisely the case `env.json` was designed to fix. +Why retry on 403 too? Django/DRF frequently returns 403 instead of 401 for rejected credentials (see [DRF docs — "If not authenticated, 403"](https://www.django-rest-framework.org/api-guide/authentication/#unauthorized-and-forbidden-responses)). Retrying on only 401 misses this common local-Django case entirely. Concrete example. You are logged in with `dailybot login` against production, and you `cd` into a repo that has `.dailybot/env.json` with an active `local-admin` profile pointing at `http://localhost:8000`: ``` + client.auth_status() | - | Attempt 1: Bearer -> http://localhost:8000 - | 403 Forbidden (Bearer unknown to local API) - | - | Attempt 2: X-API-KEY -> http://localhost:8000 - | 200 OK + | Attempt 1: X-API-KEY -> http://localhost:8000 + | 200 OK (prod Bearer never leaves the machine) | + returns { user, organization, ... } from LOCAL org ``` -`dailybot status --auth` then prints `Authenticated via API key` (not "login (OTP)") because that is what actually worked. `dailybot user list`, `dailybot form list`, `dailybot kudos give`, etc. all follow the exact same path — they never hit the "you must log in again" wall when `env.json` is providing valid credentials for a different API URL. +`dailybot status --auth` then prints `Authenticated via API key` (not "login (OTP)") because that is what is on the wire. `dailybot user list`, `dailybot form list`, `dailybot kudos give`, etc. all follow the exact same path — one round-trip, correct identity, no "you must log in again" wall. If the env.json key is ever stale, the 401/403 retry silently falls back to the Bearer, and `status --auth` reports that honestly too. -The retry costs at most one extra round-trip, only on the first request against a new server, and is completely silent to the user. The client's `_agent_auth_mode` attribute is used only by `dailybot status --auth` to describe which credential succeeded; no other command needs to care. +One extra guardrail: **`dailybot login` warns when an active env.json profile is redirecting it.** Login persists the resolved `api_url` (and the token issued by that server) into the GLOBAL `~/.config/dailybot/credentials.json`, so logging in from inside such a repo would repoint every other repo's session. The warning names the profile and the server and suggests `dailybot env off` first; the login itself still proceeds. For a bulletproof "different org per repo" story, prefer profiles with distinct `api_url`s (which is the whole point of `env.json`). `dailybot logout` remains available if a developer wants to eliminate the Bearer entirely. diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 7b159ee..a879eec 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -41,8 +41,8 @@ Files without secrets (still written `0o600` for consistency): The `env.json` file is the ONLY sanctioned place inside `.dailybot/` where API keys may live. It carries per-repo credential context (API key + optional URLs for one or more environments). Because it sits inside the repo tree, **four independent protections** apply beyond the standard `0o600`: 1. **Gitignore is mandatory (`.gitignore`).** The broad `.dailybot/*` rule in the repo's `.gitignore` covers it automatically; the only excepted file is `!.dailybot/profile.json`. `env.json` MUST NEVER be excepted — not with a per-machine dot-file trick, not with a `git update-index --assume-unchanged`, not with anything. -2. **File permissions (`0o600`).** Every read and every write via `dailybot env` re-chmods the file to owner-only. Implementation: `dailybot_cli/config.py::save_repo_env`. -3. **Root-callback refuse-if-tracked guard (fatal, applies to EVERY command).** The root `cli()` callback in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation, which internally runs `git ls-files --error-unmatch .dailybot/env.json`. If the file is tracked, `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts **before any subcommand runs**. There is no silent fallback to global auth — the entire process refuses to operate until the developer runs `git rm --cached .dailybot/env.json`. The only exempt paths are `--help` and `--version` (Click short-circuits) so the developer can always read instructions. Implementation: `dailybot_cli/config.py::_is_env_tracked_by_git` (independently mockable), invoked via `load_repo_env` from `main.py::cli`. +2. **File permissions (`0o600`).** Every write via `dailybot env` creates the file with mode `0o600` from the first byte (`os.open(..., 0o600)` — no umask window), and every read re-chmods it defensively. Implementation: `dailybot_cli/config.py::save_repo_env`. +3. **Root-callback refuse-if-tracked guard (fatal, applies to EVERY command).** The root `cli()` callback in `dailybot_cli/main.py` calls `load_repo_env()` on every invocation, which internally runs `git ls-files --error-unmatch .dailybot/env.json`. If the file is tracked, `RepoEnvError` is raised, `print_error()` writes to stderr, and `SystemExit(1)` aborts **before any subcommand runs**. There is no silent fallback to global auth — the entire process refuses to operate until the developer runs `git rm --cached .dailybot/env.json`. The exempt paths are `--help` / `--version` (Click short-circuits) so the developer can always read instructions, and the `hook` group, which prints the same error to stderr but exits 0 — its harness contract ([AGENT_HOOKS.md](AGENT_HOOKS.md)) forbids non-zero exits, and hook commands never consume env.json auth. When git is not on PATH but a `.git` ancestor exists, the guard cannot verify tracking and degrades to a loud warning instead of a silent pass. Implementation: `dailybot_cli/config.py::_is_env_tracked_by_git` (independently mockable), invoked via `load_repo_env` from `main.py::cli`. 4. **Write-time gitignore warning.** `dailybot env add` runs `git check-ignore --quiet .dailybot/env.json` after writing; if the file is NOT covered by any ignore rule, a warning fires on stderr with the exact `.gitignore` snippet to add. The warning is non-fatal because a fresh repo might not have a `.gitignore` yet, and the load-time guard (#3) catches the actual security violation. **All four protections must trip together** for a leak to happen: the developer would have to (a) remove or fail to add the `.gitignore` rule, (b) survive the write-time warning, (c) survive the load-time refuse-if-tracked check, and (d) somehow bypass the file permissions. The design is defense-in-depth on purpose. diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 597f833..6f079a2 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -1089,9 +1089,8 @@ def test_bearer_rejected_retries_with_api_key(self) -> None: assert retry_headers.get("X-API-KEY") == "valid-key" assert "Authorization" not in retry_headers - def test_api_key_rejected_retries_with_bearer(self) -> None: - """API-key-only client (no Bearer) fails → no retry. But if Bearer - is added later (e.g. only api_key set at init), retry would work.""" + def test_api_key_only_no_alt_no_retry(self) -> None: + """API-key-only client (no Bearer): no alternative → single request.""" client = DailyBotClient(api_url="http://test.com", token=None, api_key="stale-key") rejected: MagicMock = MagicMock(spec=httpx.Response) rejected.status_code = 401 @@ -1106,6 +1105,53 @@ def test_api_key_rejected_retries_with_bearer(self) -> None: assert exc_info.value.status_code == 401 assert mock_req.call_count == 1 + def test_api_key_rejected_retries_with_bearer(self) -> None: + """The reverse direction: a preferred (env.json) API key fails → + retry with the Bearer session.""" + client = DailyBotClient( + api_url="http://test.com", + token="valid-bearer", + api_key="stale-env-key", + prefer_api_key=True, + ) + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 401 + rejected.json.return_value = {"detail": "API Key Not Valid"} + + success: MagicMock = MagicMock(spec=httpx.Response) + success.status_code = 200 + success.json.return_value = {"id": 3, "uuid": "rev"} + + with patch( + "dailybot_cli.api_client.httpx.request", side_effect=[rejected, success] + ) as mock_req: + result: dict[str, Any] = client.submit_agent_report(agent_name="Test", content="Hi") + + assert result == {"id": 3, "uuid": "rev"} + assert mock_req.call_count == 2 + first_headers: dict[str, str] = mock_req.call_args_list[0][1]["headers"] + assert first_headers.get("X-API-KEY") == "stale-env-key" + assert "Authorization" not in first_headers + retry_headers: dict[str, str] = mock_req.call_args_list[1][1]["headers"] + assert retry_headers.get("Authorization") == "Bearer valid-bearer" + assert "X-API-KEY" not in retry_headers + + def test_retry_does_not_loop_when_both_credentials_fail(self) -> None: + """Double rejection: exactly one retry, then the APIError surfaces.""" + client = DailyBotClient(api_url="http://test.com", token="bad-tok", api_key="bad-key") + rejected: MagicMock = MagicMock(spec=httpx.Response) + rejected.status_code = 401 + rejected.json.return_value = {"detail": "Unauthorized"} + + with ( + patch("dailybot_cli.api_client.httpx.request", return_value=rejected) as mock_req, + pytest.raises(APIError) as exc_info, + ): + client.submit_agent_report(agent_name="Test", content="Hi") + + assert exc_info.value.status_code == 401 + assert mock_req.call_count == 2 + def test_403_triggers_retry_with_api_key(self) -> None: """403 (auth-not-provided) also retries with the alt credential. @@ -1325,6 +1371,112 @@ def test_login_endpoints_never_retry(self) -> None: assert mock_post.call_count == 1 + with ( + patch("dailybot_cli.api_client.httpx.post", return_value=rejected) as mock_post, + pytest.raises(APIError), + ): + client.verify_code("me@example.com", "123456") + + assert mock_post.call_count == 1 + + with ( + patch("dailybot_cli.api_client.httpx.post", return_value=rejected) as mock_post, + pytest.raises(APIError), + ): + client.logout() + + assert mock_post.call_count == 1 + + +class TestEnvJsonWirePreference: + """A key resolved from `.dailybot/env.json` must win on the FIRST attempt. + + Bearer-first is the long-standing default, but an env.json key expresses + per-repo intent: if the Bearer went first and the target server accepted + it, the CLI would silently operate as the wrong identity AND transmit the + global session token to whatever server the repo's env.json points at. + `prefer_api_key` (auto-detected from the key's provenance) inverts the + header priority; the 401/403 retry still covers the reverse direction. + """ + + def test_headers_prefer_api_key_when_flagged(self) -> None: + client = DailyBotClient(token="prod-bearer", api_key="env-key", prefer_api_key=True) + headers = client._headers() + assert headers["X-API-KEY"] == "env-key" + assert "Authorization" not in headers + assert client._agent_auth_mode == "api_key" + + def test_agent_headers_prefer_api_key_when_flagged(self) -> None: + client = DailyBotClient(token="prod-bearer", api_key="env-key", prefer_api_key=True) + headers = client._agent_headers() + assert headers["X-API-KEY"] == "env-key" + assert "Authorization" not in headers + assert client._agent_auth_mode == "api_key" + + def test_explicit_api_key_arg_keeps_bearer_first(self) -> None: + """An explicit api_key ctor arg (e.g. a keyed --profile) keeps the + long-standing Bearer-first order.""" + client = DailyBotClient(token="tok", api_key="profile-key") + assert client._prefer_api_key is False + assert client._headers()["Authorization"] == "Bearer tok" + + @patch("dailybot_cli.api_client.get_api_key_source", return_value="env.json") + @patch("dailybot_cli.api_client.get_api_key", return_value="env-key") + @patch("dailybot_cli.api_client.get_token", return_value="prod-bearer") + def test_auto_detects_env_json_provenance( + self, _tok: MagicMock, _key: MagicMock, _src: MagicMock + ) -> None: + """A zero-arg client whose key resolves from env.json auto-prefers it.""" + client = DailyBotClient() + assert client._prefer_api_key is True + headers = client._headers() + assert headers["X-API-KEY"] == "env-key" + assert "Authorization" not in headers + + @patch("dailybot_cli.api_client.get_api_key_source", return_value="env") + @patch("dailybot_cli.api_client.get_api_key", return_value="var-key") + @patch("dailybot_cli.api_client.get_token", return_value="prod-bearer") + def test_env_var_key_keeps_bearer_first( + self, _tok: MagicMock, _key: MagicMock, _src: MagicMock + ) -> None: + """DAILYBOT_API_KEY / config.json keys keep the historical order.""" + client = DailyBotClient() + assert client._prefer_api_key is False + assert client._headers()["Authorization"] == "Bearer prod-bearer" + + +class TestDispatchHttp: + """`_dispatch_http` routes each verb to its per-method httpx function so + long-standing per-method test patches keep working.""" + + def test_get_routes_to_httpx_get(self) -> None: + with patch("dailybot_cli.api_client.httpx.get", return_value="ok") as mock_get: + result = DailyBotClient._dispatch_http("GET", "http://x/y", timeout=1.0) + assert result == "ok" + mock_get.assert_called_once_with("http://x/y", timeout=1.0) + + def test_post_routes_to_httpx_post(self) -> None: + with patch("dailybot_cli.api_client.httpx.post", return_value="ok") as mock_post: + DailyBotClient._dispatch_http("POST", "http://x/y", json={"a": 1}) + mock_post.assert_called_once_with("http://x/y", json={"a": 1}) + + def test_patch_routes_to_httpx_patch(self) -> None: + with patch("dailybot_cli.api_client.httpx.patch", return_value="ok") as mock_patch: + DailyBotClient._dispatch_http("PATCH", "http://x/y") + mock_patch.assert_called_once_with("http://x/y") + + def test_put_routes_to_httpx_put(self) -> None: + with patch("dailybot_cli.api_client.httpx.put", return_value="ok") as mock_put: + DailyBotClient._dispatch_http("PUT", "http://x/y") + mock_put.assert_called_once_with("http://x/y") + + def test_delete_falls_through_to_httpx_request(self) -> None: + """DELETE has no dedicated branch — it must go through httpx.request + (httpx.delete rejects a json body in some supported versions).""" + with patch("dailybot_cli.api_client.httpx.request", return_value="ok") as mock_req: + DailyBotClient._dispatch_http("delete", "http://x/y", json={"a": 1}) + mock_req.assert_called_once_with("DELETE", "http://x/y", json={"a": 1}) + class TestHeadersDualAuth: def test_headers_sends_api_key_when_no_token(self) -> None: diff --git a/tests/commands_test.py b/tests/commands_test.py index 8171394..cbb517e 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -519,6 +519,58 @@ def test_login_single_org( "user@test.com", "123456", organization_id=1 ) + @patch("dailybot_cli.commands.auth.DailyBotClient") + def test_login_warns_when_env_json_redirects_the_server( + self, + mock_client_cls: MagicMock, + runner: CliRunner, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Login persists the resolved api_url into the GLOBAL credentials + file — when an active env.json profile is what points the CLI at a + different server, the user must be warned before the OTP flow.""" + repo: Path = tmp_path / "repo" + env_dir: Path = repo / ".dailybot" + env_dir.mkdir(parents=True) + (env_dir / "env.json").write_text( + json.dumps( + { + "active": "local", + "profiles": [ + { + "name": "local", + "api_key": "sk-local", + "api_url": "http://localhost:8000", + } + ], + } + ) + ) + monkeypatch.chdir(repo) + + mock_client: MagicMock = mock_client_cls.return_value + mock_client.api_url = "http://localhost:8000" + mock_client.request_code.side_effect = APIError(400, "stop here") + + result = runner.invoke(cli, ["login", "--email", "user@test.com"]) + assert "env.json" in result.output + assert "GLOBAL session" in result.output + assert "dailybot env off" in result.output + + @patch("dailybot_cli.commands.auth.DailyBotClient") + def test_login_no_warning_without_env_json( + self, + mock_client_cls: MagicMock, + runner: CliRunner, + ) -> None: + mock_client: MagicMock = mock_client_cls.return_value + mock_client.api_url = "https://api.dailybot.com" + mock_client.request_code.side_effect = APIError(400, "stop here") + + result = runner.invoke(cli, ["login", "--email", "user@test.com"]) + assert "GLOBAL session" not in result.output + @patch("dailybot_cli.commands.auth.questionary") @patch("dailybot_cli.commands.auth.DailyBotClient") @patch("dailybot_cli.commands.auth.save_credentials") diff --git a/tests/env_commands_test.py b/tests/env_commands_test.py index c3c84f1..047664c 100644 --- a/tests/env_commands_test.py +++ b/tests/env_commands_test.py @@ -224,6 +224,53 @@ def test_reports_no_active(self, runner: CliRunner, chdir_tmp: Path) -> None: assert result.exit_code == 0, result.output assert "no active" in result.output.lower() or "inactive" in result.output.lower() + def test_malformed_json_prints_actionable_message( + self, runner: CliRunner, chdir_tmp: Path + ) -> None: + """A hand-edit gone wrong must yield the path + parse error + a + clear "malformed" verdict — never a traceback.""" + from dailybot_cli.main import cli + + env_dir: Path = chdir_tmp / ".dailybot" + env_dir.mkdir() + (env_dir / "env.json").write_text("{broken json") + result = runner.invoke(cli, ["env", "show"]) + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Could not parse" in result.output + assert "malformed" in result.output.lower() + + def test_shows_disabled_no_row_when_enabled(self, runner: CliRunner, chdir_tmp: Path) -> None: + """`disabled: false` is normalized away on write, so `env show` + prints an explicit Disabled row for reassurance.""" + from dailybot_cli.main import cli + + runner.invoke(cli, ["env", "add", "--name", "live", "--key", "sk_live_x"]) + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0 + assert "Disabled" in result.output + + def test_empty_profiles_list_shows_hint(self, runner: CliRunner, chdir_tmp: Path) -> None: + """An existing file with `profiles: []` (not just a missing file) + gets the add-one hint from show/list and a clear error from use.""" + from dailybot_cli.main import cli + + env_dir: Path = chdir_tmp / ".dailybot" + env_dir.mkdir() + (env_dir / "env.json").write_text(json.dumps({"profiles": []})) + + result = runner.invoke(cli, ["env", "show"]) + assert result.exit_code == 0 + assert "no active profile" in result.output.lower() + + result = runner.invoke(cli, ["env", "list"]) + assert result.exit_code == 0 + assert "no profiles" in result.output.lower() + + result = runner.invoke(cli, ["env", "use", "ghost"]) + assert result.exit_code == 1 + combined: str = result.output + (result.stderr or "") + assert "no profile named 'ghost'" in combined.lower() + def test_no_file_message(self, runner: CliRunner, chdir_tmp: Path) -> None: from dailybot_cli.main import cli @@ -411,3 +458,21 @@ def test_root_cli_help_still_works_when_env_json_tracked( for argv in (["--help"], ["--version"]): result = runner.invoke(cli, argv) assert result.exit_code == 0, f"{argv!r} should succeed even when env.json is tracked" + + def test_hook_commands_exit_zero_when_env_json_tracked( + self, + runner: CliRunner, + chdir_tmp: Path, + ) -> None: + """The `hook` group's contract (docs/AGENT_HOOKS.md) is "always exit + 0, never break the developer's agent harness". Hooks never consume + env.json auth, so the guard degrades to a stderr warning for them + instead of aborting every agent session in the repo.""" + from dailybot_cli.main import cli + + self._stage_tracked_env_json(runner, chdir_tmp) + + result = runner.invoke(cli, ["hook", "session-start"]) + assert result.exit_code == 0, "hook commands must keep their exit-0 contract" + combined: str = result.output + (result.stderr or "") + assert "tracked" in combined.lower() # the warning still surfaces diff --git a/tests/repo_env_test.py b/tests/repo_env_test.py index 3515228..f82857f 100644 --- a/tests/repo_env_test.py +++ b/tests/repo_env_test.py @@ -80,14 +80,29 @@ def test_walk_up_from_nested_cwd(self, chdir_tmp: Path) -> None: def test_closest_ancestor_wins(self, chdir_tmp: Path) -> None: from dailybot_cli.config import find_repo_env_path - _write_env(chdir_tmp, {"profiles": []}) + outer_env: Path = _write_env(chdir_tmp, {"profiles": []}) inner: Path = chdir_tmp / "inner" inner.mkdir() - _write_env(inner, {"profiles": [{"name": "x", "api_key": "k"}]}) - assert find_repo_env_path(inner / "src") is None or ( - find_repo_env_path(inner) is not None - and find_repo_env_path(inner).parent.parent == inner # type: ignore[union-attr] - ) + inner_env: Path = _write_env(inner, {"profiles": [{"name": "x", "api_key": "k"}]}) + src: Path = inner / "src" + src.mkdir() + # From inside inner (or below), the INNER file must win, not the outer. + assert find_repo_env_path(src) == inner_env + assert find_repo_env_path(inner) == inner_env + # From the outer root, the outer file is the closest. + assert find_repo_env_path(chdir_tmp) == outer_env + + def test_stale_cwd_returns_none(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A deleted working directory resolves to None instead of crashing — + the root cli() guard calls this on every invocation.""" + from dailybot_cli import config as config_mod + + def _raise_cwd() -> Path: + raise FileNotFoundError("stale working directory") + + monkeypatch.setattr(config_mod.Path, "cwd", _raise_cwd) + assert config_mod.find_repo_env_path() is None + assert config_mod.find_repo_profile_path() is None def test_returns_none_when_missing(self, chdir_tmp: Path) -> None: from dailybot_cli.config import find_repo_env_path @@ -264,6 +279,73 @@ def test_defensive_chmod_when_file_lax(self, chdir_tmp: Path) -> None: mode: int = stat.S_IMODE(path.stat().st_mode) assert mode == 0o600 + def test_disabled_non_bool_warns_and_stays_active( + self, chdir_tmp: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """A hand-edited `"disabled": "true"` (string) is NOT a kill-switch — + the file stays active, and the developer is told loudly.""" + from dailybot_cli.config import get_active_env_profile, load_repo_env + + _write_env( + chdir_tmp, + { + "disabled": "true", + "active": "x", + "profiles": [{"name": "x", "api_key": "k"}], + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert result["disabled"] is False + active: dict[str, Any] | None = get_active_env_profile(chdir_tmp) + assert active is not None and active["name"] == "x" + captured: str = capsys.readouterr().out + assert "must be a JSON boolean" in captured + assert "ACTIVE" in captured + + def test_duplicate_profile_names_keep_first(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + { + "profiles": [ + {"name": "dup", "api_key": "first-key"}, + {"name": "dup", "api_key": "second-key"}, + ] + }, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert len(result["profiles"]) == 1 + assert result["profiles"][0]["api_key"] == "first-key" + + def test_non_dict_profile_entry_is_skipped(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + {"profiles": ["a string", {"name": "good", "api_key": "k"}, 42]}, + ) + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert [p["name"] for p in result["profiles"]] == ["good"] + + def test_warn_once_dedup_per_process( + self, chdir_tmp: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """The same warning fires once per process, not once per load.""" + from dailybot_cli.config import load_repo_env + + _write_env( + chdir_tmp, + {"profiles": [{"name": "x", "api_key": "k"}], "future_field": 1}, + ) + load_repo_env(chdir_tmp) + load_repo_env(chdir_tmp) + captured: str = capsys.readouterr().out + assert captured.count("future_field") == 1 + # --- Committed-to-git guard ------------------------------------------------- @@ -319,6 +401,48 @@ def test_no_git_at_all_is_allowed(self, chdir_tmp: Path) -> None: result: dict[str, Any] | None = load_repo_env(chdir_tmp) assert result is not None + def test_staged_but_uncommitted_env_json_raises(self, chdir_tmp: Path) -> None: + """`git ls-files` sees the index — a `git add` without a commit is + already a tracking violation and must trip the guard.""" + from dailybot_cli.config import RepoEnvError, load_repo_env + + subprocess.run(["git", "init", "-q"], cwd=chdir_tmp, check=True) + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "leaked"}]}) + subprocess.run( + ["git", "add", "-f", ".dailybot/env.json"], + cwd=chdir_tmp, + check=True, + ) + with pytest.raises(RepoEnvError): + load_repo_env(chdir_tmp) + + def test_missing_git_binary_warns_when_repo_present( + self, chdir_tmp: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """Without git on PATH the guard cannot verify tracking. Inside what + looks like a git checkout it must warn instead of passing silently.""" + from dailybot_cli.config import load_repo_env + + (chdir_tmp / ".git").mkdir() + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + with patch("shutil.which", return_value=None): + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None # degraded, not blocked + captured: str = capsys.readouterr().out + assert "cannot verify" in captured + + def test_missing_git_binary_silent_outside_repo( + self, chdir_tmp: Path, capsys: pytest.CaptureFixture[str] + ) -> None: + """No git binary AND no `.git` ancestor → nothing to warn about.""" + from dailybot_cli.config import load_repo_env + + _write_env(chdir_tmp, {"profiles": [{"name": "x", "api_key": "k"}]}) + with patch("shutil.which", return_value=None): + result: dict[str, Any] | None = load_repo_env(chdir_tmp) + assert result is not None + assert "cannot verify" not in capsys.readouterr().out + # --- get_active_env_profile ------------------------------------------------- diff --git a/tests/repo_profile_test.py b/tests/repo_profile_test.py index 3d43042..ac76712 100644 --- a/tests/repo_profile_test.py +++ b/tests/repo_profile_test.py @@ -261,6 +261,81 @@ def test_key_field_in_repo_aborts( assert exc_info.value.code == 1 +class TestResolveAgentContextEnvJson: + """env.json credentials vs. keyed agents.json profiles. + + Regression guard for the asymmetry where `agent profiles --resolve` + (via `resolve_active_profile`) showed the env.json key as the winner + while the actual command client (via `_resolve_agent_context`) used + the agents.json profile key. Display and runtime must always agree: + env.json wins, except under an explicit `--profile` flag. + """ + + def _write_env_json(self, repo_root: Path) -> None: + env_dir: Path = repo_root / ".dailybot" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "env.json").write_text( + json.dumps( + { + "active": "local", + "profiles": [{"name": "local", "api_key": "env-json-key"}], + } + ) + ) + + def test_env_json_beats_keyed_default_profile(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import save_agent_profile + + save_agent_profile("bot", "Bot", api_key="agents-json-key") + self._write_env_json(chdir_tmp) + _name, client, _meta = _resolve_agent_context(None, None) + assert client.api_key == "env-json-key" + assert client._prefer_api_key is True # wins on the wire too + + def test_explicit_profile_flag_beats_env_json(self, chdir_tmp: Path) -> None: + from dailybot_cli.config import save_agent_profile + + save_agent_profile("bot", "Bot", api_key="agents-json-key") + self._write_env_json(chdir_tmp) + _name, client, _meta = _resolve_agent_context("bot", None) + assert client.api_key == "agents-json-key" + + def test_repo_profile_slug_still_yields_to_env_json(self, chdir_tmp: Path) -> None: + """A slug pinned by profile.json is repo config, not a CLI flag — + env.json (the more specific repo-local auth file) still wins.""" + from dailybot_cli.config import save_agent_profile + + save_agent_profile("bot", "Bot", api_key="agents-json-key") + _write_repo_profile(chdir_tmp, {"profile": "bot"}) + self._write_env_json(chdir_tmp) + _name, client, _meta = _resolve_agent_context(None, None) + assert client.api_key == "env-json-key" + + def test_keyless_profile_uses_ambient_api_key( + self, chdir_tmp: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A keyless profile with DAILYBOT_API_KEY available must use it + instead of erroring out (the ambient chain includes API keys).""" + from dailybot_cli.config import save_agent_profile + + save_agent_profile("bot", "Bot") + monkeypatch.setenv("DAILYBOT_API_KEY", "ambient-key") + _name, client, _meta = _resolve_agent_context(None, None) + assert client.api_key == "ambient-key" + + def test_display_and_runtime_agree(self, chdir_tmp: Path) -> None: + """`resolve_active_profile` (display) and `_resolve_agent_context` + (runtime) must pick the same key.""" + from dailybot_cli.config import save_agent_profile + + save_agent_profile("bot", "Bot", api_key="agents-json-key") + self._write_env_json(chdir_tmp) + shown: dict[str, Any] = resolve_active_profile(None, None) + _name, client, _meta = _resolve_agent_context(None, None) + assert shown["api_key"] == client.api_key == "env-json-key" + assert shown["resolved_from"]["api_key"] == "env.json" + + # --- End-to-end CLI tests ---------------------------------------------------