Repository navigation
Fix/1389 credential store - #2012
Open
Tapraj-Pathak wants to merge 7 commits into
Open
Tapraj-Pathak wants to merge 7 commits into
Tapraj-Pathak wants to merge 7 commits into
Conversation
…t use time (1jehuang#1389) The process environment and per-provider env files stored credentials in two places and copied between them, causing three bug classes: the in-memory copy wins and goes stale, the in-memory copy leaks into child processes, and startup-time resolution sees nothing. Make the env file the truth and the process env a read-only override sampled only at use time. - jcode-provider-env: add a Creds facade (get/set/clear/has/require) over the env-file store; save_env_value_to_env_file no longer writes or removes the process env. - Migrate raw env readers of app-written secrets to the facade / load_api_key_from_env_or_config (cursor, codex, provider_fetch, anthropic, openrouter dispatch, zai alias, memory recall bench, anthropic runtime, dual-credential active-method probe). - Annotate intentional foreign/legacy env reads and process-env writes (Claude Code's own env var, Grok deployment key, Composio, AWS SDK chain, lifecycle legacy file->env sync for foreign readers) as ponytail carve-outs. - openrouter runtime: ProviderAuth now stores key *names* (+ env file), resolving the value per request via apply(); AuthResolver closures are gone; constructors fail fast with require_all() but never capture the token. Inline/config keys use InlineBearer/InlineHeader resolved at construction. - Add a ratchet script banning new raw env::var reads of secrets (scripts/check_secret_env_reads.py + baseline), wired into CI and scripts/check_guardrails.sh. - --provider auto: catalog-driven compat-profile detection already scans openai_compatible_profiles(); add a regression test proving a non-allowlisted profile (DeepSeek) with direct credentials bootstraps instead of an empty deferred MultiProvider. Test results on Windows: 6 pre-existing openrouter-runtime failures and the jcode-base environment-dependent failures (`sqlite3` CLI, browser tool, Windows path-escape JSON fixtures, Windows socket WouldBlock) are identical at pristine HEAD; no new failures from this change. New regression tests pass: auth_names_resolve_live_between_requests, creds_set_and_clear_are_file_only_and_read_live, auto_provider_detects_non_allowlisted_compat_catalog_credentials.
…ebase (1jehuang#1389) Upstream (646 commits ahead) moved new_openai_compatible_profile_runtime into openai_compatible_runtime.rs and added a Jcode-managed Muse subscription credential store. Adapt the names-based ProviderAuth refactor to the new layout: - Keep the constructor in openai_compatible_runtime.rs; gate on load_openai_compatible_profile_api_key so the native Muse subscription route is only enabled by its stored credentials (a plain Meta API key is never accepted) and still store key names, resolved live at request time. - ProviderAuth::resolve_key now uses load_runtime_api_key, which reads env-then-file and falls back to the Muse subscription store. - Drop the duplicate constructor from lib.rs and the unused autodetect timing variable; fix the loopback-catalog test fixture to InlineBearer.
Contributor
 [High impact] Safe to merge based on the reviewed changes. No outstanding blocking findings remain. SummaryThe PR moves credential reads behind shared helpers and resolves request keys when they are used.
No outstanding blocking findings remain. The reviewed changes are safe to merge. Reviews (4) · Last reviewed commit: "fix(auth): reconcile env readers/writers..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… and live per-request auth (1jehuang#1389)
…entries, isolate credential test Three review issues addressed: - provider-env: Creds::set now updates Jcode-managed process env copies when saving a replacement key, so readers that prefer the process env (load_env_value_from_env_or_config / load_api_key_from_env_or_config) see the new key immediately instead of continuing to use the stale copy. Adds regression test replacement_key_reconciles_jcode_managed_process_copy. - provider-env: Both file-reading loops now trim each line before prefix matching (line.trim().strip_prefix), restoring the behaviour of the old per-provider readers. Files with indented entries such as ' CURSOR_API_KEY=abc' were silently broken after migration to the shared reader. Adds regression test loads_api_key_from_indented_env_file_line. - openrouter-runtime: optional_bearer_resolves_live_per_request now acquires ENV_LOCK before creating its EnvVarGuards. Without the lock the test's JCODE_HOME change could race with lock-holding credential tests and corrupt their file operations.
…fallback, and test isolation (1jehuang#1389)
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Credentials: one store, read at use — delete set_var, captured auth, and the auto allowlist (#1389)
Summary
Makes the env file the single credential store. The process environment is a read-only override that is sampled at use time. Secret lookups happen per request, never cached at startup, so re-keying or editing the env file takes effect immediately without a restart.
~/.config/jcode/*.env); nothing writes or removes process env vars as a side effect of login/logout/refresh.ProviderAuthnow stores key names (key_name,env_file) and resolves the actual token only at request time.AuthResolverclosures are deleted.Included
Credsfacade + env-file store migration (jcode-provider-env)CredsAPI (get/set/clear/has/require) over env-then-file.save_env_value_to_env_fileno longer touches process env — no more secretset_var/remove_varoutside tests.ponytail:carve-outs with justification.ProviderAuth: names resolved at request time (jcode-provider-openrouter-runtime)AuthorizationBearer { key_name, env_file, label }/HeaderValue { header_name, key_name, ... }resolve live viaresolve_key()→load_runtime_api_key(env → env file → Muse subscription store).InlineBearer/InlineHeader(resolved at construction).AuthResolverdeleted;resolve_auth()→resolve_auth_names(); eagerrequire_all()keeps fail-fast at/serveboot.Catalog-driven
--provider auto(src/cli/provider_init.rs)Guardrail
scripts/check_secret_env_reads.pyratchet (baseline: 7 legacy reads / 5 files) wired into CI andscripts/check_guardrails.sh— new directenv::varreads of*KEY*/*TOKEN*/*SECRET*break the build.Regression tests added
auth_names_resolve_live_between_requests— re-key between requests, process env never set, clear → "Runjcode login" error.catalog_request_headers_follow_provider_auth_rulesauto_provider_detects_non_allowlisted_compat_catalog_credentials— onlyDEEPSEEK_API_KEYin env → auto must bootstrap the DeepSeek profile.Verification
cargo check --workspace: 0 errors.secret env read ratchet ok (7 legacy reads).origin/masterexactly (pre-existing Windows env-dir test issues) — zero new failures.Notes / caveats
on_auth_changed → reload_credentials_now()hook (documented deviation — deleting it would regresssave_named_api_keywrites secrets into the process environment, shadowing the env file until restart #1386 for the OpenAI tree).claude.rs,grok_build.rs,gmail.rs,provider-bedrockAWS chain,cursor.rsCURSOR_ACCESS_TOKEN, etc.).XDG_CONFIG_HOME/HOME/APPDATA, whichdirs::config_dir()ignores on Windows); follow-up: switch those tests toJCODE_HOME.Commits:
a74d80f74·5a1f98e2c·47828f722(28 files, +650/−277, plus reconciliation)