From da23b46bb4eb58deaa84e88c2bbb9944fe872bd7 Mon Sep 17 00:00:00 2001 From: lovanshu garg Date: Fri, 5 Jun 2026 15:17:29 +0530 Subject: [PATCH 1/4] feat(keychain): storage for openrouter api key --- CLAUDE.md | 9 ++- TESTING.md | 127 +++++++++++++++++++++++++++++++ bun.lock | 27 +++++++ packages/cli/README.md | 34 ++++++--- packages/cli/src/SetCommand.ts | 17 ++++- packages/cli/src/SetupCommand.ts | 1 + packages/cli/src/SetupForm.tsx | 1 + packages/cli/src/bootConfig.ts | 2 + packages/cli/src/keyMap.ts | 8 +- packages/cli/src/output.ts | 5 ++ packages/config/README.md | 39 +++++++++- packages/config/package.json | 1 + packages/config/src/index.ts | 22 +++++- packages/config/src/keychain.ts | 86 +++++++++++++++++++++ packages/config/src/loader.ts | 48 +++++++++++- packages/config/src/schema.ts | 2 +- packages/config/src/secrets.ts | 33 ++++++++ packages/server/src/index.ts | 27 ++++++- packages/types/src/config.ts | 11 +++ packages/types/src/index.ts | 2 +- 20 files changed, 473 insertions(+), 29 deletions(-) create mode 100644 TESTING.md create mode 100644 packages/config/src/keychain.ts create mode 100644 packages/config/src/secrets.ts diff --git a/CLAUDE.md b/CLAUDE.md index 794d2dd..fce504a 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. A plaintext value remains only as a legacy / keychain-less fallback and is flagged 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. A non-empty plaintext secret is a legacy / keychain-less fallback only 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/TESTING.md b/TESTING.md new file mode 100644 index 0000000..df46b9a --- /dev/null +++ b/TESTING.md @@ -0,0 +1,127 @@ +# Testing Checklist — `default` branch vs `main` + +This branch merges the **embedded prerelease**: 176 files, ~8,200 insertions. The +headline change is that **the default infrastructure flips from Docker +(Mongo/Neo4j/Redis) to embedded (SQLite + Ladybug + Honker)**, plus a one-command +install, a `setup` wizard, and automatic MCP client configuration. + +Use this as a manual QA pass before merging. Items are ordered by priority — the +new defaults are what every new user hits first, so test those before anything +else. + +--- + +## ⚠️ Highest-priority items (the new defaults) + +- [ ] **Fresh install with zero Docker.** On a machine with no Mongo/Neo4j/Redis + and Docker _not_ running, a brand-new config must boot. Confirm + `db_provider=sqlite`, `graph_provider=ladybug`, `queue_provider=honker` + are the defaults (`packages/config/src/schema.ts`) and that boot prints + `embedded mode — no Docker required`. +- [ ] **Embedded MCP search actually returns results.** The make-or-break test. + Ingest a repo in embedded mode, then call `smart_search`, `keyword_lookup`, + `list_knowledge`, and `retrieve_file`. Ladybug search is `CONTAINS`-based + (not fulltext) — verify each tool returns non-empty, relevant results, not + errors or empty sets. Compare the _same_ queries against Neo4j mode to + gauge ranking differences. +- [ ] **The three embedded files get created and persist:** `~/.bytebell/data.sqlite`, + `~/.bytebell/ladybug.lbug`, `~/.bytebell/queue.db`. Restart the server and + confirm indexed data survives. +- [ ] **Provider-aware boot preflight.** In embedded mode the server must _not_ + demand `mongo_uri` / `neo4j_*` / `redis_url`. Unset each embedded path + (`sqlite_path`, `ladybug_path`, `queue_db_path`) and confirm boot fails + with a clear error naming the missing key. + +--- + +## 1. One-command install (`install.sh`, `SETUP.md`) + +- [ ] `install.sh` detects missing Bun / git and prints install links. +- [ ] It clones the repo (skips if present), runs `bun install --frozen-lockfile`, + and links `bytebell` onto PATH. +- [ ] `bytebell --help` works after install; final message points to `bytebell setup`. +- [ ] Docker is **not** a prerequisite anymore (embedded default) — verify the + script doesn't hard-fail when Docker is absent. + +## 2. Setup wizard (`bytebell setup`) + +- [ ] Requires an interactive TTY; piped/CI input is handled gracefully. +- [ ] Stage order: LLM provider → infra mode → credentials → optional repo index → confirmation. +- [ ] **Infra mode defaults to "embedded (recommended)"** (changed from docker). +- [ ] OpenRouter path requires API key + model; Ollama path requires URL + model. +- [ ] Confirmation screen masks the API key. +- [ ] After apply: config written, server boots, `mcp install` runs, and (if a repo + URL was given) indexing polls to `PROCESSED`. +- [ ] `Esc` cancels cleanly at any stage. + +## 3. Config keys & provider switching (`bytebell set`) + +New keys (set via `bytebell set `; bare key with no value **toggles**): + +| Key | Values | Default | +| ------------------------------------------------ | ------------------- | --------------- | +| `db-provider` | `sqlite` ↔ `mongo` | `sqlite` | +| `graph-provider` | `ladybug` ↔ `neo4j` | `ladybug` | +| `queue-provider` | `honker` ↔ `bullmq` | `honker` | +| `sqlite-path` / `ladybug-path` / `queue-db-path` | file paths | `~/.bytebell/*` | + +- [ ] `bytebell set db-provider mongo` flips to mongo and **auto-fills** `mongo_uri`. +- [ ] `bytebell set graph-provider neo4j` auto-fills `neo4j_uri/user/password` + (random password generated). +- [ ] `bytebell set queue-provider bullmq` auto-fills `redis_url`. +- [ ] Bare toggle (`bytebell set queue-provider` with no value) flips to the other value. +- [ ] If a `set mode ` preset exists, it sets all three providers + atomically and fills their defaults — verify both presets. +- [ ] Confirm the in-flight change in `packages/config/src/schema.ts` is committed/ + intended before merging (currently shows as modified in the working tree). + +## 4. Docker mode (regression — must still work) + +- [ ] Switch to docker mode and confirm Mongo/Neo4j/Redis come up via + `infra/docker/docker-compose.yml`. +- [ ] Ingest a repo end-to-end; all four MCP tools return results (the established + baseline for search quality). +- [ ] `bytebell shutdown --with-docker` stops containers; `--keep-docker` leaves + them; embedded mode ignores both flags. +- [ ] Port-conflict handling on boot (reuse / kill / change port) behaves sanely. + +## 5. Queue providers + crash recovery + +- [ ] Honker (file-based, no Redis) queues and processes ingestion jobs in embedded mode. +- [ ] BullMQ still works after switching `queue-provider bullmq` (Redis required). +- [ ] **Orphan resumer:** kill the server mid-ingest with a doc stuck in `QUEUED`, + restart, and confirm it logs a resume and the job completes + (`resumeOrphans()` in `packages/queue/src/resumer.ts`). + +## 6. Path migration (`bytebell migrate paths`) + +- [ ] `bytebell migrate paths --dry-run` reports moves without touching disk. +- [ ] Real run moves legacy `~/.bytebell/repos/` + `.meta/` into the + commit-scoped `orgs///////` layout. +- [ ] Legacy dir with **no DB record** → reported as `abandoned` (deleted). +- [ ] DB record but **missing commitId/repoUrl** → `skippedNoCommit` / + `skippedNoRepoUrl`, data preserved. +- [ ] **Boot-time auto-reconcile:** server runs migration at startup and _refuses + to boot_ (`LayoutMigrationRequiredError`) if a record exists but can't be + migrated — verify it does not silently delete data. + +## 7. Automatic MCP install (`bytebell mcp install`) + +- [ ] Auto-detects installed clients and writes the correct JSON shape per target: + - Claude Code (`~/.claude.json`) & Claude Desktop & VS Code (`servers` key) → `{type:"http", url}` + - Cursor → `{url}` only + - Windsurf → `{serverUrl}` (note the different key name) +- [ ] Creates a `.bytebell.bak` backup before modifying each config; preserves + existing `mcpServers`/`servers` entries. +- [ ] Interactive multi-select (all detected pre-checked); non-interactive + configures all detected. +- [ ] No clients detected → prints the manual + `claude mcp add --transport http bytebell http://127.0.0.1:8080/mcp` fallback. +- [ ] Restart a client and confirm the bytebell tools actually appear and respond. + +## 8. Ingestion performance (Neo4j batch writes) + +- [ ] Ingest a large repo (1k+ files) in Neo4j mode; confirm batched transactional + upserts (`upsertFileNodesBatch` in `packages/neo4j/src/files-batch.ts`) and + that the final graph (keywords/classes/functions edges) matches per-file + behavior — no dropped relationships. 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..d42d544 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -107,15 +107,33 @@ 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: `@bb/config` overlays the keychain value on load, so the +server and all consumers resolve secrets via the normal config path. The server +warns at boot if a secret is still sitting in plaintext. ## Public exports @@ -201,7 +219,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 +240,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 +272,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 +303,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..83e17ae 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`); 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..14883e2 100644 --- a/packages/config/README.md +++ b/packages/config/README.md @@ -17,9 +17,36 @@ 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()` overlays each empty secret field with its keychain +value, so `getConfigValue`, `isConfigComplete`, and every downstream reader +resolve the secret transparently — no caller changes. Resolution order per +secret: **non-empty plaintext in `config.json` wins** (legacy / warned +fallback) → else keychain → else empty (`missing`). + +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 +91,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 +113,17 @@ 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` are stored in the OS keychain, not `config.json`; a plaintext + secret only remains as a legacy/keychain-less fallback and triggers a boot + warning recommending a re-run of `bytebell set `. 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 +131,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) 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..a99769d 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 { getSecret, SECRET_KEYS } from "./keychain.ts"; let cached: BytebellConfig | null = null; let seeded = false; @@ -43,10 +46,53 @@ export function loadConfig(): BytebellConfig { ensureBytebellHome(); const raw = fs.readFileSync(getConfigPath(), "utf8"); const parsed: unknown = JSON.parse(raw); - cached = configSchema.parse(parsed); + cached = overlaySecrets(configSchema.parse(parsed)); return cached; } +/** + * For each secret key left empty in `config.json`, overlay the value stored in + * the OS keychain. A non-empty plaintext value always wins (legacy / warned + * fallback), so every downstream reader sees the resolved secret transparently. + */ +function overlaySecrets(cfg: BytebellConfig): BytebellConfig { + let next = cfg; + for (const key of SECRET_KEYS) { + const current = readField(next, key); + if (typeof current === "string" && current.length === 0) { + const secret = getSecret(key); + if (secret !== null && secret.length > 0) { + next = writeField(next, key, secret as ConfigValue); + } + } + } + return next; +} + +/** + * Where a secret's live value comes from — see {@link SecretSource}. A non-empty + * plaintext value in `config.json` wins (and is a security smell boot warns + * about); otherwise the OS keychain; otherwise it 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]); diff --git a/packages/config/src/schema.ts b/packages/config/src/schema.ts index 904a39a..0fd0c9e 100644 --- a/packages/config/src/schema.ts +++ b/packages/config/src/schema.ts @@ -154,7 +154,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..8e3545d 100755 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -2,8 +2,15 @@ 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, + DbProviderType, + GraphProviderType, + QueueProviderType, + SecretSource, + type Config as ConfigEnum, +} from "@bb/types"; +import { getBytebellHome, getConfigValue, getSecretSource, HINTS, SECRET_KEYS } from "@bb/config"; import { connectDb } from "@bb/db"; import { connectGraph, indexesGraph } from "@bb/graph-db"; import { connectQueue, resumeOrphans } from "@bb/queue"; @@ -87,8 +94,24 @@ function checkRequiredConfig(): void { } } +/** + * Warn (do not fail) when a secret is still sitting in plaintext in config.json. + * No silent migration — re-running `bytebell set ` re-stores it in + * the OS keychain (when a backend is available). + */ +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. Re-run "bytebell set ${key} " to move it to the OS keychain.\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, From d6b108436203d143e334612f2fe25bb49614a2d4 Mon Sep 17 00:00:00 2001 From: lovanshu garg Date: Fri, 5 Jun 2026 15:20:00 +0530 Subject: [PATCH 2/4] feat(keychain): removign testing file --- TESTING.md | 127 ----------------------------------------------------- 1 file changed, 127 deletions(-) delete mode 100644 TESTING.md diff --git a/TESTING.md b/TESTING.md deleted file mode 100644 index df46b9a..0000000 --- a/TESTING.md +++ /dev/null @@ -1,127 +0,0 @@ -# Testing Checklist — `default` branch vs `main` - -This branch merges the **embedded prerelease**: 176 files, ~8,200 insertions. The -headline change is that **the default infrastructure flips from Docker -(Mongo/Neo4j/Redis) to embedded (SQLite + Ladybug + Honker)**, plus a one-command -install, a `setup` wizard, and automatic MCP client configuration. - -Use this as a manual QA pass before merging. Items are ordered by priority — the -new defaults are what every new user hits first, so test those before anything -else. - ---- - -## ⚠️ Highest-priority items (the new defaults) - -- [ ] **Fresh install with zero Docker.** On a machine with no Mongo/Neo4j/Redis - and Docker _not_ running, a brand-new config must boot. Confirm - `db_provider=sqlite`, `graph_provider=ladybug`, `queue_provider=honker` - are the defaults (`packages/config/src/schema.ts`) and that boot prints - `embedded mode — no Docker required`. -- [ ] **Embedded MCP search actually returns results.** The make-or-break test. - Ingest a repo in embedded mode, then call `smart_search`, `keyword_lookup`, - `list_knowledge`, and `retrieve_file`. Ladybug search is `CONTAINS`-based - (not fulltext) — verify each tool returns non-empty, relevant results, not - errors or empty sets. Compare the _same_ queries against Neo4j mode to - gauge ranking differences. -- [ ] **The three embedded files get created and persist:** `~/.bytebell/data.sqlite`, - `~/.bytebell/ladybug.lbug`, `~/.bytebell/queue.db`. Restart the server and - confirm indexed data survives. -- [ ] **Provider-aware boot preflight.** In embedded mode the server must _not_ - demand `mongo_uri` / `neo4j_*` / `redis_url`. Unset each embedded path - (`sqlite_path`, `ladybug_path`, `queue_db_path`) and confirm boot fails - with a clear error naming the missing key. - ---- - -## 1. One-command install (`install.sh`, `SETUP.md`) - -- [ ] `install.sh` detects missing Bun / git and prints install links. -- [ ] It clones the repo (skips if present), runs `bun install --frozen-lockfile`, - and links `bytebell` onto PATH. -- [ ] `bytebell --help` works after install; final message points to `bytebell setup`. -- [ ] Docker is **not** a prerequisite anymore (embedded default) — verify the - script doesn't hard-fail when Docker is absent. - -## 2. Setup wizard (`bytebell setup`) - -- [ ] Requires an interactive TTY; piped/CI input is handled gracefully. -- [ ] Stage order: LLM provider → infra mode → credentials → optional repo index → confirmation. -- [ ] **Infra mode defaults to "embedded (recommended)"** (changed from docker). -- [ ] OpenRouter path requires API key + model; Ollama path requires URL + model. -- [ ] Confirmation screen masks the API key. -- [ ] After apply: config written, server boots, `mcp install` runs, and (if a repo - URL was given) indexing polls to `PROCESSED`. -- [ ] `Esc` cancels cleanly at any stage. - -## 3. Config keys & provider switching (`bytebell set`) - -New keys (set via `bytebell set `; bare key with no value **toggles**): - -| Key | Values | Default | -| ------------------------------------------------ | ------------------- | --------------- | -| `db-provider` | `sqlite` ↔ `mongo` | `sqlite` | -| `graph-provider` | `ladybug` ↔ `neo4j` | `ladybug` | -| `queue-provider` | `honker` ↔ `bullmq` | `honker` | -| `sqlite-path` / `ladybug-path` / `queue-db-path` | file paths | `~/.bytebell/*` | - -- [ ] `bytebell set db-provider mongo` flips to mongo and **auto-fills** `mongo_uri`. -- [ ] `bytebell set graph-provider neo4j` auto-fills `neo4j_uri/user/password` - (random password generated). -- [ ] `bytebell set queue-provider bullmq` auto-fills `redis_url`. -- [ ] Bare toggle (`bytebell set queue-provider` with no value) flips to the other value. -- [ ] If a `set mode ` preset exists, it sets all three providers - atomically and fills their defaults — verify both presets. -- [ ] Confirm the in-flight change in `packages/config/src/schema.ts` is committed/ - intended before merging (currently shows as modified in the working tree). - -## 4. Docker mode (regression — must still work) - -- [ ] Switch to docker mode and confirm Mongo/Neo4j/Redis come up via - `infra/docker/docker-compose.yml`. -- [ ] Ingest a repo end-to-end; all four MCP tools return results (the established - baseline for search quality). -- [ ] `bytebell shutdown --with-docker` stops containers; `--keep-docker` leaves - them; embedded mode ignores both flags. -- [ ] Port-conflict handling on boot (reuse / kill / change port) behaves sanely. - -## 5. Queue providers + crash recovery - -- [ ] Honker (file-based, no Redis) queues and processes ingestion jobs in embedded mode. -- [ ] BullMQ still works after switching `queue-provider bullmq` (Redis required). -- [ ] **Orphan resumer:** kill the server mid-ingest with a doc stuck in `QUEUED`, - restart, and confirm it logs a resume and the job completes - (`resumeOrphans()` in `packages/queue/src/resumer.ts`). - -## 6. Path migration (`bytebell migrate paths`) - -- [ ] `bytebell migrate paths --dry-run` reports moves without touching disk. -- [ ] Real run moves legacy `~/.bytebell/repos/` + `.meta/` into the - commit-scoped `orgs///////` layout. -- [ ] Legacy dir with **no DB record** → reported as `abandoned` (deleted). -- [ ] DB record but **missing commitId/repoUrl** → `skippedNoCommit` / - `skippedNoRepoUrl`, data preserved. -- [ ] **Boot-time auto-reconcile:** server runs migration at startup and _refuses - to boot_ (`LayoutMigrationRequiredError`) if a record exists but can't be - migrated — verify it does not silently delete data. - -## 7. Automatic MCP install (`bytebell mcp install`) - -- [ ] Auto-detects installed clients and writes the correct JSON shape per target: - - Claude Code (`~/.claude.json`) & Claude Desktop & VS Code (`servers` key) → `{type:"http", url}` - - Cursor → `{url}` only - - Windsurf → `{serverUrl}` (note the different key name) -- [ ] Creates a `.bytebell.bak` backup before modifying each config; preserves - existing `mcpServers`/`servers` entries. -- [ ] Interactive multi-select (all detected pre-checked); non-interactive - configures all detected. -- [ ] No clients detected → prints the manual - `claude mcp add --transport http bytebell http://127.0.0.1:8080/mcp` fallback. -- [ ] Restart a client and confirm the bytebell tools actually appear and respond. - -## 8. Ingestion performance (Neo4j batch writes) - -- [ ] Ingest a large repo (1k+ files) in Neo4j mode; confirm batched transactional - upserts (`upsertFileNodesBatch` in `packages/neo4j/src/files-batch.ts`) and - that the final graph (keywords/classes/functions edges) matches per-file - behavior — no dropped relationships. From 902c5248bba97c9faf6a6008a40999dc23b7e5eb Mon Sep 17 00:00:00 2001 From: lovanshu garg Date: Mon, 8 Jun 2026 12:05:55 +0530 Subject: [PATCH 3/4] fix(migration): keychain from overlaying to migration --- CLAUDE.md | 4 +- packages/cli/README.md | 9 +++-- packages/config/src/loader.ts | 57 ++++++++++++++++++--------- packages/server/src/index.ts | 72 ++++++----------------------------- 4 files changed, 60 insertions(+), 82 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fce504a..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**: 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. A plaintext value remains only as a legacy / keychain-less fallback and is flagged at boot. +- **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) --- @@ -166,7 +166,7 @@ The `~/.bytebell/` directory is the **single source of truth** for runtime confi pid running server PID ``` -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. A non-empty plaintext secret is a legacy / keychain-less fallback only and triggers a boot warning. +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 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/packages/cli/README.md b/packages/cli/README.md index d42d544..5879446 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -131,9 +131,12 @@ any plaintext copy in `config.json`: - `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: `@bb/config` overlays the keychain value on load, so the -server and all consumers resolve secrets via the normal config path. The server -warns at boot if a secret is still sitting in plaintext. +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 diff --git a/packages/config/src/loader.ts b/packages/config/src/loader.ts index a99769d..f13c7c1 100644 --- a/packages/config/src/loader.ts +++ b/packages/config/src/loader.ts @@ -11,8 +11,8 @@ import { } from "./schema.ts"; import { SecretSource } from "@bb/types"; import { __registerCacheInvalidator, getConfigPath, resolveUnderHome } from "./paths.ts"; -import { ensureBytebellHome } from "./writer.ts"; -import { getSecret, SECRET_KEYS } from "./keychain.ts"; +import { ensureBytebellHome, setConfigValue } from "./writer.ts"; +import { getSecret, setSecret, SECRET_KEYS, KeychainUnavailableError } from "./keychain.ts"; let cached: BytebellConfig | null = null; let seeded = false; @@ -45,34 +45,57 @@ export function loadConfig(): BytebellConfig { } ensureBytebellHome(); const raw = fs.readFileSync(getConfigPath(), "utf8"); - const parsed: unknown = JSON.parse(raw); - cached = overlaySecrets(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; } /** - * For each secret key left empty in `config.json`, overlay the value stored in - * the OS keychain. A non-empty plaintext value always wins (legacy / warned - * fallback), so every downstream reader sees the resolved secret transparently. + * 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 overlaySecrets(cfg: BytebellConfig): BytebellConfig { +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) { - const secret = getSecret(key); - if (secret !== null && secret.length > 0) { - next = writeField(next, key, secret as ConfigValue); + 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 next; + return { cfg: next, migrated }; } /** - * Where a secret's live value comes from — see {@link SecretSource}. A non-empty - * plaintext value in `config.json` wins (and is a security smell boot warns - * about); otherwise the OS keychain; otherwise it is unset. + * 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) { @@ -109,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/server/src/index.ts b/packages/server/src/index.ts index 8e3545d..f73390f 100755 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -2,15 +2,8 @@ import { writeFile } from "node:fs/promises"; import path from "node:path"; import express from "express"; -import { - Config, - DbProviderType, - GraphProviderType, - QueueProviderType, - SecretSource, - type Config as ConfigEnum, -} from "@bb/types"; -import { getBytebellHome, getConfigValue, getSecretSource, HINTS, SECRET_KEYS } 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"; @@ -32,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) { @@ -95,15 +47,15 @@ function checkRequiredConfig(): void { } /** - * Warn (do not fail) when a secret is still sitting in plaintext in config.json. - * No silent migration — re-running `bytebell set ` re-stores it in - * the OS keychain (when a backend is available). + * 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. Re-run "bytebell set ${key} " to move it to the OS keychain.\n`, + `⚠ ${key} is stored in plaintext in config.json — no OS keychain backend is available on this system to migrate it.\n`, ); } } From 8cf7cad6514aaef2d397483d9612059b24c68811 Mon Sep 17 00:00:00 2001 From: lovanshu garg Date: Mon, 8 Jun 2026 12:09:56 +0530 Subject: [PATCH 4/4] fix(required): keys based on mode --- packages/cli/src/bootConfig.ts | 8 ++++-- packages/config/README.md | 31 ++++++++++++++-------- packages/config/src/schema.ts | 48 +++++++++++++++++++++++++--------- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/bootConfig.ts b/packages/cli/src/bootConfig.ts index 83e17ae..8145c91 100644 --- a/packages/cli/src/bootConfig.ts +++ b/packages/cli/src/bootConfig.ts @@ -114,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/config/README.md b/packages/config/README.md index 14883e2..9e8bec1 100644 --- a/packages/config/README.md +++ b/packages/config/README.md @@ -30,11 +30,19 @@ 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()` overlays each empty secret field with its keychain -value, so `getConfigValue`, `isConfigComplete`, and every downstream reader -resolve the secret transparently — no caller changes. Resolution order per -secret: **non-empty plaintext in `config.json` wins** (legacy / warned -fallback) → else keychain → else empty (`missing`). +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 @@ -114,9 +122,10 @@ This package does **not** own: 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` is owner-read/write only. Secrets in - `SECRET_KEYS` are stored in the OS keychain, not `config.json`; a plaintext - secret only remains as a legacy/keychain-less fallback and triggers a boot - warning recommending a re-run of `bytebell set `. + `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. @@ -144,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/src/schema.ts b/packages/config/src/schema.ts index 0fd0c9e..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> = {