Skip to content

Commit adef4a4

Browse files
sarg3ntclaude
andauthored
fix(security): P2-8 AES-256-GCM encryption-at-rest for on-disk secrets (#55)
* fix(security): P2-8 AES-256-GCM encryption-at-rest for on-disk secrets Secrets written to disk by gearbox-agent (API key, webhook secret) are now protected with AES-256-GCM envelope encryption when GEARBOX_AGENT_ENCRYPTION_KEY is set. Key changes: - aead.go: KeyProvider interface, EnvKeyProvider (reads 32-byte hex key from GEARBOX_AGENT_ENCRYPTION_KEY), encryptSecret/decryptSecret using AES-256-GCM, magic bytes GBE1 for auto-detection of encrypted files. - keys.go: loadOrCreateSecret and readSecret auto-detect and decrypt; auto-migrate existing plaintext files to encrypted on first boot with key set; hard-fail (ErrKeyRequired) if encrypted file found but no key is configured. New WriteAPIKey() for the --rotate-api-key path. - main.go: --rotate-api-key uses crypto.WriteAPIKey (respects encryption); startup warning when no encryption key is configured. Backward compatible: no GEARBOX_AGENT_ENCRYPTION_KEY → plaintext as before. Tier 2 KMS providers tracked in issue #54. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): address P2-8 Copilot review nits - aead.go: docstring no longer claims "lowercase hex" only; hex.DecodeString accepts both cases, and constraining the wire form would just be a foot-gun. - aead.go: ErrKeyRequired message no longer points to --generate-webhook-secret as the recovery path. That flag won't overwrite an existing file, so it wouldn't actually unblock recovery for an encrypted webhook secret. The doc comment for ErrKeyRequired explains this; the message itself just says "delete the file and re-generate the secret". - main.go: plaintext-encryption warning moved out of the one-shot CLI handler region (was firing before --show-api-key etc., polluting their stdout). Now only emitted on normal startup. Malformed-key validation still runs early so any subcommand that needs to decrypt fails cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(crypto): mark deterministic test fixtures gitleaks:allow The gitleaks "generic-api-key" rule was matching three hard-coded test fixtures in aead_test.go (lines 249, 355, 378): plainSecret := "abcdef0123456789..." // 64 hex chars apiKey := "abcdef1234567890..." // 64 hex chars (x2) These are deterministic fixtures used to exercise the secret-file round-trip, not real credentials — but they trip the rule because the variable names include "secret"/"key" and the values are 32+ chars of [a-z0-9]. Added //gitleaks:allow inline comments rather than broadening the global path allowlist so future test files don't get an accidental free pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(gitleaks): allowlist commit 96bd383 (P2-8 test fixtures) gitleaks-action scans the full PR diff range, so even after inline //gitleaks:allow comments were added to aead_test.go in commit 5477bd8, the historical commit 96bd383 (where the fixtures were introduced) continues to trip the generic-api-key rule. Adding the commit SHA to the existing allowlist (same pattern used for 2fdb5ba). The fixtures themselves are deterministic 64-hex-char values used to exercise the encryption-at-rest round-trip — not real secrets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ff336e2 commit adef4a4

5 files changed

Lines changed: 632 additions & 21 deletions

File tree

.gitleaks.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,13 @@ regexes = [
7373
# Allowlist - commits to ignore
7474
# 2fdb5ba: functional_test.go had an example API key value in a doc comment
7575
# (test-only, not a real credential — replaced with <YOUR_API_KEY> placeholder in a later commit)
76+
# 96bd383: aead_test.go added three 64-hex-char fixtures (plainSecret /
77+
# apiKey) to exercise the encryption-at-rest round-trip. A later commit
78+
# added inline //gitleaks:allow comments, but the gitleaks-action scans
79+
# the full PR diff range so the historical commit still trips the rule.
7680
commits = [
7781
"2fdb5ba0d2e8da23a6409527cdbf6696d90eb537",
82+
"96bd38365b022aa8b9d78886dfd3eb16e644eecf",
7883
]
7984

8085
# Stopwords - tokens that if found will stop gitleaks

gearbox-agent/cmd/gearbox-agent/main.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,14 @@ func main() {
9292
Level: logLevel,
9393
}))
9494

95+
// Hard-fail early if GEARBOX_AGENT_ENCRYPTION_KEY is set but malformed,
96+
// regardless of which subcommand we're about to run — one-shot CLI
97+
// commands like --show-api-key need a usable key too.
98+
if _, err := crypto.EncryptionConfigured(); err != nil {
99+
fmt.Fprintf(os.Stderr, "Encryption key configuration error: %v\n", err)
100+
os.Exit(1)
101+
}
102+
95103
// Handle API key commands
96104
if *showAPIKey {
97105
key, err := crypto.ReadAPIKey(cfg.APIKeyPath)
@@ -109,7 +117,7 @@ func main() {
109117
logger.Error("Failed to generate API key", "error", err)
110118
os.Exit(1)
111119
}
112-
if err := os.WriteFile(cfg.APIKeyPath, []byte(key), crypto.APIKeyFilePerms); err != nil {
120+
if err := crypto.WriteAPIKey(cfg.APIKeyPath, key); err != nil {
113121
logger.Error("Failed to write API key", "error", err)
114122
os.Exit(1)
115123
}
@@ -169,6 +177,15 @@ func main() {
169177
"built", BuildDate,
170178
)
171179

180+
// Warn once at startup if secret-file encryption is not configured.
181+
// Placed after one-shot flag handlers so it doesn't pollute their
182+
// stdout output (e.g. --show-api-key piped to a clipboard tool).
183+
if ok, _ := crypto.EncryptionConfigured(); !ok {
184+
logger.Warn("Secret files are stored in plaintext. " +
185+
"Set GEARBOX_AGENT_ENCRYPTION_KEY (64 hex chars, see 'openssl rand -hex 32') " +
186+
"to enable AES-256-GCM encryption-at-rest.")
187+
}
188+
172189
// Load or create API key
173190
apiKey, isNewKey, err := crypto.LoadOrCreateAPIKey(cfg.APIKeyPath)
174191
if err != nil {
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package crypto
2+
3+
import (
4+
"crypto/aes"
5+
"crypto/cipher"
6+
"crypto/rand"
7+
"encoding/hex"
8+
"errors"
9+
"fmt"
10+
"io"
11+
"os"
12+
)
13+
14+
// magic is the 4-byte header written at the start of every encrypted secret file.
15+
// Its presence tells the loader to decrypt rather than treat the file as plaintext.
16+
var magic = [4]byte{'G', 'B', 'E', '1'}
17+
18+
// ErrKeyRequired is returned when an encrypted file is found but no encryption key
19+
// has been configured. Key loss means rotation — there is no recovery path.
20+
//
21+
// Recovery: set GEARBOX_AGENT_ENCRYPTION_KEY to the correct value to decrypt;
22+
// or delete the file and re-generate (--rotate-api-key for the API key,
23+
// --generate-webhook-secret for the webhook secret — note the latter does
24+
// NOT overwrite an existing file, so you must rm it first).
25+
var ErrKeyRequired = errors.New(
26+
"secret file is encrypted (GBE1) but GEARBOX_AGENT_ENCRYPTION_KEY is not set; " +
27+
"set the key to decrypt, or delete the file and re-generate the secret",
28+
)
29+
30+
// KeyProvider supplies the 32-byte AES-256 key used to protect on-disk secrets.
31+
// Implementations include [EnvKeyProvider] (reads from an environment variable) and,
32+
// in future releases, cloud-KMS backends (see GitHub issue #54).
33+
type KeyProvider interface {
34+
// Key returns the 32-byte AES-256 key, or nil if encryption is not configured.
35+
// A nil key means secrets are stored as plaintext; a non-nil key means they are
36+
// encrypted with AES-256-GCM.
37+
Key() ([]byte, error)
38+
}
39+
40+
// EnvKeyProvider reads the encryption key from an environment variable.
41+
// The variable must contain exactly 64 hex characters (32 bytes / 256 bits);
42+
// both upper- and lower-case hex are accepted.
43+
// Generate a suitable value with: openssl rand -hex 32
44+
type EnvKeyProvider struct {
45+
EnvVar string // environment variable name; default "GEARBOX_AGENT_ENCRYPTION_KEY"
46+
}
47+
48+
// DefaultKeyProvider is the KeyProvider used by loadOrCreateSecret and readSecret
49+
// unless overridden in tests.
50+
var DefaultKeyProvider KeyProvider = &EnvKeyProvider{EnvVar: "GEARBOX_AGENT_ENCRYPTION_KEY"}
51+
52+
// Key implements [KeyProvider]. Returns nil, nil when the variable is unset.
53+
func (e *EnvKeyProvider) Key() ([]byte, error) {
54+
val := os.Getenv(e.EnvVar)
55+
if val == "" {
56+
return nil, nil
57+
}
58+
key, err := hex.DecodeString(val)
59+
if err != nil {
60+
return nil, fmt.Errorf("%s: invalid hex value: %w", e.EnvVar, err)
61+
}
62+
if len(key) != 32 {
63+
return nil, fmt.Errorf(
64+
"%s: must be exactly 64 hex characters (32 bytes); got %d bytes — generate with: openssl rand -hex 32",
65+
e.EnvVar, len(key),
66+
)
67+
}
68+
return key, nil
69+
}
70+
71+
// EncryptionConfigured returns true when the default key provider has a key set.
72+
// Used at startup to emit a warning when encryption is not configured.
73+
func EncryptionConfigured() (bool, error) {
74+
key, err := DefaultKeyProvider.Key()
75+
if err != nil {
76+
return false, err
77+
}
78+
return key != nil, nil
79+
}
80+
81+
// isEncrypted returns true when data begins with the GBE1 magic header.
82+
func isEncrypted(data []byte) bool {
83+
if len(data) < len(magic) {
84+
return false
85+
}
86+
return data[0] == magic[0] && data[1] == magic[1] &&
87+
data[2] == magic[2] && data[3] == magic[3]
88+
}
89+
90+
// encryptSecret encrypts plaintext using AES-256-GCM and returns the
91+
// magic-prefixed blob: magic(4) + nonce(12) + GCM-ciphertext-with-tag.
92+
func encryptSecret(plaintext string, key []byte) ([]byte, error) {
93+
block, err := aes.NewCipher(key)
94+
if err != nil {
95+
return nil, fmt.Errorf("aes.NewCipher: %w", err)
96+
}
97+
gcm, err := cipher.NewGCM(block)
98+
if err != nil {
99+
return nil, fmt.Errorf("cipher.NewGCM: %w", err)
100+
}
101+
nonce := make([]byte, gcm.NonceSize())
102+
if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
103+
return nil, fmt.Errorf("generate nonce: %w", err)
104+
}
105+
sealed := gcm.Seal(nil, nonce, []byte(plaintext), nil)
106+
107+
out := make([]byte, 0, len(magic)+len(nonce)+len(sealed))
108+
out = append(out, magic[:]...)
109+
out = append(out, nonce...)
110+
out = append(out, sealed...)
111+
return out, nil
112+
}
113+
114+
// decryptSecret decrypts a blob produced by [encryptSecret].
115+
// data must start with the GBE1 magic header.
116+
func decryptSecret(data []byte, key []byte) (string, error) {
117+
if !isEncrypted(data) {
118+
return "", errors.New("decryptSecret: data does not start with GBE1 magic")
119+
}
120+
block, err := aes.NewCipher(key)
121+
if err != nil {
122+
return "", fmt.Errorf("aes.NewCipher: %w", err)
123+
}
124+
gcm, err := cipher.NewGCM(block)
125+
if err != nil {
126+
return "", fmt.Errorf("cipher.NewGCM: %w", err)
127+
}
128+
payload := data[len(magic):]
129+
nonceSize := gcm.NonceSize()
130+
if len(payload) < nonceSize {
131+
return "", errors.New("decryptSecret: ciphertext too short")
132+
}
133+
plaintext, err := gcm.Open(nil, payload[:nonceSize], payload[nonceSize:], nil)
134+
if err != nil {
135+
return "", fmt.Errorf("gcm.Open: %w (key may be wrong or file corrupted)", err)
136+
}
137+
return string(plaintext), nil
138+
}

0 commit comments

Comments
 (0)