[DRAFT PR] Critical code review #206
Conversation
WalkthroughReplaces Boolean revocation checks with a credential-status architecture, adds LdpStatusChecker/StatusCheckException and constants, updates key resolution (PublicKeyResolverFactory) and signature handling (SignatureFactory), modernizes utilities (date/base64/verifyJwt), expands public APIs to return status-aware results for credentials and presentations, adds Java sample app, and extends tests and JSON‑LD fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant CredentialsVerifier
participant VerifierFactory
participant VerifiableCredential
participant LdpStatusChecker
participant NetworkManager
participant SignatureVerifier
Caller->>CredentialsVerifier: verifyAndGetCredentialStatus(vc, format, purposes)
CredentialsVerifier->>VerifierFactory: getVerifier(format)
VerifierFactory-->>CredentialsVerifier: VerifiableCredential
CredentialsVerifier->>VerifiableCredential: verify(vc)
alt verification succeeds
VerifiableCredential-->>CredentialsVerifier: VerificationResult(SUCCESS)
CredentialsVerifier->>LdpStatusChecker: getStatuses(vc, purposes)
LdpStatusChecker->>NetworkManager: HTTP GET statusListCredential
NetworkManager-->>LdpStatusChecker: status-list VC
LdpStatusChecker->>SignatureVerifier: verify(status-list VC)
SignatureVerifier-->>LdpStatusChecker: valid / invalid
alt signature valid
LdpStatusChecker->>LdpStatusChecker: decode Base64 + GZIP, extract bit
LdpStatusChecker-->>CredentialsVerifier: Map<purpose, CredentialStatusResult>
CredentialsVerifier-->>Caller: CredentialVerificationSummary(success, statuses)
else signature invalid / error
LdpStatusChecker-->>CredentialsVerifier: StatusCheckException
CredentialsVerifier-->>Caller: CredentialVerificationSummary(success, error-statuses)
end
else verification fails
VerifiableCredential-->>CredentialsVerifier: VerificationResult(FAILED)
CredentialsVerifier-->>Caller: CredentialVerificationSummary(failed, {})
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.kt (1)
102-107: Add ES256 to ALGORITHMS_SUPPORTED list.The
ALGORITHMS_SUPPORTEDlist omitsES256, which is fully supported throughout the codebase. Evidence showsJWS_ES256_SIGN_ALGO_CONST = "ES256"is defined, actively imported, and handled in signature verification (Util.kt, SignatureFactory.kt, keyResolver/Utils.kt). Additionally, consider adding the corresponding proof type (EcdsaSecp256r1Signature2019) to thePROOF_TYPES_SUPPORTEDlist for consistency, as it's currently also missing.
🧹 Nitpick comments (29)
vc-verifier/kotlin/vcverifier/build.gradle.kts (2)
118-119: Fix typo in jarRelease task description.The description contains a spelling error: "Assemple" should be "Assemble".
tasks.register<Jar>("jarRelease") { group = "release" - description = "Assemple Jar Release" + description = "Assemble Jar Release"
157-158: Fix typo in generatePom task description.The description contains a spelling error: "Geneate" should be "Generate".
tasks.register("generatePom") { group = "release" - description = "Geneate POM file for AAR and JAR Release" + description = "Generate POM file for AAR and JAR Release"vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/DateUtils.kt (1)
37-49: Consider logging the swallowed exception for debugging.The outer catch block swallows the exception when
OffsetDateTime.parsefails, which is intentional for the fallback mechanism. However, this can make debugging parsing issues difficult in production.Consider logging the exception at debug level:
fun parseDate(dateString: String): Date? { return try { val offsetDateTime = OffsetDateTime.parse(dateString, formatterWithOffset) Date(offsetDateTime.toInstant().toEpochMilli()) - } catch (e: Exception) { + } catch (e: Exception) { + logger.fine("Failed to parse as offset date-time, trying local date-time: ${e.message}") try { val localDateTime = LocalDateTime.parse(dateString, formatterLocal) Date(localDateTime.toInstant(ZoneOffset.UTC).toEpochMilli()) } catch (_: Exception) { null } } }vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/DateUtilsTest.kt (1)
1-134: Consider adding tests for the newformatEpochSecondsToIsoUtcmethod.The test suite provides excellent coverage for
parseDateandisValidDate. However, the newformatEpochSecondsToIsoUtcmethod (added in DateUtils.kt lines 115-118) is not tested here.Consider adding test cases like:
@Test fun testFormatEpochSecondsToIsoUtc() { val formatted = DateUtils.formatEpochSecondsToIsoUtc(1758104130L) assertEquals("2025-09-17T10:15:30Z", formatted) } @Test fun testFormatEpochZero() { val formatted = DateUtils.formatEpochSecondsToIsoUtc(0L) assertEquals("1970-01-01T00:00:00Z", formatted) } @Test fun testFormatNegativeEpoch() { val formatted = DateUtils.formatEpochSecondsToIsoUtc(-86400L) assertEquals("1969-12-31T00:00:00Z", formatted) }vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/Main.java (1)
12-13: Remove unnecessary blank lines.These empty lines serve no purpose and reduce code readability.
public static void main(String[] args) { - - System.out.println("Verifying VC from jar");vc-verifier/kotlin/sample-java-app/pom.xml (2)
36-47: Remove duplicate Google Maven repository.The Google Maven repository is declared twice with different IDs but the same URL. This is redundant and may cause confusion.
- <repository> - <id>google-maven</id> - <url>https://maven.google.com/</url> - </repository> <repository> <id>danubetech-maven-public</id> <url>https://repo.danubetech.com/repository/maven-public/</url> </repository> <repository> <id>google</id> <url>https://maven.google.com</url> </repository>
51-55: Consider using a stable release version instead of SNAPSHOT.Using a SNAPSHOT version (1.5.0-SNAPSHOT) in a sample application can lead to non-reproducible builds and unexpected behavior as SNAPSHOT artifacts can change without version updates. Consider using a stable release version once available to ensure consistent behavior for users trying the sample.
vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/SampleConstants.java (1)
11-26: Use obviously fake data in sample credentials.The credential contains realistic-looking personal information (email: "ramesh@mosip.io", mobile: "9840298402"). For sample/demo code, it's better practice to use obviously fake data (e.g., "example@example.com", "555-0100") to avoid any potential PII concerns or confusion.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.json (1)
1-25: Consider adding newline at end of file.The test resource file is missing a newline at the end, which is a common text file convention that helps with certain tools and version control systems.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json (1)
1-94: Consider adding newline at end of file.Similar to the other test resource files, this file is missing a newline at the end, which is a common text file convention.
vc-verifier/kotlin/README.md (1)
25-25: Align markdown headings with markdownlint hintsThe bolded lines used as titles (
**ldp_vc Error Codes**,** mso_mdoc Format VC Error Codes **,** vc+sd-jwt/dc+sd-jwt Format VC Error Codes **) and the##### Verification Result on Success/Failureheadings trigger markdownlint (MD036/MD037/MD001). Consider:
- Turning these into proper headings, e.g.
#### ldp_vc Error Codes,#### mso_mdoc Format VC Error Codes,#### vc+sd-jwt/dc+sd-jwt Format VC Error Codes.- Bumping the “Verification Result …” sections to
####so heading levels increase by one at a time.This keeps docs consistent and silences the lints.
Also applies to: 69-69, 78-78, 126-126
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (2)
3-9: Avoid exposing StatusCheckException directly in CredentialStatusResult
CredentialStatusResultcarries aStatusCheckException?field. Putting exception instances into a public data model:
- Couples your API surface tightly to the exception type.
- Risks leaking internal messages/stack details if this model is serialized or logged as‑is.
- Makes it harder to evolve the exception hierarchy later.
Consider replacing
error: StatusCheckException?with a lightweight structure (e.g.errorCode: StatusCheckErrorCode?plus optionalerrorMessage: String?), and keep the actual exception confined to logging.Also applies to: 47-52
23-28: Document REVOKED as a possible VCResult.status value
VerificationStatusnow includesREVOKED, but the README’svcResultsexample still documentsstatus: "valid|invalid|expired". For consistency with this enum and the new status‑list flow, consider updating the docs to mentionrevoked(or whatever external string you map it to) as an additional possible status.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (1)
53-116: Document getStatuses() contract for null vs error results
getStatusescurrently:
- Returns
nullifcredentialStatusis missing, or if no entries matchstatusPurposes.- Returns a list (possibly containing only failure entries) when entries exist but validation or downstream checks fail, with per‑purpose errors captured in
CredentialStatusResult.error.This is a reasonable contract, but it’s non‑obvious to callers. Consider tightening the KDoc to spell out these cases explicitly so integrators know when
nullmeans “no relevant status to check” vs when to inspect the returned list for failures.vc-verifier/kotlin/vcverifier/src/test/resources/contexts/school-context.json (1)
1-19: Consider more precise schema.org mappings.While this test context is functional, consider these refinements for better semantic accuracy:
- Line 12:
"amenities": "schema:benefits"→ Consider"schema:amenityFeature"which is more semantically appropriate for school amenities- Lines 13, 15, 17: Fields mapped to generic
"schema:Text"could use more specific schema.org terms if availablevc-verifier/kotlin/vcverifier/src/test/resources/contexts/farmer-context.json (2)
15-16: Duplicate schema.org mapping may cause ambiguity.Both
"district"and"villageOrTown"are mapped to"schema:addressLocality", which could lead to semantic confusion when processing this JSON-LD context. Consider using more specific or distinct schema.org terms to differentiate these geographic levels.
18-23: Consider more specific schema.org terms.Several fields are mapped to generic
"schema:Text"or"schema:Number". For better semantic precision in tests, consider using more specific schema.org terms where available (e.g.,"schema:QuantitativeValue"for measurements, domain-specific properties for agriculture-related fields).vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.json (1)
4-4: Consider using distinct values for UIN and VID.Both
UINandVIDare set to123456. Using distinct values would improve test coverage and better represent real-world scenarios where these identifiers differ.Also applies to: 21-21
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifier.kt (1)
46-46: Consider caching factory instances.New factory instances are created on each verification call (
PublicKeyResolverFactory()on line 46 andSignatureFactory()on line 53). If these factories are stateless, consider making them class-level constants or using a singleton pattern to avoid repeated instantiation overhead.Example approach:
class LdpVerifier { private val logger = Logger.getLogger(LdpVerifier::class.java.name) private var provider: BouncyCastleProvider = BouncyCastleProvider() private val publicKeyResolverFactory = PublicKeyResolverFactory() private val signatureFactory = SignatureFactory() // ... rest of class fun verify(credential: String): Boolean { // ... val publicKeyObj = publicKeyResolverFactory.get(verificationMethod) // ... val signatureVerifier = signatureFactory.get(jwsObject.header.algorithm.name) // ... } }Also applies to: 53-53
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SdJwtValidator.kt (1)
358-386: Minor: Redundant null check.Lines 374 and 383 use
isNullOrBlank()on strings obtained viaoptString(), butoptString()never returns null—it returns an empty string when the field is missing. UseisBlank()orisEmpty()instead for clarity.Apply this diff:
- if (alg.isNullOrBlank()) { + if (alg.isBlank()) { throw ValidationException( "Missing 'alg' in Key Binding JWT header","${ERROR_CODE_MISSING}KB_JWT_ALG") }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt (1)
14-14: checkStatus no-op for MSO mdoc – document intent
checkStatusreturningnullmeans MSO mdoc credentials effectively bypass status checks, similar to the SdJwt behavior. That’s fine if MSO mdoc status is intentionally unsupported, but it would help future readers if this were documented (e.g., KDoc or a brief comment) and, if needed,statusPurposeswas annotated/suppressed to avoid unused-parameter warnings.You might also want to double‑check that
CredentialsVerifier.verifyAndGetCredentialStatusconsistently interpretsnullas “no status capability” rather than an error for this format. Based on learnings.Also applies to: 47-49
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (2)
3-18: Static mock of NetworkManagerClient is never resetBoth status‑list tests use
mockkObject(NetworkManagerClient.Companion)and stubsendHTTPRequest, but there’s no corresponding teardown. Because this is a static/object mock, the stubbed behavior can bleed into other tests that happen to callNetworkManagerClient.sendHTTPRequest, causing subtle flakiness.Consider adding an
@AfterEach(or similar) that callsunmockkObject(NetworkManagerClient.Companion)or using MockK’s JUnit 5 extension plusclearAllMocks()to keep the mock scoped to each test run.Also applies to: 179-225
152-166: Strengthen status-flow tests by asserting overall verificationResult semanticsThe new tests nicely cover:
- empty status list when verification fails, and
- status entry contents for unrevoked vs revoked VCs.
Right now, the unrevoked/revoked tests only assert fields on
CredentialStatusResult. To lock in the intended contract ofCredentialVerificationSummary, consider also asserting the expectedresult.verificationResult.verificationStatusfor both cases (e.g., whether a revoked status should flip the overall verification to false or remain true but withvalid=falsein the status entry).That will make future changes to the verification/status aggregation behavior explicit and easier to reason about.
Also applies to: 188-202, 226-240
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (2)
56-75: Check whether you want to preserve VC expiry information when signatures also failRight now, if validation marks the VC as expired you only surface
ERR_VC_EXPIREDwhenverifySignatureStatusis true; if the signature fails you instead return a genericERROR_CODE_VERIFICATION_FAILED, losing the expiry signal. If expiry should remain visible even when the signature is bad (e.g. to distinguish “expired” from “tampered”), consider prioritizing the expiry error code or encoding both in the result. Otherwise, this behavior is fine but should be intentional.
78-92: Clarify semantics ofgetCredentialStatuswhencheckStatusreturns null
checkStatuscan legitimately returnnullto indicate that status checks are not performed for a given format (e.g., SD‑JWT currently bypasses status checks). MappingnulltoemptyList()makes callers unable to distinguish “status not supported” from “no status entries found”:val credentialStatusArray = credentialVerifierFactory.get(credentialFormat) .checkStatus(credential, statusPurposeList) return credentialStatusArray ?: emptyList()If that distinction matters, you may want to propagate
null(and reflect that in the API) or add a separate flag inCredentialVerificationSummaryindicating whether status checking was actually attempted. Based on learningsvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidJwkPublicKeyResolverTest.kt (1)
114-138: RSA JWK resolution test looks good; minor naming nitThe RSA positive‑path test correctly verifies that an RSA JWK can be resolved and that the resulting public key has algorithm
"RSA". That aligns well with the expanded key‑type support.For clarity, consider renaming:
val unsupportedKeyTypeDid = "did:jwk:${encodeBase64Url(jwk)}"to something like
rsaDidorrsaJwkDid, since this test is specifically the supported RSA case.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
34-55: Consider caching the defaultConfigurableDocumentLoaderindocumentLoader
getConfigurableDocumentLoadercurrently builds a new loader wheneverdocumentLoaderis null:fun getConfigurableDocumentLoader(): ConfigurableDocumentLoader { return documentLoader ?: run { val loader = ConfigurableDocumentLoader() loader.isEnableHttps = true loader.isEnableHttp = true loader.isEnableFile = false loader } }If the intent of the
documentLoaderfield is to allow both test injection and reuse of a default loader, you can cache the created instance:fun getConfigurableDocumentLoader(): ConfigurableDocumentLoader { - return documentLoader ?: run { - val loader = ConfigurableDocumentLoader() - loader.isEnableHttps = true - loader.isEnableHttp = true - loader.isEnableFile = false - loader - } + return documentLoader ?: run { + val loader = ConfigurableDocumentLoader().apply { + isEnableHttps = true + isEnableHttp = true + isEnableFile = false + } + documentLoader = loader + loader + } }This keeps the API flexible while avoiding repeated loader construction.
135-149: Preserve root cause when throwingSignatureVerificationExceptioninverifyJwt
verifyJwtwraps any exception in a newSignatureVerificationException:return try { signatureVerifier.verify(publicKey, signedData.toByteArray(UTF_8), signatureBytes) } catch (e: Exception) { throw SignatureVerificationException("Signature verification failed: ${e.message}") }This normalizes errors but drops the original cause, which can be useful when debugging crypto or parsing issues. Consider chaining the cause:
- } catch (e: Exception) { - throw SignatureVerificationException("Signature verification failed: ${e.message}") - } + } catch (e: Exception) { + throw SignatureVerificationException( + "Signature verification failed: ${e.message}", + e + ) + }(Adjust constructor signature if your exception type doesn’t yet accept a cause.)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (1)
41-47: MockK setup is fine, but manual initialization is redundantWith
@ExtendWith(MockKExtension::class)and@MockKfields, the extension already initializes mocks before each test. CallingMockKAnnotations.init(this)insetup()plusunmockkAll()inteardown()works but is more than you need. You can simplify to rely solely on the extension (and keepunmockkAll()if you prefer a hard reset):@ExtendWith(MockKExtension::class) class StatusListRevocationCheckerTest { @BeforeEach fun setup() { every { mockVerifierFactory.get(CredentialFormat.LDP_VC) } returns mockVerifier every { mockVerifier.verify(any()) } returns true checker = LdpStatusChecker(mockVerifierFactory) } }This reduces boilerplate without changing behavior.
Also applies to: 49-52
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
README.md(3 hunks)doc/sdjwt-kbjwt-verification-support.md(1 hunks)vc-verifier/kotlin/README.md(5 hunks)vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.kt(2 hunks)vc-verifier/kotlin/gradle/libs.versions.toml(2 hunks)vc-verifier/kotlin/sample-java-app/pom.xml(1 hunks)vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/Main.java(1 hunks)vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/SampleConstants.java(1 hunks)vc-verifier/kotlin/vcverifier/build.gradle.kts(3 hunks)vc-verifier/kotlin/vcverifier/publish-artifact.gradle(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialValidatorConstants.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.kt(0 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.kt(0 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/LdpValidator.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SdJwtValidator.kt(11 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifier.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifier.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifier.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.kt(0 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/PublicKeyResolverFactory.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/Utils.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidJwkPublicKeyResolver.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidWebPublicKeyResolver.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/http/HttpsPublicKeyResolver.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/networkManager/NetworkManagerClient.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/signature/SignatureFactory.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Base64Decoder.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/DateUtils.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/ValidationHelper.kt(5 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt(9 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.kt(0 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SDJwtValidatorTest.kt(19 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/PublicKeyResolverFactoryTest.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/UtilsKtTest.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidJwkPublicKeyResolverTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidWebPublicKeyResolverTest.kt(5 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/DateUtilsTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/ed25519-2020-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/farmer-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/insurance-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/jws2020-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/mosip-ida-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/odrl-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/school-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/w3vc1-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/w3vc2-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedStatusList.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedStatusList.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipUnrevokedVC.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/sd-jwt_vc/sdJwtWithKbJwtES256.txt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/sd-jwt_vc/sdJwtWithKbJwtEdDSA.txt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/sd-jwt_vc/sdJwtWithKbJwtEs256kAndCnfBeingJwk.txt(1 hunks)
💤 Files with no reviewable changes (4)
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/LdpRevokeChecker.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/MsoMdocRevokeChecker.kt
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/RevocationCheckException.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/revocation/StatusListRevocationCheckerTest.kt
👮 Files not reviewed due to content moderation or server errors (1)
- vc-verifier/kotlin/example/src/main/java/io/mosip/vccred/example/MainActivity.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
📚 Learning: 2025-11-03T12:07:06.442Z
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
Applied to files:
vc-verifier/kotlin/README.mdvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/StatusCheckerConstants.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/UtilsKtTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SdJwtValidator.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SDJwtValidatorTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
🧬 Code graph analysis (11)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/DateUtilsTest.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/DateUtils.kt (2)
parseDate(37-49)isValidDate(33-35)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidJwkPublicKeyResolver.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/Utils.kt (1)
jwkToPublicKey(214-225)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifier.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
verifyJwt(135-149)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidWebPublicKeyResolver.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/networkManager/NetworkManagerClient.kt (2)
sendHTTPRequest(12-60)sendHTTPRequest(14-58)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/UtilsKtTest.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/Utils.kt (1)
jwkToPublicKey(214-225)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/testHelpers/TestAssertions.kt (1)
assertPublicKey(7-9)
vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/Main.java (2)
vc-verifier/kotlin/sample-java-app/src/main/java/io/mosip/SampleConstants.java (1)
SampleConstants(3-44)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
getVerificationStatus(57-68)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SdJwtValidator.kt (2)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/DateUtils.kt (1)
formatEpochSecondsToIsoUtc(115-118)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/keyResolver/Utils.kt (1)
jwkToPublicKey(214-225)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
isValidHttpsUri(92-99)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (1)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (1)
isValidUri(79-90)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (4)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt (1)
verify(8-8)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifier.kt (1)
verify(32-76)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifier.kt (2)
verify(9-36)verify(11-15)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifier.kt (1)
verify(41-72)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidJwkPublicKeyResolverTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/keyResolver/types/did/DidKeyPublicKeyResolverTest.kt (1)
createParsedDid(78-83)
🪛 detekt (1.23.8)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SdJwtValidator.kt
[warning] 60-60: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/DateUtils.kt
[warning] 41-41: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt
[warning] 41-41: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 100-100: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🪛 Gitleaks (8.29.0)
vc-verifier/kotlin/vcverifier/src/test/resources/sd-jwt_vc/sdJwtWithKbJwtEdDSA.txt
[high] 1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
vc-verifier/kotlin/vcverifier/src/test/resources/sd-jwt_vc/sdJwtWithKbJwtEs256kAndCnfBeingJwk.txt
[high] 1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SDJwtValidatorTest.kt
[high] 395-395: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 550-550: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 markdownlint-cli2 (0.18.1)
vc-verifier/kotlin/README.md
25-25: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
69-69: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
69-69: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
78-78: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
78-78: Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
126-126: Heading levels should only increment by one level at a time
Expected: h4; Actual: h5
(MD001, heading-increment)
README.md
64-64: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
doc/sdjwt-kbjwt-verification-support.md
27-27: Inconsistent indentation for list items at the same level
Expected: 3; Actual: 4
(MD005, list-indent)
34-34: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
| - digest length check based on hash algo `sha-256` - 32 bytes, `sha-384` to 48 bytes, `sha-512` to 64 bytes | ||
| - Fetch Holder's Public Key from SD JWT Payload | ||
| - `cnf` is mandatory. Must be present and value MUST be either `jwk` or `kid` | ||
| - Verify Signature |
There was a problem hiding this comment.
Fix inconsistent list indentation.
The list item has 4 spaces of indentation instead of the expected 3, which is inconsistent with the other items at the same level.
Apply this diff:
- - Verify Signature
+ - Verify Signature📝 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.
| - Verify Signature | |
| - Verify Signature |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
27-27: Inconsistent indentation for list items at the same level
Expected: 3; Actual: 4
(MD005, list-indent)
🤖 Prompt for AI Agents
In doc/sdjwt-kbjwt-verification-support.md around line 27, the list item "Verify
Signature" is indented with 4 spaces instead of the expected 3; update that
line's leading spaces from 4 to 3 so its indentation matches the other sibling
list items at the same level.
|
|
||
|
|
||
|
|
||
| ### Sequence diagram - validate and verify `kb+jwt` |
There was a problem hiding this comment.
Remove extra space in heading.
The heading has multiple spaces between ### and the text. Markdown style guidelines recommend a single space.
Apply this diff:
-### Sequence diagram - validate and verify `kb+jwt`
+### Sequence diagram - validate and verify `kb+jwt`🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
34-34: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
🤖 Prompt for AI Agents
In doc/sdjwt-kbjwt-verification-support.md around line 34, the markdown heading
contains multiple spaces between the '###' and the text; edit that line to use a
single space after '###' so it reads "### Sequence diagram - validate and verify
`kb+jwt`" (remove the extra space characters).
| jsonldCommonJava = "1.8.0" | ||
| nimbusJoseJwt = "9.40" | ||
| springWeb = "5.0.20.RELEASE" | ||
| threetenbp = "1.7.0" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify threetenbp 1.7.0 compatibility and security.
The new threetenbp dependency aligns with the modernized date/time handling mentioned in the PR objectives. Ensure the chosen version 1.7.0 is stable, free from known vulnerabilities, and compatible with Android compileSdk=33 and Java 17 target.
Also applies to: 36-36
🌐 Web query:
Is threetenbp 1.7.0 compatible with Android SDK 33 and Java 17? Are there any known security issues?
💡 Result:
Short answer: Yes — threetenbp 1.7.0 runs on Java 17 and can be used in apps targeting Android API 33, but there are known security/fuzz issues in the v1.7.x line so you should upgrade to the latest 1.7.2.
Details & sources (short):
- Compatibility: threetenbp is a pure‑Java backport that “runs on Java SE 6 (or later)” (so it runs on Java 17) and is distributed as a normal Maven artifact you can include in an Android build; Android itself includes java.time from API 26 onward but threetenbp can still be used where you prefer the backport/behavior. [1][2]
- Security: OSS‑Fuzz / OSV records a security/fuzz finding that lists v1.7.0 (and v1.6.9/1.7.1) as affected (OSV-2024-171). Earlier CVE reports (CVE‑2024‑23081 / CVE‑2024‑23082) targeted older 1.6.8; Snyk/official site indicate newer 1.7.2 is the current non‑vulnerable release. Recommend updating to 1.7.2. [3][4][5]
If you want, I can:
- point to the exact Maven coordinates and the upgrade change log, or
- check your project (build.gradle / usage) and suggest whether you should use ThreeTenABP vs. the bundled java.time on Android.
Replace threetenbp 1.7.0 with 1.7.2 to address known security vulnerability.
threetenbp 1.7.0 is compatible with Java 17 and Android API 33, but contains a known security/fuzz vulnerability (OSV-2024-171). Upgrade to 1.7.2, the current non-vulnerable release.
Update gradle/libs.versions.toml line 11 and 36:
- Line 11:
threetenbp = "1.7.2" - Line 36:
threetenbp = { module = "org.threeten:threetenbp", version.ref = "threetenbp" }
🤖 Prompt for AI Agents
In vc-verifier/kotlin/gradle/libs.versions.toml around lines 11 and 36, the
project pins threetenbp to 1.7.0 which has a known security vulnerability;
update line 11 to set threetenbp = "1.7.2" and update line 36 to use the shared
reference by setting threetenbp = { module = "org.threeten:threetenbp",
version.ref = "threetenbp" } so the dependency is upgraded to the non-vulnerable
1.7.2 and both declarations remain consistent.
vc-verifier/kotlin/README.md
Outdated
| | Field | Error Code | | ||
| |---------------------------|------------------------------------| | ||
| | credential empty | ERR_EMPTY_VC | | ||
| | JWT | ERR_INVALID_JWT_FORMAT | | ||
| | alg in header | ERR_INVALID_ALG | | ||
| | typ in header | ERR_INVALID_TYP | | ||
| | vct in payload | ERR_INVALID_VCT | | ||
| | vct in payload | ERR_INVALID_VCT_URI | | ||
| | iss in payload | ERR_INVALID_ISS | | ||
| | _sd_alg in payload | ERR_INVALID_SD_ALG | | ||
| | iat in payload | ERR_ISSUANCE_DATE_IS_FUTURE_DATE | | ||
| | nbf in payload | ERR_PROCESSING_DATE_IS_FUTURE_DATE | | ||
| | exp in payload | ERR_VC_EXPIRED | | ||
| | aud in payload | ERR_INVALID_AUD | | ||
| | cnf in payload | ERR_INVALID_CNF | | ||
| | disclosure format | ERR_INVALID_DISCLOSURE_FORMAT | | ||
| | disclosure json length | ERR_INVALID_DISCLOSURE_STRUCTURE | | ||
| | disclosure claim | ERR_INVALID_DISCLOSURE_CLAIM_NAME | | ||
| | disclosure hash length | ERR_INVALID_ALG | | ||
| | disclosure digest empty | ERR_INVALID_DIGEST | | ||
| | KB JWT | ERR_INVALID_KB_JWT_FORMAT | | ||
| | KB JWT header | ERR_INVALID_KB_JWT_HEADER | | ||
| | KB JWT header | ERR_INVALID_KB_JWT_HEADER | | ||
| | alg in KB JWT header | ERR_MISSING_KB_JWT_ALG | | ||
| | alg in KB JWT header | ERR_INVALID_KB_JWT_ALG | | ||
| | typ in KB JWT header | ERR_INVALID_KB_JWT_TYP | | ||
| | cnf in KB JWT header | ERR_INVALID_CNF | | ||
| | cnf in KB JWT header | ERR_INVALID_CNF_TYPE | | ||
| | cnf in KB JWT header | ERR_INVALID_KB_SIGNATURE | | ||
| | aud in KB JWT payload | ERR_MISSING_AUD | | ||
| | nonce in KB JWT payload | ERR_MISSING_NONCE | | ||
| | sd_hash in KB JWT payload | ERR_MISSING_SD_HASH | | ||
| | iat in KB JWT payload | ERR_MISSING_IAT | | ||
| | aud in KB JWT payload | ERR_INVALID_AUD | | ||
| | nonce in KB JWT payload | ERR_INVALID_NONCE | | ||
| | iat in KB JWT payload | ERR_INVALID_KB_JWT_IAT | | ||
| | sd_hash in KB JWT payload | ERR_INVALID_SD_HASH | | ||
|
|
||
|
|
||
| For other unknown exceptions, error code will be `ERR_INVALID_UNKNOWN` |
There was a problem hiding this comment.
Fix duplicate KB JWT header row and confirm unknown-error code
In the vc+sd-jwt/dc+sd-jwt error table the KB JWT header row appears twice with the same error code (ERR_INVALID_KB_JWT_HEADER). That looks accidental; drop one row or clarify if a different condition/code was intended.
Also, this section uses ERR_INVALID_UNKNOWN as the catch‑all error, while other tables use ERR_GENERIC. Please confirm that this difference is intentional; if not, aligning on one generic code will avoid confusing consumers.
🤖 Prompt for AI Agents
In vc-verifier/kotlin/README.md around lines 80 to 119, the error table contains
a duplicated "KB JWT header" row with the same code and an inconsistent
catch-all code; remove the accidental duplicate "KB JWT header" row so only one
row for that condition remains (or, if the duplicate was meant to represent a
different condition, replace it with the correct condition and error code), and
standardize the final catch-all error code to match the rest of the docs
(replace ERR_INVALID_UNKNOWN with ERR_GENERIC unless the different code is
intentional—if intentional, add a short note explaining the divergence).
| <dependency> | ||
| <groupId>com.android.identity</groupId> | ||
| <artifactId>identity-credential</artifactId> | ||
| <version>20231002</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>info.weboftrust</groupId> | ||
| <artifactId>ld-signatures-java</artifactId> | ||
| <version>1.5.0</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>decentralized-identity</groupId> | ||
| <artifactId>jsonld-common-java</artifactId> | ||
| <version>1.8.0</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jetbrains.kotlin</groupId> | ||
| <artifactId>kotlin-stdlib</artifactId> | ||
| <version>1.8.20</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify and update outdated dependency versions.
Several dependencies appear to be quite old:
identity-credential:20231002(from October 2023)kotlin-stdlib:1.8.20(from early 2023)
These versions are over 2 years old and may contain security vulnerabilities or lack important features. Verify if newer, more secure versions are available.
🌐 Web query:
What is the latest version of com.android.identity:identity-credential?
💡 Result:
The latest released version is 20231002. [1][2]
Sources:
[1] com.android.identity:identity-credential — Maven (mvnrepository).
[2] com.android.identity group listing — Maven (shows identity-credential latest).
🌐 Web query:
What is the latest stable version of Kotlin standard library (kotlin-stdlib)?
💡 Result:
The latest stable kotlin-stdlib is version 2.2.21 (released Oct 23, 2025). [1][2]
Sources:
[1] Kotlin releases (official) — releases list showing 2.2.21.
[2] Maven Central / Maven Repository — org.jetbrains.kotlin:kotlin-stdlib:2.2.21.
Update kotlin-stdlib to the latest stable version; verify other dependencies.
Verification shows identity-credential:20231002 is already at the latest available version and does not need updating.
However, kotlin-stdlib:1.8.20 is significantly outdated. The latest stable kotlin-stdlib is version 2.2.21 (released Oct 23, 2025). Update to 2.2.21.
Also verify the currency of ld-signatures-java:1.5.0 and jsonld-common-java:1.8.0, as their release dates and available updates remain unconfirmed.
🤖 Prompt for AI Agents
vc-verifier/kotlin/sample-java-app/pom.xml lines 56-75: the pom pins
kotlin-stdlib to 1.8.20 which must be bumped to the latest stable 2.2.21; edit
the dependency version for org.jetbrains.kotlin:kotlin-stdlib to 2.2.21, run a
full build and unit tests to catch any Kotlin 2.x compatibility issues, and
while here verify whether info.weboftrust:ld-signatures-java (1.5.0) and
decentralized-identity:jsonld-common-java (1.8.0) have newer releases — if
updates exist, bump them and re-run the build/tests as well.
| @Test | ||
| fun `should return error when statusSize is zero or negative`() { | ||
| val statusListJsonNegative = readFile("classpath:ldp_vc/status-list-vc.json") | ||
| .replace("\"encodedList\"", "\"statusSize\": -2, \"encodedList\"") | ||
| val statusListJsonZero = readFile("classpath:ldp_vc/status-list-vc.json") | ||
| .replace("\"encodedList\"", "\"statusSize\": 0, \"encodedList\"") | ||
| val (replacedVC, server) = prepareVC( | ||
| "classpath:ldp_vc/vcUnrevoked-https.json", | ||
| statusListJsonNegative | ||
| ) | ||
| val (replacedVCZero, serverZero) = prepareVC( | ||
| "classpath:ldp_vc/vcUnrevoked-https.json", | ||
| statusListJsonZero | ||
| ) | ||
|
|
||
| val resultNeg = checker.getStatuses(replacedVC)!!.first() | ||
|
|
||
| assertFalse(resultNeg.valid) | ||
| assertEquals(-1, resultNeg.status) | ||
| assertEquals(StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, resultNeg.error?.errorCode) | ||
|
|
||
| val resultZero = checker.getStatuses(replacedVCZero)!!.first() | ||
|
|
||
| assertFalse(resultZero.valid) | ||
| assertEquals(-1, resultZero.status) | ||
| assertEquals(StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, resultZero.error?.errorCode) | ||
|
|
||
| server.shutdown() | ||
| } |
There was a problem hiding this comment.
Shut down both MockWebServer instances to avoid test resource leaks
In the statusSize is zero or negative test you start two servers but only shut down one:
val (replacedVC, server) = prepareVC(..., statusListJsonNegative)
val (replacedVCZero, serverZero) = prepareVC(..., statusListJsonZero)
// ...
val resultZero = checker.getStatuses(replacedVCZero)!!.first()
server.shutdown() // serverZero is never shut downThis leaves serverZero running and can cause port/resource leaks in the test suite. Consider:
- val (replacedVC, server) = prepareVC(
+ val (replacedVC, server) = prepareVC(
"classpath:ldp_vc/vcUnrevoked-https.json",
statusListJsonNegative
)
- val (replacedVCZero, serverZero) = prepareVC(
+ val (replacedVCZero, serverZero) = prepareVC(
"classpath:ldp_vc/vcUnrevoked-https.json",
statusListJsonZero
)
@@
- server.shutdown()
+ server.shutdown()
+ serverZero.shutdown()🤖 Prompt for AI Agents
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt
around lines 270-298: the test starts two MockWebServer instances (server and
serverZero) but only shuts down server, leaking serverZero; modify the test to
ensure both servers are shut down (e.g., call serverZero.shutdown() after
assertions), or better wrap server lifecycles in try/finally (or use JUnit
@AfterEach helper) so both server.shutdown() calls always run even if assertions
fail.
| @BeforeEach | ||
| fun setup() { | ||
| mockkObject(Util) | ||
| every { Util.verifyJwt( | ||
| any(), | ||
| any(), | ||
| any() | ||
| ) } returns true | ||
| } |
There was a problem hiding this comment.
Isolate MockK state for Util.verifyJwt to avoid cross‑test pollution
@BeforeEach globally mocks Util.verifyJwt:
@BeforeEach
fun setup() {
mockkObject(Util)
every { Util.verifyJwt(any(), any(), any()) } returns true
}but there is no matching @AfterEach to unmock; only the should fail for invalid KB-JWT signature test calls unmockkAll() inline. This can:
- Leave
Util.verifyJwtmocked for other test classes that run after this one. - Make behavior depend on test execution order or parallelism.
Consider adding a dedicated teardown and using more targeted unmocking:
@AfterEach
fun tearDown() {
unmockkAll() // or unmockkObject(Util) if you only mock Util here
}and in the “invalid KB‑JWT signature” test, prefer unmockkObject(Util) over unmockkAll() so you don’t wipe unrelated mocks.
Also applies to: 401-444
🤖 Prompt for AI Agents
In
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SDJwtValidatorTest.kt
around lines 19 to 27, the setup mocks Util.verifyJwt in @BeforeEach but no
corresponding teardown, risking cross-test pollution; add an @AfterEach fun
tearDown() { unmockkObject(Util) } (or unmockkAll() if you truly need to clear
everything) to restore MockK state after each test, and replace any inline
unmockkAll() call in the “invalid KB-JWT signature” test with
unmockkObject(Util) to avoid wiping unrelated mocks.
| val vc = "eyJhbGciOiJFUzI1NksiLCJ0eXAiOiJ2YytzZC1qd3QifQ.eyJpc3MiOiJodHRwczovL2lzc3Vlci5leGFtcGxlLmNvbSIsInZjdCI6InVybjpldWRpOnBpZDpkZToxIiwiaWF0IjoxNzU5ODk2MTU1LCJleHAiOjI1NDE3Mjk0MTAwMCwic3ViIjoidXNlcjEyMyIsIl9zZCI6WyJxLXhNRDBIbjFaVC1yc19UMmQyRXVHazNPVGluMUNqWVBrTU9JZDdBcFdVIiwibE9GU1hfZmtvYm9abWtCUDN4Vnl4d1ctZFNXV0did0pIX3FiTzRFdnBqMCIsIm9YWUNXRE85dEFhZWMxTDBpdXZVUlllVkFya1pmWGxXb0NIN3I4YWVDVUEiXSwiY25mIjp7ImtpZCI6Imh0dHBzOi8vaG9sZGVyLmV4YW1wbGUuY29tL2tleXMvMSJ9LCJfc2RfYWxnIjoic2hhLTI1NiJ9.MEQCIGi0VpKNYtj_ODtesOCVvvqJ2cUNHBRCLO4Luo0EDRjdAiBX2_PubKDMrZT0pAMdM5l8JtEJJ7QQW8soFI6quap2vA~WyJ6MjJaT05WUzREdkotd1pFbHJ6VEJnIiwiZ2l2ZW5fbmFtZSIsIkpvaG4iXQ~WyIyeXRpSDdXa2ZjTk0zRXBfN1IzblpBIiwiZmFtaWx5X25hbWUiLCJEb2UiXQ~WyIwbS1fY3J2QVc2YUdfbGM2enAtTXdBIiwiZW1haWwiLCJqb2huLmRvZUBleGFtcGxlLmNvbSJd~eyJhbGciOiJFUzI1NksiLCJ0eXAiOiJrYitqd3QifQ.eyJpYXQiOjE3NTk4OTYxNTUsImF1ZCI6Imh0dHBzOi8vdmVyaWZpZXIuZXhhbXBsZS5jb20iLCJub25jZSI6InJhbmRvbS1ub25jZS1HSXRpNmRSTElITmlGR0dQcmdjQTJBIiwic2RfaGFzaCI6IlZoR1k0MTFycVE4dW12ZHRfc2tuSjdaVWhaQU5UYW44QS01aEFPWm5OeUUifQ.MEUCIBjYtbtg2-joD4krmgavj6AjXQwgWmxPKelUXgn_M3hRAiEA23hIj4m8SAZG-dJstoJsHomIG0Qw0QnflRCWVQVcaho" | ||
| val status = validator.validate(vc) | ||
| assertEquals("Unsupported 'kid' format in 'cnf'. Only DID format is supported", status.validationMessage) | ||
| assertEquals("ERR_INVALID_CNF_KID",status.validationErrorCode) | ||
| } |
There was a problem hiding this comment.
Inline JWT fixtures are fine but will keep triggering secret scanners
The hard‑coded JWT‑looking strings in these tests are clearly fixtures, but tools like gitleaks flag them as potential secrets. To reduce noise in CI while keeping the coverage, you could:
- Move the full JWTs into dedicated resource files (similar to other sd‑jwt fixtures) and reference them via
ResourceUtils.getFile, or - Truncate or slightly mangle the payload/signature so they no longer parse as real JWTs, if that doesn’t affect validation logic.
This is optional, but it will make security scanning output cleaner.
Also applies to: 550-556
🧰 Tools
🪛 Gitleaks (8.29.0)
[high] 395-395: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
In
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/validator/SDJwtValidatorTest.kt
around lines 395-399 (and similarly 550-556), the tests embed full JWT-like
strings that trigger secret scanners; replace these inline JWT fixtures by
moving the full JWTs to resource files (e.g., test-resources under the same
package) and load them with ResourceUtils.getFile, or alternatively
mangle/truncate the payload/signature so they no longer parse as real JWTs while
preserving the parts the validator needs, and update the test to read the
fixture from the resource (or the mangled variant) so CI secret scanners no
longer flag them.
| "benefits": "schema:benefits", | ||
| "fullName": "schema:name", | ||
| "policyExpiresOn": "schema:expires", | ||
| "policyIssuedOn": "schema:DateTime", |
There was a problem hiding this comment.
Fix incorrect schema.org mapping.
"policyIssuedOn": "schema:DateTime" is incorrect because DateTime is a data type in schema.org, not a property. Consider using an appropriate date property instead:
Apply this diff:
- "policyIssuedOn": "schema:DateTime",
+ "policyIssuedOn": "schema:dateIssued",Alternatively, "schema:datePublished" could be appropriate depending on the semantic intent.
📝 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.
| "policyIssuedOn": "schema:DateTime", | |
| "policyIssuedOn": "schema:dateIssued", |
🤖 Prompt for AI Agents
In
vc-verifier/kotlin/vcverifier/src/test/resources/contexts/insurance-context.json
around line 17, the mapping "policyIssuedOn": "schema:DateTime" is incorrect
because DateTime is a datatype not a property; replace the value with an
appropriate schema.org date property (for example "schema:datePublished" or
"schema:dateCreated" depending on intent) so the mapping becomes
"policyIssuedOn": "schema:datePublished" (or chosen property), and update any
related tests/fixtures to reflect the new property name.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mosipRevokedVC.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (1)
57-62: Consider using an enum or sealed class for thestatusfield.The
status: Intfield uses a magic integer (0 for unrevoked, 1 for revoked based on test assertions). Consider defining a dedicated enum or sealed class for better type safety and self-documenting code:enum class CredentialStatus(val code: Int) { ACTIVE(0), REVOKED(1) // extend as needed }Alternatively, if the integer values must match an external specification, document the expected values inline.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
46-54: Lazy-initialized loader is not cached whendocumentLoaderis null.When
documentLoaderis null, a newConfigurableDocumentLoaderis created on every call but not assigned back todocumentLoader. If this is intentional (for stateless behavior), consider documenting it. Otherwise, cache the result:fun getConfigurableDocumentLoader(): ConfigurableDocumentLoader { - return documentLoader ?: run { + return documentLoader ?: synchronized(this) { + documentLoader ?: run { val loader = ConfigurableDocumentLoader() loader.isEnableHttps = true loader.isEnableHttp = true loader.isEnableFile = false + documentLoader = loader loader } + } }
131-145: Exception type inconsistency inverifyJwt.Line 133 throws
IllegalArgumentException(viarequire) for invalid JWT format, while lines 142-144 wrap other failures inSignatureVerificationException. Consider consistent exception handling:- require(parts.size == 3) { "Invalid JWT format" } + if (parts.size != 3) { + throw SignatureVerificationException("Invalid JWT format: expected 3 parts, got ${parts.size}") + }This ensures callers only need to handle one exception type for all verification failures.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (2)
41-41: Unused logger declaration.The
loggeris declared but not used in any test method. Remove it or add logging to tests where appropriate.- private val logger = Logger.getLogger(PresentationVerifierTest::class.java.name)
36-39: Consider clearing MockK mocks inteardownAll.The tests use
mockkObject(NetworkManagerClient.Companion)but don't callunmockkObjectin cleanup. This could cause test pollution if other test classes run in the same JVM:@AfterAll fun teardownAll() { Util.documentLoader = null + io.mockk.unmockkAll() }vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt (3)
53-139: VP proof verification helper is correct; consider minor maintainability/logging tweaksThe extracted
getPresentationVerificationStatuscorrectly centralizes VP proof verification, and the switch toPublicKeyResolverFactory().get(verificationMethod)plugs cleanly into the existing ED25519 branches with appropriate exception handling.Two optional improvements you might consider later:
- Inject
PublicKeyResolverFactory(or aPublicKeyResolverabstraction) intoPresentationVerifierinstead of constructing it ad‑hoc to ease testing and reuse.- Log the full exception (e.g.,
logger.log(Level.SEVERE, "Error while verifying presentation proof", e)) rather than onlye.messageto preserve stack traces for debugging.
162-171: Status-aware VC mapping mirrors existing path; could share common logicThe implementation cleanly extends
getVCVerificationResultsby reusing the same verification pipeline and adding credential-status extraction viaverifyAndGetCredentialStatus, which keeps backwards compatibility intact.If you want to reduce duplication later, consider factoring the common “VC JSON → VerificationResult → VerificationStatus + serialized VC” mapping into a shared helper and have both
getVCVerificationResultsandgetVCVerificationResultsWithCredentialStatusbuild on top of it, with the latter only adding thecredentialStatusplumbing.
173-183: NewverifyAndGetCredentialStatusAPI is consistent; clarifystatusPurposeListsemanticsThe new status-aware API is consistent with the existing
verify()method and correctly reusesgetPresentationVerificationStatusplus the new VC-status helper, so behaviour at both VP and VC levels stays aligned.Two small follow-ups you might consider:
- Add brief KDoc documenting how
statusPurposeListis interpreted (e.g., empty = “check all supported purposes” vs “skip status checks”) so API users don’t have to infer this from tests or implementation details.- Based on learnings, SD‑JWT credentials are expected to bypass status checks via a
checkStatusthat returns null; it’s good that this API explicitly usesCredentialFormat.LDP_VC, but a short comment in the verifier layer noting that SD‑JWT uses a separate path would make that design intention obvious to future readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt(6 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt(3 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithRevokedVC.json
🚧 Files skipped from review as they are similar to previous changes (2)
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/LocalDocumentLoader.kt
- vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/utils/UtilsTest.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
📚 Learning: 2025-11-03T12:07:06.442Z
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
Applied to files:
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: build-maven-vc-verifier / maven-build
🔇 Additional comments (9)
vc-verifier/kotlin/vcverifier/src/test/resources/vp/VPWithUnrevokedVC.json (1)
96-102: CredentialStatus structure is correctly compatible with LdpStatusChecker expectations.The JSON file properly uses:
statusListIndexas a string (e.g.,"104282"), whichLdpStatusCheckerparses via.toString().toInt()conversion (line 256 of LdpStatusChecker.kt)BitstringStatusListEntrytype is recognized by the implementation- Fragment-based
idnotation is standard for status list entries- All required fields are present and match the implementation's expectations
This structure aligns with existing test cases in StatusListRevocationCheckerTest.kt that validate the same format across multiple scenarios.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/constants/CredentialVerifierConstants.kt (1)
24-24: LGTM! New P-256/ES256 constants added correctly.The
P256constant correctly uses the standard NIST curve name "P-256" (secp256r1), andES256_KEY_TYPE_2019follows the established naming convention alongside the existingES256K_KEY_TYPE_2019. These additions properly support the ES256 algorithm verification flow.Also applies to: 37-40
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (2)
17-31: LGTM! New data classes support credential status verification flow.The new
PresentationResultWithCredentialStatusandVCResultWithCredentialStatusclasses properly extend the verification result model to include credential status information. The structure aligns with the existingPresentationVerificationResultpattern.
36-36: LGTM! Enum updates are appropriate.
- Adding
REVOKEDtoVerificationStatuscompletes the status representation.- Renaming
DATA_MODELtoDataModelaligns with Kotlin naming conventions (PascalCase for enum classes).Also applies to: 46-50
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/utils/Util.kt (2)
88-95: LGTM! New HTTPS URI validation helper.The
isValidHttpsUrifunction correctly validates that a URI uses HTTPS scheme with a non-null host, complementing the existingisValidUrifunction.
101-111: LGTM! Updated to use renamedDataModelenum.The
getContextVersionfunction correctly returnsDataModelenum values, aligning with the enum rename fromDATA_MODELtoDataModel.vc-verifier/kotlin/vcverifier/src/test/resources/contexts/mock-identity-context.json (1)
1-103: LGTM! Valid JSON-LD context for test resources.The mock identity context follows JSON-LD 1.1 conventions correctly:
- Properly defines the MOSIP namespace and vocabulary terms
- Uses nested
@contextpattern for language-tagged values- Includes both full field names and abbreviated variants for flexibility
This test resource will support the credential verification test scenarios with MockVerifiableCredential types.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
116-153: Verification confirmed: tests are correctly designed.The
VPVerificationStatus.INVALIDexpectation is intentional. Both tests use identical VP files with mock/invalid JWS data (".hello." payload), which correctly fail proof verification while allowing credential status checks to execute independently. The key test difference is the status list response (revoked vs unrevoked), which correctly produces oppositecredentialStatus[0].validvalues (falsefor revoked,truefor unrevoked). This design properly isolates status verification concerns from proof validation.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/PresentationVerifier.kt (1)
43-51: Refactoringverify()to delegate VP proof verification looks soundDelegating VP proof verification into
getPresentationVerificationStatuskeepsverify()simple and ensures both verify paths share identical proof logic. No behavioural regressions visible here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
vc-verifier/kotlin/README.md (2)
800-801: Fix duplicate KB JWT header row.The error table contains a duplicate "KB JWT header" row (lines 800–801) with the same error code. Remove one row or clarify if a different condition/code was intended.
Apply this diff to remove the duplicate:
| KB JWT header | ERR_INVALID_KB_JWT_HEADER | - | KB JWT header | ERR_INVALID_KB_JWT_HEADER | | alg in KB JWT header | ERR_MISSING_KB_JWT_ALG |
832-832: Confirm or align catch‑all error code for status checks.Line 832 uses
ERR_INVALID_UNKNOWNas the catch-all, while other error sections useERR_GENERIC. Please confirm this inconsistency is intentional; if not, align on a single code.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (1)
267-295: Shut down bothMockWebServerinstances to avoid test resource leaks.The test creates two MockWebServer instances (
serverat line 273 and the unnamed server at line 274) but only shuts downserverat line 294. The second server created forreplacedVCZerois never shut down, which can cause port/resource leaks.Apply this diff to fix the leak:
val (replacedVC, server) = prepareVC(statusListJson = statusListJsonNegative) - val (replacedVCZero, _) = prepareVC( + val (replacedVCZero, serverZero) = prepareVC( statusListJson = statusListJsonZero ) val resultNeg = checker.getStatuses(replacedVC).entries.first() assertFalse(resultNeg.value.isValid) assertEquals( StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, resultNeg.value.error?.errorCode ) val resultZero = checker.getStatuses(replacedVCZero).entries.first() assertFalse(resultZero.value.isValid) assertEquals( StatusCheckErrorCode.STATUS_VERIFICATION_ERROR, resultZero.value.error?.errorCode ) server.shutdown() + serverZero.shutdown()vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (1)
265-278: Prevent negative or overflowing STATUS_LIST_INDEX from reading wrong bits.The validation at line 307 only checks that
STATUS_LIST_INDEXis parseable as an Int but does not enforce non-negativity. Later at line 265, the index is re-parsed, and line 272 computesbitPosition = statusListIndex * statusSizewithout checking for:
- Negative
statusListIndex- Integer overflow in the multiplication
- Negative
bitPositionin the range check at line 273This can lead to reading from incorrect bit positions.
Apply this diff to add proper validation:
val statusListIndex = entry[STATUS_LIST_INDEX].toString().toInt() + if (statusListIndex < 0) { + throw StatusCheckException( + "'$STATUS_LIST_INDEX' must be >= 0", + INVALID_INDEX + ) + } val decodedBitSet = decodeEncodedList(encodedList) val totalBits = decodedBitSet.size * 8 if (totalBits < minimumNumberOfEntries * statusSize) { logger.warning("Decoded bitstring shorter than required herd privacy length") } val bitPosition = statusListIndex * statusSize - if (bitPosition >= totalBits) { + if (bitPosition < 0 || bitPosition >= totalBits) { throw StatusCheckException( "Bit position $bitPosition out of range", RANGE_ERROR ) }
🧹 Nitpick comments (7)
README.md (1)
61-61: Fix list indentation to align with markdown standards.Lines 61 and 66 have 4-space indentation but should use 2-space indentation per MD007.
Apply this diff:
- `statusChecker` + - `statusChecker`- **keyResolver** - PublicKeyResolverFactory for different type of verification method types are declared in this package. + - **keyResolver** - PublicKeyResolverFactory for different type of verification method types are declared in this package.Also applies to: 66-66
vc-verifier/kotlin/README.md (3)
63-66: Fix markdown list indentation throughout the file.Multiple list items have 4-space indentation when they should use 2-space indentation per MD007. Additionally, some nested items have incorrect indentation levels (e.g., lines 316–318 use 8 spaces instead of 4).
Apply consistent 2-space indentation for first-level bullets and 4-space for nested items:
- * `LdpValidator` for LDP VC + * `LdpValidator` for LDP VCThis pattern applies to all flagged lines. Consider using a markdown linter (e.g.,
markdownlint --fix) to auto-correct all indentation violations.Also applies to: 74-79, 174-176, 206-208, 220-221, 262-267, 284-292, 307-318, 489-489, 588-595
708-708: Fix table formatting issues.Line 708 has a table column count mismatch: the header declares 3 columns but the row contains 4 cells. Line 611 is missing blank lines around a table per MD058.
Apply these fixes:
- Line 708: Verify the intended table structure and ensure all rows have matching column counts.
- Line 611: Add blank lines before and after the table.
Also applies to: 611-611
725-725: Replace emphasis with proper headings.Lines 725, 768, 777, and 817 use bold text (e.g.,
**ldp_vc Error Codes**) where markdown headings should be used per MD036. Use#syntax instead:- **ldp_vc Error Codes** + ## ldp_vc Error Codes- **mso_mdoc Format VC Error Codes** + ## mso_mdoc Format VC Error Codes- **vc+sd-jwt/dc+sd-jwt Format VC Error Codes** + ## vc+sd-jwt/dc+sd-jwt Format VC Error Codes- **Status Check Error Codes** + ## Status Check Error CodesAlso applies to: 768-768, 777-777, 817-817
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (1)
8-9: Consider adding error handling for missing test resources.The
readClasspathFilefunction will throw an exception if the resource is missing, which is appropriate for tests. However, consider whether a more descriptive error message would help developers debug test failures faster.Optionally, wrap with a more descriptive error:
fun readClasspathFile(path: String): String = try { String(Files.readAllBytes(ResourceUtils.getFile(ResourceUtils.CLASSPATH_URL_PREFIX + path).toPath())) } catch (e: FileNotFoundException) { throw AssertionError("Test resource not found: $path", e) }vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (1)
66-69: Minor: Formatting inconsistency in multi-line assertEquals.The assertion spans multiple lines, which is fine, but ensure consistent formatting across all tests.
Consider:
assertEquals("${ERROR_CODE_MISSING}${CONTEXT.uppercase()}", verificationResult.verificationErrorCode)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (1)
45-45: Remove the empty default constructor.The empty default constructor can be removed for cleaner code.
Apply this diff:
-class LdpStatusChecker() { +class LdpStatusChecker {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
README.md(3 hunks)vc-verifier/kotlin/README.md(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt(0 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt(8 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.kt(4 hunks)vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt(2 hunks)vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.json(1 hunks)vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json(1 hunks)
💤 Files with no reviewable changes (1)
- vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt
✅ Files skipped from review due to trivial changes (3)
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didMosipKey.json
- vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey2.json
- vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didIdentityServiceKey.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
📚 Learning: 2025-11-03T12:07:06.442Z
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
Applied to files:
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.ktvc-verifier/kotlin/README.mdvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.ktvc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.ktREADME.mdvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.ktvc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt
🧬 Code graph analysis (4)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
readClasspathFile(8-9)mockHttpResponse(13-17)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
mockHttpResponse(13-17)readClasspathFile(8-9)
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (1)
vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (2)
readClasspathFile(8-9)mockHttpResponse(13-17)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (3)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifier.kt (1)
verify(32-76)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifier.kt (2)
verify(9-36)verify(11-15)vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifier.kt (1)
verify(41-72)
🪛 detekt (1.23.8)
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt
[warning] 45-45: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
[warning] 145-145: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🪛 markdownlint-cli2 (0.18.1)
vc-verifier/kotlin/README.md
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
91-91: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
102-102: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
107-107: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
108-108: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
110-110: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
112-112: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
113-113: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
115-115: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
116-116: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
117-117: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
220-220: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
221-221: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
262-262: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
263-263: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
267-267: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
270-270: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
284-284: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
285-285: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
290-290: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
291-291: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
292-292: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
307-307: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
308-308: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
312-312: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
314-314: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
315-315: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
316-316: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
317-317: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
318-318: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
489-489: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
588-588: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
589-589: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
591-591: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
592-592: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
593-593: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
594-594: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
595-595: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
611-611: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
708-708: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
725-725: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
768-768: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
777-777: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
817-817: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
README.md
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
66-66: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: publish-snapshot / maven-android-build
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: build-maven-vc-verifier / maven-build
🔇 Additional comments (43)
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/idaPublicKey.json (1)
1-7: LGTM!The test resource structure is valid and appropriate for testing RSA public key resolution. The JSON-LD format aligns with W3C security vocabulary standards.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEcKey.json (1)
1-28: LGTM!The DID document structure is valid and follows the W3C DID specification. The EC secp256k1 key format is appropriate for testing elliptic curve-based verification flows.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/mockDid.json (1)
1-42: LGTM!The DID document provides comprehensive test coverage with three different key types (RSA, EC secp256k1, Ed25519) using different representation formats (PEM, JWK, Multibase). This is valuable for testing heterogeneous key resolution scenarios.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/msomdoc/MsoMdocVerifiableCredential.kt (1)
54-54: LGTM - Improved immutability.Changing
cborsfromMutableListtoListis a good practice that prevents unintended mutations and aligns with Kotlin immutability best practices.vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/exception/StatusCheckException.kt (2)
3-3: LGTM - Well-structured exception class.The exception design properly captures both error message and error code, enabling both human-readable messaging and programmatic error handling.
5-16: LGTM - Comprehensive error code coverage.The error codes cover all relevant failure scenarios in the status-check flow, including encoding/decoding errors, validation errors, and retrieval failures.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/VerifiableCredential.kt (1)
9-11: Breaking API change - migration from revocation to status checks.The interface now provides
checkStatusinstead ofisRevoked. The default implementation throwsUnsupportedOperationException, which is appropriate for credential formats that don't support status checking (e.g., MSO MDOC).Note: Based on learnings, some credential types (like SD-JWT) should return null to bypass status checks, while others (like MSO MDOC) throw this exception.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/MsoMdocVerifierTest.kt (2)
18-19: LGTM - Good test data refactoring.Extracting the valid MDOC credential to a reusable field improves test maintainability and reduces duplication.
105-112: LGTM - Appropriate test coverage for unsupported operation.The test correctly validates that MSO MDOC credentials throw
UnsupportedOperationExceptionwhen status checking is attempted, consistent with the interface default behavior.vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/httpsMockKey.json (1)
1-7: LGTM!The test resource structure is valid and appropriate for testing HTTPS-based public key resolution. The format aligns with W3C security vocabulary standards.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt (1)
3-7: LGTM! Explicit imports improve clarity.Replacing wildcard imports with explicit assertion imports is a good practice that makes dependencies clearer.
vc-verifier/kotlin/vcverifier/src/test/resources/ldp_vc/public_key/didEdPublicKey.json (1)
1-26: LGTM! Valid DID document structure.The test resource provides a well-formed DID document with:
- Proper Ed25519VerificationKey2020 verification method
- Correct context for Ed25519-2020 suite
- Valid publicKeyMultibase encoding
- Appropriate authentication and assertionMethod references
vc-verifier/kotlin/vcverifier/src/test/resources/vp/public_key/didMockKey.json (1)
1-42: LGTM! Comprehensive multi-algorithm DID document.This test resource provides excellent coverage for testing multiple signature algorithms:
- RSA (RsaVerificationKey2018)
- ECDSA secp256k1 (EcdsaSecp256k1VerificationKey2019)
- Ed25519 (Ed25519VerificationKey2020)
The structure is valid and properly formatted for DID verification testing.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/LdpVerifiableCredential.kt (1)
19-21: LGTM! Clean migration to credential-status architecture.The implementation properly:
- Replaces the old
isRevokedmethod withcheckStatus- Returns the new
Map<String, CredentialStatusResult>type- Delegates to
LdpStatusCheckerfor status retrieval- Accepts optional
statusPurposesparameter for filteringThis aligns with the PR's objective to adopt a credential-status architecture and is consistent with the broader status-check refactoring across the codebase.
Based on learnings, the approach differs correctly from SD-JWT credentials (which return null to bypass status checks).
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/CredentialsVerifierTest.kt (4)
29-43: LGTM! Proper test lifecycle management.The test class correctly:
- Uses
@TestInstance(PER_CLASS)for shared setup/teardown- Mocks
NetworkManagerClientin setup- Configures
LocalDocumentLoaderfor test context loading- Resets the document loader in teardown
This ensures tests don't interfere with each other and external HTTP calls are properly mocked.
157-168: LGTM! Correct test for verification failure scenario.The test properly validates that when VC verification fails, the status list remains empty. This is the correct behavior since status checks should only proceed after successful verification.
170-199: LGTM! Comprehensive status-check test for unrevoked VC.The test properly:
- Mocks both the status list endpoint and DID document
- Verifies the credential and retrieves status
- Asserts the status result indicates "revocation" purpose with
isValid = true- Checks that no error is present
201-227: LGTM! Comprehensive status-check test for revoked VC.The test properly validates the revoked credential scenario by:
- Mocking the revoked status list and DID document
- Asserting the status result indicates "revocation" purpose with
isValid = false- Verifying no error is present in the result
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/LdpVerifierTest.kt (2)
23-36: LGTM! Clean test lifecycle setup.The test class properly:
- Uses
@TestInstance(PER_CLASS)for shared setup/teardown- Configures
LocalDocumentLoaderfor context loading- Mocks
NetworkManagerClientfor HTTP calls- Preloads mock public keys in setup
- Resets document loader in teardown
98-104: LGTM! Centralized mock key setup.The
loadMockPublicKeyshelper consolidates all public key mocking in one place, making it easier to maintain and understand which keys are available for tests.vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/PresentationVerifierTest.kt (4)
27-39: LGTM! Consistent test lifecycle management.The setup mirrors the pattern used in other test files:
- Mocks
NetworkManagerClient- Configures
LocalDocumentLoader- Properly resets state in teardown
101-127: Verify that VPVerificationStatus.INVALID is the expected status for revoked VCs.The test asserts
VPVerificationStatus.INVALIDat line 119, but the VC verification itself succeeds (VerificationStatus.SUCCESSat line 121). This suggests the VP proof verification fails, not the credential status check.Is this the intended behavior? If the VP proof should be valid but the VC is revoked, the status might need adjustment.
Please clarify:
- Should a revoked VC cause the VP proof verification to be INVALID?
- Or should the VP proof be VALID with the VC status indicating revocation separately?
129-154: Same verification concern: check VPVerificationStatus.INVALID expectation.Similar to the previous test, this asserts
VPVerificationStatus.INVALIDat line 146 while the VC verification succeeds. Please confirm this is the expected behavior for unrevoked credentials.
44-52: LGTM! Test updated to use new test utilities.The migration to
readClasspathFileis consistent with the test infrastructure improvements across the PR.vc-verifier/kotlin/vcverifier/src/test/java/testutils/TestUtils.kt (1)
13-16: Mock properly covers all sendHTTPRequest invocation scenarios.All production code calls
sendHTTPRequestwith only 2 parameters (URL andHttpMethod.GET), and the mock in TestUtils.kt correctly usesany()for the second parameter to match this pattern. Verification shows:
- 3 production call sites (HttpsPublicKeyResolver, DidWebPublicKeyResolver, LdpStatusChecker) all use GET method with 2 parameters
- All test files properly mock these calls either via
mockHttpResponse()helper or directeveryblocks- No POST calls or additional parameters (bodyParams, headers) used in current codebase
- The
any()matcher appropriately abstracts the HTTP method, allowing both GET and POST if needed in futureThe mock setup is correct and comprehensive.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/data/Data.kt (7)
3-4: LGTM!The import is correctly added to support the new
CredentialStatusResultdata class that usesStatusCheckException.
17-20: LGTM!The new data class appropriately extends the presentation result model to include credential status information.
27-31: LGTM!The design of using
Map<String, CredentialStatusResult>allows for multiple status purposes to be checked and reported simultaneously.
36-36: LGTM!The
REVOKEDstatus value is a necessary addition to support the credential status checking infrastructure.
46-50: LGTM!The rename from
DATA_MODELtoDataModelfollows Kotlin naming conventions for enum types.
57-60: LGTM!The data class appropriately models status check results with both validation state and optional error details.
62-65: LGTM!The summary class provides a convenient aggregation of both verification results and credential status checks.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/CredentialsVerifier.kt (5)
10-11: LGTM!The imports are correctly added to support the new credential status checking functionality.
18-18: LGTM!Reusing a single
CredentialVerifierFactoryinstance improves performance by avoiding repeated instantiation.
41-74: Verify the handling of expired credentials in the verification flow.The updated logic allows expired VCs to proceed to signature verification (lines 44-47 exclude
ERROR_CODE_VC_EXPIREDfrom early rejection). This means:
- An expired VC will still return
verificationStatus = trueif the signature is valid- The expiry information is conveyed via
validationMessageandvalidationErrorCodeEnsure this behavior aligns with the project's requirements for handling expired credentials. Based on learnings, the design intent seems to separate signature verification from validity checks, but confirm that downstream consumers understand that
verificationStatus = truedoesn't guarantee the credential is currently valid.
76-91: LGTM!The method appropriately handles unsupported credential formats by catching
UnsupportedOperationExceptionand returning an empty map, allowing graceful degradation. Based on learnings, this aligns with the design for SD-JWT credentials.
93-104: LGTM!The combined method provides a convenient API for callers who need both signature verification and credential status checks in a single call.
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/StatusListRevocationCheckerTest.kt (2)
1-46: LGTM!The test setup appropriately uses MockK to mock credential verification and provides clean helper methods for test data preparation.
82-453: Comprehensive test coverage.The test suite thoroughly validates the status checker across a wide range of scenarios including success cases, various error conditions, edge cases for validation, and temporal checks. The test names clearly describe the scenarios being tested.
vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/statusChecker/LdpStatusChecker.kt (4)
62-124: LGTM!The method appropriately handles both single and multiple
credentialStatusentries, with proper filtering and error handling per entry.
135-224: LGTM!The method properly fetches and validates the status list VC, including signature verification, temporal validity checks, and type/purpose validation. The exception wrapping at line 145 is appropriate (not swallowed despite the static analysis hint).
321-356: LGTM!The decoding and decompression logic properly handles the base64url format with optional 'u' prefix and includes appropriate error handling for both decoding and decompression failures.
285-285: LGTM!The helper methods for validation are simple and correctly implemented.
Also applies to: 390-391
| @Test | ||
| fun `should throw exception for mso_mdoc status check as its not supported`() { | ||
| val file = ResourceUtils.getFile(ResourceUtils.CLASSPATH_URL_PREFIX + "sd-jwt_vc/sdJwt.txt") | ||
| val vc = String(Files.readAllBytes(file.toPath())) | ||
| val unsupportedStatusCheckException = assertThrows(UnsupportedOperationException::class.java) { | ||
| MsoMdocVerifiableCredential().checkStatus(vc, null) | ||
| } | ||
|
|
||
| assertEquals("Credential status checking not supported for this credential format",unsupportedStatusCheckException.message) | ||
| } |
There was a problem hiding this comment.
Test is misplaced and uses incorrect test fixture.
This test verifies MsoMdocVerifiableCredential behavior but:
- It's located in
SdJwtVerifierTest(should be inMsoMdocVerifierTest) - It loads an SD-JWT fixture (
sd-jwt_vc/sdJwt.txt) to test MSO_MDOC status checking
Move this test to MsoMdocVerifierTest.kt and use an appropriate MSO_MDOC fixture:
- @Test
- fun `should throw exception for mso_mdoc status check as its not supported`() {
- val file = ResourceUtils.getFile(ResourceUtils.CLASSPATH_URL_PREFIX + "sd-jwt_vc/sdJwt.txt")
- val vc = String(Files.readAllBytes(file.toPath()))
- val unsupportedStatusCheckException = assertThrows(UnsupportedOperationException::class.java) {
- MsoMdocVerifiableCredential().checkStatus(vc, null)
- }
-
- assertEquals("Credential status checking not supported for this credential format",unsupportedStatusCheckException.message)
- }In MsoMdocVerifierTest.kt, add:
@Test
fun `should throw exception for mso_mdoc status check as its not supported`() {
val vc = // use appropriate MSO_MDOC fixture
val unsupportedStatusCheckException = assertThrows(UnsupportedOperationException::class.java) {
MsoMdocVerifiableCredential().checkStatus(vc, null)
}
assertEquals("Credential status checking not supported for this credential format", unsupportedStatusCheckException.message)
}🤖 Prompt for AI Agents
In
vc-verifier/kotlin/vcverifier/src/test/java/io/mosip/vercred/vcverifier/credentialverifier/verifier/SdJwtVerifierTest.kt
around lines 32-41, the test verifying MsoMdocVerifiableCredential is misplaced
and uses an SD-JWT fixture; remove this test from SdJwtVerifierTest and add the
same test to MsoMdocVerifierTest.kt using an MSO_MDOC fixture from test
resources (replace the sd-jwt file load with the appropriate MSO_MDOC resource),
then instantiate MsoMdocVerifiableCredential().checkStatus(vc, null) inside
assertThrows and assert the same error message; ensure the new test file imports
ResourceUtils/Files as needed and the test name and assertions remain identical.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
vc-verifier/kotlin/README.md (2)
816-817: Remove duplicate "KB JWT header" error row.The error table contains a redundant "KB JWT header" row on lines 816-817 with identical error code. Remove one row to eliminate the accidental duplication.
| KB JWT header | ERR_INVALID_KB_JWT_HEADER | -| KB JWT header | ERR_INVALID_KB_JWT_HEADER | | alg in KB JWT header | ERR_MISSING_KB_JWT_ALG |
833-848: Standardize catch-all error code: useERR_GENERICinstead ofERR_INVALID_UNKNOWN.Line 848 uses
ERR_INVALID_UNKNOWNas the fallback for status-check errors, while the LDP VC (line 782) and MSO MDOC (line 791) sections useERR_GENERIC. This inconsistency may confuse API consumers. Align on a single catch-all code for consistency across all error-code sections.-For other unknown exceptions, error code will be `ERR_INVALID_UNKNOWN` +For other unknown exceptions, error code will be `ERR_GENERIC`
🧹 Nitpick comments (2)
vc-verifier/kotlin/README.md (2)
255-255: Convert emphasis formatting to proper markdown headings.The following lines use bold emphasis (
**text**) in place of section headings, which violates markdown best practices (MD036) and breaks document structure for tools that parse heading hierarchy.Convert each from
**Text**to### Text:
- Line 255:
**Verify and Obtain Credential status**→### Verify and Obtain Credential status- Line 280:
**Verify Credential**→### Verify Credential- Line 305:
**Get Credential Status**→### Get Credential Status- Line 332:
**Deprecated Verify Credentials Method**→### Deprecated Verify Credentials Method- Line 496:
**Verify Presentation**→### Verify Presentation- Line 593:
**Verify Presentation with Credential Status**→### Verify Presentation with Credential Status- Line 741:
**ldp_vc Error Codes**→### ldp_vc Error Codes- Line 784:
**mso_mdoc Format VC Error Codes**→### mso_mdoc Format VC Error Codes- Line 793:
**vc+sd-jwt/dc+sd-jwt Format VC Error Codes**→### vc+sd-jwt/dc+sd-jwt Format VC Error Codes- Line 833:
**Status Check Error Codes**→### Status Check Error CodesAlso applies to: 280-280, 305-305, 332-332, 496-496, 593-593, 741-741, 784-784, 793-793, 833-833
73-118: Fix list indentation to comply with MD007 (2-space indent for nested lists).Multiple nested list items use 4-space indentation when markdown best practice (and markdownlint) expects 2 spaces. Examples:
- Lines 73–76 (Mandatory field checks)
- Lines 88–92 (validityInfo validation)
- Lines 99–117 (SD-JWT validation steps)
- Lines 206–221 (Status check flow)
- Lines 268–328 (CredentialVerifier API parameters)
Change all nested bullet points from 4 spaces to 2 spaces:
* **Mandatory field checks:** - * Common: `@context`, ... - * v1.1 specific: ... + * Common: `@context`, ... + * v1.1 specific: ...Also applies to: 206-221, 268-328
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
vc-verifier/kotlin/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
📚 Learning: 2025-11-03T12:07:06.442Z
Learnt from: abhip2565
Repo: mosip/vc-verifier PR: 202
File: vc-verifier/kotlin/vcverifier/src/main/java/io/mosip/vercred/vcverifier/credentialverifier/types/SdJwtVerifiableCredential.kt:18-21
Timestamp: 2025-11-03T12:07:06.442Z
Learning: For SD-JWT credentials in the vc-verifier project, the checkStatus method should return null to bypass status checks rather than throwing errors or implementing actual status verification.
Applied to files:
vc-verifier/kotlin/README.md
🪛 markdownlint-cli2 (0.18.1)
vc-verifier/kotlin/README.md
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
91-91: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
102-102: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
107-107: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
108-108: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
109-109: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
110-110: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
112-112: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
113-113: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
115-115: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
116-116: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
117-117: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
175-175: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
206-206: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
207-207: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
208-208: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
213-213: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
214-214: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
220-220: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
221-221: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
268-268: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
269-269: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
276-276: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
280-280: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
292-292: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
293-293: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
298-298: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
299-299: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
300-300: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
305-305: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
317-317: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
318-318: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
322-322: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
324-324: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
325-325: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
326-326: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
327-327: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
328-328: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
332-332: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
496-496: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
503-503: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
593-593: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
604-604: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
605-605: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
607-607: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
608-608: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
609-609: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
610-610: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
611-611: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
627-627: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
724-724: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
741-741: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
784-784: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
793-793: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
833-833: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: publish-snapshot / maven-android-build
- GitHub Check: build-maven-vc-verifier / maven-build
- GitHub Check: Analyze (java-kotlin)
Prafulrakhade
force-pushed
the
master
branch
from
ac477ca to
70a8433
Compare
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.