From be00a7abacc16c805c9ca9bb3085096860186552 Mon Sep 17 00:00:00 2001 From: zoetaka38 Date: Tue, 7 Jul 2026 05:34:16 +0200 Subject: [PATCH] feat(ci): real sealed-box action secrets + fix double-encryption at rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GitHub-compatible secrets API was non-functional: it advertised a placeholder all-zeros public key (which libsodium refuses to seal against) and "decrypted" the client's sealed box with the symmetric key, so no real client could ever store a secret. Enabling it also surfaced a latent double-encryption bug that corrupted every stored secret. - Derive a real curve25519 keypair deterministically from the server secret (shared across replicas, stable across restarts) and advertise the true public key. DecryptSealedBox now opens the client's anonymous ("sealed") box with the private key — the libsodium crypto_box_seal format GitHub uses. - Fix double-encryption: the upsert usecase encrypted the value AND the repository encrypted it again, while reads decrypt only once — so the CI worker's single decrypt yielded ciphertext, not the secret (steps then failed with a corrupt env var). The repository is now the single at-rest encryption boundary; the usecase passes the plaintext through. Verified live end-to-end: fetch the real public key, seal a value with libsodium (pynacl), PUT it (201), then a workflow reads it via `$API_TOKEN` and `${{ secrets.API_TOKEN }}` — both resolve to the correct plaintext (shell sees the right length) and are masked to *** in the logs. Unit tests cover the sealed-box round trip, that the public key is real and deterministic, and that the usecase hands plaintext to the repository. --- backend/cmd/server/main.go | 10 ++- .../crypto/action_secret_sealedbox.go | 47 ++++++++++++++ .../crypto/action_secret_sealedbox_test.go | 62 +++++++++++++++++++ .../secret/upsert_action_secret_usecase.go | 21 +++---- .../upsert_action_secret_usecase_test.go | 17 ++--- 5 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 backend/internal/infrastructure/crypto/action_secret_sealedbox.go create mode 100644 backend/internal/infrastructure/crypto/action_secret_sealedbox_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 738d20be..5d3cf099 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -1583,12 +1583,16 @@ func (e *actionSecretEncryptor) KeyID() string { } func (e *actionSecretEncryptor) PublicKeyBase64() string { - // Placeholder public key for GitHub API shape compatibility. - return "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" + // Real curve25519 public key derived from the server secret; clients seal + // secret values to it (GitHub's libsodium sealed-box format). + return e.SecretEncryptor.PublicKeyBase64() } func (e *actionSecretEncryptor) DecryptSealedBox(ciphertext []byte) ([]byte, error) { - return e.Decrypt(ciphertext) + // The client sends a sealed box (anonymous box) encrypted to + // PublicKeyBase64; open it here. Storage at rest still uses symmetric + // Encrypt/Decrypt. + return e.SecretEncryptor.OpenSealedBox(ciphertext) } func httpStatusToCode(status int) string { diff --git a/backend/internal/infrastructure/crypto/action_secret_sealedbox.go b/backend/internal/infrastructure/crypto/action_secret_sealedbox.go new file mode 100644 index 00000000..09470cac --- /dev/null +++ b/backend/internal/infrastructure/crypto/action_secret_sealedbox.go @@ -0,0 +1,47 @@ +package crypto + +import ( + "encoding/base64" + "fmt" + + "golang.org/x/crypto/blake2b" + "golang.org/x/crypto/curve25519" + "golang.org/x/crypto/nacl/box" +) + +// sealedBoxLabel domain-separates the curve25519 keypair derivation so it can +// never collide with any other use of the symmetric key. +const sealedBoxLabel = "open-git-action-secrets-box-v1" + +// sealedBoxKeypair deterministically derives the server's curve25519 keypair +// from the symmetric secret key. Determinism matters so every replica advertises +// the same public key and can open boxes sealed against any replica, and so the +// key survives restarts. +func (e *SecretEncryptor) sealedBoxKeypair() (pub, priv [32]byte) { + seed := blake2b.Sum256(append(append([]byte{}, e.key...), []byte(sealedBoxLabel)...)) + copy(priv[:], seed[:]) + pubSlice, err := curve25519.X25519(priv[:], curve25519.Basepoint) + if err == nil { + copy(pub[:], pubSlice) + } + return pub, priv +} + +// PublicKeyBase64 returns the base64-encoded curve25519 public key clients use +// to seal secret values (GitHub's libsodium "sealed box" format). +func (e *SecretEncryptor) PublicKeyBase64() string { + pub, _ := e.sealedBoxKeypair() + return base64.StdEncoding.EncodeToString(pub[:]) +} + +// OpenSealedBox opens an anonymous ("sealed") box that a client encrypted to the +// server's public key, returning the plaintext secret. This is the transport +// decryption; callers re-encrypt the plaintext at rest with Encrypt. +func (e *SecretEncryptor) OpenSealedBox(ciphertext []byte) ([]byte, error) { + pub, priv := e.sealedBoxKeypair() + out, ok := box.OpenAnonymous(nil, ciphertext, &pub, &priv) + if !ok { + return nil, fmt.Errorf("sealed box could not be opened with the server key") + } + return out, nil +} diff --git a/backend/internal/infrastructure/crypto/action_secret_sealedbox_test.go b/backend/internal/infrastructure/crypto/action_secret_sealedbox_test.go new file mode 100644 index 00000000..73fae17f --- /dev/null +++ b/backend/internal/infrastructure/crypto/action_secret_sealedbox_test.go @@ -0,0 +1,62 @@ +package crypto + +import ( + "crypto/rand" + "encoding/base64" + "testing" + + "golang.org/x/crypto/nacl/box" +) + +func TestSealedBoxRoundTrip(t *testing.T) { + key := make([]byte, aes256KeySize) + for i := range key { + key[i] = byte(i + 1) + } + enc := NewSecretEncryptor(key) + + // A client seals a value against the advertised public key. + pubB64 := enc.PublicKeyBase64() + pubBytes, err := base64.StdEncoding.DecodeString(pubB64) + if err != nil || len(pubBytes) != 32 { + t.Fatalf("public key not valid base64/32 bytes: %v (len %d)", err, len(pubBytes)) + } + var pub [32]byte + copy(pub[:], pubBytes) + + secret := []byte("super-secret-token-42") + sealed, err := box.SealAnonymous(nil, secret, &pub, rand.Reader) + if err != nil { + t.Fatalf("seal: %v", err) + } + + // The server opens it. + got, err := enc.OpenSealedBox(sealed) + if err != nil { + t.Fatalf("OpenSealedBox: %v", err) + } + if string(got) != string(secret) { + t.Errorf("opened secret = %q, want %q", got, secret) + } +} + +func TestPublicKeyIsNotPlaceholderAndDeterministic(t *testing.T) { + key := make([]byte, aes256KeySize) // all-zero dev key + enc := NewSecretEncryptor(key) + + pub := enc.PublicKeyBase64() + if pub == "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" || pub == "" { + t.Fatalf("public key is the placeholder/empty: %q", pub) + } + // Deterministic across instances with the same key (needed for multi-replica). + if again := NewSecretEncryptor(key).PublicKeyBase64(); again != pub { + t.Errorf("public key not deterministic: %q vs %q", pub, again) + } +} + +func TestOpenSealedBoxRejectsGarbage(t *testing.T) { + enc := NewSecretEncryptor(make([]byte, aes256KeySize)) + if _, err := enc.OpenSealedBox([]byte("not a sealed box")); err == nil { + t.Error("expected error opening non-sealed-box input") + } +} diff --git a/backend/internal/usecase/secret/upsert_action_secret_usecase.go b/backend/internal/usecase/secret/upsert_action_secret_usecase.go index 7c391976..956ef377 100644 --- a/backend/internal/usecase/secret/upsert_action_secret_usecase.go +++ b/backend/internal/usecase/secret/upsert_action_secret_usecase.go @@ -39,11 +39,11 @@ type AuditLogWriter interface { } type UpsertActionSecretInput struct { - ActorID uuid.UUID - Name string - PlaintextValue string - Visibility SecretVisibility - SelectedRepoIDs []uuid.UUID + ActorID uuid.UUID + Name string + PlaintextValue string + Visibility SecretVisibility + SelectedRepoIDs []uuid.UUID } type UpsertActionSecretUsecase struct { @@ -80,16 +80,15 @@ func (uc *UpsertActionSecretUsecase) Execute( return false, fmt.Errorf("%w: secret value exceeds maximum size", apperror.ErrValidation) } - encrypted, err := uc.enc.Encrypt([]byte(input.PlaintextValue)) - if err != nil { - return false, err - } - + // The repository is the single at-rest encryption boundary: it encrypts on + // write and decrypts on read. Pass the plaintext through here (encrypting it + // again in this layer double-encrypts it, so the worker's single decrypt + // yields ciphertext, not the secret). secret := &entity.ActionSecret{ ID: uuid.New(), OrganizationID: orgID, Name: input.Name, - EncryptedValue: string(encrypted), + EncryptedValue: input.PlaintextValue, KeyID: uc.enc.KeyID(), Visibility: string(input.Visibility), } diff --git a/backend/internal/usecase/secret/upsert_action_secret_usecase_test.go b/backend/internal/usecase/secret/upsert_action_secret_usecase_test.go index 63f11621..bc4039f8 100644 --- a/backend/internal/usecase/secret/upsert_action_secret_usecase_test.go +++ b/backend/internal/usecase/secret/upsert_action_secret_usecase_test.go @@ -125,24 +125,15 @@ func TestUpsertActionSecretUsecaseSuccess(t *testing.T) { if repo.upserted == nil { t.Fatalf("expected secret to be upserted") } - if repo.upserted.EncryptedValue == plaintext { - t.Fatalf("expected encrypted value to be stored") - } - if repo.upserted.EncryptedValue == "" { - t.Fatalf("expected encrypted value to be stored") + // The repository is the single at-rest encryption boundary, so the usecase + // hands it the plaintext value (encrypting here too would double-encrypt). + if repo.upserted.EncryptedValue != plaintext { + t.Fatalf("value passed to repo = %q, want plaintext %q", repo.upserted.EncryptedValue, plaintext) } if repo.upserted.KeyID != "test-key-id" { t.Fatalf("key id = %q, want test-key-id", repo.upserted.KeyID) } - decrypted, err := enc.Decrypt([]byte(repo.upserted.EncryptedValue)) - if err != nil { - t.Fatalf("decrypt stored value: %v", err) - } - if string(decrypted) != plaintext { - t.Fatalf("decrypted value = %q, want %q", string(decrypted), plaintext) - } - if auditRepo.action != "secret.create" { t.Fatalf("audit action = %q, want secret.create", auditRepo.action) }