fix(git-auth): send Basic over git, on both paths that speak it - #938
Open
brahyam wants to merge 1 commit into
Open
fix(git-auth): send Basic over git, on both paths that speak it#938brahyam wants to merge 1 commit into
brahyam wants to merge 1 commit into
Conversation
Git over HTTPS authenticates with HTTP Basic. Two paths send a service credential to a git remote and both defaulted to `Authorization: Bearer <secret>` when the credential configures no injection scheme: resolvePackAuth, cloning a skill pack, and brokerGitHttp, proxying git smart-HTTP for a sandboxed agent. GitHub answers both with `remote: invalid credentials`. They read the same org-scope credential rows, so one slug could not serve both correctly under any configuration — and fixing only the fetcher would have left an operator storing a raw token that clones a pack and fails the agent's own clone, one function away. The rule is now src/util/auth-header.ts and both call it. The secret goes in the PASSWORD half with a placeholder username, which is what every host documenting git-over-HTTPS token auth expects: GitHub's own actions/checkout sends x-access-token:<token>, GitLab and Azure DevOps take any username with the token as the password, Bitbucket requires x-token-auth. Putting the secret in the username half works on GitHub and fails on the other three. Bitbucket is the one host demanding a particular username rather than accepting any, so it is the one entry in a host map. Verified against GitHub on a live private repository; the other rows are the hosts' documentation and were not exercised. The default is gated on the Authorization header, because injection.header can name a custom one — PRIVATE-TOKEN, which the admin form produces when the prefix field is blank — where a Basic value is meaningless. A configured scheme is separated from the secret the way the broker already separated it: that rule was duplicated, one copy appending a space and one concatenating bare, so a single credential sent `token <secret>` through the broker and `token<secret>` through the fetcher. brokerCredentialAuthHeader keeps Bearer for the generic HTTP it proxies and is no longer exported, the git broker having been its only outside caller. This changes the wire format for credentials that configure no scheme and are used over git; a host that accepted Bearer for git, as Gitea and Forgejo do, now receives Basic, and injection.scheme restores the old value exactly. Any credential already setting one is untouched, including the pre-encoded secret plus `Basic ` pairs the broker's own tests use. The admin hint for that field said "blank = Bearer" and now says "blank = Bearer, or Basic over git" — text only, inside an existing hint span. One existing test was named for honouring a configured injection while asserting a credential that configured none, so it pinned the broken default under a name that said otherwise.
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.
What
Git over HTTPS authenticates with HTTP Basic. Two paths in this repository send a service credential to a git remote, and both defaulted to
Authorization: Bearer <secret>when the credential configures noinjection.scheme:resolvePackAuth— cloning a skill packbrokerGitHttp— proxying git smart-HTTP for a sandboxed agentGitHub answers both with
remote: invalid credentials. They read the same org-scope credential rows, so one slug could not serve both correctly under any single configuration.Both now share one rule, in
src/util/auth-header.ts:Why this shape
The secret goes in the password half with a placeholder username, which is what every host documenting git-over-HTTPS token auth expects:
x-access-token:<token>actions/checkoutsends:<PAT>— empty username, PAT as passwordx-token-auth:<token>Putting the secret in the username half works on GitHub and fails on the other three. Bitbucket is the one host that requires a specific username rather than accepting any, so it is the one entry in a host map; everything else gets
x-access-token.Verified against GitHub over HTTPS on a live private repository:
Beareris refused withremote: invalid credentials,tokenis refused, Basic clones. The other three rows are the hosts' own documentation and were not exercised.The default is gated on the
Authorizationheader. A credential can name a custom one —PRIVATE-TOKEN, which the admin form produces whenever the prefix field is left blank — and a Basic value is meaningless there, so a custom header keeps the previous scheme-prefixed behaviour unchanged.A configured scheme is separated from the secret the way the broker already separates it.
brokerCredentialAuthHeaderappends a space when the prefix does not end in whitespace; the fetcher concatenated bare, so one stored credential senttoken <secret>through the broker andtoken<secret>through the fetcher. That rule is now shared rather than duplicated.The generic broker path (
brokerCredentialCall) keepsBearer, which is correct for the HTTP APIs it proxies. Only the two git paths change.Compatibility
This changes the wire format for credentials that configure no scheme and are used over git. A host that accepted Bearer for git — Gitea and Forgejo do — now receives Basic;
injection.schemerestores the previous value exactly, and any credential that already sets one is untouched, including the pre-encodedsecret+scheme: "Basic "pairs the broker's own tests use.The admin's hint for that field said "blank = Bearer", now "blank = Bearer, or Basic over git" — text-only inside an existing
<span class="hint">, no layout or behaviour attached, which is why no screenshot accompanies it.Verified
test/pack-fetcher.test.tscovers the unconfigured default, a host-specific Basic username, an explicitly configured header and scheme, a configured scheme with and without trailing whitespace, an empty scheme on a custom header, and a custom header with none.test/git-http-broker.test.tscovers a raw token arriving as Basic rather than Bearer on the proxy path.One existing pack test was named "honors the configured injection" while asserting a credential that configured none — it now configures one, and the default has its own case.
npm run typecheck, lint,prettier --check,lint:knip, and the pack-fetcher, credential-broker, git-http-broker, keychain and service-credential-route suites pass.Related: #939 changes a different function in the same file (
src/skills/pack-fetcher.ts) and adds cases to the same test file. Each stands alone; whichever lands second needs a rebase.