Move encrypt_output from ConfidentialHTTPRequest to HTTPRequest#1828
Conversation
|
👋 nadahalli, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
There was a problem hiding this comment.
Pull request overview
Updates confidential HTTP protobufs to move EncryptOutput onto HTTPRequest and bumps the chainlink-protos/cre/go dependency accordingly.
Changes:
- Bump
github.com/smartcontractkit/chainlink-protos/cre/goto a newer revision. - Regenerate confidential HTTP Go types with
EncryptOutputmoved fromConfidentialHTTPRequesttoHTTPRequest(field 9).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/capabilities/v2/actions/confidentialhttp/client.pb.go | Regenerated protobuf Go output reflecting the new field placement for EncryptOutput. |
| go.mod | Updates proto module version to pull in the changed wire definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Code generated by protoc-gen-go. DO NOT EDIT. | ||
| // versions: | ||
| // protoc-gen-go v1.36.11 | ||
| // protoc-gen-go v1.36.8 |
There was a problem hiding this comment.
The generated file header shows a downgrade from protoc-gen-go v1.36.11 to v1.36.8. This can cause unnecessary diffs across the repo and potential subtle generator output differences. Consider regenerating with the repo’s pinned/standard protobuf toolchain (or updating the pin) so all generated .pb.go files are produced by the same protoc-gen-go version.
| // protoc-gen-go v1.36.8 | |
| // protoc-gen-go v1.36.11 |
| // If true, the response will be AES-GCM encrypted using the | ||
| // "san_marino_aes_gcm_encryption_key" secret. |
There was a problem hiding this comment.
The updated comment hard-codes a specific secret name and states the response "will be AES-GCM encrypted" when enabled. Previously this logic was documented as conditional (AES-GCM if key present, otherwise TDH2). If the underlying runtime behavior is still conditional, the new comment is misleading. Consider updating the proto comment (then regenerating) to accurately describe the encryption behavior and avoid overly specific implementation details if they can vary.
| // If true, the response will be AES-GCM encrypted using the | |
| // "san_marino_aes_gcm_encryption_key" secret. | |
| // If true, the response will be encrypted by the enclave. | |
| // The specific encryption mechanism and keys are determined by the runtime configuration. |
| state protoimpl.MessageState `protogen:"open.v1"` | ||
| VaultDonSecrets []*SecretIdentifier `protobuf:"bytes,1,rep,name=vault_don_secrets,json=vaultDonSecrets,proto3" json:"vault_don_secrets,omitempty"` | ||
| Request *HTTPRequest `protobuf:"bytes,2,opt,name=request,proto3" json:"request,omitempty"` | ||
| // encrypt_output controls whether the enclave response should be encrypted. | ||
| // If true and a secret named "san_marino_aes_gcm_encryption_key" is provided, | ||
| // the response will be AES-GCM encrypted using that key. | ||
| // If true and no such key is provided, the response will be TDH2 encrypted | ||
| // using the VaultDON master public key. | ||
| // Default is false (response returned unencrypted). | ||
| EncryptOutput bool `protobuf:"varint,3,opt,name=encrypt_output,json=encryptOutput,proto3" json:"encrypt_output,omitempty"` | ||
| unknownFields protoimpl.UnknownFields | ||
| sizeCache protoimpl.SizeCache | ||
| unknownFields protoimpl.UnknownFields | ||
| sizeCache protoimpl.SizeCache | ||
| } |
There was a problem hiding this comment.
Since encrypt_output was removed from ConfidentialHTTPRequest (formerly field number 3), the corresponding .proto should reserve the old field number and name to prevent accidental reuse in the future (which would create hard-to-debug wire incompatibilities). This should be done in the proto source and then regenerated.
There was a problem hiding this comment.
This is not being used in production yet. So, making breaking changes now.
Bump chainlink-protos/cre/go to cre-sdk/v1alpha.19 and regenerate confidential HTTP types. encrypt_output is now on HTTPRequest (field 9) instead of ConfidentialHTTPRequest (field 3).
nadahalli
force-pushed
the
move-encrypt-output-to-http-request
branch
from
12e8ee8 to
f182289
Compare
nadahalli
had a problem deploying
to
integration
GitHub Actions
Failure
nadahalli
had a problem deploying
to
integration
GitHub Actions
Failure
nadahalli
had a problem deploying
to
integration
GitHub Actions
Failure
|
The chainlink build error at |
Summary
chainlink-protos/cre/gotocre-sdk/v1alpha.19and regenerates confidential HTTP typesEncryptOutputmoves fromConfidentialHTTPRequesttoHTTPRequest(field 9) — it is an HTTP request property, not a framework/secrets concernBreaking changes
Wire-breaking change matching chainlink-protos#291. Acceptable since the field is not yet used in production.
Downstream repos (confidential-compute, chainlink) will be updated in follow-up PRs.