Add a CLI for rotating webhook secrets (#2735)

* Split out the isMinderHook helper into the GH provider code

* Add EditHook() to the GH provider interface

* Add CLI to update webhooks

cmd/server/app/webhook.go

@@ -0,0 +1,35 @@
+//
+// Copyright 2023 Stacklok, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package app
+
+import (
+	"github.com/spf13/cobra"
+)
+
+// cmdWebhook is the root command for the webhook subcommands
+func cmdWebhook() *cobra.Command {
+	var whCmd = &cobra.Command{
+		Use:   "webhook",
+		Short: "Webhook management tool",
+	}
+
+	whCmd.AddCommand(cmdWebhookUpdate())
+	return whCmd
+}
+
+func init() {
+	RootCmd.AddCommand(cmdWebhook())
+}

cmd/server/app/webhook_update.go

@@ -0,0 +1,202 @@
+//
+// Copyright 2023 Stacklok, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package app
+
+import (
+	"context"
+	"database/sql"
+	"errors"
+	"fmt"
+	"net/url"
+	"os"
+	"strings"
+
+	"github.com/rs/zerolog"
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+
+	"github.com/stacklok/minder/internal/config"
+	serverconfig "github.com/stacklok/minder/internal/config/server"
+	"github.com/stacklok/minder/internal/crypto"
+	"github.com/stacklok/minder/internal/db"
+	"github.com/stacklok/minder/internal/logger"
+	"github.com/stacklok/minder/internal/providers"
+	ghprovider "github.com/stacklok/minder/internal/providers/github"
+	provifv1 "github.com/stacklok/minder/pkg/providers/v1"
+)
+
+func cmdWebhookUpdate() *cobra.Command {
+	var updateCmd = &cobra.Command{
+		Use:   "update",
+		Short: "update the webhook configuration",
+		Long:  `Command to upgrade webhook configuration`,
+		RunE:  runCmdWebhookUpdate,
+	}
+
+	updateCmd.Flags().StringP("provider",
+		"p", "github",
+		"what provider interface must the provider implement to be updated")
+	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+
+	if err := updateCmd.MarkFlagRequired("provider"); err != nil {
+		fmt.Fprintf(os.Stderr, "Error marking flag as required: %s\n", err)
+		os.Exit(1)
+	}
+
+	return updateCmd
+}
+
+func runCmdWebhookUpdate(cmd *cobra.Command, _ []string) error {
+	cfg, err := config.ReadConfigFromViper[serverconfig.Config](viper.GetViper())
+	if err != nil {
+		return fmt.Errorf("unable to read config: %w", err)
+	}
+
+	ctx := logger.FromFlags(cfg.LoggingConfig).WithContext(context.Background())
+
+	providerName := cmd.Flag("provider").Value.String()
+
+	cryptoEng, err := crypto.EngineFromAuthConfig(&cfg.Auth)
+	if err != nil {
+		return fmt.Errorf("failed to create crypto engine: %w", err)
+	}
+
+	dbConn, _, err := cfg.Database.GetDBConnection(ctx)
+	if err != nil {
+		return fmt.Errorf("unable to connect to database: %w", err)
+	}
+	defer func(dbConn *sql.DB) {
+		err := dbConn.Close()
+		if err != nil {
+			zerolog.Ctx(ctx).Error().Err(err).Msg("error closing database connection")
+		}
+	}(dbConn)
+
+	store := db.NewStore(dbConn)
+	allProviders, err := store.GlobalListProviders(ctx)
+	if err != nil {
+		return fmt.Errorf("unable to list providers: %w", err)
+	}
+
+	webhookUrl, err := url.Parse(cfg.WebhookConfig.ExternalWebhookURL)
+	if err != nil {
+		return fmt.Errorf("unable to parse webhook url: %w", err)
+	}
+
+	for _, provider := range allProviders {
+		zerolog.Ctx(ctx).Info().Str("name", provider.Name).Str("uuid", provider.ID.String()).Msg("provider")
+		pb, err := providers.GetProviderBuilder(ctx, provider, store, cryptoEng, &cfg.Provider)
+		if err != nil {
+			return fmt.Errorf("unable to get provider builder: %w", err)
+		}
+
+		if !pb.Implements(db.ProviderType(providerName)) {
+			zerolog.Ctx(ctx).Info().
+				Str("name", provider.Name).
+				Str("uuid", provider.ID.String()).
+				Msg("provider does not implement the requested provider interface")
+			continue
+		}
+
+		var updateErr error
+
+		if db.ProviderType(providerName) == db.ProviderTypeGithub {
+			ghCli, err := pb.GetGitHub()
+			if err != nil {
+				zerolog.Ctx(ctx).Err(err).Msg("cannot get github client")
+			}
+			updateErr = updateGithubWebhooks(ctx, ghCli, store, provider, webhookUrl.Host, cfg.WebhookConfig.WebhookSecret)
+		} else {
+			updateErr = fmt.Errorf("provider type %s not supported", providerName)
+		}
+
+		if updateErr != nil {
+			zerolog.Ctx(ctx).Err(updateErr).Msg("unable to update webhooks")
+		}
+	}
+
+	return nil
+}
+
+func updateGithubWebhooks(
+	ctx context.Context,
+	ghCli provifv1.GitHub,
+	store db.Store,
+	provider db.Provider,
+	webhookHost string,
+	secret string,
+) error {
+	repos, err := store.ListRegisteredRepositoriesByProjectIDAndProvider(ctx,
+		db.ListRegisteredRepositoriesByProjectIDAndProviderParams{
+			Provider:  provider.Name,
+			ProjectID: provider.ProjectID,
+		})
+	if err != nil {
+		return fmt.Errorf("unable to list registered repositories: %w", err)
+	}
+
+	for _, repo := range repos {
+		err := updateGithubRepoHooks(ctx, ghCli, repo, webhookHost, secret)
+		if err != nil {
+			zerolog.Ctx(ctx).Err(err).Msg("unable to update repo hooks")
+			continue
+		}
+	}
+
+	return nil
+}
+
+func updateGithubRepoHooks(
+	ctx context.Context,
+	ghCli provifv1.GitHub,
+	repo db.Repository,
+	webhookHost string,
+	secret string,
+) error {
+	repoLogger := zerolog.Ctx(ctx).With().Str("repo", repo.RepoName).Str("uuid", repo.ID.String()).Logger()
+	repoLogger.Info().Msg("updating repo hooks")
+
+	hooks, err := ghCli.ListHooks(ctx, repo.RepoOwner, repo.RepoName)
+	if errors.Is(err, ghprovider.ErrNotFound) {
+		repoLogger.Debug().Msg("no hooks found")
+		return nil
+	} else if err != nil {
+		return fmt.Errorf("unable to list hooks: %w", err)
+	}
+
+	for _, hook := range hooks {
+		hookLogger := repoLogger.With().Int64("hook_id", hook.GetID()).Str("url", hook.GetURL()).Logger()
+		isMinder, err := ghprovider.IsMinderHook(hook, webhookHost)
+		if err != nil {
+			hookLogger.Err(err).Msg("unable to determine if hook is a minder hook")
+			continue
+		}
+		if !isMinder {
+			hookLogger.Info().Msg("hook is not a minder hook")
+			continue
+		}
+
+		hook.Config["secret"] = secret
+		_, err = ghCli.EditHook(ctx, repo.RepoOwner, repo.RepoName, hook.GetID(), hook)
+		if err != nil {
+			hookLogger.Err(err).Msg("unable to update hook")
+			continue
+		}
+		hookLogger.Info().Msg("hook updated")
+	}
+
+	return nil
+}

docs/docs/run_minder_server/run_the_server.md

@@ -249,6 +249,16 @@ webhook-config:
 
 After these steps, your Minder server should be ready to receive webhook events from GitHub, and add webhooks to repositories.
 
+In order to rotate webhook secrets, you can use the `minder-server` CLI tool to update the webhook secret.
+
+```bash
+minder-server webhook update -p github
+```
+
+Note that the command simply replaces the webhook secret on the provider
+side. You will need to update the webhook secret in the server configuration
+to match the provider's secret.
+
 ## Run the application
 
 ```bash

internal/controlplane/handlers_repositories_test.go

@@ -395,6 +395,10 @@ func (_ *StubGitHub) CreateHook(_ context.Context, _ string, _ string, _ *github
 	panic("unimplemented")
 }
 
+func (*StubGitHub) EditHook(context.Context, string, string, int64, *github.Hook) (*github.Hook, error) {
+	panic("unimplemented")
+}
+
 // CreatePullRequest implements v1.GitHub.
 func (*StubGitHub) CreatePullRequest(context.Context, string, string, string, string, string, string) (*github.PullRequest, error) {
 	panic("unimplemented")

internal/providers/github/common.go

@@ -493,6 +493,12 @@ func (c *GitHub) CreateHook(ctx context.Context, owner, repo string, hook *githu
 	return h, err
 }
 
+// EditHook edits an existing Hook.
+func (c *GitHub) EditHook(ctx context.Context, owner, repo string, id int64, hook *github.Hook) (*github.Hook, error) {
+	h, _, err := c.client.Repositories.EditHook(ctx, owner, repo, id, hook)
+	return h, err
+}
+
 // CreateSecurityAdvisory creates a new security advisory
 func (c *GitHub) CreateSecurityAdvisory(ctx context.Context, owner, repo, severity, summary, description string,
 	v []*github.AdvisoryVulnerability) (string, error) {
@@ -826,3 +832,20 @@ func convertRepository(repo *github.Repository) *minderv1.Repository {
 		IsFork:    *repo.Fork,
 	}
 }
+
+// IsMinderHook checks if a GitHub hook is a Minder hook
+func IsMinderHook(hook *github.Hook, hostURL string) (bool, error) {
+	configURL, ok := hook.Config["url"].(string)
+	if !ok || configURL == "" {
+		return false, fmt.Errorf("unexpected hook config structure: %v", hook.Config)
+	}
+	parsedURL, err := url.Parse(configURL)
+	if err != nil {
+		return false, err
+	}
+	if parsedURL.Host == hostURL {
+		return true, nil
+	}
+
+	return false, nil
+}

internal/repositories/github/webhooks/manager.go

@@ -148,7 +148,7 @@ func (w *webhookManager) cleanupStaleHooks(
 
 	for _, hook := range hooks {
 		// it is our hook, we can remove it
-		shouldDelete, err := isMinderHook(hook, webhookHost)
+		shouldDelete, err := ghprovider.IsMinderHook(hook, webhookHost)
 		// If err != nil, shouldDelete == false - use one error check for both calls
 		if shouldDelete {
 			err = w.DeleteWebhook(ctx, client, repoOwner, repoName, hook.GetID())
@@ -160,19 +160,3 @@ func (w *webhookManager) cleanupStaleHooks(
 
 	return nil
 }
-
-func isMinderHook(hook *github.Hook, hostURL string) (bool, error) {
-	configURL, ok := hook.Config["url"].(string)
-	if !ok || configURL == "" {
-		return false, fmt.Errorf("unexpected hook config structure: %v", hook.Config)
-	}
-	parsedURL, err := url.Parse(configURL)
-	if err != nil {
-		return false, err
-	}
-	if parsedURL.Host == hostURL {
-		return true, nil
-	}
-
-	return false, nil
-}

pkg/providers/v1/providers.go

@@ -99,6 +99,7 @@ type GitHub interface {
 	GetOwner() string
 	ListHooks(ctx context.Context, owner, repo string) ([]*github.Hook, error)
 	DeleteHook(ctx context.Context, owner, repo string, id int64) (*github.Response, error)
+	EditHook(ctx context.Context, owner, repo string, id int64, hook *github.Hook) (*github.Hook, error)
 	CreateHook(ctx context.Context, owner, repo string, hook *github.Hook) (*github.Hook, error)
 	CreateSecurityAdvisory(ctx context.Context, owner, repo, severity, summary, description string,
 		v []*github.AdvisoryVulnerability) (string, error)