docs(assurance): correct API key storage claims (#1449)

* docs(assurance): correct API key storage claims

ASSURANCE_CASE.md said keys are env-only and never stored in config
files. The CLI persists api_key/auth_token to ~/.opencodereview/config.json
with mode 0600. Align the assurance case with that behavior.

Fixes #1440

* fix(config): enforce 0600 on existing config.json after save

WriteFile only applies mode on create; keep os.Chmod(path, 0o600) after
every saveConfig write so a prior 0644 config is tightened when an API
key is persisted. Document that behavior in ASSURANCE_CASE and add a
regression test for the existing-file case.

Signed-off-by: Frank_zhu <58329837+Frank-zhu0404@users.noreply.github.com>

* fix(config): chmod existing config to 0600 before rewrite (#1440)

* fix(config): align saveConfig chmod with review suggestion (#1440)

* test(config): chmod seed to 0644 independent of umask (#1440)

WriteFile mode is masked by umask 077; set seed permissions explicitly
so TestSaveConfigEnforcesModeOnExistingFile stays umask-independent.

Signed-off-by: Frank_zhu <58329837+Frank-zhu0404@users.noreply.github.com>

---------

Signed-off-by: Frank_zhu <58329837+Frank-zhu0404@users.noreply.github.com>

ASSURANCE_CASE.md

@@ -59,13 +59,23 @@ OCR is a CLI tool that:
 | ID | Threat | Boundary | Mitigation |
 |----|--------|----------|------------|
 | T1 | Command injection via crafted diff content | 1 | All external commands are `git` only, with hardcoded subcommands; no shell expansion; `--end-of-options` used to prevent flag injection |
-| T2 | API key leakage | 2 | Keys read from environment variables only; never logged, written to output files, or transmitted beyond the configured LLM endpoint |
+| T2 | API key leakage | 2 | Keys may come from env vars, `api_key_cmd`/`auth_token_cmd`, or the user config file (see Credential sources). Never logged, written to review output files, or transmitted beyond the configured LLM endpoint |
 | T3 | Path traversal via LLM-suggested file paths | 3 | `pathutil.WithinBase()` validates all file paths against the repository root, both before and after symlink resolution |
 | T4 | DNS rebinding against local viewer | 4 | Host-header allowlist rejects requests from non-loopback origins; configurable via `OCR_VIEWER_ALLOWED_HOSTS` |
 | T5 | Man-in-the-middle on API communication | 2 | Go's `net/http` enforces TLS 1.2+ with full certificate verification by default; `InsecureSkipVerify` is never set |
 | T6 | Malicious LLM response | 2 | JSON schema validation on response structure; line number bounds checking against actual diff ranges |
 | T7 | Dependency vulnerabilities | All | `govulncheck` runs in CI; Dependabot monitors for updates; `go.sum` provides integrity verification |
 
+### Credential sources
+
+API keys are never hardcoded in the binary. They may be supplied at runtime from:
+
+1. Environment variables (`OCR_LLM_TOKEN`, provider-specific variables such as `ANTHROPIC_API_KEY`). Env-only use is supported: a complete `OCR_LLM_*` or Claude Code `ANTHROPIC_*` environment needs no config file.
+2. A credential command (`api_key_cmd` / `auth_token_cmd`) whose stdout is the secret. The command string may be stored in config; the secret itself is not.
+3. Optional persistence in the user config file (`~/.opencodereview/config.json`) as `api_key` or `auth_token`. `ocr config set` and interactive provider setup write this file in plaintext; `saveConfig` (`cmd/opencodereview/provider_cmd.go`) creates new config files with mode `0600` and tightens existing config files to `0600` before persisting credentials.
+
+`ocr config set` masks `api_key`/`auth_token` when echoing the value. Sensitive request headers are redacted in raw logs. Keys are not written to review output files.
+
 ## Secure Design Principles
 
 The following analysis maps [Saltzer & Schroeder's design principles](https://ieeexplore.ieee.org/document/1451869) to the project's implementation.
@@ -73,11 +83,11 @@ The following analysis maps [Saltzer & Schroeder's design principles](https://ie
 | Principle | How Applied |
 |-----------|-------------|
 | **Least privilege** | `CGO_ENABLED=0` eliminates C library attack surface. The CLI requires no elevated permissions. No network listeners except the opt-in viewer. |
-| **Fail-safe defaults** | API keys must be explicitly provided via environment variables. The viewer binds to localhost by default; non-loopback hosts require explicit allowlisting. |
+| **Fail-safe defaults** | Credentials are not compiled in; they must be supplied via environment variables, `api_key_cmd`/`auth_token_cmd`, or the user config file. The viewer binds to localhost by default; non-loopback hosts require explicit allowlisting. |
 | **Complete mediation** | Every viewer HTTP request is checked against the host allowlist (`internal/viewer/hostguard.go`). Every file path from agent tools is validated against the repository root (`internal/tool/filereader.go:98`, `internal/pathutil/path.go`). |
 | **Economy of mechanism** | External process execution is limited to `git` with hardcoded subcommands — no shell invocation, no arbitrary command execution. |
 | **Open design** | Fully open-source (Apache-2.0). Security relies on TLS, not obscurity. |
-| **Separation of privilege** | API authentication (keys) is separated from configuration (files). The viewer's host guard is a distinct middleware layer. |
+| **Separation of privilege** | Credential material is never compiled into the binary; optional persistence is limited to the user config file, which `saveConfig` keeps at mode `0600` (pre-write chmod on existing files; new files created as `0600`). The viewer's host guard is a distinct middleware layer. |
 | **Least common mechanism** | Each review session writes to its own JSONL file. No shared state between sessions. |
 | **Psychological acceptability** | Security defaults (HTTPS, localhost binding, host allowlist) require no user configuration. Overrides (`OCR_VIEWER_ALLOWED_HOSTS`) are explicit and documented. |
 
@@ -91,7 +101,7 @@ The following maps [OWASP Top 10](https://owasp.org/www-project-top-ten/) and [C
 | **A03:2021 Injection** (CWE-79 Cross-site Scripting) | Viewer template output is HTML-escaped by `html/template`. Defense-in-depth: every viewer response carries a strict Content-Security-Policy (`default-src 'self'`, no `unsafe-inline`) plus `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy`, and `Permissions-Policy` (`internal/viewer/securityheaders.go`). | Mitigated |
 | **A01:2021 Broken Access Control** (CWE-22 Path Traversal) | Agent file-read tool validates paths with `pathutil.WithinBase()` before and after symlink resolution (`internal/tool/filereader.go:91-112`). | Mitigated |
 | **A02:2021 Cryptographic Failures** | All API communication uses HTTPS/TLS 1.2+. Go's default TLS configuration is used without weakening. `InsecureSkipVerify` is never set. | Mitigated |
-| **A07:2021 Auth Failures** (CWE-798 Hard-coded Credentials) | API keys are read exclusively from environment variables, never embedded in code or config files, never logged. | Mitigated |
+| **A07:2021 Auth Failures** (CWE-798 Hard-coded Credentials) | API keys are never hardcoded in source. They may be read from environment variables, `api_key_cmd`/`auth_token_cmd`, or the user config file (`~/.opencodereview/config.json`); `saveConfig` enforces mode `0600` on create and tightens existing config files to `0600` before persisting credentials. Never logged; `ocr config set` masks them on display. | Mitigated |
 | **A05:2021 Security Misconfiguration** | Secure defaults: localhost-only viewer, HTTPS-only API calls, `CGO_ENABLED=0`. The `go vet` and `govulncheck` tools run in CI. | Mitigated |
 | **A06:2021 Vulnerable Components** (CWE-1104) | Dependabot monitors Go modules and GitHub Actions. `govulncheck` runs on every push/PR. Dependencies are locked via `go.sum`. | Mitigated |
 | **A08:2021 Software Integrity** | Release binaries include SHA-256 checksums. Release tags are cryptographically signed (SSH). `CGO_ENABLED=0` produces static binaries with no external shared library dependencies. | Mitigated |

cmd/opencodereview/provider_cmd.go

@@ -425,12 +425,15 @@ func saveConfig(path string, cfg *Config) error {
 	if err != nil {
 		return fmt.Errorf("marshal config: %w", err)
 	}
+	// WriteFile applies 0o600 only when creating the file; existing files keep
+	// their prior permissions. Tighten an existing config before writing any
+	// credential material; a missing file will be created as 0600 below.
+	if err := os.Chmod(path, 0o600); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("chmod config: %w", err)
+	}
 	if err := os.WriteFile(path, data, 0o600); err != nil {
 		return fmt.Errorf("write config: %w", err)
 	}
-	if err := os.Chmod(path, 0o600); err != nil {
-		return fmt.Errorf("chmod config: %w", err)
-	}
 	return nil
 }
 

cmd/opencodereview/provider_cmd_test.go

@@ -102,6 +102,47 @@ func TestSaveConfig(t *testing.T) {
 	}
 }
 
+func TestSaveConfigEnforcesModeOnExistingFile(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Windows does not honor Unix file modes the same way")
+	}
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.json")
+	if err := os.WriteFile(path, []byte(`{"provider":"old"}`), 0o644); err != nil {
+		t.Fatalf("seed config: %v", err)
+	}
+	if err := os.Chmod(path, 0o644); err != nil {
+		t.Fatalf("set seed permissions: %v", err)
+	}
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat seed: %v", err)
+	}
+	if perm := info.Mode().Perm(); perm != 0o644 {
+		t.Fatalf("seed perm = %o, want 644", perm)
+	}
+
+	cfg := &Config{
+		Provider: "anthropic",
+		Model:    "claude-opus-4-6",
+		Providers: map[string]ProviderEntry{
+			"anthropic": {APIKey: "sk-secret"},
+		},
+	}
+	if err := saveConfig(path, cfg); err != nil {
+		t.Fatalf("saveConfig: %v", err)
+	}
+
+	info, err = os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat after save: %v", err)
+	}
+	if perm := info.Mode().Perm(); perm != 0o600 {
+		t.Errorf("perm after saveConfig = %o, want 600 (existing 0644 must be tightened)", perm)
+	}
+}
+
 func TestApplyProviderDeletions(t *testing.T) {
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")