fix: shell-independent API key resolution (#11) #13
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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/utils/env-loader.test.js` | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| - 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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. 🧩 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 }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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