From f23c6676fe51e3daa56a5b8a4df5004e4ccee3d2 Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:48 +0800 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=93=90=20docs(adr):=20ADR-011=20local?= =?UTF-8?q?=20mode=20&=20credential=20source=20abstraction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/www/sidebar.generated.json | 7 + ...r-011-local-mode-and-credential-sources.md | 268 ++++++++++++++++++ .../src/content/docs/architecture/index.md | 1 + ...r-011-local-mode-and-credential-sources.md | 228 +++++++++++++++ .../content/docs/zh-cn/architecture/index.md | 2 + docs/architecture/README.md | 1 + docs/architecture/README.zh-CN.md | 2 + docs/src/SUMMARY.md | 2 + ...r-011-local-mode-and-credential-sources.md | 265 +++++++++++++++++ ...r-011-local-mode-and-credential-sources.md | 225 +++++++++++++++ 10 files changed, 1001 insertions(+) create mode 100644 apps/www/src/content/docs/architecture/adr-011-local-mode-and-credential-sources.md create mode 100644 apps/www/src/content/docs/zh-cn/architecture/adr-011-local-mode-and-credential-sources.md create mode 100644 docs/src/architecture/adr-011-local-mode-and-credential-sources.md create mode 100644 docs/src/zh-CN/architecture/adr-011-local-mode-and-credential-sources.md diff --git a/apps/www/sidebar.generated.json b/apps/www/sidebar.generated.json index 13c2710d..a2bf0c0e 100644 --- a/apps/www/sidebar.generated.json +++ b/apps/www/sidebar.generated.json @@ -200,6 +200,13 @@ { "label": "ADR-007: Phase C design alignment", "slug": "architecture/adr-007-phase-c-design-alignment" + }, + { + "label": "ADR-011: Local Mode and Credential Source Abstraction", + "slug": "architecture/adr-011-local-mode-and-credential-sources", + "translations": { + "zh-CN": "ADR-011：本地模式与凭据来源抽象" + } } ] } diff --git a/apps/www/src/content/docs/architecture/adr-011-local-mode-and-credential-sources.md b/apps/www/src/content/docs/architecture/adr-011-local-mode-and-credential-sources.md new file mode 100644 index 00000000..1f0efc24 --- /dev/null +++ b/apps/www/src/content/docs/architecture/adr-011-local-mode-and-credential-sources.md @@ -0,0 +1,268 @@ +--- +title: "ADR-011: Local Mode and Credential Source Abstraction" +description: "Status: Accepted Date: 2026-05-24 Scope: oversight-vault, oversight-authz, oversight-server (assembly, credentials resolver, agent discovery), oversight-worker (secret materialization, credential…" +--- + +**Status:** Accepted +**Date:** 2026-05-24 +**Scope:** `oversight-vault`, `oversight-authz`, `oversight-server` (assembly, credentials resolver, agent discovery), `oversight-worker` (secret materialization, credential setup), `oversight-agents` (discovery), `web/` (mode-aware UI) +**Related:** ADR-001 (Remote Agent Worker), ADR-005 (Unified Credential ↔ Agent Wiring), ADR-006 (Workspace × Team × Project) + +## Context + +Oversight today ships as a multi-tenant server that requires OAuth/OIDC login, workspace setup, and explicit credential entry before an agent can run. The runtime is correct and complete for that deployment model — but the same code base also needs to serve a different audience: + +A single developer on their own machine who already has `claude`, `codex`, or `openclaw` installed and authenticated, who wants to start Oversight, see a workspace, and dispatch an agent — with zero login, zero key entry, zero setup. This is the **local mode** that this ADR introduces. + +Two distinct problems block that experience: + +1. **Authentication and authorization are enforced for every request.** `auth_mw.rs` extracts a bearer token; every handler calls `ensure_authorized(&principal, …)`. The only escape hatch is `OVERSIGHT_DISABLE_AUTH=1` (loopback only), which is a dev-time flag, not a product mode. +2. **Every credential must live in the vault.** ADR-005 unified the resolver around `vault.get_credential` / `vault.pool().auto_select_for_adapter`. The resolver has no path for "the CLI already has its own config on disk — don't touch it." It also offers no way to source secrets from the worker's environment or an external secret manager. + +A naïve fix would gate both layers on an `OVERSIGHT_MODE=local` flag and branch inside handlers. That works for the immediate symptom but reintroduces the multi-path drift ADR-005 explicitly fixed, and it doesn't generalize: per-user keys, BYO-config in cloud, CI environment injection, and external secret managers are all variants of the same question — *where does the secret come from for this dispatch?* + +This ADR generalizes that question. + +## Decision + +**Two orthogonal abstractions, shared by cloud and local deployments:** + +1. **A `CredentialSource` enum** carried by `ResolvedCredential`. Each source produces the same downstream `Vec` contract, so adapters and workers are source-agnostic. +2. **Pluggable authentication and authorization backends** in `oversight-server` and `oversight-authz`. The cloud assembly wires OAuth + grant-based authorization (unchanged). The local assembly wires `LocalAuthLayer` (injects a fixed principal) + `AllowAllBackend` (every check passes). Handlers do not change. + +Local mode is the *first consumer* of both abstractions; it is not a special-case code path. + +### Credential sources + +```rust +// crates/oversight-vault/src/manifest.rs +pub enum CredentialSource { + /// Encrypted in the Oversight vault. Server resolves and materializes + /// secrets at dispatch time. Existing behavior; default for cloud. + Vault { namespace: String, profile_id: String }, + + /// The CLI on the worker host owns its own credentials. Oversight only + /// stores binary path + metadata. No secret transits Oversight. + HostNative { adapter_kind: String }, + + /// The worker reads named environment variables from its own process + /// environment at dispatch time. Server never sees the value. + EnvPassthrough { env_map: BTreeMap }, + + /// External resolver (1Password CLI, sops, Vault, etc.). Reserved + /// stub in S1; resolves "not implemented" until a provider lands. + ExternalRef { provider: String, path: String }, +} +``` + +Each variant produces a `ResolvedCredential { source, secrets, profile_id, namespace }` where: + +| Source | `secrets` | `setup_command` | Who rotates | +|------------------|----------------------------------------------------|-----------------|---------------------------------| +| `Vault` | `SecretMaterial::Env { name, value }` (decrypted) | Run per manifest | Oversight admin | +| `HostNative` | empty | **Skipped** | User (`claude login`, etc.) | +| `EnvPassthrough` | `SecretMaterial::EnvFromHost { name }` (no value) | Skipped | Operator (CI / docker compose) | +| `ExternalRef` | resolved by worker plugin | Per provider | External secret manager | + +`HostNative` is the critical asymmetry: **Oversight does not read the credential file**. The CLI itself authenticates via its own on-disk config when `exec`'d. A separate read-only **inspection helper** can display login status in the UI ("Logged in as user@example.com — from `~/.claude/auth.json`"), but inspection output never enters the dispatch path. + +### Source selection (two-tier, additive to ADR-005) + +ADR-005 introduced `agent_definitions.credential_id` as a vault pin. We extend the column's prefix vocabulary to encode source: + +| `credential_id` value | Meaning | +|---------------------------------------------|----------------------------------------------------| +| `vault::` | Vault pin (existing semantics, made explicit) | +| `host-native:` | Use the CLI's own config (no server-side secret) | +| `env:` | Worker env passthrough | +| `ext::` | External resolver (stub) | +| *NULL* | Fall back to per-deployment `default_credential_source`, then to ADR-005 auto-pool | + +A new `default_credential_source` configuration (per adapter kind) is consulted when `credential_id` is NULL. Cloud assembly leaves it at the existing pool auto-pick. Local assembly sets `host-native` for all CLI adapter kinds. + +Backwards compatibility: existing `agent_definitions` rows where `credential_id` lacks a known prefix are read as `vault::` using the manifest-declared namespace. No data migration is forced. + +### Credential scope (orthogonal axis) + +The vault today is global: any agent can resolve any profile. We add scoping to `provider_profile_meta`: + +| column | type | notes | +|------------|--------------|------------------------------------------------| +| `scope` | TEXT NOT NULL | `global` (default) \| `workspace` \| `user` | +| `scope_id` | TEXT NULL | workspace_id or principal_id; NULL for global | + +Resolver filters: `pool.eligible_for(principal, workspace)` runs before priority ordering. Local mode passes trivially (single principal, single default workspace). Cloud deployments opt in by editing profiles; default remains `global` so existing data is unaffected. + +Scope is **not gated on local mode** — it is a general feature that local mode happens to exercise. Cloud deployments that want per-user Claude keys get it for free. + +### Authentication and authorization assembly + +`oversight-authz` already defines a backend trait used by `ensure_authorized`. We add `AllowAllBackend`: + +```rust +// crates/oversight-authz/src/builtin.rs +pub struct AllowAllBackend; +impl AuthzBackend for AllowAllBackend { + fn evaluate(&self, _principal: &Principal, _action: &Action, _scope: &Scope) + -> Decision { Decision::allow_with_reason("local mode") } +} +``` + +`oversight-server` adds `LocalAuthLayer`, a Tower middleware that replaces `require_auth` in local assembly: + +- For every request, inject `AuthContext { principal: Principal::User("local"), workspace: "default", … }`. +- No bearer token read. No session table lookup. +- Listens on loopback by default (configurable; binding `0.0.0.0` requires explicit opt-in flag, mirroring the existing `OVERSIGHT_DISABLE_AUTH` loopback guard). + +Cloud assembly stays exactly as it is. Both assemblies are selected at `main.rs` startup — there is no per-request mode check. Once the layers are wired, handlers, dispatch, and storage code paths are identical. + +### Local mode startup + +`oversight local [--dir ] [--port ]`: + +1. Open or create `/.oversight/oversight.db` (existing behavior of `oversight serve`). +2. Run agent discovery (next section). For each detected CLI, idempotently insert an `agent_definitions` row with `credential_id = "host-native:"`. Re-runs are no-ops. +3. Seed the default workspace and default project (existing `seed_defaults`). +4. Mount `LocalAuthLayer` + `AllowAllBackend`. Skip OAuth provider initialization. +5. Expose `mode: "local"` in `GET /api/auth/status`. Frontend reads this and skips Login routing. +6. Open the dashboard URL in the user's browser (best-effort; non-fatal if it fails). + +### Agent discovery + +A new module `crates/oversight-agents/src/discovery.rs`: + +- **PATH scan**: locate `claude`, `codex`, `openclaw`, `hermes` binaries via `which`-style PATH lookup. +- **Read-only inspection** (display only, never used at invocation): + - `~/.claude/auth.json` → login email, plan tier, model list + - `~/.codex/auth.json` → login email, model list + - OpenClaw install dir → version, configured providers + - Hermes config → endpoint URL, identity +- Returns `Vec`. +- A `POST /api/agents/rescan` endpoint re-runs discovery on demand. No file watching. + +If discovery finds a CLI but the inspection helper detects no login, the seeded agent row carries `credential_status = "needs-login"`. UI surfaces a banner: *"Run `claude login` in your terminal, then click Re-scan."* Dispatch still works — the CLI itself will print the same instruction on stderr if invoked unauthenticated; Oversight surfaces the error verbatim. + +### Frontend changes + +- `GET /api/auth/status` returns `{ bootstrapped, mode: "cloud" | "local", … }`. +- Router: in local mode, the `/login` and `/bootstrap` routes are not mounted. Direct navigation redirects to dashboard. +- Workspace switcher, member management, invite flow, OAuth provider settings: hidden in local mode (data model still supports them — only UI is gated). +- Agents page: each agent card shows `credential_status` badge (logged-in / needs-login / vault-pinned) and the source ("from `~/.claude/auth.json`" / "vault profile *xyz*" / "env: `ANTHROPIC_API_KEY`"). A "Re-scan host" button appears only in local mode. + +### Data model surface (migrations) + +``` +V095__provider_profile_scope.sql + ALTER TABLE provider_profile_meta ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'; + ALTER TABLE provider_profile_meta ADD COLUMN scope_id TEXT NULL; + CREATE INDEX idx_profile_meta_scope ON provider_profile_meta(scope, scope_id); +``` + +A descriptive `agent_definitions.credential_source` column was considered (the original V094 in this design) but dropped: the prefix on `credential_id` is fully sufficient and authoritative, and a denormalised column would have required round-tripping through three storage backends (SQLite, Postgres, filesystem) plus API request/response just to mirror what `derive_credential_source_kind(credential_id)` returns in one constant-time string match. Admin queries that want to group by source call the helper at read time. + +## Architecture (data flow) + +``` + ┌──────────────── credential_id ────────────────┐ + │ vault:ns:profile │ host-native:kind │ + │ env: │ ext:provider:path │ + └────────────────────┬─────────────────────────┘ + │ + resolve_credential_for_agent(def) + │ + ┌───────────────┬──────────────┼──────────────┬──────────────┐ + │ │ │ │ │ + vault.get(ns,id) host-native: env: ext: default + pool.eligible_for (no read) env_map (stub) fallback + scope filter + │ │ │ │ + ▼ ▼ ▼ ▼ + ResolvedCredential { source, secrets, profile_id, namespace } + │ + dispatch payload (unchanged) + │ + ┌───────┴───────┐ + │ │ + embedded worker remote worker + │ │ + ▼ ▼ + MaterializedSecrets → exec adapter CLI + (HostNative skips setup_command; + EnvFromHost reads worker's own env) +``` + +## Implementation phases + +| Step | Scope | Compatibility | +|-------|----------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------| +| **S1** | `CredentialSource` enum, `ResolvedCredential` extension, resolver dispatch on prefixes, `HostNative` and `EnvPassthrough` producers, worker `EnvFromHost` materializer, `HostNative` skips `setup_command`, `derive_credential_source_kind` helper. Migration V095. | Additive. NULL `credential_id` and unprefixed values resolve exactly as today. | +| **S2** | Agent discovery module + `POST /api/agents/rescan`. Inspection helpers for Claude / Codex / OpenClaw / Hermes. | Read-only on filesystem; no behavior change for cloud. | +| **S3** | `LocalAuthLayer`, `AllowAllBackend`, `oversight local` subcommand, mode exposure in `/api/auth/status`. | New subcommand; `oversight serve` unchanged. | +| **S4** | Frontend mode-aware UI: skip Login route, hide multi-workspace surfaces, agent credential badges, Re-scan button. | UI only; cloud appearance unchanged. | +| **S5** | Scope filter integration (`pool.eligible_for`) and credential scope editor in UI. Default remains `global`. | Backward-compatible; opt-in per profile. | + +Each step lands with unit tests on the introduced surface and an integration test exercising at least one cross-crate flow. + +## Consequences + +### Positive + +- **Zero-friction local UX**: `oversight local` from a project directory → workspace and detected agents immediately available; no login, no key entry. +- **Single dispatch path**: cloud and local share the same handlers, dispatcher, worker, and adapter code. No `if mode == local` branches. +- **Credential source is a first-class concept**: future requirements (per-user vault keys, BYO host config in cloud, environment-injected dispatch, external secret managers) are new producers of the same enum, not new modes. +- **Local mode never persists secrets**: `.oversight/oversight.db` from a local install contains no API keys, eliminating an entire class of accidental disclosure (e.g., committing `.oversight/` to git). +- **Vault audit / rotation / cooldown / dispatch_log** remain meaningful — they only apply to `Vault`-source dispatches, which is correct (they're not applicable to host-native CLIs). + +### Negative + +- **Two assemblies to test**: cloud and local each need integration coverage. Mitigated by sharing 95% of code; the differences are confined to assembly wiring. +- **Inspection helper drift**: if a CLI's on-disk config format changes (e.g., a new `~/.claude/auth.json` schema), our UI badge goes stale. Inspection failures are non-fatal — dispatch still works, the badge just shows "unknown". +- **`ExternalRef` is a stub**: callers who try `ext:` get a clear "not implemented" error. Acceptable for S1; full plugin design deferred until there is a concrete consumer. + +### Neutral + +- Existing `OVERSIGHT_DISABLE_AUTH=1` flag is unchanged. It remains a dev-time loopback escape hatch on `oversight serve`. `oversight local` is the supported user-facing path for "no login." +- `oversight serve` continues to require login and explicit credentials. Local mode does not change cloud behavior in any way. + +## Alternatives considered + +### A. Mode flag with handler branches + +Add `OVERSIGHT_MODE=local` and check it inside `require_auth`, inside `ensure_authorized`, inside dispatch. Rejected: reintroduces the multi-path drift ADR-005 fixed, and offers no extensibility for the other credential-source needs (per-user keys, environment injection, external managers). + +### B. Separate desktop binary / Tauri shell + +Ship `oversight-desktop` as a distinct binary, possibly with an embedded WebView. Rejected for this ADR: larger packaging effort with no proportional UX gain over `oversight local` opening a browser. The chosen design does not preclude a future desktop shell — that shell would spawn `oversight local` under the hood. + +### C. Vault-import on first run + +`oversight local` reads `~/.claude/auth.json` once at startup and writes the token into the vault. Rejected: persists secrets where the user did not consent to that, requires `.gitignore` discipline on `.oversight/`, and means `claude login` updates do not take effect until the user manually re-imports. + +### D. Skip credential resolution entirely for local CLIs + +Special-case `agent_type ∈ {claude, codex, openclaw}` to bypass the resolver when `OVERSIGHT_MODE=local`. Rejected: same drift problem as A, and breaks the read endpoint (`GET /api/agent-definitions/:id/resolved-credential`) which clients use to display *which credential will be used* — `HostNative` gives them a meaningful answer; "bypass" gives them an empty one. + +### E. Always-allow authorization without local assembly + +Skip the `oversight-authz` backend abstraction; just have the middleware decide. Rejected: authz already has a backend trait designed for replaceability; the explicit `AllowAllBackend` is a one-file change that keeps the architecture coherent. + +## Migration and rollout + +- **No schema-incompatible changes.** V095 is additive (the originally-planned V094 `credential_source` column was dropped — see "Data model surface"); cloud deployments that take the upgrade see no behavior difference until they assign scopes explicitly. +- **No data migration script required.** Existing rows continue to resolve via the NULL → auto-pool fallback path. +- **Documentation**: a separate operations note (`docs/operations/local-mode.md`) covers `oversight local` startup, the inspection helper coverage matrix, and the "Re-scan" workflow. Will be authored alongside S2. + +## Known limitations (deliberate, not bugs) + +Captured during the post-merge e2e verification against `awaken-managed-agents`: + +- **Token / cost tracking for host-native CLI runs is best-effort.** Awaken's run record (`input_tokens` / `output_tokens`) is populated by its loop runner during runtime/cloud LLM calls, but host-native CLI agents (claude / codex CLIs) do their own LLM calls and currently don't feed usage back into the awaken run. `agent_activities` rows propagate whatever totals the run record has — for CLI runs that's `0`. Vault-side `credential_usage_events` similarly aren't populated because host-native runs are explicitly *not* attributed to a vault credential (that's the whole point of the source). Surfacing token data for host-native runs requires either (a) parsing the CLI's own usage stream (codex emits ACP `UsageUpdate`; claude reports differently) and writing back to the run record, or (b) accepting that local-mode tracking is a CLI-side concern. Out of ADR-011 scope. +- **`requirement` workflow does not auto-spawn `implementation` child issues on approval.** Per ADR-007 the server is a generic runtime, not a delivery-policy engine — decomposition is the `requirement_analyzer` agent's job via `create_issue` tool calls (the seeded prompt currently does not exercise this path; prompt customization is product policy, not infrastructure). +- **Codex CLI's built-in bubblewrap sandbox fails on some hosts** (`bwrap: loopback: Failed RTM_NEWADDR`) — host environment issue, not Oversight. The codex manifest documents the `CODEX_ACP_ARGS_JSON` env workaround in its header. + +## Open questions + +1. **Hermes inspection helper**: format of Hermes's local config is not yet stable; S2 may ship Hermes as PATH-detect only (no metadata) and add inspection in a follow-up. +2. **Windows discovery**: PATH scan and home-dir conventions need a Windows codepath. Targeted for S2 once Linux/macOS land. +3. **Multi-instance on one host**: two `oversight local` instances on the same machine bind different ports and different `.oversight/` directories. Process-isolation is sufficient; no shared lock is required. +4. **Cloud opting in to `host-native`**: a cloud user pointing an agent at `host-native:claude` is supportable but unusual (the worker host must have `claude` installed and authenticated). Not gated; documented as advanced usage. diff --git a/apps/www/src/content/docs/architecture/index.md b/apps/www/src/content/docs/architecture/index.md index adb77085..e805c164 100644 --- a/apps/www/src/content/docs/architecture/index.md +++ b/apps/www/src/content/docs/architecture/index.md @@ -14,3 +14,4 @@ This section collects architecture decision records (ADRs) and longer-lived tech - [ADR-005: Unified Credential Agent Wiring](/architecture/adr-005-unified-credential-agent-wiring/) - [ADR-006: Workspace x Team x Project Domain Model](/architecture/adr-006-workspace-team-project-model/) - [ADR-007: Agent-Driven Workflow Automation](/architecture/adr-007-agent-driven-workflow-automation/) +- [ADR-011: Local Mode and Credential Source Abstraction](/architecture/adr-011-local-mode-and-credential-sources/) diff --git a/apps/www/src/content/docs/zh-cn/architecture/adr-011-local-mode-and-credential-sources.md b/apps/www/src/content/docs/zh-cn/architecture/adr-011-local-mode-and-credential-sources.md new file mode 100644 index 00000000..a7d7bb77 --- /dev/null +++ b/apps/www/src/content/docs/zh-cn/architecture/adr-011-local-mode-and-credential-sources.md @@ -0,0 +1,228 @@ +--- +title: "ADR-011：本地模式与凭据来源抽象" +description: "状态： Accepted 日期： 2026-05-24 范围： oversight-vault、oversight-authz、oversight-server（装配、credentials resolver、agent discovery）、oversight-worker（secret 物化、credential…" +--- + +**状态：** Accepted +**日期：** 2026-05-24 +**范围：** `oversight-vault`、`oversight-authz`、`oversight-server`（装配、credentials resolver、agent discovery）、`oversight-worker`（secret 物化、credential setup）、`oversight-agents`（discovery）、`web/`（mode 感知 UI） +**相关：** ADR-001（远程 Agent Worker）、ADR-005（统一凭据 ↔ Agent 装配）、ADR-006（Workspace × Team × Project） + +## 背景 + +Oversight 目前以多租户服务形态发布：每个请求需要 OAuth/OIDC 登录、需要先创建 workspace、需要显式配置凭据才能跑 agent。这对部署是正确且完整的，但同一份代码也需要服务另一类用户： + +一位本机已经装好并登录了 `claude`、`codex`、`openclaw` 的开发者，希望启动 Oversight 后立即看到 workspace 并直接派发 agent —— 零登录、零填 key、零设置。这就是本 ADR 引入的 **本地模式**。 + +阻塞这个体验的有两个明显问题： + +1. **每个请求都强制 authn/authz。** `auth_mw.rs` 解析 bearer token；每个 handler 调 `ensure_authorized(&principal, …)`。唯一的逃生通道是 `OVERSIGHT_DISABLE_AUTH=1`（仅 loopback），那是开发期 flag，不是产品模式。 +2. **所有凭据都必须放在 vault 里。** ADR-005 把 resolver 统一到 `vault.get_credential` / `vault.pool().auto_select_for_adapter`。resolver 没有"这个 CLI 自己磁盘上已经有 config 了，不要碰"的路径，也没法从 worker 环境或外部 secret manager 取值。 + +最直觉的修法是加 `OVERSIGHT_MODE=local` flag,然后在 handler 里分支。这能解眼前症状,但会重新引入 ADR-005 刚消除的多路径漂移,而且无法泛化:per-user key、云端 BYO-config、CI 环境注入、外部 secret manager,本质上都是同一个问题 —— *这次派发的 secret 从哪里来?* + +本 ADR 把这个问题抽象出来。 + +## 决策 + +**两个正交的抽象,云端与本地共用:** + +1. `ResolvedCredential` 携带的 **`CredentialSource` 枚举**。每种 source 产出相同下游 `Vec` 契约,所以 adapter/worker 对来源无感知。 +2. `oversight-server` 与 `oversight-authz` 中 **可插拔的 authn / authz backend**。云端装配走 OAuth + 基于 grant 的授权(不变)。本地装配走 `LocalAuthLayer`(注入固定 principal)+ `AllowAllBackend`(所有检查放行)。handler 代码不变。 + +本地模式是这两个抽象的 **第一个消费者**,而不是被特殊处理的代码路径。 + +### 凭据来源 + +```rust +// crates/oversight-vault/src/manifest.rs +pub enum CredentialSource { + /// 加密存在 Oversight vault。Server 在派发时解密并物化。 + /// 现有行为;云端默认。 + Vault { namespace: String, profile_id: String }, + + /// Worker 宿主机上的 CLI 自己管理凭据。Oversight 只存二进制路径 + /// 与元数据。无 secret 经过 Oversight。 + HostNative { adapter_kind: String }, + + /// Worker 在派发时从自身进程环境读取指定环境变量。Server 看不到值。 + EnvPassthrough { env_map: BTreeMap }, + + /// 外部解析器(1Password CLI、sops、Vault 等)。S1 保留 stub, + /// 在有 provider 实现之前返回"未实现"。 + ExternalRef { provider: String, path: String }, +} +``` + +每个变体产出 `ResolvedCredential { source, secrets, profile_id, namespace }`: + +| Source | `secrets` | `setup_command` | 轮换方 | +|------------------|----------------------------------------------------|-----------------|----------------------------------| +| `Vault` | `SecretMaterial::Env { name, value }`(解密后) | 按 manifest 运行 | Oversight 管理员 | +| `HostNative` | 空 | **跳过** | 用户(`claude login` 等) | +| `EnvPassthrough` | `SecretMaterial::EnvFromHost { name }`(无值) | 跳过 | 运维(CI / docker compose) | +| `ExternalRef` | 由 worker 插件解析 | 视 provider | 外部 secret manager | + +`HostNative` 的关键不对称:**Oversight 不读凭据文件**。CLI 被 `exec` 时自己读自己的磁盘 config 完成认证。一个独立的只读 **inspection helper** 可以在 UI 显示登录状态("以 user@example.com 登录 —— 来自 `~/.claude/auth.json`"),但 inspection 输出永远不进入 dispatch 路径。 + +### Source 选择(两层,叠加在 ADR-005 之上) + +ADR-005 引入了 `agent_definitions.credential_id` 作为 vault pin。我们扩展该列的前缀语义来编码 source: + +| `credential_id` 值 | 含义 | +|--------------------------------------------|----------------------------------------------------| +| `vault::` | Vault pin(显式化的现有语义) | +| `host-native:` | 使用 CLI 自身配置(server 端无 secret) | +| `env:` | Worker 环境直通 | +| `ext::` | 外部解析器(stub) | +| *NULL* | 回退到 per-deployment `default_credential_source`,再回退到 ADR-005 auto-pool | + +新增的 `default_credential_source` 配置(按 adapter kind)在 `credential_id` 为 NULL 时被查询。云端装配保持现有 pool auto-pick。本地装配把所有 CLI adapter kind 都设为 `host-native`。 + +向后兼容:已有 `agent_definitions` 行如果 `credential_id` 不带已知前缀,按 manifest 声明的 namespace 解读为 `vault::`。无需强制数据迁移。 + +### 凭据 Scope(正交维度) + +vault 今天是全局的:任何 agent 都能解析任意 profile。我们在 `provider_profile_meta` 上加 scope: + +| 列 | 类型 | 备注 | +|------------|---------------|------------------------------------------------| +| `scope` | TEXT NOT NULL | `global`(默认) \| `workspace` \| `user` | +| `scope_id` | TEXT NULL | workspace_id 或 principal_id;global 时为 NULL | + +Resolver 过滤:`pool.eligible_for(principal, workspace)` 在 priority 排序之前运行。本地模式天然通过(单 principal、单 default workspace)。云端按需开启;默认仍是 `global`,现有数据不受影响。 + +Scope **不与本地模式绑定** —— 它是通用特性,本地模式恰好用到。云端想要 per-user Claude key 也免费拿到。 + +### Authn 与 Authz 装配 + +`oversight-authz` 已有 backend trait 被 `ensure_authorized` 使用。新增 `AllowAllBackend`: + +```rust +// crates/oversight-authz/src/builtin.rs +pub struct AllowAllBackend; +impl AuthzBackend for AllowAllBackend { + fn evaluate(&self, _principal: &Principal, _action: &Action, _scope: &Scope) + -> Decision { Decision::allow_with_reason("local mode") } +} +``` + +`oversight-server` 新增 `LocalAuthLayer`,一个 Tower middleware,在本地装配中替换 `require_auth`: + +- 每个请求注入 `AuthContext { principal: Principal::User("local"), workspace: "default", … }` +- 不读 bearer token,不查 session 表 +- 默认监听 loopback(可配置;绑定 `0.0.0.0` 需要显式 opt-in flag,与现有 `OVERSIGHT_DISABLE_AUTH` loopback 守卫一致) + +云端装配完全不动。两套装配在 `main.rs` 启动时选定 —— 没有 per-request mode 检查。装配好之后,handler、dispatch、storage 路径完全相同。 + +### 本地模式启动 + +`oversight local [--dir ] [--port ]`: + +1. 打开或创建 `/.oversight/oversight.db`(`oversight serve` 的现有行为) +2. 跑 agent discovery(见下节)。对每个检测到的 CLI 幂等插入 `agent_definitions` 行,`credential_id = "host-native:"`。重复执行无副作用 +3. 种入默认 workspace 和默认 project(现有 `seed_defaults`) +4. 挂上 `LocalAuthLayer` + `AllowAllBackend`。跳过 OAuth provider 初始化 +5. 在 `GET /api/auth/status` 暴露 `mode: "local"`。前端读这个并跳过 Login 路由 +6. 在用户浏览器中打开 dashboard URL(best-effort;失败不致命) + +### Agent Discovery + +新模块 `crates/oversight-agents/src/discovery.rs`: + +- **PATH 扫描**:用 `which` 风格的 PATH 查找定位 `claude`、`codex`、`openclaw`、`hermes` 二进制 +- **只读 inspection**(仅展示,绝不参与 invocation): + - `~/.claude/auth.json` → 登录邮箱、套餐、模型列表 + - `~/.codex/auth.json` → 登录邮箱、模型列表 + - OpenClaw 安装目录 → 版本、已配置 provider + - Hermes config → endpoint URL、身份 +- 返回 `Vec` +- `POST /api/agents/rescan` endpoint 按需重跑 discovery。不做文件监听 + +若 discovery 找到 CLI 但 inspection helper 检测到未登录,seed 的 agent 行携带 `credential_status = "needs-login"`。UI 显示提示:*"请在终端运行 `claude login` 后点击 Re-scan。"* Dispatch 仍可工作 —— CLI 自身在未登录时也会在 stderr 打印同样的提示,Oversight 原样转发。 + +### 前端变更 + +- `GET /api/auth/status` 返回 `{ bootstrapped, mode: "cloud" | "local", … }` +- 路由:本地模式下 `/login`、`/bootstrap` 不挂载,直达 dashboard +- Workspace 切换器、成员管理、邀请流、OAuth provider 设置:本地模式下隐藏(数据模型仍支持 —— 只是 UI 关闭) +- Agents 页:每张卡片显示 `credential_status` 徽章(logged-in / needs-login / vault-pinned)与来源("来自 `~/.claude/auth.json`" / "vault profile *xyz*" / "env: `ANTHROPIC_API_KEY`")。"Re-scan host" 按钮仅在本地模式出现 + +### 数据模型(迁移) + +``` +V095__provider_profile_scope.sql + ALTER TABLE provider_profile_meta ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'; + ALTER TABLE provider_profile_meta ADD COLUMN scope_id TEXT NULL; + CREATE INDEX idx_profile_meta_scope ON provider_profile_meta(scope, scope_id); +``` + +本设计原计划的描述性列 `agent_definitions.credential_source`(即 V094)已被放弃:`credential_id` 上的前缀已经足够且权威,额外的去规范化列需要在三种存储后端(SQLite、Postgres、文件系统)以及 API 请求/响应间往返同步,只是为了镜像 `derive_credential_source_kind(credential_id)` 一次常量时间字符串匹配就能得到的结果。需要按来源分组的管理查询在读取时调用该 helper。 + +## 实施阶段 + +| 阶段 | 范围 | 兼容性 | +|-------|----------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------| +| **S1** | `CredentialSource` 枚举、`ResolvedCredential` 扩展、resolver 按前缀分发、`HostNative` 与 `EnvPassthrough` producer、worker `EnvFromHost` 物化、`HostNative` 跳过 `setup_command`、`derive_credential_source_kind` helper。迁移 V095。 | 加性。NULL `credential_id` 与无前缀值解析与今日完全一致。 | +| **S2** | Agent discovery 模块 + `POST /api/agents/rescan`。Claude / Codex / OpenClaw / Hermes 的 inspection helper。 | 文件系统只读;云端行为不变。 | +| **S3** | `LocalAuthLayer`、`AllowAllBackend`、`oversight local` 子命令、`/api/auth/status` 的 mode 暴露。 | 新子命令;`oversight serve` 不变。 | +| **S4** | 前端 mode 感知 UI:跳过 Login 路由、隐藏多 workspace 表面、agent 凭据徽章、Re-scan 按钮。 | 仅 UI;云端外观不变。 | +| **S5** | Scope 过滤集成(`pool.eligible_for`)和 UI 中的凭据 scope 编辑器。默认仍 `global`。 | 向后兼容;按 profile 选择启用。 | + +每阶段附带新增表面的单元测试,以及至少一个跨 crate 流程的集成测试。 + +## 后果 + +### 正面 + +- **零摩擦本地 UX**:`oversight local` 在项目目录下 → workspace 与检测到的 agent 立即可用;无登录、无填 key +- **统一派发路径**:云端与本地共享同一套 handler、dispatcher、worker、adapter。无 `if mode == local` 分支 +- **凭据来源成为一等概念**:未来需求(per-user vault key、云端 BYO 宿主 config、环境注入派发、外部 secret manager)都是同一枚举的新 producer,不是新 mode +- **本地模式永不持久化 secret**:本地安装的 `.oversight/oversight.db` 不含任何 API key,消除一整类意外泄露(如把 `.oversight/` 提交到 git) +- **Vault audit / rotation / cooldown / dispatch_log** 仍然有效 —— 它们只对 `Vault` source 的派发生效,这是正确的(对 host-native CLI 不适用) + +### 负面 + +- **两套装配需要测试**:云端与本地各需集成覆盖。共享 95% 代码减轻了此点;差异限于装配接线 +- **Inspection helper 漂移**:若 CLI 的磁盘 config 格式变更(如新版 `~/.claude/auth.json` schema),UI 徽章会过期。Inspection 失败非致命 —— dispatch 仍工作,徽章只是显示 "unknown" +- **`ExternalRef` 是 stub**:用 `ext:` 的调用方会得到清晰的 "not implemented" 错误。S1 接受;完整 plugin 设计待出现具体消费者后再做 + +### 中性 + +- 现有 `OVERSIGHT_DISABLE_AUTH=1` flag 不变,仍是 `oversight serve` 上的 dev-time loopback 逃生通道。`oversight local` 是 "无登录" 的产品级路径 +- `oversight serve` 仍要求登录与显式凭据。本地模式完全不改云端行为 + +## 已考虑的替代方案 + +### A. 带 handler 分支的 mode flag + +加 `OVERSIGHT_MODE=local`,在 `require_auth`、`ensure_authorized`、dispatch 内部 if 分支。拒绝:重新引入 ADR-005 刚消除的多路径漂移,且无法扩展到其他凭据来源需求(per-user key、环境注入、外部 manager) + +### B. 独立 desktop 二进制 / Tauri 壳 + +发布 `oversight-desktop` 作为独立二进制,可能内嵌 WebView。本 ADR 拒绝:打包成本更大且相对 `oversight local` 打开浏览器没有等比 UX 收益。当前设计不排斥未来加 desktop 壳 —— 壳在底层 spawn `oversight local` 即可 + +### C. 首次启动时 Vault 导入 + +`oversight local` 启动时读一次 `~/.claude/auth.json` 并把 token 写入 vault。拒绝:在用户未同意的位置持久化 secret,需要对 `.oversight/` 做 `.gitignore` 纪律,且 `claude login` 的更新在用户手动重新导入之前不生效 + +### D. 本地 CLI 完全跳过凭据解析 + +把 `agent_type ∈ {claude, codex, openclaw}` 在 `OVERSIGHT_MODE=local` 时特判跳过 resolver。拒绝:与 A 同样的漂移问题,且会破坏 `GET /api/agent-definitions/:id/resolved-credential` 读端点 —— 客户端依赖该端点显示"将用哪个凭据",`HostNative` 给出有意义的答复;"跳过"只能给出空答复 + +### E. 不做本地装配的"始终放行"授权 + +不做 `oversight-authz` backend 抽象,只在 middleware 决定。拒绝:authz 已有为可替换性设计的 backend trait;显式 `AllowAllBackend` 是单文件改动,保持架构一致 + +## 迁移与上线 + +- **无 schema 不兼容变更。** V095 是加性的(原计划的 V094 `credential_source` 列已放弃,见"数据模型");云端升级后不显式分配 scope,看不到任何行为差异 +- **无需数据迁移脚本。** 已有行继续走 NULL → auto-pool 的回退路径解析 +- **文档**:独立的运维说明(`docs/operations/local-mode.md`)涵盖 `oversight local` 启动、inspection helper 覆盖矩阵、"Re-scan" 工作流。与 S2 一并提交 + +## 待定问题 + +1. **Hermes inspection helper**:Hermes 本地 config 格式尚未稳定;S2 可能先只发 Hermes 的 PATH 检测(无元数据),inspection 留到后续 +2. **Windows discovery**:PATH 扫描与 home 目录约定需要 Windows 代码分支。Linux/macOS 落地后再做 +3. **同机多实例**:同一台机器跑两个 `oversight local` 实例时,绑不同端口和不同 `.oversight/` 目录。进程隔离已经足够,不需要共享锁 +4. **云端 opt-in `host-native`**:云端用户把某 agent 指向 `host-native:claude` 是可支持但少见(worker host 必须装好并登录 `claude`)。不禁止;文档列为高级用法 diff --git a/apps/www/src/content/docs/zh-cn/architecture/index.md b/apps/www/src/content/docs/zh-cn/architecture/index.md index 4566951b..5522f09b 100644 --- a/apps/www/src/content/docs/zh-cn/architecture/index.md +++ b/apps/www/src/content/docs/zh-cn/architecture/index.md @@ -13,5 +13,7 @@ description: "这里收录长期有效的架构决策记录（ADR）和技术说 - [ADR-004: Policy Engine and Accountable Role](/zh-cn/architecture/adr-004-policy-engine-and-accountable-role/) - [ADR-005: Unified Credential Agent Wiring](/zh-cn/architecture/adr-005-unified-credential-agent-wiring/) - [ADR-006: Workspace x Team x Project Domain Model](/zh-cn/architecture/adr-006-workspace-team-project-model/) +- [ADR-007: Agent 驱动的工作流自动化](/zh-cn/architecture/adr-007-agent-driven-workflow-automation/) +- [ADR-011：本地模式与凭据来源抽象](/zh-cn/architecture/adr-011-local-mode-and-credential-sources/) 说明：目前 ADR 正文仍以英文为主，这个页面仅提供中文入口。 diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 86abf426..389765b4 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -11,3 +11,4 @@ This section collects architecture decision records (ADRs) and longer-lived tech - [ADR-005: Unified Credential Agent Wiring](./adr-005-unified-credential-agent-wiring.md) - [ADR-006: Workspace x Team x Project Domain Model](./adr-006-workspace-team-project-model.md) - [ADR-007: Agent-Driven Workflow Automation](./adr-007-agent-driven-workflow-automation.md) +- [ADR-011: Local Mode and Credential Source Abstraction](./adr-011-local-mode-and-credential-sources.md) diff --git a/docs/architecture/README.zh-CN.md b/docs/architecture/README.zh-CN.md index 364206fa..2efc1cc9 100644 --- a/docs/architecture/README.zh-CN.md +++ b/docs/architecture/README.zh-CN.md @@ -10,5 +10,7 @@ - [ADR-004: Policy Engine and Accountable Role](./adr-004-policy-engine-and-accountable-role.md) - [ADR-005: Unified Credential Agent Wiring](./adr-005-unified-credential-agent-wiring.md) - [ADR-006: Workspace x Team x Project Domain Model](./adr-006-workspace-team-project-model.md) +- [ADR-007: Agent 驱动的工作流自动化](./adr-007-agent-driven-workflow-automation.md) +- [ADR-011：本地模式与凭据来源抽象](./adr-011-local-mode-and-credential-sources.md) 说明：目前 ADR 正文仍以英文为主，这个页面仅提供中文入口。 diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index fa3ffe5e..0a9dbf8d 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -42,6 +42,7 @@ - [ADR-005: Unified Credential ↔ Agent Wiring](./architecture/adr-005-unified-credential-agent-wiring.md) - [ADR-007: Agent-Driven Workflow Automation](./architecture/adr-007-agent-driven-workflow-automation.md) - [ADR-007: Phase C design alignment](./architecture/adr-007-phase-c-design-alignment.md) +- [ADR-011: Local Mode and Credential Source Abstraction](./architecture/adr-011-local-mode-and-credential-sources.md) # 中文文档 @@ -84,3 +85,4 @@ - [ADR-003: Three-backend Storage](./zh-CN/architecture/adr-003-postgres-compatibility.md) - [ADR-007: Agent-Driven Workflow Automation](./zh-CN/architecture/adr-007-agent-driven-workflow-automation.md) - [ADR-007: Phase C design alignment](./zh-CN/architecture/adr-007-phase-c-design-alignment.md) +- [ADR-011：本地模式与凭据来源抽象](./zh-CN/architecture/adr-011-local-mode-and-credential-sources.md) diff --git a/docs/src/architecture/adr-011-local-mode-and-credential-sources.md b/docs/src/architecture/adr-011-local-mode-and-credential-sources.md new file mode 100644 index 00000000..b20d9a5a --- /dev/null +++ b/docs/src/architecture/adr-011-local-mode-and-credential-sources.md @@ -0,0 +1,265 @@ +# ADR-011: Local Mode and Credential Source Abstraction + +**Status:** Accepted +**Date:** 2026-05-24 +**Scope:** `oversight-vault`, `oversight-authz`, `oversight-server` (assembly, credentials resolver, agent discovery), `oversight-worker` (secret materialization, credential setup), `oversight-agents` (discovery), `web/` (mode-aware UI) +**Related:** ADR-001 (Remote Agent Worker), ADR-005 (Unified Credential ↔ Agent Wiring), ADR-006 (Workspace × Team × Project) + +## Context + +Oversight today ships as a multi-tenant server that requires OAuth/OIDC login, workspace setup, and explicit credential entry before an agent can run. The runtime is correct and complete for that deployment model — but the same code base also needs to serve a different audience: + +A single developer on their own machine who already has `claude`, `codex`, or `openclaw` installed and authenticated, who wants to start Oversight, see a workspace, and dispatch an agent — with zero login, zero key entry, zero setup. This is the **local mode** that this ADR introduces. + +Two distinct problems block that experience: + +1. **Authentication and authorization are enforced for every request.** `auth_mw.rs` extracts a bearer token; every handler calls `ensure_authorized(&principal, …)`. The only escape hatch is `OVERSIGHT_DISABLE_AUTH=1` (loopback only), which is a dev-time flag, not a product mode. +2. **Every credential must live in the vault.** ADR-005 unified the resolver around `vault.get_credential` / `vault.pool().auto_select_for_adapter`. The resolver has no path for "the CLI already has its own config on disk — don't touch it." It also offers no way to source secrets from the worker's environment or an external secret manager. + +A naïve fix would gate both layers on an `OVERSIGHT_MODE=local` flag and branch inside handlers. That works for the immediate symptom but reintroduces the multi-path drift ADR-005 explicitly fixed, and it doesn't generalize: per-user keys, BYO-config in cloud, CI environment injection, and external secret managers are all variants of the same question — *where does the secret come from for this dispatch?* + +This ADR generalizes that question. + +## Decision + +**Two orthogonal abstractions, shared by cloud and local deployments:** + +1. **A `CredentialSource` enum** carried by `ResolvedCredential`. Each source produces the same downstream `Vec` contract, so adapters and workers are source-agnostic. +2. **Pluggable authentication and authorization backends** in `oversight-server` and `oversight-authz`. The cloud assembly wires OAuth + grant-based authorization (unchanged). The local assembly wires `LocalAuthLayer` (injects a fixed principal) + `AllowAllBackend` (every check passes). Handlers do not change. + +Local mode is the *first consumer* of both abstractions; it is not a special-case code path. + +### Credential sources + +```rust +// crates/oversight-vault/src/manifest.rs +pub enum CredentialSource { + /// Encrypted in the Oversight vault. Server resolves and materializes + /// secrets at dispatch time. Existing behavior; default for cloud. + Vault { namespace: String, profile_id: String }, + + /// The CLI on the worker host owns its own credentials. Oversight only + /// stores binary path + metadata. No secret transits Oversight. + HostNative { adapter_kind: String }, + + /// The worker reads named environment variables from its own process + /// environment at dispatch time. Server never sees the value. + EnvPassthrough { env_map: BTreeMap }, + + /// External resolver (1Password CLI, sops, Vault, etc.). Reserved + /// stub in S1; resolves "not implemented" until a provider lands. + ExternalRef { provider: String, path: String }, +} +``` + +Each variant produces a `ResolvedCredential { source, secrets, profile_id, namespace }` where: + +| Source | `secrets` | `setup_command` | Who rotates | +|------------------|----------------------------------------------------|-----------------|---------------------------------| +| `Vault` | `SecretMaterial::Env { name, value }` (decrypted) | Run per manifest | Oversight admin | +| `HostNative` | empty | **Skipped** | User (`claude login`, etc.) | +| `EnvPassthrough` | `SecretMaterial::EnvFromHost { name }` (no value) | Skipped | Operator (CI / docker compose) | +| `ExternalRef` | resolved by worker plugin | Per provider | External secret manager | + +`HostNative` is the critical asymmetry: **Oversight does not read the credential file**. The CLI itself authenticates via its own on-disk config when `exec`'d. A separate read-only **inspection helper** can display login status in the UI ("Logged in as user@example.com — from `~/.claude/auth.json`"), but inspection output never enters the dispatch path. + +### Source selection (two-tier, additive to ADR-005) + +ADR-005 introduced `agent_definitions.credential_id` as a vault pin. We extend the column's prefix vocabulary to encode source: + +| `credential_id` value | Meaning | +|---------------------------------------------|----------------------------------------------------| +| `vault::` | Vault pin (existing semantics, made explicit) | +| `host-native:` | Use the CLI's own config (no server-side secret) | +| `env:` | Worker env passthrough | +| `ext::` | External resolver (stub) | +| *NULL* | Fall back to per-deployment `default_credential_source`, then to ADR-005 auto-pool | + +A new `default_credential_source` configuration (per adapter kind) is consulted when `credential_id` is NULL. Cloud assembly leaves it at the existing pool auto-pick. Local assembly sets `host-native` for all CLI adapter kinds. + +Backwards compatibility: existing `agent_definitions` rows where `credential_id` lacks a known prefix are read as `vault::` using the manifest-declared namespace. No data migration is forced. + +### Credential scope (orthogonal axis) + +The vault today is global: any agent can resolve any profile. We add scoping to `provider_profile_meta`: + +| column | type | notes | +|------------|--------------|------------------------------------------------| +| `scope` | TEXT NOT NULL | `global` (default) \| `workspace` \| `user` | +| `scope_id` | TEXT NULL | workspace_id or principal_id; NULL for global | + +Resolver filters: `pool.eligible_for(principal, workspace)` runs before priority ordering. Local mode passes trivially (single principal, single default workspace). Cloud deployments opt in by editing profiles; default remains `global` so existing data is unaffected. + +Scope is **not gated on local mode** — it is a general feature that local mode happens to exercise. Cloud deployments that want per-user Claude keys get it for free. + +### Authentication and authorization assembly + +`oversight-authz` already defines a backend trait used by `ensure_authorized`. We add `AllowAllBackend`: + +```rust +// crates/oversight-authz/src/builtin.rs +pub struct AllowAllBackend; +impl AuthzBackend for AllowAllBackend { + fn evaluate(&self, _principal: &Principal, _action: &Action, _scope: &Scope) + -> Decision { Decision::allow_with_reason("local mode") } +} +``` + +`oversight-server` adds `LocalAuthLayer`, a Tower middleware that replaces `require_auth` in local assembly: + +- For every request, inject `AuthContext { principal: Principal::User("local"), workspace: "default", … }`. +- No bearer token read. No session table lookup. +- Listens on loopback by default (configurable; binding `0.0.0.0` requires explicit opt-in flag, mirroring the existing `OVERSIGHT_DISABLE_AUTH` loopback guard). + +Cloud assembly stays exactly as it is. Both assemblies are selected at `main.rs` startup — there is no per-request mode check. Once the layers are wired, handlers, dispatch, and storage code paths are identical. + +### Local mode startup + +`oversight local [--dir ] [--port ]`: + +1. Open or create `/.oversight/oversight.db` (existing behavior of `oversight serve`). +2. Run agent discovery (next section). For each detected CLI, idempotently insert an `agent_definitions` row with `credential_id = "host-native:"`. Re-runs are no-ops. +3. Seed the default workspace and default project (existing `seed_defaults`). +4. Mount `LocalAuthLayer` + `AllowAllBackend`. Skip OAuth provider initialization. +5. Expose `mode: "local"` in `GET /api/auth/status`. Frontend reads this and skips Login routing. +6. Open the dashboard URL in the user's browser (best-effort; non-fatal if it fails). + +### Agent discovery + +A new module `crates/oversight-agents/src/discovery.rs`: + +- **PATH scan**: locate `claude`, `codex`, `openclaw`, `hermes` binaries via `which`-style PATH lookup. +- **Read-only inspection** (display only, never used at invocation): + - `~/.claude/auth.json` → login email, plan tier, model list + - `~/.codex/auth.json` → login email, model list + - OpenClaw install dir → version, configured providers + - Hermes config → endpoint URL, identity +- Returns `Vec`. +- A `POST /api/agents/rescan` endpoint re-runs discovery on demand. No file watching. + +If discovery finds a CLI but the inspection helper detects no login, the seeded agent row carries `credential_status = "needs-login"`. UI surfaces a banner: *"Run `claude login` in your terminal, then click Re-scan."* Dispatch still works — the CLI itself will print the same instruction on stderr if invoked unauthenticated; Oversight surfaces the error verbatim. + +### Frontend changes + +- `GET /api/auth/status` returns `{ bootstrapped, mode: "cloud" | "local", … }`. +- Router: in local mode, the `/login` and `/bootstrap` routes are not mounted. Direct navigation redirects to dashboard. +- Workspace switcher, member management, invite flow, OAuth provider settings: hidden in local mode (data model still supports them — only UI is gated). +- Agents page: each agent card shows `credential_status` badge (logged-in / needs-login / vault-pinned) and the source ("from `~/.claude/auth.json`" / "vault profile *xyz*" / "env: `ANTHROPIC_API_KEY`"). A "Re-scan host" button appears only in local mode. + +### Data model surface (migrations) + +``` +V095__provider_profile_scope.sql + ALTER TABLE provider_profile_meta ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'; + ALTER TABLE provider_profile_meta ADD COLUMN scope_id TEXT NULL; + CREATE INDEX idx_profile_meta_scope ON provider_profile_meta(scope, scope_id); +``` + +A descriptive `agent_definitions.credential_source` column was considered (the original V094 in this design) but dropped: the prefix on `credential_id` is fully sufficient and authoritative, and a denormalised column would have required round-tripping through three storage backends (SQLite, Postgres, filesystem) plus API request/response just to mirror what `derive_credential_source_kind(credential_id)` returns in one constant-time string match. Admin queries that want to group by source call the helper at read time. + +## Architecture (data flow) + +``` + ┌──────────────── credential_id ────────────────┐ + │ vault:ns:profile │ host-native:kind │ + │ env: │ ext:provider:path │ + └────────────────────┬─────────────────────────┘ + │ + resolve_credential_for_agent(def) + │ + ┌───────────────┬──────────────┼──────────────┬──────────────┐ + │ │ │ │ │ + vault.get(ns,id) host-native: env: ext: default + pool.eligible_for (no read) env_map (stub) fallback + scope filter + │ │ │ │ + ▼ ▼ ▼ ▼ + ResolvedCredential { source, secrets, profile_id, namespace } + │ + dispatch payload (unchanged) + │ + ┌───────┴───────┐ + │ │ + embedded worker remote worker + │ │ + ▼ ▼ + MaterializedSecrets → exec adapter CLI + (HostNative skips setup_command; + EnvFromHost reads worker's own env) +``` + +## Implementation phases + +| Step | Scope | Compatibility | +|-------|----------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------| +| **S1** | `CredentialSource` enum, `ResolvedCredential` extension, resolver dispatch on prefixes, `HostNative` and `EnvPassthrough` producers, worker `EnvFromHost` materializer, `HostNative` skips `setup_command`, `derive_credential_source_kind` helper. Migration V095. | Additive. NULL `credential_id` and unprefixed values resolve exactly as today. | +| **S2** | Agent discovery module + `POST /api/agents/rescan`. Inspection helpers for Claude / Codex / OpenClaw / Hermes. | Read-only on filesystem; no behavior change for cloud. | +| **S3** | `LocalAuthLayer`, `AllowAllBackend`, `oversight local` subcommand, mode exposure in `/api/auth/status`. | New subcommand; `oversight serve` unchanged. | +| **S4** | Frontend mode-aware UI: skip Login route, hide multi-workspace surfaces, agent credential badges, Re-scan button. | UI only; cloud appearance unchanged. | +| **S5** | Scope filter integration (`pool.eligible_for`) and credential scope editor in UI. Default remains `global`. | Backward-compatible; opt-in per profile. | + +Each step lands with unit tests on the introduced surface and an integration test exercising at least one cross-crate flow. + +## Consequences + +### Positive + +- **Zero-friction local UX**: `oversight local` from a project directory → workspace and detected agents immediately available; no login, no key entry. +- **Single dispatch path**: cloud and local share the same handlers, dispatcher, worker, and adapter code. No `if mode == local` branches. +- **Credential source is a first-class concept**: future requirements (per-user vault keys, BYO host config in cloud, environment-injected dispatch, external secret managers) are new producers of the same enum, not new modes. +- **Local mode never persists secrets**: `.oversight/oversight.db` from a local install contains no API keys, eliminating an entire class of accidental disclosure (e.g., committing `.oversight/` to git). +- **Vault audit / rotation / cooldown / dispatch_log** remain meaningful — they only apply to `Vault`-source dispatches, which is correct (they're not applicable to host-native CLIs). + +### Negative + +- **Two assemblies to test**: cloud and local each need integration coverage. Mitigated by sharing 95% of code; the differences are confined to assembly wiring. +- **Inspection helper drift**: if a CLI's on-disk config format changes (e.g., a new `~/.claude/auth.json` schema), our UI badge goes stale. Inspection failures are non-fatal — dispatch still works, the badge just shows "unknown". +- **`ExternalRef` is a stub**: callers who try `ext:` get a clear "not implemented" error. Acceptable for S1; full plugin design deferred until there is a concrete consumer. + +### Neutral + +- Existing `OVERSIGHT_DISABLE_AUTH=1` flag is unchanged. It remains a dev-time loopback escape hatch on `oversight serve`. `oversight local` is the supported user-facing path for "no login." +- `oversight serve` continues to require login and explicit credentials. Local mode does not change cloud behavior in any way. + +## Alternatives considered + +### A. Mode flag with handler branches + +Add `OVERSIGHT_MODE=local` and check it inside `require_auth`, inside `ensure_authorized`, inside dispatch. Rejected: reintroduces the multi-path drift ADR-005 fixed, and offers no extensibility for the other credential-source needs (per-user keys, environment injection, external managers). + +### B. Separate desktop binary / Tauri shell + +Ship `oversight-desktop` as a distinct binary, possibly with an embedded WebView. Rejected for this ADR: larger packaging effort with no proportional UX gain over `oversight local` opening a browser. The chosen design does not preclude a future desktop shell — that shell would spawn `oversight local` under the hood. + +### C. Vault-import on first run + +`oversight local` reads `~/.claude/auth.json` once at startup and writes the token into the vault. Rejected: persists secrets where the user did not consent to that, requires `.gitignore` discipline on `.oversight/`, and means `claude login` updates do not take effect until the user manually re-imports. + +### D. Skip credential resolution entirely for local CLIs + +Special-case `agent_type ∈ {claude, codex, openclaw}` to bypass the resolver when `OVERSIGHT_MODE=local`. Rejected: same drift problem as A, and breaks the read endpoint (`GET /api/agent-definitions/:id/resolved-credential`) which clients use to display *which credential will be used* — `HostNative` gives them a meaningful answer; "bypass" gives them an empty one. + +### E. Always-allow authorization without local assembly + +Skip the `oversight-authz` backend abstraction; just have the middleware decide. Rejected: authz already has a backend trait designed for replaceability; the explicit `AllowAllBackend` is a one-file change that keeps the architecture coherent. + +## Migration and rollout + +- **No schema-incompatible changes.** V095 is additive (the originally-planned V094 `credential_source` column was dropped — see "Data model surface"); cloud deployments that take the upgrade see no behavior difference until they assign scopes explicitly. +- **No data migration script required.** Existing rows continue to resolve via the NULL → auto-pool fallback path. +- **Documentation**: a separate operations note (`docs/operations/local-mode.md`) covers `oversight local` startup, the inspection helper coverage matrix, and the "Re-scan" workflow. Will be authored alongside S2. + +## Known limitations (deliberate, not bugs) + +Captured during the post-merge e2e verification against `awaken-managed-agents`: + +- **Token / cost tracking for host-native CLI runs is best-effort.** Awaken's run record (`input_tokens` / `output_tokens`) is populated by its loop runner during runtime/cloud LLM calls, but host-native CLI agents (claude / codex CLIs) do their own LLM calls and currently don't feed usage back into the awaken run. `agent_activities` rows propagate whatever totals the run record has — for CLI runs that's `0`. Vault-side `credential_usage_events` similarly aren't populated because host-native runs are explicitly *not* attributed to a vault credential (that's the whole point of the source). Surfacing token data for host-native runs requires either (a) parsing the CLI's own usage stream (codex emits ACP `UsageUpdate`; claude reports differently) and writing back to the run record, or (b) accepting that local-mode tracking is a CLI-side concern. Out of ADR-011 scope. +- **`requirement` workflow does not auto-spawn `implementation` child issues on approval.** Per ADR-007 the server is a generic runtime, not a delivery-policy engine — decomposition is the `requirement_analyzer` agent's job via `create_issue` tool calls (the seeded prompt currently does not exercise this path; prompt customization is product policy, not infrastructure). +- **Codex CLI's built-in bubblewrap sandbox fails on some hosts** (`bwrap: loopback: Failed RTM_NEWADDR`) — host environment issue, not Oversight. The codex manifest documents the `CODEX_ACP_ARGS_JSON` env workaround in its header. + +## Open questions + +1. **Hermes inspection helper**: format of Hermes's local config is not yet stable; S2 may ship Hermes as PATH-detect only (no metadata) and add inspection in a follow-up. +2. **Windows discovery**: PATH scan and home-dir conventions need a Windows codepath. Targeted for S2 once Linux/macOS land. +3. **Multi-instance on one host**: two `oversight local` instances on the same machine bind different ports and different `.oversight/` directories. Process-isolation is sufficient; no shared lock is required. +4. **Cloud opting in to `host-native`**: a cloud user pointing an agent at `host-native:claude` is supportable but unusual (the worker host must have `claude` installed and authenticated). Not gated; documented as advanced usage. diff --git a/docs/src/zh-CN/architecture/adr-011-local-mode-and-credential-sources.md b/docs/src/zh-CN/architecture/adr-011-local-mode-and-credential-sources.md new file mode 100644 index 00000000..653d77ea --- /dev/null +++ b/docs/src/zh-CN/architecture/adr-011-local-mode-and-credential-sources.md @@ -0,0 +1,225 @@ +# ADR-011：本地模式与凭据来源抽象 + +**状态：** Accepted +**日期：** 2026-05-24 +**范围：** `oversight-vault`、`oversight-authz`、`oversight-server`（装配、credentials resolver、agent discovery）、`oversight-worker`（secret 物化、credential setup）、`oversight-agents`（discovery）、`web/`（mode 感知 UI） +**相关：** ADR-001（远程 Agent Worker）、ADR-005（统一凭据 ↔ Agent 装配）、ADR-006（Workspace × Team × Project） + +## 背景 + +Oversight 目前以多租户服务形态发布：每个请求需要 OAuth/OIDC 登录、需要先创建 workspace、需要显式配置凭据才能跑 agent。这对部署是正确且完整的，但同一份代码也需要服务另一类用户： + +一位本机已经装好并登录了 `claude`、`codex`、`openclaw` 的开发者，希望启动 Oversight 后立即看到 workspace 并直接派发 agent —— 零登录、零填 key、零设置。这就是本 ADR 引入的 **本地模式**。 + +阻塞这个体验的有两个明显问题： + +1. **每个请求都强制 authn/authz。** `auth_mw.rs` 解析 bearer token；每个 handler 调 `ensure_authorized(&principal, …)`。唯一的逃生通道是 `OVERSIGHT_DISABLE_AUTH=1`（仅 loopback），那是开发期 flag，不是产品模式。 +2. **所有凭据都必须放在 vault 里。** ADR-005 把 resolver 统一到 `vault.get_credential` / `vault.pool().auto_select_for_adapter`。resolver 没有"这个 CLI 自己磁盘上已经有 config 了，不要碰"的路径，也没法从 worker 环境或外部 secret manager 取值。 + +最直觉的修法是加 `OVERSIGHT_MODE=local` flag,然后在 handler 里分支。这能解眼前症状,但会重新引入 ADR-005 刚消除的多路径漂移,而且无法泛化:per-user key、云端 BYO-config、CI 环境注入、外部 secret manager,本质上都是同一个问题 —— *这次派发的 secret 从哪里来?* + +本 ADR 把这个问题抽象出来。 + +## 决策 + +**两个正交的抽象,云端与本地共用:** + +1. `ResolvedCredential` 携带的 **`CredentialSource` 枚举**。每种 source 产出相同下游 `Vec` 契约,所以 adapter/worker 对来源无感知。 +2. `oversight-server` 与 `oversight-authz` 中 **可插拔的 authn / authz backend**。云端装配走 OAuth + 基于 grant 的授权(不变)。本地装配走 `LocalAuthLayer`(注入固定 principal)+ `AllowAllBackend`(所有检查放行)。handler 代码不变。 + +本地模式是这两个抽象的 **第一个消费者**,而不是被特殊处理的代码路径。 + +### 凭据来源 + +```rust +// crates/oversight-vault/src/manifest.rs +pub enum CredentialSource { + /// 加密存在 Oversight vault。Server 在派发时解密并物化。 + /// 现有行为;云端默认。 + Vault { namespace: String, profile_id: String }, + + /// Worker 宿主机上的 CLI 自己管理凭据。Oversight 只存二进制路径 + /// 与元数据。无 secret 经过 Oversight。 + HostNative { adapter_kind: String }, + + /// Worker 在派发时从自身进程环境读取指定环境变量。Server 看不到值。 + EnvPassthrough { env_map: BTreeMap }, + + /// 外部解析器(1Password CLI、sops、Vault 等)。S1 保留 stub, + /// 在有 provider 实现之前返回"未实现"。 + ExternalRef { provider: String, path: String }, +} +``` + +每个变体产出 `ResolvedCredential { source, secrets, profile_id, namespace }`: + +| Source | `secrets` | `setup_command` | 轮换方 | +|------------------|----------------------------------------------------|-----------------|----------------------------------| +| `Vault` | `SecretMaterial::Env { name, value }`(解密后) | 按 manifest 运行 | Oversight 管理员 | +| `HostNative` | 空 | **跳过** | 用户(`claude login` 等) | +| `EnvPassthrough` | `SecretMaterial::EnvFromHost { name }`(无值) | 跳过 | 运维(CI / docker compose) | +| `ExternalRef` | 由 worker 插件解析 | 视 provider | 外部 secret manager | + +`HostNative` 的关键不对称:**Oversight 不读凭据文件**。CLI 被 `exec` 时自己读自己的磁盘 config 完成认证。一个独立的只读 **inspection helper** 可以在 UI 显示登录状态("以 user@example.com 登录 —— 来自 `~/.claude/auth.json`"),但 inspection 输出永远不进入 dispatch 路径。 + +### Source 选择(两层,叠加在 ADR-005 之上) + +ADR-005 引入了 `agent_definitions.credential_id` 作为 vault pin。我们扩展该列的前缀语义来编码 source: + +| `credential_id` 值 | 含义 | +|--------------------------------------------|----------------------------------------------------| +| `vault::` | Vault pin(显式化的现有语义) | +| `host-native:` | 使用 CLI 自身配置(server 端无 secret) | +| `env:` | Worker 环境直通 | +| `ext::` | 外部解析器(stub) | +| *NULL* | 回退到 per-deployment `default_credential_source`,再回退到 ADR-005 auto-pool | + +新增的 `default_credential_source` 配置(按 adapter kind)在 `credential_id` 为 NULL 时被查询。云端装配保持现有 pool auto-pick。本地装配把所有 CLI adapter kind 都设为 `host-native`。 + +向后兼容:已有 `agent_definitions` 行如果 `credential_id` 不带已知前缀,按 manifest 声明的 namespace 解读为 `vault::`。无需强制数据迁移。 + +### 凭据 Scope(正交维度) + +vault 今天是全局的:任何 agent 都能解析任意 profile。我们在 `provider_profile_meta` 上加 scope: + +| 列 | 类型 | 备注 | +|------------|---------------|------------------------------------------------| +| `scope` | TEXT NOT NULL | `global`(默认) \| `workspace` \| `user` | +| `scope_id` | TEXT NULL | workspace_id 或 principal_id;global 时为 NULL | + +Resolver 过滤:`pool.eligible_for(principal, workspace)` 在 priority 排序之前运行。本地模式天然通过(单 principal、单 default workspace)。云端按需开启;默认仍是 `global`,现有数据不受影响。 + +Scope **不与本地模式绑定** —— 它是通用特性,本地模式恰好用到。云端想要 per-user Claude key 也免费拿到。 + +### Authn 与 Authz 装配 + +`oversight-authz` 已有 backend trait 被 `ensure_authorized` 使用。新增 `AllowAllBackend`: + +```rust +// crates/oversight-authz/src/builtin.rs +pub struct AllowAllBackend; +impl AuthzBackend for AllowAllBackend { + fn evaluate(&self, _principal: &Principal, _action: &Action, _scope: &Scope) + -> Decision { Decision::allow_with_reason("local mode") } +} +``` + +`oversight-server` 新增 `LocalAuthLayer`,一个 Tower middleware,在本地装配中替换 `require_auth`: + +- 每个请求注入 `AuthContext { principal: Principal::User("local"), workspace: "default", … }` +- 不读 bearer token,不查 session 表 +- 默认监听 loopback(可配置;绑定 `0.0.0.0` 需要显式 opt-in flag,与现有 `OVERSIGHT_DISABLE_AUTH` loopback 守卫一致) + +云端装配完全不动。两套装配在 `main.rs` 启动时选定 —— 没有 per-request mode 检查。装配好之后,handler、dispatch、storage 路径完全相同。 + +### 本地模式启动 + +`oversight local [--dir ] [--port ]`: + +1. 打开或创建 `/.oversight/oversight.db`(`oversight serve` 的现有行为) +2. 跑 agent discovery(见下节)。对每个检测到的 CLI 幂等插入 `agent_definitions` 行,`credential_id = "host-native:"`。重复执行无副作用 +3. 种入默认 workspace 和默认 project(现有 `seed_defaults`) +4. 挂上 `LocalAuthLayer` + `AllowAllBackend`。跳过 OAuth provider 初始化 +5. 在 `GET /api/auth/status` 暴露 `mode: "local"`。前端读这个并跳过 Login 路由 +6. 在用户浏览器中打开 dashboard URL(best-effort;失败不致命) + +### Agent Discovery + +新模块 `crates/oversight-agents/src/discovery.rs`: + +- **PATH 扫描**:用 `which` 风格的 PATH 查找定位 `claude`、`codex`、`openclaw`、`hermes` 二进制 +- **只读 inspection**(仅展示,绝不参与 invocation): + - `~/.claude/auth.json` → 登录邮箱、套餐、模型列表 + - `~/.codex/auth.json` → 登录邮箱、模型列表 + - OpenClaw 安装目录 → 版本、已配置 provider + - Hermes config → endpoint URL、身份 +- 返回 `Vec` +- `POST /api/agents/rescan` endpoint 按需重跑 discovery。不做文件监听 + +若 discovery 找到 CLI 但 inspection helper 检测到未登录,seed 的 agent 行携带 `credential_status = "needs-login"`。UI 显示提示:*"请在终端运行 `claude login` 后点击 Re-scan。"* Dispatch 仍可工作 —— CLI 自身在未登录时也会在 stderr 打印同样的提示,Oversight 原样转发。 + +### 前端变更 + +- `GET /api/auth/status` 返回 `{ bootstrapped, mode: "cloud" | "local", … }` +- 路由:本地模式下 `/login`、`/bootstrap` 不挂载,直达 dashboard +- Workspace 切换器、成员管理、邀请流、OAuth provider 设置:本地模式下隐藏(数据模型仍支持 —— 只是 UI 关闭) +- Agents 页:每张卡片显示 `credential_status` 徽章(logged-in / needs-login / vault-pinned)与来源("来自 `~/.claude/auth.json`" / "vault profile *xyz*" / "env: `ANTHROPIC_API_KEY`")。"Re-scan host" 按钮仅在本地模式出现 + +### 数据模型(迁移) + +``` +V095__provider_profile_scope.sql + ALTER TABLE provider_profile_meta ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'; + ALTER TABLE provider_profile_meta ADD COLUMN scope_id TEXT NULL; + CREATE INDEX idx_profile_meta_scope ON provider_profile_meta(scope, scope_id); +``` + +本设计原计划的描述性列 `agent_definitions.credential_source`(即 V094)已被放弃:`credential_id` 上的前缀已经足够且权威,额外的去规范化列需要在三种存储后端(SQLite、Postgres、文件系统)以及 API 请求/响应间往返同步,只是为了镜像 `derive_credential_source_kind(credential_id)` 一次常量时间字符串匹配就能得到的结果。需要按来源分组的管理查询在读取时调用该 helper。 + +## 实施阶段 + +| 阶段 | 范围 | 兼容性 | +|-------|----------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------| +| **S1** | `CredentialSource` 枚举、`ResolvedCredential` 扩展、resolver 按前缀分发、`HostNative` 与 `EnvPassthrough` producer、worker `EnvFromHost` 物化、`HostNative` 跳过 `setup_command`、`derive_credential_source_kind` helper。迁移 V095。 | 加性。NULL `credential_id` 与无前缀值解析与今日完全一致。 | +| **S2** | Agent discovery 模块 + `POST /api/agents/rescan`。Claude / Codex / OpenClaw / Hermes 的 inspection helper。 | 文件系统只读;云端行为不变。 | +| **S3** | `LocalAuthLayer`、`AllowAllBackend`、`oversight local` 子命令、`/api/auth/status` 的 mode 暴露。 | 新子命令;`oversight serve` 不变。 | +| **S4** | 前端 mode 感知 UI:跳过 Login 路由、隐藏多 workspace 表面、agent 凭据徽章、Re-scan 按钮。 | 仅 UI;云端外观不变。 | +| **S5** | Scope 过滤集成(`pool.eligible_for`)和 UI 中的凭据 scope 编辑器。默认仍 `global`。 | 向后兼容;按 profile 选择启用。 | + +每阶段附带新增表面的单元测试,以及至少一个跨 crate 流程的集成测试。 + +## 后果 + +### 正面 + +- **零摩擦本地 UX**:`oversight local` 在项目目录下 → workspace 与检测到的 agent 立即可用;无登录、无填 key +- **统一派发路径**:云端与本地共享同一套 handler、dispatcher、worker、adapter。无 `if mode == local` 分支 +- **凭据来源成为一等概念**:未来需求(per-user vault key、云端 BYO 宿主 config、环境注入派发、外部 secret manager)都是同一枚举的新 producer,不是新 mode +- **本地模式永不持久化 secret**:本地安装的 `.oversight/oversight.db` 不含任何 API key,消除一整类意外泄露(如把 `.oversight/` 提交到 git) +- **Vault audit / rotation / cooldown / dispatch_log** 仍然有效 —— 它们只对 `Vault` source 的派发生效,这是正确的(对 host-native CLI 不适用) + +### 负面 + +- **两套装配需要测试**:云端与本地各需集成覆盖。共享 95% 代码减轻了此点;差异限于装配接线 +- **Inspection helper 漂移**:若 CLI 的磁盘 config 格式变更(如新版 `~/.claude/auth.json` schema),UI 徽章会过期。Inspection 失败非致命 —— dispatch 仍工作,徽章只是显示 "unknown" +- **`ExternalRef` 是 stub**:用 `ext:` 的调用方会得到清晰的 "not implemented" 错误。S1 接受;完整 plugin 设计待出现具体消费者后再做 + +### 中性 + +- 现有 `OVERSIGHT_DISABLE_AUTH=1` flag 不变,仍是 `oversight serve` 上的 dev-time loopback 逃生通道。`oversight local` 是 "无登录" 的产品级路径 +- `oversight serve` 仍要求登录与显式凭据。本地模式完全不改云端行为 + +## 已考虑的替代方案 + +### A. 带 handler 分支的 mode flag + +加 `OVERSIGHT_MODE=local`,在 `require_auth`、`ensure_authorized`、dispatch 内部 if 分支。拒绝:重新引入 ADR-005 刚消除的多路径漂移,且无法扩展到其他凭据来源需求(per-user key、环境注入、外部 manager) + +### B. 独立 desktop 二进制 / Tauri 壳 + +发布 `oversight-desktop` 作为独立二进制,可能内嵌 WebView。本 ADR 拒绝:打包成本更大且相对 `oversight local` 打开浏览器没有等比 UX 收益。当前设计不排斥未来加 desktop 壳 —— 壳在底层 spawn `oversight local` 即可 + +### C. 首次启动时 Vault 导入 + +`oversight local` 启动时读一次 `~/.claude/auth.json` 并把 token 写入 vault。拒绝:在用户未同意的位置持久化 secret,需要对 `.oversight/` 做 `.gitignore` 纪律,且 `claude login` 的更新在用户手动重新导入之前不生效 + +### D. 本地 CLI 完全跳过凭据解析 + +把 `agent_type ∈ {claude, codex, openclaw}` 在 `OVERSIGHT_MODE=local` 时特判跳过 resolver。拒绝:与 A 同样的漂移问题,且会破坏 `GET /api/agent-definitions/:id/resolved-credential` 读端点 —— 客户端依赖该端点显示"将用哪个凭据",`HostNative` 给出有意义的答复;"跳过"只能给出空答复 + +### E. 不做本地装配的"始终放行"授权 + +不做 `oversight-authz` backend 抽象,只在 middleware 决定。拒绝:authz 已有为可替换性设计的 backend trait;显式 `AllowAllBackend` 是单文件改动,保持架构一致 + +## 迁移与上线 + +- **无 schema 不兼容变更。** V095 是加性的(原计划的 V094 `credential_source` 列已放弃,见"数据模型");云端升级后不显式分配 scope,看不到任何行为差异 +- **无需数据迁移脚本。** 已有行继续走 NULL → auto-pool 的回退路径解析 +- **文档**:独立的运维说明(`docs/operations/local-mode.md`)涵盖 `oversight local` 启动、inspection helper 覆盖矩阵、"Re-scan" 工作流。与 S2 一并提交 + +## 待定问题 + +1. **Hermes inspection helper**:Hermes 本地 config 格式尚未稳定;S2 可能先只发 Hermes 的 PATH 检测(无元数据),inspection 留到后续 +2. **Windows discovery**:PATH 扫描与 home 目录约定需要 Windows 代码分支。Linux/macOS 落地后再做 +3. **同机多实例**:同一台机器跑两个 `oversight local` 实例时,绑不同端口和不同 `.oversight/` 目录。进程隔离已经足够,不需要共享锁 +4. **云端 opt-in `host-native`**:云端用户把某 agent 指向 `host-native:claude` 是可支持但少见(worker host 必须装好并登录 `claude`)。不禁止;文档列为高级用法 From 16d8b2e0c6760848eec1eda3c66bdeb4e2054f9c Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:48 +0800 Subject: [PATCH 2/6] =?UTF-8?q?=E2=9C=A8=20feat(vault):=20SecretMaterial::?= =?UTF-8?q?EnvFromHost=20variant=20+=20worker=20host-env=20reader?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oversight-vault/src/secret_material.rs | 94 +++++++++++++++ crates/oversight-worker/src/secrets.rs | 113 ++++++++++++++++++ 2 files changed, 207 insertions(+) diff --git a/crates/oversight-vault/src/secret_material.rs b/crates/oversight-vault/src/secret_material.rs index baa583fa..a5352bc5 100644 --- a/crates/oversight-vault/src/secret_material.rs +++ b/crates/oversight-vault/src/secret_material.rs @@ -22,6 +22,36 @@ pub enum SecretMaterial { content: RedactedString, mode: u32, }, + /// Read env var `host_name` from the worker's own process + /// environment at materialization time and inject it as `name` + /// in the spawned agent process. The server never sees the + /// value. Building block for the ADR-011 `EnvPassthrough` + /// credential source. + /// + /// ADR-011 review #10: `required` defaults to `true`, meaning + /// missing host env vars FAIL the materialization rather than + /// silently fall back to an empty string. Producers that want + /// the old fall-through behaviour (e.g. for optional overrides) + /// must set `required = false` explicitly. + EnvFromHost { + /// Name of the env var injected into the spawned process. + name: String, + /// Name of the env var read from the worker's process + /// environment. Typically equal to `name`; differ when the + /// host uses a different convention (e.g. CI_ANTHROPIC_KEY) + /// than the adapter expects (ANTHROPIC_API_KEY). + host_name: String, + /// When true (default) the materializer errors if + /// `host_name` is not set on the worker. When false, a + /// missing host var materializes to an empty string so the + /// adapter sees `name=` in its env block. + #[serde(default = "default_required")] + required: bool, + }, +} + +fn default_required() -> bool { + true } impl SecretMaterial { @@ -29,6 +59,7 @@ impl SecretMaterial { match self { Self::Env { .. } => SecretMaterialKind::Env, Self::File { .. } => SecretMaterialKind::File, + Self::EnvFromHost { .. } => SecretMaterialKind::EnvFromHost, } } } @@ -37,6 +68,7 @@ impl SecretMaterial { pub enum SecretMaterialKind { Env, File, + EnvFromHost, } #[cfg(test)] @@ -67,6 +99,68 @@ mod tests { assert!(!formatted.contains("sk-secret"), "leaked: {formatted}"); } + #[test] + fn env_from_host_variant_round_trips() { + // ADR-011 building block: EnvFromHost names a worker-side env + // var to read at materialization. Round-trip the variant on + // the wire so a server emitting it and a worker decoding it + // agree on the shape. + let material = SecretMaterial::EnvFromHost { + name: "ANTHROPIC_API_KEY".into(), + host_name: "CI_ANTHROPIC_KEY".into(), + required: true, + }; + let encoded = serde_json::to_string(&material).unwrap(); + // tag = "kind", rename_all = "snake_case" → "env_from_host" + assert!(encoded.contains(r#""kind":"env_from_host""#)); + let decoded: SecretMaterial = serde_json::from_str(&encoded).unwrap(); + match decoded { + SecretMaterial::EnvFromHost { + name, + host_name, + required, + } => { + assert_eq!(name, "ANTHROPIC_API_KEY"); + assert_eq!(host_name, "CI_ANTHROPIC_KEY"); + assert!(required); + } + other => panic!("expected EnvFromHost, got {other:?}"), + } + } + + /// ADR-011 review #10: a wire payload that predates the `required` + /// field must still decode (default = true). This guards + /// rolling-upgrade compatibility: a newer worker reading from an + /// older server keeps the safe default. + #[test] + fn env_from_host_required_defaults_to_true_when_field_absent() { + let legacy = r#"{ + "kind": "env_from_host", + "name": "ANTHROPIC_API_KEY", + "host_name": "CI_ANTHROPIC_KEY" + }"#; + let decoded: SecretMaterial = serde_json::from_str(legacy).expect("legacy parses"); + match decoded { + SecretMaterial::EnvFromHost { required, .. } => { + assert!( + required, + "missing field must default to required=true (fail-closed)" + ); + } + other => panic!("expected EnvFromHost, got {other:?}"), + } + } + + #[test] + fn env_from_host_kind_routes_to_dedicated_kind_variant() { + let material = SecretMaterial::EnvFromHost { + name: "X".into(), + host_name: "Y".into(), + required: true, + }; + assert_eq!(material.kind(), SecretMaterialKind::EnvFromHost); + } + #[test] fn serde_roundtrip_preserves_value() { let material = SecretMaterial::Env { diff --git a/crates/oversight-worker/src/secrets.rs b/crates/oversight-worker/src/secrets.rs index 33e65525..8aac35e3 100644 --- a/crates/oversight-worker/src/secrets.rs +++ b/crates/oversight-worker/src/secrets.rs @@ -58,6 +58,35 @@ impl MaterializedSecrets { SecretMaterial::Env { name, value } => { env.insert(name.clone(), expose_once(value)); } + SecretMaterial::EnvFromHost { + name, + host_name, + required, + } => { + // ADR-011 review #10: fail-closed by default. + // A missing host var with `required = true` + // aborts the materialization instead of injecting + // an empty string — empty creds tend to surface + // as auth errors deep inside the adapter, far + // from the root cause. Producers that want the + // optional-override behaviour set `required = + // false` explicitly. + let host_value = match std::env::var(host_name) { + Ok(value) => value, + Err(std::env::VarError::NotPresent) if !*required => String::new(), + Err(std::env::VarError::NotPresent) => { + anyhow::bail!( + "EnvFromHost: required host env var '{host_name}' is not set on the worker (target='{name}')" + ); + } + Err(std::env::VarError::NotUnicode(_)) => { + anyhow::bail!( + "EnvFromHost: host env var '{host_name}' is not valid Unicode (target='{name}')" + ); + } + }; + env.insert(name.clone(), host_value); + } SecretMaterial::File { rel_path, content, @@ -362,6 +391,90 @@ mod tests { assert!(!path.exists(), "file should be removed"); } + /// ADR-011 building block: `EnvFromHost` reads from the worker's + /// own process environment exactly once at materialization. Use + /// a uniquely-named host env var so the test doesn't collide + /// with anything CI sets globally. + #[tokio::test] + async fn env_from_host_reads_worker_env_at_materialization() { + let ws = workspace(); + let host_var = "OVERSIGHT_TEST_ENV_FROM_HOST_OK"; + // SAFETY: tests share the process env; the unique prefix + // ensures no collision with other tests / CI vars. + std::env::set_var(host_var, "live-value-from-host"); + + let mat = MaterializedSecrets::from_init( + &[SecretMaterial::EnvFromHost { + name: "ANTHROPIC_API_KEY".into(), + host_name: host_var.into(), + required: true, + }], + ws.path(), + ) + .expect("materialize"); + + std::env::remove_var(host_var); + + assert_eq!( + mat.env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("live-value-from-host"), + "must inject the env var read from the worker process at materialization time", + ); + assert!(mat.files.is_empty(), "EnvFromHost must never write files"); + } + + /// ADR-011 review #10: required + missing host var → hard error. + /// Silent empty-string fallback would let dispatches reach the + /// adapter with no credential, surfacing as an opaque auth + /// failure far from the root cause. + #[tokio::test] + async fn env_from_host_required_and_missing_fails_materialization() { + let ws = workspace(); + let host_var = "OVERSIGHT_TEST_ENV_FROM_HOST_REQUIRED_UNSET"; + std::env::remove_var(host_var); + + let err = MaterializedSecrets::from_init( + &[SecretMaterial::EnvFromHost { + name: "TARGET_ENV".into(), + host_name: host_var.into(), + required: true, + }], + ws.path(), + ) + .expect_err("required + missing host var must error"); + let msg = err.to_string(); + assert!(msg.contains("required host env var"), "got: {msg}"); + assert!( + msg.contains(host_var), + "must name the missing var; got: {msg}" + ); + assert!( + msg.contains("TARGET_ENV"), + "must name the target; got: {msg}" + ); + } + + /// Opt-out: `required = false` keeps the old behaviour for + /// genuinely-optional overrides (extra_env shapes that should + /// silently drop when the host hasn't provided the value). + #[tokio::test] + async fn env_from_host_not_required_yields_empty_string_when_missing() { + let ws = workspace(); + let host_var = "OVERSIGHT_TEST_ENV_FROM_HOST_OPT_OUT_UNSET"; + std::env::remove_var(host_var); + + let mat = MaterializedSecrets::from_init( + &[SecretMaterial::EnvFromHost { + name: "TARGET_ENV".into(), + host_name: host_var.into(), + required: false, + }], + ws.path(), + ) + .expect("non-required materialize must succeed even with missing host var"); + assert_eq!(mat.env.get("TARGET_ENV").map(String::as_str), Some("")); + } + #[tokio::test] async fn cleanup_handles_already_missing_file() { let ws = workspace(); From 015f66c7946014af428a71e3731f94b24b32d2ad Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:48 +0800 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=A8=20feat(credentials):=20Credential?= =?UTF-8?q?Source=20abstraction=20+=20unified=20fail-closed=20dispatch=20r?= =?UTF-8?q?esolver?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oversight-server/src/credentials.rs | 745 +++++++++++++++++++- migrations/V095__provider_profile_scope.sql | 34 + 2 files changed, 774 insertions(+), 5 deletions(-) create mode 100644 migrations/V095__provider_profile_scope.sql diff --git a/crates/oversight-server/src/credentials.rs b/crates/oversight-server/src/credentials.rs index b06874c4..38dbfd23 100644 --- a/crates/oversight-server/src/credentials.rs +++ b/crates/oversight-server/src/credentials.rs @@ -18,11 +18,151 @@ use serde_json::Value; const NS_CLI: &str = "cli-credentials"; const NS_PROVIDERS: &str = "providers"; +/// Where the credential ultimately came from. Lets callers (UI, worker, +/// dispatch) treat a resolution uniformly regardless of source, while +/// still knowing which subsystem owns rotation, audit, and the +/// "needs login" UX. +/// +/// See ADR-011. The variants intentionally mirror the +/// `credential_id` prefix vocabulary (`vault:`, `host-native:`, +/// `env:`, `ext:`) so prefix parsing and source identification stay +/// 1:1. +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "kebab-case", tag = "kind")] +pub enum CredentialSource { + /// Encrypted in the Oversight vault. The server resolves and + /// materialises secrets at dispatch time. Default for cloud and + /// the only variant that participates in pool / cooldown / + /// rotation / dispatch_log. + Vault, + /// The CLI on the worker host owns its own credentials + /// (`~/.claude/`, `~/.codex/`, ...). Oversight stores no secret + /// material — it just records the binding so the UI can show + /// status. `secrets` is always empty; `setup_command` from the + /// manifest MUST be skipped because there is no token to seed + /// the on-disk auth file with. + HostNative { + /// Adapter kind whose host config is being inherited + /// (`"claude"`, `"codex"`, ...). Surfaced in the UI badge. + adapter_kind: String, + }, + /// Worker reads named env vars from its own process environment + /// at dispatch time. Server never sees the value. Reserved for + /// CI / container / external-injection scenarios; not yet wired. + EnvPassthrough, + /// External secret manager (1Password CLI, sops, Vault, ...). + /// Stub variant; resolution currently returns + /// "not yet implemented". + ExternalRef, +} + +impl CredentialSource { + /// Conventional short string used in the wire protocol's + /// `credential_source` field and in UI badges. + pub fn wire_kind(&self) -> &'static str { + match self { + Self::Vault => "vault", + Self::HostNative { .. } => "host-native", + Self::EnvPassthrough => "env", + Self::ExternalRef => "ext", + } + } +} + +/// Structured view of a parsed `agent_def.credential_id` value. Both +/// the dispatch resolver (`resolve_credential_for_agent`) and the +/// read endpoint (`get_agent_resolved_credential` in `api.rs`) +/// dispatch on the same prefix vocabulary; consolidating the parse +/// here keeps the two from drifting (review #8). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CredentialIdRef<'a> { + /// `host-native:` — non-empty adapter validated. + HostNative { adapter: &'a str }, + /// `host-native:` with no adapter after the colon. Always fail-closed. + HostNativeMissingAdapter, + /// `env:` — reserved. + EnvPassthrough { spec: &'a str }, + /// `ext::` — reserved. + ExternalRef { spec: &'a str }, + /// `:` (with optional `vault:` prefix already stripped). + Vault { + namespace: &'a str, + profile_id: &'a str, + }, + /// Malformed: didn't split into `:` after prefix stripping. + Malformed, +} + +/// Parse an `agent_def.credential_id` into its [`CredentialIdRef`] +/// variant. See ADR-011 §"Source selection" for the prefix grammar. +pub fn parse_credential_id_ref(raw: &str) -> CredentialIdRef<'_> { + if let Some(rest) = raw.strip_prefix("host-native:") { + let adapter = rest.trim(); + if adapter.is_empty() { + return CredentialIdRef::HostNativeMissingAdapter; + } + return CredentialIdRef::HostNative { adapter }; + } + if let Some(spec) = raw.strip_prefix("env:") { + return CredentialIdRef::EnvPassthrough { spec }; + } + if let Some(spec) = raw.strip_prefix("ext:") { + return CredentialIdRef::ExternalRef { spec }; + } + // `vault:` explicit prefix and the legacy 2-part `:` form + // share the same resolution path; strip the prefix when present so + // the split below sees a uniform shape. + let trimmed = raw.strip_prefix("vault:").unwrap_or(raw); + match trimmed.split_once(':') { + Some((ns, pid)) if !ns.is_empty() && !pid.is_empty() => CredentialIdRef::Vault { + namespace: ns, + profile_id: pid, + }, + _ => CredentialIdRef::Malformed, + } +} + +/// Static helper: derive the wire-format source kind from a +/// `credential_id` value without running the async resolver. Useful +/// for admin list endpoints / dashboards that want to group agents by +/// source without re-resolving each row. Mirrors the prefix vocabulary +/// in [`resolve_credential_for_agent`]. +/// +/// - `Some("host-native:")` → `Some("host-native")` +/// - `Some("env:")` → `Some("env")` +/// - `Some("ext::")` → `Some("ext")` +/// - `Some("vault::")` or `Some(":")` → `Some("vault")` +/// - `None` (NULL column) → `None` (means "Auto fallback at resolve time") +/// +/// Local agents (`agent_type == "local"`) and legacy a2a inline +/// auth_credential are NOT distinguishable from `credential_id` alone +/// because their source signal lives elsewhere; callers wanting that +/// detail should use the async resolver and read +/// `ResolvedCredential::source`. +pub fn derive_credential_source_kind(credential_id: Option<&str>) -> Option<&'static str> { + let id = credential_id?; + if id.starts_with("host-native:") { + Some("host-native") + } else if id.starts_with("env:") { + Some("env") + } else if id.starts_with("ext:") { + Some("ext") + } else { + // Both explicit `vault:` prefix and the legacy 2-part form + // resolve through the vault path — same source. + Some("vault") + } +} + #[derive(Debug, Clone)] pub struct ResolvedCredential { pub namespace: String, pub profile_id: String, pub secrets: Vec, + /// Where this resolution originated (ADR-011). Defaults to + /// `Vault` for every existing producer; new sources set it + /// explicitly at construction time. + pub source: CredentialSource, } /// Outcome shape used by both dispatch (`resolve_credential_for_agent`) @@ -105,7 +245,19 @@ pub async fn resolve_credential_for_agent( }; } - // 1. Explicit pin via credential_id = ":". + // 1. Explicit pin via credential_id. + // + // ADR-011 extends the prefix vocabulary so a single column can + // bind any of four credential sources: + // - `host-native:` — inherit the host CLI's own + // auth files; vault is never touched, secrets is empty + // - `env:` — worker reads from its own process env + // (reserved, returns a clear "not implemented" note) + // - `ext::` — external secret manager (stub) + // - `vault::` — explicit vault pin + // (equivalent to the legacy `:`) + // - anything else — treated as the legacy 2-part vault pin + // (`:`), preserved for existing rows // // Fail-CLOSED on lookup miss / malformed id. The earlier // "fall through to Auto" behaviour was dangerous: an agent @@ -115,9 +267,73 @@ pub async fn resolve_credential_for_agent( // Dispatch should refuse instead and let the operator see the // mismatch. if let Some(pin) = &def.credential_id { - let (ns, pid) = match pin.split_once(':') { - Some((ns, pid)) if !ns.is_empty() && !pid.is_empty() => (ns, pid), - _ => { + // Single parser shared with the read endpoint (review #8). + let (ns, pid) = match parse_credential_id_ref(pin) { + // host-native dispatch path: zero vault touch, empty secrets. + // Explicit pin so any malformed value fails closed instead + // of silently dropping back to Auto. + CredentialIdRef::HostNative { adapter } => { + if adapter != def.agent_type { + return AgentCredentialResolution { + mode: BindingMode::Pinned, + resolved: None, + note: Some(format!( + "credential_id '{pin}' targets adapter '{adapter}' \ + but agent_type is '{}'; dispatch refused (fail-closed)", + def.agent_type + )), + }; + } + return AgentCredentialResolution { + mode: BindingMode::Pinned, + resolved: Some(ResolvedCredential { + namespace: NS_CLI.into(), + profile_id: format!("host-native:{adapter}"), + secrets: Vec::new(), + source: CredentialSource::HostNative { + adapter_kind: adapter.to_string(), + }, + }), + note: None, + }; + } + CredentialIdRef::HostNativeMissingAdapter => { + return AgentCredentialResolution { + mode: BindingMode::Pinned, + resolved: None, + note: Some(format!( + "credential_id '{pin}' is missing the adapter_kind \ + after 'host-native:'; dispatch refused (fail-closed)" + )), + }; + } + CredentialIdRef::EnvPassthrough { .. } => { + return AgentCredentialResolution { + mode: BindingMode::Pinned, + resolved: None, + note: Some( + "credential_id 'env:*' (worker env passthrough) is reserved \ + but not yet implemented; dispatch refused (fail-closed)" + .into(), + ), + }; + } + CredentialIdRef::ExternalRef { .. } => { + return AgentCredentialResolution { + mode: BindingMode::Pinned, + resolved: None, + note: Some( + "credential_id 'ext:*' (external secret manager) is reserved \ + but not yet implemented; dispatch refused (fail-closed)" + .into(), + ), + }; + } + CredentialIdRef::Vault { + namespace, + profile_id, + } => (namespace, profile_id), + CredentialIdRef::Malformed => { return AgentCredentialResolution { mode: BindingMode::Pinned, resolved: None, @@ -317,6 +533,108 @@ pub async fn resolve_for_adapter( None } +/// Fail-closed refusal returned by [`resolve_dispatch_credential`]. +/// +/// HTTP-agnostic so both dispatch ingresses (`/internal/dispatch` and +/// `/api/cli-agents/:id/dispatch`) can map it to their own response +/// shape. Carries a human-readable `message` and the binding-mode +/// label the UI surfaces (`"Pinned"`, or the resolver's `BindingMode` +/// debug form). +#[derive(Debug, Clone)] +pub struct DispatchCredentialRefusal { + pub message: String, + pub binding_mode: String, +} + +/// Single credential-resolution entry point for ALL dispatch +/// ingresses (ADR-005 / ADR-011). Both `/internal/dispatch` and the +/// direct `/api/cli-agents/:id/dispatch` route MUST go through here so +/// the "explicit pin never silently substitutes" contract holds at +/// every boundary — a duplicated, slightly-different copy was exactly +/// how the direct route drifted into fail-open. +/// +/// Contract: +/// * `pool = Some` → delegate to [`resolve_credential_for_agent`]; +/// an explicit refusal (Pinned/Selector + no profile) becomes +/// `Err`. +/// * `pool = None` + matching `host-native:` → host-native +/// fast path (empty secrets, the CLI authenticates itself). +/// * `pool = None` + host-native adapter MISMATCH, reserved +/// (`env:` / `ext:` / bare `host-native:`), vault pin, or malformed +/// id → `Err` (fail-closed; never fall back to adapter auto-pick +/// which would ignore the pin). +/// * `pool = None` + no `credential_id` → legacy adapter auto-pick +/// via [`resolve_for_adapter`] (unpinned agents keep working in +/// no-sidecar fixtures). +pub async fn resolve_dispatch_credential( + vault: &Arc, + pool: Option<&Arc>, + agent_def: &AgentDef, +) -> Result, DispatchCredentialRefusal> { + if let Some(pool) = pool { + let resolution = resolve_credential_for_agent(vault, pool, agent_def).await; + if resolution.is_explicit_refusal() { + return Err(DispatchCredentialRefusal { + message: resolution.refusal_message(), + binding_mode: format!("{:?}", resolution.mode), + }); + } + return Ok(resolution.resolved); + } + + // No pool: honour non-vault prefixes directly; fail-closed on any + // explicit pin we can't satisfy without the pool. + let Some(pin) = agent_def.credential_id.as_deref() else { + return Ok(resolve_for_adapter(vault, None, &agent_def.agent_type).await); + }; + match parse_credential_id_ref(pin) { + CredentialIdRef::HostNative { adapter } if adapter == agent_def.agent_type => { + Ok(Some(ResolvedCredential { + namespace: NS_CLI.into(), + profile_id: format!("host-native:{adapter}"), + secrets: Vec::new(), + source: CredentialSource::HostNative { + adapter_kind: adapter.to_string(), + }, + })) + } + CredentialIdRef::HostNative { adapter } => Err(DispatchCredentialRefusal { + message: format!( + "credential_id '{pin}' adapter='{adapter}' does not match \ + agent_type='{}' (fail-closed)", + agent_def.agent_type, + ), + binding_mode: "Pinned".into(), + }), + CredentialIdRef::HostNativeMissingAdapter + | CredentialIdRef::EnvPassthrough { .. } + | CredentialIdRef::ExternalRef { .. } => Err(DispatchCredentialRefusal { + message: format!( + "credential_id '{pin}' is reserved / malformed and cannot be \ + resolved without the credential pool (fail-closed)" + ), + binding_mode: "Pinned".into(), + }), + CredentialIdRef::Vault { + namespace, + profile_id, + } => Err(DispatchCredentialRefusal { + message: format!( + "credential_id '{pin}' is a vault pin ('{namespace}:{profile_id}') \ + but the credential pool is not configured; dispatch refused (fail-closed)" + ), + binding_mode: "Pinned".into(), + }), + CredentialIdRef::Malformed => Err(DispatchCredentialRefusal { + message: format!( + "credential_id '{pin}' is not in ':' \ + or ':...' format; dispatch refused (fail-closed)" + ), + binding_mode: "Pinned".into(), + }), + } +} + /// Pool-aware resolution. Tries cli-credentials first (matches /// `adapter_type`), then falls back to providers (matches `adapter`). /// Touches the picked profile so the LRU tie-break in subsequent @@ -487,11 +805,17 @@ fn materialize_cli( // from the endpoint its host config already targets. // Returning a non-`None` `ResolvedCredential` with an // empty `secrets` Vec is the signal: "credential is - // resolved, just doesn't materialize as env vars". + // resolved, just doesn't materialize as env vars". Marked + // `source = HostNative` so the worker skips + // `setup_command` (no token to bootstrap) and the UI can + // surface the host-native badge. return Some(ResolvedCredential { namespace: NS_CLI.into(), profile_id, secrets: Vec::new(), + source: CredentialSource::HostNative { + adapter_kind: adapter_type.to_string(), + }, }); } _ => return None, @@ -543,6 +867,7 @@ fn materialize_cli( namespace: NS_CLI.into(), profile_id, secrets, + source: CredentialSource::Vault, }) } @@ -592,6 +917,7 @@ fn materialize_provider( namespace: NS_PROVIDERS.into(), profile_id, secrets, + source: CredentialSource::Vault, }) } @@ -1114,6 +1440,88 @@ mod tests { assert!(result.resolved.is_none()); } + // ── shared dispatch credential helper (no-pool ingress) ────────────── + // + // Both `/internal/dispatch` and `/api/cli-agents/:id/dispatch` route + // through `resolve_dispatch_credential`; these pin the fail-closed + // contract at the helper level so neither ingress can drift. + + #[tokio::test] + async fn dispatch_cred_no_pool_host_native_match_resolves_empty_secrets() { + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", Some("host-native:claude"), None); + let resolved = resolve_dispatch_credential(&vault, None, &def) + .await + .expect("matching host-native pin must resolve") + .expect("host-native yields a ResolvedCredential"); + assert!(matches!( + resolved.source, + CredentialSource::HostNative { .. } + )); + assert!( + resolved.secrets.is_empty(), + "host-native materialises zero secrets" + ); + } + + #[tokio::test] + async fn dispatch_cred_no_pool_host_native_mismatch_fails_closed() { + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", Some("host-native:codex"), None); + let err = resolve_dispatch_credential(&vault, None, &def) + .await + .expect_err("adapter mismatch must refuse"); + assert_eq!(err.binding_mode, "Pinned"); + } + + #[tokio::test] + async fn dispatch_cred_no_pool_vault_pin_fails_closed() { + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", Some("cli-credentials:missing-profile"), None); + let err = resolve_dispatch_credential(&vault, None, &def) + .await + .expect_err("vault pin without pool must refuse"); + assert_eq!(err.binding_mode, "Pinned"); + assert!(err.message.contains("vault pin")); + } + + #[tokio::test] + async fn dispatch_cred_no_pool_malformed_id_fails_closed() { + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", Some("not-a-pair"), None); + let err = resolve_dispatch_credential(&vault, None, &def) + .await + .expect_err("malformed credential_id must refuse"); + assert_eq!(err.binding_mode, "Pinned"); + } + + #[tokio::test] + async fn dispatch_cred_no_pool_env_pin_fails_closed() { + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", Some("env:OPENAI_API_KEY"), None); + let err = resolve_dispatch_credential(&vault, None, &def) + .await + .expect_err("reserved env: pin must refuse without a pool"); + assert_eq!(err.binding_mode, "Pinned"); + } + + #[tokio::test] + async fn dispatch_cred_no_pool_unpinned_falls_back_to_adapter_pick() { + // No credential_id => unpinned. With an empty vault the adapter + // auto-pick legitimately yields None (caller dispatches with + // empty secrets), but it must NOT be an Err — unpinned agents + // are allowed to fall through in no-sidecar fixtures. + let (vault, _pool) = ephemeral(); + let def = agent_with("claude", None, None); + let resolved = resolve_dispatch_credential(&vault, None, &def) + .await + .expect("unpinned agent must not be refused"); + assert!( + resolved.is_none(), + "empty vault yields no profile, but that's Ok(None) not Err" + ); + } + #[tokio::test] async fn resolver_pinned_credential_resolves_via_lookup() { let (vault, pool) = ephemeral(); @@ -1414,6 +1822,7 @@ mod tests { namespace: "providers".into(), profile_id: "p".into(), secrets: Vec::new(), + source: super::CredentialSource::Vault, }) } else { None @@ -1870,6 +2279,332 @@ mod tests { .contains("upstream is currently unavailable")); } + // ── ADR-011: credential_id source prefix dispatch ────────────────── + + /// `host-native:` is a zero-vault-touch resolution. + /// Resolver returns `Some(_)` with empty secrets, source = + /// HostNative, and namespace = cli-credentials so the dispatch + /// payload keeps reading the same field. No pool lookup is + /// attempted (verified indirectly: this test seeds nothing in the + /// vault and the resolver still succeeds). + #[tokio::test] + async fn resolver_host_native_pin_resolves_without_touching_vault() { + let (vault, pool) = ephemeral(); + let def = agent_with("claude", Some("host-native:claude"), None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + assert_eq!(result.mode, BindingMode::Pinned); + let resolved = result.resolved.expect("host-native always resolves"); + assert!( + resolved.secrets.is_empty(), + "host-native must never materialise secrets" + ); + assert_eq!(resolved.namespace, NS_CLI); + assert_eq!(resolved.profile_id, "host-native:claude"); + assert_eq!( + resolved.source, + CredentialSource::HostNative { + adapter_kind: "claude".into(), + } + ); + } + + /// Adapter mismatch on a `host-native:` pin must fail closed — + /// same contract as a vault pin with a mismatched adapter_type. + /// Routing a `host-native:codex` agent through the claude CLI + /// would invoke the wrong binary. + #[tokio::test] + async fn resolver_host_native_with_wrong_adapter_fails_closed() { + let (vault, pool) = ephemeral(); + let def = agent_with("codex", Some("host-native:claude"), None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + assert_eq!(result.mode, BindingMode::Pinned); + assert!(result.resolved.is_none()); + let note = result.note.expect("must explain refusal"); + assert!(note.contains("targets adapter 'claude'")); + assert!(note.contains("agent_type is 'codex'")); + assert!(note.contains("fail-closed")); + } + + /// `host-native:` with no adapter_kind is a malformed pin — + /// fail closed rather than silently treating it as wildcard. + #[tokio::test] + async fn resolver_host_native_with_empty_adapter_fails_closed() { + let (vault, pool) = ephemeral(); + let def = agent_with("claude", Some("host-native:"), None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + assert_eq!(result.mode, BindingMode::Pinned); + assert!(result.resolved.is_none()); + let note = result.note.expect("must explain refusal"); + assert!(note.contains("missing the adapter_kind")); + assert!(note.contains("fail-closed")); + } + + /// `env:*` is reserved but not implemented — fail closed with a + /// clear note. Silently substituting Auto would change behaviour + /// the operator did not ask for. + #[tokio::test] + async fn resolver_env_prefix_returns_not_implemented() { + let (vault, pool) = ephemeral(); + let def = agent_with("claude", Some("env:ANTHROPIC_API_KEY"), None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + assert_eq!(result.mode, BindingMode::Pinned); + assert!(result.resolved.is_none()); + let note = result.note.expect("must explain refusal"); + assert!(note.contains("not yet implemented")); + assert!(note.contains("env")); + } + + /// `ext:*` is reserved but not implemented — fail closed. + #[tokio::test] + async fn resolver_ext_prefix_returns_not_implemented() { + let (vault, pool) = ephemeral(); + let def = agent_with("claude", Some("ext:1password:my/key"), None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + assert_eq!(result.mode, BindingMode::Pinned); + assert!(result.resolved.is_none()); + let note = result.note.expect("must explain refusal"); + assert!(note.contains("not yet implemented")); + assert!(note.contains("ext")); + } + + /// `vault::` is the explicit form of the + /// legacy 2-part pin. Both formats must resolve identically so + /// existing rows keep working while new code can be unambiguous + /// about the source. + #[tokio::test] + async fn resolver_explicit_vault_prefix_resolves_same_as_legacy_form() { + let (vault, pool) = ephemeral(); + vault + .put( + NS_CLI, + "claude-prod", + &serde_json::json!({ + "id": "claude-prod", + "adapter_type": "claude", + "auth_kind": "api_key", + "secret": "sk-prod", + "enabled": true + }), + ) + .await + .unwrap(); + + let explicit = agent_with("claude", Some("vault:cli-credentials:claude-prod"), None); + let r_explicit = resolve_credential_for_agent(&vault, &pool, &explicit).await; + let resolved_explicit = r_explicit.resolved.expect("explicit vault resolves"); + assert_eq!(resolved_explicit.profile_id, "claude-prod"); + assert_eq!(resolved_explicit.source, CredentialSource::Vault); + + let legacy = agent_with("claude", Some("cli-credentials:claude-prod"), None); + let r_legacy = resolve_credential_for_agent(&vault, &pool, &legacy).await; + let resolved_legacy = r_legacy.resolved.expect("legacy form resolves"); + assert_eq!(resolved_legacy.profile_id, "claude-prod"); + assert_eq!(resolved_legacy.source, CredentialSource::Vault); + } + + /// Regression guard: every existing successful resolution path + /// must produce `source = Vault`. If a new producer is added that + /// returns a different source, this test must be updated + /// consciously rather than silently changing the contract. + #[tokio::test] + async fn resolver_existing_vault_path_carries_vault_source() { + let (vault, pool) = ephemeral(); + vault + .put( + NS_CLI, + "claude-auto", + &serde_json::json!({ + "id": "claude-auto", + "adapter_type": "claude", + "auth_kind": "api_key", + "secret": "sk-auto", + "enabled": true + }), + ) + .await + .unwrap(); + let def = agent_with("claude", None, None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + let resolved = result.resolved.expect("auto resolves"); + assert_eq!(resolved.source, CredentialSource::Vault); + } + + /// Existing payload-driven `auth_kind: "host_cli"` rows (the + /// pre-ADR-011 way to express host-native) must also surface + /// `source = HostNative` so UI badges line up regardless of + /// whether the operator used the new pin prefix or the legacy + /// vault row. + #[tokio::test] + async fn resolver_payload_auth_kind_host_cli_carries_host_native_source() { + let (vault, pool) = ephemeral(); + vault + .put( + NS_CLI, + "host-claude", + &serde_json::json!({ + "id": "host-claude", + "adapter_type": "claude", + "auth_kind": "host_cli", + "enabled": true + }), + ) + .await + .unwrap(); + let def = agent_with("claude", None, None); + let result = resolve_credential_for_agent(&vault, &pool, &def).await; + let resolved = result.resolved.expect("host_cli resolves"); + assert!(resolved.secrets.is_empty()); + assert_eq!( + resolved.source, + CredentialSource::HostNative { + adapter_kind: "claude".into(), + } + ); + } + + /// JSON shape of `CredentialSource` is stable — the wire format + /// is what the UI consumes via `/api/agents/:id/resolved-credential`. + /// Pin the discriminator naming so a refactor can't quietly change + /// it (which would silently break the frontend badge). + #[test] + fn credential_source_serialises_with_kebab_kind_tag() { + let vault = serde_json::to_value(CredentialSource::Vault).unwrap(); + assert_eq!(vault, serde_json::json!({"kind": "vault"})); + + let host = serde_json::to_value(CredentialSource::HostNative { + adapter_kind: "claude".into(), + }) + .unwrap(); + assert_eq!( + host, + serde_json::json!({"kind": "host-native", "adapter_kind": "claude"}) + ); + + let env = serde_json::to_value(CredentialSource::EnvPassthrough).unwrap(); + assert_eq!(env, serde_json::json!({"kind": "env-passthrough"})); + + let ext = serde_json::to_value(CredentialSource::ExternalRef).unwrap(); + assert_eq!(ext, serde_json::json!({"kind": "external-ref"})); + } + + /// R2 review #8: pin the prefix parser truth table so the + /// resolver and the read endpoint can't drift. Any new prefix + /// added to the language must also land here with a matching + /// case. + #[test] + fn parse_credential_id_ref_truth_table() { + match parse_credential_id_ref("host-native:claude") { + CredentialIdRef::HostNative { adapter } => assert_eq!(adapter, "claude"), + other => panic!("expected HostNative, got {other:?}"), + } + // Whitespace after `host-native:` is trimmed. + match parse_credential_id_ref("host-native: codex ") { + CredentialIdRef::HostNative { adapter } => assert_eq!(adapter, "codex"), + other => panic!("expected HostNative, got {other:?}"), + } + assert_eq!( + parse_credential_id_ref("host-native:"), + CredentialIdRef::HostNativeMissingAdapter + ); + assert_eq!( + parse_credential_id_ref("host-native: "), + CredentialIdRef::HostNativeMissingAdapter + ); + match parse_credential_id_ref("env:ANTHROPIC_API_KEY") { + CredentialIdRef::EnvPassthrough { spec } => assert_eq!(spec, "ANTHROPIC_API_KEY"), + other => panic!("expected Env, got {other:?}"), + } + match parse_credential_id_ref("ext:1password:my/key") { + CredentialIdRef::ExternalRef { spec } => assert_eq!(spec, "1password:my/key"), + other => panic!("expected Ext, got {other:?}"), + } + // Explicit vault: prefix stripped. + match parse_credential_id_ref("vault:cli-credentials:claude-prod") { + CredentialIdRef::Vault { + namespace, + profile_id, + } => { + assert_eq!(namespace, "cli-credentials"); + assert_eq!(profile_id, "claude-prod"); + } + other => panic!("expected Vault, got {other:?}"), + } + // Legacy 2-part form. + match parse_credential_id_ref("cli-credentials:claude-prod") { + CredentialIdRef::Vault { + namespace, + profile_id, + } => { + assert_eq!(namespace, "cli-credentials"); + assert_eq!(profile_id, "claude-prod"); + } + other => panic!("expected Vault, got {other:?}"), + } + // No colon, or empty parts → Malformed. + assert_eq!( + parse_credential_id_ref("not-a-pair"), + CredentialIdRef::Malformed + ); + assert_eq!( + parse_credential_id_ref(":missing-ns"), + CredentialIdRef::Malformed + ); + assert_eq!( + parse_credential_id_ref("missing-pid:"), + CredentialIdRef::Malformed + ); + } + + /// ADR-011 review #4: derive_credential_source_kind is the + /// static helper admin / dashboard code uses to label rows + /// without re-running the async resolver. Pin the truth table so + /// the helper and the resolver stay in lockstep. + #[test] + fn derive_credential_source_kind_covers_all_prefixes() { + assert_eq!( + derive_credential_source_kind(Some("host-native:claude")), + Some("host-native") + ); + assert_eq!( + derive_credential_source_kind(Some("env:ANTHROPIC_API_KEY")), + Some("env") + ); + assert_eq!( + derive_credential_source_kind(Some("ext:1password:my/key")), + Some("ext") + ); + assert_eq!( + derive_credential_source_kind(Some("vault:cli-credentials:claude-prod")), + Some("vault") + ); + // Legacy 2-part form (`:`, no prefix) resolves + // through the vault path; same source. + assert_eq!( + derive_credential_source_kind(Some("cli-credentials:claude-prod")), + Some("vault") + ); + // NULL → resolver falls back to Auto. Caller decides UI. + assert_eq!(derive_credential_source_kind(None), None); + } + + #[test] + fn credential_source_wire_kind_matches_pin_prefix_vocabulary() { + // wire_kind() is what the protocol field carries; pin prefix + // is what credential_id uses. They must stay in lockstep so a + // worker / UI reading wire_kind() can route on the same string + // that selects the source at resolve time. + assert_eq!(CredentialSource::Vault.wire_kind(), "vault"); + assert_eq!( + CredentialSource::HostNative { + adapter_kind: "claude".into(), + } + .wire_kind(), + "host-native" + ); + assert_eq!(CredentialSource::EnvPassthrough.wire_kind(), "env"); + assert_eq!(CredentialSource::ExternalRef.wire_kind(), "ext"); + } + #[tokio::test] async fn materialize_cli_host_cli_returns_empty_secrets() { // host_cli is the inherit-host-CLI-auth mode: the resolver diff --git a/migrations/V095__provider_profile_scope.sql b/migrations/V095__provider_profile_scope.sql new file mode 100644 index 00000000..078316b5 --- /dev/null +++ b/migrations/V095__provider_profile_scope.sql @@ -0,0 +1,34 @@ +-- ADR-011: per-credential scope so the same vault can host global, +-- per-workspace, and per-user credentials without leaking between +-- principals. +-- +-- The vault layer (oversight-vault) is intentionally scope-agnostic — +-- secrets are encrypted per-row regardless of scope. The scope is +-- applied at the credential pool layer: `pool.eligible_for(principal, +-- workspace)` filters by these columns before priority ordering. +-- +-- scope: +-- 'global' — any agent in any workspace may resolve (default; +-- preserves existing behaviour for all rows seeded +-- before this migration) +-- 'workspace' — only agents whose workspace_id == scope_id may resolve +-- 'user' — only agents whose owner_principal == scope_id may resolve +-- +-- scope_id is NULL for 'global' and the workspace_id / principal_id +-- for the other two variants. +-- +-- Nothing in this migration changes runtime behaviour: every existing +-- row keeps scope='global' and the resolver's existing pool.pick() +-- continues to see every profile. Cloud deployments opt in to scoping +-- by editing individual profiles; local mode (single user, single +-- workspace) needs no change because 'global' trivially matches. +ALTER TABLE provider_profile_meta + ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'; + +ALTER TABLE provider_profile_meta + ADD COLUMN scope_id TEXT; + +-- Index ordering: scope first because the resolver always filters by +-- scope and only narrows further by scope_id when scope <> 'global'. +CREATE INDEX IF NOT EXISTS idx_profile_meta_scope + ON provider_profile_meta(scope, scope_id); From 023373df42d7cca93a5f805adff314b28171f56d Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:48 +0800 Subject: [PATCH 4/6] =?UTF-8?q?=E2=9C=A8=20feat(agents):=20host-native=20d?= =?UTF-8?q?iscovery=20+=20codex=20ACP=20arg/MCP=20plumbing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oversight-agents/manifests/codex.toml | 36 +- crates/oversight-agents/src/discovery.rs | 826 ++++++++++++++++++ crates/oversight-agents/src/lib.rs | 1 + crates/oversight-worker/src/adapter.rs | 336 ++++++- .../oversight-worker/src/adapters/generic.rs | 13 +- 5 files changed, 1168 insertions(+), 44 deletions(-) create mode 100644 crates/oversight-agents/src/discovery.rs diff --git a/crates/oversight-agents/manifests/codex.toml b/crates/oversight-agents/manifests/codex.toml index 493831ff..c8c1778a 100644 --- a/crates/oversight-agents/manifests/codex.toml +++ b/crates/oversight-agents/manifests/codex.toml @@ -1,10 +1,40 @@ id = "codex" display_name = "OpenAI Codex CLI" spec_version = 2 -embedded_revision = 4 +embedded_revision = 7 is_builtin = true enabled = true +# ── Host bubblewrap broken? (ADR-011 e2e finding) ───────────────── +# Some Linux hosts (older kernels, restricted user-namespaces, +# locked-down seccomp profiles) can't run codex's built-in bubblewrap +# sandbox. The symptom is every shell / apply_patch op failing with +# `bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted` +# and the run terminating in seconds with no file edits. +# +# The shipped default below uses `sandbox_mode="workspace-write"` +# which engages bwrap and is the right secure default for most hosts. +# Operators on broken-bwrap hosts can override the full args via the +# `CODEX_ACP_ARGS_JSON` env var, e.g.: +# +# export CODEX_ACP_ARGS_JSON='[ +# "-y", "@zed-industries/codex-acp@latest", +# "-c", "approval_policy=\"never\"", +# "-c", "sandbox_mode=\"danger-full-access\"" +# ]' +# +# `danger-full-access` skips bwrap entirely. Only use on hosts where +# you have OTHER process containment (container, VM, user-isolation) +# — agent shell ops then run with the worker process's privileges. +# The Claude adapter is a separate option; it uses a different +# sandbox model and is unaffected by this bwrap issue. +# +# Contract: when `CODEX_ACP_ARGS_JSON` is set the operator has taken +# full responsibility for launcher args, and `[runtime.forced_args]` +# below are NOT re-applied. The override is whole-args, not key-wise. +# Editing the env var to drop the safety pins is therefore equivalent +# to editing the manifest in the vault; both require operator trust. + [detect] kind = "command" exec = "codex --version" @@ -63,7 +93,7 @@ model_env = "OVERSIGHT_CODEX_MODEL" # `model` field. default_model = "" max_concurrent = 4 -cleanup_files = [".codex-instructions.md"] +cleanup_files = [".oversight/codex-instructions.md"] [[runtime.forced_args]] key = "approval_policy" @@ -74,7 +104,7 @@ key = "sandbox_mode" value = "\"workspace-write\"" [runtime.workspace] -prompt_file = ".codex-instructions.md" +prompt_file = ".oversight/codex-instructions.md" [runtime.workspace.prompt_assembly] kind = "append_skill_section" diff --git a/crates/oversight-agents/src/discovery.rs b/crates/oversight-agents/src/discovery.rs new file mode 100644 index 00000000..2dd669ab --- /dev/null +++ b/crates/oversight-agents/src/discovery.rs @@ -0,0 +1,826 @@ +//! Local-mode agent discovery (ADR-011 S2). +//! +//! Read-only scan of the worker's host for known CLI agent binaries +//! (`claude`, `codex`, `openclaw`, `hermes`) and best-effort inspection +//! of each adapter's on-disk auth state. Used by `oversight local` at +//! startup (and on user-triggered Re-scan) to populate the agent list +//! without requiring the user to enter API keys or other configuration. +//! +//! Critically, discovery NEVER reads secret material that flows into +//! a dispatch. Each CLI authenticates itself via its own on-disk +//! config when `exec`'d by the worker; the inspection helpers here +//! exist only to surface *status* in the UI ("logged in as +//! user@example.com" / "needs login"). The dispatch path's +//! `host-native` credential source resolves to an empty +//! `SecretMaterial` vec — see ADR-011 §4 and the resolver in +//! `oversight-server::credentials`. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use oversight_models::AgentDef; +use serde::{Deserialize, Serialize}; + +/// Adapter kinds discovery knows about, paired with a UI-friendly +/// display name. Order is the canonical "preferred" order — clients +/// rendering the list may want to keep it. +pub const KNOWN_CLI_AGENTS: &[(&str, &str)] = &[ + ("claude", "Claude Code"), + ("codex", "Codex CLI"), + ("openclaw", "OpenClaw"), + ("hermes", "Hermes"), +]; + +/// Status of the inspected on-disk credential for a discovered CLI. +/// +/// `Unknown` is the safe default — discovery falls back to it when an +/// adapter doesn't yet have an inspection helper or when the host's +/// HOME directory can't be resolved. It signals "the CLI is on PATH; +/// we couldn't determine its login state from here", which is still +/// useful information for the UI. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case", tag = "kind")] +pub enum LoginStatus { + /// On-disk auth state suggests the CLI is logged in. `account` is + /// best-effort — if the inspection helper can derive an identity + /// (typically the account email) it's surfaced; absence is fine. + LoggedIn { + #[serde(default, skip_serializing_if = "Option::is_none")] + account: Option, + }, + /// No on-disk auth state was found. The CLI will likely print a + /// "please log in" message when invoked. + NeedsLogin, + /// Inspection helper not available for this adapter, or HOME + /// could not be resolved. + Unknown, +} + +/// Convert a [`DiscoveredAgent`] into the `AgentDef` row the +/// dispatcher reads. The discovered agent always becomes: +/// +/// - `agent_id = adapter_kind` (e.g. `claude`), so the local-mode +/// seed for each adapter has a stable, predictable id; +/// - `agent_type = adapter_kind` so the resolver routes through the +/// adapter's manifest; +/// - `credential_id = "host-native:"` (ADR-011) so the +/// resolver returns empty secrets and the worker `exec`s the CLI +/// directly. The "source" classification (`host-native`) is +/// derived from this prefix at read time by +/// [`oversight_server::credentials::derive_credential_source_kind`] +/// — there is no separate `credential_source` column or struct +/// field; the prefix is the source of truth. +/// +/// Production caller (`oversight local`) checks +/// `get_agent_def_by_agent_id` before inserting so re-runs are +/// idempotent. +pub fn agent_def_for_discovered(discovered: &DiscoveredAgent) -> AgentDef { + let mut def = AgentDef::new( + discovered.adapter_kind.clone(), + discovered.display_name.clone(), + String::new(), + ); + def.agent_type = discovered.adapter_kind.clone(); + def.credential_id = Some(format!("host-native:{}", discovered.adapter_kind)); + // ADR-011 review #7: phrase the description honestly. The + // discovered path is a diagnostic captured at scan time — the + // worker re-resolves PATH at dispatch via the adapter manifest's + // `detect` step, so the actual exec'd binary may differ if PATH + // changes between discovery and dispatch (or if discovery ran + // under a different PATH than the worker process). The wording + // here says "detected at" not "will run", so log readers and UI + // viewers can't be misled. + def.description = Some(format!( + "Auto-discovered host-native {} CLI (detected at {} during local-mode startup; \ + worker re-resolves PATH at dispatch)", + discovered.display_name, + discovered.binary_path.display(), + )); + // ADR-011 review #8: a CLI on PATH without an on-disk config dir + // is "needs login". Seeding it as active=true would let the agent + // appear in the active list and silently fail on first invocation. + // Inactive is the right initial state: the UI can show "needs + // login" and the user activates after `claude login`. `Unknown` + // (no inspection helper for this adapter, or HOME unresolved) + // stays active — we have no evidence either way and refusing to + // seed would be worse than seeding optimistically. + def.active = !matches!(discovered.login_status, LoginStatus::NeedsLogin); + def +} + +/// Apply a host-native overlay onto a seeded role agent in-place. +/// +/// Used by `oversight local` to bridge seeded role agents +/// (coder / reviewer / requirement_analyzer / …) onto the host CLI. +/// Returns `true` when the row was mutated and should be persisted. +/// `oversight local` re-runs discovery on every boot, so this also +/// handles the re-run case: a role agent already carrying the +/// `host-native:` pin still gets a login-state safety +/// refresh. +/// +/// Rules: +/// * the discovery-seeded adapter row itself (`agent_id == agent_type`) +/// is skipped — its login state is owned by +/// `refresh_host_native_login_state` +/// * mismatched `agent_type` (no matching discovery) leaves the row alone +/// * `credential_id == None` → write `host-native:` +/// * `credential_id == host-native:` (already overlaid) → +/// fall through to the safety refresh below +/// * any other `credential_id` (operator pin / selector) → preserved, +/// never overwritten +/// * **safety refresh**: `NeedsLogin` forces `active = false` so +/// dispatch never calls an unauthenticated CLI. We deliberately do +/// NOT auto-reactivate on `LoggedIn` / `Unknown` — that would +/// clobber a manual operator disable. Re-enabling after +/// `claude login` is a manual action (until a dedicated +/// `disabled_by_user` field distinguishes the two). +pub fn apply_host_native_overlay( + def: &mut AgentDef, + discovered_by_adapter: &std::collections::HashMap, +) -> bool { + if def.agent_id == def.agent_type { + return false; + } + let Some(login_status) = discovered_by_adapter.get(&def.agent_type) else { + return false; + }; + + let expected_pin = format!("host-native:{}", def.agent_type); + let mut mutated = false; + match def.credential_id.as_deref() { + None => { + def.credential_id = Some(expected_pin); + mutated = true; + } + Some(existing) if existing == expected_pin => { + // Already overlaid in a previous run — still eligible for + // the login-state safety refresh below. + } + Some(_) => return false, // operator-set pin: preserve it + } + + if matches!(login_status, LoginStatus::NeedsLogin) && def.active { + def.active = false; + mutated = true; + } + mutated +} + +/// Refresh the login-derived state of an already-seeded host-native +/// adapter row (`agent_id == agent_type`) in-place. Returns `true` +/// when the row changed and should be persisted. +/// +/// `oversight local` re-runs discovery on every startup. A row that +/// already carries the expected `host-native:` pin used to be +/// skipped with "already seeded", leaving a stale `active = true` if +/// the user ran `claude logout` between boots — dispatch would then +/// call an unauthenticated CLI. +/// +/// **Safety-only**: `NeedsLogin` forces `active = false`; we never +/// auto-reactivate on login restore, matching +/// [`apply_host_native_overlay`], so a manual operator disable is not +/// clobbered. `description` is refreshed unconditionally so the +/// recorded binary path stays current. +pub fn refresh_host_native_login_state( + existing: &mut AgentDef, + discovered: &DiscoveredAgent, +) -> bool { + let desired = agent_def_for_discovered(discovered); + let mut changed = false; + if matches!(discovered.login_status, LoginStatus::NeedsLogin) && existing.active { + existing.active = false; + changed = true; + } + if existing.description != desired.description { + existing.description = desired.description; + changed = true; + } + changed +} + +/// One result row from a discovery pass. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DiscoveredAgent { + /// Adapter identifier matching the manifest id (`"claude"`, + /// `"codex"`, ...). Used to construct the agent's + /// `credential_id = "host-native:"`. + pub adapter_kind: String, + /// User-friendly label for the agent list. + pub display_name: String, + /// Absolute path to the resolved binary. Useful for diagnostics + /// ("which `codex` was found?") and for adapters that want to + /// pre-fill `cmd` overrides. + pub binary_path: PathBuf, + /// Best-effort login state from on-disk inspection. + pub login_status: LoginStatus, + /// Free-form metadata surfaced in the UI badge: version, plan + /// tier, model count, config-dir path, etc. Keys are stable; + /// values may evolve as inspection helpers learn more. + pub metadata: BTreeMap, +} + +/// Public entry point. Scans PATH for every known CLI agent and +/// returns the inspection result for each one that's installed. +/// `home_dir` overrides the inspection root for tests; production +/// callers pass `None` and the function consults `HOME` (Unix) / +/// `USERPROFILE` (Windows) via [`resolve_home_dir`]. +pub fn discover_local_agents(home_dir: Option<&Path>) -> Vec { + let home = home_dir.map(Path::to_path_buf).or_else(resolve_home_dir); + KNOWN_CLI_AGENTS + .iter() + .filter_map(|(kind, display)| { + let binary_path = find_in_path(kind)?; + let (login_status, metadata) = inspect(kind, home.as_deref()); + Some(DiscoveredAgent { + adapter_kind: (*kind).into(), + display_name: (*display).into(), + binary_path, + login_status, + metadata, + }) + }) + .collect() +} + +/// Convenience: resolve `HOME` (Unix) or `USERPROFILE` (Windows). The +/// dispatch path doesn't depend on this — only the inspection helpers +/// do — so a missing value just means `LoginStatus::Unknown`. +pub fn resolve_home_dir() -> Option { + #[cfg(unix)] + { + std::env::var_os("HOME").map(PathBuf::from) + } + #[cfg(not(unix))] + { + std::env::var_os("USERPROFILE").map(PathBuf::from) + } +} + +/// Resolve a binary on PATH. Returns the first matching executable in +/// PATH order, mirroring `which`-style precedence, with symlinks +/// resolved via [`std::fs::canonicalize`] so the returned path always +/// points at the actual on-disk binary (not at e.g. `/usr/bin/claude` +/// pointing to `/opt/claude/claude`). On Unix the file must be +/// executable; on Windows we accept common executable extensions. +/// Returns `None` if PATH is unset or no match exists. +pub fn find_in_path(binary: &str) -> Option { + let paths = std::env::var_os("PATH")?; + for dir in std::env::split_paths(&paths) { + let direct = dir.join(binary); + if is_executable_file(&direct) { + return Some(canonicalize_or_keep(&direct)); + } + #[cfg(not(unix))] + { + for ext in ["exe", "cmd", "bat", "ps1"] { + let candidate = dir.join(format!("{binary}.{ext}")); + if candidate.is_file() { + return Some(canonicalize_or_keep(&candidate)); + } + } + } + } + None +} + +/// ADR-011 review #7: prefer the canonical path so the description +/// recorded on the AgentDef points at the actual binary file, +/// not at a symlink that could be retargeted later. Canonicalize +/// failures fall back to the original path so a missing-on-rerun +/// link doesn't make discovery brittle. +fn canonicalize_or_keep(path: &Path) -> PathBuf { + std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) +} + +#[cfg(unix)] +fn is_executable_file(path: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + match std::fs::metadata(path) { + Ok(meta) => meta.is_file() && (meta.permissions().mode() & 0o111 != 0), + Err(_) => false, + } +} + +#[cfg(not(unix))] +fn is_executable_file(path: &Path) -> bool { + path.is_file() +} + +fn inspect(kind: &str, home: Option<&Path>) -> (LoginStatus, BTreeMap) { + let Some(home) = home else { + return (LoginStatus::Unknown, BTreeMap::new()); + }; + match kind { + "claude" => inspect_dir_existence(home.join(".claude"), "~/.claude"), + "codex" => inspect_dir_existence(home.join(".codex"), "~/.codex"), + // openclaw / hermes inspection helpers are scheduled for a + // follow-up (ADR-011 §"Open questions"). PATH detection still + // surfaces them in the list with `Unknown` status; the UI can + // tell the user "we found this CLI but can't tell if you're + // logged in — try invoking it and see". + _ => (LoginStatus::Unknown, BTreeMap::new()), + } +} + +/// Existence-based inspection: a config directory under HOME is the +/// CLI's persistent state. Presence is a useful proxy for "the user +/// has logged in at least once"; absence is a strong "needs login" +/// signal. We deliberately do NOT parse the directory contents — the +/// format of `~/.claude/` is internal to Claude Code and likely to +/// change. +fn inspect_dir_existence( + config_dir: PathBuf, + label: &str, +) -> (LoginStatus, BTreeMap) { + let mut metadata = BTreeMap::new(); + metadata.insert("config_dir".into(), serde_json::Value::String(label.into())); + if config_dir.is_dir() { + metadata.insert( + "config_dir_path".into(), + serde_json::Value::String(config_dir.display().to_string()), + ); + (LoginStatus::LoggedIn { account: None }, metadata) + } else { + (LoginStatus::NeedsLogin, metadata) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::sync::Mutex; + use tempfile::TempDir; + + /// Cargo runs unit tests in parallel; the PATH-mutating helpers + /// below would race without serialization. A module-level mutex + /// is sufficient because only this test module touches PATH. + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + /// Build a fake PATH directory tree with an executable file + /// named `name`. Returns the tempdir (drop it to clean up). + fn fake_path_with(executable: &str) -> TempDir { + let dir = TempDir::new().expect("tempdir"); + let path = dir.path().join(executable); + fs::write(&path, "#!/bin/sh\necho fake\n").expect("write"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&path, perms).unwrap(); + } + dir + } + + /// Run `closure` with PATH temporarily set to `path_entries`. The + /// original PATH is restored even if the closure panics. Holds + /// `ENV_LOCK` for the whole call to avoid races with other tests + /// in this module. + fn with_path(path_entries: &[&Path], closure: F) { + // Tolerate a poisoned lock — a previous test panicked but the + // env was already restored in its drop. We still want to run. + let _guard = ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let original = std::env::var_os("PATH"); + let joined = std::env::join_paths(path_entries.iter().copied()).expect("join_paths"); + std::env::set_var("PATH", &joined); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(closure)); + match original { + Some(value) => std::env::set_var("PATH", value), + None => std::env::remove_var("PATH"), + } + if let Err(payload) = result { + std::panic::resume_unwind(payload); + } + } + + #[test] + fn find_in_path_returns_first_match_in_path_order() { + let first = fake_path_with("claude"); + let second = fake_path_with("claude"); + with_path(&[first.path(), second.path()], || { + let found = find_in_path("claude").expect("found"); + assert!( + found.starts_with(first.path()), + "expected first PATH entry to win; got {}", + found.display() + ); + }); + } + + #[test] + fn find_in_path_returns_none_when_binary_absent() { + let dir = TempDir::new().unwrap(); + with_path(&[dir.path()], || { + assert!(find_in_path("definitely-not-a-real-cli").is_none()); + }); + } + + #[cfg(unix)] + #[test] + fn find_in_path_skips_non_executable_files() { + let dir = TempDir::new().unwrap(); + let plain = dir.path().join("claude"); + fs::write(&plain, "not executable").unwrap(); + // No chmod +x — leave it 0644. + with_path(&[dir.path()], || { + assert!( + find_in_path("claude").is_none(), + "a non-executable file must not satisfy PATH lookup" + ); + }); + } + + #[test] + fn inspect_returns_logged_in_when_config_dir_exists() { + let home = TempDir::new().unwrap(); + fs::create_dir_all(home.path().join(".claude")).unwrap(); + let (status, metadata) = inspect("claude", Some(home.path())); + assert_eq!(status, LoginStatus::LoggedIn { account: None }); + assert_eq!( + metadata.get("config_dir").and_then(|v| v.as_str()), + Some("~/.claude") + ); + assert!(metadata.contains_key("config_dir_path")); + } + + #[test] + fn inspect_returns_needs_login_when_config_dir_absent() { + let home = TempDir::new().unwrap(); + // Intentionally do NOT create ~/.claude. + let (status, metadata) = inspect("claude", Some(home.path())); + assert_eq!(status, LoginStatus::NeedsLogin); + assert_eq!( + metadata.get("config_dir").and_then(|v| v.as_str()), + Some("~/.claude") + ); + assert!( + !metadata.contains_key("config_dir_path"), + "should not advertise a path that doesn't exist" + ); + } + + #[test] + fn inspect_returns_unknown_for_unsupported_adapter() { + let home = TempDir::new().unwrap(); + let (status, metadata) = inspect("hermes", Some(home.path())); + assert_eq!(status, LoginStatus::Unknown); + assert!(metadata.is_empty()); + } + + #[test] + fn inspect_returns_unknown_when_home_missing() { + let (status, metadata) = inspect("claude", None); + assert_eq!(status, LoginStatus::Unknown); + assert!(metadata.is_empty()); + } + + #[test] + fn discover_local_agents_returns_only_present_binaries() { + // PATH with exactly one entry containing `codex` only. + let codex_dir = fake_path_with("codex"); + let home = TempDir::new().unwrap(); + fs::create_dir_all(home.path().join(".codex")).unwrap(); + + with_path(&[codex_dir.path()], || { + let found = discover_local_agents(Some(home.path())); + assert_eq!( + found.len(), + 1, + "only the binaries actually on PATH should appear" + ); + assert_eq!(found[0].adapter_kind, "codex"); + assert_eq!(found[0].display_name, "Codex CLI"); + assert_eq!( + found[0].login_status, + LoginStatus::LoggedIn { account: None } + ); + }); + } + + /// The login_status discriminator is part of the wire format the + /// frontend reads; pin the JSON shape so a refactor can't quietly + /// rename it. + #[test] + fn login_status_serialises_with_kebab_kind_tag() { + let logged = serde_json::to_value(LoginStatus::LoggedIn { + account: Some("me@example.com".into()), + }) + .unwrap(); + assert_eq!( + logged, + serde_json::json!({"kind": "logged-in", "account": "me@example.com"}) + ); + + let needs = serde_json::to_value(LoginStatus::NeedsLogin).unwrap(); + assert_eq!(needs, serde_json::json!({"kind": "needs-login"})); + + let unknown = serde_json::to_value(LoginStatus::Unknown).unwrap(); + assert_eq!(unknown, serde_json::json!({"kind": "unknown"})); + } + + /// agent_def_for_discovered is the bridge between discovery and + /// the dispatcher. The fields it sets are the contract local mode + /// relies on — pin them here so a refactor can't quietly drop + /// `credential_id = "host-native:..."` (which would silently + /// route the spawned CLI through whatever Auto picks). + #[test] + fn agent_def_for_discovered_builds_host_native_local_agent() { + let discovered = DiscoveredAgent { + adapter_kind: "claude".into(), + display_name: "Claude Code".into(), + binary_path: PathBuf::from("/usr/local/bin/claude"), + login_status: LoginStatus::LoggedIn { account: None }, + metadata: BTreeMap::new(), + }; + let def = agent_def_for_discovered(&discovered); + assert_eq!(def.agent_id, "claude"); + assert_eq!(def.agent_type, "claude"); + assert_eq!(def.display_name, "Claude Code"); + assert_eq!( + def.credential_id.as_deref(), + Some("host-native:claude"), + "must set the ADR-011 host-native pin so the resolver bypasses vault" + ); + let description = def.description.as_deref().unwrap_or(""); + assert!( + description.contains("/usr/local/bin/claude"), + "description should surface the discovered binary path for ops visibility" + ); + // ADR-011 review #7: phrasing must not imply this exact path + // will be exec'd at dispatch — the worker re-resolves PATH. + assert!( + description.contains("detected at") && description.contains("re-resolves PATH"), + "description must clarify discovery-time vs dispatch-time path resolution; got: {description}" + ); + assert!( + def.active, + "LoggedIn agents must seed as active so they appear in the active list" + ); + } + + /// ADR-011 review #8: NeedsLogin agents must seed inactive so the + /// UI can prompt the user to run `claude login` instead of letting + /// the first dispatch attempt fail with an auth error. + #[test] + fn agent_def_for_discovered_inactive_when_needs_login() { + let discovered = DiscoveredAgent { + adapter_kind: "codex".into(), + display_name: "Codex CLI".into(), + binary_path: PathBuf::from("/usr/local/bin/codex"), + login_status: LoginStatus::NeedsLogin, + metadata: BTreeMap::new(), + }; + let def = agent_def_for_discovered(&discovered); + assert!(!def.active, "NeedsLogin agents must NOT seed as active"); + assert_eq!( + def.credential_id.as_deref(), + Some("host-native:codex"), + "credential routing should still be host-native; the row just isn't active yet" + ); + } + + /// `Unknown` (no inspection helper for this adapter, or HOME + /// unresolved) must seed active. Refusing to seed a discovered + /// CLI just because we can't introspect its config is a worse UX + /// than seeding optimistically and letting the user disable it. + #[test] + fn agent_def_for_discovered_active_when_login_status_unknown() { + let discovered = DiscoveredAgent { + adapter_kind: "hermes".into(), + display_name: "Hermes".into(), + binary_path: PathBuf::from("/usr/local/bin/hermes"), + login_status: LoginStatus::Unknown, + metadata: BTreeMap::new(), + }; + let def = agent_def_for_discovered(&discovered); + assert!(def.active, "Unknown login status defaults to active"); + } + + fn make_role_agent(agent_id: &str, agent_type: &str) -> AgentDef { + let mut def = AgentDef::new(agent_id, agent_id, "do work"); + def.agent_type = agent_type.into(); + def.active = true; + def + } + + #[test] + fn apply_host_native_overlay_pins_seeded_role_when_adapter_logged_in() { + let mut def = make_role_agent("requirement_analyzer", "claude"); + let mut map = std::collections::HashMap::new(); + map.insert( + "claude".to_string(), + LoginStatus::LoggedIn { account: None }, + ); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!(mutated); + assert_eq!( + def.credential_id.as_deref(), + Some("host-native:claude"), + "logged-in adapter must overlay the canonical host-native pin" + ); + assert!(def.active, "logged-in adapter must leave active = true"); + } + + #[test] + fn apply_host_native_overlay_marks_seeded_role_inactive_when_adapter_needs_login() { + // P1 from external review: when discovery flagged the adapter + // row as NeedsLogin (=> inactive), the overlaid role agents + // pointing at the same adapter MUST also flip to inactive. + // Otherwise dispatch would still hit the unauthenticated CLI + // even though the directly-discovered claude row is disabled. + let mut def = make_role_agent("requirement_analyzer", "claude"); + let mut map = std::collections::HashMap::new(); + map.insert("claude".to_string(), LoginStatus::NeedsLogin); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!(mutated, "overlay still records the canonical pin"); + assert_eq!(def.credential_id.as_deref(), Some("host-native:claude")); + assert!( + !def.active, + "NeedsLogin adapter must propagate to seeded role agents (inactive)" + ); + } + + #[test] + fn apply_host_native_overlay_preserves_existing_credential_pin() { + let mut def = make_role_agent("requirement_analyzer", "claude"); + def.credential_id = Some("vault:special".into()); + let mut map = std::collections::HashMap::new(); + map.insert( + "claude".to_string(), + LoginStatus::LoggedIn { account: None }, + ); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!(!mutated, "operator-set pins must not be overwritten"); + assert_eq!(def.credential_id.as_deref(), Some("vault:special")); + } + + #[test] + fn apply_host_native_overlay_skips_directly_discovered_adapter_row() { + // The discovery-seeded row has agent_id == agent_type and + // already got the right credential + active from + // agent_def_for_discovered; overlaying it again would + // re-mutate fields the seeder already settled. + let mut def = make_role_agent("claude", "claude"); + def.credential_id = None; // simulate broken state + let mut map = std::collections::HashMap::new(); + map.insert( + "claude".to_string(), + LoginStatus::LoggedIn { account: None }, + ); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!( + !mutated, + "the discovery-seeded adapter row owns its own credential + active state" + ); + } + + #[test] + fn apply_host_native_overlay_ignores_unmatched_agent_type() { + let mut def = make_role_agent("custom", "hermes"); + let mut map = std::collections::HashMap::new(); + map.insert( + "claude".to_string(), + LoginStatus::LoggedIn { account: None }, + ); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!(!mutated); + assert!(def.credential_id.is_none()); + assert!(def.active); + } + + #[test] + fn apply_host_native_overlay_marks_already_overlaid_role_inactive_when_login_lost() { + // Re-run drift: a role agent pinned host-native:claude in a + // previous boot must flip inactive when this boot's discovery + // reports NeedsLogin — closing the "dispatch to unauthenticated + // CLI after logout" gap. + let mut def = make_role_agent("requirement_analyzer", "claude"); + def.credential_id = Some("host-native:claude".into()); + def.active = true; + let mut map = std::collections::HashMap::new(); + map.insert("claude".to_string(), LoginStatus::NeedsLogin); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!(mutated, "lost login must flip the already-overlaid row"); + assert!(!def.active); + assert_eq!(def.credential_id.as_deref(), Some("host-native:claude")); + } + + #[test] + fn apply_host_native_overlay_does_not_reactivate_already_overlaid_role() { + // A manually-disabled role agent must stay disabled even when + // the adapter is logged in — we never auto-reactivate. + let mut def = make_role_agent("requirement_analyzer", "claude"); + def.credential_id = Some("host-native:claude".into()); + def.active = false; + let mut map = std::collections::HashMap::new(); + map.insert( + "claude".to_string(), + LoginStatus::LoggedIn { account: None }, + ); + + let mutated = apply_host_native_overlay(&mut def, &map); + assert!( + !mutated, + "logged-in adapter must not re-enable a disabled role" + ); + assert!(!def.active); + } + + fn discovered(adapter: &str, login: LoginStatus) -> DiscoveredAgent { + DiscoveredAgent { + adapter_kind: adapter.into(), + display_name: format!("{adapter} CLI"), + binary_path: PathBuf::from(format!("/usr/bin/{adapter}")), + login_status: login, + metadata: BTreeMap::new(), + } + } + + #[test] + fn refresh_flips_active_to_false_when_login_lost() { + // First boot logged-in => active=true; user logs out; re-run + // discovery reports NeedsLogin => existing row must flip off. + let mut existing = agent_def_for_discovered(&discovered( + "claude", + LoginStatus::LoggedIn { account: None }, + )); + assert!(existing.active); + + let changed = refresh_host_native_login_state( + &mut existing, + &discovered("claude", LoginStatus::NeedsLogin), + ); + assert!(changed); + assert!(!existing.active, "lost login must disable the agent"); + } + + #[test] + fn refresh_does_not_reactivate_on_login_restored() { + // Conservative policy: never auto-flip inactive → active on + // login restore, so a manual operator disable survives. The + // agent stays inactive until manually re-enabled. + let mut existing = agent_def_for_discovered(&discovered("claude", LoginStatus::NeedsLogin)); + assert!(!existing.active); + + let changed = refresh_host_native_login_state( + &mut existing, + &discovered("claude", LoginStatus::LoggedIn { account: None }), + ); + assert!(!changed, "login restore must not auto-reactivate"); + assert!(!existing.active); + } + + #[test] + fn refresh_is_noop_when_login_state_unchanged() { + let mut existing = agent_def_for_discovered(&discovered( + "claude", + LoginStatus::LoggedIn { account: None }, + )); + let changed = refresh_host_native_login_state( + &mut existing, + &discovered("claude", LoginStatus::LoggedIn { account: None }), + ); + assert!( + !changed, + "unchanged login state must not mark the row dirty" + ); + } + + /// Order-preservation: KNOWN_CLI_AGENTS is the canonical UI order + /// (claude → codex → openclaw → hermes). The discovery loop + /// iterates in that order, so when multiple binaries are present + /// the resulting Vec respects it. Frontend lists rely on this. + #[test] + fn discover_local_agents_preserves_canonical_order() { + // Put two binaries on PATH out of order; output must still + // be claude before codex. + let dir = TempDir::new().unwrap(); + for name in ["codex", "claude"] { + let path = dir.path().join(name); + fs::write(&path, "#!/bin/sh\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&path, perms).unwrap(); + } + } + let home = TempDir::new().unwrap(); + with_path(&[dir.path()], || { + let found = discover_local_agents(Some(home.path())); + assert_eq!(found.len(), 2); + assert_eq!(found[0].adapter_kind, "claude"); + assert_eq!(found[1].adapter_kind, "codex"); + }); + } +} diff --git a/crates/oversight-agents/src/lib.rs b/crates/oversight-agents/src/lib.rs index d6e800db..295906bf 100644 --- a/crates/oversight-agents/src/lib.rs +++ b/crates/oversight-agents/src/lib.rs @@ -4,6 +4,7 @@ pub mod catalog; pub mod cli_backend; pub mod cli_bridge; pub mod collection_prompt; +pub mod discovery; pub mod eligibility; pub mod product_plugin; mod prompts; diff --git a/crates/oversight-worker/src/adapter.rs b/crates/oversight-worker/src/adapter.rs index e7a4f1b5..3fe26205 100644 --- a/crates/oversight-worker/src/adapter.rs +++ b/crates/oversight-worker/src/adapter.rs @@ -209,13 +209,20 @@ pub struct CleanupContext { /// When a name is given but the env var isn't set the helper also falls back /// to the defaults — so manifests can declare one override without forcing /// the other to be set. +/// +/// Returns `(command, args, args_from_env)`. `args_from_env` is true when +/// the operator supplied a full args override via the env var, in which +/// case callers MUST skip subsequent `forced_args` rewriting — the +/// operator opt-out is the documented escape hatch for hosts where the +/// manifest's default sandbox/approval pins are unrunnable (e.g. broken +/// bubblewrap on locked-down Linux). pub fn resolve_acp_bin_and_args( bin_env: Option<&str>, args_env: Option<&str>, default_command: &str, default_args: &[&str], env_vars: &HashMap, -) -> (String, Vec) { +) -> (String, Vec, bool) { let command = bin_env .and_then(|name| { env_vars @@ -225,17 +232,41 @@ pub fn resolve_acp_bin_and_args( }) .unwrap_or_else(|| default_command.to_string()); - let args = args_env - .and_then(|name| { - env_vars - .get(name) - .cloned() - .or_else(|| std::env::var(name).ok()) + let mut args_from_env = false; + let raw_args_env = args_env.and_then(|name| { + env_vars + .get(name) + .cloned() + .or_else(|| std::env::var(name).ok()) + .map(|raw| (name, raw)) + }); + let parsed = raw_args_env.and_then(|(name, raw)| { + match serde_json::from_str::>(&raw) { + Ok(parsed) => Some(parsed), + Err(err) => { + // R3 review (P3): silent fallback when an operator + // sets `_ACP_ARGS_JSON` to malformed JSON + // hides the cause of "why are my flags ignored?". + // Warn so the launcher behaviour is observable; we + // still fall back to the manifest defaults below to + // keep the adapter runnable. + tracing::warn!( + env_var = %name, + error = %err, + "ignoring malformed ACP args JSON; falling back to manifest defaults" + ); + None + } + } + }); + let args = parsed + .map(|v| { + args_from_env = true; + v }) - .and_then(|v| serde_json::from_str::>(&v).ok()) .unwrap_or_else(|| default_args.iter().map(|s| s.to_string()).collect()); - (command, args) + (command, args, args_from_env) } /// Convert our `McpServerConfig` to the ACP schema `McpServer`. @@ -383,13 +414,23 @@ pub fn mcp_token_env_name(pattern: &str, server_name: &str) -> String { pattern.replace("{NAME}", &normalized) } -/// Inject MCP server URLs and bearer-token env vars as `-c {prefix}.{name}.…` -/// switches. CLIs like Codex consume MCP config via TOML overrides on the -/// command line rather than a settings file or the ACP session. +/// Inject MCP server URLs and per-header env-var bridges as +/// `-c {prefix}.{name}.…` switches. CLIs like Codex consume MCP config via +/// TOML overrides on the command line rather than a settings file or the +/// ACP session. /// /// `bearer_token_env_pattern` (e.g. `"OVERSIGHT_MCP_TOKEN_{NAME}"`) is the -/// template for the env-var name carrying the extracted Bearer token. When -/// `None`, no token bridging is performed. +/// template for the env-var name carrying the forwarded header values. +/// When `None`, no header bridging is performed. +/// +/// Each entry in `server.headers` is forwarded: +/// * `Authorization: Bearer ` → bridged via +/// `{key_prefix}.bearer_token_env_var` (standard codex bearer path). +/// * Any other header (e.g. `X-Agent-ID`) → bridged via +/// `{key_prefix}.env_http_headers.` so codex sends the +/// literal header. This is what oversight's `/mcp` endpoint expects +/// for agent identity — abusing Bearer would conflict with the +/// cloud-mode contract that Bearer carries a user session JWT. pub fn inject_command_line_mcp_config( args: &mut Vec, env_vars: &mut HashMap, @@ -407,22 +448,29 @@ pub fn inject_command_line_mcp_config( let Some(pattern) = bearer_token_env_pattern else { continue; }; - let Some(token) = server.headers.as_ref().and_then(|headers| { - headers - .get("Authorization") - .and_then(|auth| auth.strip_prefix("Bearer ")) - .or_else(|| headers.get("X-Agent-ID").map(String::as_str)) - .or_else(|| headers.get("x-agent-id").map(String::as_str)) - }) else { + let Some(headers) = server.headers.as_ref() else { continue; }; - let env_name = mcp_token_env_name(pattern, &server.name); - env_vars.insert(env_name.clone(), token.to_string()); - append_config_override( - args, - &format!("{key_prefix}.bearer_token_env_var"), - &toml_string(&env_name), - ); + for (header_name, header_value) in headers { + let bearer = header_name + .eq_ignore_ascii_case("authorization") + .then(|| header_value.strip_prefix("Bearer ").unwrap_or(header_value)); + let env_name = mcp_token_env_name(pattern, &format!("{}_{}", server.name, header_name)); + env_vars.insert(env_name.clone(), bearer.unwrap_or(header_value).to_string()); + if bearer.is_some() { + append_config_override( + args, + &format!("{key_prefix}.bearer_token_env_var"), + &toml_string(&env_name), + ); + } else { + append_config_override( + args, + &format!("{key_prefix}.env_http_headers.{header_name}"), + &toml_string(&env_name), + ); + } + } } } @@ -461,6 +509,43 @@ fn acp_timeout(env_vars: &HashMap, key: &str, default_secs: u64) /// /// Spawns an `AcpClient`, manages session setup (load or new), optionally sets the model, /// then runs `prompt()` in a background issue. Returns an `AgentProcess` with the event receiver. +/// Outcome of the `decide_set_model` policy check. +/// +/// `Skip` — caller-supplied model was empty; do nothing. +/// `Apply` — model is in the adapter's advertised list (or the +/// adapter didn't advertise one); call `set_model`. +/// `FallbackToDefault` — model is non-empty AND the adapter +/// advertised a list AND the model is not in it; skip the call so +/// the session uses its default, and log a warning so the operator +/// can fix the agent config. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum SetModelDecision { + Skip, + Apply, + FallbackToDefault, +} + +/// Pure policy: should we call `session/set_config_option` for this +/// (requested_model, advertised_models) pair? See `SetModelDecision`. +pub(crate) fn decide_set_model(requested: &str, advertised: &[String]) -> SetModelDecision { + if requested.is_empty() { + return SetModelDecision::Skip; + } + if advertised.is_empty() { + // Some adapters (older Codex) don't advertise a model list at + // all. Don't second-guess them — pass the value through and + // let the adapter accept or reject. The previous fail-closed + // contract held by `client.set_model(...).await?` is what + // surfaces an actual rejection. + return SetModelDecision::Apply; + } + if advertised.iter().any(|id| id == requested) { + SetModelDecision::Apply + } else { + SetModelDecision::FallbackToDefault + } +} + pub async fn start_via_acp( acp_config: AcpConfig, ctx: &StartContext, @@ -500,10 +585,46 @@ pub async fn start_via_acp( client.new_session(mcp_servers).await?; } - // Set model if configured - if let Some(model) = model { - if !model.is_empty() { - client.set_model(model).await?; + // Set model if configured. + // + // ADR-011 review (e2e finding): the agent edit API silently + // accepts any model string, but `session/set_config_option` + // only honours model ids the adapter advertises via + // `SessionModelState.available_models`. A typo (e.g. + // `claude-sonnet-4-5` when the CLI expects + // `claude-sonnet-4-5-20250929`) used to abort the entire run + // 3 seconds in with `failed to set ACP model to ''`. + // + // `decide_set_model` is the truth table: skip empty values, + // proceed when the adapter didn't advertise a list, proceed + // when the requested id is in the list, and fall back to the + // session default (with a warning) otherwise. Pure function so + // it's testable without a mock ACP process. + if let Some(requested) = model.as_deref() { + let advertised: Vec = client + .last_session_models() + .map(|state| { + state + .available_models + .iter() + .map(|info| info.model_id.0.to_string()) + .collect() + }) + .unwrap_or_default(); + match decide_set_model(requested, &advertised) { + SetModelDecision::Skip => {} + SetModelDecision::Apply => { + client.set_model(requested).await?; + } + SetModelDecision::FallbackToDefault => { + tracing::warn!( + requested_model = %requested, + advertised = ?advertised, + "configured model is not in the adapter's advertised list; \ + falling back to the session default. Edit the agent's model field \ + to one of the advertised values to silence this warning." + ); + } } } @@ -543,6 +664,64 @@ mod tests { ENV_LOCK.get_or_init(|| Mutex::new(())) } + // ── ADR-011 e2e fix: model-set policy ──────────────────────── + + /// Empty model → Skip. The adapter's session default is used, + /// matching the historical pre-fix behaviour when `model` was + /// `None` or `""`. + #[test] + fn decide_set_model_empty_request_skips() { + let advertised = vec!["claude-sonnet-4-5-20250929".to_string()]; + assert_eq!(decide_set_model("", &advertised), SetModelDecision::Skip); + assert_eq!(decide_set_model("", &[]), SetModelDecision::Skip); + } + + /// Adapter that didn't advertise a list → Apply. Some adapters + /// (older Codex builds) don't populate `SessionModelState`. The + /// fallback path would silently swallow the request — don't. + #[test] + fn decide_set_model_no_advertised_list_applies() { + assert_eq!(decide_set_model("anything", &[]), SetModelDecision::Apply); + } + + /// Model is in the advertised list → Apply. + #[test] + fn decide_set_model_match_in_list_applies() { + let advertised = vec![ + "claude-sonnet-4-5-20250929".to_string(), + "claude-opus-4-5-20250929".to_string(), + ]; + assert_eq!( + decide_set_model("claude-sonnet-4-5-20250929", &advertised), + SetModelDecision::Apply + ); + } + + /// The regression case: `claude-sonnet-4-5` looks valid to a + /// human, but the adapter only advertises the dated id. + /// Pre-fix: dispatch aborted with `failed to set ACP model`. + /// Post-fix: warn and fall back to the session default so the + /// run continues. + #[test] + fn decide_set_model_mismatch_falls_back() { + let advertised = vec!["claude-sonnet-4-5-20250929".to_string()]; + assert_eq!( + decide_set_model("claude-sonnet-4-5", &advertised), + SetModelDecision::FallbackToDefault + ); + } + + /// Empty match-list → Apply. Pinned because the helper's first + /// branch handles this; the test stops a refactor that swaps + /// the order from quietly turning every dispatch into a fallback. + #[test] + fn decide_set_model_does_not_fall_back_when_list_unavailable() { + assert_ne!( + decide_set_model("any", &[]), + SetModelDecision::FallbackToDefault + ); + } + #[test] fn agent_event_text_delta_serde_roundtrip() { let event = AgentEvent::TextDelta { @@ -770,7 +949,7 @@ mod tests { std::env::remove_var("MY_ARGS"); } let env = HashMap::new(); - let (cmd, args) = resolve_acp_bin_and_args( + let (cmd, args, from_env) = resolve_acp_bin_and_args( Some("MY_BIN"), Some("MY_ARGS"), "npx", @@ -779,6 +958,7 @@ mod tests { ); assert_eq!(cmd, "npx"); assert_eq!(args, vec!["-y", "@test/pkg"]); + assert!(!from_env, "defaults must not be marked as env-sourced"); } #[test] @@ -786,7 +966,7 @@ mod tests { let mut env = HashMap::new(); env.insert("MY_BIN".into(), "/custom/binary".into()); env.insert("MY_ARGS".into(), r#"["--flag","value"]"#.into()); - let (cmd, args) = resolve_acp_bin_and_args( + let (cmd, args, from_env) = resolve_acp_bin_and_args( Some("MY_BIN"), Some("MY_ARGS"), "npx", @@ -795,6 +975,7 @@ mod tests { ); assert_eq!(cmd, "/custom/binary"); assert_eq!(args, vec!["--flag", "value"]); + assert!(from_env, "json args from env must flag args_from_env"); } #[test] @@ -805,7 +986,7 @@ mod tests { } let mut env = HashMap::new(); env.insert("MY_ARGS".into(), "not valid json".into()); - let (cmd, args) = resolve_acp_bin_and_args( + let (cmd, args, from_env) = resolve_acp_bin_and_args( Some("MY_BIN"), Some("MY_ARGS"), "npx", @@ -818,13 +999,17 @@ mod tests { vec!["-y", "@test/pkg"], "invalid JSON should fall back to defaults" ); + assert!( + !from_env, + "fallback to defaults must NOT flag args_from_env" + ); } #[test] fn resolve_acp_bin_empty_command_env() { let mut env = HashMap::new(); env.insert("MY_BIN".into(), "".into()); - let (cmd, _args) = + let (cmd, _args, _from_env) = resolve_acp_bin_and_args(Some("MY_BIN"), Some("MY_ARGS"), "npx", &[], &env); assert_eq!(cmd, "", "empty string override is still used"); } @@ -833,7 +1018,7 @@ mod tests { fn resolve_acp_bin_empty_args_json() { let mut env = HashMap::new(); env.insert("MY_ARGS".into(), "[]".into()); - let (_cmd, args) = resolve_acp_bin_and_args( + let (_cmd, args, from_env) = resolve_acp_bin_and_args( Some("MY_BIN"), Some("MY_ARGS"), "npx", @@ -844,6 +1029,7 @@ mod tests { args.is_empty(), "empty JSON array should produce empty args" ); + assert!(from_env, "empty array from env still counts as override"); } #[test] @@ -855,7 +1041,7 @@ mod tests { } let env = HashMap::new(); - let (cmd, args) = resolve_acp_bin_and_args( + let (cmd, args, _from_env) = resolve_acp_bin_and_args( Some("MY_BIN"), Some("MY_ARGS"), "npx", @@ -970,6 +1156,80 @@ mod tests { assert!(args.is_empty(), "stdio-only servers are not CLI-injectable"); } + #[test] + fn inject_command_line_mcp_config_forwards_x_agent_id_via_env_http_headers() { + // Oversight's /mcp endpoint authenticates via the `X-Agent-ID` + // header. Codex CLI bridges header values from env vars via the + // `env_http_headers` table. The injection must produce a config + // that matches that contract; without it codex labels the server + // "Auth: Unsupported" and never loads the tools. + let mut args = Vec::new(); + let mut env_vars = HashMap::new(); + let mut headers = HashMap::new(); + headers.insert("X-Agent-ID".into(), "decomposer".into()); + let servers = vec![McpServerConfig { + name: "awaken".into(), + url: Some("http://127.0.0.1:17880/mcp".into()), + command: None, + args: None, + headers: Some(headers), + }]; + inject_command_line_mcp_config( + &mut args, + &mut env_vars, + &servers, + "mcp_servers", + Some("OVERSIGHT_MCP_TOKEN_{NAME}"), + ); + assert!(args.contains(&"mcp_servers.awaken.url=\"http://127.0.0.1:17880/mcp\"".into())); + let env_key = "OVERSIGHT_MCP_TOKEN_AWAKEN_X_AGENT_ID"; + assert_eq!( + env_vars.get(env_key).map(String::as_str), + Some("decomposer") + ); + assert!(args.contains(&format!( + "mcp_servers.awaken.env_http_headers.X-Agent-ID=\"{env_key}\"" + ))); + assert!( + !args.iter().any(|a| a.contains("bearer_token_env_var")), + "non-Authorization headers must NOT use bearer_token_env_var" + ); + } + + #[test] + fn inject_command_line_mcp_config_bridges_authorization_via_bearer_token() { + let mut args = Vec::new(); + let mut env_vars = HashMap::new(); + let mut headers = HashMap::new(); + headers.insert("Authorization".into(), "Bearer secret-token".into()); + let servers = vec![McpServerConfig { + name: "remote".into(), + url: Some("https://api.example/mcp".into()), + command: None, + args: None, + headers: Some(headers), + }]; + inject_command_line_mcp_config( + &mut args, + &mut env_vars, + &servers, + "mcp_servers", + Some("OVERSIGHT_MCP_TOKEN_{NAME}"), + ); + let env_key = "OVERSIGHT_MCP_TOKEN_REMOTE_AUTHORIZATION"; + assert_eq!( + env_vars.get(env_key).map(String::as_str), + Some("secret-token"), + "Bearer prefix must be stripped before storing in env var" + ); + assert!( + args.contains(&format!( + "mcp_servers.remote.bearer_token_env_var=\"{env_key}\"" + )), + "Authorization header must use the standard bearer_token_env_var path" + ); + } + #[test] fn mcp_token_env_name_normalizes_dashes_and_case() { assert_eq!( diff --git a/crates/oversight-worker/src/adapters/generic.rs b/crates/oversight-worker/src/adapters/generic.rs index 644e5c3c..810007da 100644 --- a/crates/oversight-worker/src/adapters/generic.rs +++ b/crates/oversight-worker/src/adapters/generic.rs @@ -895,7 +895,7 @@ impl CliAgentAdapter for GenericCliAdapter { async fn start(&self, ctx: &StartContext) -> anyhow::Result { let default_args: Vec<&str> = self.config.args.iter().map(String::as_str).collect(); - let (command, mut args) = resolve_acp_bin_and_args( + let (command, mut args, args_from_env) = resolve_acp_bin_and_args( self.config.acp_bin_env.as_deref(), self.config.acp_args_env.as_deref(), &self.config.command, @@ -903,8 +903,15 @@ impl CliAgentAdapter for GenericCliAdapter { &ctx.env_vars, ); - for forced in &self.config.forced_args { - force_config_override(&mut args, &forced.key, &forced.value); + // The env-var args override is the documented operator escape + // hatch for hosts where the manifest's pins are unrunnable + // (e.g. broken bubblewrap → sandbox_mode must drop to + // danger-full-access). Re-applying forced_args here would + // silently undo that, contradicting the manifest contract. + if !args_from_env { + for forced in &self.config.forced_args { + force_config_override(&mut args, &forced.key, &forced.value); + } } let mut env_vars = ctx.env_vars.clone(); From 8623a884e6929f4050ccbd336131dd6a0b15699b Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:56 +0800 Subject: [PATCH 5/6] =?UTF-8?q?=E2=9C=A8=20feat(server):=20oversight=20loc?= =?UTF-8?q?al=20subcommand,=20ServerMode,=20inactive-agent=20dispatch=20gu?= =?UTF-8?q?ard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oversight-authz/src/authorizer.rs | 125 +++ crates/oversight-server/src/api.rs | 787 +++++++++++++++++- crates/oversight-server/src/auth_api.rs | 71 +- crates/oversight-server/src/auth_mw.rs | 48 ++ crates/oversight-server/src/cli_agent_api.rs | 57 +- crates/oversight-server/src/dispatch_api.rs | 414 ++++++++- crates/oversight-server/src/main.rs | 236 +++++- crates/oversight-server/src/state.rs | 38 + crates/oversight-server/src/worker_api.rs | 1 + crates/oversight-server/tests/common/mod.rs | 1 + .../tests/config_hot_reload.rs | 1 + .../oversight-server/tests/startup_smoke.rs | 2 + .../tests/worker_network_e2e.rs | 1 + 13 files changed, 1708 insertions(+), 74 deletions(-) diff --git a/crates/oversight-authz/src/authorizer.rs b/crates/oversight-authz/src/authorizer.rs index cb633eb3..1663db71 100644 --- a/crates/oversight-authz/src/authorizer.rs +++ b/crates/oversight-authz/src/authorizer.rs @@ -9,6 +9,12 @@ use crate::{Principal, Scope, ScopeType}; pub struct Authorizer { pub(crate) index: RwLock, + /// ADR-011: when true every check returns `Decision::Allow` and + /// every visibility query returns `Visibility::All`. Used by the + /// `oversight local` assembly so single-user installs do not + /// require seeding workspace grants. Cloud assembly leaves this + /// `false` and the grant-based code path runs as before. + pub(crate) permissive: bool, } #[derive(Debug, Clone, Default)] @@ -33,12 +39,36 @@ impl Authorizer { pub fn from_index(index: Index) -> Self { Self { index: RwLock::new(index), + permissive: false, } } + /// Build a permissive authorizer: every `check*` returns `Allow`, + /// every `visible*` returns `Visibility::All`. Intended for the + /// `oversight local` single-user assembly (ADR-011) where the + /// product UX is "no login, full access". The internal grant + /// index is initialised empty so subsequent `reload*` calls + /// continue to behave normally; flipping back to non-permissive + /// is not supported because the assembly choice is a startup + /// decision, not a per-request mode. + pub fn permissive() -> Self { + Self { + index: RwLock::new(Index::default()), + permissive: true, + } + } + + /// True when this authorizer was built with [`permissive`]. Exposed + /// for assertions in integration tests and for diagnostics endpoints + /// that want to surface "this is a local-mode install". + pub fn is_permissive(&self) -> bool { + self.permissive + } + pub fn from_snapshot(snapshot: AuthorizerSnapshot) -> Result { Ok(Self { index: RwLock::new(Self::index_from_snapshot(snapshot)?), + permissive: false, }) } @@ -85,6 +115,13 @@ impl Authorizer { } pub fn check_single(&self, p: &Principal, action: &str, resource: Scope) -> Decision { + if self.permissive { + // ADR-011: local-mode short-circuit. Bypasses grant index, + // role catalog, and fallback policy because the single-user + // assembly does not seed grants. + let _ = (p, action, resource); + return Decision::Allow; + } let idx = self.index.read().unwrap(); let ancestors = idx.scope_tree.ancestors(&resource); let grants = idx.grants_for(p); @@ -162,6 +199,10 @@ impl Authorizer { } pub fn check(&self, ctx: &ExecContext, action: &str, resource: Scope) -> Decision { + if self.permissive { + let _ = (ctx, action, resource); + return Decision::Allow; + } let mut worst: Decision = Decision::Allow; for p in &ctx.chain.0 { match self.check_effective(p, action, resource.clone()) { @@ -185,6 +226,10 @@ impl Authorizer { } pub fn visible_single(&self, p: &Principal, action: &str, typ: ScopeType) -> Visibility { + if self.permissive { + let _ = (p, action, typ); + return Visibility::All; + } // Collect all data under the read lock, then drop it before calling check_single. let visible: HashSet = { let idx = self.index.read().unwrap(); @@ -255,6 +300,10 @@ impl Authorizer { } pub fn visible(&self, ctx: &ExecContext, action: &str, typ: ScopeType) -> Visibility { + if self.permissive { + let _ = (ctx, action, typ); + return Visibility::All; + } let mut result = Visibility::All; for p in &ctx.chain.0 { result = result.intersect(self.visible_effective(p, action, typ)); @@ -1006,6 +1055,82 @@ mod tests { } } + // ── ADR-011: permissive (local-mode) authorizer ──────────────── + + /// Every check_single call returns Allow, regardless of action, + /// principal, or resource. Pinned because the local-mode assembly + /// depends on this being a true short-circuit. + #[test] + fn permissive_authorizer_allows_every_check_single() { + let az = Authorizer::permissive(); + assert!(az.is_permissive()); + for action in [ + "document.read", + "document.write", + "document.delete", + "workspace.write", + "agent.write", + ] { + let d = az.check_single( + &Principal::User("local".into()), + action, + Scope::Document("anything".into()), + ); + assert!( + matches!(d, Decision::Allow), + "permissive must allow '{action}' on Document; got {d:?}" + ); + } + } + + /// Chain check (multiple principals) must also short-circuit so + /// agent-on-behalf-of-user flows don't accidentally fall through + /// to grant lookup. + #[test] + fn permissive_authorizer_allows_chain_check() { + let az = Authorizer::permissive(); + let ctx = ExecContext::new(PrincipalChain(vec![ + Principal::User("local".into()), + Principal::Agent("any-agent".into()), + ])); + assert!(matches!( + az.check(&ctx, "issue.create", Scope::Workspace), + Decision::Allow + )); + } + + /// Visibility queries must return All so listings (issues, + /// agents, workspaces) aren't silently filtered to empty in + /// local mode. + #[test] + fn permissive_authorizer_returns_visibility_all() { + let az = Authorizer::permissive(); + let v_single = az.visible_single( + &Principal::User("local".into()), + "document.read", + ScopeType::Document, + ); + assert!(matches!(v_single, Visibility::All)); + + let ctx = ExecContext::new(PrincipalChain(vec![Principal::User("local".into())])); + let v_chain = az.visible(&ctx, "issue.read", ScopeType::Issue); + assert!(matches!(v_chain, Visibility::All)); + } + + /// Default-constructed authorizer (cloud assembly) must NOT be + /// permissive — protects against an accidental field flip. + #[test] + fn non_permissive_authorizer_still_denies_business_actions_without_grants() { + let az = Authorizer::from_index(Index::default()); + assert!(!az.is_permissive()); + let d = az.check_single( + &Principal::User("u1".into()), + "document.read", + Scope::Document("d1".into()), + ); + assert!(matches!(d, Decision::Deny { .. })); + } + #[test] fn reload_picks_up_new_grants_via_index_load() { // We can't easily test against a DB here; instead, test that `reload` swaps diff --git a/crates/oversight-server/src/api.rs b/crates/oversight-server/src/api.rs index ee329422..1fb04202 100644 --- a/crates/oversight-server/src/api.rs +++ b/crates/oversight-server/src/api.rs @@ -6411,6 +6411,24 @@ async fn dispatch_mailbox_issue_run( ); } + // Prepend the agent's system_prompt so CLI-backed agents (codex, + // claude, openclaw) actually see their role instructions. The + // CliBackend → CliInvokeConfig path does not forward system_prompt + // separately, and manifests that write a workspace prompt file + // (e.g. `.codex-instructions.md`) cannot rely on the CLI + // auto-loading it. Putting the system prompt first in the user + // message is the only delivery channel that every CLI honors. + let prompt = match agent_def + .as_ref() + .map(|def| def.system_prompt.trim()) + .filter(|s| !s.is_empty()) + { + Some(sys_prompt) => format!( + "[Agent role instructions]\n{sys_prompt}\n\n---\n\n{prompt}", + prompt = prompt + ), + None => prompt, + }; let prompt = crate::agent_execution::with_background_execution_rules(&prompt); let request = awaken_runtime::RunRequest::new( thread_id.clone(), @@ -7316,7 +7334,18 @@ fn build_decomposition_delivery_summary( fn is_decomposition_agent_assignee(assignee: &oversight_models::IssueAssignee) -> bool { let role = assignee.role.to_ascii_lowercase(); - matches!(role.as_str(), "decomposer" | "planner") + if matches!(role.as_str(), "decomposer" | "planner") { + return true; + } + // The workflow `on_enter` reaction adds default-assignee agents with + // a generic role ("executor"), not "decomposer". Match by agent_id + // suffix so decomposition guidance still reaches conventionally + // named planning agents. + if let Some(agent_id) = assignee.assignee.strip_prefix("agent:") { + let id = agent_id.to_ascii_lowercase(); + return id == "decomposer" || id == "planner" || id.ends_with("_decomposer"); + } + false } fn build_decomposition_agent_guidance( @@ -7807,7 +7836,104 @@ async fn get_agent_resolved_credential( Scope::Agent(def.agent_id.clone()), )?; + // ADR-011 review #9: host-native / env / ext sources resolve + // entirely from the `credential_id` prefix and DO NOT need a + // credential pool (no vault lookup, no cooldown, no LRU touch). + // Synthesise the read-endpoint response directly from the prefix + // so test AppStates without a sidecar SQLite — and any deployment + // that skips pool wiring — still get a meaningful badge for + // those sources. Vault and Auto modes still need the pool. + // + // Review #8: prefix parsing lives in `parse_credential_id_ref`, + // shared with `resolve_credential_for_agent` so the two paths + // can't drift. + if let Some(pin) = def.credential_id.as_deref() { + use crate::credentials::{parse_credential_id_ref, CredentialIdRef}; + match parse_credential_id_ref(pin) { + CredentialIdRef::HostNative { adapter } => { + let valid = adapter == def.agent_type; + return Ok(Json(serde_json::json!({ + "mode": "pinned", + "namespace": if valid { Some("cli-credentials") } else { None }, + "profile_id": if valid { Some(format!("host-native:{adapter}")) } else { None }, + "source": if valid { + Some(serde_json::json!({ + "kind": "host-native", + "adapter_kind": adapter, + })) + } else { None }, + "note": if valid { serde_json::Value::Null } else { + serde_json::Value::String(format!( + "credential_id '{pin}' adapter='{adapter}' does not match agent_type='{}' (fail-closed)", + def.agent_type, + )) + }, + }))); + } + CredentialIdRef::HostNativeMissingAdapter => { + return Ok(Json(serde_json::json!({ + "mode": "pinned", + "namespace": null, + "profile_id": null, + "source": null, + "note": format!( + "credential_id '{pin}' is missing the adapter_kind after 'host-native:' (fail-closed)" + ), + }))); + } + CredentialIdRef::EnvPassthrough { .. } => { + return Ok(Json(serde_json::json!({ + "mode": "pinned", + "namespace": null, + "profile_id": null, + "source": null, + "note": "credential_id 'env:*' (worker env passthrough) is reserved but not yet implemented", + }))); + } + CredentialIdRef::ExternalRef { .. } => { + return Ok(Json(serde_json::json!({ + "mode": "pinned", + "namespace": null, + "profile_id": null, + "source": null, + "note": "credential_id 'ext:*' (external secret manager) is reserved but not yet implemented", + }))); + } + // Vault & Malformed fall through — vault needs the pool, + // malformed gets the resolver's fail-closed message. + CredentialIdRef::Vault { .. } | CredentialIdRef::Malformed => {} + } + } + let Some(pool) = state.credential_pool.as_ref() else { + // Mirror what dispatch would actually do for this agent when + // the pool is absent: a Vault / Malformed pin is a fail-closed + // pinned refusal, not a generic "unavailable". Reuse the shared + // dispatch resolver so the read endpoint and dispatch never + // diverge on the no-pool explanation. + if def.credential_id.is_some() { + return Ok(Json(match crate::credentials::resolve_dispatch_credential( + &state.vault, + None, + &def, + ) + .await + { + Ok(_) => serde_json::json!({ + "mode": "unavailable", + "namespace": null, + "profile_id": null, + "note": "credential pool not configured", + }), + Err(refusal) => serde_json::json!({ + "mode": "pinned", + "namespace": null, + "profile_id": null, + "source": null, + "note": refusal.message, + }), + })); + } return Ok(Json(serde_json::json!({ "mode": "unavailable", "namespace": null, @@ -7829,6 +7955,10 @@ async fn get_agent_resolved_credential( "mode": mode_str, "namespace": resolution.resolved.as_ref().map(|r| &r.namespace), "profile_id": resolution.resolved.as_ref().map(|r| &r.profile_id), + // ADR-011: surface the credential source so the UI can render + // the host-native / vault / env / ext badge without re-parsing + // credential_id. + "source": resolution.resolved.as_ref().map(|r| &r.source), "note": resolution.note, }))) } @@ -10685,6 +10815,37 @@ pub(crate) async fn handle_scheduled_agent_terminal_event( } let is_error = is_agent_terminal_error(&effective_termination); + // R2 review #4: load the awaken RunRecord BEFORE acquiring the + // store mutex. The previous ordering held `state.store.lock()` + // across the `thread_store.load_run().await`, serialising every + // request that needs the store on a file IO. RunRecord lookup + // is independent of the SQLite store; do it first. + // + // The record also gives us `started_at`, used below to dedupe + // agent-authored comments produced during this run (R2 review #3). + let run_record = if !run_id.is_empty() && run_id != "unknown" { + state.thread_store.load_run(run_id).await.ok().flatten() + } else { + None + }; + let run_tokens: (i64, i64) = run_record + .as_ref() + .map(|r| (r.input_tokens as i64, r.output_tokens as i64)) + .unwrap_or((0, 0)); + let run_started_at: Option> = run_record + .as_ref() + .and_then(|r| r.started_at) + .and_then(|secs| chrono::DateTime::::from_timestamp(secs as i64, 0)); + // R3 review (P2a): when the RunRecord is missing we still need + // a cutoff for "did the rule comment surface during this run?" + // dedupe. Without it the time-window check `created_at >= cutoff` + // would short-circuit to false and we'd re-insert agent_output + // on top of a reaction-posted comment. Use the handler entry + // time as a best-effort lower bound; any comment created + // *during* evaluate_rules below will land at or after this. + let handler_started_at = chrono::Utc::now(); + let dedupe_cutoff = run_started_at.unwrap_or(handler_started_at); + { let store = state.store.lock().await; if !run_id.is_empty() && run_id != "unknown" { @@ -10718,30 +10879,66 @@ pub(crate) async fn handle_scheduled_agent_terminal_event( tracing::warn!(issue_id, error = %error, "failed to update issue agent status"); } + // run_finished is the canonical billable / lifecycle event; + // R2 review #5 keeps token totals here only so SUM aggregations + // over `agent_activities` don't double-count when `agent_output` + // also exists below. let mut activity = oversight_models::AgentActivity::new(agent_id.to_string(), "run_finished"); activity.issue_id = Some(issue_id.clone()); activity.thread_id = Some(thread_id.to_string()); activity.source_id = Some(run_id.to_string()); activity.summary = Some(format!("termination={effective_termination:?}")); + activity.token_input = run_tokens.0; + activity.token_output = run_tokens.1; let _ = store.insert_activity(&activity); } + // Run workflow rules BEFORE deciding whether to surface the + // agent's `final_output` (R2 review #3). A workflow reaction may + // post a Comment action that already exposes the agent's text; + // emitting `agent_output` on top of that would double-render. + // + // Agents often deliver structured outputs (`KEY: value` lines like + // `review_decision: pass`) via `comment_on_issue` rather than the + // prose `final_output`, which tends to wrap the same value in + // backticks. Concat the latest in-run agent comment so workflow + // rules see the structured form when extracting output fields. + let latest_agent_comment = { + let store = state.store.lock().await; + let agent_author = format!("agent:{agent_id}"); + store + .list_issue_comments(&issue_id) + .ok() + .and_then(|comments| { + comments + .into_iter() + .filter(|c| c.author == agent_author && c.created_at >= dedupe_cutoff) + .max_by_key(|c| c.created_at) + .map(|c| c.content) + }) + }; + let rule_input = match latest_agent_comment.as_deref() { + Some(comment) if !comment.trim().is_empty() => { + format!("{response_text}\n\n{comment}") + } + _ => response_text.to_string(), + }; let ctx = build_agent_run_context( agent_id, thread_id, run_id, &effective_termination, - response_text, + &rule_input, ); - if !evaluate_rules( + let rules_matched = evaluate_rules( state, &issue_id, agent_run_trigger(&effective_termination), &ctx, ) - .await - { + .await; + if !rules_matched { tracing::debug!( issue_id, thread_id, @@ -10749,6 +10946,63 @@ pub(crate) async fn handle_scheduled_agent_terminal_event( "no workflow rule matched terminal agent event" ); } + + // R2 review #3: only insert `agent_output` when nothing else + // already surfaced this run's output. Skip when: + // - the run errored (run_finished.summary already carries the + // failure reason) + // - response_text is empty / whitespace-only + // - any agent-authored comment from THIS agent landed after the + // run started — either via the agent calling + // `comment_on_issue` itself, or via a `ReactionAction::Comment` + // posting under `agent:`. + // + // R2 review #5: token totals stay 0 on the agent_output row so + // aggregate spend queries (SUM of token_input / token_output + // grouped by issue) don't double-count. + if !is_error && !response_text.trim().is_empty() { + let agent_author = format!("agent:{agent_id}"); + let already_surfaced = { + let store = state.store.lock().await; + match store.list_issue_comments(&issue_id) { + Ok(comments) => comments.iter().any(|c| { + // R3 review (P2a): use the shared cutoff so the + // dedupe still catches reaction-posted comments + // even when `RunRecord.started_at` was missing. + c.author == agent_author && c.created_at >= dedupe_cutoff + }), + Err(error) => { + tracing::warn!( + issue_id, + agent_id, + error = %error, + "failed to read issue comments for agent_output dedupe" + ); + false + } + } + }; + + if !already_surfaced { + const MAX_OUTPUT_CHARS: usize = 8 * 1024; + let body = if response_text.chars().count() > MAX_OUTPUT_CHARS { + let truncated: String = response_text.chars().take(MAX_OUTPUT_CHARS).collect(); + format!("{truncated}\n\n…(truncated; full output in run {run_id})") + } else { + response_text.to_string() + }; + let mut output_activity = + oversight_models::AgentActivity::new(agent_id.to_string(), "agent_output"); + output_activity.issue_id = Some(issue_id.clone()); + output_activity.thread_id = Some(thread_id.to_string()); + output_activity.source_id = Some(run_id.to_string()); + // R2 review #5: tokens stay zero here — they live on + // run_finished as the canonical accounting event. + output_activity.summary = Some(body); + let store = state.store.lock().await; + let _ = store.insert_activity(&output_activity); + } + } } fn is_agent_terminal_error( @@ -11171,6 +11425,7 @@ mod tests { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: crate::state::ServerMode::Cloud, } } @@ -12504,6 +12759,528 @@ mod tests { .map(|item| item.summary.contains("newer structured result")) .unwrap_or(false)); } + /// ADR-011 e2e finding: when an agent (e.g. requirement_analyzer) + /// produces its design analysis but doesn't call `comment_on_issue` + /// / `create_document` / `attach_to_issue`, the output should still + /// be visible on the issue. The terminal-event handler surfaces it + /// as an `agent_output` activity so the UI / API render it without + /// reading the thread store. + #[tokio::test] + async fn terminal_event_surfaces_final_output_as_agent_output_activity() { + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("Surface", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new( + "Design output surfacing", + "requirement", + &workflow.id, + "analyzing", + ); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-surface".into(), issue.id.clone()); + + let analysis = "ANALYSIS_DECISION: approved\n\ + ANALYSIS_DOCUMENT:\n# Requirement Analysis\n## Goal\nAdd CHANGELOG.md."; + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-surface"), + "run-surface-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + analysis, + ) + .await; + + let store = state.store.lock().await; + let activities = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-surface"), + Some("agent_output"), + None, + 10, + 0, + ) + .expect("list activities"); + assert_eq!( + activities.len(), + 1, + "exactly one agent_output activity per run" + ); + let surfaced = &activities[0]; + assert_eq!(surfaced.agent_id, "requirement_analyzer"); + assert_eq!(surfaced.source_id.as_deref(), Some("run-surface-1")); + assert!( + surfaced + .summary + .as_deref() + .unwrap_or("") + .contains("ANALYSIS_DECISION: approved"), + "summary must carry the agent's final_output verbatim (within cap)" + ); + + // run_finished activity still emitted for status tracking — the + // new surfacing is additive, not a replacement. + let run_finished = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-surface"), + Some("run_finished"), + None, + 10, + 0, + ) + .expect("list activities"); + assert_eq!(run_finished.len(), 1); + } + + /// ADR-011 e2e finding: token totals from the awaken `RunRecord` + /// must propagate into the canonical `run_finished` activity row + /// so admins can attribute spend per-issue without scraping the + /// thread store. `agent_output` intentionally keeps token fields + /// at 0 to avoid double-counting when both activities exist for + /// one run (R2 review #5). + #[tokio::test] + async fn terminal_event_propagates_run_record_tokens_to_run_finished_only() { + use awaken_contract::contract::storage::{RunRecord, RunStore}; + + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("Tokens", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new( + "Token propagation", + "requirement", + &workflow.id, + "analyzing", + ); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-tokens".into(), issue.id.clone()); + + // Seed the awaken run record with non-zero token totals — + // mimicking a runtime/cloud agent that DID record usage. + let now = chrono::Utc::now().timestamp() as u64; + let run = RunRecord { + run_id: "run-tokens-1".into(), + thread_id: "thread-tokens".into(), + agent_id: "requirement_analyzer".into(), + parent_run_id: None, + request: None, + input: None, + output: None, + status: RunStatus::Done, + termination_reason: Some( + awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + ), + final_output: Some("ANALYSIS_DECISION: approved".into()), + error_payload: None, + dispatch_id: None, + session_id: None, + transport_request_id: None, + waiting: None, + outcome: None, + created_at: now, + started_at: Some(now), + finished_at: Some(now), + updated_at: now, + steps: 0, + input_tokens: 1234, + output_tokens: 567, + state: None, + }; + state.thread_store.create_run(&run).await.expect("seed run"); + + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-tokens"), + "run-tokens-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + "ANALYSIS_DECISION: approved", + ) + .await; + + let store = state.store.lock().await; + let activities = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-tokens"), + None, + None, + 10, + 0, + ) + .expect("list activities"); + assert!( + activities.iter().any(|a| a.activity_type == "run_finished" + && a.token_input == 1234 + && a.token_output == 567), + "run_finished activity must carry RunRecord token totals" + ); + // R2 review #5: token totals live on run_finished ONLY. + // agent_output is a surfacing event, not a billing event; + // keeping tokens off it ensures SUM aggregates don't + // double-count when both activities exist for one run. + let agent_output = activities + .iter() + .find(|a| a.activity_type == "agent_output") + .expect("agent_output activity exists"); + assert_eq!( + agent_output.token_input, 0, + "agent_output must NOT carry tokens" + ); + assert_eq!( + agent_output.token_output, 0, + "agent_output must NOT carry tokens" + ); + } + + /// R2 review #3: when a workflow rule (or the agent itself via + /// `comment_on_issue`) already posted a comment under + /// `agent:` after this run started, the `agent_output` + /// fallback must NOT add a second, duplicate surface of the same + /// run's final_output. + #[tokio::test] + async fn terminal_event_skips_agent_output_when_agent_already_commented() { + use awaken_contract::contract::storage::{RunRecord, RunStore}; + + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("Dedup", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new("Dedupe surfacing", "requirement", &workflow.id, "analyzing"); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-dedupe".into(), issue.id.clone()); + + // Seed a run with a started_at timestamp BEFORE the comment + // we're about to insert, so the dedupe check matches on the + // run-window cutoff. + let now = chrono::Utc::now().timestamp() as u64; + let run = RunRecord { + run_id: "run-dedupe-1".into(), + thread_id: "thread-dedupe".into(), + agent_id: "requirement_analyzer".into(), + parent_run_id: None, + request: None, + input: None, + output: None, + status: RunStatus::Done, + termination_reason: Some( + awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + ), + final_output: Some("the agent's narrative output".into()), + error_payload: None, + dispatch_id: None, + session_id: None, + transport_request_id: None, + waiting: None, + outcome: None, + created_at: now - 5, + started_at: Some(now - 5), + finished_at: Some(now), + updated_at: now, + steps: 0, + input_tokens: 0, + output_tokens: 0, + state: None, + }; + state.thread_store.create_run(&run).await.expect("seed run"); + + // Pre-existing agent-authored comment posted AFTER run start + // — simulates either `comment_on_issue` MCP tool call or + // a workflow reaction's `Comment` action. + let mut comment = IssueComment::new( + &issue.id, + "agent:requirement_analyzer", + "the agent's narrative output", + ); + // Force `comment.created_at` to land strictly after the run + // start so the dedupe heuristic catches it. + comment.created_at = chrono::Utc::now(); + { + let store = state.store.lock().await; + store.add_issue_comment(&comment).expect("add comment"); + } + + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-dedupe"), + "run-dedupe-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + "the agent's narrative output", + ) + .await; + + let store = state.store.lock().await; + let outputs = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-dedupe"), + Some("agent_output"), + None, + 10, + 0, + ) + .expect("list activities"); + assert!( + outputs.is_empty(), + "agent_output must NOT be inserted when an agent-authored \ + comment already surfaced the run; got: {outputs:?}" + ); + // run_finished still fires for status tracking — dedupe is + // additive, not a replacement. + let run_finished = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-dedupe"), + Some("run_finished"), + None, + 10, + 0, + ) + .expect("list activities"); + assert_eq!(run_finished.len(), 1); + } + + /// Honesty test: when no RunRecord exists (CLI host-native runs + /// today don't write to awaken's run store yet), token fields + /// default to 0 rather than crashing. This pins the contract + /// so a future refactor that *can* feed CLI tokens into awaken + /// doesn't quietly break the no-record path. + #[tokio::test] + async fn terminal_event_tokens_default_to_zero_when_run_record_missing() { + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("NoRun", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new("No run record", "requirement", &workflow.id, "analyzing"); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-norun".into(), issue.id.clone()); + + // Do NOT seed any RunRecord — `load_run` returns None. + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-norun"), + "run-norun-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + "best-effort output without a run record", + ) + .await; + + let store = state.store.lock().await; + let activities = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-norun"), + None, + None, + 10, + 0, + ) + .expect("list activities"); + let run_finished = activities + .iter() + .find(|a| a.activity_type == "run_finished") + .expect("run_finished activity"); + assert_eq!(run_finished.token_input, 0); + assert_eq!(run_finished.token_output, 0); + } + + /// R3 review (P2a): when `RunRecord.started_at` is missing the + /// dedupe used to short-circuit to false, which would re-insert + /// `agent_output` on top of an already-surfaced reaction-posted + /// comment. The handler now uses its own entry time as a + /// best-effort cutoff so any comment created during + /// `evaluate_rules` (i.e. after the handler started but before + /// the dedupe check) is still recognised. + #[tokio::test] + async fn terminal_event_skips_agent_output_when_comment_arrives_without_run_record() { + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("NoRecord", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new( + "Missing run record", + "requirement", + &workflow.id, + "analyzing", + ); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-norec".into(), issue.id.clone()); + + // Simulate the reaction-posted comment landing AFTER the + // handler's entry cutoff. We do NOT seed a RunRecord, so + // the only available cutoff is the handler's `Utc::now()`. + // Pin the comment far enough in the future that even + // multi-second handler latency keeps it on the >= side of + // the cutoff — that's the production invariant we want + // pinned regardless of test runner speed. + let mut comment = IssueComment::new( + &issue.id, + "agent:requirement_analyzer", + "the reaction-posted output", + ); + comment.created_at = chrono::Utc::now() + chrono::Duration::hours(1); + { + let store = state.store.lock().await; + store.add_issue_comment(&comment).expect("add comment"); + } + + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-norec"), + "run-norec-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + "the reaction-posted output", + ) + .await; + + let store = state.store.lock().await; + let outputs = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-norec"), + Some("agent_output"), + None, + 10, + 0, + ) + .expect("list activities"); + assert!( + outputs.is_empty(), + "agent_output must NOT be inserted when an agent-authored \ + comment already surfaced the run, even with no RunRecord; got: {outputs:?}" + ); + } + + /// Error terminations already surface their reason via + /// `run_finished.summary`; the `agent_output` activity must NOT + /// fire so the issue's activity log isn't double-spammed with the + /// same failure text. + #[tokio::test] + async fn terminal_event_skips_agent_output_activity_on_error() { + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("Error", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new("Error path", "requirement", &workflow.id, "analyzing"); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-error".into(), issue.id.clone()); + + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-error"), + "run-error-1", + &awaken_contract::contract::lifecycle::TerminationReason::Error("agent crashed".into()), + "partial output before crash", + ) + .await; + + let store = state.store.lock().await; + let outputs = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-error"), + Some("agent_output"), + None, + 10, + 0, + ) + .expect("list activities"); + assert!( + outputs.is_empty(), + "error terminations must not produce a duplicate agent_output activity" + ); + } + + /// Empty `final_output` (e.g. tool-only agent) should NOT produce a + /// blank `agent_output` activity — that would just be noise. + #[tokio::test] + async fn terminal_event_skips_agent_output_activity_when_response_text_empty() { + let store = oversight_store::Store::open_memory().expect("store should initialize"); + let workflow = Workflow::new("Empty", "issue", "analyzing"); + store.insert_workflow(&workflow).expect("insert workflow"); + let issue = Issue::new("Empty output", "requirement", &workflow.id, "analyzing"); + store.insert_issue(&issue).expect("insert issue"); + + let state = test_app_state(store); + state + .thread_issue_map + .write() + .expect("write map") + .insert("thread-empty".into(), issue.id.clone()); + + handle_scheduled_agent_terminal_event( + &state, + "requirement_analyzer", + Some("thread-empty"), + "run-empty-1", + &awaken_contract::contract::lifecycle::TerminationReason::NaturalEnd, + " \n\t ", + ) + .await; + + let store = state.store.lock().await; + let outputs = store + .list_activities_filtered( + None, + Some(&issue.id), + Some("thread-empty"), + Some("agent_output"), + None, + 10, + 0, + ) + .expect("list activities"); + assert!( + outputs.is_empty(), + "whitespace-only output must not surface" + ); + } + #[tokio::test] async fn terminal_event_uses_structured_workflow_reaction_output_block() { let store = oversight_store::Store::open_memory().expect("store should initialize"); diff --git a/crates/oversight-server/src/auth_api.rs b/crates/oversight-server/src/auth_api.rs index e22ea2eb..49e38708 100644 --- a/crates/oversight-server/src/auth_api.rs +++ b/crates/oversight-server/src/auth_api.rs @@ -166,9 +166,27 @@ async fn whoami(ctx: Option>) -> impl IntoResponse } async fn status(State(state): State) -> impl IntoResponse { + // ADR-011 review #2 + #3: the source of truth is the explicit + // `ServerMode`, not the authorizer flag. In local mode there is + // no auth-user table to consult — short-circuit before any DB + // touch so a flaky / locked / mid-migration `auth_users` table + // cannot break the no-login experience (a 500 here would push + // the frontend back into the Login route). + if state.server_mode.is_local() { + return Json(json!({ + "bootstrapped": true, + "mode": "local", + })) + .into_response(); + } + let store = state.store.lock().await; match store.count_auth_users() { - Ok(count) => Json(json!({ "bootstrapped": count > 0 })).into_response(), + Ok(count) => Json(json!({ + "bootstrapped": count > 0, + "mode": "cloud", + })) + .into_response(), Err(e) => { tracing::error!(error = %e, "count_auth_users failed"); ( @@ -761,9 +779,60 @@ mod tests { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: crate::state::ServerMode::Cloud, } } + /// ADR-011 review #2 + #3: local-mode /auth/status returns the + /// fixed shape `{bootstrapped: true, mode: "local"}` without + /// touching the auth users table. This guards two contracts: + /// - the frontend never re-renders the Login route in local mode + /// - a flaky `auth_users` table never breaks the no-login UX + /// + /// The fixture marks the state Local, then we make sure the + /// response shape is correct AND that swapping to Cloud surfaces + /// the normal `count_auth_users() == 0` bootstrap UI. + #[tokio::test] + async fn auth_status_local_mode_short_circuits_to_bootstrapped() { + let mut state = build_state(); + state.server_mode = crate::state::ServerMode::Local; + let app = routes(state); + let resp = app + .oneshot( + Request::builder() + .uri("/auth/status") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["mode"], "local"); + assert_eq!(json["bootstrapped"], true); + } + + #[tokio::test] + async fn auth_status_cloud_mode_reports_bootstrap_false_on_empty_store() { + let state = build_state(); + assert_eq!(state.server_mode, crate::state::ServerMode::Cloud); + let app = routes(state); + let resp = app + .oneshot( + Request::builder() + .uri("/auth/status") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["mode"], "cloud"); + assert_eq!(json["bootstrapped"], false); + } + #[tokio::test] async fn bootstrap_then_login_flow() { let state = build_state(); diff --git a/crates/oversight-server/src/auth_mw.rs b/crates/oversight-server/src/auth_mw.rs index d99f8804..dffafcbc 100644 --- a/crates/oversight-server/src/auth_mw.rs +++ b/crates/oversight-server/src/auth_mw.rs @@ -18,6 +18,29 @@ pub struct AuthContext { pub user: AuthUser, } +/// Stable id used for the synthetic local-mode user. Surfaces in +/// audit logs and authz traces so the source of an action is still +/// identifiable. Chosen to be obvious to operators ("this came from +/// `oversight local`") rather than collide with any real user id. +pub const LOCAL_MODE_USER_ID: &str = "local"; +pub const LOCAL_MODE_USERNAME: &str = "local"; + +/// Build the synthetic [`AuthContext`] injected into every request in +/// local mode. ADR-011: this user is conceptually the operator running +/// `oversight local` on their machine. +pub fn local_mode_auth_context() -> AuthContext { + AuthContext { + user: AuthUser { + id: LOCAL_MODE_USER_ID.into(), + username: LOCAL_MODE_USERNAME.into(), + password_hash: String::new(), + role: "admin".into(), + created_at: chrono::Utc::now(), + disabled_at: None, + }, + } +} + /// Marker inserted by the outer API-token middleware when a request has /// presented the server-level token. Internal worker endpoints can accept this /// marker for non-loopback remote workers. @@ -79,6 +102,31 @@ pub async fn require_auth( ) -> Response { let path = request.uri().path().to_string(); + // ADR-011: local-mode short-circuit. Driven by the explicit + // `ServerMode::Local` deployment flag (ADR-011 review #2), NOT by + // `authorizer.is_permissive()` — those are different concepts and + // conflating them would silently disable authentication in any + // future scenario that needs a permissive authorizer without + // being a local install (e.g. maintenance mode, test rigs). + // Internal endpoints stay loopback-only even in local mode — + // they're worker-protocol surfaces, not user-facing. + if state.server_mode.is_local() { + let is_loopback_local = request + .extensions() + .get::>() + .map(|ci| ci.0.ip().is_loopback()) + .unwrap_or(false); + let has_api_token_local = request + .extensions() + .get::() + .is_some(); + if path.starts_with("/internal/") && !is_loopback_local && !has_api_token_local { + return (StatusCode::FORBIDDEN, "internal endpoints require loopback").into_response(); + } + request.extensions_mut().insert(local_mode_auth_context()); + return next.run(request).await; + } + let is_loopback = request .extensions() .get::>() diff --git a/crates/oversight-server/src/cli_agent_api.rs b/crates/oversight-server/src/cli_agent_api.rs index 7af2b508..3ffeacee 100644 --- a/crates/oversight-server/src/cli_agent_api.rs +++ b/crates/oversight-server/src/cli_agent_api.rs @@ -887,6 +887,13 @@ async fn dispatch_issue( return response; } + // Refuse inactive agents before credential resolution / scheduling + // — `active = false` is the local-mode NeedsLogin gate. Shared with + // `/internal/dispatch` so both ingresses enforce it identically. + if let Err(response) = crate::dispatch_api::ensure_agent_dispatchable(&agent_def) { + return response; + } + let adapter_type = agent_def.agent_type.clone(); let config_version = agent_def.config_version as u64; let agent_id_fallback = agent_def.agent_id.clone(); @@ -907,33 +914,31 @@ async fn dispatch_issue( bundle }; - // ADR-005: route through the unified resolver so the agent's - // credential_id / credential_selector are honoured. When the - // pool is unavailable (test AppStates without sidecar SQLite) - // fall back to the vault-only path so dispatch keeps working. - // - // Fail-CLOSED at this boundary too: if the resolver explicitly - // refused (`Pinned` + None, or `Selector` + None), abort - // dispatch with 424 instead of silently continuing with empty - // secrets. - let resolved = match state.credential_pool.as_ref() { - Some(pool) => { - let resolution = - crate::credentials::resolve_credential_for_agent(&state.vault, pool, &agent_def) - .await; - if resolution.is_explicit_refusal() { - return ( - StatusCode::FAILED_DEPENDENCY, - Json(serde_json::json!({ - "error": resolution.refusal_message(), - "credential_binding_mode": format!("{:?}", resolution.mode), - })), - ) - .into_response(); - } - resolution.resolved + // ADR-005 / ADR-011: same shared dispatch credential resolver used + // by `/internal/dispatch`. Previously this route fell back to + // `resolve_for_adapter(..., None, ...)` when the pool was missing, + // which IGNORED `agent_def.credential_id` — an explicit + // `host-native:` / vault / env / ext / malformed pin would be + // silently substituted by an adapter auto-pick. The helper is + // fail-CLOSED on any pin it can't satisfy. + let resolved = match crate::credentials::resolve_dispatch_credential( + &state.vault, + state.credential_pool.as_ref(), + &agent_def, + ) + .await + { + Ok(resolved) => resolved, + Err(refusal) => { + return ( + StatusCode::FAILED_DEPENDENCY, + Json(serde_json::json!({ + "error": refusal.message, + "credential_binding_mode": refusal.binding_mode, + })), + ) + .into_response(); } - None => crate::credentials::resolve_for_adapter(&state.vault, None, &adapter_type).await, }; let (secrets, credential_profile_id, credential_namespace) = match resolved { Some(r) => (r.secrets, Some(r.profile_id), Some(r.namespace)), diff --git a/crates/oversight-server/src/dispatch_api.rs b/crates/oversight-server/src/dispatch_api.rs index ca2a93ce..540ac479 100644 --- a/crates/oversight-server/src/dispatch_api.rs +++ b/crates/oversight-server/src/dispatch_api.rs @@ -14,6 +14,35 @@ use crate::cli_agent_api::{ use crate::state::AppState; use crate::worker_scheduler::DispatchPayload; +/// Refuse dispatch for an inactive agent. +/// +/// `active = false` is the gate local-mode discovery uses to express +/// "this CLI needs login, don't run it" (see +/// `oversight_agents::discovery`). Both explicit dispatch ingresses +/// (`/internal/dispatch` and `/api/cli-agents/:id/dispatch`) fetch +/// agents by id rather than via `list_active_agent_defs`, so without +/// this guard a `NeedsLogin` agent could still be dispatched and end +/// up invoking an unauthenticated CLI. The schema has no +/// `disabled_reason` yet, so we refuse all inactive agents uniformly; +/// a future field can split operator-disabled vs login-disabled for +/// nicer UX. +pub(crate) fn ensure_agent_dispatchable( + agent_def: &oversight_models::AgentDef, +) -> Result<(), Response> { + if !agent_def.active { + return Err(( + StatusCode::FAILED_DEPENDENCY, + Json(serde_json::json!({ + "error": format!("agent '{}' is inactive; dispatch refused", agent_def.agent_id), + "agent_id": agent_def.agent_id, + "agent_type": agent_def.agent_type, + })), + ) + .into_response()); + } + Ok(()) +} + pub fn routes(state: AppState) -> Router { Router::new() .route("/internal/dispatch", post(dispatch)) @@ -83,6 +112,38 @@ async fn build_dispatch_payload( } }; + // ADR-011 review #1: the request body carries `agent_id` AND + // `adapter_type` as independent fields. The resolver below honours + // the agent_def fetched by `agent_id`, but every downstream wiring + // (`agent_type` on the dispatch payload, scheduler matchmaker, + // worker session, etc.) uses `req.adapter_type`. A mismatched pair + // would route a Claude credential resolution through a Codex + // worker, completely bypassing the per-source compatibility + // checks added in this PR. Reject the request at the boundary so + // the resolver and the executor agree on which adapter is in play. + if req.adapter_type != agent_def.agent_type { + return Err(( + StatusCode::BAD_REQUEST, + Json(serde_json::json!({ + "error": format!( + "dispatch request adapter_type='{}' does not match agent '{}' (agent_type='{}'); \ + refused (fail-closed)", + req.adapter_type, + req.agent_id, + agent_def.agent_type, + ), + "agent_id": req.agent_id, + "agent_type": agent_def.agent_type, + "requested_adapter_type": req.adapter_type, + })), + ) + .into_response()); + } + + // Refuse inactive agents before any credential resolution or + // scheduling — `active = false` is the local-mode NeedsLogin gate. + ensure_agent_dispatchable(&agent_def)?; + let instance_id = req .instance_id .clone() @@ -120,35 +181,31 @@ async fn build_dispatch_payload( } attach_issue_worktree_env(binding.as_ref(), &mut config_bundle)?; - // ADR-005: route through the unified resolver so the agent's - // credential_id / credential_selector are honoured. When the - // pool is unavailable (test AppStates without sidecar SQLite) - // fall back to the vault-only path so dispatch keeps working. - // - // Fail-CLOSED at this boundary too: if the resolver explicitly - // refused (`Pinned` + None, or `Selector` + None), abort - // dispatch with 424 instead of silently continuing with empty - // secrets. The resolver's refusal note propagates into the - // response so operators see why. - let resolved = match state.credential_pool.as_ref() { - Some(pool) => { - let resolution = - crate::credentials::resolve_credential_for_agent(&state.vault, pool, &agent_def) - .await; - if resolution.is_explicit_refusal() { - return Err(( - StatusCode::FAILED_DEPENDENCY, - Json(serde_json::json!({ - "error": resolution.refusal_message(), - "credential_binding_mode": format!("{:?}", resolution.mode), - })), - ) - .into_response()); - } - resolution.resolved - } - None => { - crate::credentials::resolve_for_adapter(&state.vault, None, &req.adapter_type).await + // ADR-005 / ADR-011: route through the single shared dispatch + // credential resolver so the agent's `credential_id` / + // `credential_selector` are honoured identically here and on the + // direct `/api/cli-agents/:id/dispatch` route. The helper is + // fail-CLOSED: an explicit pin it can't satisfy (vault pin without + // a pool, adapter mismatch, reserved/malformed prefix) returns a + // refusal that we surface as 424 instead of silently substituting + // another credential. + let resolved = match crate::credentials::resolve_dispatch_credential( + &state.vault, + state.credential_pool.as_ref(), + &agent_def, + ) + .await + { + Ok(resolved) => resolved, + Err(refusal) => { + return Err(( + StatusCode::FAILED_DEPENDENCY, + Json(serde_json::json!({ + "error": refusal.message, + "credential_binding_mode": refusal.binding_mode, + })), + ) + .into_response()); } }; let (secrets, credential_profile_id, credential_namespace) = match resolved { @@ -671,6 +728,7 @@ mod tests { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: crate::state::ServerMode::Cloud, } } @@ -716,6 +774,288 @@ mod tests { drop(tx); } + /// ADR-011 review #1: dispatch must refuse when the request's + /// `adapter_type` disagrees with the agent_def's `agent_type`, + /// even when the `agent_id` resolves a real row. Without this + /// gate, the resolver picks the credential for agent X but the + /// payload carries agent_type Y — letting a Claude credential + /// flow to a Codex worker. + #[tokio::test] + async fn dispatch_payload_rejects_adapter_type_mismatch_against_agent_def() { + let state = test_app_state(); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let error = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + // Caller lies about the adapter — must fail closed. + adapter_type: "codex".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect_err("mismatched adapter must be rejected"); + assert_eq!( + error.status(), + StatusCode::BAD_REQUEST, + "adapter mismatch is a client error, not a thread-binding error" + ); + } + + /// R2 review #6: when `credential_pool` is None (typical for + /// test AppStates without a sidecar SQLite), dispatch must + /// still honour an explicit `host-native:` pin and + /// produce a `ResolvedCredential` with `source = HostNative`. + /// Pre-fix this fell through to `resolve_for_adapter` which + /// ignored `credential_id` entirely — making + /// `/resolved-credential` show host-native but dispatch actually + /// route through Auto. + #[tokio::test] + async fn dispatch_payload_honors_host_native_pin_without_pool() { + let state = test_app_state(); + assert!( + state.credential_pool.is_none(), + "test fixture should not wire pool" + ); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + claude_agent.credential_id = Some("host-native:claude".into()); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let payload = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect("host-native pin must resolve without pool"); + assert_eq!( + payload.credential_namespace.as_deref(), + Some("cli-credentials") + ); + assert_eq!( + payload.credential_profile_id.as_deref(), + Some("host-native:claude") + ); + assert!( + payload.secrets.is_empty(), + "host-native must materialise zero secrets" + ); + } + + /// host-native pin with wrong adapter must still fail-closed + /// in the no-pool path (same contract as the pool path). + #[tokio::test] + async fn dispatch_payload_no_pool_host_native_mismatch_fails_closed() { + let state = test_app_state(); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + claude_agent.credential_id = Some("host-native:codex".into()); // mismatched + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let err = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect_err("adapter mismatch on host-native pin must refuse"); + assert_eq!(err.status(), StatusCode::FAILED_DEPENDENCY); + } + + /// Vault pin without a configured credential pool must fail-closed. + /// The previous behaviour fell through to `resolve_for_adapter`, + /// which IGNORED the explicit pin and substituted whatever + /// auto-pool credential matched the adapter — silently routing + /// `cli-credentials:staging` to e.g. `cli-credentials:prod` in + /// any AppState where the sidecar SQLite isn't wired (test + /// fixtures, embedded-only deployments). + #[tokio::test] + async fn dispatch_payload_no_pool_vault_pin_fails_closed() { + let state = test_app_state(); + assert!(state.credential_pool.is_none()); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + claude_agent.credential_id = Some("cli-credentials:missing-profile".into()); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let err = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect_err("vault pin without pool must fail closed"); + assert_eq!(err.status(), StatusCode::FAILED_DEPENDENCY); + } + + /// Malformed `credential_id` (not `:` and not a known prefix) + /// must fail-closed in the no-pool path — refusing dispatch + /// instead of silently dropping the pin and using auto-pool. + #[tokio::test] + async fn dispatch_payload_no_pool_malformed_credential_id_fails_closed() { + let state = test_app_state(); + assert!(state.credential_pool.is_none()); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + claude_agent.credential_id = Some("not-a-pair".into()); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let err = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect_err("malformed credential_id without pool must fail closed"); + assert_eq!(err.status(), StatusCode::FAILED_DEPENDENCY); + } + + /// Inactive agents must be refused before credential resolution / + /// scheduling. `active = false` is the local-mode NeedsLogin gate; + /// dispatching an inactive host-native agent would invoke an + /// unauthenticated CLI. + #[tokio::test] + async fn dispatch_payload_rejects_inactive_agent() { + let state = test_app_state(); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + claude_agent.active = false; + claude_agent.credential_id = Some("host-native:claude".into()); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let err = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect_err("inactive agent must not dispatch"); + assert_eq!(err.status(), StatusCode::FAILED_DEPENDENCY); + } + + /// The shared guard is what both ingresses call; pin its contract + /// directly so the direct CLI route (which can't easily build a + /// full router in a unit test) is covered by proxy. + #[test] + fn ensure_agent_dispatchable_refuses_inactive_allows_active() { + let mut def = AgentDef::new("claude", "Claude", "do work"); + def.agent_type = "claude".into(); + def.active = false; + let err = ensure_agent_dispatchable(&def).expect_err("inactive must refuse"); + assert_eq!(err.status(), StatusCode::FAILED_DEPENDENCY); + + def.active = true; + assert!( + ensure_agent_dispatchable(&def).is_ok(), + "active agent must pass the guard" + ); + } + + /// Compatible adapter_type still resolves normally — pins the + /// happy path so the new validator doesn't regress the existing + /// dispatch flow. + #[tokio::test] + async fn dispatch_payload_accepts_matching_adapter_type() { + let state = test_app_state(); + let mut claude_agent = AgentDef::new("claude", "Claude", "do work"); + claude_agent.agent_type = "claude".into(); + { + let store = state.store.try_lock().expect("store"); + store + .insert_agent_def(&claude_agent) + .expect("insert claude agent"); + } + let payload = build_dispatch_payload( + &state, + &DispatchRequest { + instance_id: None, + thread_id: None, + agent_id: claude_agent.agent_id.clone(), + adapter_type: "claude".into(), + scheduling_policy: None, + pinned_node_id: None, + prompt: "hi".into(), + parent_run_id: None, + }, + ) + .await + .expect("matching adapter must resolve"); + assert_eq!(payload.agent_type, "claude"); + } + #[tokio::test] async fn dispatch_payload_rejects_thread_agent_mismatch() { let state = test_app_state(); @@ -724,8 +1064,13 @@ mod tests { workflow.project_id = Some(project.id.clone()); let mut issue = Issue::new("Bound issue", "issue", &workflow.id, "todo"); issue.project_id = Some(project.id.clone()); - let assigned = AgentDef::new("assigned", "Assigned", "do work"); - let other = AgentDef::new("other", "Other", "do work"); + // agent_type must equal the request's adapter_type so the + // new ADR-011-#1 boundary check does not pre-empt the + // thread-binding mismatch case this test is asserting on. + let mut assigned = AgentDef::new("assigned", "Assigned", "do work"); + assigned.agent_type = "terminal".into(); + let mut other = AgentDef::new("other", "Other", "do work"); + other.agent_type = "terminal".into(); let mut thread = Thread::with_id("thread-bound"); thread .metadata @@ -779,8 +1124,11 @@ mod tests { workflow.project_id = Some(project.id.clone()); let mut issue = Issue::new("Bound issue", "issue", &workflow.id, "todo"); issue.project_id = Some(project.id.clone()); - let assigned = AgentDef::new("assigned", "Assigned", "do work"); - let other = AgentDef::new("other", "Other", "do work"); + // See ADR-011 #1 boundary check rationale on the sibling test. + let mut assigned = AgentDef::new("assigned", "Assigned", "do work"); + assigned.agent_type = "terminal".into(); + let mut other = AgentDef::new("other", "Other", "do work"); + other.agent_type = "terminal".into(); let mut thread = Thread::with_id("thread-control"); thread .metadata diff --git a/crates/oversight-server/src/main.rs b/crates/oversight-server/src/main.rs index fdecb7ca..7f00e5e9 100644 --- a/crates/oversight-server/src/main.rs +++ b/crates/oversight-server/src/main.rs @@ -35,9 +35,17 @@ struct SecurityConfig { #[derive(Debug, Clone, PartialEq, Eq)] enum StartMode { Http, + /// ADR-011: single-user, no-login, host-native agent discovery. + /// Same HTTP server as `Http`, but the authorizer is built + /// permissive and discovery seeds host-native agent rows on + /// startup. + Local, AcpStdio, GenKey, - RotateKey { old: String, new: String }, + RotateKey { + old: String, + new: String, + }, } #[tokio::main] @@ -202,16 +210,199 @@ async fn main() -> anyhow::Result<()> { } } - let authorizer = Arc::new( - oversight_server::authz_sync::load_authorizer(&shared_store, storage.as_ref()) - .await - .map_err(|error| anyhow::anyhow!("failed to load authorizer: {error}"))?, - ); + // ADR-011: `oversight local` swaps the grant-loaded authorizer for + // a permissive one before the runtime is built so every downstream + // ensure_authorized() call is a no-op. The grant tables remain + // intact in the database — local mode is an assembly choice, not + // a destructive operation. A subsequent `oversight serve` against + // the same data dir would see the same data and re-enforce the + // grant-based authorizer. + let authorizer = if mode == StartMode::Local { + tracing::info!("local mode: building permissive authorizer (ADR-011)"); + Arc::new(oversight_authz::Authorizer::permissive()) + } else { + Arc::new( + oversight_server::authz_sync::load_authorizer(&shared_store, storage.as_ref()) + .await + .map_err(|error| anyhow::anyhow!("failed to load authorizer: {error}"))?, + ) + }; + + // ADR-011: in local mode, scan PATH for known CLI agents and + // idempotently insert an `agent_definitions` row for each one + // before we read the agent list. Re-runs of `oversight local` + // are no-ops when the existing row is already host-native. + if mode == StartMode::Local { + let home = oversight_agents::discovery::resolve_home_dir(); + let discovered = oversight_agents::discovery::discover_local_agents(home.as_deref()); + tracing::info!( + count = discovered.len(), + "local mode: discovered host-native CLI agents" + ); + // R3 review (P1): keep the login_status alongside the + // adapter kind so the overlay below can flip seeded role + // agents to inactive when their adapter is NeedsLogin — + // otherwise the directly-discovered `claude` row stays + // inactive while `requirement_analyzer` / coder / reviewer + // get the pin overlaid and remain active, letting dispatch + // call an unauthenticated CLI. + let discovered_by_adapter: std::collections::HashMap< + String, + oversight_agents::discovery::LoginStatus, + > = discovered + .iter() + .map(|d| (d.adapter_kind.clone(), d.login_status.clone())) + .collect(); + for d in &discovered { + let agent_id = d.adapter_kind.as_str(); + let expected_pin = format!("host-native:{}", d.adapter_kind); + match storage.get_agent_def_by_agent_id(agent_id) { + Ok(existing) => { + // R2 review #2: only treat as "already seeded" + // when the existing row's credential_id matches + // the host-native pin. Anything else is a real + // conflict — a cloud-config row with the same + // agent_id is already in the DB (e.g. the data + // dir was previously used with `oversight serve`). + // + // Pre-fix behaviour was a warning that silently + // continued, leaving the user with a running + // server where this `agent_id` was bound to + // their old vault/env credentials — exactly the + // wrong outcome for `oversight local`. Switch to + // fail-fast so the operator must consciously + // delete or rename the conflicting row. + let existing_pin = existing.credential_id.as_deref().unwrap_or(""); + if existing_pin == expected_pin { + // R3 review (P1): the row is already host-native, + // but discovery re-runs on every `oversight local` + // boot — refresh the login-derived `active` state + // so a `claude logout`/`login` between runs is + // reflected instead of leaving a stale value. + let mut existing = existing; + if oversight_agents::discovery::refresh_host_native_login_state( + &mut existing, + d, + ) { + existing.updated_at = chrono::Utc::now(); + match storage.update_agent_def(&existing) { + Ok(()) => tracing::info!( + agent_id, + active = existing.active, + "refreshed host-native agent login state" + ), + Err(err) => anyhow::bail!( + "local mode: failed to refresh host-native agent \ + '{agent_id}': {err}" + ), + } + } else { + tracing::info!( + agent_id, + "host-native agent already seeded; login state unchanged" + ); + } + } else { + anyhow::bail!( + "local mode: agent_id '{agent_id}' already exists with \ + credential_id='{existing_pin}' but local discovery expects \ + '{expected_pin}'. Delete or rename the row in the UI / via API \ + before re-running `oversight local` to get host-native behaviour." + ); + } + } + Err(oversight_store::StoreError::NotFound(_)) => { + let def = oversight_agents::discovery::agent_def_for_discovered(d); + match storage.insert_agent_def(&def) { + Ok(()) => tracing::info!( + agent_id, + adapter = %d.adapter_kind, + binary = %d.binary_path.display(), + "seeded host-native agent for local mode" + ), + Err(err) => { + // Insert failures are NOT "row exists" — that + // case is the Ok branch above. A genuine + // failure here means the data dir / DB is + // unusable; fail fast in local mode rather + // than start a server with a half-populated + // agent table. + anyhow::bail!( + "local mode: failed to seed host-native agent '{agent_id}': {err}" + ); + } + } + } + Err(err) => { + // Non-NotFound DB error (locked, schema mismatch, + // corrupted row). Hide nothing — fail fast. + anyhow::bail!( + "local mode: failed to query agent '{agent_id}' from store: {err}" + ); + } + } + } + + // R2 review #1: the seeded role-based agents + // (coder / reviewer / deployer / requirement_analyzer / etc.) + // are what the seeded workflows actually dispatch to — but + // `seed_defaults` leaves their `credential_id = None`, so + // they fall through to the vault auto pool at dispatch time. + // In local mode without any vault credentials configured + // that means "no credentials at all," not "use the host + // CLI". Overlay `host-native:` onto those rows + // whose `agent_type` matches a discovered CLI AND that + // don't already carry an explicit pin (operator-set values + // are always preserved). + if !discovered_by_adapter.is_empty() { + let existing_defs = storage.list_agent_defs().map_err(|e| { + anyhow::anyhow!( + "local mode: failed to list agent_defs for host-native overlay: {e}" + ) + })?; + for mut def in existing_defs { + if !oversight_agents::discovery::apply_host_native_overlay( + &mut def, + &discovered_by_adapter, + ) { + continue; + } + def.updated_at = chrono::Utc::now(); + let pin = def + .credential_id + .clone() + .unwrap_or_else(|| format!("host-native:{}", def.agent_type)); + let active = def.active; + if let Err(e) = storage.update_agent_def(&def) { + tracing::warn!( + agent_id = %def.agent_id, + error = %e, + "failed to overlay host-native pin onto seeded role agent" + ); + } else { + tracing::info!( + agent_id = %def.agent_id, + agent_type = %def.agent_type, + pin = %pin, + active = active, + "local mode: overlaid host-native pin onto seeded role agent" + ); + } + } + } + } // Pre-load dynamic agent definitions from the selected storage backend // (so PG mode loads PG-seeded agents; SQLite mode loads SQLite-seeded - // agents). - let initial_agent_defs = storage.list_active_agent_defs().unwrap_or_default(); + // agents). ADR-011 review #6: local mode treats this as fatal so an + // empty agent list never silently masks a DB read failure. + let initial_agent_defs = if mode == StartMode::Local { + storage + .list_active_agent_defs() + .map_err(|e| anyhow::anyhow!("local mode: list_active_agent_defs failed: {e}"))? + } else { + storage.list_active_agent_defs().unwrap_or_default() + }; // Create workspace registry (shared between shell tools and API layer) let workspace_registry = oversight_tools::shell::new_workspace_registry(); @@ -420,6 +611,15 @@ async fn main() -> anyhow::Result<()> { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + // ADR-011 review #2: explicit deployment mode field. Decoupled + // from `authorizer.is_permissive()` so a future permissive + // authorizer scenario (e.g. maintenance mode) does NOT + // accidentally start skipping authentication. + server_mode: if mode == StartMode::Local { + crate::state::ServerMode::Local + } else { + crate::state::ServerMode::Cloud + }, }; // Start scheduler background loop. Phase 3e (ADR-003) routes background @@ -738,6 +938,7 @@ where match remaining[0].as_str() { "acp-stdio" | "--acp-stdio" if remaining.len() == 1 => Ok(StartMode::AcpStdio), + "local" if remaining.len() == 1 => Ok(StartMode::Local), "genkey" if remaining.len() == 1 => Ok(StartMode::GenKey), "rotate-key" => { let mut old: Option = None; @@ -763,7 +964,7 @@ where Ok(StartMode::RotateKey { old, new }) } _ => anyhow::bail!( - "unknown arguments: {} (supported: `acp-stdio`, `genkey`, `rotate-key --old --new `)", + "unknown arguments: {} (supported: `local`, `acp-stdio`, `genkey`, `rotate-key --old --new `)", remaining.join(" ") ), } @@ -891,6 +1092,22 @@ mod tests { assert!(err.to_string().contains("unknown arguments")); } + #[test] + fn local_mode_is_detected() { + let mode = parse_start_mode(["oversight-server", "local"]).unwrap(); + assert_eq!(mode, StartMode::Local); + } + + #[test] + fn unknown_args_message_includes_local_in_supported_list() { + // Guard against drift: `oversight local` is a documented user- + // facing entry point; the help text emitted on an unknown + // subcommand must mention it so operators can discover it. + let err = parse_start_mode(["oversight-server", "wat"]).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("local"), "missing 'local' in: {msg}"); + } + #[derive(Clone)] struct MockWorkerState { payload_tx: mpsc::Sender, @@ -1062,6 +1279,7 @@ mod tests { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: crate::state::ServerMode::Cloud, }; let app = dispatch_api::routes(app_state); diff --git a/crates/oversight-server/src/state.rs b/crates/oversight-server/src/state.rs index bef98ac8..af0deea9 100644 --- a/crates/oversight-server/src/state.rs +++ b/crates/oversight-server/src/state.rs @@ -23,6 +23,37 @@ pub type ThreadIssueMap = Arc>>; /// Keyed by agent_id so read state persists across MCP requests in the same session. pub type McpTrackerMap = Arc>>; +/// Deployment mode (ADR-011). Drives login-skip in the frontend, +/// synthetic-user injection in the auth middleware, and host-native +/// agent seeding in startup. Decoupled from `Authorizer::permissive` +/// on purpose: an authz strategy and a deployment mode are different +/// concepts — e.g. a future maintenance mode could use a permissive +/// authorizer without changing the deployment shape, and that +/// scenario must NOT silently start skipping authentication. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ServerMode { + /// Standard multi-user deployment. Login UI, multi-workspace, and + /// grant-based authorization are all active. + Cloud, + /// Single-user `oversight local` install. The middleware injects a + /// synthetic `local` admin on every request and the frontend + /// skips the Login route. + Local, +} + +impl ServerMode { + pub fn wire_str(&self) -> &'static str { + match self { + Self::Cloud => "cloud", + Self::Local => "local", + } + } + + pub fn is_local(&self) -> bool { + matches!(self, Self::Local) + } +} + #[derive(Clone)] pub struct AppState { pub store: SharedStore, @@ -94,6 +125,13 @@ pub struct AppState { /// for the same id block on it, callers for different ids proceed /// in parallel. pub discover_singleflight: Arc>>>>, + /// Deployment mode (ADR-011). The auth middleware and the + /// /api/auth/status handler consult this field instead of + /// inferring from `authorizer.is_permissive()`. Defaults to + /// [`ServerMode::Cloud`] for every existing AppState constructor; + /// `oversight local` explicitly sets it to `Local` after building + /// the permissive authorizer. + pub server_mode: ServerMode, } impl AppState { diff --git a/crates/oversight-server/src/worker_api.rs b/crates/oversight-server/src/worker_api.rs index 5fcb1ce5..530d2dcf 100644 --- a/crates/oversight-server/src/worker_api.rs +++ b/crates/oversight-server/src/worker_api.rs @@ -642,6 +642,7 @@ mod tests { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: crate::state::ServerMode::Cloud, } } diff --git a/crates/oversight-server/tests/common/mod.rs b/crates/oversight-server/tests/common/mod.rs index df6c932a..92aae39b 100644 --- a/crates/oversight-server/tests/common/mod.rs +++ b/crates/oversight-server/tests/common/mod.rs @@ -166,6 +166,7 @@ async fn build_state_with_storage( discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: oversight_server::state::ServerMode::Cloud, } } diff --git a/crates/oversight-server/tests/config_hot_reload.rs b/crates/oversight-server/tests/config_hot_reload.rs index fa3b4188..91ba6dff 100644 --- a/crates/oversight-server/tests/config_hot_reload.rs +++ b/crates/oversight-server/tests/config_hot_reload.rs @@ -105,6 +105,7 @@ async fn build_state_with_default_executor() -> (AppState, Arc AppState { discover_singleflight: std::sync::Arc::new(tokio::sync::Mutex::new( std::collections::HashMap::new(), )), + server_mode: oversight_server::state::ServerMode::Cloud, } } From 350514f08865a69c7147c1378223d143395a4ae1 Mon Sep 17 00:00:00 2001 From: Chai Zhenhua Date: Mon, 25 May 2026 07:34:57 +0800 Subject: [PATCH 6/6] =?UTF-8?q?=E2=9C=A8=20feat(web):=20mode-aware=20UI=20?= =?UTF-8?q?=E2=80=94=20skip=20login=20in=20local=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- web/src/components/Login.tsx | 12 +++++++++++- web/src/lib/api/auth.test.ts | 16 +++++++++++++++- web/src/lib/api/auth.ts | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/web/src/components/Login.tsx b/web/src/components/Login.tsx index 79b4508d..c1ab9797 100644 --- a/web/src/components/Login.tsx +++ b/web/src/components/Login.tsx @@ -28,9 +28,19 @@ export function Login({ onAuthenticated }: LoginProps) { useEffect(() => { if (bootstrapQuery.data) { + // ADR-011: in local mode the server injects a synthetic user + // on every request, so there is nothing to log in to. Skip + // straight to the dashboard. `bootstrapped` is forced true by + // the server in this mode for the same reason — defensive + // check both fields so an old cached client behaves correctly + // after the server is upgraded. + if (bootstrapQuery.data.status.mode === "local") { + onAuthenticated(); + return; + } setMode(bootstrapQuery.data.status.bootstrapped ? "login" : "bootstrap"); } - }, [bootstrapQuery.data]); + }, [bootstrapQuery.data, onAuthenticated]); useEffect(() => { if (bootstrapQuery.error) { diff --git a/web/src/lib/api/auth.test.ts b/web/src/lib/api/auth.test.ts index 19132aba..44b88e68 100644 --- a/web/src/lib/api/auth.test.ts +++ b/web/src/lib/api/auth.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { handlers } from "./auth"; +import { handlers, isLocalMode, type AuthStatus } from "./auth"; const fetchMock = vi.fn(); vi.stubGlobal("fetch", fetchMock); @@ -63,6 +63,20 @@ describe("auth handlers", () => { expect(init?.body).toContain("admin"); }); + // ADR-011: isLocalMode is the helper App.tsx / Login.tsx use to + // decide whether to skip the login route. Pin its truth table here + // so a refactor of the AuthStatus shape can't silently change which + // installs are considered "local". + it("isLocalMode treats only mode='local' as local", () => { + expect(isLocalMode({ bootstrapped: true, mode: "local" } satisfies AuthStatus)).toBe(true); + expect(isLocalMode({ bootstrapped: true, mode: "cloud" } satisfies AuthStatus)).toBe(false); + // Older servers omit `mode` entirely; treat as cloud (the safer + // default — never silently bypass login). + expect(isLocalMode({ bootstrapped: true } satisfies AuthStatus)).toBe(false); + expect(isLocalMode(null)).toBe(false); + expect(isLocalMode(undefined)).toBe(false); + }); + it("logout() POSTs /api/auth/logout", async () => { await handlers.logout(); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment diff --git a/web/src/lib/api/auth.ts b/web/src/lib/api/auth.ts index 04ca598f..2bebc8ee 100644 --- a/web/src/lib/api/auth.ts +++ b/web/src/lib/api/auth.ts @@ -4,8 +4,27 @@ import type { User } from "../../types/_generated"; export type { User } from "../../types/_generated"; +/** + * Deployment mode reported by `/api/auth/status` (ADR-011). + * + * - `cloud`: the standard multi-user deployment. Login UI and + * workspace switching are required. + * - `local`: single-user `oversight local` install. The server + * injects a synthetic `local` user on every request; the UI + * skips the Login route and hides multi-tenant surfaces. + * + * Defaults to `cloud` when the server is older than ADR-011 and + * does not emit a `mode` field. + */ +export type ServerMode = "cloud" | "local"; + export interface AuthStatus { bootstrapped: boolean; + mode?: ServerMode; +} + +export function isLocalMode(status: AuthStatus | null | undefined): boolean { + return status?.mode === "local"; } export interface AuthProvider {