🔍 Deep Repository Review: 5 Critical, 7 High, 18 Medium findings

🔍 Deep Review: microsoft/multiclouddb-sdk-for-java-samples

Date: 2026-04-27
Repo: https://github.com/microsoft/multiclouddb-sdk-for-java-samples
Reviewer: Copilot deep-review pipeline (3 parallel agents)

---

📊 Aggregate Finding Counts

Severity	TODO App	Risk Platform	Security	Total
🔴 CRITICAL	3	2	0	5
🟠 HIGH	3	4	0	7
🟡 MEDIUM	8	5	5	18
🟢 LOW	7	3	5	15
Total	21	14	10	45

---

🚨 CRITICAL Findings

C1. TodoApp: No client shutdown — resource leak

File: TodoApp.java

The MulticloudDbClient is created but never closed. No shutdown hook exists. Underlying Cosmos/Dynamo/Spanner connections leak on Ctrl+C.

Fix: Add Runtime.getRuntime().addShutdownHook() that calls client.close().

C2. TodoApp: No request body size limit — DoS

File: TodoApp.java

MAPPER.readTree(exchange.getRequestBody()) reads entire stream with no size cap. Attacker can OOM the JVM.

Fix: Add body length check before parsing.

C3. TodoApp: Path traversal in static file handler

File: TodoApp.java — handleStatic()

URL path passed directly to getResourceAsStream(). Request like GET /static/../logback.xml could serve arbitrary classpath resources. getResourceAsStream does NOT normalize .. segments.

Fix: Normalize path, reject .. segments, whitelist extensions.

C4. Risk Platform: No Authentication or Authorization

File: RiskPlatformApp.java

Entire REST API is unauthenticated. Any client can read/write any tenant's data via ?tenant=<any-id>. Zero auth, zero tenant ownership checks. Developers copying this pattern into production would have a critical vulnerability.

Fix: Add prominent WARNING in code/README about missing auth.

C5. Risk Platform: XSS in Dashboard HTML

File: index.html (riskplatform)

User data rendered via template literals into innerHTML without sanitization. Portfolio name, alert message, symbol all injected raw. A portfolio named <img src=x onerror=alert(1)> executes JS.

Fix: Use textContent or sanitize helper before innerHTML insertion.

---

🟠 HIGH Findings

H1. TodoApp: XSS via onclick handlers in index.html

escHtml() escapes <>&" but NOT single quotes. IDs inside single-quoted JS onclick attrs can break out. Stored XSS risk.

H2. TodoApp: CORS Access-Control-Allow-Origin: *

Wildcarding all origins on data-mutating API. Should warn this is not production-safe.

H3. TodoApp: ConfigLoader silently swallows IOException

If config file is malformed, returns empty config silently. Only treat null stream as optional.

H4. Risk Platform: deletePosition Missing portfolio Query Param

Client JS sends DELETE /api/positions/{id}?tenant=X but server requires &portfolio=Y. Every position delete returns 400.

H5. Risk Platform: deleteAlert Missing portfolio Query Param

Same bug as H4. Every alert delete returns 400.

H6. Risk Platform: N+1 Query Pattern in Portfolio Listing

For N portfolios, executes N+1 queries (1 list + N position lookups). Causes latency and RU cost at scale.

H7. Risk Platform: Dashboard Fires 5+ Serial Queries

5 serial DB queries per dashboard load. No parallelism, no caching.

---

🟡 MEDIUM Findings (18 total)

TODO App (8)

• M1: Duplicates ConfigLoader logic instead of reusing it
• M2: Spanner config has inconsistent database naming (testdb vs todoapp)
• M3: listTodos() fetches only first page, no pagination
• M4: PortableCrudQuerySample pagination demo is incomplete (prints hasMore but never fetches next page)
• M5: Error responses leak internal exception messages and SDK categories
• M6: pom.xml URL points to personal fork (allenkim0129) not microsoft org
• M7: Cleanup scripts reference Risk Platform databases, no TODO app cleanup exists
• M8: HttpExchange not always closed on error paths

Risk Platform (5)

• M9: No pagination — all queries read only first page (200 items max)
• M10: Hardcoded tenant database names in ResourceProvisioner — no dynamic onboarding
• M11: Query parameters not URL-decoded
• M12: Cosmos emulator key in properties should have comment explaining it is well-known
• M13: CORS allows all origins

Security Cross-Cutting (5)

• M14: HTTP servers bind to 0.0.0.0 (all interfaces) not localhost
• M15: CORS Access-Control-Allow-Origin: * on both apps
• M16: No request body size limits — readAllBytes() with no cap
• M17: Error messages expose internal SDK details
• M18: MulticloudDbException category leaked to client responses

---

🟢 LOW Findings (15 total, selected)

• No Content-Security-Policy header on HTML pages
• TodoApp server.setExecutor(null) uses single-thread executor
• No input validation on document IDs (length, characters)
• No rate limiting on either HTTP server
• No authentication (expected for demo)
• Naive query parameter parsing without URL decoding
• Accessibility gaps in TODO UI (no ARIA roles, no screen reader support)
• Inconsistent system property names (todo.config vs multiclouddb.config)
• Risk Platform main file at 32KB/600 lines (acceptable for sample)
• Dead code: unused pad() method in RiskPlatformApp

---

✅ What's Done Well

Area	Assessment
Credential Management	Emulator-only keys committed; cloud values gitignored with .template files
SDK Usage	Correct use of MulticloudDbClient, Key, ResourceAddress, QueryRequest
Multi-Tenant Model	Database-per-tenant via ResourceAddress — strongest isolation
Partition Key Design	Positions grouped by portfolio — optimal for access patterns
Provider Portability	Same code runs on Cosmos, DynamoDB, Spanner via config swap
Schema Provisioning	provisionSchema() correctly used for multi-DB setup
Portable Queries	Good demonstration of expression DSL with parameters
Capabilities API	Displayed in both TODO and Risk Platform dashboards
Cleanup Scripts	Named targets, preview before delete, interactive confirmation
Dependencies	All recent versions, no known critical CVEs

---

SDK Feature Demonstration Coverage

Feature	Demonstrated	Quality
CRUD (create/read/update/delete/upsert)	✅ Yes	Good
Portable query DSL with parameters	✅ Yes	Good
Native query escape hatch	✅ Yes	Good
provisionSchema()	✅ Yes	Good
Capabilities API	✅ Yes	Good
ResourceAddress per-tenant routing	✅ Yes	Excellent
QueryRequest.partitionKey() scoping	✅ Yes	Good
Provider switching via config	✅ Yes	Excellent
Pagination with continuation tokens	❌ No	Printed but never followed
Error handling (category-specific)	⚠️ Partial	Caught but not branched on
Retry/backoff patterns	❌ No	Not demonstrated
Batch operations	❌ No	Not demonstrated
TTL / metadata	❌ No	Not demonstrated
ORDER BY / limit	❌ No	Not demonstrated

---

🎯 Top Priority Actions

#	Action
1	Fix path traversal in TodoApp static handler
2	Fix XSS in both dashboards — sanitize user data
3	Add shutdown hook to close MulticloudDbClient
4	Fix broken delete buttons in Risk Platform (add &portfolio= param)
5	Add request body size limits to both servers

This issue aggregates findings across TodoApp.java, RiskPlatformApp.java, riskplatform/index.html, and related configuration and client files. Start by splitting one finding into a separate issue, then inspect the named file and its relevant request or rendering path. Done should be defined per finding with a focused change and a test or reproducible check, rather than attempting all 45 items together.