Skip to content

Commit 6e48567

Browse files
sarg3ntclaude
andcommitted
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>
1 parent 96bd383 commit 6e48567

2 files changed

Lines changed: 21 additions & 8 deletions

File tree

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

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

95-
// Warn once at startup if secret-file encryption is not configured.
96-
if ok, err := crypto.EncryptionConfigured(); err != nil {
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 {
9799
fmt.Fprintf(os.Stderr, "Encryption key configuration error: %v\n", err)
98100
os.Exit(1)
99-
} else if !ok {
100-
logger.Warn("Secret files are stored in plaintext. " +
101-
"Set GEARBOX_AGENT_ENCRYPTION_KEY (64 hex chars, see 'openssl rand -hex 32') " +
102-
"to enable AES-256-GCM encryption-at-rest.")
103101
}
104102

105103
// Handle API key commands
@@ -179,6 +177,15 @@ func main() {
179177
"built", BuildDate,
180178
)
181179

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+
182189
// Load or create API key
183190
apiKey, isNewKey, err := crypto.LoadOrCreateAPIKey(cfg.APIKeyPath)
184191
if err != nil {

gearbox-agent/internal/framework/crypto/aead.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@ var magic = [4]byte{'G', 'B', 'E', '1'}
1717

1818
// ErrKeyRequired is returned when an encrypted file is found but no encryption key
1919
// 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).
2025
var ErrKeyRequired = errors.New(
2126
"secret file is encrypted (GBE1) but GEARBOX_AGENT_ENCRYPTION_KEY is not set; " +
22-
"set the key to decrypt, or rotate the secret (--rotate-api-key / --generate-webhook-secret)",
27+
"set the key to decrypt, or delete the file and re-generate the secret",
2328
)
2429

2530
// KeyProvider supplies the 32-byte AES-256 key used to protect on-disk secrets.
@@ -33,7 +38,8 @@ type KeyProvider interface {
3338
}
3439

3540
// EnvKeyProvider reads the encryption key from an environment variable.
36-
// The variable must contain exactly 64 lowercase hex characters (32 bytes / 256 bits).
41+
// The variable must contain exactly 64 hex characters (32 bytes / 256 bits);
42+
// both upper- and lower-case hex are accepted.
3743
// Generate a suitable value with: openssl rand -hex 32
3844
type EnvKeyProvider struct {
3945
EnvVar string // environment variable name; default "GEARBOX_AGENT_ENCRYPTION_KEY"

0 commit comments

Comments
 (0)