From f0251e19245ccf67f6d329c40015d9a95fd81327 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Wed, 3 Jun 2026 19:18:59 +0530 Subject: [PATCH 1/3] =?UTF-8?q?feat(github-app):=20P4.1=20=E2=80=94=20inst?= =?UTF-8?q?all=20flow=20+=20installation=20token=20minter,=20flag-OFF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of the GitHub App layer (P4) on top of the existing manual per-repo webhook: a team installs the InstaNode App once instead of pasting a webhook into each repo, and private-repo clones can use short-lived, least-privilege, server-minted installation tokens instead of a long-lived PAT. Gated by GITHUB_APP_ENABLED (default false) → /integrations/github/{install,callback} return 501 github_app_disabled until the operator registers the App (infra/GITHUB-APP-RUNBOOK.md). Manual webhook + source=git remain unchanged. - config: GitHubAppEnabled flag + GITHUB_APP_{ID,SLUG,PRIVATE_KEY,WEBHOOK_SECRET, CLIENT_ID,CLIENT_SECRET} (operator k8s secrets) + default-OFF/true/plumbing tests. - migration 066: github_installations (installation_id PK ↔ team) + testhelpers DDL. - internal/github: App token minter — RS256 App-JWT (≤10m) → POST /app/installations/{id}/access_tokens (contents:read, ~1h), Redis-cached ~55m, never persisted at rest. Injectable HTTP client + clock; 100% covered (mocked GitHub + miniredis cache + all error arms). - models: github_installations CRUD (upsert/get/list/suspend/delete), sqlmock-tested. - handlers: GET /integrations/github/install (RequireAuth → 302 to the App page with a signed anti-CSRF state) + /callback (state-verified → persist link → 302 to dashboard). HS256 state sign/verify (100% covered) + DB-gated handler integration tests (flag-off 501, misconfigured 503, install 302, callback invalid-state/invalid-id/happy-persist). - agent_action entries for the new codes; router + testhelpers wiring. Token minting into the source=git clone path + the App webhook (push→auto redeploy) are P4.2. Contract sync (openapi/llms/MCP) + dashboard UI are P4.3, deferred to flag-on (as with P2/P3). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/config/config.go | 29 ++ internal/config/config_test.go | 152 +++++++---- .../migrations/066_github_installations.sql | 26 ++ internal/github/app.go | 138 ++++++++++ internal/github/app_test.go | 258 ++++++++++++++++++ internal/handlers/github_app.go | 160 +++++++++++ .../handlers/github_app_integration_test.go | 169 ++++++++++++ internal/handlers/github_app_state_test.go | 78 ++++++ internal/handlers/helpers.go | 15 + internal/models/github_installation.go | 144 ++++++++++ internal/models/github_installation_test.go | 136 +++++++++ internal/router/router.go | 7 + internal/testhelpers/testhelpers.go | 15 + 13 files changed, 1268 insertions(+), 59 deletions(-) create mode 100644 internal/db/migrations/066_github_installations.sql create mode 100644 internal/github/app.go create mode 100644 internal/github/app_test.go create mode 100644 internal/handlers/github_app.go create mode 100644 internal/handlers/github_app_integration_test.go create mode 100644 internal/handlers/github_app_state_test.go create mode 100644 internal/models/github_installation.go create mode 100644 internal/models/github_installation_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 90fd9d49..1431f070 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -188,6 +188,20 @@ type Config struct { // Off → /deploy/new rejects source=git with 501; tarball/image unaffected. DeploySourceGitEnabled bool + // GitHub App (P4) — install-once push-to-deploy + short-lived installation + // tokens for private-repo clones. Distinct from the GitHub OAuth *login* app + // above (GitHubClientID/Secret). GitHubAppEnabled gates the whole feature: + // off → /integrations/github/* and POST /webhooks/github reject with 501. + // All values are operator k8s secrets (NOT ConfigMap) — the private key can + // mint tokens for every installation. + GitHubAppEnabled bool + GitHubAppID string // GITHUB_APP_ID — numeric App ID (JWT iss) + GitHubAppSlug string // GITHUB_APP_SLUG — public slug for the install URL (github.com/apps/) + GitHubAppPrivateKey string // GITHUB_APP_PRIVATE_KEY — RSA PEM (RS256 signing) + GitHubAppWebhookSecret string // GITHUB_APP_WEBHOOK_SECRET — X-Hub-Signature-256 HMAC key + GitHubAppClientID string // GITHUB_APP_CLIENT_ID — for the install/callback OAuth handshake + GitHubAppClientSecret string // GITHUB_APP_CLIENT_SECRET + // Email-feedback webhook secrets. Each provider authenticates its // callbacks differently — these env vars give the handler the shared // secret (Brevo, SendGrid) or topic ARN (SES via SNS) it needs to @@ -451,6 +465,21 @@ func Load() *Config { cfg.DeploySourceGitEnabled = false } + // GITHUB_APP_ENABLED: default FALSE (off until the operator registers the + // App and provisions GITHUB_APP_* secrets — see infra/GITHUB-APP-RUNBOOK.md). + switch strings.ToLower(strings.TrimSpace(os.Getenv("GITHUB_APP_ENABLED"))) { + case "true", "1", "yes": + cfg.GitHubAppEnabled = true + default: + cfg.GitHubAppEnabled = false + } + cfg.GitHubAppID = os.Getenv("GITHUB_APP_ID") + cfg.GitHubAppSlug = os.Getenv("GITHUB_APP_SLUG") + cfg.GitHubAppPrivateKey = os.Getenv("GITHUB_APP_PRIVATE_KEY") + cfg.GitHubAppWebhookSecret = os.Getenv("GITHUB_APP_WEBHOOK_SECRET") + cfg.GitHubAppClientID = os.Getenv("GITHUB_APP_CLIENT_ID") + cfg.GitHubAppClientSecret = os.Getenv("GITHUB_APP_CLIENT_SECRET") + if len(cfg.JWTSecret) < 32 { panic("JWT_SECRET must be at least 32 bytes") } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 89448308..49fc244c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -63,6 +63,8 @@ func allKeys() []string { "METRICS_TOKEN", "DASHBOARD_BASE_URL", "API_PUBLIC_URL", "DELETION_CONFIRMATION_TTL_MINUTES", "FAMILY_BINDINGS_ENABLED", "DEPLOY_SOURCE_IMAGE_ENABLED", "DEPLOY_SOURCE_GIT_ENABLED", + "GITHUB_APP_ENABLED", "GITHUB_APP_ID", "GITHUB_APP_SLUG", "GITHUB_APP_PRIVATE_KEY", + "GITHUB_APP_WEBHOOK_SECRET", "GITHUB_APP_CLIENT_ID", "GITHUB_APP_CLIENT_SECRET", "BREVO_WEBHOOK_SECRET", "SES_SNS_SUBSCRIPTION_ARN", "SENDGRID_WEBHOOK_PUBLIC_KEY", "WORKER_INTERNAL_JWT_SECRET", "ADMIN_PATH_PREFIX", @@ -248,6 +250,9 @@ func TestLoad_HappyPath_AppliesDefaults(t *testing.T) { if cfg.DeploySourceGitEnabled { t.Error("DeploySourceGitEnabled default must be false (off until operator canary)") } + if cfg.GitHubAppEnabled { + t.Error("GitHubAppEnabled default must be false (off until operator registers the App)") + } // Object store mode resolution: with everything empty → "admin" / "minio-admin" if cfg.ObjectStoreMode != "admin" || cfg.ObjectStoreBackend != "minio-admin" { t.Errorf("ObjectStoreMode/Backend defaults: %q/%q", cfg.ObjectStoreMode, cfg.ObjectStoreBackend) @@ -256,48 +261,48 @@ func TestLoad_HappyPath_AppliesDefaults(t *testing.T) { func TestLoad_OverrideDefaults(t *testing.T) { applyBaselineEnv(t, map[string]string{ - "PORT": "9090", - "REDIS_URL": "redis://r:6379", - "ENVIRONMENT": "production", - "INSTANT_ENABLED_SERVICES": "postgres", - "GEOLITE2_DB_PATH": "/data/geo.mmdb", - "RAZORPAY_KEY_ID": "rzp_test_x", - "RESEND_API_KEY": "re_x", - "BREVO_API_KEY": "br_x", - "EMAIL_PROVIDER": "brevo", - "EMAIL_FROM_ADDRESS": "noreply@x.dev", - "EMAIL_FROM_NAME": "X", - "GITHUB_CLIENT_ID": "gh-x", - "GOOGLE_CLIENT_ID": "g-x", - "GOOGLE_REDIRECT_URI": "https://x/callback", - "REDIS_PROVISION_BACKEND": "upstash", - "REDIS_PROVISION_HOST": "redis.x", - "MONGO_HOST": "mongo.x", - "POSTGRES_PROVISION_BACKEND": "neon", - "NEON_API_KEY": "nk-x", - "PROVISIONER_ADDR": "prov:50051", - "PROVISIONER_SECRET": "ps", - "NATS_HOST": "nats.x", - "QUEUE_BACKEND": "legacy_open", - "NATS_PUBLIC_HOST": "public.x", - "NATS_OPERATOR_SEED": "SO_seed", + "PORT": "9090", + "REDIS_URL": "redis://r:6379", + "ENVIRONMENT": "production", + "INSTANT_ENABLED_SERVICES": "postgres", + "GEOLITE2_DB_PATH": "/data/geo.mmdb", + "RAZORPAY_KEY_ID": "rzp_test_x", + "RESEND_API_KEY": "re_x", + "BREVO_API_KEY": "br_x", + "EMAIL_PROVIDER": "brevo", + "EMAIL_FROM_ADDRESS": "noreply@x.dev", + "EMAIL_FROM_NAME": "X", + "GITHUB_CLIENT_ID": "gh-x", + "GOOGLE_CLIENT_ID": "g-x", + "GOOGLE_REDIRECT_URI": "https://x/callback", + "REDIS_PROVISION_BACKEND": "upstash", + "REDIS_PROVISION_HOST": "redis.x", + "MONGO_HOST": "mongo.x", + "POSTGRES_PROVISION_BACKEND": "neon", + "NEON_API_KEY": "nk-x", + "PROVISIONER_ADDR": "prov:50051", + "PROVISIONER_SECRET": "ps", + "NATS_HOST": "nats.x", + "QUEUE_BACKEND": "legacy_open", + "NATS_PUBLIC_HOST": "public.x", + "NATS_OPERATOR_SEED": "SO_seed", "NATS_SYSTEM_ACCOUNT_PUBLIC_KEY": "ACSYS", - "NATS_USE_TLS": "true", - "R2_ENDPOINT": "r2.x", - "R2_BUCKET_NAME": "x-bucket", - "R2_API_TOKEN": "r2tok", - "DEPLOY_DOMAIN": "x.dev", - "COMPUTE_PROVIDER": "k8s", - "KUBE_NAMESPACE_APPS": "x-apps", - "METRICS_TOKEN": strings.Repeat("M", 64), - "DASHBOARD_BASE_URL": "https://dash.x", - "API_PUBLIC_URL": "https://api.x/", - "BREVO_WEBHOOK_SECRET": "brevo-wh", - "SES_SNS_SUBSCRIPTION_ARN": "arn:aws:sns:x", - "SENDGRID_WEBHOOK_PUBLIC_KEY": "sg-key", - "WORKER_INTERNAL_JWT_SECRET": " worker-secret ", - "TRUSTED_PROXY_CIDRS": "10.0.0.0/8", - "MAXMIND_LICENSE_KEY": "mm", + "NATS_USE_TLS": "true", + "R2_ENDPOINT": "r2.x", + "R2_BUCKET_NAME": "x-bucket", + "R2_API_TOKEN": "r2tok", + "DEPLOY_DOMAIN": "x.dev", + "COMPUTE_PROVIDER": "k8s", + "KUBE_NAMESPACE_APPS": "x-apps", + "METRICS_TOKEN": strings.Repeat("M", 64), + "DASHBOARD_BASE_URL": "https://dash.x", + "API_PUBLIC_URL": "https://api.x/", + "BREVO_WEBHOOK_SECRET": "brevo-wh", + "SES_SNS_SUBSCRIPTION_ARN": "arn:aws:sns:x", + "SENDGRID_WEBHOOK_PUBLIC_KEY": "sg-key", + "WORKER_INTERNAL_JWT_SECRET": " worker-secret ", + "TRUSTED_PROXY_CIDRS": "10.0.0.0/8", + "MAXMIND_LICENSE_KEY": "mm", }) cfg := Load() if cfg.Port != "9090" || cfg.Environment != "production" { @@ -368,6 +373,35 @@ func TestLoad_DeploySourceGitEnabled(t *testing.T) { } } +func TestLoad_GitHubAppEnabled(t *testing.T) { + for _, val := range []string{"true", "1", "yes", "TRUE", " Yes "} { + applyBaselineEnv(t, map[string]string{"GITHUB_APP_ENABLED": val}) + if !Load().GitHubAppEnabled { + t.Errorf("GITHUB_APP_ENABLED=%q should enable", val) + } + } + for _, val := range []string{"false", "0", "no", "maybe", ""} { + applyBaselineEnv(t, map[string]string{"GITHUB_APP_ENABLED": val}) + if Load().GitHubAppEnabled { + t.Errorf("GITHUB_APP_ENABLED=%q should stay disabled", val) + } + } + // the GITHUB_APP_* values are plumbed verbatim. + applyBaselineEnv(t, map[string]string{ + "GITHUB_APP_ID": "12345", + "GITHUB_APP_PRIVATE_KEY": "-----BEGIN RSA PRIVATE KEY-----\nx\n-----END RSA PRIVATE KEY-----", + "GITHUB_APP_WEBHOOK_SECRET": "whsec", + "GITHUB_APP_CLIENT_ID": "Iv1.abc", + "GITHUB_APP_CLIENT_SECRET": "cs", + }) + c := Load() + if c.GitHubAppID != "12345" || c.GitHubAppWebhookSecret != "whsec" || + c.GitHubAppClientID != "Iv1.abc" || c.GitHubAppClientSecret != "cs" || + c.GitHubAppPrivateKey == "" { + t.Errorf("GITHUB_APP_* not plumbed: %+v", c.GitHubAppID) + } +} + func TestLoad_DeletionTTL_OverrideAndInvalid(t *testing.T) { applyBaselineEnv(t, map[string]string{"DELETION_CONFIRMATION_TTL_MINUTES": "30"}) if got := Load().DeletionConfirmationTTLMinutes; got != 30 { @@ -421,16 +455,16 @@ func TestLoad_ObjectStore_MinioFallback(t *testing.T) { func TestLoad_ObjectStore_ExplicitOverridesFallback(t *testing.T) { applyBaselineEnv(t, map[string]string{ - "OBJECT_STORE_ENDPOINT": "nyc3.digitaloceanspaces.com", - "OBJECT_STORE_PUBLIC_URL": "https://s3.instanode.dev", - "OBJECT_STORE_ACCESS_KEY": "AKIA", - "OBJECT_STORE_SECRET_KEY": "SECRET", - "OBJECT_STORE_BUCKET": "do-bucket", - "OBJECT_STORE_REGION": "nyc3", - "OBJECT_STORE_SECURE": "true", + "OBJECT_STORE_ENDPOINT": "nyc3.digitaloceanspaces.com", + "OBJECT_STORE_PUBLIC_URL": "https://s3.instanode.dev", + "OBJECT_STORE_ACCESS_KEY": "AKIA", + "OBJECT_STORE_SECRET_KEY": "SECRET", + "OBJECT_STORE_BUCKET": "do-bucket", + "OBJECT_STORE_REGION": "nyc3", + "OBJECT_STORE_SECURE": "true", "OBJECT_STORE_ALLOW_SHARED_KEY": "true", // Set MINIO_* so we prove they DON'T win. - "MINIO_ENDPOINT": "minio:9000", + "MINIO_ENDPOINT": "minio:9000", "MINIO_ROOT_USER": "ignored", }) cfg := Load() @@ -571,16 +605,16 @@ func TestLoad_RazorpayPlanIDs(t *testing.T) { }) c := Load() checks := map[string]string{ - "hobby": c.RazorpayPlanIDHobby, - "hp": c.RazorpayPlanIDHobbyPlus, - "pro": c.RazorpayPlanIDPro, - "growth": c.RazorpayPlanIDGrowth, - "team": c.RazorpayPlanIDTeam, - "hobby_y": c.RazorpayPlanIDHobbyYearly, - "hp_y": c.RazorpayPlanIDHobbyPlusYearly, - "pro_y": c.RazorpayPlanIDProYearly, - "growth_y": c.RazorpayPlanIDGrowthYearly, - "team_y": c.RazorpayPlanIDTeamYearly, + "hobby": c.RazorpayPlanIDHobby, + "hp": c.RazorpayPlanIDHobbyPlus, + "pro": c.RazorpayPlanIDPro, + "growth": c.RazorpayPlanIDGrowth, + "team": c.RazorpayPlanIDTeam, + "hobby_y": c.RazorpayPlanIDHobbyYearly, + "hp_y": c.RazorpayPlanIDHobbyPlusYearly, + "pro_y": c.RazorpayPlanIDProYearly, + "growth_y": c.RazorpayPlanIDGrowthYearly, + "team_y": c.RazorpayPlanIDTeamYearly, } for tag, got := range checks { want := "plan_" + tag diff --git a/internal/db/migrations/066_github_installations.sql b/internal/db/migrations/066_github_installations.sql new file mode 100644 index 00000000..ea062bf2 --- /dev/null +++ b/internal/db/migrations/066_github_installations.sql @@ -0,0 +1,26 @@ +-- 066_github_installations.sql — GitHub App installations, P4.1. +-- +-- WHY: P4 adds install-once push-to-deploy on top of the existing manual +-- per-repo webhook (app_github_connections). When a team installs the InstaNode +-- GitHub App, GitHub assigns an installation_id; we persist the installation↔team +-- link so that (a) the install/callback flow can bind it, (b) the App webhook can +-- resolve an incoming push's installation_id → owning team before acting, and +-- (c) the token-minter can mint a short-lived installation access token for +-- private-repo clones. +-- +-- We store ONLY the installation_id + account metadata — never an access token +-- (those are minted on demand from the App private key and cached in Redis, 1h +-- TTL). suspended_at is set when GitHub sends an installation `suspend` event so +-- the webhook can stop acting without deleting the row. + +CREATE TABLE IF NOT EXISTS github_installations ( + installation_id BIGINT PRIMARY KEY, + team_id UUID NOT NULL REFERENCES teams(id) ON DELETE CASCADE, + account_login TEXT NOT NULL DEFAULT '', -- the org/user the App is installed on + suspended_at TIMESTAMPTZ, -- non-NULL → installation suspended + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- A team may have multiple installations (personal + orgs); look them up by team. +CREATE INDEX IF NOT EXISTS idx_github_installations_team ON github_installations(team_id); diff --git a/internal/github/app.go b/internal/github/app.go new file mode 100644 index 00000000..0f024e9d --- /dev/null +++ b/internal/github/app.go @@ -0,0 +1,138 @@ +// Package github implements the InstaNode GitHub App (P4) token layer: it signs +// an App JWT with the App's RSA private key and exchanges it for short-lived, +// least-privilege installation access tokens used to clone private repos during +// a source=git build. Tokens are minted on demand and cached in Redis (~55 min); +// the only secret stored at rest is the App private key (an operator k8s secret). +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/redis/go-redis/v9" +) + +// githubAPIBase is the GitHub REST base. A package var so tests can point it at +// a local httptest server (the injected httpDo also intercepts, but keeping the +// base overridable keeps request-shape assertions honest). +var githubAPIBase = "https://api.github.com" + +// installationTokenTTL caches a minted token below GitHub's 1h expiry so a build +// never races the boundary. +const installationTokenTTL = 55 * time.Minute + +// App mints installation access tokens for the InstaNode GitHub App. +type App struct { + appID string + privateKey interface{} // *rsa.PrivateKey (parsed from PEM) + rdb *redis.Client + // httpDo / now are injectable for deterministic tests. + httpDo func(*http.Request) (*http.Response, error) + now func() time.Time +} + +// NewApp parses the App private-key PEM and returns a minter. rdb may be nil +// (no caching — every call mints). Returns an error on a malformed key or empty +// appID so a misconfigured deployment fails loudly at construction, not mid-build. +func NewApp(appID, privateKeyPEM string, rdb *redis.Client) (*App, error) { + if strings.TrimSpace(appID) == "" { + return nil, fmt.Errorf("github app: GITHUB_APP_ID is empty") + } + key, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(privateKeyPEM)) + if err != nil { + return nil, fmt.Errorf("github app: parse private key: %w", err) + } + return &App{ + appID: appID, + privateKey: key, + rdb: rdb, + httpDo: http.DefaultClient.Do, + now: time.Now, + }, nil +} + +// appJWT mints a short-lived RS256 JWT identifying the App (GitHub requires +// exp ≤ 10 min; we use 9 min and backdate iat 30s for clock-skew tolerance). +func (a *App) appJWT() (string, error) { + now := a.now() + claims := jwt.RegisteredClaims{ + Issuer: a.appID, + IssuedAt: jwt.NewNumericDate(now.Add(-30 * time.Second)), + ExpiresAt: jwt.NewNumericDate(now.Add(9 * time.Minute)), + } + tok := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + signed, err := tok.SignedString(a.privateKey) + if err != nil { + return "", fmt.Errorf("github app: sign jwt: %w", err) + } + return signed, nil +} + +// InstallationToken returns a `contents:read`-scoped installation access token, +// served from the Redis cache when warm or freshly minted from GitHub otherwise. +func (a *App) InstallationToken(ctx context.Context, installationID int64) (string, error) { + cacheKey := fmt.Sprintf("ghapp:insttok:%d", installationID) + if a.rdb != nil { + if v, err := a.rdb.Get(ctx, cacheKey).Result(); err == nil && v != "" { + return v, nil + } + } + + appJWT, err := a.appJWT() + if err != nil { + return "", err + } + url := fmt.Sprintf("%s/app/installations/%d/access_tokens", githubAPIBase, installationID) + // Least privilege: only the permissions a clone build needs. + body := strings.NewReader(`{"permissions":{"contents":"read","metadata":"read"}}`) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, body) + if err != nil { + return "", fmt.Errorf("github app: build request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+appJWT) + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + req.Header.Set("Content-Type", "application/json") + + resp, err := a.httpDo(req) + if err != nil { + return "", fmt.Errorf("github app: access_tokens request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if resp.StatusCode != http.StatusCreated { + return "", fmt.Errorf("github app: access_tokens status %d: %s", resp.StatusCode, snippet(raw)) + } + var out struct { + Token string `json:"token"` + ExpiresAt time.Time `json:"expires_at"` + } + if err := json.Unmarshal(raw, &out); err != nil { + return "", fmt.Errorf("github app: decode access_tokens: %w", err) + } + if out.Token == "" { + return "", fmt.Errorf("github app: access_tokens returned an empty token") + } + if a.rdb != nil { + // Best-effort cache; a failure here just means the next clone re-mints. + _ = a.rdb.Set(ctx, cacheKey, out.Token, installationTokenTTL).Err() + } + return out.Token, nil +} + +// snippet truncates an error body for safe logging (never echo a full GitHub +// response into our error chain / logs). +func snippet(b []byte) string { + const max = 200 + s := strings.TrimSpace(string(b)) + if len(s) > max { + return s[:max] + "…" + } + return s +} diff --git a/internal/github/app_test.go b/internal/github/app_test.go new file mode 100644 index 00000000..f3c6f552 --- /dev/null +++ b/internal/github/app_test.go @@ -0,0 +1,258 @@ +package github + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "io" + "net/http" + "strings" + "testing" + "time" + + "github.com/alicebob/miniredis/v2" + "github.com/golang-jwt/jwt/v4" + "github.com/redis/go-redis/v9" +) + +// testKeyPEM generates a throwaway RSA private key in PKCS#1 PEM (the format +// GitHub issues App keys in). +func testKeyPEM(t *testing.T) (string, *rsa.PrivateKey) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("gen key: %v", err) + } + pemBytes := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + return string(pemBytes), key +} + +// roundTripFunc adapts a func to an http transport-ish injectable. +func doFunc(fn func(*http.Request) (*http.Response, error)) func(*http.Request) (*http.Response, error) { + return fn +} + +func jsonResp(status int, body string) *http.Response { + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(strings.NewReader(body)), + Header: make(http.Header), + } +} + +func TestNewApp_Validation(t *testing.T) { + pemStr, _ := testKeyPEM(t) + if _, err := NewApp("", pemStr, nil); err == nil { + t.Error("empty appID must error") + } + if _, err := NewApp("123", "not a pem", nil); err == nil { + t.Error("malformed private key must error") + } + if _, err := NewApp("123", pemStr, nil); err != nil { + t.Errorf("valid config must construct: %v", err) + } +} + +func TestAppJWT_SignsAndVerifies(t *testing.T) { + pemStr, key := testKeyPEM(t) + a, err := NewApp("999", pemStr, nil) + if err != nil { + t.Fatal(err) + } + fixed := time.Unix(1_700_000_000, 0) + a.now = func() time.Time { return fixed } + + tokStr, err := a.appJWT() + if err != nil { + t.Fatalf("appJWT: %v", err) + } + claims := jwt.RegisteredClaims{} + // Skip time-based claim validation: we sign with a fixed past `now`, so an + // exp check against the real clock would (correctly) say "expired". We only + // assert the signature verifies and the claim values are right. + parser := jwt.NewParser(jwt.WithoutClaimsValidation()) + _, err = parser.ParseWithClaims(tokStr, &claims, func(*jwt.Token) (interface{}, error) { + return &key.PublicKey, nil + }) + if err != nil { + t.Fatalf("verify: %v", err) + } + if claims.Issuer != "999" { + t.Errorf("iss = %q want 999", claims.Issuer) + } + // exp must be within GitHub's 10-min cap and iat backdated for skew. + if claims.ExpiresAt.Sub(fixed) > 10*time.Minute { + t.Error("exp exceeds GitHub's 10-min cap") + } + if !claims.IssuedAt.Before(fixed) { + t.Error("iat must be backdated for clock skew") + } +} + +func TestInstallationToken_MintAndRequestShape(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) // nil rdb → no cache, always mints + var gotURL, gotAuth, gotBody string + a.httpDo = doFunc(func(r *http.Request) (*http.Response, error) { + gotURL = r.URL.String() + gotAuth = r.Header.Get("Authorization") + b, _ := io.ReadAll(r.Body) + gotBody = string(b) + return jsonResp(http.StatusCreated, `{"token":"ghs_minted","expires_at":"2026-01-01T00:00:00Z"}`), nil + }) + + tok, err := a.InstallationToken(context.Background(), 42) + if err != nil { + t.Fatalf("InstallationToken: %v", err) + } + if tok != "ghs_minted" { + t.Errorf("token = %q", tok) + } + if !strings.HasSuffix(gotURL, "/app/installations/42/access_tokens") { + t.Errorf("url = %q", gotURL) + } + if !strings.HasPrefix(gotAuth, "Bearer ") { + t.Errorf("auth header must be a Bearer App JWT, got %q", gotAuth) + } + if !strings.Contains(gotBody, `"contents":"read"`) { + t.Errorf("must request least-privilege contents:read, got %q", gotBody) + } +} + +func TestInstallationToken_Non201_Errors(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + return jsonResp(http.StatusForbidden, `{"message":"Bad credentials"}`), nil + }) + if _, err := a.InstallationToken(context.Background(), 7); err == nil || + !strings.Contains(err.Error(), "status 403") { + t.Fatalf("403 must error, got: %v", err) + } +} + +func TestInstallationToken_EmptyToken_Errors(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + return jsonResp(http.StatusCreated, `{"token":"","expires_at":"2026-01-01T00:00:00Z"}`), nil + }) + if _, err := a.InstallationToken(context.Background(), 7); err == nil || + !strings.Contains(err.Error(), "empty token") { + t.Fatalf("empty token must error, got: %v", err) + } +} + +func TestInstallationToken_BadJSON_Errors(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + return jsonResp(http.StatusCreated, `not json`), nil + }) + if _, err := a.InstallationToken(context.Background(), 7); err == nil || + !strings.Contains(err.Error(), "decode") { + t.Fatalf("bad json must error, got: %v", err) + } +} + +func TestInstallationToken_TransportError(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + return nil, io.ErrUnexpectedEOF + }) + if _, err := a.InstallationToken(context.Background(), 7); err == nil || + !strings.Contains(err.Error(), "access_tokens request") { + t.Fatalf("transport error must propagate, got: %v", err) + } +} + +func TestInstallationToken_RedisCache(t *testing.T) { + mr, err := miniredis.Run() + if err != nil { + t.Fatal(err) + } + defer mr.Close() + rdb := redis.NewClient(&redis.Options{Addr: mr.Addr()}) + defer func() { _ = rdb.Close() }() + + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, rdb) + var mints int + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + mints++ + return jsonResp(http.StatusCreated, `{"token":"ghs_cached","expires_at":"2026-01-01T00:00:00Z"}`), nil + }) + + // First call mints + caches. + if tok, err := a.InstallationToken(context.Background(), 55); err != nil || tok != "ghs_cached" { + t.Fatalf("mint: tok=%q err=%v", tok, err) + } + // Second call is served from cache — no second mint. + if tok, err := a.InstallationToken(context.Background(), 55); err != nil || tok != "ghs_cached" { + t.Fatalf("cache hit: tok=%q err=%v", tok, err) + } + if mints != 1 { + t.Errorf("expected exactly 1 mint (then cache), got %d", mints) + } + // The cache key carries the installation id. + if v, _ := mr.Get("ghapp:insttok:55"); v != "ghs_cached" { + t.Errorf("cache key not set: %q", v) + } +} + +// A non-RSA key makes jwt's RS256 SignedString return ErrInvalidKeyType — the +// only way appJWT's (otherwise unreachable) sign-error branch fires. +func TestAppJWT_SignError(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.privateKey = "not an rsa key" + if _, err := a.appJWT(); err == nil || !strings.Contains(err.Error(), "sign jwt") { + t.Fatalf("bad signing key must error, got: %v", err) + } +} + +// Same broken key surfaces through InstallationToken's appJWT() propagation. +func TestInstallationToken_JWTError(t *testing.T) { + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.privateKey = "not an rsa key" + if _, err := a.InstallationToken(context.Background(), 9); err == nil || + !strings.Contains(err.Error(), "sign jwt") { + t.Fatalf("jwt error must propagate, got: %v", err) + } +} + +// An invalid API base makes http.NewRequestWithContext fail (the build-request +// defensive branch). +func TestInstallationToken_BadRequestURL(t *testing.T) { + orig := githubAPIBase + githubAPIBase = "://not a url" + defer func() { githubAPIBase = orig }() + + pemStr, _ := testKeyPEM(t) + a, _ := NewApp("999", pemStr, nil) + a.httpDo = doFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("httpDo must not be reached when request build fails") + return nil, nil + }) + if _, err := a.InstallationToken(context.Background(), 9); err == nil || + !strings.Contains(err.Error(), "build request") { + t.Fatalf("bad URL must error at request build, got: %v", err) + } +} + +func TestSnippet_Truncates(t *testing.T) { + if got := snippet([]byte(" short ")); got != "short" { + t.Errorf("snippet trim: %q", got) + } + long := strings.Repeat("x", 500) + if got := snippet([]byte(long)); len(got) <= len(long) && !strings.HasSuffix(got, "…") { + t.Errorf("snippet must truncate long bodies, got len %d", len(got)) + } +} diff --git a/internal/handlers/github_app.go b/internal/handlers/github_app.go new file mode 100644 index 00000000..ba890533 --- /dev/null +++ b/internal/handlers/github_app.go @@ -0,0 +1,160 @@ +package handlers + +// github_app.go — GitHub App install flow (P4.1). Distinct from the GitHub +// OAuth *login* (auth.go) and from the manual per-repo webhook +// (github_deploy.go): here a team installs the InstaNode GitHub App once, and we +// persist the installation↔team link (github_installations) so the App webhook +// can resolve pushes and the token-minter (internal/github) can mint short-lived +// clone tokens for private repos. +// +// Routes (registered in router.go), all gated by cfg.GitHubAppEnabled: +// GET /integrations/github/install RequireAuth → 302 to the App install page +// with a signed anti-CSRF state. +// GET /integrations/github/callback GitHub redirects here post-install with +// installation_id + state → persist link, +// 302 to the dashboard. +// +// Token minting + push-to-deploy wiring land in P4.2; P4.1 stops at "installed". + +import ( + "database/sql" + "fmt" + "log/slog" + "net/url" + "strconv" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/golang-jwt/jwt/v4" + + "instant.dev/internal/config" + "instant.dev/internal/middleware" + "instant.dev/internal/models" + "instant.dev/internal/plans" +) + +// githubAppStatePurpose tags the state JWT so it can't be confused with a +// session or OAuth-login token. +const githubAppStatePurpose = "gh_app_install" + +// githubAppStateTTL bounds the install→callback round-trip. +const githubAppStateTTL = 10 * time.Minute + +// GitHubAppHandler serves the install/callback flow. +type GitHubAppHandler struct { + db *sql.DB + cfg *config.Config + plan *plans.Registry +} + +// NewGitHubAppHandler constructs the handler. +func NewGitHubAppHandler(db *sql.DB, cfg *config.Config, planRegistry *plans.Registry) *GitHubAppHandler { + return &GitHubAppHandler{db: db, cfg: cfg, plan: planRegistry} +} + +// disabledOrMisconfigured returns a non-nil error response when the App feature +// is off or its config is incomplete, so both handlers fail the same way. +func (h *GitHubAppHandler) disabledOrMisconfigured(c *fiber.Ctx) error { + if !h.cfg.GitHubAppEnabled { + return respondError(c, fiber.StatusNotImplemented, "github_app_disabled", + "GitHub App install is rolling out and not yet enabled. Use POST /api/v1/deployments/:id/github (manual webhook) or source=git with a token for now.") + } + if h.cfg.GitHubAppSlug == "" { + return respondError(c, fiber.StatusServiceUnavailable, "github_app_misconfigured", + "GitHub App is enabled but GITHUB_APP_SLUG is unset") + } + return nil +} + +// Install (GET /integrations/github/install) redirects the authed team to the +// GitHub App install page, carrying a signed state that binds the eventual +// callback to this team. +func (h *GitHubAppHandler) Install(c *fiber.Ctx) error { + if err := h.disabledOrMisconfigured(c); err != nil { + return err + } + teamID := middleware.GetTeamID(c) + if teamID == "" { + return respondError(c, fiber.StatusUnauthorized, "unauthorized", "A session token is required") + } + state, err := signGitHubAppState(h.cfg.JWTSecret, teamID, time.Now()) + if err != nil { + slog.Error("github_app.install.state_sign_failed", "error", err) + return respondError(c, fiber.StatusServiceUnavailable, "state_unavailable", + "Could not start the GitHub App install") + } + installURL := fmt.Sprintf("https://github.com/apps/%s/installations/new?state=%s", + url.PathEscape(h.cfg.GitHubAppSlug), url.QueryEscape(state)) + return c.Redirect(installURL, fiber.StatusFound) +} + +// Callback (GET /integrations/github/callback) is where GitHub returns the user +// after install. It verifies the state, persists the installation↔team link, and +// bounces to the dashboard. +func (h *GitHubAppHandler) Callback(c *fiber.Ctx) error { + if err := h.disabledOrMisconfigured(c); err != nil { + return err + } + teamID, err := verifyGitHubAppState(h.cfg.JWTSecret, c.Query("state")) + if err != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_state", + "The install state is missing, expired, or invalid — restart the install from the dashboard") + } + instIDStr := c.Query("installation_id") + instID, perr := strconv.ParseInt(instIDStr, 10, 64) + if perr != nil || instID <= 0 { + return respondError(c, fiber.StatusBadRequest, "invalid_installation_id", + "GitHub did not return a valid installation_id") + } + teamUUID, terr := parseTeamID(teamID) + if terr != nil { + return respondError(c, fiber.StatusBadRequest, "invalid_team", "Team ID in state is not a valid UUID") + } + // account_login is enriched by the installation webhook (P4.2); empty here. + if _, err := models.UpsertGitHubInstallation(c.Context(), h.db, instID, teamUUID, ""); err != nil { + slog.Error("github_app.callback.persist_failed", "error", err, "installation_id", instID) + return respondError(c, fiber.StatusServiceUnavailable, "install_persist_failed", + "Could not save the GitHub installation") + } + slog.Info("github_app.callback.installed", "installation_id", instID, "team_id", teamID, + "setup_action", c.Query("setup_action")) + dest := h.cfg.DashboardBaseURL + "/integrations/github?installed=" + strconv.FormatInt(instID, 10) + return c.Redirect(dest, fiber.StatusFound) +} + +// signGitHubAppState mints a short-lived HS256 state token binding an install to +// a team (anti-CSRF — the callback must present a state WE issued). +func signGitHubAppState(secret, teamID string, now time.Time) (string, error) { + claims := jwt.MapClaims{ + "team_id": teamID, + "purpose": githubAppStatePurpose, + "iat": now.Unix(), + "exp": now.Add(githubAppStateTTL).Unix(), + } + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + return tok.SignedString([]byte(secret)) +} + +// verifyGitHubAppState validates the state token (signature, HS256-only, exp via +// the jwt package's default clock) and returns the bound team_id. +func verifyGitHubAppState(secret, raw string) (string, error) { + if raw == "" { + return "", fmt.Errorf("empty state") + } + claims := jwt.MapClaims{} + parser := jwt.NewParser(jwt.WithValidMethods([]string{"HS256"})) + _, err := parser.ParseWithClaims(raw, claims, func(*jwt.Token) (interface{}, error) { + return []byte(secret), nil + }) + if err != nil { + return "", err + } + if p, _ := claims["purpose"].(string); p != githubAppStatePurpose { + return "", fmt.Errorf("state purpose mismatch") + } + teamID, _ := claims["team_id"].(string) + if teamID == "" { + return "", fmt.Errorf("state missing team_id") + } + return teamID, nil +} diff --git a/internal/handlers/github_app_integration_test.go b/internal/handlers/github_app_integration_test.go new file mode 100644 index 00000000..eed7ad1e --- /dev/null +++ b/internal/handlers/github_app_integration_test.go @@ -0,0 +1,169 @@ +package handlers_test + +// github_app_integration_test.go — end-to-end coverage for the GitHub App +// install flow (P4.1): flag-off 501, misconfigured 503, install 302, and the +// callback (invalid state / invalid installation_id / happy persist+redirect). +// DB-gated; skips locally without a DB. + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/config" + "instant.dev/internal/testhelpers" +) + +// signInstallState mints a valid install state token for the test team using +// the same secret + claim shape the handler verifies (testhelpers.TestJWTSecret). +func signInstallState(t *testing.T, teamID string) string { + t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "team_id": teamID, + "purpose": "gh_app_install", + "iat": time.Now().Add(-time.Minute).Unix(), + "exp": time.Now().Add(5 * time.Minute).Unix(), + }) + s, err := tok.SignedString([]byte(testhelpers.TestJWTSecret)) + require.NoError(t, err) + return s +} + +func appEnabled(c *config.Config) { c.GitHubAppEnabled = true; c.GitHubAppSlug = "instanode" } +func appNoSlug(c *config.Config) { c.GitHubAppEnabled = true; c.GitHubAppSlug = "" } + +func TestGitHubAppInstall_FlagOff_501(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + teamID := testhelpers.MustCreateTeamDB(t, db, "pro") + jwtTok := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamID, "gh@example.com") + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy") // flag default OFF + defer clean() + + req := httptest.NewRequest(http.MethodGet, "/integrations/github/install", nil) + req.Header.Set("Authorization", "Bearer "+jwtTok) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusNotImplemented, resp.StatusCode) +} + +func TestGitHubAppInstall_Misconfigured_503(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + teamID := testhelpers.MustCreateTeamDB(t, db, "pro") + jwtTok := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamID, "gh@example.com") + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appNoSlug) + defer clean() + + req := httptest.NewRequest(http.MethodGet, "/integrations/github/install", nil) + req.Header.Set("Authorization", "Bearer "+jwtTok) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) +} + +func TestGitHubAppInstall_Redirects(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + teamID := testhelpers.MustCreateTeamDB(t, db, "pro") + jwtTok := testhelpers.MustSignSessionJWT(t, uuid.NewString(), teamID, "gh@example.com") + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + req := httptest.NewRequest(http.MethodGet, "/integrations/github/install", nil) + req.Header.Set("Authorization", "Bearer "+jwtTok) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusFound, resp.StatusCode) + loc := resp.Header.Get("Location") + assert.True(t, strings.HasPrefix(loc, "https://github.com/apps/instanode/installations/new?state="), + "Location should point at the App install page, got %q", loc) + assert.Contains(t, loc, "state=", "install must carry an anti-CSRF state") +} + +func TestGitHubAppCallback_InvalidState_400(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + req := httptest.NewRequest(http.MethodGet, "/integrations/github/callback?installation_id=99&state=bogus", nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + +func TestGitHubAppCallback_InvalidInstallationID_400(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + teamID := testhelpers.MustCreateTeamDB(t, db, "pro") + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + state := signInstallState(t, teamID) + // installation_id missing/non-numeric → 400. + req := httptest.NewRequest(http.MethodGet, "/integrations/github/callback?installation_id=abc&state="+state, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + +func TestGitHubAppCallback_PersistsAndRedirects(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + teamID := testhelpers.MustCreateTeamDB(t, db, "pro") + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + state := signInstallState(t, teamID) + req := httptest.NewRequest(http.MethodGet, + "/integrations/github/callback?installation_id=778899&setup_action=install&state="+state, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusFound, resp.StatusCode) + assert.Contains(t, resp.Header.Get("Location"), "/integrations/github?installed=778899") + + // the installation↔team link was persisted. + var gotTeam string + row := db.QueryRow(`SELECT team_id::text FROM github_installations WHERE installation_id = $1`, int64(778899)) + require.NoError(t, row.Scan(&gotTeam)) + assert.Equal(t, teamID, gotTeam) +} diff --git a/internal/handlers/github_app_state_test.go b/internal/handlers/github_app_state_test.go new file mode 100644 index 00000000..71176da6 --- /dev/null +++ b/internal/handlers/github_app_state_test.go @@ -0,0 +1,78 @@ +package handlers + +// github_app_state_test.go — unit tests for the GitHub App install state token +// (sign/verify), P4.1. No DB. + +import ( + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" +) + +const ghStateSecret = "test-github-app-state-secret-0123456789" + +func TestGitHubAppState_RoundTrip(t *testing.T) { + tok, err := signGitHubAppState(ghStateSecret, "team-abc", time.Now()) + if err != nil { + t.Fatalf("sign: %v", err) + } + got, err := verifyGitHubAppState(ghStateSecret, tok) + if err != nil { + t.Fatalf("verify: %v", err) + } + if got != "team-abc" { + t.Errorf("team_id = %q want team-abc", got) + } +} + +func TestGitHubAppState_Rejections(t *testing.T) { + // empty + if _, err := verifyGitHubAppState(ghStateSecret, ""); err == nil { + t.Error("empty state must error") + } + // expired (signed 1h in the past → exp 50m ago) + expired, _ := signGitHubAppState(ghStateSecret, "t", time.Now().Add(-time.Hour)) + if _, err := verifyGitHubAppState(ghStateSecret, expired); err == nil { + t.Error("expired state must error") + } + // wrong secret + good, _ := signGitHubAppState(ghStateSecret, "t", time.Now()) + if _, err := verifyGitHubAppState("a-different-secret-aaaaaaaaaaaaaaaa", good); err == nil { + t.Error("wrong secret must error") + } + // garbage + if _, err := verifyGitHubAppState(ghStateSecret, "not.a.jwt"); err == nil { + t.Error("garbage must error") + } +} + +func TestGitHubAppState_PurposeAndTeamGuards(t *testing.T) { + // wrong purpose + wrongPurpose := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "team_id": "t", "purpose": "session", "exp": time.Now().Add(time.Hour).Unix(), + }) + wp, _ := wrongPurpose.SignedString([]byte(ghStateSecret)) + if _, err := verifyGitHubAppState(ghStateSecret, wp); err == nil { + t.Error("wrong purpose must error") + } + // missing team_id + noTeam := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "purpose": githubAppStatePurpose, "exp": time.Now().Add(time.Hour).Unix(), + }) + nt, _ := noTeam.SignedString([]byte(ghStateSecret)) + if _, err := verifyGitHubAppState(ghStateSecret, nt); err == nil { + t.Error("missing team_id must error") + } +} + +func TestGitHubAppState_RejectsNonHS256(t *testing.T) { + // A token using "none" alg must be rejected by WithValidMethods. + tok := jwt.NewWithClaims(jwt.SigningMethodNone, jwt.MapClaims{ + "team_id": "t", "purpose": githubAppStatePurpose, "exp": time.Now().Add(time.Hour).Unix(), + }) + none, _ := tok.SignedString(jwt.UnsafeAllowNoneSignatureType) + if _, err := verifyGitHubAppState(ghStateSecret, none); err == nil { + t.Error("alg=none must be rejected") + } +} diff --git a/internal/handlers/helpers.go b/internal/handlers/helpers.go index 9410bc0b..1f687ea2 100644 --- a/internal/handlers/helpers.go +++ b/internal/handlers/helpers.go @@ -501,6 +501,21 @@ var codeToAgentAction = map[string]errorCodeMeta{ "invalid_git_url": { AgentAction: "Tell the user the git_url is invalid. Pass an http(s) clone URL with a host and no embedded credentials, e.g. https://github.com/owner/repo — for a private repo send the token via git_token — see https://instanode.dev/docs/deploy.", }, + "github_app_disabled": { + AgentAction: "Tell the user GitHub App install is still rolling out and not yet enabled. Use the manual webhook (POST /api/v1/deployments/:id/github) or source=git with a token for now — see https://instanode.dev/docs/deploy.", + }, + "github_app_misconfigured": { + AgentAction: "Tell the user GitHub App integration is enabled but not fully configured on the server. This is an operator-side gap — retry later or contact support — see https://instanode.dev/status.", + }, + "invalid_installation_id": { + AgentAction: "Tell the user GitHub did not return a valid installation. Restart the install from the dashboard — see https://instanode.dev/docs/deploy.", + }, + "install_persist_failed": { + AgentAction: "Tell the user the GitHub installation could not be saved due to a transient backend error. Retry the install shortly — see https://instanode.dev/status.", + }, + "state_unavailable": { + AgentAction: "Tell the user the GitHub install could not be started due to a transient backend error. Retry shortly — see https://instanode.dev/status.", + }, "source_image_disabled": { AgentAction: "Tell the user deploying from a prebuilt image (source=image) is still rolling out and not yet enabled. Upload source as a tarball (<=10 MiB) for now — see https://instanode.dev/docs/deploy.", }, diff --git a/internal/models/github_installation.go b/internal/models/github_installation.go new file mode 100644 index 00000000..bb3dd0c3 --- /dev/null +++ b/internal/models/github_installation.go @@ -0,0 +1,144 @@ +package models + +// github_installation.go — model layer for GitHub App installations (migration +// 066, P4.1). One row per (team, installation) — the link the install/callback +// flow persists and the App webhook resolves an incoming installation_id against +// before acting. No access token is stored here; tokens are minted on demand +// (internal/github) from the App private key and cached in Redis. + +import ( + "context" + "database/sql" + "fmt" + "time" + + "github.com/google/uuid" +) + +// GitHubInstallation links a GitHub App installation to a team. +type GitHubInstallation struct { + InstallationID int64 + TeamID uuid.UUID + AccountLogin string + SuspendedAt sql.NullTime // non-NULL → installation suspended; do not act + CreatedAt time.Time + UpdatedAt time.Time +} + +// ErrGitHubInstallationNotFound is returned when a lookup yields no rows. +type ErrGitHubInstallationNotFound struct { + InstallationID int64 +} + +func (e *ErrGitHubInstallationNotFound) Error() string { + return fmt.Sprintf("github installation not found: %d", e.InstallationID) +} + +const githubInstallationColumns = `installation_id, team_id, account_login, + suspended_at, created_at, updated_at` + +func scanGitHubInstallation(row interface { + Scan(dest ...any) error +}) (*GitHubInstallation, error) { + i := &GitHubInstallation{} + if err := row.Scan( + &i.InstallationID, &i.TeamID, &i.AccountLogin, + &i.SuspendedAt, &i.CreatedAt, &i.UpdatedAt, + ); err != nil { + return nil, err + } + return i, nil +} + +// UpsertGitHubInstallation inserts or updates the installation↔team link. The +// installation_id is GitHub's primary key; a re-install (or a callback replay) +// updates team_id/account_login and clears any prior suspension. +func UpsertGitHubInstallation(ctx context.Context, db *sql.DB, installationID int64, teamID uuid.UUID, accountLogin string) (*GitHubInstallation, error) { + row := db.QueryRowContext(ctx, ` + INSERT INTO github_installations (installation_id, team_id, account_login) + VALUES ($1, $2, $3) + ON CONFLICT (installation_id) DO UPDATE + SET team_id = EXCLUDED.team_id, + account_login = EXCLUDED.account_login, + suspended_at = NULL, + updated_at = now() + RETURNING `+githubInstallationColumns, + installationID, teamID, accountLogin) + i, err := scanGitHubInstallation(row) + if err != nil { + return nil, fmt.Errorf("models.UpsertGitHubInstallation: %w", err) + } + return i, nil +} + +// GetGitHubInstallation fetches one installation by its id. +func GetGitHubInstallation(ctx context.Context, db *sql.DB, installationID int64) (*GitHubInstallation, error) { + row := db.QueryRowContext(ctx, + `SELECT `+githubInstallationColumns+` FROM github_installations WHERE installation_id = $1`, + installationID) + i, err := scanGitHubInstallation(row) + if err == sql.ErrNoRows { + return nil, &ErrGitHubInstallationNotFound{InstallationID: installationID} + } + if err != nil { + return nil, fmt.Errorf("models.GetGitHubInstallation: %w", err) + } + return i, nil +} + +// ListGitHubInstallationsByTeam returns all (non-deleted) installations a team +// has, newest first. +func ListGitHubInstallationsByTeam(ctx context.Context, db *sql.DB, teamID uuid.UUID) ([]*GitHubInstallation, error) { + rows, err := db.QueryContext(ctx, + `SELECT `+githubInstallationColumns+` FROM github_installations WHERE team_id = $1 ORDER BY created_at DESC`, + teamID) + if err != nil { + return nil, fmt.Errorf("models.ListGitHubInstallationsByTeam: %w", err) + } + defer func() { _ = rows.Close() }() + var out []*GitHubInstallation + for rows.Next() { + i, err := scanGitHubInstallation(rows) + if err != nil { + return nil, fmt.Errorf("models.ListGitHubInstallationsByTeam scan: %w", err) + } + out = append(out, i) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("models.ListGitHubInstallationsByTeam rows: %w", err) + } + return out, nil +} + +// SetGitHubInstallationSuspended toggles the suspended flag (GitHub `suspend` / +// `unsuspend` installation events). Suspended installations are kept (not +// deleted) so an unsuspend restores the link without a re-install. +func SetGitHubInstallationSuspended(ctx context.Context, db *sql.DB, installationID int64, suspended bool) error { + var suspendedAt interface{} + if suspended { + suspendedAt = time.Now().UTC() + } + res, err := db.ExecContext(ctx, + `UPDATE github_installations SET suspended_at = $2, updated_at = now() WHERE installation_id = $1`, + installationID, suspendedAt) + if err != nil { + return fmt.Errorf("models.SetGitHubInstallationSuspended: %w", err) + } + n, _ := res.RowsAffected() + if n == 0 { + return &ErrGitHubInstallationNotFound{InstallationID: installationID} + } + return nil +} + +// DeleteGitHubInstallation removes the link (GitHub `deleted` installation +// event). Returns the number of rows removed (0 = already gone, not an error). +func DeleteGitHubInstallation(ctx context.Context, db *sql.DB, installationID int64) (int64, error) { + res, err := db.ExecContext(ctx, + `DELETE FROM github_installations WHERE installation_id = $1`, installationID) + if err != nil { + return 0, fmt.Errorf("models.DeleteGitHubInstallation: %w", err) + } + n, _ := res.RowsAffected() + return n, nil +} diff --git a/internal/models/github_installation_test.go b/internal/models/github_installation_test.go new file mode 100644 index 00000000..00f0fb98 --- /dev/null +++ b/internal/models/github_installation_test.go @@ -0,0 +1,136 @@ +package models + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/google/uuid" + "github.com/stretchr/testify/require" +) + +func ghInstallRow() *sqlmock.Rows { + return sqlmock.NewRows([]string{ + "installation_id", "team_id", "account_login", "suspended_at", "created_at", "updated_at", + }).AddRow(int64(42), uuid.New(), "acme", nil, time.Now(), time.Now()) +} + +func TestUpsertGitHubInstallation(t *testing.T) { + ctx := context.Background() + db, mock := newMock(t) + mock.ExpectQuery(`INSERT INTO github_installations`).WillReturnRows(ghInstallRow()) + got, err := UpsertGitHubInstallation(ctx, db, 42, uuid.New(), "acme") + require.NoError(t, err) + require.Equal(t, int64(42), got.InstallationID) + + // scan/db error + db2, mock2 := newMock(t) + mock2.ExpectQuery(`INSERT INTO github_installations`).WillReturnError(errors.New("boom")) + _, err = UpsertGitHubInstallation(ctx, db2, 42, uuid.New(), "acme") + require.ErrorContains(t, err, "boom") +} + +func TestGetGitHubInstallation(t *testing.T) { + ctx := context.Background() + db, mock := newMock(t) + mock.ExpectQuery(`FROM github_installations WHERE installation_id`).WillReturnRows(ghInstallRow()) + got, err := GetGitHubInstallation(ctx, db, 42) + require.NoError(t, err) + require.Equal(t, "acme", got.AccountLogin) + + // not found + db2, mock2 := newMock(t) + mock2.ExpectQuery(`FROM github_installations WHERE installation_id`).WillReturnError(errNoRows()) + _, err = GetGitHubInstallation(ctx, db2, 7) + var nf *ErrGitHubInstallationNotFound + require.ErrorAs(t, err, &nf) + require.Contains(t, nf.Error(), "7") + + // other error + db3, mock3 := newMock(t) + mock3.ExpectQuery(`FROM github_installations WHERE installation_id`).WillReturnError(errors.New("boom")) + _, err = GetGitHubInstallation(ctx, db3, 7) + require.ErrorContains(t, err, "boom") +} + +func TestListGitHubInstallationsByTeam(t *testing.T) { + ctx := context.Background() + team := uuid.New() + + db, mock := newMock(t) + rows := sqlmock.NewRows([]string{ + "installation_id", "team_id", "account_login", "suspended_at", "created_at", "updated_at", + }).AddRow(int64(1), team, "a", nil, time.Now(), time.Now()). + AddRow(int64(2), team, "b", time.Now(), time.Now(), time.Now()) + mock.ExpectQuery(`FROM github_installations WHERE team_id`).WillReturnRows(rows) + got, err := ListGitHubInstallationsByTeam(ctx, db, team) + require.NoError(t, err) + require.Len(t, got, 2) + + // query error + db2, mock2 := newMock(t) + mock2.ExpectQuery(`FROM github_installations WHERE team_id`).WillReturnError(errors.New("boom")) + _, err = ListGitHubInstallationsByTeam(ctx, db2, team) + require.ErrorContains(t, err, "boom") + + // scan error (wrong column count) + db3, mock3 := newMock(t) + mock3.ExpectQuery(`FROM github_installations WHERE team_id`). + WillReturnRows(sqlmock.NewRows([]string{"installation_id"}).AddRow(int64(1))) + _, err = ListGitHubInstallationsByTeam(ctx, db3, team) + require.ErrorContains(t, err, "scan") + + // rows.Err() + db4, mock4 := newMock(t) + mock4.ExpectQuery(`FROM github_installations WHERE team_id`). + WillReturnRows(ghInstallRow().RowError(0, errors.New("rowboom"))) + _, err = ListGitHubInstallationsByTeam(ctx, db4, team) + require.ErrorContains(t, err, "rowboom") +} + +func TestSetGitHubInstallationSuspended(t *testing.T) { + ctx := context.Background() + + // suspend = true + db, mock := newMock(t) + mock.ExpectExec(`UPDATE github_installations SET suspended_at`). + WillReturnResult(sqlmock.NewResult(0, 1)) + require.NoError(t, SetGitHubInstallationSuspended(ctx, db, 42, true)) + + // unsuspend (suspended=false) — 1 row + db2, mock2 := newMock(t) + mock2.ExpectExec(`UPDATE github_installations SET suspended_at`). + WillReturnResult(sqlmock.NewResult(0, 1)) + require.NoError(t, SetGitHubInstallationSuspended(ctx, db2, 42, false)) + + // 0 rows → not found + db3, mock3 := newMock(t) + mock3.ExpectExec(`UPDATE github_installations SET suspended_at`). + WillReturnResult(sqlmock.NewResult(0, 0)) + var nf *ErrGitHubInstallationNotFound + require.ErrorAs(t, SetGitHubInstallationSuspended(ctx, db3, 9, true), &nf) + + // exec error + db4, mock4 := newMock(t) + mock4.ExpectExec(`UPDATE github_installations SET suspended_at`). + WillReturnError(errors.New("boom")) + require.ErrorContains(t, SetGitHubInstallationSuspended(ctx, db4, 9, true), "boom") +} + +func TestDeleteGitHubInstallation(t *testing.T) { + ctx := context.Background() + + db, mock := newMock(t) + mock.ExpectExec(`DELETE FROM github_installations`).WillReturnResult(sqlmock.NewResult(0, 1)) + n, err := DeleteGitHubInstallation(ctx, db, 42) + require.NoError(t, err) + require.Equal(t, int64(1), n) + + // exec error + db2, mock2 := newMock(t) + mock2.ExpectExec(`DELETE FROM github_installations`).WillReturnError(errors.New("boom")) + _, err = DeleteGitHubInstallation(ctx, db2, 42) + require.ErrorContains(t, err, "boom") +} diff --git a/internal/router/router.go b/internal/router/router.go index f7b07d40..39c7c340 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -687,6 +687,13 @@ func NewWithHooks(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *mid githubReceiveH := handlers.NewGitHubDeployHandler(db, cfg, planRegistry) app.Post("/webhooks/github/:webhook_id", githubReceiveH.Receive) + // GitHub App install flow (P4.1), gated by cfg.GitHubAppEnabled. Install + // requires auth (binds the install to the team); Callback is state- + // authenticated (GitHub presents no session, only the signed state). + githubAppH := handlers.NewGitHubAppHandler(db, cfg, planRegistry) + app.Get("/integrations/github/install", middleware.RequireAuth(cfg), githubAppH.Install) + app.Get("/integrations/github/callback", githubAppH.Callback) + // Deploy — Phase 6 (auth required on all endpoints). // POST /deploy/new is gated by RequireEnvAccess(ActionDeploy) — the // env scope arrives as a multipart form field (not JSON or query), so diff --git a/internal/testhelpers/testhelpers.go b/internal/testhelpers/testhelpers.go index 045929e8..689e15c1 100644 --- a/internal/testhelpers/testhelpers.go +++ b/internal/testhelpers/testhelpers.go @@ -531,6 +531,16 @@ func runMigrations(t *testing.T, db *sql.DB) { )`, `CREATE UNIQUE INDEX IF NOT EXISTS uq_app_github_connection ON app_github_connections(app_id)`, `CREATE INDEX IF NOT EXISTS idx_app_github_connections_team ON app_github_connections(team_id)`, + // 066_github_installations — GitHub App (P4.1) installation↔team links. + `CREATE TABLE IF NOT EXISTS github_installations ( + installation_id BIGINT PRIMARY KEY, + team_id UUID NOT NULL REFERENCES teams(id) ON DELETE CASCADE, + account_login TEXT NOT NULL DEFAULT '', + suspended_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() + )`, + `CREATE INDEX IF NOT EXISTS idx_github_installations_team ON github_installations(team_id)`, `CREATE TABLE IF NOT EXISTS pending_github_deploys ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), connection_id UUID NOT NULL REFERENCES app_github_connections(id) ON DELETE CASCADE, @@ -1218,6 +1228,11 @@ func NewTestAppWithServices(t *testing.T, db *sql.DB, rdb *redis.Client, service api.Delete("/deployments/:id/github", githubDeployH.Disconnect) app.Post("/webhooks/github/:webhook_id", githubDeployH.Receive) + // GitHub App install flow (P4.1) — mirror router.go. + githubAppH := handlers.NewGitHubAppHandler(db, cfg, planReg) + app.Get("/integrations/github/install", middleware.RequireAuth(cfg), githubAppH.Install) + app.Get("/integrations/github/callback", githubAppH.Callback) + // A/B-experiment conversion sink — wired into the test app so // handler tests can exercise the full route stack (router + // auth middleware + JSON handler) end-to-end. From 3e910dfb8af9181618acdc27ceade2c7521e9b81 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Wed, 3 Jun 2026 19:33:38 +0530 Subject: [PATCH 2/3] test(github-app): cover handler error arms (no-team, sign-error, callback) CI patch-coverage flagged github_app.go handler arms the routed integration tests can't reach. Add a signInstallStateFn seam + white-box tests for the no-team guard (RequireAuth normally guarantees a team), the unreachable state-sign error, and the Callback disabled-feature return; plus DB-gated integration tests for the callback invalid-team-UUID (parseTeamID) and persist-error (team_id FK violation) arms. All github_app.go branches now exercised (white-box locally + integration in CI). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/handlers/github_app.go | 7 +- .../handlers/github_app_integration_test.go | 39 ++++++++ internal/handlers/github_app_whitebox_test.go | 95 +++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 internal/handlers/github_app_whitebox_test.go diff --git a/internal/handlers/github_app.go b/internal/handlers/github_app.go index ba890533..0debf2d0 100644 --- a/internal/handlers/github_app.go +++ b/internal/handlers/github_app.go @@ -40,6 +40,11 @@ const githubAppStatePurpose = "gh_app_install" // githubAppStateTTL bounds the install→callback round-trip. const githubAppStateTTL = 10 * time.Minute +// signInstallStateFn is the state signer the Install handler uses — a package +// var so a test can force the (otherwise unreachable, HS256-never-fails) sign +// error path. +var signInstallStateFn = signGitHubAppState + // GitHubAppHandler serves the install/callback flow. type GitHubAppHandler struct { db *sql.DB @@ -77,7 +82,7 @@ func (h *GitHubAppHandler) Install(c *fiber.Ctx) error { if teamID == "" { return respondError(c, fiber.StatusUnauthorized, "unauthorized", "A session token is required") } - state, err := signGitHubAppState(h.cfg.JWTSecret, teamID, time.Now()) + state, err := signInstallStateFn(h.cfg.JWTSecret, teamID, time.Now()) if err != nil { slog.Error("github_app.install.state_sign_failed", "error", err) return respondError(c, fiber.StatusServiceUnavailable, "state_unavailable", diff --git a/internal/handlers/github_app_integration_test.go b/internal/handlers/github_app_integration_test.go index eed7ad1e..df6c9d17 100644 --- a/internal/handlers/github_app_integration_test.go +++ b/internal/handlers/github_app_integration_test.go @@ -140,6 +140,45 @@ func TestGitHubAppCallback_InvalidInstallationID_400(t *testing.T) { assert.Equal(t, http.StatusBadRequest, resp.StatusCode) } +// state carries a team_id that isn't a UUID → parseTeamID fails → 400. +func TestGitHubAppCallback_InvalidTeamUUID_400(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + state := signInstallState(t, "not-a-uuid") + req := httptest.NewRequest(http.MethodGet, "/integrations/github/callback?installation_id=12&state="+state, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + +// state's team_id is a valid UUID but no such team exists → the upsert hits the +// team_id FK and fails → 503 install_persist_failed. +func TestGitHubAppCallback_PersistError_503(t *testing.T) { + daDeployNeedsDB(t) + db, cleanDB := testhelpers.SetupTestDB(t) + defer cleanDB() + rdb, cleanRedis := testhelpers.SetupTestRedis(t) + defer cleanRedis() + + app, clean := testhelpers.NewTestAppWithServices(t, db, rdb, "deploy", appEnabled) + defer clean() + + state := signInstallState(t, uuid.NewString()) // valid UUID, not in teams + req := httptest.NewRequest(http.MethodGet, "/integrations/github/callback?installation_id=34&state="+state, nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) +} + func TestGitHubAppCallback_PersistsAndRedirects(t *testing.T) { daDeployNeedsDB(t) db, cleanDB := testhelpers.SetupTestDB(t) diff --git a/internal/handlers/github_app_whitebox_test.go b/internal/handlers/github_app_whitebox_test.go new file mode 100644 index 00000000..014bef0b --- /dev/null +++ b/internal/handlers/github_app_whitebox_test.go @@ -0,0 +1,95 @@ +package handlers + +// github_app_whitebox_test.go — branch coverage for github_app.go arms that the +// routed integration tests can't reach: the no-team guard (RequireAuth normally +// guarantees a team), the unreachable state-sign error (HS256 never fails — via +// the signInstallStateFn seam), and the Callback disabled-feature early return. +// No DB: these arms return before any DB access. + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + + "instant.dev/internal/config" + "instant.dev/internal/middleware" +) + +func ghAppTestHandler(enabled bool, slug string) *GitHubAppHandler { + return NewGitHubAppHandler(nil, &config.Config{ + JWTSecret: "whitebox-secret-aaaaaaaaaaaaaaaaaaaa", + GitHubAppEnabled: enabled, + GitHubAppSlug: slug, + DashboardBaseURL: "http://localhost:5173", + }, nil) +} + +// ghAppFiberApp builds a fiber app whose ErrorHandler swallows respondError's +// ErrResponseWritten sentinel (mirrors the production/testhelpers router), so a +// handler that already wrote its status isn't overwritten with a 500. +func ghAppFiberApp() *fiber.App { + return fiber.New(fiber.Config{ + ErrorHandler: func(c *fiber.Ctx, err error) error { + if errors.Is(err, ErrResponseWritten) { + return nil + } + return fiber.DefaultErrorHandler(c, err) + }, + }) +} + +// Install with no team in context (no RequireAuth in front) → 401. +func TestGitHubAppInstall_NoTeam_WhiteBox(t *testing.T) { + h := ghAppTestHandler(true, "instanode") + app := ghAppFiberApp() + app.Get("/install", h.Install) // deliberately no RequireAuth / no team locals + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/install", nil)) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("no team must 401, got %d", resp.StatusCode) + } +} + +// Force the (otherwise unreachable) state-sign error via the seam → 503. +func TestGitHubAppInstall_SignError_WhiteBox(t *testing.T) { + orig := signInstallStateFn + signInstallStateFn = func(string, string, time.Time) (string, error) { + return "", errors.New("forced sign failure") + } + defer func() { signInstallStateFn = orig }() + + h := ghAppTestHandler(true, "instanode") + app := ghAppFiberApp() + // Inject a team so we pass the auth guard and reach the sign step. + app.Get("/install", func(c *fiber.Ctx) error { + c.Locals(middleware.LocalKeyTeamID, "11111111-1111-1111-1111-111111111111") + return h.Install(c) + }) + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/install", nil)) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusServiceUnavailable { + t.Errorf("sign failure must 503, got %d", resp.StatusCode) + } +} + +// Callback with the feature flag OFF → 501 (the disabled early-return). +func TestGitHubAppCallback_Disabled_WhiteBox(t *testing.T) { + h := ghAppTestHandler(false, "") + app := ghAppFiberApp() + app.Get("/cb", h.Callback) + resp, err := app.Test(httptest.NewRequest(http.MethodGet, "/cb?installation_id=1&state=x", nil)) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusNotImplemented { + t.Errorf("callback with feature off must 501, got %d", resp.StatusCode) + } +} From a0427b16a9d3f66c1db61ee901c18d69d211bbbd Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Wed, 3 Jun 2026 19:49:40 +0530 Subject: [PATCH 3/3] fix(github-app): satisfy agent_action + openapi route contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two registry-iterating contract tests caught the new surface: - agent_action for github_app_disabled referenced a bare /api/v1 path; switch to the canonical https://api.instanode.dev/api/v1/... host (TestAgentActionContract_APIPathsUseAPIHost). - the new /integrations/github/{install,callback} routes are registered but flag-gated OFF (501 until P4.3), so documenting them in the agent-facing OpenAPI now would mislead agents — whitelist them in the route-coverage gate with justification (contract sync lands at flag-on, P4.3), matching the /auth/exchange precedent. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/handlers/helpers.go | 2 +- internal/handlers/openapi_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/handlers/helpers.go b/internal/handlers/helpers.go index 1f687ea2..19b03326 100644 --- a/internal/handlers/helpers.go +++ b/internal/handlers/helpers.go @@ -502,7 +502,7 @@ var codeToAgentAction = map[string]errorCodeMeta{ AgentAction: "Tell the user the git_url is invalid. Pass an http(s) clone URL with a host and no embedded credentials, e.g. https://github.com/owner/repo — for a private repo send the token via git_token — see https://instanode.dev/docs/deploy.", }, "github_app_disabled": { - AgentAction: "Tell the user GitHub App install is still rolling out and not yet enabled. Use the manual webhook (POST /api/v1/deployments/:id/github) or source=git with a token for now — see https://instanode.dev/docs/deploy.", + AgentAction: "Tell the user GitHub App install is still rolling out and not yet enabled. Use the manual webhook (POST https://api.instanode.dev/api/v1/deployments/:id/github) or source=git with a token for now — see https://instanode.dev/docs/deploy.", }, "github_app_misconfigured": { AgentAction: "Tell the user GitHub App integration is enabled but not fully configured on the server. This is an operator-side gap — retry later or contact support — see https://instanode.dev/status.", diff --git a/internal/handlers/openapi_test.go b/internal/handlers/openapi_test.go index 1950eb9c..5111c955 100644 --- a/internal/handlers/openapi_test.go +++ b/internal/handlers/openapi_test.go @@ -702,6 +702,13 @@ func TestOpenAPI_CoversAllRegisteredRoutes(t *testing.T) { // the builder and security_txt_test.go for the wire contract. "GET /.well-known/security.txt": true, "GET /security.txt": true, + // GitHub App install flow (P4.1) is flag-gated OFF (GITHUB_APP_ENABLED). + // While off, both endpoints return 501 github_app_disabled — documenting + // them in the agent-facing OpenAPI now would mislead agents into thinking + // the App path works. The contract sync (OpenAPI + llms.txt + MCP) lands + // in P4.3 alongside the flag flip. See SCOPE-P4-github-app + github_app.go. + "GET /integrations/github/install": true, + "GET /integrations/github/callback": true, } var missing []string