From ca8c7f2b8c46821cae61908f627c8a9e39afd687 Mon Sep 17 00:00:00 2001 From: Dinesh Pandiyan Date: Sun, 25 Oct 2026 14:53:24 +1100 Subject: [PATCH] feat: GitHub App connect, follows, webhooks and reconcile - internal/github: app JWT, installation tokens, OAuth, REST, webhook parsing - internal/githubsync: account-bound OAuth, sealed tokens, follow access rules, delivery dedupe, coalesced refresh intent, reconcile and resync - schema 12 integration tables; session-only management routes - DIFFX_SERVER_GITHUB_* config, all-or-nothing, secrets env-only --- cmd/diffx-server/main.go | 7 + docs/architecture.md | 5 + docs/system.md | 33 ++ internal/config/server.go | 72 +++ internal/config/server_test.go | 50 +++ internal/github/app.go | 148 ++++++ internal/github/client.go | 269 +++++++++++ internal/github/github_test.go | 643 +++++++++++++++++++++++++++ internal/github/oauth.go | 142 ++++++ internal/github/pulls.go | 169 +++++++ internal/github/webhook.go | 114 +++++ internal/githubsync/seal.go | 47 ++ internal/githubsync/service.go | 533 ++++++++++++++++++++++ internal/githubsync/service_test.go | 388 ++++++++++++++++ internal/githubsync/store.go | 71 +++ internal/remoteserver/github.go | 211 +++++++++ internal/remoteserver/github_test.go | 198 +++++++++ internal/remoteserver/identity.go | 13 + internal/remoteserver/server.go | 35 +- internal/reviewstore/github.go | 213 +++++++++ internal/reviewstore/reset_test.go | 35 +- internal/reviewstore/store.go | 7 +- packages/api/openapi.yaml | 245 ++++++++++ packages/api/src/schema.d.ts | 363 +++++++++++++++ tools/check-boundaries/main.go | 9 +- 25 files changed, 4006 insertions(+), 14 deletions(-) create mode 100644 internal/github/app.go create mode 100644 internal/github/client.go create mode 100644 internal/github/github_test.go create mode 100644 internal/github/oauth.go create mode 100644 internal/github/pulls.go create mode 100644 internal/github/webhook.go create mode 100644 internal/githubsync/seal.go create mode 100644 internal/githubsync/service.go create mode 100644 internal/githubsync/service_test.go create mode 100644 internal/githubsync/store.go create mode 100644 internal/remoteserver/github.go create mode 100644 internal/remoteserver/github_test.go create mode 100644 internal/reviewstore/github.go diff --git a/cmd/diffx-server/main.go b/cmd/diffx-server/main.go index ce6e6272..4cdb303b 100644 --- a/cmd/diffx-server/main.go +++ b/cmd/diffx-server/main.go @@ -79,6 +79,13 @@ func parseSettings(arguments []string, stderr io.Writer) (remoteserver.Settings, if _, err := remoteserver.Retention(settings.RetentionDays); err != nil { return settings, false, err } + if err := values.GitHub.Validate(settings.InsecureCookies); err != nil { + return settings, false, err + } + if github := values.GitHub; github.Enabled() { + settings.GitHub = &remoteserver.GitHubSettings{AppID: github.AppID, ClientID: github.ClientID, ClientSecret: github.ClientSecret, + PrivateKeyFile: github.PrivateKeyFile, WebhookSecret: github.WebhookSecret, TokenKey: github.TokenKey, PublicURL: github.PublicURL} + } return settings, false, nil } func main() { diff --git a/docs/architecture.md b/docs/architecture.md index 055c137a..322a09fd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -72,6 +72,11 @@ personal lifecycle discovery. - `reviewdata`: storage-independent identities, results and errors. - `reviewsession`: a resolved review and its source/policy capabilities. - `identityservice`: account, password, session and API-token policies. +- `github`: GitHub wire adapter: app JWT, installation tokens, OAuth web flow, + REST reads, webhook signatures and payloads. No storage or diffx policy. +- `githubsync`: GitHub integration rules: which accounts may follow which + repositories, sealed user tokens, webhook deduplication, coalesced pull + request refresh intent and reconciliation. Remote composition only. - `reviewstore`: SQLite transactions, identity persistence, stored reads, retention and queue adapter. - `ingestionqueue`: admission/lease contract and worker failure policy; jobs fail diff --git a/docs/system.md b/docs/system.md index 6c01cc26..aeaa7c78 100644 --- a/docs/system.md +++ b/docs/system.md @@ -124,6 +124,39 @@ Transient ingestion notifications remain hints; clients reload durable catalog state after reconnecting. Queue workers may execute out of order, but stream freshness follows collection time and acceptance sequence. +### GitHub pull requests + +The remote server optionally integrates a GitHub App, enabled only when every +`DIFFX_SERVER_GITHUB_*` setting is present; secrets come from the environment +only. Local composition has no GitHub routes. + +Access is the intersection of the GitHub user and the app installation. An +account connects GitHub in Settings through the app's OAuth web flow; the +authorization state is bound to the starting account and consumed once, and the +callback, a cross-site top-level navigation, completes only for that account's +session. User access and refresh tokens are sealed with AES-GCM under +`DIFFX_SERVER_GITHUB_TOKEN_KEY`, bound to the account. An account may follow +only repositories GitHub lists for that user's installations. Listing, +following, Resync and startup reconciliation recheck access; delivery to a +follower rechecks it when its last verification is older than eight hours. +Lost access, uninstalls, removed repositories, revoked authorizations and +rejected refreshes mark follows lost; lost follows receive nothing until access +is verified again. GitHub management routes need a browser session and CSRF +token, like account management, because they handle GitHub credentials. + +Webhooks arrive at `POST /api/v2/github/webhooks`, authenticated only by the +`X-Hub-Signature-256` HMAC, verified before parsing. A delivery ID is recorded +only after its effects apply, and recorded IDs are acknowledged without effect, +so GitHub redeliveries are safe; every effect is idempotent. Pull request events +for actively followed repositories coalesce into one refresh intent per pull +request by advancing its generation. GitHub never retries failed deliveries, so +the server reconciles once at startup and on Settings Resync: it requests +refreshes for every open pull request of each followed repository and for +previously open ones missing from GitHub's list, which closed without a +delivery. Resync's run identity makes the resulting submissions fresh, extending +retention of pull requests idle since their last change; startup +reconciliation and webhooks do not. There is no periodic poll. + ### Browse and review The comparison selector reads scopes of the open observation. It does not diff --git a/internal/config/server.go b/internal/config/server.go index 811675e8..c33f4d44 100644 --- a/internal/config/server.go +++ b/internal/config/server.go @@ -2,13 +2,16 @@ package config import ( "bytes" + "encoding/base64" "encoding/json" "errors" "fmt" "io" "net" + "net/url" "os" "strconv" + "strings" ) // DefaultServerPort keeps diffx-server off the local `diffx [PATH]` range (7981-7990) @@ -24,6 +27,62 @@ type ServerValues struct { SignupEnabled bool `json:"signupEnabled"` InsecureCookies bool `json:"insecureCookies"` RetentionDays int `json:"retentionDays"` + GitHub GitHub `json:"github"` +} + +// GitHub configures the optional GitHub App integration. Secrets come only from +// DIFFX_SERVER_GITHUB_* environment variables, never the config file. +type GitHub struct { + AppID int64 `json:"appId"` + ClientID string `json:"clientId"` + PrivateKeyFile string `json:"privateKeyFile"` + PublicURL string `json:"publicUrl"` + ClientSecret string `json:"-"` + WebhookSecret string `json:"-"` + TokenKey string `json:"-"` +} + +// Enabled reports whether any GitHub setting is present; Validate requires all. +func (github GitHub) Enabled() bool { + return github != GitHub{} +} + +// Validate requires the complete integration or none of it. publicUrl is the +// browser-facing origin for OAuth callbacks; plain HTTP is development only. +func (github GitHub) Validate(insecureCookies bool) error { + if !github.Enabled() { + return nil + } + missing := make([]string, 0) + for _, field := range []struct { + name string + present bool + }{ + {"DIFFX_SERVER_GITHUB_APP_ID", github.AppID > 0}, + {"DIFFX_SERVER_GITHUB_CLIENT_ID", github.ClientID != ""}, + {"DIFFX_SERVER_GITHUB_PRIVATE_KEY_FILE", github.PrivateKeyFile != ""}, + {"DIFFX_SERVER_GITHUB_PUBLIC_URL", github.PublicURL != ""}, + {"DIFFX_SERVER_GITHUB_CLIENT_SECRET", github.ClientSecret != ""}, + {"DIFFX_SERVER_GITHUB_WEBHOOK_SECRET", github.WebhookSecret != ""}, + {"DIFFX_SERVER_GITHUB_TOKEN_KEY", github.TokenKey != ""}, + } { + if !field.present { + missing = append(missing, field.name) + } + } + if len(missing) > 0 { + return fmt.Errorf("incomplete GitHub integration; also set %s", strings.Join(missing, ", ")) + } + public, err := url.Parse(github.PublicURL) + if err != nil || public.Host == "" || public.User != nil || public.RawQuery != "" || public.Fragment != "" || strings.TrimSuffix(public.Path, "/") != "" || + public.Scheme != "https" && (public.Scheme != "http" || !insecureCookies) { + return errors.New("GitHub publicUrl must be an https origin such as https://diffx.example.com (http only with insecure cookies)") + } + key, err := base64.StdEncoding.DecodeString(github.TokenKey) + if err != nil || len(key) != 32 { + return errors.New("DIFFX_SERVER_GITHUB_TOKEN_KEY must be 32 random bytes in standard base64, such as the output of: openssl rand -base64 32") + } + return nil } // LoadServerFile resolves defaults, an explicitly selected file, then environment. @@ -59,6 +118,12 @@ func LoadServerFile(path string) (ServerValues, error) { {"DIFFX_SERVER_HOST", &values.Host}, {"DIFFX_SERVER_STATE", &values.State}, {"DIFFX_SERVER_USERNAME", &values.Username}, + {"DIFFX_SERVER_GITHUB_CLIENT_ID", &values.GitHub.ClientID}, + {"DIFFX_SERVER_GITHUB_PRIVATE_KEY_FILE", &values.GitHub.PrivateKeyFile}, + {"DIFFX_SERVER_GITHUB_PUBLIC_URL", &values.GitHub.PublicURL}, + {"DIFFX_SERVER_GITHUB_CLIENT_SECRET", &values.GitHub.ClientSecret}, + {"DIFFX_SERVER_GITHUB_WEBHOOK_SECRET", &values.GitHub.WebhookSecret}, + {"DIFFX_SERVER_GITHUB_TOKEN_KEY", &values.GitHub.TokenKey}, } { if value, exists := os.LookupEnv(field.name); exists { *field.target = value @@ -79,6 +144,13 @@ func LoadServerFile(path string) (ServerValues, error) { *field.target = value } } + if raw, exists := os.LookupEnv("DIFFX_SERVER_GITHUB_APP_ID"); exists { + value, err := strconv.ParseInt(raw, 10, 64) + if err != nil { + return values, fmt.Errorf("invalid DIFFX_SERVER_GITHUB_APP_ID: %w", err) + } + values.GitHub.AppID = value + } for _, field := range []struct { name string target *bool diff --git a/internal/config/server_test.go b/internal/config/server_test.go index 12cf0429..650fe5f0 100644 --- a/internal/config/server_test.go +++ b/internal/config/server_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" ) @@ -86,3 +87,52 @@ func TestServerConfigRejectsMalformedFiles(t *testing.T) { }) } } + +func TestGitHubConfigIsAllOrNothingWithSecretsFromEnvironment(t *testing.T) { + t.Setenv("DIFFX_SERVER_CONFIG", "") + key := "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + complete := map[string]string{ + "DIFFX_SERVER_GITHUB_APP_ID": "12", "DIFFX_SERVER_GITHUB_CLIENT_ID": "client", "DIFFX_SERVER_GITHUB_PRIVATE_KEY_FILE": "/keys/app.pem", + "DIFFX_SERVER_GITHUB_PUBLIC_URL": "https://diffx.example.com", "DIFFX_SERVER_GITHUB_CLIENT_SECRET": "secret", + "DIFFX_SERVER_GITHUB_WEBHOOK_SECRET": "hook", "DIFFX_SERVER_GITHUB_TOKEN_KEY": key, + } + for name, value := range complete { + t.Setenv(name, value) + } + values, err := LoadServerFile("") + if err != nil || values.GitHub.AppID != 12 || values.GitHub.TokenKey != key || values.GitHub.Validate(false) != nil { + t.Fatalf("complete integration: %+v %v %v", values.GitHub, err, values.GitHub.Validate(false)) + } + for _, tc := range []struct{ name, value, want string }{ + {"DIFFX_SERVER_GITHUB_WEBHOOK_SECRET", "", "DIFFX_SERVER_GITHUB_WEBHOOK_SECRET"}, + {"DIFFX_SERVER_GITHUB_PUBLIC_URL", "http://diffx.example.com", "https origin"}, + {"DIFFX_SERVER_GITHUB_PUBLIC_URL", "https://diffx.example.com/app", "https origin"}, + {"DIFFX_SERVER_GITHUB_TOKEN_KEY", "short", "32 random bytes"}, + } { + t.Run(tc.name+"="+tc.value, func(t *testing.T) { + t.Setenv(tc.name, tc.value) + values, err := LoadServerFile("") + if err != nil { + t.Fatal(err) + } + if err := values.GitHub.Validate(false); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("validation %v, want %q", err, tc.want) + } + }) + } + t.Setenv("DIFFX_SERVER_GITHUB_PUBLIC_URL", "http://127.0.0.1:7980") + values, _ = LoadServerFile("") + if err := values.GitHub.Validate(true); err != nil { + t.Fatalf("development http public url with insecure cookies: %v", err) + } + path := filepath.Join(t.TempDir(), "server.json") + if err := os.WriteFile(path, []byte(`{"github":{"appId":1,"clientSecret":"in-file"}}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := LoadServerFile(path); err == nil || !strings.Contains(err.Error(), "clientSecret") { + t.Fatalf("secret accepted from config file: %v", err) + } + if (GitHub{}).Validate(false) != nil || (GitHub{}).Enabled() { + t.Fatal("absent integration must be valid and disabled") + } +} diff --git a/internal/github/app.go b/internal/github/app.go new file mode 100644 index 00000000..1aba0cd4 --- /dev/null +++ b/internal/github/app.go @@ -0,0 +1,148 @@ +package github + +import ( + "context" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "net/http" + "strconv" + "time" +) + +const tokenRefreshMargin = 5 * time.Minute + +type Token struct { + Value string + ExpiresAt time.Time +} + +// ParsePrivateKey accepts PKCS#1 ("RSA PRIVATE KEY") or PKCS#8 ("PRIVATE KEY") PEM. +func ParsePrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) { + block, _ := pem.Decode(pemBytes) + if block == nil { + return nil, errors.New("github app private key must be PEM encoded") + } + switch block.Type { + case "RSA PRIVATE KEY": + key, err := x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("invalid github app private key: %w", err) + } + return key, nil + case "PRIVATE KEY": + parsed, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("invalid github app private key: %w", err) + } + key, ok := parsed.(*rsa.PrivateKey) + if !ok { + return nil, errors.New("github app private key must be RSA") + } + return key, nil + } + return nil, fmt.Errorf("unsupported github app private key PEM type %q", block.Type) +} + +// appJWT signs an RS256 app token; iat is backdated for clock drift and exp stays under GitHub's 10m cap. +func (c *Client) appJWT(now time.Time) (string, error) { + claims, err := json.Marshal(struct { + IssuedAt int64 `json:"iat"` + ExpiresAt int64 `json:"exp"` + Issuer string `json:"iss"` + }{now.Add(-60 * time.Second).Unix(), now.Add(9 * time.Minute).Unix(), strconv.FormatInt(c.app.ID, 10)}) + if err != nil { + return "", err + } + unsigned := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`)) + "." + + base64.RawURLEncoding.EncodeToString(claims) + digest := sha256.Sum256([]byte(unsigned)) + signature, err := rsa.SignPKCS1v15(rand.Reader, c.app.PrivateKey, crypto.SHA256, digest[:]) + if err != nil { + return "", errors.New("sign github app JWT failed") + } + return unsigned + "." + base64.RawURLEncoding.EncodeToString(signature), nil +} + +func (c *Client) AppSlug(ctx context.Context) (string, error) { + jwt, err := c.appJWT(time.Now()) + if err != nil { + return "", err + } + var app struct { + Slug string `json:"slug"` + } + if _, err := c.get(ctx, c.apiURL+"/app", jwt, &app); err != nil { + return "", err + } + if app.Slug == "" { + return "", errors.New("github app response missing slug") + } + return app.Slug, nil +} + +// InstallationToken mints a token narrowed to repositoryIDs and permissions when given. +func (c *Client) InstallationToken(ctx context.Context, installationID int64, repositoryIDs []int64, permissions map[string]string) (Token, error) { + if installationID <= 0 { + return Token{}, errors.New("github installation ID is required") + } + jwt, err := c.appJWT(time.Now()) + if err != nil { + return Token{}, err + } + body := struct { + RepositoryIDs []int64 `json:"repository_ids,omitempty"` + Permissions map[string]string `json:"permissions,omitempty"` + }{repositoryIDs, permissions} + target := c.apiURL + "/app/installations/" + strconv.FormatInt(installationID, 10) + "/access_tokens" + request, err := c.newRequest(ctx, http.MethodPost, target, jwt, body) + if err != nil { + return Token{}, err + } + var response struct { + Token string `json:"token"` + ExpiresAt time.Time `json:"expires_at"` + } + if _, err := c.send(request, &response); err != nil { + return Token{}, err + } + if response.Token == "" || response.ExpiresAt.IsZero() { + return Token{}, errors.New("github installation token response incomplete") + } + return Token{Value: response.Token, ExpiresAt: response.ExpiresAt}, nil +} + +// installationBearer reuses an installation-wide token until tokenRefreshMargin before expiry. +func (c *Client) installationBearer(ctx context.Context, installationID int64) (string, error) { + c.mu.Lock() + token, ok := c.tokens[installationID] + c.mu.Unlock() + if ok && time.Until(token.ExpiresAt) > tokenRefreshMargin { + return token.Value, nil + } + token, err := c.InstallationToken(ctx, installationID, nil, nil) + if err != nil { + return "", err + } + c.mu.Lock() + c.tokens[installationID] = token + c.mu.Unlock() + return token.Value, nil +} + +// forgetRejected drops a cached token GitHub no longer accepts. +func (c *Client) forgetRejected(installationID int64, err error) error { + if errors.Is(err, ErrUnauthorized) { + c.mu.Lock() + delete(c.tokens, installationID) + c.mu.Unlock() + } + return err +} diff --git a/internal/github/client.go b/internal/github/client.go new file mode 100644 index 00000000..8359f132 --- /dev/null +++ b/internal/github/client.go @@ -0,0 +1,269 @@ +// Package github adapts GitHub App REST, OAuth and webhook wire formats. +// It holds no storage and no diffx policy. +package github + +import ( + "bytes" + "context" + "crypto/rsa" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" + "unicode/utf8" +) + +const ( + defaultAPIURL = "https://api.github.com" + defaultWebURL = "https://github.com" + apiVersion = "2022-11-28" + userAgent = "diffx" + maxResponseBytes = 10 << 20 + maxMessageBytes = 512 + pageSize = "100" +) + +var ( + // ErrUnauthorized matches 401 responses and rejected OAuth codes or refresh tokens. + ErrUnauthorized = errors.New("github: unauthorized") + ErrInvalidRepository = errors.New("github: repository must be owner/name") +) + +// Error is a non-2xx GitHub API response. Rate-limited 403s report Status 429. +type Error struct { + Status int + Message string +} + +func (err *Error) Error() string { + if err.Message == "" { + return fmt.Sprintf("github: HTTP %d", err.Status) + } + return fmt.Sprintf("github: HTTP %d: %s", err.Status, err.Message) +} + +func (err *Error) Is(target error) bool { + return target == ErrUnauthorized && err.Status == http.StatusUnauthorized +} + +type App struct { + ID int64 + ClientID string + ClientSecret string + PrivateKey *rsa.PrivateKey + WebhookSecret []byte +} + +type Option func(*Client) + +func WithAPIURL(url string) Option { + return func(client *Client) { client.apiURL = strings.TrimRight(url, "/") } +} + +// WithWebURL sets the origin of the OAuth authorize and token endpoints. +func WithWebURL(url string) Option { + return func(client *Client) { client.webURL = strings.TrimRight(url, "/") } +} + +func WithHTTPClient(httpClient *http.Client) Option { + return func(client *Client) { client.http = httpClient } +} + +type Client struct { + app App + apiURL string + webURL string + http *http.Client + mu sync.Mutex + tokens map[int64]Token +} + +func New(app App, options ...Option) (*Client, error) { + switch { + case app.ID <= 0: + return nil, errors.New("github app ID is required") + case app.ClientID == "": + return nil, errors.New("github app client ID is required") + case app.ClientSecret == "": + return nil, errors.New("github app client secret is required") + case app.PrivateKey == nil: + return nil, errors.New("github app private key is required") + case len(app.WebhookSecret) == 0: + return nil, errors.New("github app webhook secret is required") + } + app.WebhookSecret = bytes.Clone(app.WebhookSecret) + client := &Client{app: app, apiURL: defaultAPIURL, webURL: defaultWebURL, + http: &http.Client{Timeout: 30 * time.Second}, tokens: map[int64]Token{}} + for _, option := range options { + option(client) + } + if client.http == nil { + return nil, errors.New("github HTTP client is required") + } + if err := validateBaseURL(client.apiURL); err != nil { + return nil, fmt.Errorf("github API URL: %w", err) + } + if err := validateBaseURL(client.webURL); err != nil { + return nil, fmt.Errorf("github web URL: %w", err) + } + return client, nil +} + +func validateBaseURL(raw string) error { + parsed, err := url.Parse(raw) + if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" || + parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" { + return errors.New("must be an HTTP(S) URL without credentials, query, or fragment") + } + return nil +} + +func setHeaders(header http.Header, accept string) { + header.Set("Accept", accept) + header.Set("X-GitHub-Api-Version", apiVersion) + header.Set("User-Agent", userAgent) +} + +func (c *Client) newRequest(ctx context.Context, method, target, bearer string, body any) (*http.Request, error) { + var reader io.Reader + if body != nil { + data, err := json.Marshal(body) + if err != nil { + return nil, err + } + reader = bytes.NewReader(data) + } + request, err := http.NewRequestWithContext(ctx, method, target, reader) + if err != nil { + return nil, err + } + setHeaders(request.Header, "application/vnd.github+json") + if body != nil { + request.Header.Set("Content-Type", "application/json") + } + if bearer != "" { + request.Header.Set("Authorization", "Bearer "+bearer) + } + return request, nil +} + +func (c *Client) get(ctx context.Context, target, bearer string, out any) (http.Header, error) { + request, err := c.newRequest(ctx, http.MethodGet, target, bearer, nil) + if err != nil { + return nil, err + } + return c.send(request, out) +} + +// send decodes a bounded 2xx JSON response into out and returns its headers. +func (c *Client) send(request *http.Request, out any) (http.Header, error) { + response, err := c.http.Do(request) + if err != nil { + return nil, err + } + defer response.Body.Close() + data, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBytes+1)) + if err != nil { + return nil, err + } + if len(data) > maxResponseBytes { + return nil, errors.New("github response exceeds 10 MiB") + } + if response.StatusCode < 200 || response.StatusCode > 299 { + return nil, responseError(response, data) + } + if err := json.Unmarshal(data, out); err != nil { + return nil, fmt.Errorf("invalid github response: %w", err) + } + return response.Header, nil +} + +func responseError(response *http.Response, data []byte) *Error { + status := response.StatusCode + if (status == http.StatusForbidden || status == http.StatusTooManyRequests) && + (response.Header.Get("X-RateLimit-Remaining") == "0" || response.Header.Get("Retry-After") != "") { + status = http.StatusTooManyRequests + } + var body struct { + Message string `json:"message"` + } + if json.Unmarshal(data, &body) != nil { + return &Error{Status: status} + } + return &Error{Status: status, Message: bounded(body.Message)} +} + +func bounded(text string) string { + if len(text) <= maxMessageBytes { + return text + } + text = text[:maxMessageBytes] + for !utf8.ValidString(text) { + text = text[:len(text)-1] + } + return text +} + +// collect follows Link rel="next" from path until limit items are gathered. +func collect[P, T any](ctx context.Context, c *Client, bearer, path string, limit int, items func(P) []T) ([]T, error) { + all := []T{} + for target := c.apiURL + path; target != "" && len(all) < limit; { + var page P + header, err := c.get(ctx, target, bearer, &page) + if err != nil { + return nil, err + } + batch := items(page) + if len(batch) == 0 { + break + } + all = append(all, batch...) + if target, err = c.nextPage(header); err != nil { + return nil, err + } + } + if len(all) > limit { + all = all[:limit] + } + return all, nil +} + +// nextPage refuses links off the API origin so credentials stay with GitHub. +func (c *Client) nextPage(header http.Header) (string, error) { + next := nextLink(header) + if next == "" { + return "", nil + } + parsed, err := url.Parse(next) + if err != nil { + return "", errors.New("invalid github pagination link") + } + base, err := url.Parse(c.apiURL) + if err != nil || parsed.Scheme != base.Scheme || parsed.Host != base.Host { + return "", errors.New("github pagination link leaves the API origin") + } + return next, nil +} + +func nextLink(header http.Header) string { + for _, value := range header.Values("Link") { + for _, link := range strings.Split(value, ",") { + target, params, ok := strings.Cut(strings.TrimSpace(link), ";") + target = strings.TrimSpace(target) + if !ok || len(target) < 2 || target[0] != '<' || target[len(target)-1] != '>' { + continue + } + for _, param := range strings.Split(params, ";") { + if strings.TrimSpace(param) == `rel="next"` { + return target[1 : len(target)-1] + } + } + } + } + return "" +} diff --git a/internal/github/github_test.go b/internal/github/github_test.go new file mode 100644 index 00000000..a887fb4c --- /dev/null +++ b/internal/github/github_test.go @@ -0,0 +1,643 @@ +package github + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/hmac" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/hex" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "strings" + "sync/atomic" + "testing" + "time" +) + +var testKey = func() *rsa.PrivateKey { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + panic(err) + } + return key +}() + +const testSecret = "client-secret-value" + +func testClient(t *testing.T, handler http.Handler) *Client { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + client, err := New(App{ID: 42, ClientID: "Iv1.client", ClientSecret: testSecret, PrivateKey: testKey, + WebhookSecret: []byte("webhook-secret")}, WithAPIURL(server.URL+"/"), WithWebURL(server.URL)) + if err != nil { + t.Fatal(err) + } + return client +} + +func writeJSON(t *testing.T, writer http.ResponseWriter, value any) { + t.Helper() + writer.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(writer).Encode(value); err != nil { + t.Error(err) + } +} + +func tokenHandler(t *testing.T, mints *atomic.Int32, lifetime time.Duration) http.HandlerFunc { + return func(writer http.ResponseWriter, request *http.Request) { + count := mints.Add(1) + writeJSON(t, writer, map[string]string{"token": fmt.Sprintf("ghs_%d", count), + "expires_at": time.Now().Add(lifetime).UTC().Format(time.RFC3339)}) + } +} + +func TestParsePrivateKey(t *testing.T) { + pkcs8, err := x509.MarshalPKCS8PrivateKey(testKey) + if err != nil { + t.Fatal(err) + } + for name, block := range map[string]*pem.Block{ + "pkcs1": {Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(testKey)}, + "pkcs8": {Type: "PRIVATE KEY", Bytes: pkcs8}, + } { + key, err := ParsePrivateKey(pem.EncodeToMemory(block)) + if err != nil || !key.Equal(testKey) { + t.Fatalf("%s: key=%v err=%v", name, key != nil, err) + } + } + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + ecPKCS8, err := x509.MarshalPKCS8PrivateKey(ecKey) + if err != nil { + t.Fatal(err) + } + for name, input := range map[string][]byte{ + "not pem": []byte("not a key"), + "bad der": pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: []byte("junk")}), + "ec key": pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: ecPKCS8}), + "wrong pem": pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: []byte("x")}), + } { + if _, err := ParsePrivateKey(input); err == nil { + t.Fatalf("%s: expected error", name) + } + } +} + +func TestNewValidatesApp(t *testing.T) { + valid := App{ID: 1, ClientID: "c", ClientSecret: "s", PrivateKey: testKey, WebhookSecret: []byte("w")} + if _, err := New(valid); err != nil { + t.Fatal(err) + } + cases := map[string]func(*App){ + "id": func(app *App) { app.ID = 0 }, + "client": func(app *App) { app.ClientID = "" }, + "secret": func(app *App) { app.ClientSecret = "" }, + "key": func(app *App) { app.PrivateKey = nil }, + "webhook": func(app *App) { app.WebhookSecret = nil }, + } + for name, mutate := range cases { + app := valid + mutate(&app) + if _, err := New(app); err == nil { + t.Fatalf("%s: expected error", name) + } + } + for _, option := range []Option{WithAPIURL("ftp://example.com"), WithWebURL("https://u:p@github.com"), WithHTTPClient(nil)} { + if _, err := New(valid, option); err == nil { + t.Fatal("expected option validation error") + } + } +} + +func TestAppSlugSignsJWT(t *testing.T) { + var authorization string + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if request.URL.Path != "/app" || request.Header.Get("Accept") != "application/vnd.github+json" || + request.Header.Get("X-GitHub-Api-Version") != "2022-11-28" || request.Header.Get("User-Agent") != "diffx" { + t.Errorf("unexpected request %s %v", request.URL.Path, request.Header) + } + authorization = request.Header.Get("Authorization") + writeJSON(t, writer, map[string]string{"slug": "diffx-app"}) + })) + before := time.Now().Unix() + slug, err := client.AppSlug(context.Background()) + if err != nil || slug != "diffx-app" { + t.Fatalf("slug=%q err=%v", slug, err) + } + jwt, ok := strings.CutPrefix(authorization, "Bearer ") + parts := strings.Split(jwt, ".") + if !ok || len(parts) != 3 { + t.Fatalf("authorization %q", authorization) + } + var header struct{ Alg, Typ string } + var claims struct { + IssuedAt int64 `json:"iat"` + ExpiresAt int64 `json:"exp"` + Issuer string `json:"iss"` + } + decodeSegment(t, parts[0], &header) + decodeSegment(t, parts[1], &claims) + if header.Alg != "RS256" || header.Typ != "JWT" || claims.Issuer != "42" { + t.Fatalf("header=%+v claims=%+v", header, claims) + } + if claims.IssuedAt < before-61 || claims.IssuedAt > before-59 || claims.ExpiresAt-claims.IssuedAt != 600 { + t.Fatalf("claims times %+v from %d", claims, before) + } + signature, err := base64.RawURLEncoding.DecodeString(parts[2]) + if err != nil { + t.Fatal(err) + } + digest := sha256.Sum256([]byte(parts[0] + "." + parts[1])) + if err := rsa.VerifyPKCS1v15(&testKey.PublicKey, crypto.SHA256, digest[:], signature); err != nil { + t.Fatalf("signature: %v", err) + } +} + +func decodeSegment(t *testing.T, segment string, out any) { + t.Helper() + data, err := base64.RawURLEncoding.DecodeString(segment) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(data, out); err != nil { + t.Fatal(err) + } +} + +func TestVerifyWebhook(t *testing.T) { + client := testClient(t, http.NotFoundHandler()) + body := []byte(`{"action":"opened"}`) + mac := hmac.New(sha256.New, []byte("webhook-secret")) + mac.Write(body) + valid := "sha256=" + hex.EncodeToString(mac.Sum(nil)) + if !client.VerifyWebhook(body, valid) { + t.Fatal("valid signature rejected") + } + for name, signature := range map[string]string{ + "empty": "", + "no prefix": strings.TrimPrefix(valid, "sha256="), + "sha1": "sha1=" + strings.TrimPrefix(valid, "sha256="), + "bad hex": "sha256=zz", + "short": valid[:len(valid)-2], + "wrong": "sha256=" + strings.Repeat("00", 32), + } { + if client.VerifyWebhook(body, signature) { + t.Fatalf("%s: accepted", name) + } + } + if client.VerifyWebhook([]byte(`{"action":"closed"}`), valid) { + t.Fatal("tampered body accepted") + } +} + +func TestInstallationTokenRequestBody(t *testing.T) { + var bodies []string + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if request.Method != http.MethodPost || request.URL.Path != "/app/installations/7/access_tokens" || + len(strings.Split(strings.TrimPrefix(request.Header.Get("Authorization"), "Bearer "), ".")) != 3 { + t.Errorf("unexpected token request %s %s", request.Method, request.URL.Path) + } + data, err := io.ReadAll(request.Body) + if err != nil { + t.Error(err) + } + bodies = append(bodies, string(data)) + writeJSON(t, writer, map[string]string{"token": "ghs_scoped", "expires_at": "2030-01-01T00:00:00Z"}) + })) + token, err := client.InstallationToken(context.Background(), 7, []int64{1, 2}, map[string]string{"contents": "read"}) + if err != nil || token.Value != "ghs_scoped" || !token.ExpiresAt.Equal(time.Date(2030, 1, 1, 0, 0, 0, 0, time.UTC)) { + t.Fatalf("token=%+v err=%v", token, err) + } + if _, err := client.InstallationToken(context.Background(), 7, nil, nil); err != nil { + t.Fatal(err) + } + want := []string{`{"repository_ids":[1,2],"permissions":{"contents":"read"}}`, `{}`} + if !reflect.DeepEqual(bodies, want) { + t.Fatalf("bodies %q", bodies) + } +} + +func TestInstallationTokenCache(t *testing.T) { + for name, test := range map[string]struct { + lifetime time.Duration + mints int32 + }{"fresh": {time.Hour, 1}, "near expiry": {4 * time.Minute, 2}} { + t.Run(name, func(t *testing.T) { + var mints atomic.Int32 + mux := http.NewServeMux() + mux.Handle("POST /app/installations/9/access_tokens", tokenHandler(t, &mints, test.lifetime)) + mux.HandleFunc("GET /repos/octo/repo/pulls/1", func(writer http.ResponseWriter, request *http.Request) { + if want := fmt.Sprintf("Bearer ghs_%d", mints.Load()); request.Header.Get("Authorization") != want { + t.Errorf("authorization %q want %q", request.Header.Get("Authorization"), want) + } + writeJSON(t, writer, map[string]int{"number": 1}) + }) + client := testClient(t, mux) + for range 2 { + if _, err := client.PullRequest(context.Background(), 9, "octo/repo", 1); err != nil { + t.Fatal(err) + } + } + if mints.Load() != test.mints { + t.Fatalf("mints %d want %d", mints.Load(), test.mints) + } + }) + } +} + +func TestInstallationTokenForgottenAfterUnauthorized(t *testing.T) { + var mints, reads atomic.Int32 + mux := http.NewServeMux() + mux.Handle("POST /app/installations/9/access_tokens", tokenHandler(t, &mints, time.Hour)) + mux.HandleFunc("GET /repos/octo/repo/pulls/1", func(writer http.ResponseWriter, request *http.Request) { + if reads.Add(1) == 1 { + writer.WriteHeader(http.StatusUnauthorized) + writeJSON(t, writer, map[string]string{"message": "Bad credentials"}) + return + } + writeJSON(t, writer, map[string]int{"number": 1}) + }) + client := testClient(t, mux) + if _, err := client.PullRequest(context.Background(), 9, "octo/repo", 1); !errors.Is(err, ErrUnauthorized) { + t.Fatalf("err %v", err) + } + if _, err := client.PullRequest(context.Background(), 9, "octo/repo", 1); err != nil { + t.Fatal(err) + } + if mints.Load() != 2 { + t.Fatalf("mints %d", mints.Load()) + } +} + +const forkPullRequest = `{ + "number": 12, "state": "closed", "draft": false, "title": "Fix", "body": null, + "html_url": "https://github.com/octo/repo/pull/12", "merged_at": "2026-01-02T03:04:05Z", + "created_at": "2026-01-01T00:00:00Z", "updated_at": "2026-01-02T03:04:05Z", + "user": {"login": "fork-user", "id": 77, "html_url": "https://github.com/fork-user", "type": "User"}, + "head": {"label": "fork-user:fix", "ref": "fix", "sha": "aaaa", "user": {"login": "fork-user", "id": 77}, "repo": null}, + "base": {"label": "octo:main", "ref": "main", "sha": "bbbb", + "repo": {"id": 101, "name": "repo", "full_name": "octo/repo", "private": true, + "clone_url": "https://github.com/octo/repo.git", "owner": {"login": "octo", "id": 5}}} +}` + +func TestPullRequestMapsForkWithDeletedHead(t *testing.T) { + var mints atomic.Int32 + mux := http.NewServeMux() + mux.Handle("POST /app/installations/9/access_tokens", tokenHandler(t, &mints, time.Hour)) + mux.HandleFunc("GET /repos/octo/repo/pulls/12", func(writer http.ResponseWriter, request *http.Request) { + io.WriteString(writer, forkPullRequest) + }) + client := testClient(t, mux) + pull, err := client.PullRequest(context.Background(), 9, "octo/repo", 12) + if err != nil { + t.Fatal(err) + } + want := PullRequest{Number: 12, State: "closed", Merged: true, Title: "Fix", HTMLURL: "https://github.com/octo/repo/pull/12", + Author: Account{ID: 77, Login: "fork-user", HTMLURL: "https://github.com/fork-user"}, + Base: Ref{Ref: "main", SHA: "bbbb", Repository: Repository{ID: 101, FullName: "octo/repo", + CloneURL: "https://github.com/octo/repo.git", Private: true}}, + Head: Ref{Ref: "fix", SHA: "aaaa"}, + CreatedAt: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), UpdatedAt: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)} + if !reflect.DeepEqual(pull, want) { + t.Fatalf("pull\n%+v\nwant\n%+v", pull, want) + } +} + +func TestOpenPullRequestsFollowsLinks(t *testing.T) { + var mints atomic.Int32 + var server *httptest.Server + mux := http.NewServeMux() + mux.Handle("POST /app/installations/9/access_tokens", tokenHandler(t, &mints, time.Hour)) + mux.HandleFunc("GET /repos/octo/repo/pulls", func(writer http.ResponseWriter, request *http.Request) { + query := request.URL.Query() + if query.Get("state") != "open" || query.Get("per_page") != "100" { + t.Errorf("query %v", query) + } + if query.Get("page") == "2" { + writer.Header().Set("Link", `<`+server.URL+`/repos/octo/repo/pulls?state=open&per_page=100&page=1>; rel="prev"`) + io.WriteString(writer, `[{"number": 3}]`) + return + } + writer.Header().Set("Link", `<`+server.URL+`/repos/octo/repo/pulls?state=open&per_page=100&page=2>; rel="next", <`+ + server.URL+`/repos/octo/repo/pulls?state=open&per_page=100&page=2>; rel="last"`) + io.WriteString(writer, `[{"number": 1}, {"number": 2}]`) + }) + server = httptest.NewServer(mux) + t.Cleanup(server.Close) + client, err := New(App{ID: 42, ClientID: "c", ClientSecret: "s", PrivateKey: testKey, WebhookSecret: []byte("w")}, + WithAPIURL(server.URL)) + if err != nil { + t.Fatal(err) + } + pulls, err := client.OpenPullRequests(context.Background(), 9, "octo/repo") + if err != nil { + t.Fatal(err) + } + if len(pulls) != 3 || pulls[0].Number != 1 || pulls[2].Number != 3 { + t.Fatalf("pulls %+v", pulls) + } +} + +func TestOpenPullRequestsCapsResults(t *testing.T) { + var mints, pages atomic.Int32 + var server *httptest.Server + mux := http.NewServeMux() + mux.Handle("POST /app/installations/9/access_tokens", tokenHandler(t, &mints, time.Hour)) + mux.HandleFunc("GET /repos/octo/repo/pulls", func(writer http.ResponseWriter, request *http.Request) { + page := pages.Add(1) + writer.Header().Set("Link", fmt.Sprintf(`<%s/repos/octo/repo/pulls?page=%d>; rel="next"`, server.URL, page+1)) + items := make([]map[string]int, 100) + for index := range items { + items[index] = map[string]int{"number": int(page)*100 + index} + } + writeJSON(t, writer, items) + }) + server = httptest.NewServer(mux) + t.Cleanup(server.Close) + client, err := New(App{ID: 42, ClientID: "c", ClientSecret: "s", PrivateKey: testKey, WebhookSecret: []byte("w")}, + WithAPIURL(server.URL)) + if err != nil { + t.Fatal(err) + } + pulls, err := client.OpenPullRequests(context.Background(), 9, "octo/repo") + if err != nil || len(pulls) != 3000 || pages.Load() != 30 { + t.Fatalf("pulls=%d pages=%d err=%v", len(pulls), pages.Load(), err) + } +} + +func TestPaginationRejectsForeignOrigin(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("Link", `; rel="next"`) + io.WriteString(writer, `{"installations": [{"id": 1}]}`) + })) + if _, err := client.UserRepositories(context.Background(), "ghu_token"); err == nil { + t.Fatal("expected foreign pagination link error") + } +} + +func TestUserRepositories(t *testing.T) { + var server *httptest.Server + mux := http.NewServeMux() + mux.HandleFunc("GET /user/installations", func(writer http.ResponseWriter, request *http.Request) { + if request.Header.Get("Authorization") != "Bearer ghu_token" || request.URL.Query().Get("per_page") != "100" { + t.Errorf("request %v %v", request.Header, request.URL) + } + if request.URL.Query().Get("page") == "2" { + io.WriteString(writer, `{"total_count": 2, "installations": [{"id": 20, "account": {"login": "org"}}]}`) + return + } + writer.Header().Set("Link", `<`+server.URL+`/user/installations?per_page=100&page=2>; rel="next"`) + io.WriteString(writer, `{"total_count": 2, "installations": [{"id": 10, "account": {"login": "me"}}]}`) + }) + mux.HandleFunc("GET /user/installations/10/repositories", func(writer http.ResponseWriter, request *http.Request) { + io.WriteString(writer, `{"total_count": 1, "repositories": [{"id": 1, "full_name": "me/a", "clone_url": "https://github.com/me/a.git", "private": true}]}`) + }) + mux.HandleFunc("GET /user/installations/20/repositories", func(writer http.ResponseWriter, request *http.Request) { + io.WriteString(writer, `{"total_count": 1, "repositories": [{"id": 2, "full_name": "org/b", "clone_url": "https://github.com/org/b.git"}]}`) + }) + server = httptest.NewServer(mux) + t.Cleanup(server.Close) + client, err := New(App{ID: 42, ClientID: "c", ClientSecret: "s", PrivateKey: testKey, WebhookSecret: []byte("w")}, + WithAPIURL(server.URL)) + if err != nil { + t.Fatal(err) + } + repositories, err := client.UserRepositories(context.Background(), "ghu_token") + if err != nil { + t.Fatal(err) + } + want := []Repository{ + {ID: 1, FullName: "me/a", CloneURL: "https://github.com/me/a.git", Private: true, InstallationID: 10}, + {ID: 2, FullName: "org/b", CloneURL: "https://github.com/org/b.git", InstallationID: 20}, + } + if !reflect.DeepEqual(repositories, want) { + t.Fatalf("repositories %+v", repositories) + } +} + +func TestAuthorizeURL(t *testing.T) { + client := testClient(t, http.NotFoundHandler()) + parsed, err := url.Parse(client.AuthorizeURL("st&ate", "https://diffx.example/callback")) + if err != nil { + t.Fatal(err) + } + query := parsed.Query() + if parsed.Path != "/login/oauth/authorize" || query.Get("client_id") != "Iv1.client" || + query.Get("state") != "st&ate" || query.Get("redirect_uri") != "https://diffx.example/callback" { + t.Fatalf("authorize URL %s", parsed) + } +} + +func TestExchange(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if err := request.ParseForm(); err != nil { + t.Error(err) + } + if request.Method != http.MethodPost || request.URL.Path != "/login/oauth/access_token" || + request.Header.Get("Accept") != "application/json" || request.PostForm.Get("client_id") != "Iv1.client" || + request.PostForm.Get("client_secret") != testSecret || request.PostForm.Get("code") != "code-1" || + request.PostForm.Get("redirect_uri") != "https://diffx.example/callback" { + t.Errorf("unexpected exchange %s %v %v", request.URL.Path, request.Header, request.PostForm) + } + io.WriteString(writer, `{"access_token":"ghu_a","expires_in":28800,"refresh_token":"ghr_r","refresh_token_expires_in":15897600,"token_type":"bearer","scope":""}`) + })) + before := time.Now() + token, err := client.Exchange(context.Background(), "code-1", "https://diffx.example/callback") + if err != nil || token.AccessToken != "ghu_a" || token.RefreshToken != "ghr_r" { + t.Fatalf("token=%+v err=%v", token, err) + } + if lifetime := token.ExpiresAt.Sub(before); lifetime < 8*time.Hour || lifetime > 8*time.Hour+time.Minute { + t.Fatalf("expires at %v", token.ExpiresAt) + } + if lifetime := token.RefreshExpiresAt.Sub(before); lifetime < 184*24*time.Hour || lifetime > 184*24*time.Hour+time.Minute { + t.Fatalf("refresh expires at %v", token.RefreshExpiresAt) + } +} + +func TestExchangeWithoutExpiry(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + io.WriteString(writer, `{"access_token":"ghu_a","token_type":"bearer"}`) + })) + token, err := client.Exchange(context.Background(), "code", "") + if err != nil || !token.ExpiresAt.IsZero() || !token.RefreshExpiresAt.IsZero() || token.RefreshToken != "" { + t.Fatalf("token=%+v err=%v", token, err) + } +} + +func TestOAuthErrors(t *testing.T) { + for code, unauthorized := range map[string]bool{ + "bad_refresh_token": true, "bad_verification_code": true, "incorrect_client_credentials": false, + } { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + if err := request.ParseForm(); err != nil { + t.Error(err) + } + if request.PostForm.Get("grant_type") != "refresh_token" || request.PostForm.Get("refresh_token") != "ghr_old" { + t.Errorf("refresh form %v", request.PostForm) + } + writeJSON(t, writer, map[string]string{"error": code, "error_description": "nope", "error_uri": "https://docs"}) + })) + _, err := client.Refresh(context.Background(), "ghr_old") + if err == nil || errors.Is(err, ErrUnauthorized) != unauthorized { + t.Fatalf("%s: err %v", code, err) + } + if strings.Contains(err.Error(), testSecret) || strings.Contains(err.Error(), "ghr_old") { + t.Fatalf("%s: error leaks secret: %v", code, err) + } + } +} + +func TestUnauthorizedResponse(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + writer.WriteHeader(http.StatusUnauthorized) + writeJSON(t, writer, map[string]string{"message": "Bad credentials " + strings.Repeat("x", 2000)}) + })) + _, err := client.User(context.Background(), "ghu_bad") + var apiError *Error + if !errors.Is(err, ErrUnauthorized) || !errors.As(err, &apiError) || apiError.Status != 401 || + len(apiError.Message) > maxMessageBytes || !strings.HasPrefix(apiError.Message, "Bad credentials") { + t.Fatalf("err %v", err) + } +} + +func TestRateLimitResponses(t *testing.T) { + for name, test := range map[string]struct { + status int + header map[string]string + want int + }{ + "primary": {http.StatusForbidden, map[string]string{"X-RateLimit-Remaining": "0"}, 429}, + "secondary": {http.StatusForbidden, map[string]string{"Retry-After": "60"}, 429}, + "429": {http.StatusTooManyRequests, nil, 429}, + "forbidden": {http.StatusForbidden, map[string]string{"X-RateLimit-Remaining": "4999"}, 403}, + } { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + for key, value := range test.header { + writer.Header().Set(key, value) + } + writer.WriteHeader(test.status) + io.WriteString(writer, `{"message":"API rate limit exceeded"}`) + })) + _, err := client.User(context.Background(), "ghu") + var apiError *Error + if !errors.As(err, &apiError) || apiError.Status != test.want || errors.Is(err, ErrUnauthorized) { + t.Fatalf("%s: err %v", name, err) + } + } +} + +func TestOversizeResponse(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + io.WriteString(writer, `{"login":"`+strings.Repeat("a", maxResponseBytes)+`"}`) + })) + if _, err := client.User(context.Background(), "ghu"); err == nil || !strings.Contains(err.Error(), "exceeds") { + t.Fatalf("err %v", err) + } +} + +func TestCancelledContext(t *testing.T) { + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + writeJSON(t, writer, map[string]any{"id": 1, "login": "me"}) + })) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := client.User(ctx, "ghu"); !errors.Is(err, context.Canceled) { + t.Fatalf("err %v", err) + } +} + +func TestRepositoryPath(t *testing.T) { + for fullName, want := range map[string]string{ + "octo/repo": "/repos/octo/repo", + "Octo-1/re.po_x": "/repos/Octo-1/re.po_x", + "octo": "", + "octo/repo/pulls": "", + "/repo": "", + "octo/": "", + "../repo": "", + "octo/..": "", + "octo/.": "", + "octo/re po": "", + "octo/re%2Fpo": "", + "octo/repo?x=1": "", + } { + path, err := repositoryPath(fullName) + if want == "" { + if !errors.Is(err, ErrInvalidRepository) { + t.Fatalf("%q: path=%q err=%v", fullName, path, err) + } + continue + } + if err != nil || path != want { + t.Fatalf("%q: path=%q err=%v", fullName, path, err) + } + } + client := testClient(t, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + t.Errorf("unexpected request %s", request.URL) + })) + if _, err := client.OpenPullRequests(context.Background(), 1, "a/b/c"); !errors.Is(err, ErrInvalidRepository) { + t.Fatalf("err %v", err) + } +} + +func TestParseEvent(t *testing.T) { + cases := map[string]struct { + event string + body string + want any + }{ + "fork pull request": {"pull_request", `{"action":"synchronize","number":12, + "pull_request":` + forkPullRequest + `, + "repository":{"id":101,"name":"repo","full_name":"octo/repo","private":true,"clone_url":"https://github.com/octo/repo.git"}, + "installation":{"id":9,"node_id":"x"},"sender":{"login":"fork-user","id":77}}`, + PullRequestEvent{Action: "synchronize", InstallationID: 9, Number: 12, Repository: Repository{ID: 101, + FullName: "octo/repo", CloneURL: "https://github.com/octo/repo.git", Private: true, InstallationID: 9}}}, + "installation": {"installation", `{"action":"deleted","installation":{"id":9,"account":{"login":"octo","id":5}, + "app_id":42},"repositories":[{"id":101,"full_name":"octo/repo","private":true}],"sender":{"id":5}}`, + InstallationEvent{Action: "deleted", InstallationID: 9}}, + "installation repositories": {"installation_repositories", `{"action":"added","installation":{"id":9}, + "repository_selection":"selected", + "repositories_added":[{"id":102,"node_id":"n","name":"new","full_name":"octo/new","private":false}], + "repositories_removed":[],"sender":{"id":5}}`, + InstallationRepositoriesEvent{Action: "added", InstallationID: 9, + Added: []Repository{{ID: 102, FullName: "octo/new", InstallationID: 9}}, + Removed: []Repository{}}}, + "authorization": {"github_app_authorization", `{"action":"revoked","sender":{"login":"me","id":77,"type":"User"}}`, + AuthorizationEvent{Action: "revoked", UserID: 77}}, + "ping": {"ping", `{"zen":"Keep it logically awesome.","hook_id":1}`, nil}, + "unknown": {"issues", `not json`, nil}, + } + for name, test := range cases { + got, err := ParseEvent(test.event, []byte(test.body)) + if err != nil || !reflect.DeepEqual(got, test.want) { + t.Fatalf("%s: got=%#v err=%v", name, got, err) + } + } + for name, test := range map[string]struct{ event, body string }{ + "malformed": {"pull_request", `{`}, + "missing action": {"installation", `{"installation":{"id":9}}`}, + "missing installation": {"pull_request", `{"action":"opened","number":1,"repository":{"id":1}}`}, + "missing repository": {"pull_request", `{"action":"opened","number":1,"installation":{"id":9}}`}, + "missing sender": {"github_app_authorization", `{"action":"revoked"}`}, + } { + if _, err := ParseEvent(test.event, []byte(test.body)); err == nil { + t.Fatalf("%s: expected error", name) + } + } +} diff --git a/internal/github/oauth.go b/internal/github/oauth.go new file mode 100644 index 00000000..b13c7752 --- /dev/null +++ b/internal/github/oauth.go @@ -0,0 +1,142 @@ +package github + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "strconv" + "strings" + "time" +) + +const maxUserRepositories = 10000 + +// UserToken times are zero when GitHub omits the matching expiry. +type UserToken struct { + AccessToken string + ExpiresAt time.Time + RefreshToken string + RefreshExpiresAt time.Time +} + +func (c *Client) AuthorizeURL(state, redirectURI string) string { + query := url.Values{"client_id": {c.app.ClientID}, "redirect_uri": {redirectURI}, "state": {state}} + return c.webURL + "/login/oauth/authorize?" + query.Encode() +} + +func (c *Client) Exchange(ctx context.Context, code, redirectURI string) (UserToken, error) { + if code == "" { + return UserToken{}, errors.New("github OAuth code is required") + } + return c.userToken(ctx, url.Values{"code": {code}, "redirect_uri": {redirectURI}}) +} + +func (c *Client) Refresh(ctx context.Context, refreshToken string) (UserToken, error) { + if refreshToken == "" { + return UserToken{}, errors.New("github refresh token is required") + } + return c.userToken(ctx, url.Values{"grant_type": {"refresh_token"}, "refresh_token": {refreshToken}}) +} + +// userToken posts to the OAuth token endpoint, which reports failures as HTTP 200 {"error":...}. +func (c *Client) userToken(ctx context.Context, form url.Values) (UserToken, error) { + form.Set("client_id", c.app.ClientID) + form.Set("client_secret", c.app.ClientSecret) + request, err := http.NewRequestWithContext(ctx, http.MethodPost, c.webURL+"/login/oauth/access_token", strings.NewReader(form.Encode())) + if err != nil { + return UserToken{}, err + } + setHeaders(request.Header, "application/json") + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + now := time.Now() + var response struct { + AccessToken string `json:"access_token"` + ExpiresIn int64 `json:"expires_in"` + RefreshToken string `json:"refresh_token"` + RefreshTokenExpiresIn int64 `json:"refresh_token_expires_in"` + Error string `json:"error"` + } + if _, err := c.send(request, &response); err != nil { + return UserToken{}, err + } + switch response.Error { + case "": + case "bad_verification_code", "bad_refresh_token": + return UserToken{}, fmt.Errorf("%w: %s", ErrUnauthorized, response.Error) + default: + return UserToken{}, fmt.Errorf("github OAuth error: %s", bounded(response.Error)) + } + if response.AccessToken == "" { + return UserToken{}, errors.New("github OAuth response missing access token") + } + token := UserToken{AccessToken: response.AccessToken, RefreshToken: response.RefreshToken} + if response.ExpiresIn > 0 { + token.ExpiresAt = now.Add(time.Duration(response.ExpiresIn) * time.Second) + } + if response.RefreshTokenExpiresIn > 0 { + token.RefreshExpiresAt = now.Add(time.Duration(response.RefreshTokenExpiresIn) * time.Second) + } + return token, nil +} + +func (c *Client) User(ctx context.Context, accessToken string) (Account, error) { + var user accountJSON + if _, err := c.get(ctx, c.apiURL+"/user", accessToken, &user); err != nil { + return Account{}, err + } + if user.ID <= 0 || user.Login == "" { + return Account{}, errors.New("github user response missing identity") + } + return user.account(), nil +} + +type installationsPage struct { + Installations []struct { + ID int64 `json:"id"` + } `json:"installations"` +} + +type repositoriesPage struct { + Repositories []repositoryJSON `json:"repositories"` +} + +// UserRepositories lists repositories both the user and the app can access, +// tagged with their installation and capped at 10000. +func (c *Client) UserRepositories(ctx context.Context, accessToken string) ([]Repository, error) { + installations, err := collect(ctx, c, accessToken, "/user/installations?per_page="+pageSize, maxUserRepositories, + func(page installationsPage) []int64 { + ids := make([]int64, 0, len(page.Installations)) + for _, installation := range page.Installations { + ids = append(ids, installation.ID) + } + return ids + }) + if err != nil { + return nil, err + } + repositories := []Repository{} + for _, installationID := range installations { + remaining := maxUserRepositories - len(repositories) + if remaining <= 0 { + break + } + if installationID <= 0 { + return nil, errors.New("github installation response missing ID") + } + path := "/user/installations/" + strconv.FormatInt(installationID, 10) + "/repositories?per_page=" + pageSize + batch, err := collect(ctx, c, accessToken, path, remaining, func(page repositoriesPage) []Repository { + result := make([]Repository, len(page.Repositories)) + for index, repository := range page.Repositories { + result[index] = repository.repository(installationID) + } + return result + }) + if err != nil { + return nil, err + } + repositories = append(repositories, batch...) + } + return repositories, nil +} diff --git a/internal/github/pulls.go b/internal/github/pulls.go new file mode 100644 index 00000000..491397e3 --- /dev/null +++ b/internal/github/pulls.go @@ -0,0 +1,169 @@ +package github + +import ( + "context" + "errors" + "net/url" + "strconv" + "strings" + "time" +) + +const maxOpenPullRequests = 3000 + +type Account struct { + ID int64 + Login string + HTMLURL string +} + +// Repository.InstallationID is set only by installation-scoped listings and webhooks. +type Repository struct { + ID int64 + FullName string + CloneURL string + Private bool + InstallationID int64 +} + +type Ref struct { + Ref string + SHA string + Repository Repository +} + +// PullRequest.Head.Repository is zero when the head fork was deleted. +type PullRequest struct { + Number int + State string + Merged bool + Draft bool + Title string + Body string + HTMLURL string + Author Account + Base Ref + Head Ref + CreatedAt time.Time + UpdatedAt time.Time +} + +type accountJSON struct { + ID int64 `json:"id"` + Login string `json:"login"` + HTMLURL string `json:"html_url"` +} + +func (account accountJSON) account() Account { + return Account{ID: account.ID, Login: account.Login, HTMLURL: account.HTMLURL} +} + +type repositoryJSON struct { + ID int64 `json:"id"` + FullName string `json:"full_name"` + CloneURL string `json:"clone_url"` + Private bool `json:"private"` +} + +func (repository repositoryJSON) repository(installationID int64) Repository { + return Repository{ID: repository.ID, FullName: repository.FullName, CloneURL: repository.CloneURL, + Private: repository.Private, InstallationID: installationID} +} + +type refJSON struct { + Ref string `json:"ref"` + SHA string `json:"sha"` + Repo *repositoryJSON `json:"repo"` +} + +func (ref refJSON) ref() Ref { + result := Ref{Ref: ref.Ref, SHA: ref.SHA} + if ref.Repo != nil { + result.Repository = ref.Repo.repository(0) + } + return result +} + +type pullRequestJSON struct { + Number int `json:"number"` + State string `json:"state"` + MergedAt *time.Time `json:"merged_at"` + Draft bool `json:"draft"` + Title string `json:"title"` + Body string `json:"body"` + HTMLURL string `json:"html_url"` + User accountJSON `json:"user"` + Base refJSON `json:"base"` + Head refJSON `json:"head"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +func (pull pullRequestJSON) pullRequest() PullRequest { + return PullRequest{Number: pull.Number, State: pull.State, Merged: pull.MergedAt != nil, Draft: pull.Draft, + Title: pull.Title, Body: pull.Body, HTMLURL: pull.HTMLURL, Author: pull.User.account(), + Base: pull.Base.ref(), Head: pull.Head.ref(), CreatedAt: pull.CreatedAt, UpdatedAt: pull.UpdatedAt} +} + +func (c *Client) PullRequest(ctx context.Context, installationID int64, fullName string, number int) (PullRequest, error) { + path, err := repositoryPath(fullName) + if err != nil { + return PullRequest{}, err + } + if number <= 0 { + return PullRequest{}, errors.New("github pull request number must be positive") + } + bearer, err := c.installationBearer(ctx, installationID) + if err != nil { + return PullRequest{}, err + } + var pull pullRequestJSON + if _, err := c.get(ctx, c.apiURL+path+"/pulls/"+strconv.Itoa(number), bearer, &pull); err != nil { + return PullRequest{}, c.forgetRejected(installationID, err) + } + return pull.pullRequest(), nil +} + +// OpenPullRequests returns at most 3000 open pull requests. +func (c *Client) OpenPullRequests(ctx context.Context, installationID int64, fullName string) ([]PullRequest, error) { + path, err := repositoryPath(fullName) + if err != nil { + return nil, err + } + bearer, err := c.installationBearer(ctx, installationID) + if err != nil { + return nil, err + } + pulls, err := collect(ctx, c, bearer, path+"/pulls?state=open&per_page="+pageSize, maxOpenPullRequests, + func(page []pullRequestJSON) []pullRequestJSON { return page }) + if err != nil { + return nil, c.forgetRejected(installationID, err) + } + result := make([]PullRequest, len(pulls)) + for index, pull := range pulls { + result[index] = pull.pullRequest() + } + return result, nil +} + +// repositoryPath builds /repos/{owner}/{name} from exactly "owner/name". +func repositoryPath(fullName string) (string, error) { + owner, name, ok := strings.Cut(fullName, "/") + if !ok || !validName(owner) || !validName(name) { + return "", ErrInvalidRepository + } + return "/repos/" + url.PathEscape(owner) + "/" + url.PathEscape(name), nil +} + +func validName(name string) bool { + if name == "" || len(name) > 100 || name == "." || name == ".." { + return false + } + for _, char := range name { + if !(char >= 'a' && char <= 'z' || char >= 'A' && char <= 'Z' || char >= '0' && char <= '9' || + char == '-' || char == '_' || char == '.') { + return false + } + } + return true +} diff --git a/internal/github/webhook.go b/internal/github/webhook.go new file mode 100644 index 00000000..90d8ba84 --- /dev/null +++ b/internal/github/webhook.go @@ -0,0 +1,114 @@ +package github + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "strings" +) + +type PullRequestEvent struct { + Action string + InstallationID int64 + Repository Repository + Number int +} + +// InstallationEvent.Action is created, deleted, suspend, unsuspend or new_permissions_accepted. +type InstallationEvent struct { + Action string + InstallationID int64 +} + +type InstallationRepositoriesEvent struct { + Action string + InstallationID int64 + Added []Repository + Removed []Repository +} + +// AuthorizationEvent reports a user revoking the app (github_app_authorization). +type AuthorizationEvent struct { + Action string + UserID int64 +} + +// VerifyWebhook checks an X-Hub-Signature-256 header in constant time. +func (c *Client) VerifyWebhook(body []byte, signature256 string) bool { + encoded, ok := strings.CutPrefix(signature256, "sha256=") + if !ok { + return false + } + signature, err := hex.DecodeString(encoded) + if err != nil || len(signature) != sha256.Size { + return false + } + mac := hmac.New(sha256.New, c.app.WebhookSecret) + mac.Write(body) + return hmac.Equal(signature, mac.Sum(nil)) +} + +type webhookPayload struct { + Action string `json:"action"` + Number int `json:"number"` + Installation struct { + ID int64 `json:"id"` + } `json:"installation"` + Repository repositoryJSON `json:"repository"` + RepositoriesAdded []repositoryJSON `json:"repositories_added"` + RepositoriesRemoved []repositoryJSON `json:"repositories_removed"` + Sender struct { + ID int64 `json:"id"` + } `json:"sender"` +} + +// ParseEvent decodes a verified webhook body by X-GitHub-Event name. It returns +// PullRequestEvent, InstallationEvent, InstallationRepositoriesEvent or +// AuthorizationEvent, and (nil, nil) for events diffx ignores, including ping. +func ParseEvent(event string, body []byte) (any, error) { + switch event { + case "pull_request", "installation", "installation_repositories", "github_app_authorization": + default: + return nil, nil + } + var payload webhookPayload + if err := json.Unmarshal(body, &payload); err != nil { + return nil, fmt.Errorf("invalid %s webhook: %w", event, err) + } + if payload.Action == "" { + return nil, fmt.Errorf("invalid %s webhook: missing action", event) + } + if event == "github_app_authorization" { + if payload.Sender.ID <= 0 { + return nil, fmt.Errorf("invalid %s webhook: missing sender", event) + } + return AuthorizationEvent{Action: payload.Action, UserID: payload.Sender.ID}, nil + } + installationID := payload.Installation.ID + if installationID <= 0 { + return nil, fmt.Errorf("invalid %s webhook: missing installation", event) + } + switch event { + case "pull_request": + if payload.Number <= 0 || payload.Repository.ID <= 0 { + return nil, fmt.Errorf("invalid %s webhook: missing pull request or repository", event) + } + return PullRequestEvent{Action: payload.Action, InstallationID: installationID, + Repository: payload.Repository.repository(installationID), Number: payload.Number}, nil + case "installation": + return InstallationEvent{Action: payload.Action, InstallationID: installationID}, nil + } + return InstallationRepositoriesEvent{Action: payload.Action, InstallationID: installationID, + Added: repositories(payload.RepositoriesAdded, installationID), + Removed: repositories(payload.RepositoriesRemoved, installationID)}, nil +} + +func repositories(items []repositoryJSON, installationID int64) []Repository { + result := make([]Repository, len(items)) + for index, item := range items { + result[index] = item.repository(installationID) + } + return result +} diff --git a/internal/githubsync/seal.go b/internal/githubsync/seal.go new file mode 100644 index 00000000..21bf54f2 --- /dev/null +++ b/internal/githubsync/seal.go @@ -0,0 +1,47 @@ +package githubsync + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "errors" +) + +// sealer encrypts user tokens at rest. Associated data binds each ciphertext +// to its account and field, so rows cannot be swapped between accounts. +type sealer struct{ aead cipher.AEAD } + +func newSealer(key []byte) (sealer, error) { + if len(key) != 32 { + return sealer{}, errors.New("GitHub token key must be 32 bytes") + } + block, err := aes.NewCipher(key) + if err != nil { + return sealer{}, err + } + aead, err := cipher.NewGCM(block) + if err != nil { + return sealer{}, err + } + return sealer{aead}, nil +} + +func (s sealer) seal(ownerID, field, value string) ([]byte, error) { + nonce := make([]byte, s.aead.NonceSize()) + if _, err := rand.Read(nonce); err != nil { + return nil, err + } + return s.aead.Seal(nonce, nonce, []byte(value), []byte(ownerID+"\x00"+field)), nil +} + +func (s sealer) open(ownerID, field string, sealed []byte) (string, error) { + size := s.aead.NonceSize() + if len(sealed) < size { + return "", errors.New("sealed GitHub token is malformed") + } + plain, err := s.aead.Open(nil, sealed[:size], sealed[size:], []byte(ownerID+"\x00"+field)) + if err != nil { + return "", errors.New("sealed GitHub token cannot be opened; was the token key changed?") + } + return string(plain), nil +} diff --git a/internal/githubsync/service.go b/internal/githubsync/service.go new file mode 100644 index 00000000..04e1873c --- /dev/null +++ b/internal/githubsync/service.go @@ -0,0 +1,533 @@ +// Package githubsync owns the GitHub integration's application rules: which +// accounts may follow which repositories, webhook deduplication, coalesced +// refresh intent and reconciliation. It holds no transport or SQL. +package githubsync + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "errors" + "fmt" + "slices" + "strings" + "sync" + "time" + + "github.com/flexdinesh/diffx/internal/github" +) + +// GitHub is the adapter subset this service needs. +type GitHub interface { + AppSlug(ctx context.Context) (string, error) + VerifyWebhook(body []byte, signature256 string) bool + AuthorizeURL(state, redirectURI string) string + Exchange(ctx context.Context, code, redirectURI string) (github.UserToken, error) + Refresh(ctx context.Context, refreshToken string) (github.UserToken, error) + User(ctx context.Context, accessToken string) (github.Account, error) + UserRepositories(ctx context.Context, accessToken string) ([]github.Repository, error) + OpenPullRequests(ctx context.Context, installationID int64, fullName string) ([]github.PullRequest, error) +} + +var ( + ErrForbidden = errors.New("GitHub repository is not accessible to this account") + ErrNotConnected = errors.New("GitHub is not connected") + ErrReconnect = errors.New("GitHub authorization expired or was revoked; connect again") + ErrSignature = errors.New("invalid webhook signature") +) + +// verificationAge bounds how long a follow's verified access is trusted before +// fan-out rechecks it; it matches GitHub's user access token lifetime. +const verificationAge = 8 * time.Hour + +const stateLifetime = 10 * time.Minute + +type Options struct { + PublicURL string + TokenKey []byte + Now func() time.Time +} + +type Service struct { + store Store + github GitHub + sealer sealer + callback string + now func() time.Time + // Refresh tokens rotate on use, so each account refreshes serially. + locksMu sync.Mutex + locks map[string]*sync.Mutex + slugMu sync.Mutex + slug string +} + +func New(store Store, client GitHub, options Options) (*Service, error) { + sealer, err := newSealer(options.TokenKey) + if err != nil { + return nil, err + } + now := options.Now + if now == nil { + now = time.Now + } + return &Service{store: store, github: client, sealer: sealer, callback: strings.TrimSuffix(options.PublicURL, "/") + "/api/v2/github/callback", now: now, locks: make(map[string]*sync.Mutex)}, nil +} + +func (s *Service) millis() int64 { return s.now().UnixMilli() } + +func (s *Service) lock(ownerID string) func() { + s.locksMu.Lock() + mu, ok := s.locks[ownerID] + if !ok { + mu = &sync.Mutex{} + s.locks[ownerID] = mu + } + s.locksMu.Unlock() + mu.Lock() + return mu.Unlock +} + +func stateHash(state string) string { + digest := sha256.Sum256([]byte(state)) + return hex.EncodeToString(digest[:]) +} + +func randomToken() (string, error) { + value := make([]byte, 32) + if _, err := rand.Read(value); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(value), nil +} + +// Connect starts authorization bound to the account; only that account's +// session can complete it. +func (s *Service) Connect(ctx context.Context, ownerID string) (string, error) { + state, err := randomToken() + if err != nil { + return "", err + } + if err := s.store.PutOAuthState(ctx, stateHash(state), ownerID, s.now().Add(stateLifetime).UnixMilli()); err != nil { + return "", err + } + return s.github.AuthorizeURL(state, s.callback), nil +} + +// Complete exchanges the callback code. A state issued to another account is refused. +func (s *Service) Complete(ctx context.Context, ownerID, state, code string) error { + issued, err := s.store.TakeOAuthState(ctx, stateHash(state), s.millis()) + if errors.Is(err, ErrNotFound) || err == nil && issued != ownerID { + return ErrForbidden + } + if err != nil { + return err + } + token, err := s.github.Exchange(ctx, code, s.callback) + if err != nil { + return err + } + user, err := s.github.User(ctx, token.AccessToken) + if err != nil { + return err + } + defer s.lock(ownerID)() + return s.saveTokens(ctx, Connection{OwnerID: ownerID, UserID: user.ID, Login: user.Login, Status: "active", ConnectedAt: s.millis()}, token) +} + +func (s *Service) saveTokens(ctx context.Context, connection Connection, token github.UserToken) error { + var err error + if connection.AccessToken, err = s.sealer.seal(connection.OwnerID, "access", token.AccessToken); err != nil { + return err + } + if connection.RefreshToken, err = s.sealer.seal(connection.OwnerID, "refresh", token.RefreshToken); err != nil { + return err + } + connection.AccessExpiresAt, connection.RefreshExpiresAt = token.ExpiresAt.UnixMilli(), token.RefreshExpiresAt.UnixMilli() + if token.ExpiresAt.IsZero() { + connection.AccessExpiresAt = 0 + } + if token.RefreshExpiresAt.IsZero() { + connection.RefreshExpiresAt = 0 + } + return s.store.PutConnection(ctx, connection) +} + +// accessToken returns a usable user token, refreshing it a minute before expiry. +// A rejected refresh expires the connection and loses its follows. +func (s *Service) accessToken(ctx context.Context, ownerID string) (string, error) { + connection, err := s.store.Connection(ctx, ownerID) + if errors.Is(err, ErrNotFound) { + return "", ErrNotConnected + } + if err != nil { + return "", err + } + if connection.Status != "active" { + return "", ErrReconnect + } + if connection.AccessExpiresAt == 0 || connection.AccessExpiresAt > s.now().Add(time.Minute).UnixMilli() { + return s.sealer.open(ownerID, "access", connection.AccessToken) + } + refresh, err := s.sealer.open(ownerID, "refresh", connection.RefreshToken) + if err != nil { + return "", err + } + token, err := s.github.Refresh(ctx, refresh) + if errors.Is(err, github.ErrUnauthorized) { + connection.Status = "expired" + if err := s.store.PutConnection(ctx, connection); err != nil { + return "", err + } + return "", ErrReconnect + } + if err != nil { + return "", err + } + if err := s.saveTokens(ctx, connection, token); err != nil { + return "", err + } + return token.AccessToken, nil +} + +// Status reports the account's connection and the app installation page. +type Status struct { + Connected bool `json:"connected"` + Login string `json:"login,omitempty"` + State string `json:"state,omitempty"` + InstallURL string `json:"installUrl"` +} + +func (s *Service) Status(ctx context.Context, ownerID string) (Status, error) { + install, err := s.installURL(ctx) + if err != nil { + return Status{}, err + } + connection, err := s.store.Connection(ctx, ownerID) + if errors.Is(err, ErrNotFound) { + return Status{InstallURL: install}, nil + } + if err != nil { + return Status{}, err + } + return Status{Connected: true, Login: connection.Login, State: connection.Status, InstallURL: install}, nil +} + +func (s *Service) installURL(ctx context.Context) (string, error) { + s.slugMu.Lock() + defer s.slugMu.Unlock() + if s.slug == "" { + slug, err := s.github.AppSlug(ctx) + if err != nil { + return "", err + } + s.slug = slug + } + return "https://github.com/apps/" + s.slug + "/installations/new", nil +} + +func (s *Service) Disconnect(ctx context.Context, ownerID string) error { + defer s.lock(ownerID)() + return s.store.DeleteConnection(ctx, ownerID) +} + +// Repository is a repository the account can follow or already follows. +type Repository struct { + ID int64 `json:"id"` + FullName string `json:"fullName"` + Private bool `json:"private"` + Followed bool `json:"followed"` + // Lost marks a followed repository the account can no longer read. + Lost bool `json:"lost"` +} + +// Repositories lists what both the GitHub user and the app can read, plus +// followed repositories that lost access. +func (s *Service) Repositories(ctx context.Context, ownerID string) ([]Repository, error) { + accessible, err := s.verify(ctx, ownerID) + if err != nil { + return nil, err + } + follows, err := s.store.Follows(ctx, ownerID) + if err != nil { + return nil, err + } + followed := make(map[int64]bool, len(follows)) + for _, follow := range follows { + followed[follow.RepositoryID] = true + } + result := make([]Repository, 0, len(accessible)+len(follows)) + seen := make(map[int64]bool, len(accessible)) + for _, repository := range accessible { + seen[repository.ID] = true + result = append(result, Repository{ID: repository.ID, FullName: repository.FullName, Private: repository.Private, Followed: followed[repository.ID]}) + } + for _, follow := range follows { + if !seen[follow.RepositoryID] { + result = append(result, Repository{ID: follow.RepositoryID, FullName: follow.FullName, Followed: true, Lost: true}) + } + } + slices.SortFunc(result, func(a, b Repository) int { return strings.Compare(a.FullName, b.FullName) }) + return result, nil +} + +// verify lists the account's accessible repositories and updates every follow's +// access accordingly. It is the only path that marks a follow active. +func (s *Service) verify(ctx context.Context, ownerID string) ([]github.Repository, error) { + defer s.lock(ownerID)() + token, err := s.accessToken(ctx, ownerID) + if errors.Is(err, ErrReconnect) { + return nil, errors.Join(err, s.loseFollows(ctx, ownerID)) + } + if err != nil { + return nil, err + } + accessible, err := s.github.UserRepositories(ctx, token) + if errors.Is(err, github.ErrUnauthorized) { + connection, lookup := s.store.Connection(ctx, ownerID) + if lookup != nil { + return nil, lookup + } + connection.Status = "expired" + if err := s.store.PutConnection(ctx, connection); err != nil { + return nil, err + } + return nil, errors.Join(ErrReconnect, s.loseFollows(ctx, ownerID)) + } + if err != nil { + return nil, err + } + follows, err := s.store.Follows(ctx, ownerID) + if err != nil { + return nil, err + } + byID := make(map[int64]github.Repository, len(accessible)) + for _, repository := range accessible { + byID[repository.ID] = repository + } + now := s.millis() + for _, follow := range follows { + if repository, ok := byID[follow.RepositoryID]; ok { + follow.Status, follow.VerifiedAt, follow.InstallationID, follow.FullName = "active", now, repository.InstallationID, repository.FullName + } else { + follow.Status = "lost" + } + if err := s.store.PutFollow(ctx, follow); err != nil { + return nil, err + } + } + return accessible, nil +} + +func (s *Service) loseFollows(ctx context.Context, ownerID string) error { + follows, err := s.store.Follows(ctx, ownerID) + if err != nil { + return err + } + for _, follow := range follows { + follow.Status = "lost" + if err := s.store.PutFollow(ctx, follow); err != nil { + return err + } + } + return nil +} + +// Follow subscribes the account after verifying access, then requests a +// refresh of every open pull request so existing work appears immediately. +func (s *Service) Follow(ctx context.Context, ownerID string, repositoryID int64) error { + accessible, err := s.verify(ctx, ownerID) + if err != nil { + return err + } + index := slices.IndexFunc(accessible, func(r github.Repository) bool { return r.ID == repositoryID }) + if index < 0 { + return ErrForbidden + } + repository := accessible[index] + now := s.millis() + if err := s.store.PutFollow(ctx, Follow{OwnerID: ownerID, RepositoryID: repository.ID, InstallationID: repository.InstallationID, FullName: repository.FullName, Status: "active", VerifiedAt: now, CreatedAt: now}); err != nil { + return err + } + return s.reconcileRepository(ctx, Follow{RepositoryID: repository.ID, InstallationID: repository.InstallationID, FullName: repository.FullName}, "") +} + +func (s *Service) Unfollow(ctx context.Context, ownerID string, repositoryID int64) error { + return s.store.DeleteFollow(ctx, ownerID, repositoryID) +} + +// Resync rechecks the account's access and refreshes every followed +// repository's pull requests. Its run identity makes each resulting submission +// fresh, extending retention of pull requests idle since their last change. +func (s *Service) Resync(ctx context.Context, ownerID string) error { + if _, err := s.verify(ctx, ownerID); err != nil { + return err + } + follows, err := s.store.Follows(ctx, ownerID) + if err != nil { + return err + } + run, err := randomToken() + if err != nil { + return err + } + var failures []error + for _, follow := range follows { + if follow.Status == "active" { + failures = append(failures, s.reconcileRepository(ctx, follow, run)) + } + } + return errors.Join(failures...) +} + +// Reconcile recovers deliveries GitHub never retries: it rechecks every +// connected account's access, then requests refreshes for every open or +// previously open pull request of each actively followed repository. +func (s *Service) Reconcile(ctx context.Context) error { + connections, err := s.store.Connections(ctx) + if err != nil { + return err + } + var failures []error + repositories := make(map[int64]Follow) + for _, connection := range connections { + if _, err := s.verify(ctx, connection.OwnerID); err != nil && !errors.Is(err, ErrReconnect) { + failures = append(failures, fmt.Errorf("verify GitHub access for account %s: %w", connection.OwnerID, err)) + continue + } + follows, err := s.store.Follows(ctx, connection.OwnerID) + if err != nil { + return err + } + for _, follow := range follows { + if follow.Status == "active" { + repositories[follow.RepositoryID] = follow + } + } + } + for _, follow := range repositories { + failures = append(failures, s.reconcileRepository(ctx, follow, "")) + } + return errors.Join(failures...) +} + +// reconcileRepository requests refreshes for listed open pull requests and for +// known-open ones missing from the list, which closed without a delivery. +func (s *Service) reconcileRepository(ctx context.Context, follow Follow, run string) error { + open, err := s.github.OpenPullRequests(ctx, follow.InstallationID, follow.FullName) + if err != nil { + return fmt.Errorf("list open pull requests of %s: %w", follow.FullName, err) + } + known, err := s.store.KnownOpen(ctx, follow.RepositoryID) + if err != nil { + return err + } + numbers := make([]int, 0, len(open)+len(known)) + for _, pull := range open { + numbers = append(numbers, pull.Number) + } + for _, number := range known { + if !slices.Contains(numbers, number) { + numbers = append(numbers, number) + } + } + now := s.millis() + for _, number := range numbers { + ref := PullRequestRef{RepositoryID: follow.RepositoryID, InstallationID: follow.InstallationID, FullName: follow.FullName, Number: number} + if err := s.store.RequestRefresh(ctx, ref, run, now); err != nil { + return err + } + } + return nil +} + +// pullRequestActions change a pull request's diff, metadata or state. +var pullRequestActions = []string{"opened", "reopened", "synchronize", "edited", "closed", "ready_for_review", "converted_to_draft"} + +// Webhook verifies and applies one delivery. Duplicate delivery IDs are +// acknowledged without effect; a delivery is recorded only after it applies, +// so a failed one can be redelivered. Every effect is idempotent. +func (s *Service) Webhook(ctx context.Context, event, delivery, signature string, body []byte) error { + if !s.github.VerifyWebhook(body, signature) { + return ErrSignature + } + if delivery == "" || len(delivery) > 128 { + return errors.New("webhook delivery ID required") + } + seen, err := s.store.SeenDelivery(ctx, delivery) + if err != nil || seen { + return err + } + parsed, err := github.ParseEvent(event, body) + if err != nil { + return err + } + if err := s.apply(ctx, parsed); err != nil { + return err + } + return s.store.RecordDelivery(ctx, delivery, event, s.millis()) +} + +func (s *Service) apply(ctx context.Context, event any) error { + switch event := event.(type) { + case github.PullRequestEvent: + if !slices.Contains(pullRequestActions, event.Action) { + return nil + } + follows, err := s.store.RepositoryFollows(ctx, event.Repository.ID) + if err != nil || !slices.ContainsFunc(follows, func(f Follow) bool { return f.Status == "active" }) { + return err + } + ref := PullRequestRef{RepositoryID: event.Repository.ID, InstallationID: event.InstallationID, FullName: event.Repository.FullName, Number: event.Number} + return s.store.RequestRefresh(ctx, ref, "", s.millis()) + case github.InstallationEvent: + if event.Action == "deleted" || event.Action == "suspend" { + return s.store.LoseInstallation(ctx, event.InstallationID, nil) + } + case github.InstallationRepositoriesEvent: + if len(event.Removed) > 0 { + ids := make([]int64, 0, len(event.Removed)) + for _, repository := range event.Removed { + ids = append(ids, repository.ID) + } + return s.store.LoseInstallation(ctx, event.InstallationID, ids) + } + case github.AuthorizationEvent: + if event.Action == "revoked" { + return s.store.RevokeUser(ctx, event.UserID) + } + } + return nil +} + +// Followers returns accounts whose access to the repository is verified within +// verificationAge, rechecking stale follows first. Accounts that lost access +// are excluded; their follows stay lost until verified again. +func (s *Service) Followers(ctx context.Context, repositoryID int64) ([]string, error) { + follows, err := s.store.RepositoryFollows(ctx, repositoryID) + if err != nil { + return nil, err + } + stale := s.now().Add(-verificationAge).UnixMilli() + owners := make([]string, 0, len(follows)) + for _, follow := range follows { + if follow.Status != "active" { + continue + } + if follow.VerifiedAt <= stale { + accessible, err := s.verify(ctx, follow.OwnerID) + if errors.Is(err, ErrReconnect) { + continue + } + if err != nil { + return nil, err + } + if !slices.ContainsFunc(accessible, func(r github.Repository) bool { return r.ID == repositoryID }) { + continue + } + } + owners = append(owners, follow.OwnerID) + } + return owners, nil +} diff --git a/internal/githubsync/service_test.go b/internal/githubsync/service_test.go new file mode 100644 index 00000000..25361d5f --- /dev/null +++ b/internal/githubsync/service_test.go @@ -0,0 +1,388 @@ +package githubsync_test + +import ( + "bytes" + "context" + "errors" + "net/url" + "slices" + "sync" + "testing" + "time" + + "github.com/flexdinesh/diffx/internal/github" + "github.com/flexdinesh/diffx/internal/githubsync" + "github.com/flexdinesh/diffx/internal/reviewstore" +) + +type fakeGitHub struct { + mu sync.Mutex + repositories map[string][]github.Repository // by access token + open map[string][]github.PullRequest + users map[string]github.Account + refreshes int + refreshError error + listings int +} + +func (f *fakeGitHub) AppSlug(context.Context) (string, error) { return "diffx-test", nil } +func (f *fakeGitHub) VerifyWebhook(_ []byte, signature string) bool { + return signature == "valid" +} +func (f *fakeGitHub) AuthorizeURL(state, redirect string) string { + return "https://github.test/authorize?" + url.Values{"state": {state}, "redirect_uri": {redirect}}.Encode() +} +func (f *fakeGitHub) Exchange(_ context.Context, code, _ string) (github.UserToken, error) { + return github.UserToken{AccessToken: "access-" + code, ExpiresAt: time.Now().Add(8 * time.Hour), RefreshToken: "refresh-" + code}, nil +} +func (f *fakeGitHub) Refresh(_ context.Context, refresh string) (github.UserToken, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.refreshes++ + if f.refreshError != nil { + return github.UserToken{}, f.refreshError + } + return github.UserToken{AccessToken: "access-" + refresh[len("refresh-"):], ExpiresAt: time.Now().Add(8 * time.Hour), RefreshToken: refresh}, nil +} +func (f *fakeGitHub) User(_ context.Context, token string) (github.Account, error) { + return f.users[token], nil +} +func (f *fakeGitHub) UserRepositories(_ context.Context, token string) ([]github.Repository, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.listings++ + return slices.Clone(f.repositories[token]), nil +} +func (f *fakeGitHub) OpenPullRequests(_ context.Context, _ int64, fullName string) ([]github.PullRequest, error) { + return f.open[fullName], nil +} + +// refreshLog records refresh intent while delegating to the real store. +type refreshLog struct { + githubsync.Store + mu sync.Mutex + refs []githubsync.PullRequestRef + runs []string +} + +func (r *refreshLog) RequestRefresh(ctx context.Context, ref githubsync.PullRequestRef, run string, now int64) error { + r.mu.Lock() + r.refs, r.runs = append(r.refs, ref), append(r.runs, run) + r.mu.Unlock() + return r.Store.RequestRefresh(ctx, ref, run, now) +} + +func (r *refreshLog) take() ([]githubsync.PullRequestRef, []string) { + r.mu.Lock() + defer r.mu.Unlock() + refs, runs := r.refs, r.runs + r.refs, r.runs = nil, nil + return refs, runs +} + +var appRepo = github.Repository{ID: 10, FullName: "octo/app", InstallationID: 5} +var otherRepo = github.Repository{ID: 11, FullName: "octo/secret", InstallationID: 5} + +type fixture struct { + store *reviewstore.Store + github *fakeGitHub + log *refreshLog + service *githubsync.Service + clock *time.Time + alice string + bob string +} + +func newFixture(t *testing.T) *fixture { + t.Helper() + store, err := reviewstore.Open("") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + alice, err := store.User("alice", "alice") + if err != nil { + t.Fatal(err) + } + bob, err := store.User("bob", "bob") + if err != nil { + t.Fatal(err) + } + clock := time.Now() + f := &fixture{store: store, clock: &clock, alice: alice.ID, bob: bob.ID, + github: &fakeGitHub{ + repositories: map[string][]github.Repository{"access-alice": {appRepo, otherRepo}, "access-bob": {appRepo}}, + open: map[string][]github.PullRequest{"octo/app": {{Number: 1}, {Number: 2}}}, + users: map[string]github.Account{"access-alice": {ID: 100, Login: "alice-gh"}, "access-bob": {ID: 200, Login: "bob-gh"}}, + }} + f.log = &refreshLog{Store: store.GitHub()} + f.service, err = githubsync.New(f.log, f.github, githubsync.Options{PublicURL: "https://diffx.test", TokenKey: bytes.Repeat([]byte{7}, 32), Now: func() time.Time { return *f.clock }}) + if err != nil { + t.Fatal(err) + } + return f +} + +func (f *fixture) connect(t *testing.T, owner, code string) { + t.Helper() + authorize, err := f.service.Connect(t.Context(), owner) + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authorize) + if err != nil { + t.Fatal(err) + } + if got := parsed.Query().Get("redirect_uri"); got != "https://diffx.test/api/v2/github/callback" { + t.Fatalf("redirect %q", got) + } + if err := f.service.Complete(t.Context(), owner, parsed.Query().Get("state"), code); err != nil { + t.Fatal(err) + } +} + +func TestAuthorizationIsBoundToStartingAccountAndSealed(t *testing.T) { + f := newFixture(t) + authorize, err := f.service.Connect(t.Context(), f.alice) + if err != nil { + t.Fatal(err) + } + parsed, err := url.Parse(authorize) + if err != nil { + t.Fatal(err) + } + state := parsed.Query().Get("state") + if err := f.service.Complete(t.Context(), f.bob, state, "bob"); !errors.Is(err, githubsync.ErrForbidden) { + t.Fatalf("another account completed alice's authorization: %v", err) + } + // The failed attempt consumed the state, so it cannot be replayed either. + if err := f.service.Complete(t.Context(), f.alice, state, "alice"); !errors.Is(err, githubsync.ErrForbidden) { + t.Fatalf("consumed state reused: %v", err) + } + f.connect(t, f.alice, "alice") + connection, err := f.store.GitHub().Connection(t.Context(), f.alice) + if err != nil || connection.Login != "alice-gh" || connection.UserID != 100 { + t.Fatalf("connection %+v %v", connection, err) + } + for _, sealed := range [][]byte{connection.AccessToken, connection.RefreshToken} { + if bytes.Contains(sealed, []byte("alice")) { + t.Fatal("token stored in plaintext") + } + } + status, err := f.service.Status(t.Context(), f.alice) + if err != nil || !status.Connected || status.State != "active" || status.InstallURL != "https://github.com/apps/diffx-test/installations/new" { + t.Fatalf("status %+v %v", status, err) + } +} + +func TestFollowRequiresAccessAndRefreshesOpenPullRequests(t *testing.T) { + f := newFixture(t) + if err := f.service.Follow(t.Context(), f.alice, appRepo.ID); !errors.Is(err, githubsync.ErrNotConnected) { + t.Fatalf("unconnected follow: %v", err) + } + f.connect(t, f.bob, "bob") + if err := f.service.Follow(t.Context(), f.bob, otherRepo.ID); !errors.Is(err, githubsync.ErrForbidden) { + t.Fatalf("followed a repository the GitHub user cannot read: %v", err) + } + if err := f.service.Follow(t.Context(), f.bob, appRepo.ID); err != nil { + t.Fatal(err) + } + refs, _ := f.log.take() + if len(refs) != 2 || refs[0].Number != 1 || refs[1].Number != 2 || refs[0].InstallationID != 5 { + t.Fatalf("open pull requests not requested: %+v", refs) + } + repositories, err := f.service.Repositories(t.Context(), f.bob) + if err != nil || len(repositories) != 1 || !repositories[0].Followed || repositories[0].Lost { + t.Fatalf("repositories %+v %v", repositories, err) + } + // Access removed on GitHub: the follow is listed as lost and gets nothing. + f.github.repositories["access-bob"] = nil + repositories, err = f.service.Repositories(t.Context(), f.bob) + if err != nil || len(repositories) != 1 || !repositories[0].Lost { + t.Fatalf("lost follow %+v %v", repositories, err) + } + followers, err := f.service.Followers(t.Context(), appRepo.ID) + if err != nil || len(followers) != 0 { + t.Fatalf("lost follower kept: %v %v", followers, err) + } +} + +func TestWebhookVerifiesDeduplicatesAndIgnoresUnfollowed(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + if err := f.service.Follow(t.Context(), f.alice, appRepo.ID); err != nil { + t.Fatal(err) + } + f.log.take() + body := []byte(`{"action":"synchronize","number":3,"installation":{"id":5},"repository":{"id":10,"full_name":"octo/app"}}`) + if err := f.service.Webhook(t.Context(), "pull_request", "d1", "forged", body); !errors.Is(err, githubsync.ErrSignature) { + t.Fatalf("unsigned delivery: %v", err) + } + for range 2 { + if err := f.service.Webhook(t.Context(), "pull_request", "d1", "valid", body); err != nil { + t.Fatal(err) + } + } + refs, runs := f.log.take() + if len(refs) != 1 || refs[0].Number != 3 || runs[0] != "" { + t.Fatalf("duplicate delivery applied twice: %+v", refs) + } + unfollowed := []byte(`{"action":"opened","number":4,"installation":{"id":5},"repository":{"id":11,"full_name":"octo/secret"}}`) + labeled := []byte(`{"action":"labeled","number":3,"installation":{"id":5},"repository":{"id":10,"full_name":"octo/app"}}`) + for id, payload := range map[string][]byte{"d2": unfollowed, "d3": labeled} { + if err := f.service.Webhook(t.Context(), "pull_request", id, "valid", payload); err != nil { + t.Fatal(err) + } + } + if refs, _ := f.log.take(); len(refs) != 0 { + t.Fatalf("unfollowed or irrelevant delivery requested refresh: %+v", refs) + } + revoke := []byte(`{"action":"revoked","sender":{"id":100}}`) + if err := f.service.Webhook(t.Context(), "github_app_authorization", "d4", "valid", revoke); err != nil { + t.Fatal(err) + } + status, err := f.service.Status(t.Context(), f.alice) + if err != nil || status.State != "revoked" { + t.Fatalf("revocation not applied: %+v %v", status, err) + } + if err := f.service.Webhook(t.Context(), "pull_request", "d5", "valid", body); err != nil { + t.Fatal(err) + } + if refs, _ := f.log.take(); len(refs) != 0 { + t.Fatalf("revoked follower still requests refreshes: %+v", refs) + } +} + +func TestInstallationRemovalLosesFollows(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + for _, id := range []int64{appRepo.ID, otherRepo.ID} { + if err := f.service.Follow(t.Context(), f.alice, id); err != nil { + t.Fatal(err) + } + } + removed := []byte(`{"action":"removed","installation":{"id":5},"repositories_added":[],"repositories_removed":[{"id":11,"full_name":"octo/secret"}]}`) + if err := f.service.Webhook(t.Context(), "installation_repositories", "r1", "valid", removed); err != nil { + t.Fatal(err) + } + follows, err := f.store.GitHub().Follows(t.Context(), f.alice) + if err != nil || len(follows) != 2 || follows[0].Status != "active" || follows[1].Status != "lost" { + t.Fatalf("follows %+v %v", follows, err) + } + deleted := []byte(`{"action":"deleted","installation":{"id":5}}`) + if err := f.service.Webhook(t.Context(), "installation", "r2", "valid", deleted); err != nil { + t.Fatal(err) + } + follows, err = f.store.GitHub().Follows(t.Context(), f.alice) + if err != nil || follows[0].Status != "lost" { + t.Fatalf("uninstall kept follows active: %+v %v", follows, err) + } +} + +func TestExpiredRefreshLosesFollowsAndAsksToReconnect(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + if err := f.service.Follow(t.Context(), f.alice, appRepo.ID); err != nil { + t.Fatal(err) + } + *f.clock = f.clock.Add(9 * time.Hour) + if _, err := f.service.Repositories(t.Context(), f.alice); err != nil { + t.Fatalf("expired access token not refreshed: %v", err) + } + if f.github.refreshes != 1 { + t.Fatalf("refreshes %d", f.github.refreshes) + } + *f.clock = f.clock.Add(9 * time.Hour) + f.github.refreshError = github.ErrUnauthorized + if _, err := f.service.Repositories(t.Context(), f.alice); !errors.Is(err, githubsync.ErrReconnect) { + t.Fatalf("rejected refresh: %v", err) + } + status, err := f.service.Status(t.Context(), f.alice) + if err != nil || status.State != "expired" { + t.Fatalf("status %+v %v", status, err) + } + followers, err := f.service.Followers(t.Context(), appRepo.ID) + if err != nil || len(followers) != 0 { + t.Fatalf("expired follower kept: %v %v", followers, err) + } +} + +func TestFollowersRecheckOnlyStaleVerification(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + f.connect(t, f.bob, "bob") + for _, owner := range []string{f.alice, f.bob} { + if err := f.service.Follow(t.Context(), owner, appRepo.ID); err != nil { + t.Fatal(err) + } + } + listings := f.github.listings + followers, err := f.service.Followers(t.Context(), appRepo.ID) + if err != nil || len(followers) != 2 || f.github.listings != listings { + t.Fatalf("fresh verification rechecked: %v %v %d", followers, err, f.github.listings-listings) + } + *f.clock = f.clock.Add(7 * time.Hour) + f.github.repositories["access-bob"] = nil + if followers, _ := f.service.Followers(t.Context(), appRepo.ID); len(followers) != 2 { + t.Fatalf("verification within 8h must be trusted: %v", followers) + } + *f.clock = f.clock.Add(2 * time.Hour) + followers, err = f.service.Followers(t.Context(), appRepo.ID) + if err != nil || !slices.Equal(followers, []string{f.alice}) { + t.Fatalf("stale verification not rechecked: %v %v", followers, err) + } +} + +func TestReconcileRecoversMissedClosesAndResyncExtendsRetention(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + if err := f.service.Follow(t.Context(), f.alice, appRepo.ID); err != nil { + t.Fatal(err) + } + f.log.take() + // PR 2 closed without a delivery; it is still known open. + f.github.open["octo/app"] = []github.PullRequest{{Number: 1}} + if err := f.service.Reconcile(t.Context()); err != nil { + t.Fatal(err) + } + refs, runs := f.log.take() + numbers := make([]int, 0, len(refs)) + for _, ref := range refs { + numbers = append(numbers, ref.Number) + } + if !slices.Equal(numbers, []int{1, 2}) || runs[0] != "" { + t.Fatalf("reconcile requested %v runs %q", numbers, runs) + } + if err := f.service.Resync(t.Context(), f.alice); err != nil { + t.Fatal(err) + } + _, runs = f.log.take() + if len(runs) != 2 || runs[0] == "" || runs[0] != runs[1] { + t.Fatalf("resync must share one retention run: %q", runs) + } + if err := f.service.Resync(t.Context(), f.alice); err != nil { + t.Fatal(err) + } + _, again := f.log.take() + if again[0] == runs[0] { + t.Fatal("each resync needs a fresh retention run") + } +} + +func TestDisconnectRemovesFollows(t *testing.T) { + f := newFixture(t) + f.connect(t, f.alice, "alice") + if err := f.service.Follow(t.Context(), f.alice, appRepo.ID); err != nil { + t.Fatal(err) + } + if err := f.service.Disconnect(t.Context(), f.alice); err != nil { + t.Fatal(err) + } + follows, err := f.store.GitHub().Follows(t.Context(), f.alice) + if err != nil || len(follows) != 0 { + t.Fatalf("follows survived disconnect: %+v %v", follows, err) + } + if status, err := f.service.Status(t.Context(), f.alice); err != nil || status.Connected { + t.Fatalf("status %+v %v", status, err) + } +} diff --git a/internal/githubsync/store.go b/internal/githubsync/store.go new file mode 100644 index 00000000..d86beda9 --- /dev/null +++ b/internal/githubsync/store.go @@ -0,0 +1,71 @@ +package githubsync + +import ( + "context" + "errors" +) + +// ErrNotFound reports an absent connection, follow or OAuth state. +var ErrNotFound = errors.New("not found") + +// Connection is one account's GitHub user authorization. Tokens are sealed; +// storage never sees plaintext. +type Connection struct { + OwnerID string + UserID int64 + Login string + AccessToken []byte + AccessExpiresAt int64 + RefreshToken []byte + RefreshExpiresAt int64 + Status string // active, revoked or expired + ConnectedAt int64 +} + +// Follow is an account's subscription to a repository's pull requests. Lost +// follows stay listed but receive nothing until access is verified again. +type Follow struct { + OwnerID string + RepositoryID int64 + InstallationID int64 + FullName string + Status string // active or lost + VerifiedAt int64 + CreatedAt int64 +} + +// PullRequestRef addresses one pull request through the installation that can read it. +type PullRequestRef struct { + RepositoryID int64 + InstallationID int64 + FullName string + Number int +} + +// Store persists integration state atomically per operation. +type Store interface { + PutOAuthState(ctx context.Context, hash, ownerID string, expiresAt int64) error + // TakeOAuthState consumes an unexpired state exactly once. + TakeOAuthState(ctx context.Context, hash string, now int64) (string, error) + PutConnection(ctx context.Context, connection Connection) error + Connection(ctx context.Context, ownerID string) (Connection, error) + Connections(ctx context.Context) ([]Connection, error) + // DeleteConnection removes the authorization and every follow of the account. + DeleteConnection(ctx context.Context, ownerID string) error + // RevokeUser marks every connection of the GitHub user revoked and their follows lost. + RevokeUser(ctx context.Context, userID int64) error + PutFollow(ctx context.Context, follow Follow) error + DeleteFollow(ctx context.Context, ownerID string, repositoryID int64) error + Follows(ctx context.Context, ownerID string) ([]Follow, error) + RepositoryFollows(ctx context.Context, repositoryID int64) ([]Follow, error) + // LoseInstallation marks follows lost for the installation, or only its listed repositories. + LoseInstallation(ctx context.Context, installationID int64, repositoryIDs []int64) error + // SeenDelivery and RecordDelivery deduplicate webhook deliveries by GUID. + SeenDelivery(ctx context.Context, id string) (bool, error) + RecordDelivery(ctx context.Context, id, event string, now int64) error + // RequestRefresh coalesces refresh intent per pull request by advancing its + // generation. A nonempty retention run makes the next submission fresh. + RequestRefresh(ctx context.Context, ref PullRequestRef, retentionRun string, now int64) error + // KnownOpen lists pull request numbers last observed open. + KnownOpen(ctx context.Context, repositoryID int64) ([]int, error) +} diff --git a/internal/remoteserver/github.go b/internal/remoteserver/github.go new file mode 100644 index 00000000..f56a7da2 --- /dev/null +++ b/internal/remoteserver/github.go @@ -0,0 +1,211 @@ +package remoteserver + +import ( + "context" + "encoding/base64" + "errors" + "io" + "log" + "net/http" + "os" + "strconv" + "strings" + + "github.com/flexdinesh/diffx/internal/github" + "github.com/flexdinesh/diffx/internal/githubsync" + "github.com/flexdinesh/diffx/internal/reviewstore" + "github.com/flexdinesh/diffx/internal/serverapp" +) + +// GitHubSettings enables the GitHub App integration; nil disables its routes. +type GitHubSettings struct { + AppID int64 + ClientID string + ClientSecret string + PrivateKeyFile string + WebhookSecret string + TokenKey string + PublicURL string + // apiURL and webURL replace github.com in tests. + apiURL, webURL string +} + +// maxWebhookBytes is GitHub's documented webhook payload cap. +const maxWebhookBytes = 25 << 20 + +func composeGitHub(store *reviewstore.Store, settings *GitHubSettings) (*githubsync.Service, error) { + if settings == nil { + return nil, nil + } + pem, err := os.ReadFile(settings.PrivateKeyFile) + if err != nil { + return nil, errors.New("read GitHub App private key file: " + err.Error()) + } + key, err := github.ParsePrivateKey(pem) + if err != nil { + return nil, err + } + tokenKey, err := base64.StdEncoding.DecodeString(settings.TokenKey) + if err != nil { + return nil, errors.New("GitHub token key must be standard base64") + } + var options []github.Option + if settings.apiURL != "" { + options = append(options, github.WithAPIURL(settings.apiURL), github.WithWebURL(settings.webURL)) + } + client, err := github.New(github.App{ID: settings.AppID, ClientID: settings.ClientID, ClientSecret: settings.ClientSecret, PrivateKey: key, WebhookSecret: []byte(settings.WebhookSecret)}, options...) + if err != nil { + return nil, err + } + return githubsync.New(store.GitHub(), client, githubsync.Options{PublicURL: settings.PublicURL, TokenKey: tokenKey}) +} + +// reconcileAtStartup recovers missed deliveries once, then holds its task slot +// for the server lifetime. Failures are logged and Resync remains available. +func reconcileAtStartup(service *githubsync.Service) serverapp.Task { + return func(ctx context.Context) error { + if err := service.Reconcile(ctx); err != nil && ctx.Err() == nil { + log.Printf("GitHub reconcile: %v", err) + } + <-ctx.Done() + return nil + } +} + +// githubPublic serves the routes GitHub itself calls: signed webhooks and the +// OAuth callback, a cross-site top-level navigation whose state binds it to +// the session that started it. +func (remote *transport) githubPublic(w http.ResponseWriter, r *http.Request) bool { + switch r.URL.Path { + case "/api/v2/github/webhooks": + remote.githubWebhook(w, r) + return true + case "/api/v2/github/callback": + remote.githubCallback(w, r) + return true + } + return false +} + +func (remote *transport) githubWebhook(w http.ResponseWriter, r *http.Request) { + if remote.github == nil { + http.NotFound(w, r) + return + } + if r.Method != http.MethodPost { + problem(w, 405, "Method not allowed") + return + } + body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxWebhookBytes)) + if err != nil { + problem(w, 413, "Webhook payload too large") + return + } + err = remote.github.Webhook(r.Context(), r.Header.Get("X-GitHub-Event"), r.Header.Get("X-GitHub-Delivery"), r.Header.Get("X-Hub-Signature-256"), body) + switch { + case errors.Is(err, githubsync.ErrSignature): + problem(w, 401, "Invalid webhook signature") + case err != nil: + log.Printf("GitHub webhook: %v", err) + problem(w, 500, "Webhook not applied") + default: + w.WriteHeader(http.StatusAccepted) + } +} + +func (remote *transport) githubCallback(w http.ResponseWriter, r *http.Request) { + if remote.github == nil { + http.NotFound(w, r) + return + } + if r.Method != http.MethodGet { + problem(w, 405, "Method not allowed") + return + } + p, err := remote.authenticate(r) + if err != nil || p.session == nil { + http.Redirect(w, r, "/settings?github=signin", http.StatusSeeOther) + return + } + query := r.URL.Query() + if query.Get("error") != "" || query.Get("code") == "" { + http.Redirect(w, r, "/settings?github=cancelled", http.StatusSeeOther) + return + } + if err := remote.github.Complete(r.Context(), p.account.ID, query.Get("state"), query.Get("code")); err != nil { + log.Printf("GitHub authorization: %v", err) + http.Redirect(w, r, "/settings?github=failed", http.StatusSeeOther) + return + } + http.Redirect(w, r, "/settings?github=connected", http.StatusSeeOther) +} + +// githubAccount serves integration management. Like account routes it needs a +// browser session, because it handles GitHub user credentials. +func (remote *transport) githubAccount(w http.ResponseWriter, r *http.Request, p principal) { + if remote.github == nil { + problem(w, 404, "GitHub integration is not configured") + return + } + ctx, owner := r.Context(), p.account.ID + path := strings.TrimPrefix(r.URL.Path, "/api/v2/github/") + switch { + case path == "connection" && r.Method == http.MethodGet: + status, err := remote.github.Status(ctx, owner) + githubResult(w, status, err) + case path == "connection" && r.Method == http.MethodPost: + url, err := remote.github.Connect(ctx, owner) + githubResult(w, struct { + AuthorizeURL string `json:"authorizeUrl"` + }{url}, err) + case path == "connection" && r.Method == http.MethodDelete: + githubResult(w, nil, remote.github.Disconnect(ctx, owner)) + case path == "repositories" && r.Method == http.MethodGet: + repositories, err := remote.github.Repositories(ctx, owner) + githubResult(w, struct { + Repositories []githubsync.Repository `json:"repositories"` + }{repositories}, err) + case path == "resync" && r.Method == http.MethodPost: + githubResult(w, nil, remote.github.Resync(ctx, owner)) + case strings.HasPrefix(path, "follows/"): + id, err := strconv.ParseInt(strings.TrimPrefix(path, "follows/"), 10, 64) + if err != nil || id <= 0 { + problem(w, 404, "Repository not found") + return + } + switch r.Method { + case http.MethodPut: + githubResult(w, nil, remote.github.Follow(ctx, owner, id)) + case http.MethodDelete: + githubResult(w, nil, remote.github.Unfollow(ctx, owner, id)) + default: + problem(w, 405, "Method not allowed") + } + case path == "connection" || path == "repositories" || path == "resync": + problem(w, 405, "Method not allowed") + default: + http.NotFound(w, r) + } +} + +func githubResult(w http.ResponseWriter, value any, err error) { + var api *github.Error + switch { + case err == nil && value == nil: + w.WriteHeader(http.StatusNoContent) + case err == nil: + writeJSON(w, 200, value) + case errors.Is(err, githubsync.ErrNotConnected): + problem(w, 409, err.Error()) + case errors.Is(err, githubsync.ErrReconnect): + problem(w, 409, githubsync.ErrReconnect.Error()) + case errors.Is(err, githubsync.ErrForbidden): + problem(w, 403, err.Error()) + case errors.As(err, &api) && api.Status == 429: + w.Header().Set("Retry-After", "60") + problem(w, 429, "GitHub rate limit reached; try again later") + default: + log.Printf("GitHub integration: %v", err) + problem(w, 502, "GitHub request failed") + } +} diff --git a/internal/remoteserver/github_test.go b/internal/remoteserver/github_test.go new file mode 100644 index 00000000..1e518110 --- /dev/null +++ b/internal/remoteserver/github_test.go @@ -0,0 +1,198 @@ +package remoteserver + +import ( + "crypto/hmac" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/hex" + "encoding/json" + "encoding/pem" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/flexdinesh/diffx/internal/identityservice" +) + +// fakeGitHubAPI serves the API and OAuth endpoints the integration calls. +func fakeGitHubAPI(t *testing.T) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + reply := func(path, body string) { + mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + }) + } + reply("/app", `{"slug":"diffx-test"}`) + reply("/login/oauth/access_token", `{"access_token":"user-access","expires_in":28800,"refresh_token":"user-refresh","refresh_token_expires_in":15897600}`) + reply("/user", `{"id":100,"login":"mona","html_url":"https://github.com/mona"}`) + reply("/user/installations", `{"installations":[{"id":5}]}`) + reply("/user/installations/5/repositories", `{"repositories":[{"id":10,"full_name":"octo/app","private":true}]}`) + reply("/app/installations/5/access_tokens", `{"token":"installation","expires_at":"2099-01-01T00:00:00Z"}`) + reply("/repos/octo/app/pulls", `[]`) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + return server +} + +func githubSettings(t *testing.T, api string) *GitHubSettings { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "app.pem") + if err := os.WriteFile(path, pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}), 0600); err != nil { + t.Fatal(err) + } + return &GitHubSettings{AppID: 1, ClientID: "client", ClientSecret: "secret", PrivateKeyFile: path, WebhookSecret: "hook-secret", + TokenKey: base64.StdEncoding.EncodeToString(make([]byte, 32)), PublicURL: "https://diffx.test", apiURL: api, webURL: api} +} + +func signed(body string) string { + mac := hmac.New(sha256.New, []byte("hook-secret")) + mac.Write([]byte(body)) + return "sha256=" + hex.EncodeToString(mac.Sum(nil)) +} + +func webhook(handler http.Handler, delivery, signature, body string) int { + r := httptest.NewRequest("POST", "/api/v2/github/webhooks", strings.NewReader(body)) + r.Header.Set("X-GitHub-Event", "pull_request") + r.Header.Set("X-GitHub-Delivery", delivery) + r.Header.Set("X-Hub-Signature-256", signature) + w := httptest.NewRecorder() + handler.ServeHTTP(w, r) + return w.Code +} + +func TestGitHubRoutesAbsentWithoutConfiguration(t *testing.T) { + store, handler := identityHandler(t, Settings{SignupEnabled: true}) + if _, err := identityservice.New(store).Provision(t.Context(), "user", testPassword); err != nil { + t.Fatal(err) + } + cookie, _ := loginIdentity(t, handler, "user") + if w := identityRequest(handler, "GET", "/api/v2/github/connection", "", cookie, "", ""); w.Code != 404 { + t.Fatalf("unconfigured connection: %d", w.Code) + } + if code := webhook(handler, "d", signed("{}"), "{}"); code != 404 { + t.Fatalf("unconfigured webhook: %d", code) + } +} + +func TestGitHubConnectFollowAndWebhookBoundaries(t *testing.T) { + api := fakeGitHubAPI(t) + store, handler := identityHandler(t, Settings{SignupEnabled: true, GitHub: githubSettings(t, api.URL)}) + identity := identityservice.New(store) + if _, err := identity.Provision(t.Context(), "user", testPassword); err != nil { + t.Fatal(err) + } + token := provisionToken(t, store, "automation", identityservice.PurposeAPI) + cookie, csrf := loginIdentity(t, handler, "user") + + body := `{"action":"opened","number":1,"installation":{"id":5},"repository":{"id":10,"full_name":"octo/app"}}` + if code := webhook(handler, "d1", "sha256=00", body); code != 401 { + t.Fatalf("forged webhook: %d", code) + } + if code := webhook(handler, "d1", signed(body), body); code != 202 { + t.Fatalf("signed webhook without diffx credentials: %d", code) + } + + if w := identityRequest(handler, "GET", "/api/v2/github/connection", "", nil, "", "Bearer "+token); w.Code != 403 { + t.Fatalf("API token reached GitHub credentials: %d", w.Code) + } + if w := identityRequest(handler, "POST", "/api/v2/github/connection", "", cookie, "", ""); w.Code != 403 { + t.Fatalf("connect without CSRF: %d", w.Code) + } + w := identityRequest(handler, "POST", "/api/v2/github/connection", "", cookie, csrf, "") + var started struct { + AuthorizeURL string `json:"authorizeUrl"` + } + if w.Code != 200 || json.Unmarshal(w.Body.Bytes(), &started) != nil { + t.Fatalf("connect: %d %s", w.Code, w.Body.String()) + } + authorize, err := url.Parse(started.AuthorizeURL) + if err != nil { + t.Fatal(err) + } + callback := "/api/v2/github/callback?" + url.Values{"code": {"code"}, "state": {authorize.Query().Get("state")}}.Encode() + // GitHub's redirect is a cross-site top-level navigation carrying the Lax session cookie. + r := httptest.NewRequest("GET", callback, nil) + r.Header.Set("Sec-Fetch-Site", "cross-site") + r.AddCookie(cookie) + w = httptest.NewRecorder() + handler.ServeHTTP(w, r) + if w.Code != 303 || w.Header().Get("Location") != "/settings?github=connected" { + t.Fatalf("callback: %d %s", w.Code, w.Header().Get("Location")) + } + w = identityRequest(handler, "GET", "/api/v2/github/connection", "", cookie, "", "") + if w.Code != 200 || !strings.Contains(w.Body.String(), `"login":"mona"`) || !strings.Contains(w.Body.String(), "apps/diffx-test/installations/new") { + t.Fatalf("connection: %d %s", w.Code, w.Body.String()) + } + if w := identityRequest(handler, "PUT", "/api/v2/github/follows/99", "", cookie, csrf, ""); w.Code != 403 { + t.Fatalf("followed inaccessible repository: %d %s", w.Code, w.Body.String()) + } + if w := identityRequest(handler, "PUT", "/api/v2/github/follows/10", "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("follow: %d %s", w.Code, w.Body.String()) + } + w = identityRequest(handler, "GET", "/api/v2/github/repositories", "", cookie, "", "") + if w.Code != 200 || !strings.Contains(w.Body.String(), `"fullName":"octo/app","private":true,"followed":true,"lost":false`) { + t.Fatalf("repositories: %d %s", w.Code, w.Body.String()) + } + if w := identityRequest(handler, "POST", "/api/v2/github/resync", "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("resync: %d %s", w.Code, w.Body.String()) + } + if w := identityRequest(handler, "DELETE", "/api/v2/github/connection", "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("disconnect: %d", w.Code) + } +} + +func TestGitHubCallbackRefusesForeignAndMissingSessions(t *testing.T) { + api := fakeGitHubAPI(t) + store, handler := identityHandler(t, Settings{SignupEnabled: true, GitHub: githubSettings(t, api.URL)}) + identity := identityservice.New(store) + for _, name := range []string{"alice", "bob"} { + if _, err := identity.Provision(t.Context(), name, testPassword); err != nil { + t.Fatal(err) + } + } + alice, csrf := loginIdentity(t, handler, "alice") + bob, _ := loginIdentity(t, handler, "bob") + w := identityRequest(handler, "POST", "/api/v2/github/connection", "", alice, csrf, "") + var started struct { + AuthorizeURL string `json:"authorizeUrl"` + } + if err := json.Unmarshal(w.Body.Bytes(), &started); err != nil { + t.Fatal(err) + } + authorize, err := url.Parse(started.AuthorizeURL) + if err != nil { + t.Fatal(err) + } + callback := "/api/v2/github/callback?" + url.Values{"code": {"code"}, "state": {authorize.Query().Get("state")}}.Encode() + for _, tc := range []struct { + cookie *http.Cookie + location string + }{{nil, "/settings?github=signin"}, {bob, "/settings?github=failed"}} { + r := httptest.NewRequest("GET", callback, nil) + if tc.cookie != nil { + r.AddCookie(tc.cookie) + } + w := httptest.NewRecorder() + handler.ServeHTTP(w, r) + if w.Code != 303 || w.Header().Get("Location") != tc.location { + t.Fatalf("callback: %d %s, want %s", w.Code, w.Header().Get("Location"), tc.location) + } + } + w = identityRequest(handler, "GET", "/api/v2/github/connection", "", bob, "", "") + if strings.Contains(w.Body.String(), `"connected":true`) { + t.Fatalf("bob completed alice's authorization: %s", w.Body.String()) + } +} diff --git a/internal/remoteserver/identity.go b/internal/remoteserver/identity.go index 1bb4afde..b7a4e383 100644 --- a/internal/remoteserver/identity.go +++ b/internal/remoteserver/identity.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/flexdinesh/diffx/internal/githubsync" "github.com/flexdinesh/diffx/internal/identityservice" "github.com/flexdinesh/diffx/internal/serverapp" "github.com/flexdinesh/diffx/packages/api" @@ -29,6 +30,7 @@ type transport struct { assets fs.FS limiter *authLimiter authSlots chan struct{} + github *githubsync.Service } type principal struct { account identityservice.Account @@ -78,6 +80,9 @@ func (remote *transport) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("Cache-Control", "no-store") w.Header().Set("Referrer-Policy", "no-referrer") w.Header().Set("X-Content-Type-Options", "nosniff") + if remote.githubPublic(w, r) { + return + } if !serverapp.SameOrigin(r) { problem(w, 403, "Cross-origin access denied") return @@ -120,6 +125,14 @@ func (remote *transport) ServeHTTP(w http.ResponseWriter, r *http.Request) { problem(w, 403, "CSRF token required") return } + if strings.HasPrefix(r.URL.Path, "/api/v2/github/") { + if p.session == nil { + problem(w, 403, "Browser session required") + return + } + remote.githubAccount(w, r, p) + return + } if strings.HasPrefix(r.URL.Path, "/api/v2/account/") || r.URL.Path == "/api/v2/auth/logout" { if p.session == nil { problem(w, 403, "Browser session required") diff --git a/internal/remoteserver/server.go b/internal/remoteserver/server.go index b890179a..ede873b5 100644 --- a/internal/remoteserver/server.go +++ b/internal/remoteserver/server.go @@ -28,6 +28,7 @@ type Settings struct { SignupEnabled bool InsecureCookies bool RetentionDays int + GitHub *GitHubSettings } type application struct { @@ -78,27 +79,39 @@ func MultiHandler(ctx context.Context, store *reviewstore.Store, assets fs.FS, o settings = options[0] } ctx, cancel := context.WithCancel(ctx) - handler, closeServices, worker, err := compose(ctx, store, assets, settings) + handler, closeServices, tasks, err := compose(ctx, store, assets, settings) if err != nil { cancel() return nil, nil, err } - done := make(chan error, 1) - go func() { done <- worker(ctx) }() + done := make(chan error, len(tasks)) + for _, task := range tasks { + go func() { done <- task(ctx) }() + } var once sync.Once var closeErr error return handler, func() error { - once.Do(func() { cancel(); closeErr = errors.Join(<-done, closeServices()) }) + once.Do(func() { + cancel() + for range tasks { + closeErr = errors.Join(closeErr, <-done) + } + closeErr = errors.Join(closeErr, closeServices()) + }) return closeErr }, nil } -func compose(ctx context.Context, store *reviewstore.Store, assets fs.FS, settings Settings) (http.Handler, func() error, serverapp.Task, error) { +func compose(ctx context.Context, store *reviewstore.Store, assets fs.FS, settings Settings) (http.Handler, func() error, []serverapp.Task, error) { identity := identityservice.New(store) if err := bootstrap(ctx, identity, settings); err != nil { return nil, nil, nil, err } + github, err := composeGitHub(store, settings.GitHub) + if err != nil { + return nil, nil, nil, err + } applications := ®istry{ctx: ctx, store: store, assets: assets, applications: make(map[string]application)} - remote := &transport{identity: identity, applications: applications, settings: settings, assets: assets, limiter: newLimiter(), authSlots: make(chan struct{}, 4)} + remote := &transport{identity: identity, applications: applications, settings: settings, assets: assets, limiter: newLimiter(), authSlots: make(chan struct{}, 4), github: github} worker := func(ctx context.Context) error { return ingestionqueue.Run(ctx, store.Queue(), func(ctx context.Context, owner string, input ingestion.Request) (string, error) { accounts, err := identity.Accounts(ctx) @@ -118,7 +131,11 @@ func compose(ctx context.Context, store *reviewstore.Store, assets fs.FS, settin return "", review.Error(403, "Account unavailable") }) } - return remote, applications.close, worker, nil + tasks := []serverapp.Task{worker} + if github != nil { + tasks = append(tasks, reconcileAtStartup(github)) + } + return remote, applications.close, tasks, nil } func bootstrap(ctx context.Context, identity *identityservice.Service, settings Settings) error { if (settings.Username == "") != (settings.Password == "") { @@ -157,7 +174,7 @@ func Run(ctx context.Context, settings Settings, ready func(string)) error { if err := store.PruneExpired(time.Now()); err != nil { return err } - handler, closeServices, worker, err := compose(ctx, store, webui.Assets(), settings) + handler, closeServices, tasks, err := compose(ctx, store, webui.Assets(), settings) if err != nil { return err } @@ -169,7 +186,7 @@ func Run(ctx context.Context, settings Settings, ready func(string)) error { defer listener.Close() server := &http.Server{Handler: handler, ReadHeaderTimeout: 5 * time.Second, ReadTimeout: 2 * time.Minute, IdleTimeout: 60 * time.Second} defer server.Close() - return serverapp.Run(ctx, []serverapp.Endpoint{{Server: server, Listener: listener}}, []serverapp.Task{worker, serverapp.Prune(store.PruneExpired)}, func() { + return serverapp.Run(ctx, []serverapp.Endpoint{{Server: server, Listener: listener}}, append(tasks, serverapp.Prune(store.PruneExpired)), func() { if ready != nil { ready(listener.Addr().String()) } diff --git a/internal/reviewstore/github.go b/internal/reviewstore/github.go new file mode 100644 index 00000000..356fb4d4 --- /dev/null +++ b/internal/reviewstore/github.go @@ -0,0 +1,213 @@ +package reviewstore + +import ( + "context" + "database/sql" + "errors" + "strings" + + "github.com/flexdinesh/diffx/internal/githubsync" +) + +// GitHub integration state arrived in schema 12; every table is additive. +func initializeGitHub(transaction *sql.Tx) error { + statements := []string{ + `CREATE TABLE IF NOT EXISTS github_oauth_states (state_hash TEXT PRIMARY KEY, owner_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, expires_at INTEGER NOT NULL)`, + `CREATE TABLE IF NOT EXISTS github_connections (owner_id TEXT PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, user_id INTEGER NOT NULL, login TEXT NOT NULL, access_token BLOB NOT NULL, access_expires_at INTEGER NOT NULL, refresh_token BLOB NOT NULL, refresh_expires_at INTEGER NOT NULL, status TEXT NOT NULL, connected_at INTEGER NOT NULL)`, + `CREATE INDEX IF NOT EXISTS github_connections_user ON github_connections(user_id)`, + `CREATE TABLE IF NOT EXISTS github_follows (owner_id TEXT NOT NULL REFERENCES github_connections(owner_id) ON DELETE CASCADE, repository_id INTEGER NOT NULL, installation_id INTEGER NOT NULL, full_name TEXT NOT NULL, status TEXT NOT NULL, verified_at INTEGER NOT NULL, created_at INTEGER NOT NULL, PRIMARY KEY(owner_id, repository_id))`, + `CREATE INDEX IF NOT EXISTS github_follows_repository ON github_follows(repository_id, status)`, + `CREATE INDEX IF NOT EXISTS github_follows_installation ON github_follows(installation_id)`, + `CREATE TABLE IF NOT EXISTS github_deliveries (id TEXT PRIMARY KEY, event TEXT NOT NULL, received_at INTEGER NOT NULL)`, + // One row per pull request coalesces refresh intent: webhooks and + // reconciliation advance generation; the refresh task records the last + // generation it processed and the state it observed. + `CREATE TABLE IF NOT EXISTS github_pull_requests (repository_id INTEGER NOT NULL, number INTEGER NOT NULL, installation_id INTEGER NOT NULL, full_name TEXT NOT NULL, generation INTEGER NOT NULL, refreshed_generation INTEGER NOT NULL DEFAULT 0, state TEXT NOT NULL DEFAULT 'open', retention_run TEXT NOT NULL DEFAULT '', requested_at INTEGER NOT NULL, PRIMARY KEY(repository_id, number))`, + `CREATE INDEX IF NOT EXISTS github_pull_requests_pending ON github_pull_requests(requested_at) WHERE generation>refreshed_generation`, + } + for _, statement := range statements { + if _, err := transaction.Exec(statement); err != nil { + return err + } + } + return nil +} + +// GitHub adapts the store to the integration's storage contract. +func (store *Store) GitHub() *GitHubStore { return &GitHubStore{store: store} } + +type GitHubStore struct{ store *Store } + +func (s *GitHubStore) PutOAuthState(ctx context.Context, hash, ownerID string, expiresAt int64) error { + _, err := s.store.db.ExecContext(ctx, `INSERT INTO github_oauth_states(state_hash,owner_id,expires_at) VALUES(?,?,?)`, hash, ownerID, expiresAt) + return err +} + +func (s *GitHubStore) TakeOAuthState(ctx context.Context, hash string, now int64) (string, error) { + var owner string + err := s.store.db.QueryRowContext(ctx, `DELETE FROM github_oauth_states WHERE state_hash=? AND expires_at>? RETURNING owner_id`, hash, now).Scan(&owner) + if errors.Is(err, sql.ErrNoRows) { + return "", githubsync.ErrNotFound + } + return owner, err +} + +func (s *GitHubStore) PutConnection(ctx context.Context, c githubsync.Connection) error { + _, err := s.store.db.ExecContext(ctx, `INSERT INTO github_connections(owner_id,user_id,login,access_token,access_expires_at,refresh_token,refresh_expires_at,status,connected_at) + VALUES(?,?,?,?,?,?,?,?,?) ON CONFLICT(owner_id) DO UPDATE SET user_id=excluded.user_id,login=excluded.login,access_token=excluded.access_token, + access_expires_at=excluded.access_expires_at,refresh_token=excluded.refresh_token,refresh_expires_at=excluded.refresh_expires_at,status=excluded.status,connected_at=excluded.connected_at`, + c.OwnerID, c.UserID, c.Login, c.AccessToken, c.AccessExpiresAt, c.RefreshToken, c.RefreshExpiresAt, c.Status, c.ConnectedAt) + return err +} + +const connectionColumns = `owner_id,user_id,login,access_token,access_expires_at,refresh_token,refresh_expires_at,status,connected_at` + +func scanConnection(row scanner) (githubsync.Connection, error) { + var c githubsync.Connection + err := row.Scan(&c.OwnerID, &c.UserID, &c.Login, &c.AccessToken, &c.AccessExpiresAt, &c.RefreshToken, &c.RefreshExpiresAt, &c.Status, &c.ConnectedAt) + if errors.Is(err, sql.ErrNoRows) { + return c, githubsync.ErrNotFound + } + return c, err +} + +func (s *GitHubStore) Connection(ctx context.Context, ownerID string) (githubsync.Connection, error) { + return scanConnection(s.store.db.QueryRowContext(ctx, `SELECT `+connectionColumns+` FROM github_connections WHERE owner_id=?`, ownerID)) +} + +func (s *GitHubStore) Connections(ctx context.Context) ([]githubsync.Connection, error) { + rows, err := s.store.db.QueryContext(ctx, `SELECT `+connectionColumns+` FROM github_connections ORDER BY owner_id`) + if err != nil { + return nil, err + } + defer rows.Close() + result := make([]githubsync.Connection, 0) + for rows.Next() { + c, err := scanConnection(rows) + if err != nil { + return nil, err + } + result = append(result, c) + } + return result, rows.Err() +} + +// Follows cascade with their connection. +func (s *GitHubStore) DeleteConnection(ctx context.Context, ownerID string) error { + _, err := s.store.db.ExecContext(ctx, `DELETE FROM github_connections WHERE owner_id=?`, ownerID) + return err +} + +func (s *GitHubStore) RevokeUser(ctx context.Context, userID int64) error { + transaction, err := s.store.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer transaction.Rollback() + if _, err := transaction.ExecContext(ctx, `UPDATE github_connections SET status='revoked' WHERE user_id=?`, userID); err != nil { + return err + } + if _, err := transaction.ExecContext(ctx, `UPDATE github_follows SET status='lost' WHERE owner_id IN (SELECT owner_id FROM github_connections WHERE user_id=?)`, userID); err != nil { + return err + } + return transaction.Commit() +} + +func (s *GitHubStore) PutFollow(ctx context.Context, f githubsync.Follow) error { + _, err := s.store.db.ExecContext(ctx, `INSERT INTO github_follows(owner_id,repository_id,installation_id,full_name,status,verified_at,created_at) VALUES(?,?,?,?,?,?,?) + ON CONFLICT(owner_id,repository_id) DO UPDATE SET installation_id=excluded.installation_id,full_name=excluded.full_name,status=excluded.status,verified_at=excluded.verified_at`, + f.OwnerID, f.RepositoryID, f.InstallationID, f.FullName, f.Status, f.VerifiedAt, f.CreatedAt) + return err +} + +func (s *GitHubStore) DeleteFollow(ctx context.Context, ownerID string, repositoryID int64) error { + _, err := s.store.db.ExecContext(ctx, `DELETE FROM github_follows WHERE owner_id=? AND repository_id=?`, ownerID, repositoryID) + return err +} + +func (s *GitHubStore) follows(ctx context.Context, where string, argument any) ([]githubsync.Follow, error) { + rows, err := s.store.db.QueryContext(ctx, `SELECT owner_id,repository_id,installation_id,full_name,status,verified_at,created_at FROM github_follows WHERE `+where+` ORDER BY full_name,owner_id`, argument) + if err != nil { + return nil, err + } + defer rows.Close() + result := make([]githubsync.Follow, 0) + for rows.Next() { + var f githubsync.Follow + if err := rows.Scan(&f.OwnerID, &f.RepositoryID, &f.InstallationID, &f.FullName, &f.Status, &f.VerifiedAt, &f.CreatedAt); err != nil { + return nil, err + } + result = append(result, f) + } + return result, rows.Err() +} + +func (s *GitHubStore) Follows(ctx context.Context, ownerID string) ([]githubsync.Follow, error) { + return s.follows(ctx, "owner_id=?", ownerID) +} + +func (s *GitHubStore) RepositoryFollows(ctx context.Context, repositoryID int64) ([]githubsync.Follow, error) { + return s.follows(ctx, "repository_id=?", repositoryID) +} + +func (s *GitHubStore) LoseInstallation(ctx context.Context, installationID int64, repositoryIDs []int64) error { + query := `UPDATE github_follows SET status='lost' WHERE installation_id=?` + arguments := []any{installationID} + if len(repositoryIDs) > 0 { + query += ` AND repository_id IN (?` + strings.Repeat(",?", len(repositoryIDs)-1) + `)` + for _, id := range repositoryIDs { + arguments = append(arguments, id) + } + } + _, err := s.store.db.ExecContext(ctx, query, arguments...) + return err +} + +func (s *GitHubStore) SeenDelivery(ctx context.Context, id string) (bool, error) { + var found int + err := s.store.db.QueryRowContext(ctx, `SELECT 1 FROM github_deliveries WHERE id=?`, id).Scan(&found) + if errors.Is(err, sql.ErrNoRows) { + return false, nil + } + return err == nil, err +} + +func (s *GitHubStore) RecordDelivery(ctx context.Context, id, event string, now int64) error { + _, err := s.store.db.ExecContext(ctx, `INSERT INTO github_deliveries(id,event,received_at) VALUES(?,?,?) ON CONFLICT DO NOTHING`, id, event, now) + return err +} + +func (s *GitHubStore) RequestRefresh(ctx context.Context, ref githubsync.PullRequestRef, retentionRun string, now int64) error { + _, err := s.store.db.ExecContext(ctx, `INSERT INTO github_pull_requests(repository_id,number,installation_id,full_name,generation,retention_run,requested_at) VALUES(?,?,?,?,1,?,?) + ON CONFLICT(repository_id,number) DO UPDATE SET installation_id=excluded.installation_id,full_name=excluded.full_name,generation=generation+1, + retention_run=CASE WHEN excluded.retention_run<>'' THEN excluded.retention_run ELSE retention_run END,requested_at=excluded.requested_at`, + ref.RepositoryID, ref.Number, ref.InstallationID, ref.FullName, retentionRun, now) + return err +} + +func (s *GitHubStore) KnownOpen(ctx context.Context, repositoryID int64) ([]int, error) { + rows, err := s.store.db.QueryContext(ctx, `SELECT number FROM github_pull_requests WHERE repository_id=? AND state='open' ORDER BY number`, repositoryID) + if err != nil { + return nil, err + } + defer rows.Close() + result := make([]int, 0) + for rows.Next() { + var number int + if err := rows.Scan(&number); err != nil { + return nil, err + } + result = append(result, number) + } + return result, rows.Err() +} + +// pruneGitHub removes expired OAuth states and delivery identities older than +// GitHub's redelivery window with margin. +func pruneGitHub(transaction *sql.Tx, now int64) error { + if _, err := transaction.Exec(`DELETE FROM github_oauth_states WHERE expires_at<=?`, now); err != nil { + return err + } + _, err := transaction.Exec(`DELETE FROM github_deliveries WHERE received_at<=?`, now-7*24*60*60*1000) + return err +} diff --git a/internal/reviewstore/reset_test.go b/internal/reviewstore/reset_test.go index 5c09b445..453355f4 100644 --- a/internal/reviewstore/reset_test.go +++ b/internal/reviewstore/reset_test.go @@ -59,7 +59,8 @@ func TestAdditiveSchemaUpgradesInPlace(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := store.db.Exec("PRAGMA user_version=11"); err != nil { + // Schema 11 had no GitHub integration tables. + if _, err := store.db.Exec("DROP TABLE github_follows; DROP TABLE github_connections; DROP TABLE github_oauth_states; DROP TABLE github_deliveries; DROP TABLE github_pull_requests; PRAGMA user_version=11"); err != nil { t.Fatal(err) } store.Close() @@ -68,6 +69,9 @@ func TestAdditiveSchemaUpgradesInPlace(t *testing.T) { t.Fatalf("schema 11 must upgrade in place: %v", err) } defer store.Close() + if _, err := store.GitHub().Follows(t.Context(), user.ID); err != nil { + t.Fatalf("upgrade did not create integration tables: %v", err) + } var version int if err := store.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != schemaVersion { t.Fatalf("upgraded version %d %v", version, err) @@ -78,6 +82,35 @@ func TestAdditiveSchemaUpgradesInPlace(t *testing.T) { } } +func TestFailedUpgradeRollsBack(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.db") + store, err := Open(path) + if err != nil { + t.Fatal(err) + } + // A conflicting object makes the schema 12 statements fail midway. + if _, err := store.db.Exec("DROP TABLE github_follows; DROP TABLE github_oauth_states; CREATE VIEW github_follows AS SELECT 1 AS owner_id; PRAGMA user_version=11"); err != nil { + t.Fatal(err) + } + store.Close() + if store, err := Open(path); err == nil { + store.Close() + t.Fatal("conflicting upgrade succeeded") + } + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + defer db.Close() + var version, tables int + if err := db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 11 { + t.Fatalf("failed upgrade advanced version: %d %v", version, err) + } + if err := db.QueryRow("SELECT COUNT(*) FROM sqlite_master WHERE name='github_oauth_states'").Scan(&tables); err != nil || tables != 0 { + t.Fatalf("failed upgrade kept partial tables: %d %v", tables, err) + } +} + func TestCurrentSchemaSurvivesRestartAndFutureVersionRefused(t *testing.T) { path := filepath.Join(t.TempDir(), "state.db") store, err := Open(path) diff --git a/internal/reviewstore/store.go b/internal/reviewstore/store.go index e630c080..5747510d 100644 --- a/internal/reviewstore/store.go +++ b/internal/reviewstore/store.go @@ -235,6 +235,9 @@ func (store *Store) initialize(path string) error { if err := initializeAuthentication(transaction); err != nil { return err } + if err := initializeGitHub(transaction); err != nil { + return err + } if _, err := transaction.Exec(fmt.Sprintf("PRAGMA user_version = %d", schemaVersion)); err != nil { return err } @@ -297,6 +300,8 @@ func (store *Store) PruneExpired(now time.Time) error { if err := endStreams(transaction); err != nil { return err } - + if err := pruneGitHub(transaction, now.UnixMilli()); err != nil { + return err + } return transaction.Commit() } diff --git a/packages/api/openapi.yaml b/packages/api/openapi.yaml index 7a117e52..fb99c416 100644 --- a/packages/api/openapi.yaml +++ b/packages/api/openapi.yaml @@ -238,6 +238,218 @@ paths: $ref: "#/components/responses/Problem" default: $ref: "#/components/responses/Problem" + /api/v2/github/webhooks: + post: + operationId: receiveGitHubWebhook + description: Distributed only, when the GitHub App integration is configured. Called by GitHub; authenticated by the X-Hub-Signature-256 HMAC, not by diffx credentials. Repeated X-GitHub-Delivery IDs are acknowledged without effect. + parameters: + - { + in: header, + name: X-GitHub-Event, + required: true, + schema: { type: string }, + } + - { + in: header, + name: X-GitHub-Delivery, + required: true, + schema: { type: string }, + } + - { + in: header, + name: X-Hub-Signature-256, + required: true, + schema: { type: string }, + } + requestBody: + required: true + content: + application/json: + schema: + type: object + responses: + "202": + description: Delivery applied or already applied. + "401": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + "413": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/github/callback: + get: + operationId: completeGitHubAuthorization + description: OAuth redirect target. Completes authorization only for the browser session that started it, then redirects to /settings with a github=connected|cancelled|failed|signin query. + parameters: + - { in: query, name: code, schema: { type: string } } + - { in: query, name: state, schema: { type: string } } + responses: + "303": + description: Redirect to settings. + /api/v2/github/connection: + get: + security: + - browserSession: [] + operationId: getGitHubConnection + description: The account's GitHub connection. 404 when the integration is not configured. Distributed browser sessions only. + responses: + "200": + description: Connection status. + content: + application/json: + schema: + $ref: "#/components/schemas/GitHubConnection" + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + post: + security: + - browserSession: [] + operationId: connectGitHub + description: Start GitHub authorization for this account; navigate the browser to authorizeUrl. Distributed browser sessions only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "200": + description: Authorization started. + content: + application/json: + schema: + type: object + additionalProperties: false + required: [authorizeUrl] + properties: + authorizeUrl: + type: string + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + delete: + security: + - browserSession: [] + operationId: disconnectGitHub + description: Remove the account's GitHub authorization and every follow. Existing snapshots remain until retention expiry. Distributed browser sessions only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Disconnected. + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/github/repositories: + get: + security: + - browserSession: [] + operationId: listGitHubRepositories + description: Repositories both the GitHub user and the app can read, plus followed repositories that lost access. Listing rechecks access. Distributed browser sessions only. + responses: + "200": + description: Repositories. + content: + application/json: + schema: + type: object + additionalProperties: false + required: [repositories] + properties: + repositories: + type: array + items: + $ref: "#/components/schemas/GitHubRepository" + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + "409": + $ref: "#/components/responses/Problem" + "429": + $ref: "#/components/responses/Problem" + "502": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/github/follows/{repositoryId}: + parameters: + - in: path + name: repositoryId + required: true + schema: + type: integer + format: int64 + minimum: 1 + put: + security: + - browserSession: [] + operationId: followGitHubRepository + description: Follow a repository's pull requests after verifying access, then request a refresh of its open pull requests. Distributed browser sessions only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Followed. + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + "409": + $ref: "#/components/responses/Problem" + "429": + $ref: "#/components/responses/Problem" + "502": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + delete: + security: + - browserSession: [] + operationId: unfollowGitHubRepository + description: Stop following; existing snapshots remain until retention expiry. Distributed browser sessions only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Unfollowed. + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/github/resync: + post: + security: + - browserSession: [] + operationId: resyncGitHub + description: Recheck access and refresh every followed repository's pull requests, including ones closed without a delivery. Resulting submissions are fresh, extending retention. Distributed browser sessions only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Refresh requested. + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + "409": + $ref: "#/components/responses/Problem" + "429": + $ref: "#/components/responses/Problem" + "502": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" /api/v2/health: get: operationId: getIngestionHealth @@ -1101,6 +1313,39 @@ components: type: integer format: int64 description: Unix milliseconds. + GitHubConnection: + type: object + additionalProperties: false + required: [connected, installUrl] + properties: + connected: + type: boolean + login: + type: string + state: + type: string + enum: [active, revoked, expired] + description: Revoked or expired connections must connect again; their follows receive nothing meanwhile. + installUrl: + type: string + description: GitHub page for installing the app on more repositories. + GitHubRepository: + type: object + additionalProperties: false + required: [id, fullName, private, followed, lost] + properties: + id: + type: integer + format: int64 + fullName: + type: string + private: + type: boolean + followed: + type: boolean + lost: + type: boolean + description: Followed but no longer readable by this account; receives nothing until access returns. SessionAssociation: type: object additionalProperties: false diff --git a/packages/api/src/schema.d.ts b/packages/api/src/schema.d.ts index 473797cc..f0b3ea37 100644 --- a/packages/api/src/schema.d.ts +++ b/packages/api/src/schema.d.ts @@ -124,6 +124,113 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v2/github/webhooks": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** @description Distributed only, when the GitHub App integration is configured. Called by GitHub; authenticated by the X-Hub-Signature-256 HMAC, not by diffx credentials. Repeated X-GitHub-Delivery IDs are acknowledged without effect. */ + post: operations["receiveGitHubWebhook"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/github/callback": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** @description OAuth redirect target. Completes authorization only for the browser session that started it, then redirects to /settings with a github=connected|cancelled|failed|signin query. */ + get: operations["completeGitHubAuthorization"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/github/connection": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** @description The account's GitHub connection. 404 when the integration is not configured. Distributed browser sessions only. */ + get: operations["getGitHubConnection"]; + put?: never; + /** @description Start GitHub authorization for this account; navigate the browser to authorizeUrl. Distributed browser sessions only. */ + post: operations["connectGitHub"]; + /** @description Remove the account's GitHub authorization and every follow. Existing snapshots remain until retention expiry. Distributed browser sessions only. */ + delete: operations["disconnectGitHub"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/github/repositories": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** @description Repositories both the GitHub user and the app can read, plus followed repositories that lost access. Listing rechecks access. Distributed browser sessions only. */ + get: operations["listGitHubRepositories"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/github/follows/{repositoryId}": { + parameters: { + query?: never; + header?: never; + path: { + repositoryId: number; + }; + cookie?: never; + }; + get?: never; + /** @description Follow a repository's pull requests after verifying access, then request a refresh of its open pull requests. Distributed browser sessions only. */ + put: operations["followGitHubRepository"]; + post?: never; + /** @description Stop following; existing snapshots remain until retention expiry. Distributed browser sessions only. */ + delete: operations["unfollowGitHubRepository"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/github/resync": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** @description Recheck access and refresh every followed repository's pull requests, including ones closed without a delivery. Resulting submissions are fresh, extending retention. Distributed browser sessions only. */ + post: operations["resyncGitHub"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v2/health": { parameters: { query?: never; @@ -616,6 +723,26 @@ export interface components { */ updatedAt: number; }; + GitHubConnection: { + connected: boolean; + login?: string; + /** + * @description Revoked or expired connections must connect again; their follows receive nothing meanwhile. + * @enum {string} + */ + state?: "active" | "revoked" | "expired"; + /** @description GitHub page for installing the app on more repositories. */ + installUrl: string; + }; + GitHubRepository: { + /** Format: int64 */ + id: number; + fullName: string; + private: boolean; + followed: boolean; + /** @description Followed but no longer readable by this account; receives nothing until access returns. */ + lost: boolean; + }; SessionAssociation: { sourceId: string; harness: string; @@ -1254,6 +1381,242 @@ export interface operations { default: components["responses"]["Problem"]; }; }; + receiveGitHubWebhook: { + parameters: { + query?: never; + header: { + "X-GitHub-Event": string; + "X-GitHub-Delivery": string; + "X-Hub-Signature-256": string; + }; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": Record; + }; + }; + responses: { + /** @description Delivery applied or already applied. */ + 202: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 401: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + 413: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + completeGitHubAuthorization: { + parameters: { + query?: { + code?: string; + state?: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Redirect to settings. */ + 303: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + getGitHubConnection: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Connection status. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["GitHubConnection"]; + }; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + connectGitHub: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Authorization started. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + authorizeUrl: string; + }; + }; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + disconnectGitHub: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Disconnected. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + listGitHubRepositories: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Repositories. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + repositories: components["schemas"]["GitHubRepository"][]; + }; + }; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + 409: components["responses"]["Problem"]; + 429: components["responses"]["Problem"]; + 502: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + followGitHubRepository: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path: { + repositoryId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Followed. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + 409: components["responses"]["Problem"]; + 429: components["responses"]["Problem"]; + 502: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + unfollowGitHubRepository: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path: { + repositoryId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Unfollowed. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + resyncGitHub: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Refresh requested. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + 409: components["responses"]["Problem"]; + 429: components["responses"]["Problem"]; + 502: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; getIngestionHealth: { parameters: { query?: never; diff --git a/tools/check-boundaries/main.go b/tools/check-boundaries/main.go index 8015e6a7..a637657f 100644 --- a/tools/check-boundaries/main.go +++ b/tools/check-boundaries/main.go @@ -29,7 +29,10 @@ var allowed = map[string][]string{ "internal/contextservice": {"internal/review", "internal/reviewdata", "internal/reviewsession", "internal/ingestion"}, "internal/controlapi": {"internal/config", "internal/review"}, "internal/diffsource": {"internal/processlock", "internal/review"}, - "internal/hooks": {"internal/ingestion", "internal/processlock", "internal/submission"}, + // GitHub wire adapter; integration policy lives in githubsync. + "internal/github": {}, + "internal/githubsync": {"internal/github"}, + "internal/hooks": {"internal/ingestion", "internal/processlock", "internal/submission"}, "internal/httpapi": { "internal/review", "internal/reviewdata", "internal/reviewsession", "internal/contextservice", "internal/reviewservice", "internal/ingestion", "internal/processmetrics", "packages/api", @@ -45,14 +48,14 @@ var allowed = map[string][]string{ "internal/processlock": {}, "internal/processmetrics": {}, "internal/remoteserver": { - "internal/contextservice", "internal/identityservice", "internal/ingestion", "internal/ingestionqueue", "internal/review", + "internal/contextservice", "internal/github", "internal/githubsync", "internal/identityservice", "internal/ingestion", "internal/ingestionqueue", "internal/review", "internal/reviewstore", "internal/serverapp", "internal/webui", "packages/api", }, "internal/review": {}, "internal/reviewdata": {"internal/review", "internal/ingestion"}, "internal/reviewservice": {"internal/review", "internal/reviewdata", "internal/reviewsession", "internal/contextservice", "internal/ingestion"}, "internal/reviewsession": {"internal/review"}, - "internal/reviewstore": {"internal/identityservice", "internal/review", "internal/reviewdata", "internal/ingestion", "internal/ingestionqueue", "internal/processlock"}, + "internal/reviewstore": {"internal/githubsync", "internal/identityservice", "internal/review", "internal/reviewdata", "internal/ingestion", "internal/ingestionqueue", "internal/processlock"}, "internal/serverapp": { "internal/contextservice", "internal/httpapi", "internal/ingestion", "internal/ingestionqueue", "internal/mcpapi", "internal/review", "internal/reviewdata", "internal/reviewservice", "internal/version",