Skip to content

Commit 4dc6c56

Browse files
committed
fix: JSON encode errors, encrypt under lock, docs accuracy (PR review round 3)
- Handle JSON encoding errors in retry.go and run.go instead of discarding - Move credential encryption inside advisory lock in store.Create to prevent race with concurrent RotateAll - Fix misleading --force error example in CLI docs Not applied: --output flag registration (already exists as persistent flag on root command), max_concurrent_executions_per_team default of 10 (spec says 0 = unlimited, Go zero value is correct)
1 parent b82cac9 commit 4dc6c56

4 files changed

Lines changed: 14 additions & 9 deletions

File tree

packages/engine/internal/cli/retry.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ and everything downstream re-execute.`,
9292

9393
// JSON output mode.
9494
if outputFormat == "json" {
95-
_ = json.NewEncoder(cmd.OutOrStdout()).Encode(result)
95+
if encErr := json.NewEncoder(cmd.OutOrStdout()).Encode(result); encErr != nil {
96+
return fmt.Errorf("encoding JSON output: %w", encErr)
97+
}
9698
return exitErr
9799
}
98100

packages/engine/internal/cli/run.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ func newRunCommand() *cobra.Command {
111111

112112
// JSON output mode.
113113
if outputFormat == "json" {
114-
_ = json.NewEncoder(cmd.OutOrStdout()).Encode(result)
114+
if encErr := json.NewEncoder(cmd.OutOrStdout()).Encode(result); encErr != nil {
115+
return fmt.Errorf("encoding JSON output: %w", encErr)
116+
}
115117
return exitErr
116118
}
117119

packages/engine/internal/secret/store.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,11 @@ func (s *Store) Create(ctx context.Context, name, typeName string, data map[stri
3131
return nil, fmt.Errorf("marshaling credential data: %w", err)
3232
}
3333

34-
ciphertext, nonce, err := s.Encryptor.Encrypt(plaintext)
35-
if err != nil {
36-
return nil, fmt.Errorf("encrypting credential: %w", err)
37-
}
38-
3934
teamID := auth.TeamIDFromContext(ctx)
4035

4136
// Use a transaction with the same advisory lock as RotateAll to serialize
42-
// credential writes with key rotation.
37+
// credential writes with key rotation. Encryption happens inside the lock
38+
// to prevent a concurrent RotateAll from changing keys between encrypt and insert.
4339
tx, err := s.DB.BeginTx(ctx, nil)
4440
if err != nil {
4541
return nil, fmt.Errorf("starting transaction: %w", err)
@@ -50,6 +46,11 @@ func (s *Store) Create(ctx context.Context, name, typeName string, data map[stri
5046
return nil, fmt.Errorf("acquiring advisory lock: %w", err)
5147
}
5248

49+
ciphertext, nonce, err := s.Encryptor.Encrypt(plaintext)
50+
if err != nil {
51+
return nil, fmt.Errorf("encrypting credential: %w", err)
52+
}
53+
5354
var cred Credential
5455
err = tx.QueryRowContext(ctx,
5556
`INSERT INTO credentials (name, type, encrypted_data, nonce, team_id)

packages/site/src/content/docs/cli-reference/workflow-commands.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ Execution ghi789-jkl012: completed
322322
If the execution is not in a failed state:
323323

324324
```text
325-
Error: execution abc123-def456 is not in a failed state (status: completed). Use --force to retry anyway.
325+
Error: execution abc123-def456 is not in a failed state (status: completed).
326326
```
327327

328328
If `--from-step` references a step that does not exist:

0 commit comments

Comments
 (0)