From 9f56251b3d53ee36c1245ee2866cb39cb712fb0e Mon Sep 17 00:00:00 2001 From: Brahyam Meneses Date: Fri, 4 Sep 2026 15:47:34 +0200 Subject: [PATCH] fix(git-auth): send Basic over git, on both paths that speak it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git over HTTPS authenticates with HTTP Basic. Two paths send a service credential to a git remote and both defaulted to `Authorization: Bearer ` 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:, 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 ` through the broker and `token` 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. --- plugins/admin/public/index.html | 2 +- src/api/credential-broker.ts | 11 ++-- src/api/git-http-broker.ts | 4 +- src/skills/pack-fetcher.ts | 11 ++-- src/util/auth-header.ts | 21 +++++++ test/git-http-broker.test.ts | 48 +++++++++++++++ test/pack-fetcher.test.ts | 100 +++++++++++++++++++++++++++++++- 7 files changed, 183 insertions(+), 14 deletions(-) create mode 100644 src/util/auth-header.ts diff --git a/plugins/admin/public/index.html b/plugins/admin/public/index.html index fc63aa97e..b17fb9cd9 100644 --- a/plugins/admin/public/index.html +++ b/plugins/admin/public/index.html @@ -4747,7 +4747,7 @@

Shared service credentials

> diff --git a/src/api/credential-broker.ts b/src/api/credential-broker.ts index 7555935e4..87ebd965e 100644 --- a/src/api/credential-broker.ts +++ b/src/api/credential-broker.ts @@ -1,6 +1,7 @@ import type { CapabilityClaims } from "../auth/capability-token.ts"; import type { ScopeId } from "../types.ts"; import type { CredentialUsageSink } from "../admin/credential-usage-sink.ts"; +import { credentialAuthValue, gitAuthHeader } from "../util/auth-header.ts"; import type { DecryptedServiceCredential, ServiceCredentialReader } from "../credentials/keychain.ts"; interface BrokerFetchResponse { @@ -84,11 +85,13 @@ export function brokerPathAllowed(pathname: string, prefixes?: string[]): boolea ); } -export function brokerCredentialAuthHeader(rec: DecryptedServiceCredential): [string, string] { +function brokerCredentialAuthHeader(rec: DecryptedServiceCredential): [string, string] { const injHeader = rec.injection?.header || "Authorization"; - const rawScheme = rec.injection?.scheme ?? "Bearer "; - const injScheme = rawScheme && !/\s$/.test(rawScheme) ? `${rawScheme} ` : rawScheme; - return [injHeader, `${injScheme}${rec.secret}`]; + return [injHeader, credentialAuthValue(rec.injection?.scheme ?? "Bearer ", rec.secret)]; +} + +export function gitCredentialAuthHeader(rec: DecryptedServiceCredential): [string, string] { + return gitAuthHeader(rec.injection, rec.secret, rec.host); } export async function brokerCredentialCall(opts: { diff --git a/src/api/git-http-broker.ts b/src/api/git-http-broker.ts index 6f325cb98..adbad6c83 100644 --- a/src/api/git-http-broker.ts +++ b/src/api/git-http-broker.ts @@ -3,7 +3,7 @@ import { Readable } from "node:stream"; import { CREDENTIAL_BROKER_AUD, verifyCapabilityToken, type CapabilityClaims } from "../auth/capability-token.ts"; import { scopeId as makeScopeId } from "../types.ts"; import { type DecryptedServiceCredential, isValidCredentialSlug } from "../credentials/keychain.ts"; -import { brokerCredentialAuthHeader, brokerPathAllowed } from "./credential-broker.ts"; +import { brokerPathAllowed, gitCredentialAuthHeader } from "./credential-broker.ts"; import { CAPABILITY_HEADER } from "./contract.ts"; import { headerValue, pipeToResponse, sendJson } from "./http.ts"; import type { BaseCtx } from "./routes/route.ts"; @@ -200,7 +200,7 @@ export async function brokerGitHttp(ctx: BaseCtx): Promise { } const headers = callerHeaders(ctx); - const [authHeader, authValue] = brokerCredentialAuthHeader(rec); + const [authHeader, authValue] = gitCredentialAuthHeader(rec); headers[authHeader] = authValue; let upstreamResp: GitHttpFetchResponse; diff --git a/src/skills/pack-fetcher.ts b/src/skills/pack-fetcher.ts index f1645cb9d..06ef964d8 100644 --- a/src/skills/pack-fetcher.ts +++ b/src/skills/pack-fetcher.ts @@ -6,6 +6,7 @@ import { join, relative, sep } from "node:path"; import { lookup as dnsLookup } from "node:dns/promises"; import { isIP } from "node:net"; import { errMessage } from "../util/errors.ts"; +import { gitAuthHeader } from "../util/auth-header.ts"; import { isPrivateNetworkIp } from "../util/network.ts"; import { isProbablyBinary } from "./seed.ts"; import type { FetchedRepo, RepoFile } from "./ingest.ts"; @@ -129,14 +130,16 @@ export async function resolvePackAuth( ) { throw new Error(`skill pack credential is not authorized for ${repo.pathname}`); } - const header = credential.injection?.header?.trim() || "Authorization"; - const scheme = credential.injection?.scheme ?? "Bearer "; - return { header, value: `${scheme}${credential.secret}`, secret: credential.secret }; + const [header, value] = gitAuthHeader(credential.injection, credential.secret, credential.host); + return { header, value, secret: credential.secret }; } const host = connectorHostFor(pack.url); if (host) { const token = await sources.connectorToken(host, pack.createdBy); - if (token) return { header: "Authorization", value: `Bearer ${token}`, secret: token }; + if (token) { + const [tokenHeader, tokenValue] = gitAuthHeader(undefined, token, host); + return { header: tokenHeader, value: tokenValue, secret: token }; + } } return undefined; } diff --git a/src/util/auth-header.ts b/src/util/auth-header.ts new file mode 100644 index 000000000..29974992f --- /dev/null +++ b/src/util/auth-header.ts @@ -0,0 +1,21 @@ +const BASIC_USERNAME_BY_HOST = new Map([["bitbucket.org", "x-token-auth"]]); + +const DEFAULT_BASIC_USERNAME = "x-access-token"; + +export function credentialAuthValue(scheme: string, secret: string): string { + return `${scheme && !/\s$/.test(scheme) ? `${scheme} ` : scheme}${secret}`; +} + +export function gitAuthHeader( + injection: { header?: string; scheme?: string } | undefined, + secret: string, + host: string, +): [string, string] { + const header = injection?.header?.trim() || "Authorization"; + const scheme = injection?.scheme; + if (scheme !== undefined || header.toLowerCase() !== "authorization") { + return [header, credentialAuthValue(scheme ?? "Bearer ", secret)]; + } + const user = BASIC_USERNAME_BY_HOST.get(host.toLowerCase().replace(/^www\./, "")) ?? DEFAULT_BASIC_USERNAME; + return [header, `Basic ${Buffer.from(`${user}:${secret}`, "utf8").toString("base64")}`]; +} diff --git a/test/git-http-broker.test.ts b/test/git-http-broker.test.ts index f781ade5d..c11cdddc3 100644 --- a/test/git-http-broker.test.ts +++ b/test/git-http-broker.test.ts @@ -148,6 +148,54 @@ test("git HTTP broker streams a smart-HTTP request through the pinned service cr }); }); +test("git HTTP broker sends a raw token as Basic, not Bearer", async () => { + let seen: { headers: Record } | undefined; + const deps: ServerDeps = { + control: {} as ServerDeps["control"], + serviceCreds: { + getServiceCredentialSecret: async () => ({ + slug: "gitlab", + name: "GitLab git", + secret: "ghp_rawtoken", + host: "gitlab.example", + allowedMethods: ["GET", "POST"], + allowedPathPrefixes: ["/acme/repo.git"], + enabled: true, + }), + } as unknown as ServerDeps["serviceCreds"], + gitHttpFetch: async (url, init) => { + seen = { headers: init.headers }; + return { + status: 200, + headers: { "content-type": "application/x-git-upload-pack-advertisement" }, + body: Readable.from(["0000"]), + }; + }, + }; + const c = await ctx("/v1/credentials/git/gitlab/acme/repo.git/info/refs?service=git-upload-pack", "GET", deps); + c.req.headers["x-agent-capability"] = await mintCapabilityToken( + { + actorId: "B-LEGACY", + scopeId: "channel:C1", + aud: CREDENTIAL_BROKER_AUD, + credentials: ["gitlab"], + botActor: true, + liveActor: true, + members: [{ id: "B-LEGACY", type: "internal" }], + exp: Date.now() + CAPABILITY_TTL_MS, + }, + SECRET, + ); + c.app.authorizesCapabilityScope = async () => true; + await brokerGitHttp(c); + + assert.equal(c.res.statusCode, 200); + assert.equal( + seen?.headers.Authorization, + `Basic ${Buffer.from("x-access-token:ghp_rawtoken", "utf8").toString("base64")}`, + ); +}); + test("git HTTP broker rejects the wrong audience before contacting upstream", async () => { const deps: ServerDeps = { control: {} as ServerDeps["control"], diff --git a/test/pack-fetcher.test.ts b/test/pack-fetcher.test.ts index 0312852a8..b6bff9cb8 100644 --- a/test/pack-fetcher.test.ts +++ b/test/pack-fetcher.test.ts @@ -142,14 +142,108 @@ test("resolveRef rejects an arg-smuggling ref before invoking git", async () => ); }); -test("resolvePackAuth: an explicit host-bound slug honors the configured injection", async () => { +const basic = (secret: string, user = "x-access-token") => + `Basic ${Buffer.from(`${user}:${secret}`, "utf8").toString("base64")}`; + +test("resolvePackAuth: a slug with no injection configured authenticates the way git over HTTPS does", async () => { const sources = { serviceCredential: async (s: string) => ({ secret: "svc:" + s, host: "github.com", enabled: true }), connectorToken: async () => "connector", }; assert.deepEqual( await resolvePackAuth(sources, { url: "https://github.com/o/r", authCredentialSlug: "dep", createdBy: "u1" }), - { header: "Authorization", value: "Bearer svc:dep", secret: "svc:dep" }, + { header: "Authorization", value: basic("svc:dep"), secret: "svc:dep" }, + ); +}); + +test("resolvePackAuth: an explicit host-bound slug honors the configured injection", async () => { + const sources = { + serviceCredential: async (s: string) => ({ + secret: "svc:" + s, + host: "github.com", + enabled: true, + injection: { header: "X-Auth", scheme: "token " }, + }), + connectorToken: async () => "connector", + }; + assert.deepEqual( + await resolvePackAuth(sources, { url: "https://github.com/o/r", authCredentialSlug: "dep", createdBy: "u1" }), + { header: "X-Auth", value: "token svc:dep", secret: "svc:dep" }, + ); +}); + +test("resolvePackAuth: a configured scheme is separated from the secret whether or not it was padded", async () => { + const withScheme = (scheme: string) => ({ + serviceCredential: async (slug: string) => ({ + secret: "svc:" + slug, + host: "github.com", + enabled: true, + injection: { scheme }, + }), + connectorToken: async () => "connector", + }); + const pack = { url: "https://github.com/o/r", authCredentialSlug: "dep", createdBy: "u1" }; + + assert.equal((await resolvePackAuth(withScheme("token "), pack))!.value, "token svc:dep"); + assert.equal( + (await resolvePackAuth(withScheme("token"), pack))!.value, + "token svc:dep", + "an unpadded prefix gets the separator, as it does through the credential broker", + ); +}); + +test("resolvePackAuth: a custom header keeps the scheme-prefixed form, because Basic is only meaningful on Authorization", async () => { + const sources = { + serviceCredential: async (slug: string) => ({ + secret: "svc:" + slug, + host: "gitlab.example", + enabled: true, + injection: { header: "PRIVATE-TOKEN" }, + }), + connectorToken: async () => "connector", + }; + assert.deepEqual( + await resolvePackAuth(sources, { + url: "https://gitlab.example/o/r", + authCredentialSlug: "dep", + createdBy: "u1", + }), + { header: "PRIVATE-TOKEN", value: "Bearer svc:dep", secret: "svc:dep" }, + ); +}); + +test("resolvePackAuth: an empty configured scheme on a custom header sends the secret alone", async () => { + const sources = { + serviceCredential: async (s: string) => ({ + secret: "svc:" + s, + host: "gitlab.example", + enabled: true, + injection: { header: "PRIVATE-TOKEN", scheme: "" }, + }), + connectorToken: async () => "connector", + }; + assert.deepEqual( + await resolvePackAuth(sources, { + url: "https://gitlab.example/o/r", + authCredentialSlug: "dep", + createdBy: "u1", + }), + { header: "PRIVATE-TOKEN", value: "svc:dep", secret: "svc:dep" }, + ); +}); + +test("resolvePackAuth: the Basic username follows the host where the host demands one", async () => { + const sources = { + serviceCredential: async (s: string) => ({ secret: "svc:" + s, host: "bitbucket.org", enabled: true }), + connectorToken: async () => "connector", + }; + assert.deepEqual( + await resolvePackAuth(sources, { + url: "https://bitbucket.org/o/r", + authCredentialSlug: "dep", + createdBy: "u1", + }), + { header: "Authorization", value: basic("svc:dep", "x-token-auth"), secret: "svc:dep" }, ); }); @@ -210,7 +304,7 @@ test("resolvePackAuth: a github repo with no slug reuses the registrant's connec }; assert.deepEqual(await resolvePackAuth(sources, { url: "https://github.com/o/r.git", createdBy: "alice" }), { header: "Authorization", - value: "Bearer ghtok", + value: basic("ghtok"), secret: "ghtok", }); assert.deepEqual(calls, [["api.github.com", "alice"]], "looks up the registrant's api.github.com connector token");