-
Notifications
You must be signed in to change notification settings - Fork 94
feat(mint): add --pem-dir to bootstrap PEMs during deploy #1690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cae5d89
fee4c96
e7011d0
d48c0fc
2d86c83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ repos: | |
| - id: end-of-file-fixer | ||
| - id: trailing-whitespace | ||
| - id: detect-private-key | ||
| exclude: "internal/layers/secrets_test\\.go$|internal/security/scanner_test\\.go$|internal/dispatch/gcf/provisioner_test\\.go$|tests/.*test_.*\\.py$" | ||
| exclude: "internal/layers/secrets_test\\.go$|internal/security/scanner_test\\.go$|internal/dispatch/gcf/provisioner_test\\.go$|internal/cli/mint_test\\.go$|tests/.*test_.*\\.py$" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [high] protected-path Protected infrastructure file modified without a linked issue. The change adds internal/cli/mint_test.go$ to the detect-private-key exclusion list, justified by fake PEM test fixtures. Human reviewers must verify protected-path changes regardless, and the absence of a linked issue means there is no traceable authorization. Suggested fix: Link a tracking issue that authorizes the .pre-commit-config.yaml change, or have a human reviewer explicitly approve this protected-path modification. |
||
| - id: check-added-large-files | ||
| args: ["--maxkb=1000"] | ||
| - id: check-merge-conflict | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,24 @@ package cli | |
| import ( | ||
| "bufio" | ||
| "context" | ||
| "crypto" | ||
| "crypto/rand" | ||
| "crypto/rsa" | ||
| "crypto/sha256" | ||
| "crypto/x509" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "encoding/pem" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "os" | ||
| "path/filepath" | ||
| "sort" | ||
| "strconv" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| "golang.org/x/term" | ||
|
|
@@ -41,6 +52,216 @@ func resolveRole(role string) string { | |
| return role | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] style githubAPIBaseURL and githubHTTPClient are package-level mutable globals swapped via save/restore in tests. Fragile if tests ever run in parallel. Suggested fix: Consider passing HTTP client and base URL via a struct or function parameters instead of package-level vars. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] style githubAPIBaseURL and githubHTTPClient are package-level mutable globals swapped via save/restore in tests. Fragile if tests ever run in parallel. Suggested fix: Consider passing an HTTP client and base URL via a struct or function parameters instead of package-level vars. |
||
| // githubAPIBaseURL is the base URL for the GitHub API. | ||
| // Overridden in tests to use httptest servers. | ||
| var githubAPIBaseURL = "https://api.github.com" | ||
|
|
||
| var githubHTTPClient = &http.Client{Timeout: 30 * time.Second} | ||
|
|
||
| // lookupAppID fetches the numeric app ID for a public GitHub App by slug. | ||
| // It makes an unauthenticated GET request to the GitHub API. | ||
| func lookupAppID(ctx context.Context, slug string) (int, error) { | ||
| url := githubAPIBaseURL + "/apps/" + slug | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("creating request for app %s: %w", slug, err) | ||
| } | ||
| req.Header.Set("Accept", "application/vnd.github+json") | ||
|
|
||
| resp, err := githubHTTPClient.Do(req) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("looking up app %s: %w", slug, err) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] correctness lookupAppID uses http.DefaultClient which has no timeout. A hung connection to the GitHub API could block indefinitely if no context deadline is set by the caller. Suggested fix: Replace http.DefaultClient with a client that has a Timeout field set (e.g., 30s), or document that callers must set a context deadline. |
||
| defer func() { | ||
| io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) | ||
| resp.Body.Close() | ||
| }() | ||
|
|
||
| if resp.StatusCode == http.StatusNotFound { | ||
| return 0, fmt.Errorf("GitHub App %q not found — ensure the app exists and is publicly visible", slug) | ||
| } | ||
| if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusTooManyRequests { | ||
| return 0, fmt.Errorf("GitHub API rate limit exceeded for app %s — unauthenticated requests are limited to 60/hour; try again later", slug) | ||
| } | ||
| if resp.StatusCode != http.StatusOK { | ||
| return 0, fmt.Errorf("GitHub API returned %d for app %s", resp.StatusCode, slug) | ||
| } | ||
|
|
||
| var app struct { | ||
| ID int `json:"id"` | ||
| } | ||
| if err := json.NewDecoder(resp.Body).Decode(&app); err != nil { | ||
| return 0, fmt.Errorf("decoding app %s response: %w", slug, err) | ||
| } | ||
| if app.ID == 0 { | ||
| return 0, fmt.Errorf("GitHub App %s has no numeric ID", slug) | ||
| } | ||
| return app.ID, nil | ||
| } | ||
|
|
||
| // verifyPEMMatchesApp confirms a PEM private key belongs to the given GitHub | ||
| // App by generating a JWT and calling GET /app with it. Returns nil on success. | ||
| func verifyPEMMatchesApp(ctx context.Context, pemData []byte, appID int, slug string) error { | ||
| block, _ := pem.Decode(pemData) | ||
| if block == nil { | ||
| return fmt.Errorf("failed to decode PEM block") | ||
| } | ||
| key, err := x509.ParsePKCS1PrivateKey(block.Bytes) | ||
| if err != nil { | ||
| pkcs8Key, pkcs8Err := x509.ParsePKCS8PrivateKey(block.Bytes) | ||
| if pkcs8Err != nil { | ||
| return fmt.Errorf("parsing private key: %w", pkcs8Err) | ||
| } | ||
| var ok bool | ||
| key, ok = pkcs8Key.(*rsa.PrivateKey) | ||
| if !ok { | ||
| return fmt.Errorf("key is not RSA") | ||
| } | ||
| } | ||
|
|
||
| now := time.Now() | ||
| headerJSON, _ := json.Marshal(map[string]string{"alg": "RS256", "typ": "JWT"}) | ||
| claimsJSON, _ := json.Marshal(map[string]interface{}{ | ||
| "iss": strconv.Itoa(appID), | ||
| "iat": now.Add(-60 * time.Second).Unix(), | ||
| "exp": now.Add(5 * time.Minute).Unix(), | ||
| }) | ||
| signingInput := base64.RawURLEncoding.EncodeToString(headerJSON) + "." + | ||
| base64.RawURLEncoding.EncodeToString(claimsJSON) | ||
| hashed := sha256.Sum256([]byte(signingInput)) | ||
| sig, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA256, hashed[:]) | ||
| if err != nil { | ||
| return fmt.Errorf("signing JWT: %w", err) | ||
| } | ||
| jwt := signingInput + "." + base64.RawURLEncoding.EncodeToString(sig) | ||
|
|
||
| url := githubAPIBaseURL + "/app" | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("creating verify request: %w", err) | ||
| } | ||
| req.Header.Set("Authorization", "Bearer "+jwt) | ||
| req.Header.Set("Accept", "application/vnd.github+json") | ||
|
|
||
| resp, err := githubHTTPClient.Do(req) | ||
| if err != nil { | ||
| return fmt.Errorf("verifying PEM against GitHub: %w", err) | ||
| } | ||
| defer func() { | ||
| io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) | ||
| resp.Body.Close() | ||
| }() | ||
|
|
||
| if resp.StatusCode == http.StatusUnauthorized { | ||
| return fmt.Errorf("PEM does not match GitHub App %q (app ID %d) — the key may belong to a different app or have been revoked", slug, appID) | ||
| } | ||
| if resp.StatusCode != http.StatusOK { | ||
| return fmt.Errorf("unexpected status %d verifying PEM for app %s", resp.StatusCode, slug) | ||
| } | ||
|
|
||
| var respApp struct { | ||
| ID int `json:"id"` | ||
| } | ||
| if err := json.NewDecoder(resp.Body).Decode(&respApp); err != nil { | ||
| return fmt.Errorf("decoding verify response for app %s: %w", slug, err) | ||
| } | ||
| if respApp.ID != appID { | ||
| return fmt.Errorf("PEM authenticated as app %d but expected app %d (%s)", respApp.ID, appID, slug) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // listPEMFiles returns the basenames of .pem files in dir, for diagnostics. | ||
| func listPEMFiles(dir string) []string { | ||
| entries, err := os.ReadDir(dir) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| var names []string | ||
| for _, e := range entries { | ||
| if !e.IsDir() && strings.HasSuffix(e.Name(), ".pem") { | ||
| names = append(names, e.Name()) | ||
| } | ||
| } | ||
| sort.Strings(names) | ||
| return names | ||
| } | ||
|
|
||
| // validatePEMDir checks that pemDir exists, is a directory, and contains valid | ||
| // RSA PEM files for all default mint roles. Returns the validated PEM data keyed | ||
| // by role. This is the offline-only portion of PEM validation — no network calls. | ||
| func validatePEMDir(pemDir string) (map[string][]byte, error) { | ||
| info, err := os.Stat(pemDir) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("--pem-dir %q: %w", pemDir, err) | ||
| } | ||
| if !info.IsDir() { | ||
| return nil, fmt.Errorf("--pem-dir %q is not a directory", pemDir) | ||
| } | ||
|
|
||
| roles := defaultMintRoles() | ||
|
|
||
| for _, role := range roles { | ||
| pemPath := filepath.Join(pemDir, role+".pem") | ||
| if _, err := os.Stat(pemPath); err != nil { | ||
| found := listPEMFiles(pemDir) | ||
| expected := make([]string, len(roles)) | ||
| for i, r := range roles { | ||
| expected[i] = r + ".pem" | ||
| } | ||
| return nil, fmt.Errorf("missing PEM file for role %q: %s\n expected files: %s\n found in dir: %s", | ||
| role, pemPath, strings.Join(expected, ", "), strings.Join(found, ", ")) | ||
| } | ||
| } | ||
|
|
||
| pemsByRole := make(map[string][]byte, len(roles)) | ||
| for _, role := range roles { | ||
| pemPath := filepath.Join(pemDir, role+".pem") | ||
| pemData, err := os.ReadFile(pemPath) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reading PEM for role %q: %w", role, err) | ||
| } | ||
| if err := appsetup.ValidateRSAPEM(pemData); err != nil { | ||
| return nil, fmt.Errorf("invalid PEM for role %q (%s): %w", role, pemPath, err) | ||
| } | ||
| pemsByRole[role] = pemData | ||
| } | ||
| return pemsByRole, nil | ||
| } | ||
|
|
||
| // loadAppSetPEMs reads PEM files from pemDir and discovers app IDs from the | ||
| // GitHub API, returning maps ready for gcf.Config. | ||
| func loadAppSetPEMs(ctx context.Context, pemDir, appSet string) (map[string][]byte, map[string]string, error) { | ||
| if err := appsetup.ValidateAppSet(appSet); err != nil { | ||
| return nil, nil, fmt.Errorf("invalid app set: %w", err) | ||
| } | ||
|
|
||
| pemsByRole, err := validatePEMDir(pemDir) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| agentPEMs := make(map[string][]byte, len(pemsByRole)) | ||
| agentAppIDs := make(map[string]string, len(pemsByRole)) | ||
|
|
||
| for role, pemData := range pemsByRole { | ||
| slug := appsetup.AppSlug(appSet, role) | ||
| appID, err := lookupAppID(ctx, slug) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("looking up app ID for %s: %w", slug, err) | ||
| } | ||
|
|
||
| if err := verifyPEMMatchesApp(ctx, pemData, appID, slug); err != nil { | ||
| return nil, nil, fmt.Errorf("verifying PEM for role %q: %w", role, err) | ||
| } | ||
|
|
||
| agentPEMs[role] = pemData | ||
| agentAppIDs[role] = strconv.Itoa(appID) | ||
| } | ||
|
|
||
| return agentPEMs, agentAppIDs, nil | ||
| } | ||
|
|
||
| func newMintCmd() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "mint", | ||
|
|
@@ -63,13 +284,18 @@ func newMintDeployCmd() *cobra.Command { | |
| var sourceDir string | ||
| var skipDeploy bool | ||
| var dryRun bool | ||
| var pemDir string | ||
|
|
||
| cmd := &cobra.Command{ | ||
| Use: "deploy", | ||
| Short: "Deploy or update the token mint Cloud Function", | ||
| Long: `Deploys the fullsend-mint Cloud Function and supporting GCP infrastructure | ||
| (service account, WIF pool/provider). Does NOT enroll any org — use | ||
| 'fullsend mint enroll' after deployment.`, | ||
| 'fullsend mint enroll' after deployment. | ||
|
|
||
| Most runs need only --project and --region. The optional --pem-dir flag is | ||
| for first-time bootstrap only: it seeds the default app set's PEM secrets so | ||
| that 'mint enroll' can work without running 'admin install' first.`, | ||
| Args: cobra.NoArgs, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if project == "" { | ||
|
|
@@ -102,6 +328,12 @@ func newMintDeployCmd() *cobra.Command { | |
| if skipDeploy { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] correctness The dry-run branch duplicates PEM directory validation logic (stat dir, check role files exist, validate each PEM) that loadAppSetPEMs already implements. Drift risk if validation rules change. Suggested fix: Extract shared PEM directory validation into a helper function called by both the dry-run and real code paths. |
||
| printer.StepInfo("Would skip code deployment (--skip-deploy)") | ||
| } | ||
| if pemDir != "" { | ||
| if _, err := validatePEMDir(pemDir); err != nil { | ||
| return err | ||
| } | ||
| printer.StepInfo(fmt.Sprintf("Would bootstrap app set %q with PEMs from %s (app ID lookup and PEM verification skipped in dry-run)", appsetup.DefaultAppSet, pemDir)) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -116,16 +348,33 @@ func newMintDeployCmd() *cobra.Command { | |
| deployMode = gcf.DeploySkip | ||
| } | ||
|
|
||
| // Deploy requires at least a placeholder org for the WIF condition. | ||
| // The actual orgs are registered via 'mint enroll'. | ||
| provisioner := gcf.NewProvisioner(gcf.Config{ | ||
| cfg := gcf.Config{ | ||
| ProjectID: project, | ||
| Region: region, | ||
| GitHubOrgs: []string{gcf.PlaceholderOrg}, | ||
| AgentAppIDs: map[string]string{gcf.PlaceholderOrg: "0"}, | ||
| FunctionSourceDir: sourceDir, | ||
| DeployMode: deployMode, | ||
| }, gcpClient) | ||
| } | ||
|
|
||
| if pemDir != "" { | ||
| printer.StepStart(fmt.Sprintf("Loading PEMs and discovering app IDs for app set %q", appsetup.DefaultAppSet)) | ||
| agentPEMs, agentAppIDs, err := loadAppSetPEMs(ctx, pemDir, appsetup.DefaultAppSet) | ||
| if err != nil { | ||
| printer.StepFail("Failed to load app set PEMs") | ||
| return fmt.Errorf("loading app set PEMs: %w", err) | ||
| } | ||
| printer.StepDone(fmt.Sprintf("Loaded %d role PEMs for app set %q", len(agentPEMs), appsetup.DefaultAppSet)) | ||
|
|
||
| // The default app set name ("fullsend-ai") doubles as the PEM storage | ||
| // key prefix. Custom app sets must use admin install instead. | ||
| cfg.GitHubOrgs = []string{appsetup.DefaultAppSet} | ||
| cfg.AgentPEMs = agentPEMs | ||
| cfg.AgentAppIDs = agentAppIDs | ||
| } else { | ||
| cfg.GitHubOrgs = []string{gcf.PlaceholderOrg} | ||
| cfg.AgentAppIDs = map[string]string{gcf.PlaceholderOrg: "0"} | ||
| } | ||
|
|
||
| provisioner := gcf.NewProvisioner(cfg, gcpClient) | ||
|
|
||
| printer.StepStart("Provisioning mint infrastructure") | ||
| result, err := provisioner.Provision(ctx) | ||
|
|
@@ -137,12 +386,17 @@ func newMintDeployCmd() *cobra.Command { | |
| mintURL := result["FULLSEND_MINT_URL"] | ||
| printer.StepDone(fmt.Sprintf("Mint deployed at %s", mintURL)) | ||
| printer.Blank() | ||
| printer.Summary("Deployment complete", []string{ | ||
|
|
||
| summaryLines := []string{ | ||
| fmt.Sprintf("Project: %s", project), | ||
| fmt.Sprintf("Region: %s", region), | ||
| fmt.Sprintf("URL: %s", mintURL), | ||
| "Next: fullsend mint enroll <org> --project=" + project, | ||
| }) | ||
| } | ||
| if pemDir != "" { | ||
| summaryLines = append(summaryLines, fmt.Sprintf("App set: %s (PEMs bootstrapped)", appsetup.DefaultAppSet)) | ||
| } | ||
| summaryLines = append(summaryLines, "Next: fullsend mint enroll <org> --project="+project) | ||
| printer.Summary("Deployment complete", summaryLines) | ||
|
|
||
| return nil | ||
| }, | ||
|
|
@@ -153,6 +407,7 @@ func newMintDeployCmd() *cobra.Command { | |
| cmd.Flags().StringVar(&sourceDir, "source-dir", "", "path to local mint source (default: embedded)") | ||
| cmd.Flags().BoolVar(&skipDeploy, "skip-deploy", false, "skip code upload, reuse existing function") | ||
| cmd.Flags().BoolVar(&dryRun, "dry-run", false, "preview changes without making them") | ||
| cmd.Flags().StringVar(&pemDir, "pem-dir", "", "optional: directory containing {role}.pem files to bootstrap the default app set") | ||
|
|
||
| return cmd | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.