Repository navigation
fix(config): use options.apiKey as the provider credential, with {env:}/{file:} interpolation (#28) - #29
Open
tungmv wants to merge 1 commit into
Open
fix(config): use options.apiKey as the provider credential, with {env:}/{file:} interpolation (#28)#29tungmv wants to merge 1 commit into
tungmv wants to merge 1 commit into
Conversation
The endpoint builder read auth.json/auth-overlay.json only, so a provider
whose key lived in the documented `options.apiKey` slot reached the upstream
with `Authorization: Bearer ` (empty). The provider answers
[code=missing_api_key], which reads as a client bug rather than a config key
that was dropped, and the turn burns a round trip before failing.
Two arms were broken, and both presented identically, so both are covered:
- a literal key was ignored (auth-only lookup);
- a `{env:NAME}` reference was forwarded verbatim, because ocserve has no
config substitution at all.
Config `options.apiKey` now follows upstream precedence (config first, auth
layer as the fallback) and is interpolated: `{env:NAME}` and `{file:PATH}`,
the latter resolved relative to the declaring config rather than the process
cwd. `options/baseURL` is interpolated too (a per-environment gateway host).
This mattered beyond a single provider because opencode 2.x no longer writes
auth.json; keys live in the `credential` table, so on a 2.x install every
config-declared provider was unauthenticated unless its key was duplicated by
hand into auth-overlay.json.
Three deliberate deviations from upstream, all fail-loud:
- an unset `{env:NAME}` expands to "" (upstream parity) but logs the field
path; silence here is what made Reliary#28 an afternoon instead of a minute;
- a provider whose configured key resolves empty is named at boot, without
the secret;
- only the two documented forms expand. An unknown `{form:...}` is passed
through byte for byte instead of truncated.
`OCSERVE_CONFIG_INTERPOLATE=0` restores the verbatim behaviour, declared in
the env contract with docs/ENVIRONMENT.md regenerated.
Tests: runtime::provider_key_tests x9, plus a live wire repro against a
capture sink that logs the Authorization header (pre-fix `Bearer `, post-fix
`Bearer secret-from-env` for `{env:FAKE_API_KEY}` and `Bearer sk-literal` for
a literal). Gates: fmt, clippy -D warnings, check-guards (rule 58 included),
check-docs, check-matrix. TRACEABILITY row K-PROVIDER-KEY.
Refs Reliary#28
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
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.
Fixes #28.
The bug
The endpoint builder took the base URL from
provider.<id>.options.baseURLbut the key only from the auth layer, defaulting tounwrap_or(""):So a provider configured in the documented shape reached the upstream with
Authorization: Bearer(empty). The provider answers[code=missing_api_key] missing or invalid authorization header, which reads as a client bug, and the turn burns a round trip first.Two arms were broken and both presented identically:
"apiKey": "sk-literal"BearerBearer sk-literal"apiKey": "{env:FAKE_API_KEY}"BearerBearer secret-from-envThe second one is not only an
{env:}problem: ocserve has no config substitution at all, so the placeholder was forwarded verbatim.Why it was universal on opencode 2.x
load_auth()reads~/.local/share/opencode/auth.jsonandauth-overlay.json. opencode 2.x no longer writesauth.json; keys live in thecredentialtable ofopencode.db. On a 2.x install every config-declared provider was therefore unauthenticated unless its key had been duplicated by hand intoauth-overlay.json.I have deliberately not added the
credentialtable as a third auth source in this PR. That is a separate decision (which integration IDs map to which provider IDs, and how an OAuth row becomes a bearer), and this change stands on its own without it.The fix
options.apiKeyis now the first credential source, with the auth layer as the fallback (upstream precedence, and the same order theW5overlay rule assumes). Values are interpolated for{env:NAME}and{file:PATH};options/baseURLtoo, since a per-environment gateway host is the same problem.{file:}resolves relative to the declaring config rather than the process cwd (upstream uses cwd). The config-relative form is the one that makes a repo-local config work from any directory.Three deliberate deviations, all fail-loud
{env:NAME}expands to the empty string exactly as upstream, but logs the field path. Silence here is the whole reason options.apiKey in opencode.json is ignored: providers get an empty Bearer token ({env:VAR} and literals both) #28 took an afternoon: a provider with a configured key and a missing credential was indistinguishable from one that never had a key. Now the boot log says which variable is unset.{form:...}is passed through byte for byte instead of truncated, so a typo surfaces at the provider rather than as a half-eaten credential.OCSERVE_CONFIG_INTERPOLATE=0restores the old verbatim behaviour, for an operator who shares one config file with another server and does not want ocserve resolving references out of it. Interpolation does turn the config into code ({file:…}reads a path), so a kill switch seemed warranted; it is declared in the env contract anddocs/ENVIRONMENT.mdis regenerated.Tests
runtime::provider_key_tests× 9:literal_config_apikey_reaches_the_endpoint— isolates "auth-only lookup" from "no interpolation"env_reference_in_config_apikey_is_interpolatedunset_env_reference_warns_and_expands_empty— parity and visibilityother_placeholder_forms_are_left_verbatimfile_reference_resolves_relative_to_the_config_dirmissing_file_reference_warns_with_the_resolved_pathplain_strings_are_returned_untouchedauth_layer_is_the_fallback_and_config_wins— the W5 precedence thatPUT /auth/{providerID}relies oninterpolation_kill_switch_returns_the_raw_valuePlus a live wire repro, which is what the issue was filed with: a local capture sink logs the
Authorizationheader it receives.AUTH='Bearer 'for both armsAUTH='Bearer secret-from-env'andAUTH='Bearer sk-literal'Gates run
cargo fmt --check,cargo clippy -p ocserve --all-targets -- -D warnings,scripts/check-guards.sh(including rule 58, which is why the env contract entry and the regenerateddocs/ENVIRONMENT.mdare in this diff),scripts/check-docs.py,scripts/check-matrix.sh. TRACEABILITY rowK-PROVIDER-KEY.Two failures are pre-existing on clean
mainin this environment and untouched by this change, so they are visible here for transparency: 8partition::/resilience::tests fail on macOS (the cgroup partition dance is Linux-only), andevent-validate-selftest.pyneedsjsonschema, which was missing from the system Python (installed into a venv to run the gate).