EnsureRightLabelOnSecret: dual label support for workflow_owner and org_id#21680
Merged
prashantkumar1982 merged 6 commits intodevelopfrom Apr 1, 2026
Merged
EnsureRightLabelOnSecret: dual label support for workflow_owner and org_id#21680prashantkumar1982 merged 6 commits intodevelopfrom
prashantkumar1982 merged 6 commits intodevelopfrom
Conversation
…rg_id Update EnsureRightLabelOnSecret to accept explicit workflowOwner and orgID parameters, supporting both ETH address (left-padded) and SHA256(org_id) label encodings. Centralize all label utilities in vaultutils/labels.go. Made-with: Cursor
product-security-plaid-production
bot
requested review from
krehermann,
silaslenihan and
vyzaldysanchez
Contributor
|
👋 prashantkumar1982, 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! |
Contributor
|
I see you updated files related to
|
Contributor
|
✅ No conflicts with other open PRs targeting |
| // - Ethereum address (detected via common.IsHexAddress): left-padded with 12 zero bytes | ||
| // followed by the 20-byte address, matching the legacy workflow_owner encoding. | ||
| // - org_id (any non-address string): SHA256 hash of the org_id string. | ||
| func OwnerToLabel(owner string) [32]byte { |
Contributor
There was a problem hiding this comment.
I would consider adding a version bit going forward
That way we can just check this first and then if it's present we'll know it's an org; if not we know it has to be an address
Made-with: Cursor
Replace generic OwnerToLabel (which auto-detected type via IsHexAddress) with WorkflowOwnerToLabel and OrgIDToLabel to preserve backward compat with callers that pass non-address strings through HexToAddress. Made-with: Cursor
Resolve conflicts: keep ciphertextSize validation from develop alongside dual-label EnsureRightLabelOnSecret signature; merge both test suites. Made-with: Cursor
Wire workflowOwner and orgID from request-level fields on GetSecretsRequest/CreateSecretsRequest/UpdateSecretsRequest instead of secretRequest.Id.Owner. Only check orgID label when the gate limiter is enabled; rename OrgId -> OrgID per Go naming convention. Made-with: Cursor
Made-with: Cursor
|
| // WorkflowOwnerToLabel converts a workflow owner string to a 32-byte TDH2 ciphertext | ||
| // label using the Ethereum address encoding: 12 zero bytes followed by the 20-byte address. | ||
| // This matches the legacy label format used when secrets are encrypted with a workflow owner. | ||
| func WorkflowOwnerToLabel(owner string) [32]byte { |
Contributor
There was a problem hiding this comment.
@prashantkumar1982 Is it worth accepting a common.Address here? Atm HexToAddress will truncate to the right length; I also wonder what it will do if the input isn't hex 🤔
Contributor
There was a problem hiding this comment.
(Feel free to follow up if this is urgent btw)
cedric-cordenier
approved these changes
Apr 1, 2026
timothyF95
approved these changes
Apr 1, 2026
prashantkumar1982
added a commit
that referenced
this pull request
Apr 2, 2026
…rg_id (#21680) * EnsureRightLabelOnSecret: dual label support for workflow_owner and org_id Update EnsureRightLabelOnSecret to accept explicit workflowOwner and orgID parameters, supporting both ETH address (left-padded) and SHA256(org_id) label encodings. Centralize all label utilities in vaultutils/labels.go. Made-with: Cursor * fix testifylint: use require.Error for error assertions Made-with: Cursor * fix: use dedicated label functions for workflowOwner and orgID Replace generic OwnerToLabel (which auto-detected type via IsHexAddress) with WorkflowOwnerToLabel and OrgIDToLabel to preserve backward compat with callers that pass non-address strings through HexToAddress. Made-with: Cursor * gate orgID label check behind VaultOrgIdAsSecretOwnerEnabled limiter Wire workflowOwner and orgID from request-level fields on GetSecretsRequest/CreateSecretsRequest/UpdateSecretsRequest instead of secretRequest.Id.Owner. Only check orgID label when the gate limiter is enabled; rename OrgId -> OrgID per Go naming convention. Made-with: Cursor * fix goimports ordering in system-tests vault.go Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EnsureRightLabelOnSecretto accept explicitworkflowOwnerandorgIDparameters, supporting dual label encoding: ETH address (left-padded) for legacy secrets andSHA256(org_id)for new secrets. Either parameter can be empty to skip that check.core/capabilities/vault/vaultutils/labels.go:OwnerToLabel,EncryptSecretWithWorkflowOwner,EncryptSecretWithOrgID.validator.go,plugin.go, system-testsvault.go) to use the new signature and shared utilities.EnsureRightLabelOnSecretandOwnerToLabelcovering dual-label matching, backward compat, error cases, and migration scenarios.Context
Part of the JWT-based authorization work for Vault. This PR is standalone with no behavior change — existing callers pass
""fororgID, preserving current behavior. TheorgIDparameter will be wired in by a later PR (plugin changes + final wiring).