diff --git a/CLAUDE.md b/CLAUDE.md index 794d2dd..a7b8f8d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,7 @@ TUI / HTTP client → Express (bytebell-server) → BullMQ (in-process) → Inge - **Local persistence**: `~/.bytebell/` (config, logs) - **LLM Provider**: OpenRouter (default) or local Ollama, selected via `Config.LlmProvider` - **Logging**: Winston (file + stdout) -- **Secret storage**: plaintext in `~/.bytebell/config.json` (mode `0600`). OS-keychain integration is not implemented. +- **Secret storage**: secrets (`openrouter_api_key`, `neo4j_password`) live in the OS keychain via `@napi-rs/keyring` (`@bb/config`). `bytebell set ` stores them there (its secret-key setters call `storeSecret`); `config.json` (mode `0600`) holds an empty string for keychain-backed secrets. On load, `loadConfig` cleanly migrates any plaintext secret it finds into the keychain and clears the field. A plaintext secret only persists when the OS has no keychain backend (e.g. headless Linux without Secret Service), and the server warns about it at boot. - **Package manager**: Bun (workspaces) --- @@ -156,7 +156,8 @@ The `~/.bytebell/` directory is the **single source of truth** for runtime confi config.json server_port, mongo_uri, neo4j_uri/user/password, redis_url, openrouter_api_key, openrouter_model, concurrency.github, log_level, log_retention_days - (mode 0600; openrouter_api_key stored in plaintext) + (mode 0600; secrets like openrouter_api_key / + neo4j_password live in the OS keychain, empty here) install_id UUID generated on first run (local-only, never transmitted) repos// cloned source trees for every indexed repo logs/ @@ -165,9 +166,9 @@ The `~/.bytebell/` directory is the **single source of truth** for runtime confi pid running server PID ``` -There is no OS-keychain integration; `openrouter_api_key` lives in plaintext in `config.json` (mode `0600`). +Secrets (`openrouter_api_key`, `neo4j_password`) are stored in the OS keychain (macOS Keychain / Linux Secret Service / Windows Credential Manager) via `@napi-rs/keyring`; `config.json` (mode `0600`) keeps an empty string for them. On load, `@bb/config` migrates any plaintext secret it finds into the keychain and clears the field — keychain is authoritative thereafter. A plaintext secret only persists on systems with no keychain backend and triggers a boot warning. -- `bytebell set ` is the only sanctioned write path to `config.json`. Manual edits work but are not advertised. +- `bytebell set ` is the sanctioned write path for `config.json`. For secret keys (`openrouter-api-key`, `neo4j-password`) it routes the value to the OS keychain instead of writing plaintext. Manual edits work but are not advertised. --- diff --git a/bun.lock b/bun.lock index a1974b2..52ad3f7 100644 --- a/bun.lock +++ b/bun.lock @@ -48,6 +48,7 @@ "version": "0.0.0", "dependencies": { "@bb/types": "workspace:*", + "@napi-rs/keyring": "^1.3.0", "zod": "^4.3.6", }, }, @@ -467,6 +468,32 @@ "@msgpackr-extract/msgpackr-extract-win32-x64": ["@msgpackr-extract/msgpackr-extract-win32-x64@3.0.3", "", { "os": "win32", "cpu": "x64" }, "sha512-x0fWaQtYp4E6sktbsdAqnehxDgEc/VwM7uLsRCYWaiGu0ykYdZPiS8zCWdnjHwyiumousxfBm4SO31eXqwEZhQ=="], + "@napi-rs/keyring": ["@napi-rs/keyring@1.3.0", "", { "optionalDependencies": { "@napi-rs/keyring-darwin-arm64": "1.3.0", "@napi-rs/keyring-darwin-x64": "1.3.0", "@napi-rs/keyring-freebsd-x64": "1.3.0", "@napi-rs/keyring-linux-arm-gnueabihf": "1.3.0", "@napi-rs/keyring-linux-arm64-gnu": "1.3.0", "@napi-rs/keyring-linux-arm64-musl": "1.3.0", "@napi-rs/keyring-linux-riscv64-gnu": "1.3.0", "@napi-rs/keyring-linux-x64-gnu": "1.3.0", "@napi-rs/keyring-linux-x64-musl": "1.3.0", "@napi-rs/keyring-win32-arm64-msvc": "1.3.0", "@napi-rs/keyring-win32-ia32-msvc": "1.3.0", "@napi-rs/keyring-win32-x64-msvc": "1.3.0" } }, "sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA=="], + + "@napi-rs/keyring-darwin-arm64": ["@napi-rs/keyring-darwin-arm64@1.3.0", "", { "os": "darwin", "cpu": "arm64" }, "sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ=="], + + "@napi-rs/keyring-darwin-x64": ["@napi-rs/keyring-darwin-x64@1.3.0", "", { "os": "darwin", "cpu": "x64" }, "sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw=="], + + "@napi-rs/keyring-freebsd-x64": ["@napi-rs/keyring-freebsd-x64@1.3.0", "", { "os": "freebsd", "cpu": "x64" }, "sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw=="], + + "@napi-rs/keyring-linux-arm-gnueabihf": ["@napi-rs/keyring-linux-arm-gnueabihf@1.3.0", "", { "os": "linux", "cpu": "arm" }, "sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ=="], + + "@napi-rs/keyring-linux-arm64-gnu": ["@napi-rs/keyring-linux-arm64-gnu@1.3.0", "", { "os": "linux", "cpu": "arm64" }, "sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg=="], + + "@napi-rs/keyring-linux-arm64-musl": ["@napi-rs/keyring-linux-arm64-musl@1.3.0", "", { "os": "linux", "cpu": "arm64" }, "sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw=="], + + "@napi-rs/keyring-linux-riscv64-gnu": ["@napi-rs/keyring-linux-riscv64-gnu@1.3.0", "", { "os": "linux", "cpu": "none" }, "sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ=="], + + "@napi-rs/keyring-linux-x64-gnu": ["@napi-rs/keyring-linux-x64-gnu@1.3.0", "", { "os": "linux", "cpu": "x64" }, "sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A=="], + + "@napi-rs/keyring-linux-x64-musl": ["@napi-rs/keyring-linux-x64-musl@1.3.0", "", { "os": "linux", "cpu": "x64" }, "sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw=="], + + "@napi-rs/keyring-win32-arm64-msvc": ["@napi-rs/keyring-win32-arm64-msvc@1.3.0", "", { "os": "win32", "cpu": "arm64" }, "sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw=="], + + "@napi-rs/keyring-win32-ia32-msvc": ["@napi-rs/keyring-win32-ia32-msvc@1.3.0", "", { "os": "win32", "cpu": "ia32" }, "sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ=="], + + "@napi-rs/keyring-win32-x64-msvc": ["@napi-rs/keyring-win32-x64-msvc@1.3.0", "", { "os": "win32", "cpu": "x64" }, "sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg=="], + "@pkgr/core": ["@pkgr/core@0.2.9", "", {}, "sha512-QNqXyfVS2wm9hweSYD2O7F0G06uurj9kZ96TRQE5Y9hU7+tgdZwIkbAKc5Ocy1HxEY2kuDQa6cQ1WRs/O5LFKA=="], "@russellthehippo/honker-node": ["@russellthehippo/honker-node@0.3.3", "", { "optionalDependencies": { "@russellthehippo/honker-node-darwin-arm64": "0.3.3", "@russellthehippo/honker-node-darwin-x64": "0.3.3", "@russellthehippo/honker-node-linux-arm64-gnu": "0.3.3", "@russellthehippo/honker-node-linux-x64-gnu": "0.3.3" } }, "sha512-MwC5gfn3o0FmOU429B5vp1ltDuPnGEtqLE5EfZGpi1VO3HQr7TbFZM08OepyjwNxYFY0tb0DXTOvD484EDFGCA=="], diff --git a/packages/cli/README.md b/packages/cli/README.md index a76a909..5879446 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -107,15 +107,36 @@ infra/docker/docker-compose.yml up -d` for **only the services the The package does **not** own: -- Any other subcommand (index, ls, clean, models, keys, cost, server, +- Any other subcommand (index, ls, clean, models, cost, server, mcp, update) — all deferred per the catalog below. - Live infra connection probes — the CLI cannot import `@bb/mongo` / `@bb/redis` per the tier rule. Format-only validation in v0; future `bytebell config doctor` will probe via a running server. - The Ink dashboard (`bytebell` no-args) — needs the server's HTTP API - activity feed. -- OpenRouter API key handling — own subcommand (`bytebell keys set`) - with `keytar` keychain backing. + +### Secrets — handled by `bytebell set`, stored in the OS keychain + +There is no separate secrets command. The secret-bearing keys +(`openrouter-api-key`, `neo4j-password`) are written by the **same `bytebell +set`** path as everything else; their `KEY_MAP` setters call +`@bb/config.storeSecret`, which stores the value in the OS keychain +(macOS Keychain / Linux Secret Service / Windows Credential Manager) and clears +any plaintext copy in `config.json`: + +- `bytebell set openrouter-api-key ` / `bytebell set neo4j-password ` — + store in the keychain. If no keychain backend is available (e.g. headless + Linux without Secret Service, CI), it falls back to a plaintext write in + `config.json` and prints a warning. +- `bytebell set` (no args) / `bytebell setup` — the interactive forms use the + same setters (masked input), so secrets entered there also land in the keychain. + +Reads are transparent: on first load `@bb/config` performs a **clean migration** +— any plaintext secret found in `config.json` is moved into the keychain and the +plaintext field is cleared on disk. The keychain is the source of truth from +then on; `getConfigValue` resolves it without callers knowing. The only case a +plaintext secret persists is on a system with no keychain backend (headless +Linux without Secret Service, CI, etc.), and the server warns about it at boot. ## Public exports @@ -201,7 +222,6 @@ will touch when implemented. Only the **bolded** entries ship in v0. | `bytebell` (first-run auto-launch of setup form) | If `isConfigComplete()` returns false, redirect to `bytebell set` form ([docs/arch.md:170](../../docs/arch.md#L170)) | After dashboard lands | | `bytebell models set ` | Validate model via OpenRouter API + write `openrouter_model` | After OpenRouter helper | | `bytebell models ls` | Curated 5-10 models, on-the-fly OpenRouter pricing | Same | -| `bytebell keys set` | Interactive masked prompt → `keytar` keychain → write key | After `keytar` integration | | `bytebell cost` | Read `~/.bytebell/cost-ledger.sqlite` via `bun:sqlite`, render breakdowns | After cost ledger lands in `@bb/llm` | | `bytebell server stop \| status \| logs` | Kill / inspect `bytebell-server`, tail server logs (start is shipped — see above) | After `@bb/server` health surface | | `bytebell mcp` | Print MCP endpoint URL + sample MCP-client config | After dashboard pane | @@ -223,7 +243,6 @@ will touch when implemented. Only the **bolded** entries ship in v0. | `bytebell` (first-run auto-launch of setup form) | If `isConfigComplete()` returns false, redirect to `bytebell set` form ([docs/arch.md:170](../../docs/arch.md#L170)) | After dashboard lands | | `bytebell models set ` | Validate model via OpenRouter API + write `openrouter_model` | After OpenRouter helper | | `bytebell models ls` | Curated 5-10 models, on-the-fly OpenRouter pricing | Same | -| `bytebell keys set` | Interactive masked prompt → `keytar` keychain → write key | After `keytar` integration | | `bytebell cost` | Read `~/.bytebell/cost-ledger.sqlite` via `bun:sqlite`, render breakdowns | After cost ledger lands in `@bb/llm` | | `bytebell server stop \| status \| logs` | Kill / inspect `bytebell-server`, tail server logs (start is shipped — see above) | After `@bb/server` health surface | | **`bytebell mcp install`** | **Detect installed coding tools (Claude Code, Cursor, Claude Desktop, Windsurf, VS Code) and merge a `bytebell` MCP server entry into each one's config, pointing at `http://127.0.0.1:/mcp`.** | **Shipped** | @@ -256,7 +275,6 @@ will touch when implemented. Only the **bolded** entries ship in v0. defaults - Live connection probes inside the setup form - First-run auto-launch of setup form (needs the dashboard pane first) -- OpenRouter API key in the setup form (separate `bytebell keys set`) - Tests — workspace has no test infra yet - Color theming via `kleur` / `picocolors` — manual ANSI for now - Distinct exit codes per failure mode (today: `1` = typed/handled error, @@ -288,9 +306,10 @@ Adding a new subcommand: 3. Wire into `src/index.ts`: `program.addCommand(buildCommand())`. 4. If the command speaks to `bytebell-server`: HTTP only (e.g. `fetch` to `http://localhost:`). Never import `@bb/server`. -5. If the command needs OS primitives (`keytar`, `bun:sqlite`, - `child_process`): add the dep to `package.json`, but never import a - domain / strategy / infra-non-config workspace package. +5. If the command needs OS primitives (`bun:sqlite`, `child_process`) or a + secret store, prefer reusing `@bb/config` (which owns OS-keychain access via + `@napi-rs/keyring`); otherwise add the dep to `package.json`, but never + import a domain / strategy / infra-non-config workspace package. 6. Update _Public exports_ / _Out of scope_ in this file and the table above — move the row from "deferred" to "shipped". diff --git a/packages/cli/src/SetCommand.ts b/packages/cli/src/SetCommand.ts index d66bf96..8f9b0d3 100644 --- a/packages/cli/src/SetCommand.ts +++ b/packages/cli/src/SetCommand.ts @@ -1,10 +1,11 @@ import { Command } from "commander"; import React from "react"; import { render } from "ink"; -import { HINTS, getConfigValue } from "@bb/config"; +import { SecretSource } from "@bb/types"; +import { HINTS, getConfigValue, getSecretSource, isSecretKey } from "@bb/config"; import { KEY_MAP, validKeysList } from "./keyMap.ts"; import { SetupForm } from "./SetupForm.tsx"; -import { error, list, success } from "./output.ts"; +import { error, list, success, warn } from "./output.ts"; export function buildSetCommand(): Command { const cmd = new Command("set"); @@ -58,7 +59,17 @@ async function runSet(key?: string, value?: string): Promise { try { mappedKey.setter(value); - success(`Set ${key} to ${mappedKey.redact ? "" : value}`); + // Secrets route to the OS keychain via their setter; report where it landed. + if (isSecretKey(mappedKey.configKey)) { + if (getSecretSource(mappedKey.configKey) === SecretSource.Keychain) { + success(`Set ${key} (stored in OS keychain).`); + } else { + success(`Set ${key} (plaintext).`); + warn(`No OS keychain backend available — ${key} was written to config.json in plaintext.`); + } + } else { + success(`Set ${key} to ${mappedKey.redact ? "" : value}`); + } } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); error(`Failed to set ${key}: ${message}`); diff --git a/packages/cli/src/SetupCommand.ts b/packages/cli/src/SetupCommand.ts index 8ad6942..75f5ba2 100644 --- a/packages/cli/src/SetupCommand.ts +++ b/packages/cli/src/SetupCommand.ts @@ -95,6 +95,7 @@ function applyConfig(result: InstallWizardResult): void { providerEntry.setter(result.provider); if (result.provider === "openrouter") { + // keyEntry.setter routes the API key to the OS keychain (see keyMap.ts). const keyEntry = KEY_MAP["openrouter-api-key"]; const modelEntry = KEY_MAP["openrouter-model"]; if (keyEntry === undefined) { diff --git a/packages/cli/src/SetupForm.tsx b/packages/cli/src/SetupForm.tsx index 0350856..622aa6e 100644 --- a/packages/cli/src/SetupForm.tsx +++ b/packages/cli/src/SetupForm.tsx @@ -135,6 +135,7 @@ export function SetupForm({ onDone }: SetupFormProps): ReactElement { if (entry === undefined) { throw new Error(`No KEY_MAP entry for "${row.cliKey}"`); } + // Secret keys route to the OS keychain via their KEY_MAP setter. entry.setter(values[row.id] ?? ""); } exit(); diff --git a/packages/cli/src/bootConfig.ts b/packages/cli/src/bootConfig.ts index df68b8e..8145c91 100644 --- a/packages/cli/src/bootConfig.ts +++ b/packages/cli/src/bootConfig.ts @@ -86,6 +86,8 @@ export function applyInfraDefaults(): ApplyDefaultsResult { continue; } const value = entry.computeDefault(); + // Secret keys (e.g. an auto-generated Neo4j password) route to the OS keychain + // via their KEY_MAP setter, so a routine boot leaves no plaintext secret. const setter = KEY_MAP[entry.cliKey]; if (setter === undefined) { throw new Error(`internal: KEY_MAP entry "${entry.cliKey}" missing`); @@ -112,8 +114,12 @@ const CONFIG_HINT_KEYS: Partial> = { }; export function checkPreflight(): PreflightResult { - const provider = getConfigValue(Config.LlmProvider); - const required = requiredKeysFor(provider); + const required = requiredKeysFor( + getConfigValue(Config.LlmProvider), + getConfigValue(Config.DbProvider), + getConfigValue(Config.GraphProvider), + getConfigValue(Config.QueueProvider), + ); const missing: PreflightResult["missing"] = []; for (const configKey of required) { const value = getConfigValue(configKey); diff --git a/packages/cli/src/keyMap.ts b/packages/cli/src/keyMap.ts index 5481c9f..f5461be 100644 --- a/packages/cli/src/keyMap.ts +++ b/packages/cli/src/keyMap.ts @@ -1,4 +1,4 @@ -import { LLM_PROVIDERS, LOG_LEVELS, setConfigValue, type LlmProvider, type LogLevel } from "@bb/config"; +import { LLM_PROVIDERS, LOG_LEVELS, setConfigValue, storeSecret, type LlmProvider, type LogLevel } from "@bb/config"; import { Config, DbProviderType, GraphProviderType, IngestionStrategyType, QueueProviderType } from "@bb/types"; type Setter = (raw: string) => void; @@ -90,7 +90,8 @@ export const KEY_MAP: Record = { "neo4j-password": { configKey: Config.Neo4jPassword, redact: true, - setter: (s) => setConfigValue(Config.Neo4jPassword, s), + // Secret: store in the OS keychain (plaintext fallback only without a backend). + setter: (s) => storeSecret(Config.Neo4jPassword, s), }, redis: { configKey: Config.RedisUrl, @@ -120,7 +121,8 @@ export const KEY_MAP: Record = { "openrouter-api-key": { configKey: Config.OpenrouterApiKey, redact: true, - setter: (s) => setConfigValue(Config.OpenrouterApiKey, s), + // Secret: store in the OS keychain (plaintext fallback only without a backend). + setter: (s) => storeSecret(Config.OpenrouterApiKey, s), }, "openrouter-model": { configKey: Config.OpenrouterModel, diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 7956dbc..79696b1 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -17,6 +17,11 @@ export function error(line: string, hint?: string): void { } } +export function warn(line: string): void { + const YELLOW = ""; + process.stderr.write(`${paint(YELLOW, `⚠ ${line}`, process.stderr)}\n`); +} + export function list(label: string, items: readonly string[]): void { process.stderr.write(`${label}\n`); for (const item of items) { diff --git a/packages/config/README.md b/packages/config/README.md index 57ba333..9e8bec1 100644 --- a/packages/config/README.md +++ b/packages/config/README.md @@ -17,9 +17,44 @@ Single source of truth for runtime settings stored in - Memoized in-process load - Atomic, validating writes via `setConfigValue` - Required-field completeness check with CLI-hint strings +- OS-keychain storage for secrets (`SECRET_KEYS`), with a transparent + read-overlay and a warned plaintext fallback This package does **not** read from `process.env` and never will. +### Secrets (OS keychain) + +`SECRET_KEYS` = { `openrouter_api_key`, `neo4j_password` } — the only persisted +secrets. Their live value is stored in the OS keychain (macOS Keychain, Linux +Secret Service, Windows Credential Manager) under service `"bytebell"`, account += the `Config` enum value. `config.json` holds an **empty string** for a +keychain-backed secret. + +Read path: `loadConfig()` performs a **clean migration**. Per secret: + +1. If `config.json` has a non-empty plaintext value AND the OS keychain is + available → move the value into the keychain, clear the plaintext field on + disk (atomic write), and use the keychain value going forward. Migration is + one-shot — once cleared, subsequent loads find the field empty. +2. Else if the field is empty → overlay the keychain value (if any). +3. Else (plaintext present **and** keychain unavailable, e.g. headless Linux + without Secret Service) → leave plaintext as-is; the server boot warning + flags it. This is the only case where a plaintext secret persists. + +The keychain is the source of truth. `getConfigValue`, `isConfigComplete`, and +every downstream reader resolve the secret transparently — no caller changes. + +Public surface: `getSecret` / `setSecret` / `deleteSecret` / +`isKeychainAvailable` / `isSecretKey` / `KeychainUnavailableError` (keychain +primitives), `storeSecret` (composes keychain with the config writer), +and `getSecretSource(key) → SecretSource` (`Plaintext` | `Keychain` | `Missing`, +the `@bb/types` enum — for boot/CLI warnings). `storeSecret` writes to the keychain and clears any +plaintext copy; if no keychain backend exists it writes plaintext and returns +`"plaintext"` so the caller can warn. The `@bb/cli` `set` command's secret-key +setters call `storeSecret`, so `bytebell set openrouter-api-key …` +stores to the keychain with no separate command. Seeded/test configs never touch +the keychain (`seedConfig` bypasses the overlay). + `setBytebellHomeResolver` registers an override function invoked on every `getBytebellHome()` call (no caching). The resolver returns the home directory to use for the current invocation, or `null` to fall through to the @@ -64,11 +99,11 @@ Anything not in this list is internal — do not import from subpaths. - `~/.bytebell/` directory creation (mode `0700`) - `~/.bytebell/config.json` content + atomic writes (mode `0600`) - Default values for every config key +- OS-keychain entries for `SECRET_KEYS` (service `"bytebell"`) This package does **not** own: - `~/.bytebell/install_id` — assigned to a later package -- `~/.bytebell/keys.json` — out of scope for v0 - `~/.bytebell/logs/` — `@bb/logger` - `~/.bytebell/cost-ledger.sqlite` — `@bb/llm` @@ -86,14 +121,18 @@ This package does **not** own: `isConfigComplete()` rather than thrown by the loader. 5. **Atomic writes.** Every write is `tmp → fsync → rename`. A crash mid-write leaves the previous `config.json` intact. -6. **File mode `0600`.** `config.json` contains the OpenRouter API key in - plaintext (v0 decision); the file is owner-read/write only. +6. **File mode `0600`.** `config.json` is owner-read/write only. Secrets in + `SECRET_KEYS` live in the OS keychain, not `config.json`; `loadConfig()` + migrates any plaintext secret it finds into the keychain. A plaintext + secret only persists on systems with no keychain backend, and triggers a + boot warning. 7. **No public file paths besides home + config.** Other files under `~/.bytebell/` are not addressed by this package. ## External dependencies - `zod` — runtime schema + parsing +- `@napi-rs/keyring` — synchronous OS-keychain access for `SECRET_KEYS` - Node built-ins — `node:fs`, `node:os`, `node:path` No HTTP, no DB, no logger. This package boots before everything else. @@ -101,7 +140,8 @@ No HTTP, no DB, no logger. This package boots before everything else. ## What is intentionally out of scope - `install_id` generation/reading (deferred ownership) -- OS keychain / `keys.json` / encrypted secrets +- Encrypting non-secret connection URIs that may embed credentials + (`mongo_uri`, `neo4j_uri`, `redis_url`) — not in `SECRET_KEYS` - Logger initialization - A `bytebell set` CLI command (lives in `@bb/cli`; uses `setConfigValue` primitive) @@ -113,9 +153,9 @@ To add a new config key: 1. Add a new `Config` enum entry in `src/schema.ts`. 2. Add the field to `configSchema` with a `.default(...)`. 3. Add a `ConfigValueMap` entry mapping the enum to its TS type. -4. If required, add the enum to `REQUIRED_KEYS` (infra-always) or to - `PROVIDER_REQUIRED_KEYS[]` (provider-specific — driven by - `Config.LlmProvider` at completeness-check time). +4. If the key is required, add it to the relevant branch of `requiredKeysFor` + (provider-conditional — both the CLI `bytebell boot` preflight and the server's + startup check call this with all four providers). 5. Add a hint string to `HINTS`. 6. Add cases to `readField` and `writeField`. 7. Update this `README.md` if the new key changes invariants or ownership. diff --git a/packages/config/package.json b/packages/config/package.json index d31ebe4..5c8e92b 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -13,6 +13,7 @@ }, "dependencies": { "@bb/types": "workspace:*", + "@napi-rs/keyring": "^1.3.0", "zod": "^4.3.6" } } diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 9d2e6a0..9ffb652 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -1,11 +1,31 @@ export { LOG_LEVELS, LLM_PROVIDERS, HINTS, requiredKeysFor } from "./schema.ts"; export type { BytebellConfig, ConfigValue, ConfigValueMap, LogLevel, LlmProvider } from "./schema.ts"; -export { loadConfig, getConfigValue, isConfigComplete, seedConfig, __isSeeded, __resetSeedForTests } from "./loader.ts"; +export { + loadConfig, + getConfigValue, + isConfigComplete, + getSecretSource, + seedConfig, + __isSeeded, + __resetSeedForTests, +} from "./loader.ts"; export type { ConfigCompletenessResult } from "./loader.ts"; export { setConfigValue, ensureBytebellHome, ConfigSeededError } from "./writer.ts"; +export { + SECRET_KEYS, + isSecretKey, + getSecret, + setSecret, + deleteSecret, + isKeychainAvailable, + KeychainUnavailableError, +} from "./keychain.ts"; +export { storeSecret } from "./secrets.ts"; +export type { SecretWriteResult } from "./secrets.ts"; + export { getBytebellHome, getConfigPath, diff --git a/packages/config/src/keychain.ts b/packages/config/src/keychain.ts new file mode 100644 index 0000000..0aeba13 --- /dev/null +++ b/packages/config/src/keychain.ts @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: AGPL-3.0-only WITH non-commercial-clause +import { Entry } from "@napi-rs/keyring"; +import { Config } from "@bb/types"; +import { __notifyConfigChanged } from "./paths.ts"; + +/** + * OS-keychain storage for persisted secrets. + * + * Secrets live in the platform credential store (macOS Keychain, Linux Secret + * Service, Windows Credential Manager) instead of plaintext in `config.json`. + * The keychain account is the `Config` enum value itself (e.g. + * `"openrouter_api_key"`), which equals the JSON field name in `config.json`. + * + * Every operation is wrapped: an absent or locked backend degrades gracefully + * (reads return `null`, deletes are best-effort) rather than throwing at the + * call site — only `setSecret` surfaces a typed error so callers can fall back + * to a warned plaintext write. + */ + +const SERVICE = "bytebell"; + +/** Config keys whose value is a secret and is stored in the OS keychain. */ +export const SECRET_KEYS: ReadonlySet = new Set([Config.OpenrouterApiKey, Config.Neo4jPassword]); + +/** Thrown by {@link setSecret} when no keychain backend can store the value. */ +export class KeychainUnavailableError extends Error { + constructor(cause?: unknown) { + super("OS keychain is unavailable on this machine"); + this.name = "KeychainUnavailableError"; + if (cause !== undefined) { + this.cause = cause; + } + } +} + +/** True when `key` is a secret-bearing config key (kept out of plaintext writes). */ +export function isSecretKey(key: Config): boolean { + return SECRET_KEYS.has(key); +} + +function entryFor(key: Config): Entry { + return new Entry(SERVICE, key); +} + +/** Read a secret from the keychain. Returns `null` if absent or the backend is unavailable. */ +export function getSecret(key: Config): string | null { + try { + return entryFor(key).getPassword(); + } catch { + return null; + } +} + +/** Store a secret in the keychain. Throws {@link KeychainUnavailableError} if no backend can store it. */ +export function setSecret(key: Config, value: string): void { + try { + entryFor(key).setPassword(value); + } catch (cause: unknown) { + throw new KeychainUnavailableError(cause); + } + __notifyConfigChanged(); +} + +/** Delete a secret from the keychain. Best-effort; never throws. */ +export function deleteSecret(key: Config): void { + try { + entryFor(key).deletePassword(); + } catch { + // already absent or backend unavailable — nothing to do + } + __notifyConfigChanged(); +} + +/** + * Probe whether the OS keychain backend is usable. Read-only (no write, so no + * GUI prompt on macOS): a missing probe entry returns `null` when the backend + * is present and throws when it is absent. + */ +export function isKeychainAvailable(): boolean { + try { + new Entry(SERVICE, "__availability_probe__").getPassword(); + return true; + } catch { + return false; + } +} diff --git a/packages/config/src/loader.ts b/packages/config/src/loader.ts index e4a4c02..f13c7c1 100644 --- a/packages/config/src/loader.ts +++ b/packages/config/src/loader.ts @@ -6,10 +6,13 @@ import { type ConfigValue, HINTS, readField, + writeField, requiredKeysFor, } from "./schema.ts"; +import { SecretSource } from "@bb/types"; import { __registerCacheInvalidator, getConfigPath, resolveUnderHome } from "./paths.ts"; -import { ensureBytebellHome } from "./writer.ts"; +import { ensureBytebellHome, setConfigValue } from "./writer.ts"; +import { getSecret, setSecret, SECRET_KEYS, KeychainUnavailableError } from "./keychain.ts"; let cached: BytebellConfig | null = null; let seeded = false; @@ -42,11 +45,77 @@ export function loadConfig(): BytebellConfig { } ensureBytebellHome(); const raw = fs.readFileSync(getConfigPath(), "utf8"); - const parsed: unknown = JSON.parse(raw); - cached = configSchema.parse(parsed); + const parsed = configSchema.parse(JSON.parse(raw)); + const { cfg, migrated } = resolveSecrets(parsed); + // Persist cleared plaintext fields for migrated secrets. Each setConfigValue + // call invalidates `cached` via __notifyConfigChanged, so we set the cache + // *after* the persistence loop. + for (const key of migrated) { + setConfigValue(key, "" as ConfigValue); + } + cached = cfg; return cached; } +/** + * Resolve secrets on load: migrate any plaintext secret into the OS keychain + * (clearing the plaintext field), and overlay keychain values into empty + * fields. The keychain is the source of truth; plaintext only remains as the + * stored value when no keychain backend is available on this machine. + */ +function resolveSecrets(cfg: BytebellConfig): { cfg: BytebellConfig; migrated: Config[] } { + let next = cfg; + const migrated: Config[] = []; + for (const key of SECRET_KEYS) { + const current = readField(next, key); + if (typeof current === "string" && current.length > 0) { + try { + setSecret(key, current); + // In-memory cfg keeps the value (it now lives in the keychain). The + // plaintext field on disk is cleared by loadConfig's persist loop. + migrated.push(key); + } catch (err: unknown) { + if (!(err instanceof KeychainUnavailableError)) { + throw err; + } + // No keychain backend on this system — leave plaintext as the stored + // value. The server boot warning flags it. + } + continue; + } + const secret = getSecret(key); + if (secret !== null && secret.length > 0) { + next = writeField(next, key, secret as ConfigValue); + } + } + return { cfg: next, migrated }; +} + +/** + * Where a secret's live value comes from — see {@link SecretSource}. After + * `loadConfig` runs, `Plaintext` can only mean the OS keychain is unavailable + * on this machine (so migration could not happen); otherwise the value lives + * in the keychain or is unset. + */ +export function getSecretSource(key: Config): SecretSource { + if (readPlaintextSecret(key).length > 0) { + return SecretSource.Plaintext; + } + const secret = getSecret(key); + return secret !== null && secret.length > 0 ? SecretSource.Keychain : SecretSource.Missing; +} + +/** Read a secret's raw plaintext value straight from `config.json`, bypassing the keychain overlay. */ +function readPlaintextSecret(key: Config): string { + try { + const parsed = configSchema.parse(JSON.parse(fs.readFileSync(getConfigPath(), "utf8"))); + const value = readField(parsed, key); + return typeof value === "string" ? value : ""; + } catch { + return ""; + } +} + /** Path-valued keys whose stored value is resolved to an absolute path on read. */ const PATH_KEYS: ReadonlySet = new Set([Config.SqlitePath, Config.LadybugPath, Config.QueueDbPath]); @@ -63,7 +132,7 @@ export type ConfigCompletenessResult = { ok: true } | { ok: false; missing: Conf export function isConfigComplete(): ConfigCompletenessResult { const cfg = loadConfig(); const missing: Config[] = []; - for (const key of requiredKeysFor(cfg.llm_provider)) { + for (const key of requiredKeysFor(cfg.llm_provider, cfg.db_provider, cfg.graph_provider, cfg.queue_provider)) { const value = readField(cfg, key); if (typeof value === "string" && value.length === 0) { missing.push(key); diff --git a/packages/config/src/schema.ts b/packages/config/src/schema.ts index 904a39a..8409848 100644 --- a/packages/config/src/schema.ts +++ b/packages/config/src/schema.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { Config } from "@bb/types"; +import { Config, DbProviderType, GraphProviderType, QueueProviderType } from "@bb/types"; export { Config }; @@ -130,21 +130,45 @@ export type ConfigValueMap = { export type ConfigValue = ConfigValueMap[K]; -export const REQUIRED_KEYS: readonly Config[] = [ - Config.MongoUri, - Config.Neo4jUri, - Config.Neo4jUser, - Config.Neo4jPassword, - Config.RedisUrl, -]; - -const PROVIDER_REQUIRED_KEYS: Readonly> = { +const LLM_PROVIDER_REQUIRED_KEYS: Readonly> = { openrouter: [Config.OpenrouterApiKey], ollama: [Config.OllamaUrl, Config.OllamaModel], }; -export function requiredKeysFor(provider: LlmProvider): readonly Config[] { - return [...REQUIRED_KEYS, ...PROVIDER_REQUIRED_KEYS[provider]]; +/** + * The set of config keys that must be non-empty for the current provider + * selection. Embedded providers (sqlite / ladybug / honker) need their on-disk + * paths instead of Docker connection URIs, so the list is provider-conditional + * — there are no unconditionally required infra keys. The server's required + * check and the CLI's `bytebell boot` preflight both call this so the rule + * lives in exactly one place. + */ +export function requiredKeysFor( + llmProvider: LlmProvider, + dbProvider: string, + graphProvider: string, + queueProvider: string, +): readonly Config[] { + const keys: Config[] = []; + if (dbProvider === DbProviderType.Mongo) { + keys.push(Config.MongoUri); + } else if (dbProvider === DbProviderType.Sqlite) { + keys.push(Config.SqlitePath); + } + if (graphProvider === GraphProviderType.Neo4j) { + keys.push(Config.Neo4jUri, Config.Neo4jUser, Config.Neo4jPassword); + } else if (graphProvider === GraphProviderType.Ladybug) { + keys.push(Config.LadybugPath); + } + if (queueProvider === QueueProviderType.Bullmq) { + keys.push(Config.RedisUrl); + } else if (queueProvider === QueueProviderType.Honker) { + keys.push(Config.QueueDbPath); + } + for (const k of LLM_PROVIDER_REQUIRED_KEYS[llmProvider]) { + keys.push(k); + } + return keys; } export const HINTS: Readonly> = { @@ -154,7 +178,7 @@ export const HINTS: Readonly> = { [Config.Neo4jUser]: "bytebell set neo4j-user ", [Config.Neo4jPassword]: "bytebell set neo4j-password ", [Config.RedisUrl]: "bytebell set redis ", - [Config.OpenrouterApiKey]: "bytebell keys set", + [Config.OpenrouterApiKey]: "bytebell set openrouter-api-key ", [Config.OpenrouterModel]: "bytebell models set ", [Config.OpenrouterFallbackModel1]: "bytebell set openrouter-fallback-model-1 ", [Config.OpenrouterFallbackModel2]: "bytebell set openrouter-fallback-model-2 ", diff --git a/packages/config/src/secrets.ts b/packages/config/src/secrets.ts new file mode 100644 index 0000000..9f2042e --- /dev/null +++ b/packages/config/src/secrets.ts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: AGPL-3.0-only WITH non-commercial-clause +import { Config } from "@bb/types"; +import type { ConfigValue } from "./schema.ts"; +import { setSecret, KeychainUnavailableError } from "./keychain.ts"; +import { setConfigValue } from "./writer.ts"; + +/** + * Secret write helpers that compose keychain storage with the plaintext config + * writer. Kept separate from `keychain.ts` so the keychain module stays free of + * any dependency on the config writer/loader (which would form an import cycle). + */ + +export type SecretWriteResult = "keychain" | "plaintext"; + +/** + * Store a secret in the OS keychain and clear any stale plaintext copy so the + * keychain value is authoritative. If no keychain backend is available, fall + * back to a plaintext write in `config.json` and report `"plaintext"` so the + * caller can warn the user. + */ +export function storeSecret(key: Config, value: string): SecretWriteResult { + try { + setSecret(key, value); + setConfigValue(key, "" as ConfigValue); + return "keychain"; + } catch (err: unknown) { + if (err instanceof KeychainUnavailableError) { + setConfigValue(key, value as ConfigValue); + return "plaintext"; + } + throw err; + } +} diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 6bb4bb8..f73390f 100755 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -2,8 +2,8 @@ import { writeFile } from "node:fs/promises"; import path from "node:path"; import express from "express"; -import { Config, DbProviderType, GraphProviderType, QueueProviderType, type Config as ConfigEnum } from "@bb/types"; -import { getBytebellHome, getConfigValue, HINTS } from "@bb/config"; +import { Config, SecretSource } from "@bb/types"; +import { getBytebellHome, getConfigValue, getSecretSource, HINTS, requiredKeysFor, SECRET_KEYS } from "@bb/config"; import { connectDb } from "@bb/db"; import { connectGraph, indexesGraph } from "@bb/graph-db"; import { connectQueue, resumeOrphans } from "@bb/queue"; @@ -25,56 +25,15 @@ import { registerRoutes } from "./routes.ts"; import { installShutdownHandlers } from "./shutdown.ts"; import { reconcileLegacyLayout } from "./legacyLayout.ts"; -const REQUIRED: ConfigEnum[] = [ - Config.MongoUri, - Config.RedisUrl, - Config.Neo4jUri, - Config.Neo4jUser, - Config.Neo4jPassword, - Config.OpenrouterApiKey, -]; - function checkRequiredConfig(): void { + const required = requiredKeysFor( + getConfigValue(Config.LlmProvider), + getConfigValue(Config.DbProvider), + getConfigValue(Config.GraphProvider), + getConfigValue(Config.QueueProvider), + ); const missing: string[] = []; const hints: string[] = []; - const dbProvider = getConfigValue(Config.DbProvider); - const graphProvider = getConfigValue(Config.GraphProvider); - const queueProvider = getConfigValue(Config.QueueProvider); - - const required = [...REQUIRED]; - const remove = (key: ConfigEnum): void => { - const idx = required.indexOf(key); - if (idx !== -1) { - required.splice(idx, 1); - } - }; - - if (dbProvider !== DbProviderType.Mongo) { - remove(Config.MongoUri); - } - if (graphProvider !== GraphProviderType.Neo4j) { - // Embedded graph (ladybug) needs no Neo4j connection details. - remove(Config.Neo4jUri); - remove(Config.Neo4jUser); - remove(Config.Neo4jPassword); - } - if (queueProvider !== QueueProviderType.Bullmq) { - remove(Config.RedisUrl); - } - - // Embedded mode keeps its stores on disk — refuse to boot if any path the - // active embedded provider depends on is unset, instead of failing later - // with a cryptic file lock / IO error. - if (dbProvider === DbProviderType.Sqlite) { - required.push(Config.SqlitePath); - } - if (graphProvider === GraphProviderType.Ladybug) { - required.push(Config.LadybugPath); - } - if (queueProvider === QueueProviderType.Honker) { - required.push(Config.QueueDbPath); - } - for (const key of required) { const value = getConfigValue(key); if (typeof value === "string" && value.length === 0) { @@ -87,8 +46,24 @@ function checkRequiredConfig(): void { } } +/** + * After loadConfig's clean migration, a plaintext secret means the OS keychain + * is unavailable on this machine (e.g. headless Linux without Secret Service / + * D-Bus) and we could not migrate it. Warn, but don't fail — the file is 0600. + */ +function warnPlaintextSecrets(): void { + for (const key of SECRET_KEYS) { + if (getSecretSource(key) === SecretSource.Plaintext) { + process.stderr.write( + `⚠ ${key} is stored in plaintext in config.json — no OS keychain backend is available on this system to migrate it.\n`, + ); + } + } +} + async function main(): Promise { checkRequiredConfig(); + warnPlaintextSecrets(); const dbProvider = getConfigValue(Config.DbProvider); await connectDb(dbProvider); // Self-heal the legacy on-disk layout: migrate what has a DB record, drop diff --git a/packages/types/src/config.ts b/packages/types/src/config.ts index b869ad4..69012b1 100644 --- a/packages/types/src/config.ts +++ b/packages/types/src/config.ts @@ -64,6 +64,17 @@ export enum QueueProviderType { Bullmq = "bullmq", Honker = "honker", } + +/** + * Where a persisted secret's live value comes from. `Plaintext` means it is + * sitting in `config.json` (a security smell — boot warns); `Keychain` means it + * lives in the OS credential store; `Missing` means it is not set anywhere. + */ +export enum SecretSource { + Plaintext = "plaintext", + Keychain = "keychain", + Missing = "missing", +} /** * Active ingestion strategy. `flat-folder` is the historic default that * produces `:Repo` + `:Folder` summaries via per-folder LLM passes. diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d2e464a..84b8644 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,4 @@ -export { Config, DbProviderType, GraphProviderType, QueueProviderType } from "./config.ts"; +export { Config, DbProviderType, GraphProviderType, QueueProviderType, SecretSource } from "./config.ts"; export { JobType, JobPriority } from "./job.ts"; export type { GithubIndexPayload,