feat: require an explicit llm input and never fall back to hosted credentials - #104
Merged
Conversation
…dentials The action resolved credentials by precedence: a provider key if present, a licence if present, otherwise CodeBoarding's hosted tier. An empty value was therefore indistinguishable from "no preference", so a repository that selected Anthropic and had not added its secret yet ran green on CodeBoarding's hosted OpenRouter tier -- a different vendor, a different model, our money -- and nothing in the run said so. Credentials are now named, not inferred. `llm` is required and takes `hosted`, `license`, or a provider name; each provider has its own `<name>_api_key` input. Anything ambiguous is refused: a named provider without its key, a hosted tier carrying a provider key, a licence where it would not be spent, a second provider's key. Refusals happen before the checkout and the engine install, and name the input and the secret to fix. A licence alongside a provider key stays valid and is reported as `byok+license`. Metering that combination needs proxy work and is not in scope. The provider table is mirrored from the pinned engine rather than hand-copied per site, and the foreign-selector list with-auth.sh strips is now derived from it. That list had already fallen behind: it was missing ORCAROUTER_API_KEY, which 0.13.10 added, so an inherited value could select a provider the workflow never asked for. A drift test installs the pinned release in CI and fails when the two disagree. Ships as feat: rather than feat!: on purpose -- see AGENTS.md. A major bump would freeze v1 and leave every existing workflow on the old silent-fallback behaviour permanently, which is the opposite of the intent. BREAKING: `llm_api_key` and `llm_provider` are replaced by `llm` plus per-provider inputs. Workflows that set neither must add `llm: hosted`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
CodeBoarding reviewStatus: 1 changed component See the full change in CodeBoarding. graph LR
n_Diff_Analysis_Engine["Diff Analysis Engine"]
n_Mermaid_Diagram_Renderer["Mermaid Diagram Renderer"]
n_OIDC_Relay_Handler["OIDC Relay Handler"]
n_Repository_Analysis_Driver["Repository Analysis Driver"]
n_OIDC_Relay_Server["OIDC Relay Server"]
n_LLM_Credential_Resolver["LLM Credential Resolver"]
n_Diff_Analysis_Engine -- "Passes computed structural diffs for diagram ge…" --> n_Mermaid_Diagram_Renderer
n_Repository_Analysis_Driver -- "Produces analysis JSON artifacts consumed for d…" --> n_Diff_Analysis_Engine
n_OIDC_Relay_Server -- "Dispatches incoming HTTP requests to relay hand…" --> n_OIDC_Relay_Handler
n_LLM_Credential_Resolver -- "Supplies auth configuration and triggers relay…" --> n_OIDC_Relay_Server
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_LLM_Credential_Resolver added;
linkStyle 3 stroke:#0b5d23,stroke-width:2px;
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4ae9e9d0f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ence Four things the Codex review was right about: - The credential check is a Python program and ran before `setup-python`. A hosted runner ships a system python3 and would never notice; a self-hosted one without it would fail a valid configuration. Python is provisioned first now, which still leaves the check ahead of the engine install. - `Post review failure` writes the same sticky comment on `failure()`, so a run stopped for a missing secret posted the fix and then buried it under "see the workflow logs". It now stands down when the credential check is what failed. - The drift test treated any `agents.llm_config` import error as "engine not installed" and skipped, so a pin that moved a module would retire the very check meant to catch it. Installed-ness is asked of the distribution; an import failure after that is raised. - The foreign-variable list spared every variable the selected provider could use rather than the ones the run resolved. `llm: openai` with only `openai_base_url` therefore let an inherited OPENAI_API_KEY credential the run: the same silent substitution this contract removes, one provider narrower. Also switches this repo's own workflows from `llm: hosted` to `llm: license`. CodeBoarding's repositories run on the CodeBoarding plan, and the secret is already configured here. The README examples stay on `hosted`, which is the right starting point for someone reading them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/github-storage-scopes.md is an uncommitted local draft about Actions storage scopes. It has nothing to do with the credential contract and was swept in by a `git add -A`; it belongs wherever its author decides, not here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a14cd8dbd8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…eaches Aliases were three extra values to document, test and keep in step with the picker, in exchange for accepting a spelling nobody was asked to type. The table is now keyed by the ONE value `llm:` accepts, each provider's inputs are named after it (`llm: X` always pairs with `X_api_key`), and an unrecognised value is refused with the accepted list. Bedrock keeps the friendlier `aws_bedrock` rather than the engine's internal `aws`; a `core` field carries that translation and nothing else does, so the drift test still compares against the engine name exactly. Also removed: the `hosted` field the resolver emitted and nothing read, and the OIDC check in configure-auth.sh, which preflight had already made unreachable. One decision point was the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bad192161
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
`resolve_llm` described half the job. The module also writes every sentence the user reads about a credential problem: preflight puts the message in the step output, and action.yml posts that same string as the pull request comment, the error annotation and the job summary. Naming it `llm_credentials` and saying so in the docstring keeps the rule and its explanation together, which is the point of having them in one file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three more from the Codex review, all real: - `"\n".join(...)` left foreign-envs unterminated, and `read` reports failure on an unterminated final line, so `while read` never ran its body for that record. VERCEL_BASE_URL sorts last, so an Anthropic run inherited it and core saw two providers configured. The list is terminated and the loop now handles a partial final line, either of which alone would fix it. The existing stripping test passed throughout because none of the variables it named was last, so the new one asserts the last entry specifically. - Endpoint and region are first-class inputs now, but the reusable-analysis name hashed only provider and model, so pointing `openai_base_url` at another gateway could restore a warm start built against the old one. A backend id covering the tier, provider, endpoints and region feeds the state identity. It carries no key: rotating a secret must not throw away reusable analysis, and an artifact name must never be built from one. - The README's input table still named `aws_region` after the rename. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`license_with_provider_key` could be raised and nothing asserted it, which is what happens when a failure surface is only implicit in its raise sites. The codes are declared in one frozenset now, ConfigError asserts membership, and a test walks one configuration per code and asserts the set produced is exactly the set declared, so adding a code without a case fails the build. These codes are an interface: the action emits them as `llm_config_error` and the webview keys on them, so adding one silently was a contract change nobody reviewed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agreed naming: - supported-providers.json says what the list IS, which is the same set the refusal message recites back when a value is not recognised. - credential_check.py leads with the verb, because refusing with a reason is the primary job. `resolve_llm` named only the resolving half, and the module also writes every sentence the user reads about a credential problem. - verify-credentials.sh matches the verb-noun shape of its siblings (configure-auth, fetch-state, deliver-sync, render-review) rather than being the one noun-first script in the directory. configure-auth.sh keeps its name for now, though after this change it only starts the hosted relay. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Add the secret and wire it as X" describes the fix rather than being it. The reader is often someone who has never edited a workflow, so a refusal now names the exact secrets page as a link, the exact workflow file, and the YAML to paste, indented as it will sit in the `with:` block. That needs two renderings, because the surfaces differ: `::error::` annotations cannot carry newlines, so `message` stays one plain line for the annotation while `details` carries the markdown for the pull request comment and the job summary. Only the refusals with one exact answer get a snippet. "Remove one of these" has two valid fixes, so picking one would be guessing, and it stays prose. The repository and workflow file come from GITHUB_REPOSITORY and GITHUB_WORKFLOW_REF, so off a runner the remedy degrades to prose rather than emitting a half-built link. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f476ac16
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…heck crashes Three things, all from questions the summary could not answer: - `byok+license` runs on YOUR key, always. A direct provider call never reaches CodeBoarding, so there is nothing for a licence to pay for there; it is recorded and not spent. That was already the behaviour and nothing pinned it, so a test now asserts the provider key is exported, no licence is staged, and the relay is never started. - "Tier: byok+license" names the configuration without answering the question a summary is read to answer. The summary now states which credential pays, says outright that a wired licence is not spent on a direct call, and names a non-default endpoint or region, which is the setting most likely to be wrong and least likely to be noticed. - The credential check is `continue-on-error` so a refusal can be reported before the job dies. A crash inside it -- an unreadable table, a missing python3 -- therefore left `error` empty and let the run reach the checkout and the engine install. The stop step watches the step outcome as well, so "credentials are decided before anything expensive happens" holds when the deciding breaks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17c0e8cc5d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The comment is the product, but it is not always reachable. A manual dispatch has no pull request to comment on, and a token without `pull-requests: write` cannot write one, so the diagram had nowhere to go on either. The same body now also goes to the job summary, which before this change was blank for every review run. It is free in both senses that matter: GitHub excludes logs and job summaries from the artifact storage allowance outright, and the 1MiB per-step cap is measured against diagrams of a few kilobytes (1.4KB on this repository, 2.6KB on Core's five-component graph). `continue-on-error`, because a summary that fails to write must never fail a review that succeeded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t is not there Two from the review, both mine: - `_require_id_token` checked only ACTIONS_ID_TOKEN_REQUEST_URL while the relay refuses to start without the token as well, and a runner can expose one alone. I made this reachable by removing the equivalent check from configure-auth.sh as unreachable: it was not, it covered exactly this case. So a half-configured runner passed preflight and failed generically after the engine install, which is what the check exists to prevent. - The new credential reporting said "your own OpenAI key" for endpoint-only runs that resolve no key at all: `llm: openai` with just a base URL, and every keyless ollama or litellm run. with-auth.sh strips any inherited key, so the phrase named a credential that was not there. The headline is now written once, by the check, and the shell echoes it instead of restating it. That duplication is how the two came to disagree; a test asserts the summary's phrase appears in the headline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A licensed run reported `Provider: openrouter`, which is CodeBoarding's routing decision rather than the user's configuration. It invites "why does my CodeBoarding plan say openrouter?", and it implies a commitment we have not made: the proxy's upstream can change without notice. The provider row and the `llm_provider` output are now empty on `hosted` and `license`, and populated whenever the key is the user's own, where it is their configuration and the first thing they would check. Only the reporting is withheld. The plan still resolves a provider, because the analysis has to be pointed somewhere, and the backend id still separates hosted from licensed from bring-your-own-OpenRouter, so a reusable analysis is never restored across them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 107f35bfc2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… an endpoint's credentials Two from the review, both reachable: - `_clean_key` stripped any leading `[A-Z0-9_]+=`, meant for a key pasted as `ANTHROPIC_API_KEY=sk-...`. It also ate the start of real credentials: `ABC=DEF` became `DEF`, and the base64 token `AWSKEY123=` became the empty string, which preflight then reported as a key the user had not set. That is the misleading failure this whole contract exists to remove, and Bedrock bearer tokens are base64, so it was reachable. Only the input's own variable name is stripped now. - The summary published endpoint values verbatim, and endpoints are deliberately not masked because a wrong one is the thing you most want to see. A custom endpoint is user-supplied text that can carry authentication in userinfo or a query, and the job summary renders on the run page, which is public for a public repository. Published endpoints keep their scheme, host and path, and lose anything that could be a credential. The value the analysis uses is untouched; only the reporting is trimmed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ivanmilevtues
approved these changes
Aug 27, 2026
ivanmilevtues
left a comment
ivanmilevtues
left a comment
Member
There was a problem hiding this comment.
I prompted quite a lot, it seems legfit. And we'd never leak the key nice!
| timeout-minutes: 60 | ||
| steps: | ||
| - uses: CodeBoarding/CodeBoarding-action@v2 | ||
| - uses: CodeBoarding/CodeBoarding-action@v1 |
Member
There was a problem hiding this comment.
that is really good catch, there is no v2 💀
| with: | ||
| mode: sync | ||
| # CodeBoarding's own repositories run on the CodeBoarding plan. | ||
| llm: license |
Member
There was a problem hiding this comment.
this is fine no need to change it. But tbh it sounds like it should be "provider"
Contributor
Author
There was a problem hiding this comment.
We use provider often for the LLM provider (Anthropic etc.) so provider is probably not the best word either but maybe tier? Nvm, tier doesn't quite work either. I'll keep it for now.
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
2 participants
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.
What changed
Credentials are now named, not inferred.
llmis required; each provider has its own key input.Why
The old rule was precedence: provider key if present, else licence, else hosted. An empty value was indistinguishable from "no preference", so a repo that picked Anthropic and hadn't added its secret yet ran green on our hosted OpenRouter tier - different vendor, different model, our money - and nothing in the run said so.
So a repo could report an Anthropic review that Anthropic never produced. That's the bug.
The contract
llmis requiredllm: hostedllm: hosted+ any provider keyllm: hosted+license_keyllm: licensellm: licensewithoutlicense_keyllm: license+ a provider keyllm: anthropic+anthropic_api_keyllm: anthropic, key empty/absentllm: anthropic+openai_api_keyllm: anthropic+ key +license_keybyok+license)Refusals happen before the checkout and the engine install, so a misconfigured run fails in seconds rather than after a minute of setup.
One accepted spelling per provider, and inputs are named after it, so
llm: Xalways pairs withX_api_key. No aliases: an unrecognised value is refused with the accepted list.byok+licenseis deliberately valid - "my CodeBoarding plan, my own tokens". Nothing meters it today (a BYOK run never reaches our proxy), so it is recorded and reported, not enforced.Breaking:
llm_api_key/llm_providerare gone; a workflow setting neither must addllm: hosted. Shipped asfeat:notfeat!:on purpose - a major bump would freezev1and leave every existing workflow on the old behaviour permanently. See AGENTS.md.What a run reports
Every run says what it resolved, so nobody has to infer it from behaviour. The job summary,
which was blank for review runs before this PR:
llm: license— the upstream behind our proxy is our routing decision, not yourconfiguration, so it is not named:
licensellm: anthropic+anthropic_api_key— your key, so your provider is named:byokanthropicllm: ollama+ollama_base_url— endpoint-only runs resolve no key at all, so theydo not claim one, and a non-default endpoint is shown because it is the setting most
likely to be wrong and least likely to be noticed:
byokollamaOLLAMA_BASE_URLhttp://localhost:11434With
byok+licensethe summary also states that the plan is wired and not spent: adirect provider call never reaches CodeBoarding, so your own key always pays.
Alongside that:
llm_tier,llm_provider(empty on the hosted tiers, for the reason above) andllm_config_error, which the webview reads;workflow_dispatchrun, or one whose token cannot write comments, still has a record;own secrets page and the exact YAML to paste:
The one-line form goes to the
::error::annotation, which cannot carry newlines.Files
scripts/action/supported-providers.jsonscripts/action/credential_check.pyConfigError.messageis the exact string posted on the PR.scripts/action/verify-credentials.shscripts/action/configure-auth.shscripts/action/with-auth.shVerified on a real runner
Every row is a run of this branch at
4a5209b, not a unit test. Re-run after the lastchange rather than carried over, so the links match the code being reviewed.
running on CodeBoarding's hosted tier, on your plan, thenUsing CodeBoarding hosted credentials (license). Review completed and reached the job summary.OPENROUTER_API_KEY+CODEBOARDING_LICENSE.running on your own OpenRouter key, called directly. The wired CodeBoarding plan is not spent on a direct call, thenUsing direct openrouter credentials. No relay started.skipped.The throwaway pull requests those runs came from are closed and their branches deleted.
Two live bugs these found:
ORCAROUTER_API_KEYwas missing from the hand-copied strip list(0.13.10 added it), and the strip list dropped its own last entry because
readfails on anunterminated final line, so an inherited
VERCEL_BASE_URLsurvived an Anthropic run.One thing this does not do: the engine's own debug log still prints its provider name, so
openrouterappears in a licensed run's full log. Withholding it there is a Core change,not an action one. Nothing CodeBoarding reports names it.
🤖 Generated with Claude Code