diff --git a/README.md b/README.md index 61b766ea..34f050e5 100644 --- a/README.md +++ b/README.md @@ -63,22 +63,28 @@ make build ## Platform Support -`cr` stores secrets in the OS credential store through the shared -`cli-common/credstore` library. +`cr` stores secrets in explicit credential stores through the shared +`cli-common/credstore` library. A built-in `local-os` store is always available +for the current user. -| Platform | Default credential backend | +| Platform | Built-in `local-os` backend | |----------|----------------------------| | macOS | Keychain | | Windows | Credential Manager | | Linux | Secret Service | -Supported backend names are `keychain`, `wincred`, `secret-service`, `file`, -`pass`, and `memory`. Backend selection precedence is: +Configured stores can also target `1Password` desktop, service account, or +Connect backends, `pass`, encrypted file storage, or in-memory storage. Configure +additional stores from interactive `cr init` under **Configure secrets +storage**. The built-in `local-os` store is read-only configuration and cannot +be removed. -1. `--backend ` -2. `CODEREVIEW_KEYRING_BACKEND=` -3. `keyring.backend` in `config.yml` -4. OS default +Interactive `cr init` discovers available secrets backends by default so it can +offer local 1Password account/vault choices and passive backend availability +checks. Use `cr init --secret-backend-discovery=safe` to skip active inventory +probes, or `--secret-backend-discovery=off` to skip backend discovery entirely. +The `CR_SECRET_BACKEND_DISCOVERY` environment variable accepts the same +`full`, `safe`, and `off` values when the flag is not supplied. Secrets are never written to `config.yml`. Non-secret config lives in the `codereview` config directory resolved by the operating system, and durable @@ -151,14 +157,12 @@ sequence looks like: ```bash cr --profile work init --non-interactive \ - --set-default \ --git-host github.com \ --git-auth-mode pat \ --llm-provider anthropic \ --llm-auth subscription \ --llm-adapter claude_cli \ - --llm-reviewer-model-tier medium \ - --keyring-backend file + --llm-reviewer-model-tier medium cr --profile work config route set \ --host github.com \ @@ -169,7 +173,8 @@ cr --profile work config agent-source add ~/.config/codereview/agents cr --profile work config llm models set medium claude-sonnet-4-6 cr config retention set --max-age-days 30 --enforcement manual_only printf '%s' "$GITHUB_TOKEN" | cr set-credential \ - --ref codereview/work \ + --store local-os \ + --name codereview/work \ --key git_token \ --stdin \ --overwrite @@ -177,19 +182,12 @@ printf '%s' "$GITHUB_TOKEN" | cr set-credential \ Use `cr config route` for repository routing, `cr config agent-source` for trusted source paths, `cr config llm models` for `llm.model_map`, and -`cr config retention` for durable run-data policy. Use `cr init ---non-interactive --keyring-backend ` to persist a keyring backend, -`--reset-keyring-backend` to clear it, `--disable-reviewer` to remove separate -reviewer credentials, `--llm-reviewer-model-tier` or -`--clear-llm-reviewer-model-tier` for the durable reviewer baseline, and -`--set-default` to make the target profile the default during init. For -backward compatibility, init may still persist a runtime `--backend` when the -command writes credentials or configures API-key LLM auth, but the explicit -`--keyring-backend` / `--reset-keyring-backend` flags are the readable -scripted surface to prefer. +`cr config retention` for durable run-data policy. Use interactive `cr init` to +configure additional credential stores. Use `--disable-reviewer` to remove +separate reviewer credentials, `--llm-reviewer-model-tier` or +`--clear-llm-reviewer-model-tier` for the durable reviewer baseline. ```yaml -default_profile: personal repository_profiles: - profile: personal-reviewer-a match: @@ -218,11 +216,11 @@ profiles: Route matching is deterministic: `host + namespace + repo` routes beat `host + namespace` routes, omitted `repos` means all repos in that namespace, -and no match falls back to `default_profile`. Host matching is case-insensitive -after normalization, while namespace and repo matching are case-sensitive after -trimming whitespace. An explicit `--profile` bypasses repository routing. Route -targets still use the profile's configured auth mode. Passing `--profile ""` -also counts as explicit and resolves `default_profile` without route lookup. +and no match fails with an actionable error asking for `--profile` or a route. +Host matching is case-insensitive after normalization, while namespace and repo +matching are case-sensitive after trimming whitespace. An explicit `--profile` +bypasses repository routing. Route targets still use the profile's configured +auth mode. Passing `--profile ""` is invalid. For GitHub App auth, `cr review` can use the PR owner/repo to look up the app installation when `github_app_installation_id` is not staged. @@ -230,17 +228,18 @@ Add or replace one credential later: ```bash printf '%s' "$GITHUB_TOKEN" | cr set-credential \ - --ref codereview/default \ + --store local-os \ + --name codereview/default \ --key git_token \ --stdin \ --overwrite ``` -Credential refs use the `codereview/` service/profile form. `cr` -accepts secrets by `--stdin` or `--from-env` during setup and credential writes; -it does not read runtime tokens directly from arbitrary environment variables. -Reviewer credentials use a separate credential ref that also stores key -`git_token`; keep it distinct from the user Git ref. +Credential names use the visible `codereview/` form. `cr` accepts secrets +by `--stdin` or `--from-env` during setup and credential writes; it does not +read runtime tokens directly from arbitrary environment variables. Reviewer +credentials use a separate credential name that also stores key `git_token`; +keep it distinct from the user Git credential in the same store. ### Org Deployment / MDM @@ -271,27 +270,38 @@ permissions needed by the enabled review workflow: Thread resolution uses the pull request review-thread GraphQL surface, so keep Pull requests write access when `review_policy.resolve_threads` is enabled. -Either omit `keyring.backend` to use the platform default, or set it -per-platform and run `set-credential` with the same backend selection. - Example non-secret config template: ```yaml -default_profile: work +secrets: + stores: + work-1password: + display_name: 1Password Work + backend: + kind: op-desktop + onepassword: + account_url: signalft.1password.com + vault_name: Employee profiles: work: git: host: github.com auth_mode: pat - credential_ref: codereview/work + credential: + store: local-os + name: codereview/work reviewer_credentials: auth_mode: github_app - credential_ref: codereview/work-reviewer-app + credential: + store: work-1password + name: codereview/work-reviewer-app llm: provider: anthropic auth: api_key adapter: anthropic_api - credential_ref: codereview/work-llm + credential: + store: work-1password + name: codereview/work-llm model_map: medium: claude-sonnet-4-6 agent_sources: @@ -301,7 +311,7 @@ profiles: ``` For adapter-managed LLM credentials, use `auth: subscription` and omit -`llm.credential_ref`. Codex-backed profiles must set `provider: openai`, +`llm.credential`. Codex-backed profiles must set `provider: openai`, `auth: subscription`, and `adapter: codex_cli` together. Pi-backed profiles must set `provider: pi`, `auth: subscription`, and `adapter: pi_rpc` together: @@ -402,32 +412,37 @@ cr --profile work init --non-interactive \ --llm-credential-ref codereview/work-llm printf '%s' "$USER_GITHUB_TOKEN" | cr set-credential \ - --ref codereview/work \ + --store local-os \ + --name codereview/work \ --key git_token \ --stdin \ --overwrite printf '%s' "$GITHUB_APP_ID" | cr set-credential \ - --ref codereview/work-reviewer-app \ + --store local-os \ + --name codereview/work-reviewer-app \ --key github_app_id \ --stdin \ --overwrite printf '%s' "$GITHUB_APP_PRIVATE_KEY" | cr set-credential \ - --ref codereview/work-reviewer-app \ + --store local-os \ + --name codereview/work-reviewer-app \ --key github_app_private_key \ --stdin \ --overwrite # Optional: needed for cr me and other commands without repository context. printf '%s' "$GITHUB_APP_INSTALLATION_ID" | cr set-credential \ - --ref codereview/work-reviewer-app \ + --store local-os \ + --name codereview/work-reviewer-app \ --key github_app_installation_id \ --stdin \ --overwrite printf '%s' "$ANTHROPIC_API_KEY" | cr set-credential \ - --ref codereview/work-llm \ + --store local-os \ + --name codereview/work-llm \ --key anthropic_api_key \ --stdin \ --overwrite @@ -465,15 +480,14 @@ credential backend. Run `cr config show` to inspect the active profile and credential status. ```yaml -default_profile: default -keyring: - backend: keychain profiles: default: git: host: github.com auth_mode: pat - credential_ref: codereview/default + credential: + store: local-os + name: codereview/default llm: provider: anthropic auth: subscription @@ -507,9 +521,8 @@ Supported values: | `data.retention.enforcement` | `at_write` applies review-time pruning before each `cr review`; `manual_only` disables review-time pruning and leaves `cr data prune` as the explicit maintenance path. | `subscription` LLM auth means the adapter owns its own credentials, such as a -logged-in CLI or local runtime. `api_key` LLM auth requires -`llm.credential_ref` and stores a provider-specific API key in the credential -backend. +logged-in CLI or local runtime. `api_key` LLM auth requires `llm.credential` +and stores a provider-specific API key in the selected credential store. Reviewer agents use provider-neutral `model_tier` values as minimum floors. At runtime, `cr` combines the profile's `llm.reviewer_model_tier` baseline with @@ -578,14 +591,14 @@ Credential key matrix: | Profile field | Purpose | Auth/provider | Required keys | Optional keys | v1 behavior | |---------------|---------|---------------|---------------|---------------|-------------| -| `git.credential_ref` | User Git host auth | `pat` | `git_token` | None | Supported | -| `reviewer_credentials.credential_ref` | Reviewer Git host auth | `pat` | `git_token` | None | Supported; must use a distinct ref from `git.credential_ref` in the same profile | -| `git.credential_ref` / `reviewer_credentials.credential_ref` | Git host auth | `github_app` | `github_app_id`, `github_app_private_key` | `github_app_installation_id` | Supported for GitHub. `cr review` can discover the installation from the PR repository when the optional installation ID is omitted; commands without repository context require it | -| `git.credential_ref` / `reviewer_credentials.credential_ref` | Git host auth | `oauth_device` | None | None | Reserved; config recognizes the mode but v1 rejects it and does not accept future keys such as `git_oauth_access_token` or `git_oauth_refresh_token` | -| `llm.credential_ref` | Anthropic direct API auth | `api_key` + `anthropic` | `anthropic_api_key` | None | Supported | -| `llm.credential_ref` | OpenAI direct API auth | `api_key` + `openai` | `openai_api_key` | None | Supported | -| Omitted `llm.credential_ref` | Adapter-managed LLM auth | `subscription` + `anthropic`/`openai`/`pi` | None | None | Supported; credentials are owned by the selected adapter. `openai + codex_cli` is best-effort/beta until Codex exposes an explicit all-tools-disabled flag | -| `llm.credential_ref` | Pi direct API auth | `api_key` + `pi` | None | None | Unsupported; use adapter-managed `subscription` auth with `pi_rpc` | +| `git.credential` | User Git host auth | `pat` | `git_token` | None | Supported | +| `reviewer_credentials.credential` | Reviewer Git host auth | `pat` | `git_token` | None | Supported; must use a distinct credential location from `git.credential` in the same profile | +| `git.credential` / `reviewer_credentials.credential` | Git host auth | `github_app` | `github_app_id`, `github_app_private_key` | `github_app_installation_id` | Supported for GitHub. `cr review` can discover the installation from the PR repository when the optional installation ID is omitted; commands without repository context require it | +| `git.credential` / `reviewer_credentials.credential` | Git host auth | `oauth_device` | None | None | Reserved; config recognizes the mode but v1 rejects it and does not accept future keys such as `git_oauth_access_token` or `git_oauth_refresh_token` | +| `llm.credential` | Anthropic direct API auth | `api_key` + `anthropic` | `anthropic_api_key` | None | Supported | +| `llm.credential` | OpenAI direct API auth | `api_key` + `openai` | `openai_api_key` | None | Supported | +| Omitted `llm.credential` | Adapter-managed LLM auth | `subscription` + `anthropic`/`openai`/`pi` | None | None | Supported; credentials are owned by the selected adapter. `openai + codex_cli` is best-effort/beta until Codex exposes an explicit all-tools-disabled flag | +| `llm.credential` | Pi direct API auth | `api_key` + `pi` | None | None | Unsupported; use adapter-managed `subscription` auth with `pi_rpc` | Upgrade note: pre-matrix versions used the generic `llm_api_key` key for direct LLM API credentials. Re-provision API-key LLM refs with `anthropic_api_key` or @@ -678,8 +691,8 @@ All commands accept the global flags: | Flag | Semantics | |------|-----------| -| `--profile ` | Select a configured profile and bypass repo-aware routing. When omitted, PR-aware commands may use `repository_profiles`, then fall back to `default_profile`; during `init`, empty means `default`. | -| `--backend ` | Select the credential backend for this invocation. One of `keychain`, `wincred`, `secret-service`, `file`, `pass`, `memory`. | +| `--profile ` | Select a configured profile and bypass repo-aware routing. When omitted, PR-aware commands use `repository_profiles`; commands that cannot resolve a route require `--profile`. During `init`, an omitted profile starts with the suggested name `default`. | +| `--backend ` | Compatibility/runtime backend selector. It cannot override an explicit credential store destination. | ### `cr` @@ -715,19 +728,20 @@ Flags: |------|-----------| | `--non-interactive` | Required in v1. Run without prompts. | | `--git-host ` | Git host, default `github.com`. The PR host must match this value. | -| `--git-credential-ref ` | Credential ref for Git auth. Defaults to `codereview/`. | +| `--git-credential-ref ` | Credential name for Git auth. Defaults to `codereview/`. | | `--git-token-stdin` | Read the Git token from stdin and write key `git_token`. | | `--git-token-from-env ` | Read the Git token from an environment variable and write key `git_token`. | -| `--reviewer-credential-ref ` | Credential ref for reviewer Git auth. Defaults to `codereview/-reviewer` when reviewer credentials are requested. | +| `--reviewer-credential-ref ` | Credential name for reviewer Git auth. Defaults to `codereview/-reviewer` when reviewer credentials are requested. | | `--reviewer-auth-mode ` | Reviewer Git auth mode, default `pat`. `github_app` writes config and prints follow-up credential commands. | | `--reviewer-token-stdin` | Read the reviewer Git token from stdin and write key `git_token`; PAT reviewer auth only. | | `--reviewer-token-from-env ` | Read the reviewer Git token from an environment variable and write key `git_token`; PAT reviewer auth only. | | `--llm-provider ` | LLM provider, default `anthropic`; also selects whether API-key ingress writes `anthropic_api_key` or `openai_api_key`. `pi` is adapter-managed and does not accept API-key ingress. | -| `--llm-auth ` | LLM auth mode, default `subscription`. Use `api_key` for keyring-managed direct API keys. | +| `--llm-auth ` | LLM auth mode, default `subscription`. Use `api_key` for credential-store-managed direct API keys. | | `--llm-adapter ` | LLM adapter, default `claude_cli`. | -| `--llm-credential-ref ` | Credential ref for LLM API-key auth. Defaults to `codereview/-llm` when `--llm-auth api_key`. | +| `--llm-credential-ref ` | Credential name for LLM API-key auth. Defaults to `codereview/-llm` when `--llm-auth api_key`. | | `--llm-api-key-stdin` | Read the LLM API key from stdin and write `anthropic_api_key` or `openai_api_key` according to `--llm-provider`. | | `--llm-api-key-from-env ` | Read the LLM API key from an environment variable and write `anthropic_api_key` or `openai_api_key` according to `--llm-provider`. | +| `--secret-backend-discovery ` | Interactive secrets-backend discovery mode: `full` runs active inventory probes such as 1Password account/vault lookup, `safe` uses only passive availability checks, and `off` skips backend discovery. Defaults to `full`; `CR_SECRET_BACKEND_DISCOVERY` can set the same value when the flag is omitted. | | `--agent-source ` | Add a trusted agent source directory. Repeatable. | | `--major-event ` | `comment` or `request_changes`. Controls review event for major findings. | | `--allow-self-approve` | Store profile policy allowing self approval. Live review can still require `--allow-self-approve` depending on invocation. | @@ -737,7 +751,7 @@ Flags: | `--replace-profile` | Replace an existing profile config. | Only one stdin secret ingress flag may be used at a time. PAT reviewer -credentials use key `git_token` under their own credential ref, so +credentials use key `git_token` under their own credential name, so `--reviewer-credential-ref` must differ from `--git-credential-ref`. GitHub App reviewer credentials use `github_app_id` and `github_app_private_key`, plus optional `github_app_installation_id`; `init` does not accept reviewer token @@ -749,25 +763,26 @@ ingress flag. `--allow-self-review` is intentionally runtime-only on ### `cr set-credential` ```text -cr set-credential --ref --key (--stdin | --from-env ) [flags] +cr set-credential --store --name --key (--stdin | --from-env ) [flags] ``` -Writes one secret value to the credential store. Globally allowed keys are +Writes one secret value to the selected credential store. Globally allowed keys are `git_token`, `github_app_id`, `github_app_private_key`, `github_app_installation_id`, `anthropic_api_key`, and `openai_api_key`. When -`config.yml` declares the target ref, `set-credential` narrows that global -allowlist to the exact key set expected for that ref. PAT user Git refs and PAT -reviewer refs use `git_token`; GitHub App refs use `github_app_id`, +`config.yml` declares the target credential location, `set-credential` narrows +that global allowlist to the exact key set expected for that credential. PAT +user Git credentials and PAT reviewer credentials use `git_token`; GitHub App credentials use `github_app_id`, `github_app_private_key`, and optional `github_app_installation_id`; Anthropic -LLM API-key refs use `anthropic_api_key`; OpenAI LLM API-key refs use +LLM API-key credentials use `anthropic_api_key`; OpenAI LLM API-key credentials use `openai_api_key`. Flags: | Flag | Semantics | |------|-----------| -| `--ref ` | Required credential ref, such as `codereview/default`. | -| `--key ` | Required key name. Git refs accept the keys described in the credential key matrix; LLM API-key refs accept `anthropic_api_key` or `openai_api_key`. | +| `--store ` | Required credential store id, such as `local-os` or a configured store. | +| `--name ` | Required credential name, such as `codereview/default`. | +| `--key ` | Required key name. Git credentials accept the keys described in the credential key matrix; LLM API-key credentials accept `anthropic_api_key` or `openai_api_key`. | | `--stdin` | Read the secret from stdin. | | `--from-env ` | Read the secret from an environment variable. | | `--overwrite` | Replace an existing key. Without it, existing keys are not overwritten. | @@ -782,9 +797,9 @@ cr config show [--json] ``` Shows the resolved active profile, selected credential backend and source, -credential refs, non-secret profile config, review policy, and data retention. -For each declared credential ref, it reports whether expected keys are present -and labels optional keys as optional. +credential locations, non-secret profile config, review policy, and data +retention. For each declared credential location, it reports whether expected +keys are present and labels optional keys as optional. For each configured agent source, it reports deployment-material status, canonical path, warnings, and SHA-256 fingerprint prefix without inlining agent definition contents. @@ -863,15 +878,13 @@ Flags: | Flag | Semantics | |------|-----------| | `--all` | Also remove the active profile from `config.yml` and clear the disposable cache root. Inactive profiles and durable data are not touched. | -| `--dry-run` | Report the credential refs, config profile reset, and cache cleanup that would happen without deleting credentials, config, cache, or data. | +| `--dry-run` | Report the credential locations, config profile reset, and cache cleanup that would happen without deleting credentials, config, cache, or data. | | `--json` | Emit a JSON result. | Without `--all`, this removes secret keyring entries only and leaves `config.yml` intact. With `--all`, `--profile ` selects the profile to -reset; otherwise the default profile is reset. If other profiles remain after a -profile reset, the default profile is updated deterministically to the first -remaining profile name when needed. If the last profile is reset, `config.yml` -is removed. +reset; without a selected profile, the command fails. If the last profile is +reset, `config.yml` is removed. `config clear` never touches durable review data. Use `cr data purge` for the data pillar. @@ -1194,7 +1207,7 @@ The HTTP cache is under the OS cache directory for the `cr` binary. Releases before this state-layout alignment wrote data/cache below a nested `codereview` child inside the `cr` root. Commands that write review state migrate those legacy entries into the `cr` root without changing config or -credential refs. +credential locations. ### Posting, Markers, And Idempotency diff --git a/cmd/cr/main_test.go b/cmd/cr/main_test.go index b9f8284d..6e186d29 100644 --- a/cmd/cr/main_test.go +++ b/cmd/cr/main_test.go @@ -77,7 +77,7 @@ func TestRunConfigShowJSON(t *testing.T) { } var stdout, stderr bytes.Buffer - code := run([]string{"config", "show", "--json"}, strings.NewReader(""), &stdout, &stderr) + code := run([]string{"--profile", "home", "config", "show", "--json"}, strings.NewReader(""), &stdout, &stderr) if code != 0 { t.Fatalf("run exit code = %d, want 0; stderr = %q", code, stderr.String()) } @@ -144,6 +144,13 @@ cases: func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) { statedirtest.Hermetic(t) t.Setenv("CODEREVIEW_KEYRING_PASSPHRASE", "test-passphrase") + path, err := config.Path() + if err != nil { + t.Fatalf("config Path: %v", err) + } + if err := config.Save(path, mainCredentialCommandTestConfig()); err != nil { + t.Fatalf("config Save: %v", err) + } const ( firstSecret = "distinctive-secret-one" secondSecret = "distinctive-secret-two" @@ -158,25 +165,26 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) { wantCode int }{ { - name: "init", + name: "set initial credential", args: []string{ - "--backend", "file", - "init", - "--non-interactive", - "--git-token-from-env", "CR_TOKEN_ONE", + "set-credential", + "--store", "test-file", + "--name", "codereview/default", + "--key", "git_token", + "--from-env", "CR_TOKEN_ONE", }, }, { name: "config show", - args: []string{"--backend", "file", "config", "show", "--json"}, + args: []string{"--profile", "default", "config", "show", "--json"}, wantCode: 0, }, { name: "existing credential failure", args: []string{ - "--backend", "file", "set-credential", - "--ref", "codereview/default", + "--store", "test-file", + "--name", "codereview/default", "--key", "git_token", "--from-env", "CR_TOKEN_TWO", }, @@ -185,9 +193,9 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) { { name: "set overwrite", args: []string{ - "--backend", "file", "set-credential", - "--ref", "codereview/default", + "--store", "test-file", + "--name", "codereview/default", "--key", "git_token", "--from-env", "CR_TOKEN_TWO", "--overwrite", @@ -196,7 +204,7 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) { }, { name: "config clear", - args: []string{"--backend", "file", "config", "clear", "--json"}, + args: []string{"--profile", "default", "config", "clear", "--json"}, wantCode: 0, }, } @@ -216,14 +224,15 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) { func mainTestConfig() config.File { return config.File{ - DefaultProfile: "home", - Keyring: config.KeyringConfig{Backend: "memory"}, Profiles: map[string]config.Profile{ "home": { Git: config.GitConfig{ - Host: "github.com", - AuthMode: config.GitAuthModePAT, - CredentialRef: "codereview/home", + Host: "github.com", + AuthMode: config.GitAuthModePAT, + Credential: config.CredentialLocation{ + Store: config.LocalOSCredentialStoreID, + Name: "codereview/home", + }, }, LLM: config.LLMConfig{ Provider: config.LLMProviderAnthropic, @@ -236,6 +245,24 @@ func mainTestConfig() config.File { } } +func mainCredentialCommandTestConfig() config.File { + cfg := mainTestConfig() + cfg.Secrets = config.SecretsConfig{ + Stores: map[string]config.SecretsStore{ + "test-file": { + DisplayName: "Test file store", + Backend: config.SecretsStoreBackend{ + Kind: config.SecretsBackendKind("file"), + }, + }, + }, + } + profile := cfg.Profiles["home"] + profile.Git.Credential = config.CredentialLocation{Store: "test-file", Name: "codereview/default"} + cfg.Profiles = map[string]config.Profile{"default": profile} + return cfg +} + func assertNoSecretLeak(t *testing.T, output string, secrets ...string) { t.Helper() for _, secret := range secrets { diff --git a/docs/config-command-surface-design.md b/docs/config-command-surface-design.md index 77c7f93a..650288b7 100644 --- a/docs/config-command-surface-design.md +++ b/docs/config-command-surface-design.md @@ -11,11 +11,11 @@ bootstrap, secret ingress, or the full long-term profile management API. This document is the semantic parent for: -- `#167`: `cr config path` and default-profile commands +- `#167`: `cr config path` - `#166`: `cr config route` management commands - `#169`: `cr config resolve-profile` route preview - `#168`: `cr config agent-source` management commands -- `#294`: `cr config secrets-profile` management commands +- `#294`: `cr config credential-store` inspection commands Related but out of scope for this batch: @@ -49,10 +49,10 @@ The codebase already supports: - GitHub App reviewer auth The missing piece is command coverage. Today `cr config` exposes `show`, -`clear`, and `llm models`, but path/default/route/route-preview/agent-source -operations still require manual YAML edits. Named secrets-management profiles -also need a non-interactive command surface, but that surface must remain -config-only until the later secrets-profile resolver/selection tickets land. +`clear`, and `llm models`, but path/route/route-preview/agent-source +operations still require manual YAML edits. Credential-store inspection is +available through `cr config credential-store`; creating, editing, and deleting +configured stores belongs to the interactive `cr init` secrets-storage flow. ## Shared Command Rules @@ -63,7 +63,7 @@ These rules apply across the batch unless a ticket explicitly narrows them. - Commands that operate on a profile should use the existing global `--profile ` flag. - When a command is not repository-aware, `--profile` selects the target - profile and otherwise falls back to `default_profile`. + profile; commands that need a profile must fail if none is selected. - Commands introduced in this batch must not invent a second profile-selection mechanism. @@ -103,13 +103,11 @@ These rules apply across the batch unless a ticket explicitly narrows them. ## Ticket Ownership Matrix -### `#167`: path and default profile +### `#167`: path Owns: - `cr config path [--json]` -- `cr config default get [--json]` -- `cr config default set ` Must not own: @@ -120,8 +118,6 @@ Must not own: Required semantics: -- `default set` changes only `default_profile` -- `default set` validates that the target profile exists - path inspection reports resolved config location only; broader state-path expansion is optional follow-up work, not required by this ticket @@ -137,7 +133,6 @@ Must not own: - route preview output beyond what is needed for route list/mutation - runtime route-resolution redesign -- default-profile mutation Required semantics: @@ -189,8 +184,7 @@ Must not own: Required semantics: - reuse the same repository-resolution path already used by runtime commands -- report whether resolution came from explicit `--profile`, matched route, or - fallback to `default_profile` +- report whether resolution came from explicit `--profile` or a matched route - remain a local preview command, not an execution path - if the existing runtime resolution path does not expose enough metadata for preview output, `#169` may add a shared resolution-result wrapper/helper that @@ -237,40 +231,30 @@ Path normalization contract: This keeps mutation semantics deterministic and local-string-based rather than filesystem-state-based. -### `#294`: secrets-management profile mutation +### `#294`: credential-store inspection Owns: -- `cr config secrets-profile list [--json]` -- `cr config secrets-profile get [--json]` -- `cr config secrets-profile set [--backend ] [--label ] [--clear-label] [--op-timeout ] [--op-vault-id ] [--op-item-title-prefix ] [--op-item-tag ] [--op-item-field-title ] [--op-connect-host ] [--op-connect-token-env ] [--op-service-token-env ] [--op-desktop-account-id ]` -- `cr config secrets-profile default get [--json]` -- `cr config secrets-profile default set ` -- `cr config secrets-profile default unset` -- `cr config secrets-profile remove ` +- `cr config credential-store list [--json]` +- `cr config credential-store get [--json]` Must not own: - secret ingress or secret-value printing -- runtime credential routing away from legacy `keyring.backend` -- review-profile references to secrets-management profiles before the later selection tickets +- creating, editing, deleting, or selecting credential stores for a write +- any user-facing credential-store default Required semantics: -- `list` and `default get` report the effective secrets-profile inventory/default, including the projected `legacy-default` compatibility entry when no explicit profiles exist -- `default set` and `default unset` mutate only `config.secrets.default_profile` -- `default set` rejects `legacy-default` because it is reserved/projected, not configured state -- `set` is patch-style: - - create requires `--backend` - - update preserves omitted fields - - `--clear-label` explicitly clears the label - - `--label` plus `--clear-label` is a usage error - - whitespace-only `--label` is rejected - - update requires at least one mutation flag - - `--backend op`, `--backend op-connect`, and `--backend op-desktop` also require `--op-vault-id`; `op-connect` additionally requires `--op-connect-host` -- `remove` is idempotent for already-absent configured profiles, but rejects `legacy-default` and blocks removing the configured default profile until it is unset or moved +- `list` reports the effective credential-store inventory, including the + read-only built-in `local-os` store +- `get` reports one effective credential store by id +- configured stores are listed after `local-os` in stable order +- `local-os` is projected and is never persisted under `secrets.stores` +- JSON output must not include an `is_default` field for credential stores - backend validation must reuse shared credstore metadata rather than duplicating backend-name knowledge in command code -- this ticket is config-only: mixed configs with both `keyring.backend` and `secrets.*` keep legacy runtime backend behavior until the later resolver ticket changes credential selection +- old `config secrets-profile ...` subcommands are rejected instead of acting as + compatibility aliases ## `init` Boundary @@ -300,7 +284,7 @@ The likely future design questions remain: - whether profile mutation should be patch-like or declarative replacement - whether field-specific subcommands and `profile set` should coexist -- how profile mutation should interact with credential refs and route safety +- how profile mutation should interact with credential locations and route safety Those questions belong to later work after the current true-up lands. diff --git a/docs/init-config-surface.md b/docs/init-config-surface.md index 7bb578d3..12416ae3 100644 --- a/docs/init-config-surface.md +++ b/docs/init-config-surface.md @@ -41,40 +41,30 @@ intentionally unsupported. | Path | Interactive handling | Scripted owner | Defaults and second-run prepopulation | Mutation semantics | Helper/command owner and expected tests | |------|----------------------|----------------|---------------------------------------|--------------------|-----------------------------------------| -| config.default_profile | Core profile wizard lets the user preserve, set to the selected profile, or choose another existing profile. | Existing `cr config default set`; `cr init --set-default` owns the scripted "make this profile default" case during init. | First init sets the created profile as default. Existing value is pre-populated and preserved by default. | Overwrite only to an existing profile. Profile rename updates this field when it points at the renamed profile. | #177 shared profile rename/default helper. #180 tests create, preserve, set, and rename updates. | -| config.keyring.backend | Configure secrets management now keeps `keyring.backend` only as an explicit legacy-compatibility row inside the secrets-management workflow. | `cr init --keyring-backend` and `--reset-keyring-backend` remain the scripted durable surface. Existing global `--backend` stays runtime-only and still conflicts with persisted defaults. | Empty means backend auto/env selection. Existing value is pre-populated in the legacy compatibility editor. | Preserve by leaving the compatibility row unchanged. Overwrite validates through credential backend parsing. Clearing the selection resets the field to auto/env behavior. Persistent `op`, `op-connect`, and `op-desktop` values remain rejected here; 1Password must be configured through a named secrets-management profile instead. | Existing keyring validation plus #184/#187 tests cover set, reset, backend conflict, legacy runtime persistence, and unrelated config preservation. #297 adds the interactive legacy-compatibility row. | -| config.secrets.default_profile | Secrets-management workflow can stage or clear the configured default named secrets-management profile while creating or editing a profile. | `cr config secrets-profile default get/set/unset`. Not owned by legacy `keyring.backend`. | Empty means there is no configured default named secrets-management profile. Legacy configs instead project a read-only `legacy-default` compatibility profile in presentation/helpers. Existing default state is pre-populated in the editor. | Overwrite only to an existing named secrets-management profile id. `legacy-default` is reserved/projected and cannot be set. Global runtime `--backend` still conflicts with a configured named default during the current init invocation. | #293 establishes schema, validation, and compatibility projection. #294 adds scripted default get/set/unset. #297 adds interactive staging. | -| config.secrets.profiles..label | Secrets-management workflow can create and edit human-friendly secrets-management profile labels directly. | `cr config secrets-profile set --label/--clear-label`. | Optional. Existing values are pre-populated. New profiles start from a backend-derived default label so the UI never opens on a blank unknown item. | Preserve on skip. Overwrite trims whitespace. Must remain a single line. Duplicate labels are allowed; stable ids remain authoritative and are deconflicted separately for new profiles by slugging the chosen label and appending `-2`, `-3`, and so on when needed. | #293 establishes schema/validation plus safe display summaries. #294 adds scripted create/update/clear-label behavior. #297 adds flat interactive editing. | -| config.secrets.profiles..backend.kind | Secrets-management workflow can create a named profile for any surfaced backend and edit an existing profile's durable backend choice inline. | `cr config secrets-profile set --backend`. | Required for an explicit named secrets-management profile. Existing values are pre-populated. Legacy configs with no explicit `secrets.profiles` continue to project `legacy-default` from `keyring.backend` or auto/env resolution without persisting a synthetic entry. | Preserve on skip. Overwrite validates against recognized credential-store backend names. Backend-specific 1Password metadata is edited in the same flat workflow and cleared automatically when switching away from 1Password. | #293 establishes schema/validation, projected legacy inventory, and config-show summaries. #294 adds scripted create/update/remove/default management. #297 adds interactive inventory and flat editing. | -| config.secrets.profiles..backend.onepassword.timeout | Secrets-management workflow edits the non-secret 1Password timeout inline whenever the selected backend kind needs it. | `cr config secrets-profile set --op-timeout`. | Empty input normalizes to the current service-account/desktop default timeout when that backend family requires one. Existing value is pre-populated. | Preserve on skip. Overwrite validates as a Go duration string. Switching away from 1Password clears the field from config. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.vault_id | Secrets-management workflow edits the non-secret 1Password vault selection inline. | `cr config secrets-profile set --op-vault-id`. | Required for `op`, `op-connect`, and `op-desktop`. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Switching away from 1Password clears the field from config. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.item_title_prefix | Secrets-management workflow edits the optional 1Password item naming override inline. | `cr config secrets-profile set --op-item-title-prefix`. | Empty means use the underlying keyring library default. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Switching away from 1Password clears the field from config. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.item_tag | Secrets-management workflow edits the optional 1Password item tag override inline. | `cr config secrets-profile set --op-item-tag`. | Empty means use the underlying keyring library default. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Switching away from 1Password clears the field from config. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.item_field_title | Secrets-management workflow edits the optional 1Password item-field title override inline. | `cr config secrets-profile set --op-item-field-title`. | Empty means use the underlying keyring library default. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Switching away from 1Password clears the field from config. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.connect_host | Secrets-management workflow edits the non-secret 1Password Connect endpoint inline when `op-connect` is selected. | `cr config secrets-profile set --op-connect-host`. | Required for `op-connect`. Ignored for other backend kinds. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Backend switches clear it when no longer relevant. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.connect_token_env | Secrets-management workflow edits the non-secret env-var name for the 1Password Connect token inline when `op-connect` is selected. | `cr config secrets-profile set --op-connect-token-env`. | Defaults to `OP_CONNECT_TOKEN` for `op-connect`. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Backend switches clear it when no longer relevant. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.service_token_env | Secrets-management workflow edits the non-secret env-var name for the 1Password service-account token inline when `op` is selected. | `cr config secrets-profile set --op-service-token-env`. | Defaults to `OP_SERVICE_ACCOUNT_TOKEN` for `op`. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Backend switches clear it when no longer relevant. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.secrets.profiles..backend.onepassword.desktop_account_id | Secrets-management workflow edits the optional explicit 1Password desktop account id inline when `op-desktop` is selected. | `cr config secrets-profile set --op-desktop-account-id`. | Optional for `op-desktop`; empty means the runtime may fall back to `OP_DESKTOP_ACCOUNT_ID`. Existing value is pre-populated. | Preserve on skip. Overwrite trims whitespace. Backend switches clear it when no longer relevant. | #296 adds schema, validation, runtime mapping, and command coverage. #297 adds interactive editing. | -| config.profiles..secrets_profile | Review-profile editing now exposes a selection-only `Secrets management` field that chooses an existing named secrets-management profile or clears back to the built-in/default runtime path. It must not configure new backends inline. | Manual YAML plus the interactive selector. Still not owned by legacy `keyring.backend`. | Empty means the review profile follows legacy/default runtime resolution: use `secrets.default_profile` when configured, otherwise fall back to legacy `keyring.backend`/env/auto behavior. Existing explicit values are pre-populated when valid. Missing/deleted refs are surfaced as repairable recovery selections during interactive init. | Overwrite only to an existing named secrets-management profile id. `legacy-default` is reserved/projected and must not be persisted directly. Clearing the field restores legacy/default runtime resolution rather than forcing a named profile. Interactive init may temporarily open configs with `ErrSecretsProfileNotFound` so the user can repair the dangling ref, but final validation/save still rejects unresolved missing refs. | #295 introduces the schema plus runtime credential routing through this field. #298 adds the user-facing selector and missing-reference recovery UX. | -| config.repository_profiles[] | Route editor opens directly with existing namespace and repo routes prefilled so the user can edit them in place or blank the field to remove all routes for the profile. | Existing `cr config route set` and `cr config route unset`; #186 documents route commands. #187 must not add a nested multi-route init grammar. | Empty or omitted means default-profile fallback. Existing routes are prefilled on later runs. | Leaving the prefilled text unchanged preserves routes. Mutations must converge through shared route helpers and preserve unrelated routes. Blanking the field removes the profile's routes. | #177 extracts route helpers. #185 tests list/edit/remove, route canonicalization, and preservation. | +| config.secrets.stores | Configure secrets storage creates and edits user-configured credential stores. The built-in OS credential store is projected as `local-os` and is not persisted here. | Interactive main-menu secrets-storage flow only. Scripted credential writes choose an existing store explicitly. | Omitted means only the built-in OS credential store is available. Existing stores are pre-populated in the inventory. | Store ids must not be `local-os`. Store metadata is non-secret. Creating, editing, and deleting configured stores never deletes credential values. | #356 establishes explicit stores and removes ambient/default credential-store selection. | +| config.llm_runtimes | Configure LLM runtimes creates and edits reusable runtime definitions independently from review profiles. Profiles select one configured runtime when composing a reviewer. | Interactive main-menu LLM runtime flow only for now. Future config commands may own scripted runtime CRUD. | Omitted means no reusable runtimes are configured yet. The profile editor can send the user to the runtime flow when no runtime exists. Existing runtimes are pre-populated in the inventory. | Runtime names are stable ids. Subscription runtimes store only non-secret provider/auth/adapter settings. API-key runtimes store an explicit credential location and never the secret value. Deleting a runtime requires affected profiles to select a replacement or be edited. | #356 makes runtime setup standalone and keeps profile composition explicit. | +| config.repository_profiles[] | Route editor opens directly with existing namespace and repo routes prefilled so the user can edit them in place or blank the field to remove all routes for the profile. | Existing `cr config route set` and `cr config route unset`; #186 documents route commands. #187 must not add a nested multi-route init grammar. | Empty or omitted means no automatic repository profile selection; runtime commands require `--profile` until a matching route is configured. Existing routes are prefilled on later runs. | Leaving the prefilled text unchanged preserves routes. Mutations must converge through shared route helpers and preserve unrelated routes. Blanking the field removes the profile's routes. | #177 extracts route helpers. #185 tests list/edit/remove, route canonicalization, and preservation. | | config.repository_profiles[].profile | Route wizard selects the profile that a route resolves to. | Existing `cr config route set --profile ...`. | No default inside an entry; required when a route exists. Existing profile is pre-populated. | Overwrite only to an existing profile. Profile rename updates matching route entries. | #177 profile rename/route reference helper. #185 tests rename updates and invalid profile rejection. | | config.repository_profiles[].match.host | Route wizard derives from selected profile host or pasted PR URL. | Existing `cr config route set --host`. | Required for a route. Existing host is pre-populated and normalized. | Must match the target profile's `git.host`. Host edits on a profile with routes are blocked or reconciled by #185. | #177 route helper, #185 PR URL derivation and host/profile validation tests. | | config.repository_profiles[].match.namespace | Route wizard asks for owner/org or derives from PR URL. | Existing `cr config route set --namespace`. | Required for a route. Existing namespace is pre-populated. | Overwrite validates non-empty and preserves repo-vs-namespace route identity. | #177 route helper. #185 namespace route tests. | | config.repository_profiles[].match.repos[] | Route wizard supports namespace-wide route when omitted or repo-specific routes when provided. | Existing repeatable `cr config route set --repo` and `cr config route unset --repo`. | Omitted means namespace-wide route. Existing repos are pre-populated in deterministic order. | Preserve on skip. Add/edit/remove dedupes and sorts via shared route helper. Clear repos converts only when the user explicitly chooses namespace-wide routing. | #177 route helper. #185 repo route and namespace conversion tests. | -| config.profiles. | Core profile wizard selects existing profile, creates a new profile, or renames an existing profile. | Existing global `--profile` with `cr init --non-interactive` owns scripted create/replace. Scripted rename is intentionally unsupported by init and belongs to future profile-management command design. | Empty global `--profile` means `default` during current init. Existing profile names are pre-populated. | Create requires a valid profile body. Rename preserves credential refs by default, updates default_profile and routes, and does not delete old keyring entries. | #177 profile rename helper. #180 tests create/select/rename and validation. | +| config.profiles. | Core profile wizard selects existing profile, creates a new profile, or renames an existing profile. | Existing global `--profile` with `cr init --non-interactive` owns scripted create/replace. Scripted rename is intentionally unsupported by init and belongs to future profile-management command design. | When `cr init` starts without a global `--profile`, it suggests `default` as the new profile name. Existing profile names are pre-populated. | Create requires a valid profile body. Rename preserves credential locations by default, updates repository routes, and does not delete old credential-store entries. | #177 profile rename helper. #180 tests create/select/rename and validation. | | config.profiles..git.host | Core profile wizard edits Git host. | Existing `cr init --git-host`; existing route commands own route-safe scripted changes. | Current init defaults to `github.com`. Existing value is pre-populated. | Set validates non-empty normalized host. If routes reference the profile and reconciliation is not selected, block or defer the edit. | #180 blocks/defer route-unsafe host edits. #185 implements reconciliation tests. | | config.profiles..git.auth_mode | Core profile wizard chooses `pat` or `github_app`; `oauth_device` remains reserved. | `cr init --git-auth-mode`, parallel to existing reviewer auth mode. Current init otherwise defaults user Git auth to PAT. | Existing value is pre-populated. New profile defaults to `pat`. | Overwrite only to supported v1 modes. Switching auth modes re-plans credential keys but does not delete old secrets. | #179 credential-ref/key-spec planner. #180 auth-mode prompt tests. | -| config.profiles..git.credential_ref | Credential-ref planner chooses ref for user Git auth before any secret ingress. | Existing `cr init --git-credential-ref`; `cr set-credential` writes secrets. | New profile defaults to `codereview/`. Existing ref is pre-populated and preserved by default. | Flattened interactive init shows the effective Git storage label inline. Leaving that field at the effective default means "follow the selected Git scope's default label" across later scope changes; entering a different value makes it a preserved custom override. Clear is invalid because Git credentials are required. | #179 planner tests default/custom refs and required state. #181 secret ingress tests. | +| config.profiles..git.credential.store | User Git auth chooses an existing credential store before any secret ingress. | Interactive credential-writing flows and scripted `cr set-credential --store`. | New profiles start with `local-os` selected. Existing store id is pre-populated. | Must be `local-os` or a configured store id. Store setup is not available inline from this flow. | #356 explicit destination tests. | +| config.profiles..git.credential.name | User Git auth names the credential bundle before any secret ingress. | Interactive credential-writing flows and scripted `cr set-credential --name`. | New profile defaults to `codereview/`. Existing name is pre-populated and preserved by default. | Must use the `codereview/` credential grammar. It must differ from other credentials in the same profile when the store also matches. | #356 explicit credential-location tests. | | config.profiles..git.identity_cache | Not shown as an editable init field. Preserve only. | Intentionally unsupported for init and config mutation. Runtime identity refresh owns it. | Existing cache is preserved. New profiles omit it. | Preserve unless a future explicit cache invalidation ticket owns behavior. Profile rename does not rewrite cache contents. | Preserve-only regression in #177/#180. | -| config.profiles..reviewer_credentials | Optional reviewer credential section. Wizard supports skip, preserve, enable, edit, or clear reviewer config. | Existing `cr init --reviewer-credential-ref` and `--reviewer-auth-mode` own enable/edit. `cr init --disable-reviewer` owns scripted removal. | Omitted means posting uses Git credentials. Existing section is pre-populated. | Clear removes the whole section. Enable requires auth mode and credential ref. Ref must differ from Git and LLM refs. | #179 planner and #180 optional section tests. #181 secret ingress tests. | +| config.profiles..reviewer_credentials | Optional reviewer credential section. Wizard supports skip, preserve, enable, edit, or clear reviewer config. | Existing `cr init --reviewer-credential-ref` and `--reviewer-auth-mode` own enable/edit. `cr init --disable-reviewer` owns scripted removal. | Omitted means posting uses Git credentials. Existing section is pre-populated. | Clear removes the whole section. Enable requires auth mode and credential location. The store/name pair must differ from Git and LLM credential locations. | #179 planner and #180 optional section tests. #181 secret ingress tests. | | config.profiles..reviewer_credentials.auth_mode | Reviewer wizard chooses PAT or GitHub App; `oauth_device` remains reserved. | Existing `cr init --reviewer-auth-mode`. | Current init defaults reviewer mode to `pat` when reviewer credentials are requested. Existing value is pre-populated. | Overwrite only to supported v1 modes. Switching modes re-plans key specs and preserves old secrets unless explicit overwrite/migration occurs. | #179 credential bundle tests for PAT and GitHub App. | -| config.profiles..reviewer_credentials.credential_ref | Credential-ref planner chooses reviewer Git ref. | Existing `cr init --reviewer-credential-ref`; `cr set-credential` writes secrets. | New reviewer section defaults to `codereview/-reviewer`, or `codereview/