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");