-
Notifications
You must be signed in to change notification settings - Fork 5
fix: local dev qol #466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: local dev qol #466
Conversation
WalkthroughAdds utility scripts (DB multi-create and eid-wallet token fetch), a Neo4j PID-cleanup wrapper in dev Docker compose, extra logging in eid-wallet onboarding, TypeScript/type-only import refinements and stricter schemas in evault-core HTTP routes, and small formatting/logging tweaks in registry services. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Evault as Evault HTTP Server
participant Prov as ProvisioningService
participant DB as DbService
Client->>Evault: POST /provision {registryEntropy, namespace, verificationId, publicKey}
note right of Evault `#DDEBF7`: body validated by schema (type-only imports)
Evault->>Prov: provisionEVault(ProvisionRequest)
alt success
Prov-->>Evault: result
Evault-->>Client: 200 {result}
else failure
Prov-->>Evault: error
Evault-->>Client: 500 {error}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
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: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
197-213: Gate new debug logs behind a dev flag to avoid leaking sensitive dataYou’re logging
registryEntropyand the full provision response, which is great for local debugging but exposes entropy tokens and identifiers in the browser console if this ships to production. Consider guarding these logs with a dev check:- const registryEntropy = entropyRes.data.token; - console.log("Registry entropy:", registryEntropy); + const registryEntropy = entropyRes.data.token; + if (import.meta.env.DEV) { + console.log("Registry entropy:", registryEntropy); + } @@ - ); - console.log("Provision response:", provisionRes.data); + ); + if (import.meta.env.DEV) { + console.log("Provision response:", provisionRes.data); + }scripts/get-eid-wallet-token.sh (1)
3-45: Normalize REGISTRY_URL to avoid double slashes in request pathIf
PUBLIC_REGISTRY_URLis configured with a trailing/(e.g.https://registry.local/), the current concatenation will hit//platforms/certification, which some servers treat as a different path. You can harden this by stripping a trailing slash once:-# Get the registry URL from environment or use default -REGISTRY_URL="${PUBLIC_REGISTRY_URL:-http://localhost:4321}" +# Get the registry URL from environment or use default, strip trailing slash +REGISTRY_URL="${PUBLIC_REGISTRY_URL:-http://localhost:4321}" +REGISTRY_URL="${REGISTRY_URL%/}"Everything else in this helper (curl error handling plus jq/grep fallback) looks good for local dev tooling. Based on learnings, this also avoids reintroducing the deprecated
PUBLIC_PLATFORM_URLpattern and sticks toPUBLIC_REGISTRY_URL.infrastructure/evault-core/src/core/http/server.ts (1)
295-363: Clarify token–eName binding in/public-keyand reusevalidateTokenaccordinglyThe
validateTokenhelper correctly verifies a Bearer JWT against the registry’s JWKS and returns its payload, but in the/public-keyhandler the payload is only checked for existence:const tokenPayload = await validateToken(...); if (!tokenPayload) { return reply.status(401).send({ error: "Invalid or missing authentication token" }); }There is no further check that the token is actually authorized to update the specific
eNamefromX-ENAME(for example by matching asub,ename,platform, or scope claim intokenPayload).If your security model expects tokens to be bound to a particular eName or platform, you likely want an additional check here (and a 403 if the token’s claims don’t authorize updating this eName). If, instead, any valid registry‑issued token is intentionally allowed to set any eName’s public key, the current behavior is fine but should be documented.
Also applies to: 423-464, 509-516
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/Project.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/codeStyleConfig.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/kotlinc.xmlis excluded by!**/gen/**
📒 Files selected for processing (8)
db/init-multiple-databases.sh(1 hunks)dev-docker-compose.yaml(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte(2 hunks)infrastructure/evault-core/src/core/http/server.ts(3 hunks)platforms/registry/src/index.ts(5 hunks)platforms/registry/src/services/HealthCheckService.ts(4 hunks)platforms/registry/src/services/VaultService.ts(1 hunks)scripts/get-eid-wallet-token.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-13T10:34:52.527Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 415
File: infrastructure/eid-wallet/src/env.d.ts:8-8
Timestamp: 2025-11-13T10:34:52.527Z
Learning: In infrastructure/eid-wallet, PUBLIC_PLATFORM_URL should not be added to .env.example or configured as a static environment variable. The platform URL is extracted dynamically through URI parsing according to the protocol specification, and all fallbacks for platform URL are being removed.
Applied to files:
scripts/get-eid-wallet-token.sh
⏰ 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: test-web3-adapter-integration
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (5)
platforms/registry/src/services/HealthCheckService.ts (1)
1-87: LGTM! Formatting and code quality improvements with no behavioral changes.The refactoring improves code consistency and readability:
- Quote style standardization across imports and string literals
- Multi-line formatting for complex expressions (regex, error logging) enhances readability
Number.parseInton line 84 is a good practice over globalparseIntAll logic remains unchanged and the improvements align with the PR's quality-of-life objective.
platforms/registry/src/services/VaultService.ts (1)
1-32: VaultService CRUD methods look correct and type usage is consistentThe switch to type-only imports and the repository-backed create/find/update/delete methods are all correct and side‑effect free. No functional or type issues spotted here.
db/init-multiple-databases.sh (1)
1-39: Multi-database init script is sensible and idempotentThe script cleanly handles an empty
POSTGRES_MULTIPLE_DATABASES, trims entries, checks existence viapg_database, and only creates missing DBs. This is a solid fit for dev containers with the official Postgres entrypoint.dev-docker-compose.yaml (1)
112-141: Neo4j PID cleanup and Postgres multi-DB wiring look correct for local dev
- The Neo4j override (
entrypoint: ["/bin/sh", "-c"]with a small shell prelude that removes stale*.pidfiles and thenexec /startup/docker-entrypoint.sh neo4j) follows the pattern recommended in Neo4j’s own Docker docs and should resolve “stuck” dev containers due to leftover PIDs. (neo4j.com)- Mounting
./db/init-multiple-databases.shinto/docker-entrypoint-initdb.dtogether withPOSTGRES_MULTIPLE_DATABASESis correct; just remember that these init scripts only run when thepostgres_datavolume is first created. If you later change the database list, you’ll need to recreate that volume for the script to run again.Also applies to: 158-164
platforms/registry/src/index.ts (1)
12-20: ---Use Fastify's structured logger instead of
console.logfor production loggingThe added
console.logstatements bypass Fastify's built-in Pino logger and produce unstructured output incompatible with log aggregation. Per Fastify best practices, replace with:
request.log.info("message")inside route handlers (includes automatic request context), orfastify.log.info("message")for server-level eventsThis ensures consistent, structured logging. (Or wrap in a dev-only check if you want it only in dev environments.)
Also applies to: 41-51, 109-119
| // Whois endpoint - returns both W3ID identifier and public key | ||
| server.get( | ||
| "/whois", | ||
| { | ||
| schema: { | ||
| tags: ["identity"], | ||
| description: "Get eVault W3ID identifier and public key", | ||
| headers: { | ||
| type: "object", | ||
| required: ["X-ENAME"], | ||
| properties: { | ||
| "X-ENAME": { type: "string" }, | ||
| }, | ||
| }, | ||
| response: { | ||
| 200: { | ||
| type: "object", | ||
| properties: { | ||
| w3id: { type: "string" }, | ||
| publicKey: { type: "string", nullable: true }, | ||
| }, | ||
| }, | ||
| 400: { | ||
| type: "object", | ||
| properties: { | ||
| error: { type: "string" }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Check Fastify version and find the server file
cd infrastructure/evault-core 2>/dev/null || cd .
find . -name "package.json" -type f | head -5 | xargs grep -l "fastify" 2>/dev/null | head -3Repository: MetaState-Prototype-Project/prototype
<shell_result>
Repository: MetaState-Prototype-Project/prototype
Exit code: 0
stdout:
./package.json
</shell_result>
Script executed:
#!/bin/bash
# Check Fastify version
cat infrastructure/evault-core/package.json 2>/dev/null | grep -A2 -B2 "fastify" | head -20Repository: MetaState-Prototype-Project/prototype
<shell_result>
Repository: MetaState-Prototype-Project/prototype
Exit code: 0
stdout:
},
"dependencies": {
"@fastify/cors": "^8.5.0",
"@fastify/formbody": "^8.0.2",
"@fastify/swagger": "^8.14.0",
"@fastify/swagger-ui": "^3.0.0",
"axios": "^1.6.7",
"cors": "^2.8.5",
"dotenv": "^16.4.5",
"express": "^4.18.2",
"fastify": "^4.26.2",
"graphql": "^16.10.0",
"graphql-type-json": "^0.3.2",
</shell_result>
Script executed:
#!/bin/bash
# View the actual server.ts file to see the current state
cat -n infrastructure/evault-core/src/core/http/server.ts | sed -n '1,100p'Repository: MetaState-Prototype-Project/prototype
<shell_result>
Repository: MetaState-Prototype-Project/prototype
Exit code: 0
stdout:
1 import swagger from "@fastify/swagger";
2 import swaggerUi from "@fastify/swagger-ui";
3 import axios from "axios";
4 import fastify, { type FastifyInstance } from "fastify";
5 import * as jose from "jose";
6 import type {
7 ProvisionRequest,
8 ProvisioningService,
9 } from "../../services/ProvisioningService";
10 import type { DbService } from "../db/db.service";
11 import { type TypedReply, type TypedRequest, WatcherRequest } from "./types";
12
13 interface WatcherSignatureRequest {
14 w3id: string;
15 logEntryId: string;
16 proof: {
17 signature: string;
18 alg: string;
19 kid: string;
20 };
21 }
22
23 export async function registerHttpRoutes(
24 server: FastifyInstance,
25 evault: any, // EVault instance to access publicKey
26 provisioningService?: ProvisioningService,
27 dbService?: DbService,
28 ): Promise<void> {
29 // Register Swagger
30 await server.register(swagger, {
31 swagger: {
32 info: {
33 title: "eVault Core API",
34 description: "API documentation for eVault Core HTTP endpoints",
35 version: "1.0.0",
36 },
37 tags: [
38 { name: "identity", description: "Identity related endpoints" },
39 {
40 name: "watchers",
41 description: "Watcher signature related endpoints",
42 },
43 {
44 name: "provisioning",
45 description: "eVault provisioning endpoints",
46 },
47 ],
48 },
49 });
50
51 await server.register(swaggerUi, {
52 routePrefix: "/docs",
53 });
54
55 // Whois endpoint - returns both W3ID identifier and public key
56 server.get(
57 "/whois",
58 {
59 schema: {
60 tags: ["identity"],
61 description: "Get eVault W3ID identifier and public key",
62 headers: {
63 type: "object",
64 required: ["X-ENAME"],
65 properties: {
66 "X-ENAME": { type: "string" },
67 },
68 },
69 response: {
70 200: {
71 type: "object",
72 properties: {
73 w3id: { type: "string" },
74 publicKey: { type: "string", nullable: true },
75 },
76 },
77 400: {
78 type: "object",
79 properties: {
80 error: { type: "string" },
81 },
82 },
83 },
84 },
85 },
86 async (request: TypedRequest<{}>, reply: TypedReply) => {
87 const eName =
88 request.headers["x-ename"] || request.headers["X-ENAME"];
89
90 if (!eName || typeof eName !== "string") {
91 return reply
92 .status(400)
93 .send({ error: "X-ENAME header is required" });
94 }
95
96 // Get public key from database if dbService is available
97 let publicKey: string | null = null;
98 if (dbService) {
99 try {
100 publicKey = await dbService.getPublicKey(eName);
</shell_result>
Fastify header validation case sensitivity JSON schema v5
<web_search_result>
Short answer: HTTP header names are case‑insensitive (RFC); Node lowercases header names on incoming messages; Fastify exposes those lowercased names on request.headers and therefore your JSON‑Schema (headers) should use lowercase property names. [1][2][3]
Sources
- RFC 7230 (header field names are case‑insensitive). [3]
- Fastify Request / Validation docs (Fastify uses request.headers and schema-based validation). [1]
- Node.js HTTP docs (IncomingMessage/headers keys are lower-cased). [2]
</web_search_result>
Script executed:
#!/bin/bash
# View the /public-key and other endpoints mentioned in "Also applies to: 370-407"
cat -n infrastructure/evault-core/src/core/http/server.ts | sed -n '350,420p'Repository: MetaState-Prototype-Project/prototype
<shell_result>
Repository: MetaState-Prototype-Project/prototype
Exit code: 0
stdout:
350 if (error.code) {
351 console.error(`Token validation error code: ${error.code}`);
352 }
353 if (error.response) {
354 console.error(
355 `Token validation HTTP error: ${error.response.status} - ${error.response.statusText}`,
356 );
357 }
358 if (error.cause) {
359 console.error(`Token validation error cause:`, error.cause);
360 }
361 return null;
362 }
363 }
364
365 // PATCH endpoint to save public key
366 server.patch<{ Body: { publicKey: string } }>(
367 "/public-key",
368 {
369 schema: {
370 tags: ["identity"],
371 description: "Save public key for a user's eName",
372 headers: {
373 type: "object",
374 required: ["X-ENAME", "Authorization"],
375 properties: {
376 "X-ENAME": { type: "string" },
377 Authorization: { type: "string" },
378 },
379 },
380 body: {
381 type: "object",
382 required: ["publicKey"],
383 properties: {
384 publicKey: { type: "string" },
385 },
386 },
387 response: {
388 200: {
389 type: "object",
390 properties: {
391 success: { type: "boolean" },
392 message: { type: "string" },
393 },
394 },
395 400: {
396 type: "object",
397 properties: {
398 error: { type: "string" },
399 },
400 },
401 401: {
402 type: "object",
403 properties: {
404 error: { type: "string" },
405 },
406 },
407 },
408 },
409 },
410 async (
411 request: TypedRequest<{ publicKey: string }>,
412 reply: TypedReply,
413 ) => {
414 const eName =
415 request.headers["x-ename"] || request.headers["X-ENAME"];
416
417 if (!eName || typeof eName !== "string") {
418 return reply
419 .status(400)
420 .send({ error: "X-ENAME header is required" });
</shell_result>
Fix header schema casing to match Fastify's lower‑cased header keys
Fastify normalizes incoming header names to lower case, and JSON-schema validation must use lowercase property names to match. In your schemas at lines 64-66 and 374-377, you currently require mixed‑case headers:
/whois(line 64-66): requires"X-ENAME"/public-key(line 374-377): requires"X-ENAME"and"Authorization"
Even if clients send X-ENAME or Authorization, Fastify's validator will reject the request with a 400 error because by the time validation runs, the header object only contains x-ename and authorization. The handler's fallback code checking both cases (lines 88 and 415) will never execute.
Update the schemas to use lowercase keys:
// /whois (lines 62-68)
headers: {
type: "object",
- required: ["X-ENAME"],
+ required: ["x-ename"],
properties: {
- "X-ENAME": { type: "string" },
+ "x-ename": { type: "string" },
},
}, // /public-key (lines 372-379)
headers: {
type: "object",
- required: ["X-ENAME", "Authorization"],
+ required: ["x-ename", "authorization"],
properties: {
- "X-ENAME": { type: "string" },
- Authorization: { type: "string" },
+ "x-ename": { type: "string" },
+ authorization: { type: "string" },
},
},The handler code that reads request.headers["x-ename"] and request.headers.authorization can remain unchanged.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in infrastructure/evault-core/src/core/http/server.ts around lines 55-84 (whois
schema) and around lines 370-380 (public-key schema), the JSON-schema header
property names use mixed-case ("X-ENAME", "Authorization") but Fastify
lowercases headers for validation; change the schema header property keys and
any entries in the required arrays to lowercase ("x-ename" and "authorization"
as applicable) so the validator matches incoming requests; leave the handler
code that reads request.headers["x-ename"] and request.headers.authorization
unchanged.
Description of change
adds missing script for postgres dev docker container
adds a script to get the wallet token
fixes some biome errors and runs formatter
Issue Number
n/a
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.