feat: server-level GitHub machine credential for authenticated requests
Anonymous GitHub is capped at 60 requests/hour and cannot read private repositories, so a machine credential usable by a free (non-enterprise) account is needed to lift the request budget and reach private release assets. - Add internal/githubauth: a process-wide credential delivered via env/secret, applied by default to every outbound GitHub request. - Support two modes: a Personal Access Token sent as `Authorization: Bearer`, and a GitHub App that mints a short-lived RS256 JWT (stdlib crypto, no new dependency), exchanges it for a ~1h installation token, caches it, and single-flights a refresh a few minutes before expiry. - Inject the credential at the two GitHub call paths: the rpm github provider (releases scan + ranged asset-header GETs) and the generic byte proxy (private release-asset downloads for github.com hosts). - Honor precedence: a remote's own username/password overrides the server credential; no credential configured stays anonymous. - Fail closed at startup on partial App configuration; never persist the credential to the DB, return it from an API, or log it. - Read GITHUB_TOKEN / GITHUB_APP_ID / GITHUB_APP_INSTALLATION_ID / GITHUB_APP_PRIVATE_KEY[_PATH] via the existing getenv convention. - Document PAT vs App setup, the free-account fine-grained PAT scopes (Contents:read + Metadata:read), precedence, and the rate-limit implication.
This commit is contained in:
@@ -4,9 +4,11 @@ import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"path"
|
||||
"strings"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/githubauth"
|
||||
"git.unkin.net/unkin/artifactapi/internal/provider"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
)
|
||||
@@ -59,10 +61,42 @@ func (p *Provider) RewriteResponse(_ []byte, _ models.Remote, _ string) ([]byte,
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (p *Provider) AuthHeaders(_ context.Context, remote models.Remote) (http.Header, error) {
|
||||
// AuthHeaders authenticates outbound requests. A per-remote username/password
|
||||
// (Basic auth) takes precedence. Otherwise, when the remote points at a GitHub
|
||||
// host (e.g. a releases_remote proxying private release assets), the process-wide
|
||||
// GitHub credential is attached as a bearer token so private downloads work.
|
||||
func (p *Provider) AuthHeaders(ctx context.Context, remote models.Remote) (http.Header, error) {
|
||||
h := http.Header{}
|
||||
if remote.Username != "" {
|
||||
h.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(remote.Username+":"+remote.Password)))
|
||||
return h, nil
|
||||
}
|
||||
if isGitHubHost(remote.BaseURL) {
|
||||
if c := githubauth.Server(); c != nil {
|
||||
tok, err := c.Token(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if tok != "" {
|
||||
h.Set("Authorization", "Bearer "+tok)
|
||||
}
|
||||
}
|
||||
}
|
||||
return h, nil
|
||||
}
|
||||
|
||||
// isGitHubHost reports whether rawURL targets a GitHub API/download host that
|
||||
// accepts the server credential. objects.githubusercontent.com is deliberately
|
||||
// excluded: release-asset downloads 302-redirect there with a pre-signed URL
|
||||
// that must not carry an Authorization header.
|
||||
func isGitHubHost(rawURL string) bool {
|
||||
u, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
switch strings.ToLower(u.Hostname()) {
|
||||
case "github.com", "www.github.com", "api.github.com", "codeload.github.com", "uploads.github.com":
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -4,11 +4,56 @@ import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/githubauth"
|
||||
"git.unkin.net/unkin/artifactapi/internal/provider"
|
||||
"git.unkin.net/unkin/artifactapi/internal/provider/generic"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
)
|
||||
|
||||
type staticCred string
|
||||
|
||||
func (s staticCred) Token(context.Context) (string, error) { return string(s), nil }
|
||||
|
||||
func TestProvider_AuthHeaders_GitHubServerCredential(t *testing.T) {
|
||||
githubauth.SetServer(staticCred("ghs_server"))
|
||||
t.Cleanup(func() { githubauth.SetServer(nil) })
|
||||
|
||||
p := &generic.Provider{}
|
||||
h, err := p.AuthHeaders(context.Background(), models.Remote{BaseURL: "https://github.com"})
|
||||
if err != nil {
|
||||
t.Fatalf("auth headers: %v", err)
|
||||
}
|
||||
if h.Get("Authorization") != "Bearer ghs_server" {
|
||||
t.Fatalf("Authorization = %q, want Bearer ghs_server", h.Get("Authorization"))
|
||||
}
|
||||
}
|
||||
|
||||
func TestProvider_AuthHeaders_NonGitHubHostNoServerCredential(t *testing.T) {
|
||||
githubauth.SetServer(staticCred("ghs_server"))
|
||||
t.Cleanup(func() { githubauth.SetServer(nil) })
|
||||
|
||||
p := &generic.Provider{}
|
||||
h, _ := p.AuthHeaders(context.Background(), models.Remote{BaseURL: "https://example.com/downloads"})
|
||||
if h.Get("Authorization") != "" {
|
||||
t.Fatalf("server credential must not be sent to non-github host, got %q", h.Get("Authorization"))
|
||||
}
|
||||
}
|
||||
|
||||
func TestProvider_AuthHeaders_PerRemoteOverridesServerCredential(t *testing.T) {
|
||||
githubauth.SetServer(staticCred("ghs_server"))
|
||||
t.Cleanup(func() { githubauth.SetServer(nil) })
|
||||
|
||||
p := &generic.Provider{}
|
||||
h, _ := p.AuthHeaders(context.Background(), models.Remote{
|
||||
BaseURL: "https://github.com",
|
||||
Username: "user",
|
||||
Password: "pass",
|
||||
})
|
||||
if got := h.Get("Authorization"); got != "Basic dXNlcjpwYXNz" {
|
||||
t.Fatalf("per-remote Basic auth must win, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProvider_Type(t *testing.T) {
|
||||
p := &generic.Provider{}
|
||||
if p.Type() != models.PackageGeneric {
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
rpmlib "github.com/cavaliergopher/rpm"
|
||||
"golang.org/x/time/rate"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/githubauth"
|
||||
"git.unkin.net/unkin/artifactapi/internal/provider"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
)
|
||||
@@ -76,6 +77,10 @@ type GitHubProvider struct {
|
||||
// the shared background work queue instead of an inline per-replica scan.
|
||||
syncer *Syncer
|
||||
|
||||
// serverCred overrides the process-wide GitHub credential for this provider
|
||||
// instance. nil falls back to githubauth.Server(); set directly in tests.
|
||||
serverCred githubauth.Credential
|
||||
|
||||
mu sync.Mutex
|
||||
scanning map[string]bool
|
||||
lastScan map[string]time.Time
|
||||
@@ -136,8 +141,8 @@ func (p *GitHubProvider) RewriteResponse(_ []byte, _ models.Remote, _ string) ([
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) AuthHeaders(_ context.Context, remote models.Remote) (http.Header, error) {
|
||||
return githubHeaders(remote, false), nil
|
||||
func (p *GitHubProvider) AuthHeaders(ctx context.Context, remote models.Remote) (http.Header, error) {
|
||||
return p.githubHeaders(ctx, remote, false)
|
||||
}
|
||||
|
||||
// ServeRemote answers a request against a github_rpm remote. It refreshes the
|
||||
@@ -414,7 +419,11 @@ func (p *GitHubProvider) fetchReleases(ctx context.Context, remote models.Remote
|
||||
if err != nil {
|
||||
return nil, "", false, err
|
||||
}
|
||||
copyHeaders(req, githubHeaders(remote, true))
|
||||
hdr, err := p.githubHeaders(ctx, remote, true)
|
||||
if err != nil {
|
||||
return nil, "", false, err
|
||||
}
|
||||
copyHeaders(req, hdr)
|
||||
if page == 1 && etag != "" {
|
||||
req.Header.Set("If-None-Match", etag)
|
||||
}
|
||||
@@ -571,7 +580,11 @@ func (p *GitHubProvider) rangeGet(ctx context.Context, remote models.Remote, dow
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
copyHeaders(req, githubHeaders(remote, false))
|
||||
hdr, err := p.githubHeaders(ctx, remote, false)
|
||||
if err != nil {
|
||||
return nil, false, err
|
||||
}
|
||||
copyHeaders(req, hdr)
|
||||
req.Header.Set("Range", fmt.Sprintf("bytes=0-%d", n-1))
|
||||
|
||||
if err := p.limiterWait(ctx); err != nil {
|
||||
@@ -599,7 +612,11 @@ func (p *GitHubProvider) computeSHA256(ctx context.Context, remote models.Remote
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
copyHeaders(req, githubHeaders(remote, false))
|
||||
hdr, err := p.githubHeaders(ctx, remote, false)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
copyHeaders(req, hdr)
|
||||
|
||||
if err := p.limiterWait(ctx); err != nil {
|
||||
return "", err
|
||||
@@ -638,23 +655,48 @@ func sha256FromDigest(digest string) (string, bool) {
|
||||
return "", false
|
||||
}
|
||||
|
||||
func githubHeaders(remote models.Remote, api bool) http.Header {
|
||||
// githubHeaders builds the outbound headers for a GitHub request, attaching a
|
||||
// bearer credential when one is available. A per-remote credential wins; absent
|
||||
// that, the process-wide server credential is used; absent both, the request is
|
||||
// unauthenticated (anonymous, subject to the 60/hr cap).
|
||||
func (p *GitHubProvider) githubHeaders(ctx context.Context, remote models.Remote, api bool) (http.Header, error) {
|
||||
h := http.Header{}
|
||||
if api {
|
||||
h.Set("Accept", "application/vnd.github+json")
|
||||
h.Set("X-GitHub-Api-Version", "2022-11-28")
|
||||
}
|
||||
if tok := githubToken(remote); tok != "" {
|
||||
tok, err := p.githubToken(ctx, remote)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if tok != "" {
|
||||
h.Set("Authorization", "Bearer "+tok)
|
||||
}
|
||||
return h
|
||||
return h, nil
|
||||
}
|
||||
|
||||
func githubToken(remote models.Remote) string {
|
||||
// githubToken resolves the bearer token for a remote. Precedence: a per-remote
|
||||
// credential (password, then username) overrides the server credential.
|
||||
func (p *GitHubProvider) githubToken(ctx context.Context, remote models.Remote) (string, error) {
|
||||
if remote.Password != "" {
|
||||
return remote.Password
|
||||
return remote.Password, nil
|
||||
}
|
||||
return remote.Username
|
||||
if remote.Username != "" {
|
||||
return remote.Username, nil
|
||||
}
|
||||
if c := p.serverCredential(); c != nil {
|
||||
return c.Token(ctx)
|
||||
}
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// serverCredential returns this provider's server credential, defaulting to the
|
||||
// process-wide one installed at startup.
|
||||
func (p *GitHubProvider) serverCredential() githubauth.Credential {
|
||||
if p.serverCred != nil {
|
||||
return p.serverCred
|
||||
}
|
||||
return githubauth.Server()
|
||||
}
|
||||
|
||||
func copyHeaders(req *http.Request, h http.Header) {
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package rpm
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/githubauth"
|
||||
"git.unkin.net/unkin/artifactapi/pkg/models"
|
||||
)
|
||||
|
||||
// staticCred is a test Credential yielding a fixed token.
|
||||
type staticCred string
|
||||
|
||||
func (s staticCred) Token(context.Context) (string, error) { return string(s), nil }
|
||||
|
||||
func TestGitHubServerCredentialAttachedToReleasesAndAssets(t *testing.T) {
|
||||
fx := newGitHubFixture(t, true)
|
||||
p := newTestProvider()
|
||||
p.serverCred = staticCred("ghp_server_secret")
|
||||
store := newFakeStore()
|
||||
|
||||
if err := p.scan(context.Background(), fx.remote(), store); err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
if got := fx.releaseAuth; got != "Bearer ghp_server_secret" {
|
||||
t.Fatalf("releases Authorization = %q, want Bearer ghp_server_secret", got)
|
||||
}
|
||||
if got := fx.assetAuth; got != "Bearer ghp_server_secret" {
|
||||
t.Fatalf("asset Authorization = %q, want Bearer ghp_server_secret", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubPerRemoteCredentialOverridesServer(t *testing.T) {
|
||||
fx := newGitHubFixture(t, true)
|
||||
p := newTestProvider()
|
||||
p.serverCred = staticCred("ghp_server_secret")
|
||||
store := newFakeStore()
|
||||
|
||||
remote := fx.remote()
|
||||
remote.Password = "ghp_remote_wins"
|
||||
|
||||
if err := p.scan(context.Background(), remote, store); err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
if got := fx.releaseAuth; got != "Bearer ghp_remote_wins" {
|
||||
t.Fatalf("releases Authorization = %q, want per-remote token to win", got)
|
||||
}
|
||||
if got := fx.assetAuth; got != "Bearer ghp_remote_wins" {
|
||||
t.Fatalf("asset Authorization = %q, want per-remote token to win", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubNoCredentialSendsNoAuthHeader(t *testing.T) {
|
||||
fx := newGitHubFixture(t, true)
|
||||
p := newTestProvider() // serverCred nil, package Server() unset in unit tests
|
||||
store := newFakeStore()
|
||||
|
||||
if err := p.scan(context.Background(), fx.remote(), store); err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
if fx.releaseAuth != "" {
|
||||
t.Fatalf("expected no Authorization header, got %q", fx.releaseAuth)
|
||||
}
|
||||
if fx.assetAuth != "" {
|
||||
t.Fatalf("expected no asset Authorization header, got %q", fx.assetAuth)
|
||||
}
|
||||
// Requests still succeed anonymously.
|
||||
if rows, _ := store.ListRPMMetadataEntries(context.Background(), "acme-rpm"); len(rows) != 1 {
|
||||
t.Fatalf("anonymous scan should still derive metadata, got %d rows", len(rows))
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitHubETag304FlowWithAuth(t *testing.T) {
|
||||
fx := newGitHubFixture(t, true)
|
||||
fx.etag = `"v1"`
|
||||
p := newTestProvider()
|
||||
p.serverCred = staticCred("ghp_server_secret")
|
||||
store := newFakeStore()
|
||||
|
||||
etag, changed, err := p.scanWithState(context.Background(), fx.remote(), store, "")
|
||||
if err != nil {
|
||||
t.Fatalf("first scan: %v", err)
|
||||
}
|
||||
if !changed || etag != `"v1"` {
|
||||
t.Fatalf("first scan changed=%v etag=%q, want true and \"v1\"", changed, etag)
|
||||
}
|
||||
|
||||
// Re-scan with the captured ETag: a 304 means no change and no asset fetch.
|
||||
etag2, changed2, err := p.scanWithState(context.Background(), fx.remote(), store, etag)
|
||||
if err != nil {
|
||||
t.Fatalf("second scan: %v", err)
|
||||
}
|
||||
if changed2 {
|
||||
t.Fatal("expected no change on 304")
|
||||
}
|
||||
if etag2 != `"v1"` {
|
||||
t.Fatalf("etag = %q, want preserved \"v1\"", etag2)
|
||||
}
|
||||
if fx.notModHit != 1 {
|
||||
t.Fatalf("expected exactly one 304 response, got %d", fx.notModHit)
|
||||
}
|
||||
// The conditional request still carried the credential.
|
||||
if fx.releaseAuth != "Bearer ghp_server_secret" {
|
||||
t.Fatalf("conditional request Authorization = %q, want the server credential", fx.releaseAuth)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGitHubCredentialAbsentFromRemoteJSON asserts the server credential never
|
||||
// appears in a remote's serialized API representation, and per-remote secrets
|
||||
// stay redacted by the models.Remote json:"-" tags.
|
||||
func TestGitHubCredentialAbsentFromRemoteJSON(t *testing.T) {
|
||||
githubauth.SetServer(staticCred("ghp_super_secret_server_token"))
|
||||
t.Cleanup(func() { githubauth.SetServer(nil) })
|
||||
|
||||
remote := models.Remote{
|
||||
Name: "acme-rpm",
|
||||
PackageType: models.PackageGitHubRPM,
|
||||
BaseURL: "https://api.github.com/repos/acme/tools",
|
||||
Username: "per_remote_user",
|
||||
Password: "per_remote_secret",
|
||||
}
|
||||
b, err := json.Marshal(remote)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal remote: %v", err)
|
||||
}
|
||||
js := string(b)
|
||||
for _, secret := range []string{"ghp_super_secret_server_token", "per_remote_secret", "per_remote_user"} {
|
||||
if strings.Contains(js, secret) {
|
||||
t.Fatalf("credential %q leaked into remote JSON: %s", secret, js)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -73,6 +73,8 @@ type githubFixture struct {
|
||||
etag string // when set, served as ETag; matching If-None-Match yields 304
|
||||
releasesHit int // total releases-list requests (200 + 304)
|
||||
notModHit int // releases-list requests answered 304
|
||||
releaseAuth string // Authorization header seen on the last releases request
|
||||
assetAuth string // Authorization header seen on the last asset request
|
||||
mu sync.Mutex
|
||||
}
|
||||
|
||||
@@ -94,6 +96,7 @@ func newGitHubFixture(t *testing.T, withDigest bool) *githubFixture {
|
||||
}
|
||||
f.mu.Lock()
|
||||
f.releasesHit++
|
||||
f.releaseAuth = r.Header.Get("Authorization")
|
||||
etag := f.etag
|
||||
if etag != "" && r.Header.Get("If-None-Match") == etag {
|
||||
f.notModHit++
|
||||
@@ -130,6 +133,7 @@ func newGitHubFixture(t *testing.T, withDigest bool) *githubFixture {
|
||||
}
|
||||
rng := r.Header.Get("Range")
|
||||
f.mu.Lock()
|
||||
f.assetAuth = r.Header.Get("Authorization")
|
||||
if rng != "" {
|
||||
f.rangeHit[name]++
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user