vault: validate encrypted value size in request validator#21759
Conversation
(cherry picked from commit bcc73ae)
product-security-plaid-production
bot
requested review from
shileiwill and
vyzaldysanchez
|
✅ No conflicts with other open PRs targeting |
|
I see you updated files related to
|
![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: 5f9636c276
ℹ️ 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.
NewCapability now allocates a second limiter for ciphertext size, but (*Capability).Close still only closes MaxRequestBatchSizeLimiter. On nodes where this capability is restarted (process reloads, integration tests, hot reconfiguration), the unclosed MaxCiphertextLengthLimiter can leave behind limiter resources/goroutines and accumulate over time; the new limiter should be closed alongside the existing one.
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.
NewHandler now creates ciphertextLimiter, but (*handler).Close still closes only writeMethodsEnabled and MaxRequestBatchSizeLimiter. This introduces a lifecycle leak for the new limiter whenever the gateway handler is stopped and started again, so the ciphertext limiter needs to be included in the close path.
Useful? React with 👍 / 👎.
|
prashantkumar1982
deleted the
codex/vault-encrypted-value-size-validation-2.39.2
branch
2 participants
Summary
VaultCiphertextSizeLimitin the Vault request validator for create/update requestsEncryptedValuepayloads before label verificationTesting