vault: validate encrypted value size in request validator#21756
vault: validate encrypted value size in request validator#21756prashantkumar1982 merged 1 commit intodevelopfrom
Conversation
product-security-plaid-production
bot
requested review from
george-dorin and
ilija42
|
👋 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! |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcc73ae63c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err != nil { | ||
| return nil, fmt.Errorf("could not create request batch size limiter: %w", err) | ||
| } | ||
| ciphertextLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultCiphertextSizeLimit) |
There was a problem hiding this comment.
The new ciphertextLimiter created for RequestValidator is never closed, while Close() only shuts down MaxRequestBatchSizeLimiter. In deployments where the capability is restarted (tests, reconfiguration, process lifecycle), this leaves the limiter's underlying resources/subscriptions alive and can accumulate goroutines or watchers over time. Add MaxCiphertextLengthLimiter.Close() to (*Capability).Close() alongside the existing limiter cleanup.
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return nil, fmt.Errorf("could not create request batch size limiter: %w", err) | ||
| } | ||
| ciphertextLimiter, err := limits.MakeUpperBoundLimiter(limitsFactory, cresettings.Default.VaultCiphertextSizeLimit) |
There was a problem hiding this comment.
This adds a second limiter (ciphertextLimiter) to the handler's validator, but (*handler).Close() still only closes writeMethodsEnabled and MaxRequestBatchSizeLimiter. If the gateway handler is started/stopped repeatedly, the unclosed ciphertext limiter can leak internal limiter resources and degrade long-running processes. Include MaxCiphertextLengthLimiter.Close() in the errors.Join(...) cleanup list.
Useful? React with 👍 / 👎.
|
prashantkumar1982
deleted the
codex/vault-encrypted-value-size-validation
branch
3 participants
Summary
VaultCiphertextSizeLimitin the Vault request validator for create/update requestsEncryptedValuepayloads before label verification