Code review · Round 10 · Independent baseline

Two repositories, measured from scratch

A fresh pass over the TrustWeave SDK and the TrustWeave SaaS, scored without reference to the nine remediation rounds that came before it. Four previously-reported HIGH findings verified closed; three new authorization gaps found in the Verifiable Intent chain verifier.

2026-09-05 SDK 231,134 LOC main / 87,152 test / 106 modules SaaS 22,189 LOC main / 12,998 test / 187 frontend sources Method: static analysis, no build executed
trustweave
7.0/ 10

Cryptographic core is disciplined and fails closed. The weak edge is the newest plugin — Verifiable Intent — plus release engineering for a 106-module published SDK.

Security
7.5
Correctness
8.0
Architecture
7.0
Testing
6.5
Supply chain & CI
6.0
trustweave-saas
6.9/ 10

Tenant isolation and authorization are consistently right where I traced them. Schema ownership is split against itself, and the backend has no static analysis at all.

Security
8.0
Correctness
7.0
Architecture
7.0
Testing
7.0
Supply chain & CI
5.0
On the gap with round 9

Round 9 scored these 98 and 96. That number answers a different question: it measures how completely each round's own findings were remediated, and it is accurate for that. This one measures the codebase against what a security-critical identity SDK needs before a 1.0 — including things no prior round had opened, like the constraint-enforcement asymmetry below and the absence of API-surface control. Both readings can be true at once.

TrustWeave SDK

Findings

Eleven findings. The two HIGH items are in credentials/plugins/verifiable-intent, the plugin that carries payment authorization.

High

L3 checkout fulfilments are never constraint-checked

TW-R10-01

ConstraintChecker.check has exactly one call site in the whole repository, and it sits inside the if (l3Payment != null) branch. When an agent presents only an L3 checkout, verifyL3 runs its signature, sd_hash, pair-identity and temporal checks — and then no constraint from the L2 checkout mandate is evaluated at all. That includes mandate.checkout.line_items, the constraint that bounds what the agent may buy.

The chain verifier only checks that a line_items constraint is present (open_checkout_contains_line_items), never that the fulfilment satisfies it. This also routes around ConstraintChecker's deliberate fail-closed handling: that class refuses an open mandate whose line_items it cannot evaluate, and the checkout path never asks it. Structurally this is the same shape as the omit-payment-mandate bypass closed earlier — an L3 accepted with its authority unbounded.

ChainVerifier.kt:236 only ConstraintChecker.check call site, inside `if (l3Payment != null)` ChainVerifier.kt:279 `if (l3Checkout != null) { verifyL3(...); l3CheckoutResolved = l3Checkout.resolve() }` ConstraintChecker.kt:71 line_items → fails closed for open mandates, if it is ever reached
Fix: lift the constraint block into a helper taking (mandate, fulfilment, requiredVct) and call it from both L3 branches. The checkout side should fail closed on unevaluable line_items exactly as the payment side does today.
High

Replay protection is opt-in and silently skipped

TW-R10-02

verifyChain takes expectedL2Aud and expectedL2Nonce defaulted to null. A verifier that omits them gets valid = true with a note in checksSkipped — a field nothing forces the caller to read. A captured presentation replays against a different verifier.

The contrast inside the same signature makes the case: now carries a comment explaining that it defaults to the host clock precisely so a caller who omits it cannot accidentally pin verification to a time of the presenter's choosing. Audience and nonce were not given the same treatment, and they are the two parameters a presentation verifier most needs.

ChainVerifier.kt:55-56 expectedL2Aud: String? = null, expectedL2Nonce: String? = null ChainVerifier.kt:83,466 audNonce() records "skipped" and returns null → chain still valid VerifiableIntent.kt:38-39 same defaults on the public facade
Fix: make both required for presentation verification, or add requireReplayProtection: Boolean = true that fails the chain when either expected value is absent. Leave an explicit opt-out for the offline/audit case.
Medium

A missing exp means never expires

TW-R10-03

expired() and future() both return null — pass — when the claim is absent. An L1, L2 or L3 with no exp clears every temporal check. The one-hour L3 lifetime cap is likewise conditional on both iat and exp being present, so omitting exp removes the cap rather than tripping it.

ChainVerifier.kt expired(): `val v = (exp as? JsonPrimitive)?.longOrNull ?: return null` ChainVerifier.kt `if (iat != null && exp != null && exp - iat > MAX_L3_LIFETIME_SECONDS)`
Fix: require exp and iat on L3 at minimum, and treat their absence at L1/L2 as a policy decision the caller states rather than a silent pass.
Medium

Payment-instrument cross-check passes when the L2 side is absent

TW-R10-04

paymentInstrumentCrossCheck returns null (pass) when the L2 payment mandate carries no payment_instrument. The L3 side is required to have one by paymentRequiredFields; the L2 open mandate is not. So an open mandate that never named an instrument authorizes an L3 that names any instrument, and the check reports nothing.

ChainVerifier.kt `val l2Pi = (l2Payment?.get("payment_instrument") as? JsonObject) ?: return null`
Fix: require payment_instrument on the open payment mandate, or record the skip in checksSkipped so it is at least visible to the caller.
Medium

7,354 lines of production code with no tests

TW-R10-05

Twenty-one modules have a src/main and no test source set at all. Nine are DID method plugins the SDK advertises as supported methods. The repository has 3,975 tests overall, so this is not thin testing — it is a cliff at the plugin edge.

932 credentials/plugins/status-list/database 501 did/plugins/plc 768 did/plugins/sol 478 did/registrar-server-ktor 725 did/plugins/ion 430 did/plugins/ens 548 did/plugins/cheqd 309 credentials/plugins/anchor 541 did/plugins/polygon 277 wallet/wallet-services 514 credentials/plugins/chapi 182 credentials/plugins/status-list/server 511 did/registrar-server-spring 161 anchors/plugins/starknet + btcr, tezos, threebox, venafi, salesforce, servicenow
Fix: the DID plugins share a resolution/creation contract — one parameterised conformance suite in testkit would cover nine of these at once, the way InMemoryKmsContractConformanceTest already does for KMS.
Medium

No API-surface control on 106 published modules

TW-R10-06

Neither explicitApi() nor the binary-compatibility validator is applied anywhere in the build (the only matches in the tree are inside node_modules). Every declaration is public by default, and nothing mechanically catches an ABI break before it ships to consumers. For a library at 0.7.0 that has already had to write release notes for three signature-format changes, this is the gap that keeps producing them.

build.gradle.kts freeCompilerArgs = ["-Xjsr305=strict"] only; no explicitApi, no apiValidation grep explicitApi|binary-compatibility-validator → matches only under reference-wallet/expo/node_modules
Fix: apply binary-compatibility-validator and commit .api dumps — that alone turns an ABI break into a failed PR. Then adopt explicitApi() module by module, starting with the five -core modules.
Medium

108 exceptions swallowed without a log line

TW-R10-07

Of 536 broad catch (e: Exception|Throwable) blocks in main source, 35 have an empty body and 73 return a bare null/false with nothing recorded. I traced the ones in the verification path — PresentationVerification.kt, SdJwtProofEngine.kt, ProofEngineUtils.kt — and they are all genuinely fail-closed, so this is not a correctness finding. It is an operability one: a verification that failed because a JWT would not parse is indistinguishable, from the outside, from one that failed because a signature was wrong.

PresentationVerification.kt:284,321,527,605 SdJwtProofEngine.kt:463,643,744 ProofEngineUtils.kt:506,677 BlockchainAnchor.kt:145 AbstractEvmAnchorClient.kt:258,362,408,491,498 ProviderChain.kt:63
Fix: a debug-level log at each swallow point, carrying the exception. Worth doing before the anchor plugins go to production, where these are network failures being reported as clean negatives.
Low

Coverage measured on 3 of 106 modules

TW-R10-08

Kover is applied in credentials/credential-api and kms/kms-core only, and CI runs no coverage task and enforces no threshold. There is no repo-wide number, which is how TW-R10-05 stayed invisible.

Fix: apply Kover in the root subprojects {} block next to ktlint and add a merged report to CI. A threshold can come later; the number is the useful part now.
Low

32,692 lint violations permanently baselined

TW-R10-09

Across 123 config/ktlint/baseline.xml files. The gate on new code works and is the right call — but a baseline this size is indefinite debt, and it grows every time a module is legitimately re-baselined.

Fix: a scheduled ktlintFormat sweep, one domain per release, shrinking the baseline on a known cadence rather than never.
Low

No signing, SBOM, or provenance on published artifacts

TW-R10-10

The build publishes to Maven with no GPG signature, no CycloneDX SBOM, and no build attestation. Dependabot is configured for both Gradle and Actions (good), but there is no CodeQL or other SAST, and CI is a single OS on a single JDK. For a library whose value proposition is verifiable trust, artifact provenance is close to table stakes.

Fix: GPG signing is required for Maven Central anyway — do it now. Add cyclonedx-gradle-plugin and GitHub artifact attestation to the release workflow.
Low

Test dependencies invert the module layering

TW-R10-11

credentials/credential-api declares a testImplementation on :trust, the facade that sits above it, and did/did-core does the same on :did:registrar. Production layering is clean and acyclic; the test configurations quietly are not, which constrains how these modules can be split or published independently.

credentials/credential-api/build.gradle.kts:42 testImplementation(project(":trust")) did/did-core/build.gradle.kts:23 testImplementation(project(":did:registrar"))
Fix: move those tests up into :trust, or push the shared fixtures down into :testkit where the rest of the doubles already live.
TrustWeave SaaS

Findings

Six findings. Nothing I traced was an authorization defect — the pressure here is on schema ownership and on the tooling that is present in the library repo and absent in this one.

High

The default profile lets Hibernate own the schema, with Flyway switched off

SA-R10-01

application.yml sets ddl-auto: update and flyway.enabled: false. The 29 hand-written migrations are therefore never applied on the default path — Hibernate derives the schema from the entities instead. Everything that exists only in SQL is silently absent there: the check constraints in V9__data_integrity_constraints.sql, the index in V24__webhook_recovery_monitor_index.sql, the seed rows in V4__Add_blockchain_network_seed_data.sql.

The team knows: the staging profile carries a comment saying "the default profile's update has been hiding gaps that only a fresh database reveals." Both deployment profiles correctly use validate + Flyway. What is missing is a CI job that boots against a clean Postgres under those settings, so the drift is still only discovered at deploy time.

application.yml:16 ddl-auto: update application.yml:24 flyway.enabled: false application-staging.yml / application-fly.yml ddl-auto: validate, flyway.enabled: true .github/workflows/ci.yml runs :server:test only — no fresh-DB migration boot
Fix: add a CI job that starts a clean Postgres, runs the app with SPRING_PROFILES_ACTIVE=staging, and fails on a validate mismatch. Then flip the default profile to validate + Flyway so dev and prod agree by construction.
Medium

No static analysis on 22k lines of backend Kotlin

SA-R10-02

No ktlint, no detekt, no spotless anywhere in the SaaS build. The library repo enforces ktlint on all 106 modules and gates PRs on it; this repo, which handles tenancy, billing and key custody, has none. The frontend is better served — CI runs lint, tests and a production build.

Fix: apply ktlint with a generated baseline, mirroring the library's setup, and add ktlintCheck to the backend CI job. detekt would add real value here given the amount of authorization logic.
Medium

No dependency update or audit path

SA-R10-03

No .github/dependabot.yml in this repo, and no npm audit step in CI, on a Spring Boot service with 16 frontend runtime dependencies and a Keycloak/Stripe/Kill Bill surface. The library repo has Dependabot for both Gradle and Actions.

Fix: copy the library's dependabot.yml and add npm and Docker ecosystems; add npm audit --audit-level=high to the frontend job.
Medium

Tenant isolation holds by convention, not by construction

SA-R10-04

The security config is properly fail-closed — anyRequest().denyAll() — but /api/** resolves only to .authenticated(). Every one of the 44 controllers must therefore remember to call tenantContext.require* itself. The ones I traced do it correctly, including a post-fetch ownership re-check, and the three dev-only controllers are properly @Profile("dev"). The discipline is real; nothing enforces it.

KeycloakSecurityConfig.kt .requestMatchers("/api/**").authenticated() OrganizationController.kt:39 tenantContext.requireOrganizationAccess(id) ✓ CredentialOfferController.kt:110-114 requireTrustSpaceAccess + offer.organizationId == orgId ✓ DevController / InMemoryTestnetController / DemoWalletVerifierController → @Profile("dev") ✓
Fix: an ArchUnit-style test asserting every @RestController outside an explicit public allowlist references TenantContext. Cheap, and it converts the convention into a gate.
Low

Eleven unused repository methods, several tenant-unscoped

SA-R10-05

Of 63 derived query methods, 11 have no caller outside the repository layer. Four take no organization or trust-space parameter, so a future caller could pick one up without noticing it returns cross-tenant rows. findByIdAndOrganizationId being among the unused ones is the telling detail — the scoped variant exists and is not the one in use.

unscoped + unused: findByTransactionHash, findByVerificationMethodId, findByStatus, findByBlockchainNetworkId also unused: findByIdAndOrganizationId, findByGroupId, findBySubjectIdAndStatus, findByStatusOrderByCreatedAtAsc, 3× countByTrustSpaceId…
Fix: delete them. They can be re-derived in one line when something needs them, and the scoped signature will be the obvious one to write.
Low

Default datasource falls back to postgres/postgres over plaintext

SA-R10-06

jdbc:postgresql://127.0.0.1:5432/trustweave_saas?sslmode=disable with postgres/postgres as the default credentials. Appropriate for local development, and both deployment profiles override it — but it is the same default profile that already carries SA-R10-01, so a deployment that forgets to set a profile fails open twice.

Fix: drop the credential defaults so an unset DATABASE_PASSWORD fails at startup rather than reaching for postgres.
Regression check

Verified closed

Previously-reported findings I re-checked in the source rather than taking on report. All hold.

PKCE is now mandatory for the OID4VCI authorization-code flow, with a helper and an actionable error message. Oidc4VciService.kt:220 · Pkce.kt
did:web resolution is guarded against SSRF via the shared private-network guard. AbstractWebDidMethod.kt:124 · common/net/PrivateNetworkGuard.kt
The Verifiable Intent omit-payment-mandate bypass fails closed, with the reasoning recorded in a comment at the check. ChainVerifier.kt · "L3 payment presented without its authorizing L2 payment mandate"
The JUnit silent-skip trap stayed fixed: 1,241 of 1,265 expression-body test functions use runBlocking<Unit>. The 24 exceptions are @BeforeEach fixtures, private helpers, one commented-out test, and vendored node_modules — none are live @Test methods. repo-wide, 3,975 @Test methods
No hardcoded credentials and no TLS verification bypass in either repository. Every password/secret/apiKey string literal match is inside a KDoc usage example. both repos, main source
The SaaS public verification endpoint reads revocation from the published status list rather than the local mirror column, so an independent verifier reaches the same decision. Rate-limited, and the mirror is consulted only for the reason. PublicVerificationController.kt:60-90
The SaaS CI pins the reviewed library revision and verifies its source hash before building — a genuinely good supply-chain control that the library repo does not itself have an equivalent of. .github/workflows/ci.yml · .trustweave-revision, .trustweave-source-sha256
Scoring

How the numbers were reached

Dimension SDK SaaS What moved it
Security 7.5 8.0 SDK: strong crypto hygiene and fail-closed verification, less two HIGH authorization gaps in the VI plugin. SaaS: fail-closed routing, consistent tenant scoping, profile-gated dev surface.
Correctness 8.0 7.0 SDK: fail-closed by default throughout; deducted for silent temporal passes and unlogged swallows. SaaS: careful reasoning in the code, deducted for split schema authority.
Architecture 7.0 7.0 SDK: clean acyclic domain layering and a real SPI plugin system; no API-surface control, inverted test deps. SaaS: conventional Spring layering, isolation by convention.
Testing 6.5 7.0 SDK: 3,975 tests with genuinely adversarial cases, against 21 modules with none and coverage measured on 3 of 106. SaaS: 541 backend tests, 63 frontend test files; no migration-boot test.
Supply chain & CI 6.0 5.0 SDK: ktlint gated and Dependabot on, but no signing, SBOM, SAST or coverage gate. SaaS: no static analysis, no Dependabot, no audit — offset by the library source-pinning.
Overall 7.0 6.9 Equal weight across the five dimensions.
Sequence

What I would do first

  1. TW-R10-01 — run the constraint check on the L3 checkout side. It is a contained change in one file and it closes an unbounded-authority path in the payment plugin.
  2. TW-R10-02 — make audience and nonce required, or gate them behind a flag that defaults to on. Same file, and it is the difference between a replayable presentation and one that is not.
  3. SA-R10-01 — add the fresh-database migration boot to CI. This is the one that turns a class of deploy-time surprises into PR-time failures.
  4. TW-R10-06 — apply the binary-compatibility validator and commit the .api dumps before 1.0, while the surface is still cheap to change.
  5. SA-R10-02 and TW-R10-05 — ktlint on the SaaS backend, and one shared DID-method conformance suite covering nine untested plugins at once.