-
Notifications
You must be signed in to change notification settings - Fork 2
fix: shell-independent API key resolution (#11) #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e866cfc
cf5e069
13bb1f8
61ff895
3dc379f
901f40b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| # Shell-Independent API Key Resolution | ||
|
|
||
| **Issue:** [#11](https://github.com/jrenaldi79/sidecar/issues/11) - Sidecar fails with 'Missing Authentication header' in non-interactive shells | ||
| **Date:** 2026-03-14 | ||
| **Status:** Draft | ||
|
|
||
| ## Problem | ||
|
|
||
| Sidecar CLI fails when API keys are exported in `~/.zshrc` but not `~/.zshenv`. Non-interactive shells (Claude Code's Bash tool, CI/CD, cron) don't source `~/.zshrc`, so `process.env` lacks the keys. | ||
|
|
||
| The irony: sidecar already has two shell-independent key stores (`~/.config/sidecar/.env` and `~/.local/share/opencode/auth.json`), but doesn't load them into `process.env` early enough. The validator checks `process.env` first and fails before reaching fallback logic. | ||
|
|
||
| ## Design | ||
|
|
||
| ### New Module: `src/utils/env-loader.js` | ||
|
|
||
| Single exported function `loadCredentials()` that projects all credential sources into `process.env` with deterministic priority: | ||
|
|
||
| ``` | ||
| 1. process.env (already set) <- highest, never overwritten | ||
| 2. ~/.config/sidecar/.env <- user-configured via `sidecar setup` | ||
| 3. ~/.local/share/opencode/auth.json <- OpenCode SDK fallback | ||
| ``` | ||
|
|
||
| Behavior: | ||
| - Per-provider, first source wins. Existing `process.env` values are never overwritten. | ||
| - Respects `SIDECAR_ENV_DIR` override for `.env` path (via `getEnvPath()` from `api-key-store.js`). | ||
| - Uses existing `parseEnvContent()` from `api-key-store.js` for `.env` parsing (no separate `dotenv` dependency needed). | ||
| - Handles `LEGACY_KEY_NAMES` migration (e.g., `GEMINI_API_KEY` to `GOOGLE_GENERATIVE_AI_API_KEY`), consolidating the migration that currently lives in both `bin/sidecar.js` and `api-key-store.js`. | ||
| - Auth.json keys are loaded in-memory only (no writes back to `.env`). | ||
| - Info-level logging when a key is loaded: `"Loaded GOOGLE_GENERATIVE_AI_API_KEY from sidecar .env"` (key values are never logged). Info-level is intentional: users hitting this bug are unlikely to have `LOG_LEVEL=debug`. | ||
| - Gracefully handles missing files (no error if `.env` or `auth.json` don't exist). | ||
|
|
||
| **Relationship to existing `resolveKeyValue()` in `api-key-store.js`:** The existing function gives `.env` file priority over `process.env`. After `loadCredentials()` projects all sources into `process.env`, `resolveKeyValue()` remains correct for its use case (setup UI display), but runtime validation now uses `process.env` as the single source of truth. No conflict because `loadCredentials()` never overwrites existing `process.env` values. | ||
|
|
||
| **Known limitation:** `auth.json` path (`~/.local/share/opencode/auth.json`) follows the Linux XDG layout. This matches OpenCode's own behavior and is not a regression. | ||
|
|
||
| ### Update: `bin/sidecar.js` | ||
|
|
||
| Replace the existing early dotenv load and legacy key migration (lines 14-19) with a single `loadCredentials()` call before any validation or command dispatch. This consolidates three scattered concerns (dotenv loading, legacy migration, auth.json import) into one call. | ||
|
|
||
| **MCP entry point:** `sidecar mcp` dispatches through `bin/sidecar.js` via `handleMcp()`, so `loadCredentials()` runs before any MCP tool handler touches API keys. No separate MCP-specific loading needed. | ||
|
|
||
| ### Update: `src/utils/validators.js` | ||
|
|
||
| Remove the special auth.json existence check from `validateApiKey()`. After `loadCredentials()` runs, all available keys are in `process.env`, so the validator becomes a pure `process.env` check. No fallback logic needed. | ||
|
|
||
| Improved error message when keys are truly missing: | ||
|
|
||
| ``` | ||
| Error: GOOGLE_GENERATIVE_AI_API_KEY not found. | ||
|
|
||
| In non-interactive shells (Claude Code, CI), ~/.zshrc is not sourced. | ||
| Fix with one of: | ||
| - Run `sidecar setup` to store keys in sidecar's config | ||
| - Move your export to ~/.zshenv (sourced by all zsh shells) | ||
| - Add key to ~/.local/share/opencode/auth.json | ||
| ``` | ||
|
|
||
| ### Update: Documentation | ||
|
|
||
| - `skill/SKILL.md`: Add note under "Option B (Direct)" warning zsh users that `~/.zshrc` exports only work in interactive terminals. Recommend `~/.zshenv` or `sidecar setup`. | ||
| - Add troubleshooting entry for this specific scenario. | ||
|
|
||
| ## What We're NOT Doing | ||
|
|
||
| - **No sourcing shell profiles.** Executing `~/.zshenv` or `~/.zshrc` from Node.js is fragile, non-portable (bash vs zsh vs fish), and may have side effects or hang in CI. Both Gemini and GPT independently flagged this as "architecturally leaky." | ||
| - **No writing back to `.env` from auth.json.** Importing auth.json keys is in-memory only for the current process. Avoids side-effect writes during simple commands like `sidecar list`. | ||
| - **No loading arbitrary `.env` from cwd.** Only sidecar's own config directory `.env` is loaded. | ||
|
|
||
| ## Testing | ||
|
|
||
| ### Unit Tests: `tests/env-loader.test.js` | ||
|
|
||
| - Priority order: `process.env` > `.env` file > `auth.json` | ||
| - No-overwrite: existing `process.env` values are preserved | ||
| - Missing files: graceful handling when `.env` or `auth.json` don't exist | ||
| - `SIDECAR_ENV_DIR` override: respects custom `.env` path | ||
| - Per-provider merge: one key from `.env`, another from `auth.json` | ||
| - Security: file permissions, no secret logging | ||
|
|
||
| ### Update: `tests/utils/validators.test.js` | ||
|
|
||
| - Remove tests for auth.json special-case fallback in `validateApiKey()` | ||
| - Add test for improved error message content | ||
| - Verify validation passes when keys come from `.env` (loaded via `loadCredentials()`) | ||
|
|
||
| ### Integration Test | ||
|
|
||
| - Mocked integration test that calls `loadCredentials()` + `validateApiKey()` with a stubbed `process.env` and temp `.env` file. No real API call needed; just verify the key reaches `process.env` and validation passes. | ||
|
|
||
| ## Multi-Model Review | ||
|
|
||
| Design reviewed by Gemini and GPT-4 via sidecar. Both independently recommended Option B with the same priority order. Key feedback incorporated: | ||
| - Reject Option C (sourcing shell profiles) as fragile and unsafe | ||
| - Centralize loading in a single module | ||
| - Fix the "lying validator" that checks auth.json existence without loading keys | ||
| - Keep auth.json imports in-memory only | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /** | ||
| * Credential Loader | ||
| * | ||
| * Loads API keys from multiple sources into process.env at CLI bootstrap. | ||
| * Priority: process.env (already set) > sidecar .env > auth.json | ||
| * Never overwrites existing process.env values. | ||
| */ | ||
| const { logger } = require('./logger'); | ||
| const { loadEnvEntries, PROVIDER_ENV_MAP, LEGACY_KEY_NAMES } = require('./api-key-store'); | ||
| const { readAuthJsonKeys } = require('./auth-json'); | ||
|
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "package.json module type:"
jq -r '.type // "<missing>"' package.json
echo
echo "CommonJS usage in changed src/bin files:"
rg -nP --type=js '\b(require\s*\(|module\.exports\b)' src/utils/env-loader.js bin/sidecar.jsRepository: jrenaldi79/sidecar Length of output: 1582 🏁 Script executed: # Check the three imported modules
echo "=== logger.js (first 15 lines) ==="
head -15 src/utils/logger.js
echo
echo "=== api-key-store.js (first 15 lines) ==="
head -15 src/utils/api-key-store.js
echo
echo "=== auth-json.js (first 15 lines) ==="
head -15 src/utils/auth-json.js
echo
echo "=== Check for module.exports in dependencies ==="
rg -n 'module\.exports|export ' src/utils/{logger,api-key-store,auth-json}.js
echo
echo "=== Check if CLAUDE.md was updated ==="
rg 'env-loader|env_loader' CLAUDE.md || echo "No mention of env-loader in CLAUDE.md"Repository: jrenaldi79/sidecar Length of output: 1998 Convert to ESM as part of broader package migration. This file uses CommonJS ( To complete the conversion:
CLAUDE.md was correctly updated with env-loader.js documentation. 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Load credentials from all sources into process.env. | ||
| * Call once at CLI startup, before any validation. | ||
| * | ||
| * Sources (in priority order): | ||
| * 1. process.env - already set, never overwritten | ||
| * 2. ~/.config/sidecar/.env - user-configured via `sidecar setup` | ||
| * 3. ~/.local/share/opencode/auth.json - OpenCode SDK fallback | ||
| */ | ||
| function loadCredentials() { | ||
| // Step 1: Load from sidecar .env file | ||
| const fileEntries = loadEnvEntries(); | ||
| for (const [, envVar] of Object.entries(PROVIDER_ENV_MAP)) { | ||
| if (!process.env[envVar]) { | ||
| const fromFile = fileEntries.get(envVar); | ||
| if (fromFile && fromFile.length > 0) { | ||
| process.env[envVar] = fromFile; | ||
| logger.info(`Loaded ${envVar} from sidecar .env`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Step 1b: Handle legacy key names in process.env | ||
| for (const [oldName, newName] of Object.entries(LEGACY_KEY_NAMES)) { | ||
| if (process.env[oldName] && !process.env[newName]) { | ||
| process.env[newName] = process.env[oldName]; | ||
| logger.info(`Migrated ${oldName} to ${newName}`); | ||
| } | ||
| } | ||
|
|
||
| // Step 2: Import from auth.json (lowest priority) | ||
| const authKeys = readAuthJsonKeys(); | ||
| for (const [provider, key] of Object.entries(authKeys)) { | ||
| const envVar = PROVIDER_ENV_MAP[provider]; | ||
| if (envVar && !process.env[envVar]) { | ||
| process.env[envVar] = key; | ||
| logger.info(`Loaded ${envVar} from auth.json`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = { loadCredentials }; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -226,12 +226,10 @@ const { validateMcpSpec, validateMcpConfigFile } = require('./mcp-validators'); | |||||||||||||||||||||||||
| const { MODEL_THINKING_SUPPORT, getSupportedThinkingLevels, validateThinkingLevel } = require('./thinking-validators'); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Validate API key is present for the given model's provider | ||||||||||||||||||||||||||
| * Validate API key is present for the given model's provider. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * NOTE: OpenCode manages its own credentials via `opencode auth`. | ||||||||||||||||||||||||||
| * The env var check is a convenience hint, not a hard requirement. | ||||||||||||||||||||||||||
| * If OpenCode has credentials configured, the model will work | ||||||||||||||||||||||||||
| * even without the env var set. | ||||||||||||||||||||||||||
| * Assumes loadCredentials() has already run, projecting all credential | ||||||||||||||||||||||||||
| * sources (sidecar .env, auth.json) into process.env. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * @param {string} model - The model string (e.g., 'openrouter/google/gemini-2.5-flash') | ||||||||||||||||||||||||||
| * @returns {{valid: boolean, error?: string}} | ||||||||||||||||||||||||||
|
|
@@ -249,17 +247,14 @@ function validateApiKey(model) { | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!process.env[providerInfo.key]) { | ||||||||||||||||||||||||||
| // Check if OpenCode has credentials configured (auth.json) | ||||||||||||||||||||||||||
| const os = require('os'); | ||||||||||||||||||||||||||
| const authPath = path.join(os.homedir(), '.local', 'share', 'opencode', 'auth.json'); | ||||||||||||||||||||||||||
| if (fs.existsSync(authPath)) { | ||||||||||||||||||||||||||
| // OpenCode manages its own auth — skip env var check | ||||||||||||||||||||||||||
| return { valid: true }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| valid: false, | ||||||||||||||||||||||||||
| error: `Error: ${providerInfo.key} environment variable is required for ${providerInfo.name} models. Set it with: export ${providerInfo.key}=your-api-key` | ||||||||||||||||||||||||||
| error: `Error: ${providerInfo.key} not found.\n\n` + | ||||||||||||||||||||||||||
| 'In non-interactive shells (Claude Code, CI), ~/.zshrc is not sourced.\n' + | ||||||||||||||||||||||||||
| 'Fix with one of:\n' + | ||||||||||||||||||||||||||
| ' - Run `sidecar setup` to store keys in sidecar\'s config\n' + | ||||||||||||||||||||||||||
| ' - Move your export to ~/.zshenv (sourced by all zsh shells)\n' + | ||||||||||||||||||||||||||
| ' - Add key to ~/.local/share/opencode/auth.json' | ||||||||||||||||||||||||||
|
Comment on lines
+252
to
+257
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid hardcoding a single auth.json path in the remediation text. The message currently implies only one location. In non-default setups this can misdirect users; make it explicitly “default path” or generic. Suggested wording tweak- ' - Add key to ~/.local/share/opencode/auth.json'
+ ' - Add key to your OpenCode auth.json (default: ~/.local/share/opencode/auth.json)'📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language identifiers to fenced code blocks.
Both fenced blocks should include a language/info string to satisfy MD040.
Proposed fix
@@
-
+textError: GOOGLE_GENERATIVE_AI_API_KEY not found.
@@
Also applies to: 50-58
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents