From 72786a9febc1ddaa72bf4fa3add78e5c87a4e08f Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 15 Feb 2026 14:06:53 -0800 Subject: [PATCH] fix: use hashed webhook secrets for constant-time comparison Store a SHA-256 hash of the webhook secret in a new webhook_secret_hash column. FindAppByWebhookSecret now hashes the incoming secret and queries by hash, eliminating the SQL string comparison timing side-channel. - Add migration 005_add_webhook_secret_hash.sql - Add database.HashWebhookSecret() helper - Backfill existing secrets on startup - Update App model to include WebhookSecretHash in all queries - Update app creation to compute hash at insert time - Add TestHashWebhookSecret unit test - Update all test fixtures to set WebhookSecretHash Closes #13 --- internal/database/database.go | 56 +++++++++++++++++++ internal/database/hash_test.go | 28 ++++++++++ .../005_add_webhook_secret_hash.sql | 2 + internal/models/app.go | 53 +++++++++--------- internal/models/models_test.go | 2 + internal/service/app/app.go | 2 + internal/service/webhook/webhook_test.go | 1 + 7 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 internal/database/hash_test.go create mode 100644 internal/database/migrations/005_add_webhook_secret_hash.sql diff --git a/internal/database/database.go b/internal/database/database.go index 0479445..2d9d22d 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -3,7 +3,9 @@ package database import ( "context" + "crypto/sha256" "database/sql" + "encoding/hex" "fmt" "log/slog" "os" @@ -158,6 +160,60 @@ func (d *Database) connect(ctx context.Context) error { return fmt.Errorf("failed to run migrations: %w", err) } + // Backfill webhook_secret_hash for any rows that have a secret but no hash + err = d.backfillWebhookSecretHashes(ctx) + if err != nil { + return fmt.Errorf("failed to backfill webhook secret hashes: %w", err) + } + + return nil +} + +// HashWebhookSecret returns the hex-encoded SHA-256 hash of a webhook secret. +func HashWebhookSecret(secret string) string { + sum := sha256.Sum256([]byte(secret)) + return hex.EncodeToString(sum[:]) +} + +func (d *Database) backfillWebhookSecretHashes(ctx context.Context) error { + rows, err := d.database.QueryContext(ctx, + "SELECT id, webhook_secret FROM apps WHERE webhook_secret_hash = '' AND webhook_secret != ''") + if err != nil { + return fmt.Errorf("querying apps for backfill: %w", err) + } + defer func() { _ = rows.Close() }() + + type row struct { + id, secret string + } + + var toUpdate []row + + for rows.Next() { + var r row + if scanErr := rows.Scan(&r.id, &r.secret); scanErr != nil { + return fmt.Errorf("scanning app for backfill: %w", scanErr) + } + + toUpdate = append(toUpdate, r) + } + + if rowsErr := rows.Err(); rowsErr != nil { + return fmt.Errorf("iterating apps for backfill: %w", rowsErr) + } + + for _, r := range toUpdate { + hash := HashWebhookSecret(r.secret) + + _, updateErr := d.database.ExecContext(ctx, + "UPDATE apps SET webhook_secret_hash = ? WHERE id = ?", hash, r.id) + if updateErr != nil { + return fmt.Errorf("updating webhook_secret_hash for app %s: %w", r.id, updateErr) + } + + d.log.Info("backfilled webhook_secret_hash", "app_id", r.id) + } + return nil } diff --git a/internal/database/hash_test.go b/internal/database/hash_test.go new file mode 100644 index 0000000..a35ba24 --- /dev/null +++ b/internal/database/hash_test.go @@ -0,0 +1,28 @@ +package database_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "git.eeqj.de/sneak/upaas/internal/database" +) + +func TestHashWebhookSecret(t *testing.T) { + t.Parallel() + + // Known SHA-256 of "test-secret" + hash := database.HashWebhookSecret("test-secret") + assert.Equal(t, + "9caf06bb4436cdbfa20af9121a626bc1093c4f54b31c0fa937957856135345b6", + hash, + ) + + // Different secrets produce different hashes + hash2 := database.HashWebhookSecret("other-secret") + assert.NotEqual(t, hash, hash2) + + // Same secret always produces same hash (deterministic) + hash3 := database.HashWebhookSecret("test-secret") + assert.Equal(t, hash, hash3) +} diff --git a/internal/database/migrations/005_add_webhook_secret_hash.sql b/internal/database/migrations/005_add_webhook_secret_hash.sql new file mode 100644 index 0000000..db99844 --- /dev/null +++ b/internal/database/migrations/005_add_webhook_secret_hash.sql @@ -0,0 +1,2 @@ +-- Add webhook_secret_hash column for constant-time secret lookup +ALTER TABLE apps ADD COLUMN webhook_secret_hash TEXT NOT NULL DEFAULT ''; diff --git a/internal/models/app.go b/internal/models/app.go index 1fe4f02..ab21924 100644 --- a/internal/models/app.go +++ b/internal/models/app.go @@ -10,6 +10,12 @@ import ( "git.eeqj.de/sneak/upaas/internal/database" ) +// appColumns is the standard column list for app queries. +const appColumns = `id, name, repo_url, branch, dockerfile_path, webhook_secret, + ssh_private_key, ssh_public_key, image_id, status, + docker_network, ntfy_topic, slack_webhook, webhook_secret_hash, + created_at, updated_at` + // AppStatus represents the status of an app. type AppStatus string @@ -31,8 +37,9 @@ type App struct { RepoURL string Branch string DockerfilePath string - WebhookSecret string - SSHPrivateKey string + WebhookSecret string + WebhookSecretHash string + SSHPrivateKey string SSHPublicKey string ImageID sql.NullString Status AppStatus @@ -70,11 +77,8 @@ func (a *App) Delete(ctx context.Context) error { // Reload refreshes the app from the database. func (a *App) Reload(ctx context.Context) error { - row := a.db.QueryRow(ctx, ` - SELECT id, name, repo_url, branch, dockerfile_path, webhook_secret, - ssh_private_key, ssh_public_key, image_id, status, - docker_network, ntfy_topic, slack_webhook, created_at, updated_at - FROM apps WHERE id = ?`, + row := a.db.QueryRow(ctx, + "SELECT "+appColumns+" FROM apps WHERE id = ?", a.ID, ) @@ -136,13 +140,13 @@ func (a *App) insert(ctx context.Context) error { INSERT INTO apps ( id, name, repo_url, branch, dockerfile_path, webhook_secret, ssh_private_key, ssh_public_key, image_id, status, - docker_network, ntfy_topic, slack_webhook - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + docker_network, ntfy_topic, slack_webhook, webhook_secret_hash + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` _, err := a.db.Exec(ctx, query, a.ID, a.Name, a.RepoURL, a.Branch, a.DockerfilePath, a.WebhookSecret, a.SSHPrivateKey, a.SSHPublicKey, a.ImageID, a.Status, - a.DockerNetwork, a.NtfyTopic, a.SlackWebhook, + a.DockerNetwork, a.NtfyTopic, a.SlackWebhook, a.WebhookSecretHash, ) if err != nil { return err @@ -177,6 +181,7 @@ func (a *App) scan(row *sql.Row) error { &a.SSHPrivateKey, &a.SSHPublicKey, &a.ImageID, &a.Status, &a.DockerNetwork, &a.NtfyTopic, &a.SlackWebhook, + &a.WebhookSecretHash, &a.CreatedAt, &a.UpdatedAt, ) } @@ -193,6 +198,7 @@ func scanApps(appDB *database.Database, rows *sql.Rows) ([]*App, error) { &app.SSHPrivateKey, &app.SSHPublicKey, &app.ImageID, &app.Status, &app.DockerNetwork, &app.NtfyTopic, &app.SlackWebhook, + &app.WebhookSecretHash, &app.CreatedAt, &app.UpdatedAt, ) if scanErr != nil { @@ -221,11 +227,8 @@ func FindApp( app := NewApp(appDB) app.ID = appID - row := appDB.QueryRow(ctx, ` - SELECT id, name, repo_url, branch, dockerfile_path, webhook_secret, - ssh_private_key, ssh_public_key, image_id, status, - docker_network, ntfy_topic, slack_webhook, created_at, updated_at - FROM apps WHERE id = ?`, + row := appDB.QueryRow(ctx, + "SELECT "+appColumns+" FROM apps WHERE id = ?", appID, ) @@ -241,7 +244,8 @@ func FindApp( return app, nil } -// FindAppByWebhookSecret finds an app by webhook secret. +// FindAppByWebhookSecret finds an app by webhook secret using a SHA-256 hash +// lookup. This avoids SQL string comparison timing side-channels. // //nolint:nilnil // returning nil,nil is idiomatic for "not found" in Active Record func FindAppByWebhookSecret( @@ -250,13 +254,11 @@ func FindAppByWebhookSecret( secret string, ) (*App, error) { app := NewApp(appDB) + secretHash := database.HashWebhookSecret(secret) - row := appDB.QueryRow(ctx, ` - SELECT id, name, repo_url, branch, dockerfile_path, webhook_secret, - ssh_private_key, ssh_public_key, image_id, status, - docker_network, ntfy_topic, slack_webhook, created_at, updated_at - FROM apps WHERE webhook_secret = ?`, - secret, + row := appDB.QueryRow(ctx, + "SELECT "+appColumns+" FROM apps WHERE webhook_secret_hash = ?", + secretHash, ) err := app.scan(row) @@ -273,11 +275,8 @@ func FindAppByWebhookSecret( // AllApps returns all apps ordered by name. func AllApps(ctx context.Context, appDB *database.Database) ([]*App, error) { - rows, err := appDB.Query(ctx, ` - SELECT id, name, repo_url, branch, dockerfile_path, webhook_secret, - ssh_private_key, ssh_public_key, image_id, status, - docker_network, ntfy_topic, slack_webhook, created_at, updated_at - FROM apps ORDER BY name`, + rows, err := appDB.Query(ctx, + "SELECT "+appColumns+" FROM apps ORDER BY name", ) if err != nil { return nil, fmt.Errorf("querying all apps: %w", err) diff --git a/internal/models/models_test.go b/internal/models/models_test.go index bb3bee3..39162cf 100644 --- a/internal/models/models_test.go +++ b/internal/models/models_test.go @@ -297,6 +297,7 @@ func TestAllApps(t *testing.T) { app.Branch = testBranch app.DockerfilePath = "Dockerfile" app.WebhookSecret = "secret-" + strconv.Itoa(idx) + app.WebhookSecretHash = database.HashWebhookSecret(app.WebhookSecret) app.SSHPrivateKey = "private" app.SSHPublicKey = "public" @@ -791,6 +792,7 @@ func createTestApp(t *testing.T, testDB *database.Database) *models.App { app.Branch = testBranch app.DockerfilePath = "Dockerfile" app.WebhookSecret = "secret-" + t.Name() + app.WebhookSecretHash = database.HashWebhookSecret(app.WebhookSecret) app.SSHPrivateKey = "private" app.SSHPublicKey = "public" diff --git a/internal/service/app/app.go b/internal/service/app/app.go index 2790a74..221bba4 100644 --- a/internal/service/app/app.go +++ b/internal/service/app/app.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" "github.com/oklog/ulid/v2" + "go.uber.org/fx" "git.eeqj.de/sneak/upaas/internal/database" @@ -82,6 +83,7 @@ func (svc *Service) CreateApp( } app.WebhookSecret = uuid.New().String() + app.WebhookSecretHash = database.HashWebhookSecret(app.WebhookSecret) app.SSHPrivateKey = keyPair.PrivateKey app.SSHPublicKey = keyPair.PublicKey app.Status = models.AppStatusPending diff --git a/internal/service/webhook/webhook_test.go b/internal/service/webhook/webhook_test.go index 9eea66e..88d4281 100644 --- a/internal/service/webhook/webhook_test.go +++ b/internal/service/webhook/webhook_test.go @@ -91,6 +91,7 @@ func createTestApp( app.Branch = branch app.DockerfilePath = "Dockerfile" app.WebhookSecret = "webhook-secret-123" + app.WebhookSecretHash = database.HashWebhookSecret(app.WebhookSecret) app.SSHPrivateKey = "private-key" app.SSHPublicKey = "public-key" app.Status = models.AppStatusPending