Automated Test: feature-authz-crypto-implementation #312
+673
−9
Conversation
This file contains 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
…tory closes #33831 Signed-off-by: mposolda <mposolda@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces a pluggable crypto provider framework for the Keycloak authorization client. A new AuthzClientCryptoProvider implementation with ASN.1 encoding/decoding utilities enables ECDSA signature handling. CryptoIntegration was refactored to support multiple providers with ordering precedence. All existing crypto providers now implement an order() method. Changes
Sequence Diagram(s)sequenceDiagram
participant AC as AuthzClient
participant CI as CryptoIntegration
participant SL as ServiceLoader
participant ACP as AuthzClientCryptoProvider
participant DCP as DefaultCryptoProvider
AC->>AC: create(config)
AC->>CI: init(classloader)
CI->>SL: load(CryptoProvider)
SL->>ACP: discover()
SL->>DCP: discover()
SL-->>CI: [ACP(order:100), DCP(order:200)]
CI->>CI: sort by order() descending
CI->>CI: select first provider (DCP, order:200)
CI-->>AC: crypto ready
sequenceDiagram
participant Test as ECDSAAlgorithmTest
participant Sig as Signature
participant ACP as AuthzClientCryptoProvider
participant ASN1D as ASN1Decoder
participant ASN1E as ASN1Encoder
Test->>Sig: sign(data)
Sig-->>Test: concatenated RS bytes
Test->>ACP: concatenatedRSToASN1DER(rsBytes, length)
ACP->>ASN1E: create().write(R).write(S).writeDerSeq()
ASN1E-->>ACP: DER-encoded sequence
ACP-->>Test: DER bytes
Test->>ACP: asn1derToConcatenatedRS(derBytes, length)
ACP->>ASN1D: readSequence()
ASN1D-->>ACP: [R, S integers]
ACP->>ACP: convert to bytes, concatenate
ACP-->>Test: concatenated RS bytes
Test->>Test: assert roundtrip equality
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Decoder.java`:
- Around line 156-163: The bounds check in ASN1Decoder that currently uses "if
(length >= limit)" is incorrect for partially-read streams because it ignores
bytes already consumed (count); change it to verify the requested length does
not exceed the remaining bytes by using "if (length > (limit - count))" (or
equivalent) so that length larger than the remaining buffer throws, while valid
lengths that fit the remaining bytes are allowed.
- Around line 49-63: In readSequence(), the loop subtracts the full TLV byte
array returned by readNext() from the SEQUENCE content length, causing a
mismatch; change the loop in ASN1Decoder.readSequence to decrement length by
only the value (V) portion length consumed (not the entire TLV), e.g. obtain the
value length from the TLV metadata or by querying the length parsed by
readNext()/readLength() or by measuring the stream position before/after calling
readNext(), and use that consumed-value-length to update length while retaining
the existing tag check against ASN1Encoder.SEQUENCE.
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/AuthzClientCryptoProvider.java`:
- Around line 100-164: Validate inputs and remove no-op ASN.1 writes: in
concatenatedRSToASN1DER ensure signLength is positive, even, and
signature.length >= signLength (throw IOException or IllegalArgumentException on
bad input) to avoid ArrayIndexOutOfBoundsException; use a single ASN1Encoder
instance (e.g., encoder = ASN1Encoder.create()) and write r and s once to build
the DER sequence instead of calling ASN1Encoder.create().write(...) repeatedly;
likewise, in asn1derToConcatenatedRS validate derEncodedSignatureValue is
non-null and signLength is positive/even and verify seq size==2 before reading
integers; keep integerToBytes as-is but rely on the input validation to
guarantee lengths.
- Around line 60-67: The method
AuthzClientCryptoProvider.getBouncyCastleProvider() currently returns the
provider from KeyStore.getDefaultType(), which is not guaranteed to be
BouncyCastle; change it to return a proper BouncyCastle provider instance (e.g.
instantiate or lookup org.bouncycastle.jce.provider.BouncyCastleProvider) and
ensure the provider is registered if necessary before returning; update
getBouncyCastleProvider() to either return Security.getProvider("BC") or new
BouncyCastleProvider() so callers receive a true BouncyCastle provider.
In `@common/src/main/java/org/keycloak/common/crypto/CryptoIntegration.java`:
- Around line 56-70: The selection currently picks the first entry from
foundProviders which can be nondeterministic when multiple CryptoProvider
implementations return the same highest order(); modify CryptoIntegration to
detect ties by computing the max order (from foundProviders) and collecting all
providers whose order() == maxOrder; if more than one provider matches, fail
fast by throwing an IllegalStateException (include the conflicting provider
class names) instead of silently ignoring them; otherwise continue and use the
single top provider (foundProviders.get(0)) and keep the existing debug logging.
🧹 Nitpick comments (3)
authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Encoder.java (1)
85-91: Unnecessarythrows IOExceptiondeclarations.
ByteArrayOutputStream.write()methods never throwIOException. Thethrows IOExceptionon these methods and their callers is superfluous, though it doesn't cause runtime issues.♻️ Optional: Remove unnecessary throws declarations
- void write(byte[] bytes) throws IOException { + void write(byte[] bytes) { os.write(bytes); } - void write(int b) throws IOException { + void write(int b) { os.write(b); }This would require propagating the change to
writeEncoded,writeLength, and potentially public methods. Since consistency withASN1Decoder(which does needIOException) may be desired, this is optional.authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Decoder.java (1)
184-193:mark()resetscountunconditionally, which may cause issues if called multiple times.Setting
count = 0inmark()means the count no longer reflects total bytes read from the stream, only bytes since the last mark. This design choice works for the currentreadNext()usage but could be error-prone ifmark()is called in other contexts. Consider documenting this behavior or renaming to clarify intent.authz/client/src/test/java/org/keycloak/authorization/client/test/ECDSAAlgorithmTest.java (1)
39-56: Use curve-specific EC keys per algorithm for more reliable tests.Right now all algorithms share a default EC key pair, which may not match ES384/ES512 curve sizes. Generating curve-specific keys makes the test deterministic across providers and ensures signature lengths align with the algorithm.
🔧 Suggested update (per‑algorithm key pair)
- private final KeyPair keyPair; - - public ECDSAAlgorithmTest() throws Exception { - keyPair = KeyPairGenerator.getInstance("EC").genKeyPair(); - } + private KeyPair generateKeyPair(ECDSAAlgorithm algorithm) throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC"); + switch (algorithm) { + case ES256: + kpg.initialize(new java.security.spec.ECGenParameterSpec("secp256r1")); + break; + case ES384: + kpg.initialize(new java.security.spec.ECGenParameterSpec("secp384r1")); + break; + case ES512: + kpg.initialize(new java.security.spec.ECGenParameterSpec("secp521r1")); + break; + default: + throw new IllegalArgumentException("Unsupported algorithm " + algorithm); + } + return kpg.generateKeyPair(); + } private void test(ECDSAAlgorithm algorithm) throws Exception { + KeyPair keyPair = generateKeyPair(algorithm); AuthzClientCryptoProvider prov = new AuthzClientCryptoProvider(); byte[] data = "Something to sign".getBytes(StandardCharsets.UTF_8);
Comment on lines
+49
to
+63
| public List<byte[]> readSequence() throws IOException { | ||
| int tag = readTag(); | ||
| int tagNo = readTagNumber(tag); | ||
| if (tagNo != ASN1Encoder.SEQUENCE) { | ||
| throw new IOException("Invalid Sequence tag " + tagNo); | ||
| } | ||
| int length = readLength(); | ||
| List<byte[]> result = new ArrayList<>(); | ||
| while (length > 0) { | ||
| byte[] bytes = readNext(); | ||
| result.add(bytes); | ||
| length = length - bytes.length; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Potential length mismatch in readSequence() loop.
The length variable holds the content length of the SEQUENCE (excluding tag and length bytes). However, readNext() returns the complete TLV (tag + length + value) bytes. Subtracting bytes.length from length will result in incorrect tracking, potentially causing the loop to terminate early or continue past the sequence boundary.
🐛 Proposed fix
Track the bytes consumed during readNext() separately, or compute only the value portion length:
public List<byte[]> readSequence() throws IOException {
int tag = readTag();
int tagNo = readTagNumber(tag);
if (tagNo != ASN1Encoder.SEQUENCE) {
throw new IOException("Invalid Sequence tag " + tagNo);
}
int length = readLength();
+ int startCount = count;
List<byte[]> result = new ArrayList<>();
- while (length > 0) {
+ while (count - startCount < length) {
byte[] bytes = readNext();
result.add(bytes);
- length = length - bytes.length;
}
return result;
}Alternatively, track consumed bytes within the loop by noting count before and after readNext().
🤖 Prompt for AI Agents
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Decoder.java`
around lines 49 - 63, In readSequence(), the loop subtracts the full TLV byte
array returned by readNext() from the SEQUENCE content length, causing a
mismatch; change the loop in ASN1Decoder.readSequence to decrement length by
only the value (V) portion length consumed (not the entire TLV), e.g. obtain the
value length from the TLV metadata or by querying the length parsed by
readNext()/readLength() or by measuring the stream position before/after calling
readNext(), and use that consumed-value-length to update length while retaining
the existing tag check against ASN1Encoder.SEQUENCE.
Comment on lines
+156
to
+163
| if (length < 0) { | ||
| throw new IOException("corrupted stream - negative length found"); | ||
| } | ||
|
|
||
| if (length >= limit) // after all we must have read at least 1 byte | ||
| { | ||
| throw new IOException("corrupted stream - out of bounds length found"); | ||
| } |
There was a problem hiding this comment.
Bounds check may be incorrect for partially-read streams.
The check length >= limit uses limit (total input size) but doesn't account for bytes already consumed (count). A length value that exceeds remaining bytes should be rejected.
🛡️ Suggested improvement
- if (length >= limit) // after all we must have read at least 1 byte
+ if (length > limit - count) // check against remaining bytes
{
throw new IOException("corrupted stream - out of bounds length found");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (length < 0) { | |
| throw new IOException("corrupted stream - negative length found"); | |
| } | |
| if (length >= limit) // after all we must have read at least 1 byte | |
| { | |
| throw new IOException("corrupted stream - out of bounds length found"); | |
| } | |
| if (length < 0) { | |
| throw new IOException("corrupted stream - negative length found"); | |
| } | |
| if (length > limit - count) // check against remaining bytes | |
| { | |
| throw new IOException("corrupted stream - out of bounds length found"); | |
| } |
🤖 Prompt for AI Agents
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Decoder.java`
around lines 156 - 163, The bounds check in ASN1Decoder that currently uses "if
(length >= limit)" is incorrect for partially-read streams because it ignores
bytes already consumed (count); change it to verify the requested length does
not exceed the remaining bytes by using "if (length > (limit - count))" (or
equivalent) so that length larger than the remaining buffer throws, while valid
lengths that fit the remaining bytes are allowed.
Comment on lines
+60
to
+67
| @Override | ||
| public Provider getBouncyCastleProvider() { | ||
| try { | ||
| return KeyStore.getInstance(KeyStore.getDefaultType()).getProvider(); | ||
| } catch (KeyStoreException e) { | ||
| throw new IllegalStateException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/AuthzClientCryptoProvider.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 8759
🏁 Script executed:
find . -name "CryptoProvider.java" -type fRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 156
🏁 Script executed:
cat ./common/src/main/java/org/keycloak/common/crypto/CryptoProvider.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 5116
🏁 Script executed:
find . -name "BouncyIntegration.java" -type fRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 157
🏁 Script executed:
cat ./common/src/main/java/org/keycloak/common/util/BouncyIntegration.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 1518
🏁 Script executed:
rg "implements CryptoProvider" -A 10Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 6300
🏁 Script executed:
sed -n '1,100p' crypto/default/src/main/java/org/keycloak/crypto/def/DefaultCryptoProvider.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 4275
🏁 Script executed:
rg "BC_PROVIDER_ID|BCFIPS_PROVIDER_ID" -B 2 -A 2Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 5088
🏁 Script executed:
rg "getBouncyCastleProvider" authz/clientRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 238
🏁 Script executed:
rg "getBouncyCastleProvider()" --type javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 888
🏁 Script executed:
rg "public Provider getBouncyCastleProvider" -A 5Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 2774
Correct the provider returned to match the BouncyCastle contract.
The interface specifies that getBouncyCastleProvider() must return a BouncyCastle provider, but this implementation returns the provider for the default KeyStore type (typically SUN). This violates the contract and will cause crypto operations to be routed to an unexpected provider.
🔧 Suggested fix
+import java.security.Security;
+import org.keycloak.common.util.BouncyIntegration;
...
public Provider getBouncyCastleProvider() {
- try {
- return KeyStore.getInstance(KeyStore.getDefaultType()).getProvider();
- } catch (KeyStoreException e) {
- throw new IllegalStateException(e);
- }
+ Provider bc = Security.getProvider(BouncyIntegration.PROVIDER);
+ if (bc == null) {
+ throw new IllegalStateException("BouncyCastle provider not available");
+ }
+ return bc;
}🤖 Prompt for AI Agents
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/AuthzClientCryptoProvider.java`
around lines 60 - 67, The method
AuthzClientCryptoProvider.getBouncyCastleProvider() currently returns the
provider from KeyStore.getDefaultType(), which is not guaranteed to be
BouncyCastle; change it to return a proper BouncyCastle provider instance (e.g.
instantiate or lookup org.bouncycastle.jce.provider.BouncyCastleProvider) and
ensure the provider is registered if necessary before returning; update
getBouncyCastleProvider() to either return Security.getProvider("BC") or new
BouncyCastleProvider() so callers receive a true BouncyCastle provider.
Comment on lines
+100
to
+164
| public ECDSACryptoProvider getEcdsaCryptoProvider() { | ||
| return new ECDSACryptoProvider() { | ||
| @Override | ||
| public byte[] concatenatedRSToASN1DER(byte[] signature, int signLength) throws IOException { | ||
| int len = signLength / 2; | ||
| int arraySize = len + 1; | ||
|
|
||
| byte[] r = new byte[arraySize]; | ||
| byte[] s = new byte[arraySize]; | ||
| System.arraycopy(signature, 0, r, 1, len); | ||
| System.arraycopy(signature, len, s, 1, len); | ||
| BigInteger rBigInteger = new BigInteger(r); | ||
| BigInteger sBigInteger = new BigInteger(s); | ||
|
|
||
| ASN1Encoder.create().write(rBigInteger); | ||
| ASN1Encoder.create().write(sBigInteger); | ||
|
|
||
| return ASN1Encoder.create() | ||
| .writeDerSeq( | ||
| ASN1Encoder.create().write(rBigInteger), | ||
| ASN1Encoder.create().write(sBigInteger)) | ||
| .toByteArray(); | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] asn1derToConcatenatedRS(byte[] derEncodedSignatureValue, int signLength) throws IOException { | ||
| int len = signLength / 2; | ||
|
|
||
| List<byte[]> seq = ASN1Decoder.create(derEncodedSignatureValue).readSequence(); | ||
| if (seq.size() != 2) { | ||
| throw new IOException("Invalid sequence with size different to 2"); | ||
| } | ||
|
|
||
| BigInteger rBigInteger = ASN1Decoder.create(seq.get(0)).readInteger(); | ||
| BigInteger sBigInteger = ASN1Decoder.create(seq.get(1)).readInteger(); | ||
|
|
||
| byte[] r = integerToBytes(rBigInteger, len); | ||
| byte[] s = integerToBytes(sBigInteger, len); | ||
|
|
||
| byte[] concatenatedSignatureValue = new byte[signLength]; | ||
| System.arraycopy(r, 0, concatenatedSignatureValue, 0, len); | ||
| System.arraycopy(s, 0, concatenatedSignatureValue, len, len); | ||
|
|
||
| return concatenatedSignatureValue; | ||
| } | ||
|
|
||
| @Override | ||
| public ECPublicKey getPublicFromPrivate(ECPrivateKey ecPrivateKey) { | ||
| throw new UnsupportedOperationException("Not supported yet."); | ||
| } | ||
|
|
||
| private byte[] integerToBytes(BigInteger s, int qLength) { | ||
| byte[] bytes = s.toByteArray(); | ||
| if (qLength < bytes.length) { | ||
| byte[] tmp = new byte[qLength]; | ||
| System.arraycopy(bytes, bytes.length - tmp.length, tmp, 0, tmp.length); | ||
| return tmp; | ||
| } else if (qLength > bytes.length) { | ||
| byte[] tmp = new byte[qLength]; | ||
| System.arraycopy(bytes, 0, tmp, tmp.length - bytes.length, bytes.length); | ||
| return tmp; | ||
| } | ||
| return bytes; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add input validation and remove no-op ASN.1 writes.
concatenatedRSToASN1DER will throw ArrayIndexOutOfBoundsException if signature.length or signLength is unexpected. Also, the two ASN1Encoder.create().write(...) calls are unused.
🔧 Suggested fix (validation + cleanup)
public byte[] concatenatedRSToASN1DER(byte[] signature, int signLength) throws IOException {
+ if (signLength <= 0 || (signLength % 2) != 0) {
+ throw new IOException("Invalid signature length: " + signLength);
+ }
+ if (signature.length != signLength) {
+ throw new IOException("Expected signature length " + signLength + " but got " + signature.length);
+ }
int len = signLength / 2;
int arraySize = len + 1;
byte[] r = new byte[arraySize];
byte[] s = new byte[arraySize];
System.arraycopy(signature, 0, r, 1, len);
System.arraycopy(signature, len, s, 1, len);
BigInteger rBigInteger = new BigInteger(r);
BigInteger sBigInteger = new BigInteger(s);
-
- ASN1Encoder.create().write(rBigInteger);
- ASN1Encoder.create().write(sBigInteger);
return ASN1Encoder.create()
.writeDerSeq(
ASN1Encoder.create().write(rBigInteger),
ASN1Encoder.create().write(sBigInteger))🤖 Prompt for AI Agents
In
`@authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/AuthzClientCryptoProvider.java`
around lines 100 - 164, Validate inputs and remove no-op ASN.1 writes: in
concatenatedRSToASN1DER ensure signLength is positive, even, and
signature.length >= signLength (throw IOException or IllegalArgumentException on
bad input) to avoid ArrayIndexOutOfBoundsException; use a single ASN1Encoder
instance (e.g., encoder = ASN1Encoder.create()) and write r and s once to build
the DER sequence instead of calling ASN1Encoder.create().write(...) repeatedly;
likewise, in asn1derToConcatenatedRS validate derEncodedSignatureValue is
non-null and signLength is positive/even and verify seq size==2 before reading
integers; keep integerToBytes as-is but rely on the input validation to
guarantee lengths.
Comment on lines
56
to
+70
| List<CryptoProvider> foundProviders = StreamSupport.stream(ServiceLoader.load(CryptoProvider.class, classLoader).spliterator(), false) | ||
| .sorted(Comparator.comparingInt(CryptoProvider::order).reversed()) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| if (foundProviders.isEmpty()) { | ||
| throw new IllegalStateException("Not able to load any cryptoProvider with the classLoader: " + classLoader); | ||
| } else if (foundProviders.size() > 1) { | ||
| throw new IllegalStateException("Multiple crypto providers loaded with the classLoader: " + classLoader + | ||
| ". Make sure only one cryptoProvider available on the classpath. Available providers: " +foundProviders); | ||
| } else { | ||
| logger.debugf("Detected crypto provider: %s", foundProviders.get(0).getClass().getName()); | ||
| if (foundProviders.size() > 1) { | ||
| StringBuilder builder = new StringBuilder("Ignored crypto providers: "); | ||
| for (int i = 1 ; i < foundProviders.size() ; i++) { | ||
| builder.append(foundProviders.get(i).getClass().getName() + ", "); | ||
| } | ||
| logger.debugf(builder.toString()); | ||
| } |
There was a problem hiding this comment.
Guard against ties in order() to avoid nondeterministic provider selection.
Right now, multiple providers with the same highest order (e.g., several return 200) will be chosen based on ServiceLoader iteration order, and the others are silently ignored. That can lead to unintended provider selection (including potential compliance/security drift). Consider enforcing a deterministic tie-breaker or failing fast when the top order is ambiguous.
🔧 Suggested fix (deterministic tie check)
- List<CryptoProvider> foundProviders = StreamSupport.stream(ServiceLoader.load(CryptoProvider.class, classLoader).spliterator(), false)
- .sorted(Comparator.comparingInt(CryptoProvider::order).reversed())
- .collect(Collectors.toList());
+ List<CryptoProvider> foundProviders = StreamSupport.stream(ServiceLoader.load(CryptoProvider.class, classLoader).spliterator(), false)
+ .sorted(Comparator.comparingInt(CryptoProvider::order).reversed()
+ .thenComparing(p -> p.getClass().getName()))
+ .collect(Collectors.toList());
+
+ int topOrder = foundProviders.get(0).order();
+ List<CryptoProvider> topProviders = foundProviders.stream()
+ .filter(p -> p.order() == topOrder)
+ .collect(Collectors.toList());
+ if (topProviders.size() > 1) {
+ throw new IllegalStateException("Multiple crypto providers share the same order: " + topOrder + " -> " +
+ topProviders.stream().map(p -> p.getClass().getName()).collect(Collectors.joining(", ")));
+ }🤖 Prompt for AI Agents
In `@common/src/main/java/org/keycloak/common/crypto/CryptoIntegration.java`
around lines 56 - 70, The selection currently picks the first entry from
foundProviders which can be nondeterministic when multiple CryptoProvider
implementations return the same highest order(); modify CryptoIntegration to
detect ties by computing the max order (from foundProviders) and collecting all
providers whose order() == maxOrder; if more than one provider matches, fail
fast by throwing an IllegalStateException (include the conflicting provider
class names) instead of silently ignoring them; otherwise continue and use the
single top provider (foundProviders.get(0)) and keep the existing debug logging.
Sign in
to join this conversation on GitHub.
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.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores