Conversation
…_decrypt Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds optional field-level (envelope-style) encryption to step.event_publish and introduces a new step.event_decrypt pipeline step to decrypt encrypted CloudEvent payload fields on the consumer side.
Changes:
- Extend
step.event_publishwith an optionalencryptionconfig block and emit encryption metadata via CloudEvents-style extension attributes. - Add shared AES-GCM encryption/decryption helpers (DEK generation + wrapping).
- Add
step.event_decryptand corresponding unit tests; register the new step in the pipelinesteps plugin.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/pipelinesteps/plugin.go | Registers step.event_decrypt in plugin step lists and step factory map. |
| module/pipeline_step_event_publish.go | Parses encryption config, encrypts configured payload fields, and attaches encryption metadata to the published envelope. |
| module/pipeline_step_event_encryption.go | Implements key resolution, DEK generation, AES-GCM encrypt/decrypt helpers, and payload field encryption/decryption utilities. |
| module/pipeline_step_event_decrypt.go | Implements consumer-side decryption step that reads encryption metadata from the current event and decrypts specified fields. |
| module/pipeline_step_event_decrypt_test.go | Adds unit/integration tests for helpers plus publish→decrypt round-trips. |
| // Read encryption extension attributes. | ||
| encryptedDEKB64, _ := event["encrypteddek"].(string) | ||
| encryptedFields, _ := event["encryptedfields"].(string) | ||
| keyID, _ := event["keyid"].(string) | ||
|
|
||
| // Override keyID from step configuration if provided. | ||
| if s.keyID != "" { | ||
| keyID = s.keyID | ||
| } | ||
|
|
||
| // If the event has no encryption metadata, pass through unchanged. | ||
| if encryptedDEKB64 == "" || encryptedFields == "" || keyID == "" { | ||
| return &StepResult{Output: event}, nil | ||
| } |
There was a problem hiding this comment.
step.event_decrypt decrypts whenever encrypteddek/encryptedfields/keyid are present, but it does not check the encryption extension (algorithm/provider). That makes it easy to accidentally attempt decryption on events produced by a different scheme, yielding confusing failures. Consider validating event["encryption"] (e.g., require "AES-256-GCM") and either return a clear error or passthrough when it’s missing/unsupported.
| // Build event envelope for broker/EventPublisher paths | ||
| event := s.buildEventEnvelope(resolvedPayload, resolvedHeaders, resolvedSource) | ||
| event := s.buildEventEnvelope(resolvedPayload, resolvedHeaders, resolvedSource, encMeta) | ||
|
|
||
| if s.broker != "" { | ||
| // Try EventPublisher interface first (supports external plugins like Bento) |
There was a problem hiding this comment.
With encryption configured, this code encrypts resolvedPayload and computes encMeta, but the EventBus publish path later sends only resolvedPayload (not the envelope/extensions). That would publish ciphertext without encrypteddek/encryptedfields/keyid, so consumers cannot decrypt. Consider either requiring a broker/provider when encryption is enabled, or extending the EventBus path to carry the same encryption metadata.
| enc := &EventEncryptionConfig{ | ||
| Provider: "aes", | ||
| Algorithm: "AES-256-GCM", | ||
| } | ||
| if p, ok := encCfg["provider"].(string); ok && p != "" { | ||
| enc.Provider = p | ||
| } | ||
| if k, ok := encCfg["key_id"].(string); ok { | ||
| enc.KeyID = k | ||
| } | ||
| if a, ok := encCfg["algorithm"].(string); ok && a != "" { | ||
| enc.Algorithm = a | ||
| } |
There was a problem hiding this comment.
The encryption.provider and encryption.algorithm config values are parsed and then effectively ignored by the encryption helpers (encryption is always AES-GCM and the DEK is always 32 bytes). This can mislead operators (e.g., algorithm set to something else will be reported in the event metadata but not actually used). Consider validating provider/algorithm against the supported set and returning a config error (or overriding them to the actual values used).
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
|
@copilot apply changes based on the comments in this thread Also ensure tests are passing |
…roker required, fix test assertion, decrypt algorithm validation Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Applied all reviewer feedback in commit
|
|
@copilot fix failing tests |
…fctl type registry Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Fixed in commit
All tests pass now. |
step.event_publishhad no encryption support, making it unsuitable for compliance-sensitive workloads (PII/PHI) where specific fields must be encrypted before hitting the transport layer.Changes
step.event_publish— optional encryption configencryptionblock; silently ignored whenkey_idorfieldsare absent (fully backward-compatible)encryption,keyid,encrypteddek,encryptedfieldskey_idmust be an environment variable reference ($VARor${VAR}) — literal strings are rejected to prevent operators from accidentally using non-secret identifiers (e.g. KMS ARNs) as key materialprovidermust beaesorenvelope;algorithmmust beAES-256-GCM— unsupported values return a config error at step creation timebrokerorprovideris required when encryption is configured — prevents publishing ciphertext via the EventBus path without the decryption metadata extensionsstep.event_decrypt— new consumer-side stepencrypteddek,encryptedfields,keyidextensions from the incoming CloudEvent inpc.Currentencryptionextension (algorithm) before decryption — unsupported algorithms return a clear error rather than producing garbage outputkey_idconfig overrides thekeyidextension (useful when the master key lives in a different env on the consumer side)Supporting internals (
pipeline_step_event_encryption.go)resolveEncryptionKey— resolves master key exclusively from$VARor${VAR}env references; literal key strings are rejectedapplyEventFieldEncryption/decryptEventFields— stateless helpers shared by both stepsOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.