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..6511b17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -270,15 +270,27 @@ 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. +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 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 `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 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 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). + +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/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/api_client.py b/dailybot_cli/api_client.py index 52eab7f..7e5d91d 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -7,9 +7,24 @@ 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) + +# 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 @@ -129,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 @@ -208,9 +242,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 +260,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 +268,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 +458,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 +521,16 @@ 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,21 +557,17 @@ 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) def create_chat_completion( @@ -490,11 +595,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 +618,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 +665,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 +678,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 +693,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 +759,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 +777,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 +817,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 +854,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 +875,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 +889,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 +902,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 +914,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 +926,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 +938,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,21 +1021,16 @@ 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) def submit_form_response( @@ -973,11 +1053,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 +1129,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 +1142,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 +1160,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 +1173,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,11 +1188,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 = 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) body: Any = response.json() @@ -1158,11 +1228,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 +1255,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 +1276,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 +1290,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 +1303,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 +1315,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,21 +1347,16 @@ 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) def get_user(self, user_uuid: str, *, include_email: bool = False) -> dict[str, Any]: @@ -1308,11 +1364,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 +1395,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 +1459,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 +1499,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 +1513,13 @@ 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/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/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/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/dailybot_cli/config.py b/dailybot_cli/config.py index 53707b5..33c8a32 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,45 @@ 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. + + ``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() @@ -384,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(): @@ -526,6 +607,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 +642,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 +731,535 @@ 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`. + A stale working directory (deleted while a shell was still inside it) + resolves to ``None`` rather than crashing — there is no tree to walk. + """ + 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(): + 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"): + # 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: + 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 = 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, + "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 + 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 + + +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..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) @@ -487,7 +488,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 +526,79 @@ 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 f"[dim]{get_api_url()} (default)[/dim]", + ) + table.add_row( + "Webapp URL", + 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) + + +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..6954a6c 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 @@ -27,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`. @@ -72,6 +79,27 @@ 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. + # + # 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)) + if ctx.invoked_subcommand != "hook": + raise SystemExit(1) from exc + if api_url: set_api_url_override(api_url) if app_url: @@ -99,6 +127,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/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 d245709..51106e2 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,199 @@ 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`. + +> ## 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 **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 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 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: +> +> 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: + +- 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 + +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: + ``` + 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. + ``` + **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). + +### 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=...`) + +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 +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`) + +> `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` + +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` **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. + +The mechanics: + +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. + +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: 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 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. + +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. + +### 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 +274,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..a879eec 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -29,12 +29,28 @@ 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, **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 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. + +**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). + ## Secrets in Output Never display, log, or echo a full secret. Always mask: diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 3decc7d..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,20 +1105,106 @@ 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_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=forbidden) as mock_req, + 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 == 403 - assert mock_req.call_count == 1 + 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. + + 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."} + + success: MagicMock = MagicMock(spec=httpx.Response) + success.status_code = 200 + success.json.return_value = {"id": 2, "uuid": "xyz"} + + 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 +1254,230 @@ 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 + + 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: """_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..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") @@ -1031,18 +1083,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 +1119,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 +1139,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") diff --git a/tests/env_commands_test.py b/tests/env_commands_test.py new file mode 100644 index 0000000..047664c --- /dev/null +++ b/tests/env_commands_test.py @@ -0,0 +1,478 @@ +"""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_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 + + 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 _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 + + 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, + ) + 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) + + 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" + + 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 new file mode 100644 index 0000000..f82857f --- /dev/null +++ b/tests/repo_env_test.py @@ -0,0 +1,1033 @@ +"""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 + + outer_env: Path = _write_env(chdir_tmp, {"profiles": []}) + inner: Path = chdir_tmp / "inner" + inner.mkdir() + 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 + + 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 + + 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 ------------------------------------------------- + + +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 + + 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 ------------------------------------------------- + + +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 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 ---------------------------------------------------