fix: set WorkflowOwner and OrgId on vault GetSecretsRequest in relay handler#21991
fix: set WorkflowOwner and OrgId on vault GetSecretsRequest in relay handler#21991
Conversation
The confidential relay handler constructs a vault GetSecretsRequest when forwarding secret fetches from the enclave, but only sets owner on each SecretIdentifier, not on the top-level request fields. The vault plugin validates the TDH2 label against WorkflowOwner/OrgId from the request payload, so both must be populated for label verification to pass. Also bumps chainlink-common to pick up the OrgID field on SecretsRequestParams.
|
👋 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! |
product-security-plaid-production
bot
requested review from
ettec and
vyzaldysanchez
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
Fixes confidential relay secret fetches by ensuring the relay handler populates the top-level WorkflowOwner and OrgId fields on the Vault GetSecretsRequest, aligning with Vault plugin label validation introduced in #21639.
Changes:
- Set
WorkflowOwnerandOrgIdon the top-levelvault.GetSecretsRequestin the confidential relay handler. - Bump
chainlink-common(and updatego.sum) to pick upOrgIDsupport onSecretsRequestParams. - Bump
chainlink-protos/cre/goto the referenced newer version.
Human review focus:
core/capabilities/confidentialrelay/handler.go:226-244— confirmWorkflowOwner/OrgIdvalues match what Vault expects for TDH2 label validation (including normalization expectations).- Dependency bumps (
go.mod/go.sum) — ensure no unintended transitive changes affect other capabilities.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/capabilities/confidentialrelay/handler.go | Populate top-level Vault request identity fields needed for ciphertext label validation. |
| go.mod | Bump chainlink-common and chainlink-protos/cre/go versions. |
| go.sum | Update checksums to match the module version bumps. |
Covers the MethodSecretsGet code path in the relay handler, verifying that WorkflowOwner is EIP-55 normalized on the vault request and OrgId is passed through from the incoming params.
|
|
CORA - Pending Reviewers
Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown For more details, see the full review summary. |
|
| vaultReq := &vault.GetSecretsRequest{ | ||
| Requests: make([]*vault.SecretRequest, 0, len(params.Secrets)), | ||
| Requests: make([]*vault.SecretRequest, 0, len(params.Secrets)), | ||
| WorkflowOwner: normalizedOwner, |
There was a problem hiding this comment.
When is this relay going to be productionized or enabled on staging?
If sometime soon, then lets keep this as a gated behavior like other places.
As examples, see these:
chainlink/core/services/workflows/v2/secrets.go
Lines 202 to 204 in eb25fa1
| Method: vault.MethodGetSecrets, | ||
| CapabilityId: vault.CapabilityID, | ||
| Config: values.EmptyMap(), | ||
| Metadata: capabilities.RequestMetadata{ |
There was a problem hiding this comment.
You need to set OrgID here too, and behind the same gate as this:
chainlink/core/services/workflows/v2/secrets.go
Lines 202 to 204 in eb25fa1
Summary
The confidential relay handler forwards secret-fetch requests from the enclave to the vault DON. It constructs a
GetSecretsRequestbut only sets the owner on eachSecretIdentifierinsideRequests, not on the top-levelWorkflowOwnerandOrgIdfields.The vault plugin's
observeGetSecretsRequestreadsWorkflowOwnerandOrgIdfrom the top-level request fields to validate the TDH2 ciphertext label viaEnsureRightLabelOnSecret. When both are empty,expectedLabelsis empty and every secret fetch fails with:This was introduced by #21639 which added label validation to
GetSecrets(previously only onCreateSecrets).Also bumps chainlink-common to pick up the
OrgIDfield onSecretsRequestParams(smartcontractkit/chainlink-common#1975).