Skip to content

Conversation

@coodos
Copy link
Contributor

@coodos coodos commented Nov 24, 2025

Description of change

Issue Number

closes #454

Type of change

  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • New Features

    • Added signature verification library supporting eVault public key validation
    • Introduced public key synchronization during login with error resilience
    • Enabled CORS support for cross-origin requests with relaxed security settings
  • Enhancements

    • Improved token validation with fallback URL resolution and enhanced logging
    • Added test token generation utility for development
  • Documentation

    • Added comprehensive README for the signature validator library

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The PR introduces public key synchronization for eVault after login, creates a new signature validation library, adds CORS support to the evault-core server, and enhances token validation with fallback URL resolution. It includes a test token generation utility and makes VaultController's public key sync method public with updated logic.

Changes

Cohort / File(s) Summary
Environment configuration
infrastructure/eid-wallet/src/env.d.ts
Added PUBLIC_EID_WALLET_TOKEN environment variable export
VaultController updates
infrastructure/eid-wallet/src/lib/global/controllers/evault.ts
Made syncPublicKey method public; updated to check existing keys via /whois endpoint, retrieve keys using fixed KEY_ID ("default"), derive token from PUBLIC_EID_WALLET_TOKEN, and patch public keys via PATCH /public-key
State initialization
infrastructure/eid-wallet/src/lib/global/state.ts
Reformatted VaultController constructor call to multi-line format
Login flow integration
infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte
Added non-blocking calls to syncPublicKey after PIN and biometric health checks with dedicated error handling
Test token generation
infrastructure/evault-core/generate-test-token.js
New Node.js script for building and signing test JWTs using provided JWK with 1-year validity
CORS and dependencies
infrastructure/evault-core/package.json
Added @fastify/cors@^8.5.0 dependency
CORS plugin registration
infrastructure/evault-core/src/index.ts
Registered @fastify/cors plugin with relaxed settings (GET/POST/PUT/PATCH/DELETE/OPTIONS, Content-Type/Authorization/X-ENAME headers)
Token validation enhancement
infrastructure/evault-core/src/core/http/server.ts
Enhanced JWKS resolution to use REGISTRY_URL or PUBLIC_REGISTRY_URL fallback; added detailed logging for validation header, JWKS fetch results, JWT payload, and error details
Signature validator module
infrastructure/signature-validator/package.json, src/index.ts, tsconfig.json, README.md, .gitignore
New TypeScript library providing verifySignature function for ECDSA-P256 signature verification against eVault public keys, with types VerifySignatureOptions and VerifySignatureResult
Minor formatting
platforms/blabsy/src/components/common/seo.tsx, platforms/blabsy/src/lib/context/theme-context.tsx
Parameter list reformatting and whitespace adjustments

Sequence Diagrams

sequenceDiagram
    participant User
    participant LoginPage as Login Page<br/>+page.svelte
    participant VC as VaultController
    participant eVault as eVault API
    participant Registry as Registry
    
    User->>LoginPage: Enter PIN/Biometric
    LoginPage->>VC: Health check
    VC-->>LoginPage: Health OK
    
    rect rgb(200, 220, 255)
        Note over LoginPage,eVault: New: Non-blocking public key sync
        LoginPage->>VC: syncPublicKey(ename)
        par
            VC->>Registry: GET /resolve?w3id=ename
            Registry-->>VC: vault URI
            VC->>eVault: GET /whois
            eVault-->>VC: Existing key (or none)
            alt Key exists
                VC->>VC: Mark as saved
            else Key not found
                VC->>VC: Retrieve via KeyService<br/>(KEY_ID: "default")
                VC->>eVault: PATCH /public-key<br/>(with derived token)
                eVault-->>VC: Key patched
                VC->>VC: Mark as saved
            end
        and
            LoginPage->>LoginPage: Continue to main flow
        end
        VC-->>LoginPage: Sync complete (or error ignored)
    end
    
    LoginPage->>User: Login successful
Loading
sequenceDiagram
    participant Client
    participant SigValidator as Signature Validator<br/>verifySignature()
    participant Registry as Registry
    participant eVault as eVault API
    participant WebCrypto as Web Crypto API
    
    Client->>SigValidator: verifySignature({eName, signature, payload, registryBaseUrl})
    SigValidator->>Registry: GET /resolve?w3id=eName
    Registry-->>SigValidator: vault URL
    SigValidator->>eVault: GET /whois
    eVault-->>SigValidator: Public key (multibase)
    SigValidator->>SigValidator: Decode multibase key
    SigValidator->>SigValidator: Decode signature (base58btc/base64)
    SigValidator->>WebCrypto: importKey (ECDSA, P-256)
    WebCrypto-->>SigValidator: CryptoKey
    SigValidator->>WebCrypto: verify (SHA-256)
    WebCrypto-->>SigValidator: boolean
    SigValidator-->>Client: VerifySignatureResult {valid, publicKey?, error?}
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • VaultController syncPublicKey refactor (evault.ts): Logic now includes WHOIS checks, conditional key retrieval, and token derivation—requires careful review of the flow and token handling strategy.
  • Login integration (+page.svelte): Non-blocking async calls add complexity to error handling; verify that error suppression does not mask critical issues.
  • Signature validator module (signature-validator/*): New module with Web Crypto integration, multibase decoding, and error handling paths—review type safety, error messages, and verification correctness.
  • Token validation enhancement (server.ts): Fallback URL resolution and expanded logging require verification that REGISTRY_URL and PUBLIC_REGISTRY_URL precedence is correct.

Possibly related PRs

  • feat: health check #437: Modifies evault controller and login flow (evault.ts and +page.svelte) with complementary changes—one adds health-check API, the other exposes/syncs public keys and calls that sync from login.
  • fix: eid wallet link #403: Modifies the same VaultController file (evault.ts); retrieved PR tweaks timeout/backoff while this PR changes syncPublicKey visibility and logic.

Suggested labels

evault-refactor

Suggested reviewers

  • sosweetham
  • ananyayaya129

Poem

🐰 A token hops through vault doors wide,
Keys sync as login's gentle guide.
New validators hop about,
Verifying signatures—without a doubt!
CORS blooms, the server's blessed,
This refactor puts eVault to the test!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'Feat/signature verification library' is directly related to the main objective: creating a signature verification library to abstract validation logic, matching the linked issue #454.
Description check ✅ Passed The PR description follows the required template with all major sections present. However, the 'How the change has been tested' section is empty, and the checklist items are marked complete despite the description stating they are 'not marked complete'.
Linked Issues check ✅ Passed The PR successfully implements the core objective from issue #454 by creating a signature-validator library that abstracts signature validation logic, enabling reuse across the codebase rather than manual validation repetition.
Out of Scope Changes check ✅ Passed Beyond the signature-validator library, the PR includes supporting infrastructure changes (eVault token handling, CORS, key sync) that enable the library to function properly within the system architecture.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coodos coodos marked this pull request as ready for review November 24, 2025 20:32
@coodos coodos force-pushed the feat/signature-verification-library branch from 47e640c to 56aceac Compare November 24, 2025 20:33
@coodos coodos merged commit 7d94cea into main Nov 24, 2025
3 of 5 checks passed
@coodos coodos deleted the feat/signature-verification-library branch November 24, 2025 20:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (6)
infrastructure/signature-validator/src/index.ts (3)

102-130: Consider making timeout configurable and adding retry logic.

The hardcoded 10-second timeout and lack of retry logic may cause issues with transient network failures. Consider:

  1. Making the timeout configurable via options
  2. Adding retry logic with exponential backoff for network requests

194-208: Remove unnecessary Uint8Array copy.

Line 197 creates an unnecessary copy since publicKeyBytes is already a Uint8Array. Use .buffer directly.

-    const publicKeyBuffer = new Uint8Array(publicKeyBytes).buffer;
+    const publicKeyBuffer = publicKeyBytes.buffer;
     
     const publicKey = await crypto.subtle.importKey(

210-228: Remove unnecessary Uint8Array copy.

Line 217 creates an unnecessary copy since signatureBytes is already a Uint8Array. Use .buffer directly.

     const payloadBuffer = new TextEncoder().encode(payload);
 
-    // Create a new ArrayBuffer from the signature Uint8Array
-    const signatureBuffer = new Uint8Array(signatureBytes).buffer;
+    const signatureBuffer = signatureBytes.buffer;
 
     // Verify the signature
infrastructure/evault-core/src/index.ts (1)

24-24: Fastify CORS configuration is very permissive; consider tightening it

origin: true combined with credentials: true effectively allows credentialed requests from any origin, which is risky for an auth/token-bearing service. It’s safer to restrict origins to known frontends and only enable credentials if you truly rely on cookies.

For example, you could externalize allowed origins and drop credentials by default:

-    // Register CORS plugin with relaxed settings
-    await fastifyServer.register(fastifyCors, {
-        origin: true, // Allow all origins
-        methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
-        allowedHeaders: ["Content-Type", "Authorization", "X-ENAME"],
-        credentials: true,
-    });
+    // Register CORS plugin with restricted settings (adjust to your deployment)
+    await fastifyServer.register(fastifyCors, {
+        // e.g. comma‑separated list in env, or a single known origin
+        origin: process.env.EVAULT_ALLOWED_ORIGINS
+            ? process.env.EVAULT_ALLOWED_ORIGINS.split(",")
+            : false, // disable CORS if not configured
+        methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
+        allowedHeaders: ["Content-Type", "Authorization", "X-ENAME"],
+        credentials: false, // set to true only if you depend on cookies
+    });

Also applies to: 120-126

infrastructure/evault-core/src/core/http/server.ts (1)

289-334: Token validation relies solely on signature; consider claim checks and JWKS caching

The improved logging, timeout, and REGISTRY_URL/PUBLIC_REGISTRY_URL fallback are good, but:

  • validateToken currently only verifies the JWT against the registry JWKS and returns the payload.
  • /public-key accepts any non-null payload, so any registry-signed token can update a public key, regardless of its intended audience/purpose.
  • JWKS is fetched on every call, which can add latency and load to the registry.

Consider:

  1. Enforcing semantic checks on the payload (either here or in the /public-key handler), e.g.:
-      const { payload } = await jose.jwtVerify(token, JWKS);
-      
-      console.log(`Token validation: Token verified successfully, payload:`, payload);
-      return payload;
+      const { payload } = await jose.jwtVerify(token, JWKS);
+
+      // Example: enforce expected claims (adjust to your contract)
+      if (payload.purpose !== "public-key-sync") {
+        console.error("Token validation: unexpected purpose claim:", payload.purpose);
+        return null;
+      }
+
+      console.log(`Token validation: Token verified successfully, payload:`, payload);
+      return payload;

…and/or checking iss, aud, and any binding you expect between the token and the eName being updated.

  1. Caching the JWKS (and the resulting createLocalJWKSet instance) with a reasonable TTL instead of re-fetching it on every validation.

These aren’t blocking but will materially strengthen the security and performance of this endpoint.

infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (1)

101-110: Consider extracting duplicate sync logic to a helper function.

The public key sync logic is duplicated between the PIN and biometric authentication flows (also appears at lines 183-192). Consider extracting this into a helper function to improve maintainability.

Apply this refactor:

+async function syncPublicKeyNonBlocking(
+    globalState: GlobalState,
+    vault: { ename: string }
+) {
+    try {
+        await globalState.vaultController.syncPublicKey(vault.ename);
+    } catch (error) {
+        console.error("Error syncing public key:", error);
+        // Continue to app even if sync fails - non-blocking
+    }
+}
+
 // In PIN flow (line 101):
-                    // Sync public key to eVault core
-                    try {
-                        await globalState.vaultController.syncPublicKey(
-                            vault.ename,
-                        );
-                    } catch (error) {
-                        console.error("Error syncing public key:", error);
-                        // Continue to app even if sync fails - non-blocking
-                    }
+                    await syncPublicKeyNonBlocking(globalState, vault);

And similarly for the biometric flow at lines 183-192.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 022cfa3 and 56aceac.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • infrastructure/eid-wallet/src/env.d.ts (1 hunks)
  • infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (5 hunks)
  • infrastructure/eid-wallet/src/lib/global/state.ts (1 hunks)
  • infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (2 hunks)
  • infrastructure/evault-core/generate-test-token.js (1 hunks)
  • infrastructure/evault-core/package.json (1 hunks)
  • infrastructure/evault-core/src/core/http/server.ts (1 hunks)
  • infrastructure/evault-core/src/index.ts (2 hunks)
  • infrastructure/signature-validator/.gitignore (1 hunks)
  • infrastructure/signature-validator/README.md (1 hunks)
  • infrastructure/signature-validator/package.json (1 hunks)
  • infrastructure/signature-validator/src/index.ts (1 hunks)
  • infrastructure/signature-validator/tsconfig.json (1 hunks)
  • platforms/blabsy/src/components/common/seo.tsx (1 hunks)
  • platforms/blabsy/src/lib/context/theme-context.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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:

  • infrastructure/eid-wallet/src/env.d.ts
  • infrastructure/eid-wallet/src/lib/global/controllers/evault.ts
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.

Applied to files:

  • platforms/blabsy/src/lib/context/theme-context.tsx
🧬 Code graph analysis (3)
infrastructure/eid-wallet/src/lib/global/state.ts (1)
infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (1)
  • VaultController (50-542)
infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (2)
infrastructure/eid-wallet/src/lib/global/controllers/user.ts (2)
  • isFake (88-98)
  • isFake (100-113)
infrastructure/eid-wallet/src/env.d.ts (1)
  • PUBLIC_EID_WALLET_TOKEN (8-8)
infrastructure/evault-core/src/core/http/server.ts (1)
infrastructure/evault-core/generate-test-token.js (1)
  • token (39-50)
🪛 GitHub Actions: Build All Packages
infrastructure/signature-validator/package.json

[error] 1-1: ERR_PNPM_OUTDATED_LOCKFILE: Cannot install with 'frozen-lockfile' because pnpm-lock.yaml is not up to date with the package.json. The lockfile needs to be updated. Specifiers in the lockfile don't match: 5 dependencies added: @types/node@^20.11.24, typescript@^5.3.3, vitest@^1.6.1, axios@^1.6.7, multiformats@^13.3.2. Run 'pnpm install' without --frozen-lockfile to update the lockfile.

🪛 GitHub Actions: Check Format
infrastructure/signature-validator/package.json

[error] 1-1: pnpm install failed: Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with /infrastructure/signature-validator/package.json. This environment uses frozen lockfiles by default. Update the lockfile or run 'pnpm install --no-frozen-lockfile'.

🪛 GitHub Actions: Tests [evault-core]
infrastructure/signature-validator/package.json

[error] 1-1: pnpm install failed with frozen-lockfile: lockfile is not up to date with package.json. Specifiers in the lockfile don't match: 5 dependencies were added (@types/node@^20.11.24, typescript@^5.3.3, vitest@^1.6.1, axios@^1.6.7, multiformats@^13.3.2). Run 'pnpm install --no-frozen-lockfile' to update the lockfile.

🔇 Additional comments (13)
infrastructure/signature-validator/.gitignore (1)

1-5: LGTM!

The .gitignore patterns are appropriate for a TypeScript library project.

infrastructure/signature-validator/tsconfig.json (1)

1-18: LGTM!

The TypeScript configuration is appropriate for a Node.js library with strict type checking enabled and proper output settings.

infrastructure/signature-validator/src/index.ts (3)

7-28: LGTM!

The interfaces are well-documented with clear property descriptions and appropriate types.


35-67: LGTM!

The decoding logic correctly handles both hex and base58btc formats with appropriate fallback behavior and error handling.


154-241: LGTM!

The verification flow is well-structured with comprehensive input validation, proper use of Web Crypto API, and robust error handling.

infrastructure/signature-validator/README.md (1)

1-67: LGTM!

The documentation is comprehensive, well-structured, and includes clear usage examples and API specifications that accurately reflect the implementation.

platforms/blabsy/src/components/common/seo.tsx (1)

11-11: SEO component signature formatting is safe

Inline destructuring in the function signature does not change behavior; no further action needed.

platforms/blabsy/src/lib/context/theme-context.tsx (1)

60-60: Whitespace-only change around dark-theme effect

The added blank lines do not affect the forced dark theme behavior or side effects; no changes required.

Also applies to: 98-98

infrastructure/eid-wallet/src/lib/global/state.ts (1)

38-42: VaultController instantiation refactor is formatting-only

The multi-line constructor call preserves the same arguments and order, so behavior stays identical.

infrastructure/evault-core/package.json (1)

19-19: @fastify/cors dependency matches new Fastify CORS usage

Adding @fastify/cors here aligns with the plugin registration in src/index.ts; nothing else in the dependency set conflicts with this change from what’s shown. Please ensure the lockfile is updated and that the selected plugin version is compatible with your Fastify version in your environment.

infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (1)

183-192: LGTM: Non-blocking sync appropriately implemented.

The non-blocking public key sync after biometric authentication is correctly implemented with proper error handling. Note: This is a duplicate of the PIN flow logic; see comment on lines 101-110 for refactoring suggestion.

infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (2)

133-154: LGTM: Context-aware key retrieval logic is sound.

The logic correctly determines the context (pre-verification vs onboarding) based on the user's fake status and retrieves the public key accordingly. The error handling appropriately logs failures and returns early without throwing.


92-191: The syncPublicKey implementation correctly matches server-side expectations.

All three verification points have been confirmed:

  1. ✓ Server-side /public-key endpoint exists and validates the exact flow: requires X-ENAME header, Authorization Bearer token, and publicKey in request body.

  2. ✓ Bearer token authentication is properly validated server-side using JWT verification against JWKS (fetched from registry's /.well-known/jwks.json).

  3. ✓ The X-ENAME header is validated and used to identify which user's public key is being saved. The Bearer token verifies this is an authorized platform service (via PUBLIC_EID_WALLET_TOKEN).

The client implementation aligns with server expectations: X-ENAME header, Bearer token authorization (when PUBLIC_EID_WALLET_TOKEN is available), and publicKey payload are all provided as required.

declare module "$env/static/public" {
export const PUBLIC_REGISTRY_URL: string;
export const PUBLIC_PROVISIONER_URL: string;
export const PUBLIC_EID_WALLET_TOKEN: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

PUBLIC_EID_WALLET_TOKEN in $env/static/public will be exposed to the browser

Declaring PUBLIC_EID_WALLET_TOKEN in the $env/static/public module means its value is statically inlined into client bundles and is not secret. If this token is used for authenticating sensitive operations (e.g., calling eVault /public-key), exposing it to all client code (and, transitively, attackers running JS in the browser) significantly weakens your security model.

Please:

  • Confirm that this token is truly safe to be public, or
  • Move it to a private env module ($env/static/private/dynamic private) and adjust the flow so that only trusted server-side code uses it.

Based on prior learnings about avoiding PUBLIC_* envs for critical configuration (e.g., PUBLIC_PLATFORM_URL), this is worth a careful threat-model review rather than treating it as just another public constant.


🏁 Script executed:

#!/bin/bash
# Examine env.d.ts file content to see token declaration context
cat infrastructure/eid-wallet/src/env.d.ts

Length of output: 251


🏁 Script executed:

#!/bin/bash
# Search for how PUBLIC_EID_WALLET_TOKEN is used in the codebase
rg -n "PUBLIC_EID_WALLET_TOKEN" infrastructure/eid-wallet/

Length of output: 469


🏁 Script executed:

#!/bin/bash
# Check for .env.example or .env template files
fd -t f "\\.env" infrastructure/eid-wallet/ | head -20

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for eVault API calls and /public-key references to understand token usage
rg -n "public-key|eVault|PUBLIC_EID_WALLET" infrastructure/eid-wallet/ -A 3 -B 3

Length of output: 47537


Comment on lines +1 to +5
import {
PUBLIC_REGISTRY_URL,
PUBLIC_PROVISIONER_URL,
PUBLIC_EID_WALLET_TOKEN,
} from "$env/static/public";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that PUBLIC_EID_WALLET_TOKEN is configured in the environment.

The import of PUBLIC_EID_WALLET_TOKEN represents a shift from registry entropy to environment-based token derivation. Ensure this token is properly configured in your deployment environment.


🌐 Web query:

What is the recommended way to configure PUBLIC_EID_WALLET_TOKEN in SvelteKit for production deployments?

💡 Result:

Short answer

  • Don’t expose a sensitive wallet token to the browser. If the EID wallet token is secret, keep it server‑side (no PUBLIC_ prefix) and implement a server endpoint that uses it. [1][2]

If the token truly must be client‑visible

  • Add PUBLIC_EID_WALLET_TOKEN to your production environment variables in your host (Vercel/Netlify/Cloudflare/AWS/SST) rather than committing it to source or .env. Use the host’s dashboard/secret store so it’s injected at build/run time. [3][4]
  • Import it in client code via SvelteKit’s APIs:
    • Build‑time/static value: import { PUBLIC_EID_WALLET_TOKEN } from '$env/static/public'.
    • Runtime/dynamic value: import { env } from '$env/dynamic/public' and read env.PUBLIC_EID_WALLET_TOKEN. Prefer dynamic if the value may change between builds. [1][2]

Why this is recommended

  • PUBLIC_ prefix marks variables safe for client exposure; SvelteKit only exposes variables with that prefix to browser code. Use host environment settings to manage production values and avoid committing secrets. [1][2][3]

References

  1. SvelteKit environment variables docs (public vs private, static vs dynamic). [SvelteKit docs]. [1]
  2. $env/static/public and $env/dynamic/public usage (build-time vs runtime). [SvelteKit docs]. [2]
  3. Deployers’ guidance: set environment variables in your hosting provider (example: SST SvelteKitSite / hosting dashboards). [SST / hosting docs]. [3]
  4. Platform notes on framework env var naming and how hosts expose vars (Vercel discussion). [4]

I need to verify the codebase to understand the context better before rewriting the review comment.

Based on the web search results and the learnings from the repository, I can now provide my final analysis.

Remove the PUBLIC_EID_WALLET_TOKEN import and convert to a server-side private variable.

The import of PUBLIC_EID_WALLET_TOKEN conflicts with SvelteKit security best practices and eID wallet token handling standards. Authentication tokens should be kept secret and the signing key should be treated like any other credential and revealed only to services that need it. Private environment variables should only be accessed inside .server files, and secrets should be handled using server-only modules by adding .server to a filename or placing files inside $lib/server.

Since evault.ts is a client-side library controller in src/lib/global/controllers/, the token should not be imported there. Instead:

  • Convert PUBLIC_EID_WALLET_TOKEN to a private variable (remove PUBLIC_ prefix)
  • Move token usage to a server-side endpoint (e.g., +server.ts or .server.ts file)
  • Have the client-side code call the server endpoint rather than using the token directly

This aligns with the project's pattern noted in the learnings: public environment variables should not be added as configuration fallbacks.

Comment on lines 95 to 101
const savedKey = localStorage.getItem(`publicKeySaved_${eName}`);
if (savedKey === "true") {
console.log(`Public key already saved for ${eName}, skipping sync`);
console.log(
`Public key already saved for ${eName}, skipping sync`,
);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider more robust sync state tracking than localStorage.

Using localStorage to track whether the public key was synced can be unreliable since users can clear browser storage. This could lead to:

  • Unnecessary PATCH requests if localStorage is cleared
  • False assumptions about sync state across sessions

Consider alternatives:

  1. Store sync state in the Tauri Store alongside other vault data for persistence
  2. Always check /whois (which you already do) and skip the localStorage check entirely, relying on the server as source of truth

Apply this diff to use Tauri Store instead:

-            // Check if we've already saved the public key
-            const savedKey = localStorage.getItem(`publicKeySaved_${eName}`);
-            if (savedKey === "true") {
+            // Check if we've already saved the public key from store
+            const savedKey = await this.#store.get<boolean>(`publicKeySaved_${eName}`);
+            if (savedKey === true) {
                 console.log(
                     `Public key already saved for ${eName}, skipping sync`,
                 );

And update the save call at line 185:

-            // Mark as saved
-            localStorage.setItem(`publicKeySaved_${eName}`, "true");
+            // Mark as saved in store
+            await this.#store.set(`publicKeySaved_${eName}`, true);

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +163 to 169
// Get authentication token from environment variable
const authToken = PUBLIC_EID_WALLET_TOKEN || null;
if (!authToken) {
console.warn(
"PUBLIC_EID_WALLET_TOKEN not set, request may fail authentication",
);
authToken = entropyResponse.data?.token || null;
} catch (error) {
console.error("Failed to get auth token from registry:", error);
// Continue without token - server will reject if auth is required
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Review the security implications of continuing without authentication token.

The code warns but continues if PUBLIC_EID_WALLET_TOKEN is not set, which could lead to:

  • Silent authentication failures on the server side
  • Unexpected 401/403 responses that aren't clearly traced to missing token
  • Potential security bypass if the server doesn't properly enforce authentication

Consider one of these approaches:

  1. Fail fast (recommended): Throw an error if the token is not set, making misconfigurations immediately visible:
             const authToken = PUBLIC_EID_WALLET_TOKEN || null;
             if (!authToken) {
-                console.warn(
-                    "PUBLIC_EID_WALLET_TOKEN not set, request may fail authentication",
-                );
+                throw new Error(
+                    "PUBLIC_EID_WALLET_TOKEN not set, cannot authenticate public key sync",
+                );
             }
  1. Make it truly optional: If the token is genuinely optional (e.g., for development), add a clearer comment explaining when/why it's optional.

Which approach aligns with your authentication requirements?


I'll investigate the authentication token handling to verify the security implications. Let me examine the codebase context.


🏁 Script executed:

#!/bin/bash
# Find and examine the evault.ts file with context around the flagged lines
fd -t f "evault.ts" --exec head -n 200 {} \; | tail -n 150

Length of output: 5194


🏁 Script executed:

#!/bin/bash
# Search for authToken usage in the file to understand how it's handled
rg -n "authToken" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -A 5 -B 5

Length of output: 832


🏁 Script executed:

#!/bin/bash
# Check for authentication-related setup and configuration in the wallet
rg -n "PUBLIC_EID_WALLET_TOKEN" infrastructure/eid-wallet/ -B 3 -A 3

Length of output: 2048


🏁 Script executed:

#!/bin/bash
# Search for similar authentication patterns in the codebase
rg -n "throw new Error" infrastructure/eid-wallet/src/lib/global/controllers/ | head -20

Length of output: 1329


🏁 Script executed:

#!/bin/bash
# Look for environment variable validation patterns
rg -n "|| null||| undefined" infrastructure/eid-wallet/src/lib/global/controllers/ -B 2 -A 2 | head -30

Length of output: 2936


🏁 Script executed:

#!/bin/bash
# Check for tests related to evault or authentication
fd -t f "\.(test|spec)\.(ts|js)$" infrastructure/eid-wallet/ --exec grep -l "evault\|authToken\|PUBLIC_EID_WALLET_TOKEN" {} \;

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for documentation or comments about authentication requirements
rg -n "authentication|token" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 2 -A 2

Length of output: 360


🏁 Script executed:

#!/bin/bash
# Check if this is development-only code by looking at build/export context
rg -n "export|async function" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | head -10

Length of output: 33


🏁 Script executed:

#!/bin/bash
# Look for where this function is called to understand the context
rg -n "evault" infrastructure/eid-wallet/src/ --type ts -B 2 -A 2 | grep -E "(import|from|\.ts)" | head -20

Length of output: 541


🏁 Script executed:

#!/bin/bash
# Check .env files or configuration for how PUBLIC_EID_WALLET_TOKEN is documented
find infrastructure/eid-wallet -name ".env*" -o -name "*.env" -o -name "*.config.*" | xargs cat 2>/dev/null | grep -i token || echo "No env files found"

Length of output: 18


🏁 Script executed:

#!/bin/bash
# Search for error handling patterns around authentication failures
rg -n "401|403|Unauthorized|authentication" infrastructure/eid-wallet/src/lib/global/controllers/ -B 2 -A 2 | head -30

Length of output: 2958


🏁 Script executed:

#!/bin/bash
# Check if authToken is conditionally used (making it optional) or always required
rg -n "if.*authToken|authToken.*if" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 3 -A 3

Length of output: 533


🏁 Script executed:

#!/bin/bash
# Look for README or documentation files
fd -t f "(README|AUTHENTICATION|SECURITY)" infrastructure/eid-wallet/ -i

Length of output: 100


🏁 Script executed:

#!/bin/bash
# Check the request call to see if authentication is actually needed
rg -n "fetch|axios|request" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -A 10 | head -40

Length of output: 1538


🏁 Script executed:

#!/bin/bash
# Examine if there's a specific section about public key sync mentioned in error
rg -n "public key sync|sync" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 5 -A 5

Length of output: 5399


🏁 Script executed:

#!/bin/bash
# Check the function signature and purpose around line 163
sed -n '130,180p' infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 1946


🏁 Script executed:

#!/bin/bash
# Get broader context of the entire function
sed -n '100,220p' infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 4465


🏁 Script executed:

#!/bin/bash
# Check if this is part of a route handler and what its endpoint is
rg -n "POST|GET|PUT|DELETE|router\." infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 5 -A 10 | head -50

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for any try-catch or error handling around this code
rg -n "try|catch|error" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 3 -A 3

Length of output: 10043


🏁 Script executed:

#!/bin/bash
# Check what the function returns and if auth token is required for the return
rg -n "return|response" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 2 -A 2 | head -40

Length of output: 1072


🏁 Script executed:

#!/bin/bash
# See the full function that contains lines 163-169
awk '/^(export\s+|async\s+|function\s+).*\{/,/^}/' infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | grep -n "PUBLIC_EID_WALLET_TOKEN" -B 50 -A 20 | head -100

Length of output: 3113


🏁 Script executed:

#!/bin/bash
# Check if authToken validation exists elsewhere in the codebase
rg -n "authToken.*throw|throw.*authToken" infrastructure/eid-wallet/ -B 2 -A 2

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for environment variable validation patterns to understand best practices in this codebase
rg -n "process\.env\.[A-Z_]+" infrastructure/eid-wallet/src/lib/global/controllers/ -B 2 -A 5 | head -50

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check if PUBLIC_EID_WALLET_TOKEN is marked as optional in env schema/validation
rg -n "PUBLIC_EID_WALLET_TOKEN" infrastructure/eid-wallet/ -B 5 -A 5 --type ts --type js

Length of output: 2852


🏁 Script executed:

#!/bin/bash
# Get the exact content of the file to ensure we have accurate line numbers
cat -n infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | sed -n '150,180p'

Length of output: 1312


🏁 Script executed:

#!/bin/bash
# Check if there's any server-side validation mentioned in comments or logs
rg -n "server|validate|enforce" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -C 2 | head -30

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look at the complete function to understand the full flow
wc -l infrastructure/eid-wallet/src/lib/global/controllers/evault.ts && grep -n "^function\|^export\|^async" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 100


🏁 Script executed:

#!/bin/bash
# Simple approach: output the entire file to see the context
cat infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 18309


🏁 Script executed:

#!/bin/bash
# Look for where the token is passed in headers or request
rg -n "header|Authorization|Bearer" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 3 -A 3

Length of output: 1219


🏁 Script executed:

#!/bin/bash
# Check if the response includes the authToken or if it's used in request
rg -n "headers|Authorization" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 10 -A 10

Length of output: 2795


🏁 Script executed:

#!/bin/bash
# Look for the complete request being made to understand auth necessity
grep -n "fetch\|axios\|request" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -A 20 | head -60

Length of output: 2244


🏁 Script executed:

#!/bin/bash
# Check git history or comments about this token requirement
git log -p infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | grep -A 10 -B 10 "PUBLIC_EID_WALLET_TOKEN" | head -50

Length of output: 59


🏁 Script executed:

#!/bin/bash
# Check how other controllers handle required environment variables
ls -la infrastructure/eid-wallet/src/lib/global/controllers/ && find infrastructure/eid-wallet/src/lib/global/controllers/ -name "*.ts" -exec basename {} \;

Length of output: 455


🏁 Script executed:

#!/bin/bash
# Look at other controller implementations for pattern comparison
find infrastructure/eid-wallet/src/lib/global/controllers/ -name "*.ts" ! -name "evault.ts" -exec rg -l "process\.env\|throw.*Error" {} \; | head -5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check one other controller to see how they handle env vars
for file in $(find infrastructure/eid-wallet/src/lib/global/controllers/ -name "*.ts" ! -name "evault.ts" | head -1); do
  echo "=== $file ===" && grep -n "process\.env\|throw" "$file" -B 2 -A 2 | head -30
done

Length of output: 69


🏁 Script executed:

#!/bin/bash
# Search for authentication enforcement or validation on API routes
rg -n "middleware|validate|auth" infrastructure/eid-wallet/src/ --type ts -B 2 -A 2 | head -40

Length of output: 3529


🏁 Script executed:

#!/bin/bash
# Check if there's any documentation about required environment variables
find infrastructure/eid-wallet -type f \( -name "*.md" -o -name ".env.example" -o -name ".env.sample" \) -exec cat {} \;

Length of output: 1713


🏁 Script executed:

#!/bin/bash
# List all *.md files to find documentation
find infrastructure/eid-wallet -type f -name "*.md" -o -name "*.MD"

Length of output: 35


🏁 Script executed:

#!/bin/bash
# Check root documentation
cat README.md 2>/dev/null || echo "No README at root"

Length of output: 2106


🏁 Script executed:

#!/bin/bash
# Look for configuration schema or type definitions that might specify required fields
fd -t f "(schema|config|types|interface)" infrastructure/eid-wallet/src --exec grep -l "PUBLIC_EID_WALLET_TOKEN" {} \;

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check if there are type definitions or env validation files
fd -t f "(env|config)" infrastructure/eid-wallet/src -type f | grep -E "\.(ts|js)$"

Length of output: 170


🏁 Script executed:

#!/bin/bash
# Check if this endpoint is public/unauthenticated based on naming or comments
grep -n "public\|unauthenticated\|route\|endpoint" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts -B 3 -A 3

Length of output: 5959


🏁 Script executed:

#!/bin/bash
# Simple check: Just show the function containing line 163
sed -n '1,200p' infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | tail -100

Length of output: 3665


🏁 Script executed:

#!/bin/bash
# Simpler: Just cat the entire file
head -300 infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 9861


🏁 Script executed:

#!/bin/bash
# Get line counts and structure
wc -l infrastructure/eid-wallet/src/lib/global/controllers/evault.ts
grep -n "function\|const.*=.*(\|export" infrastructure/eid-wallet/src/lib/global/controllers/evault.ts | head -20

Length of output: 920


🏁 Script executed:

#!/bin/bash
# Show lines 150-190 with exact numbers
awk 'NR>=150 && NR<=190 {print NR": "$0}' infrastructure/eid-wallet/src/lib/global/controllers/evault.ts

Length of output: 1703


🏁 Script executed:

#!/bin/bash
# Check if file exists and is readable
test -f infrastructure/eid-wallet/src/lib/global/controllers/evault.ts && echo "File exists" || echo "File not found"

Length of output: 11


🏁 Script executed:

#!/bin/bash
# List the directory to confirm structure
ls -la infrastructure/eid-wallet/src/lib/global/controllers/ 2>/dev/null || ls -la infrastructure/eid-wallet/src/lib/global/ 2>/dev/null || find infrastructure -type d -name controllers

Length of output: 409


🏁 Script executed:

#!/bin/bash
# Locate the file
find . -name "evault.ts" -type f 2>/dev/null

Length of output: 115


🏁 Script executed:

#!/bin/bash
# Check directory structure
find infrastructure/eid-wallet -type d | head -20

Length of output: 1161


🏁 Script executed:

#!/bin/bash
# List what's in infrastructure
ls -la infrastructure/ 2>/dev/null || echo "Infrastructure dir listing failed"

Length of output: 538


🏁 Script executed:

#!/bin/bash
# Find all TypeScript files in the repository
fd -t f "\.ts$" infrastructure/ 2>/dev/null | head -30

Length of output: 1688


🏁 Script executed:

#!/bin/bash
# Broader search for evault file
fd evault -t f

Length of output: 868


🏁 Script executed:

#!/bin/bash
# Check if we're in the right repo by listing top-level contents
ls -la ./

Length of output: 2662


🏁 Script executed:

#!/bin/bash
# Find the file anywhere
find . -name "*evault*" -type f 2>/dev/null

Length of output: 507


🏁 Script executed:

#!/bin/bash
# List all files in src if it exists
find src -name "*.ts" 2>/dev/null | grep -E "(evault|controller)" | head -20

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Comprehensive file search
find . -type f -name "*.ts" 2>/dev/null | grep -i evault

Length of output: 4429


🏁 Script executed:

#!/bin/bash
# Look for any wallet-related files
find . -type f -path "*/wallet/*" -name "*.ts" 2>/dev/null | head -30

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Check the directory structure for any hints
find . -type d -name "*wallet*" -o -type d -name "*controller*" 2>/dev/null | head -20

Length of output: 838


🏁 Script executed:

#!/bin/bash
# Locate files that might contain this code
rg "PUBLIC_EID_WALLET_TOKEN" -l

Length of output: 101


🏁 Script executed:

#!/bin/bash
# Search for the specific warning message
rg "PUBLIC_EID_WALLET_TOKEN not set" -B 10 -A 10

Length of output: 2107


🏁 Script executed:

#!/bin/bash
# Search more broadly
rg "PUBLIC_EID_WALLET_TOKEN" -B 5 -A 15

Length of output: 4639


🏁 Script executed:

#!/bin/bash
# Try searching for the console.warn message
rg "request may fail authentication" -B 10 -A 10

Length of output: 2107


🏁 Script executed:

#!/bin/bash
# Search for evault related code
rg "evault" -B 3 -A 3 -i

Length of output: 274654


🏁 Script executed:

#!/bin/bash
# Find any references to this specific pattern
rg "authToken.*PUBLIC_EID" -B 5 -A 10

Length of output: 1600


🏁 Script executed:

#!/bin/bash
# Search for the exact snippet
rg "Get authentication token from environment variable" -B 5 -A 15

Length of output: 2106


🏁 Script executed:

#!/bin/bash
# Comprehensive search for authentication patterns
rg "const authToken" -B 3 -A 10

Length of output: 1431


🏁 Script executed:

#!/bin/bash
# Check all TypeScript/JavaScript files at once
fd -e ts -e js --exec rg -l "PUBLIC_EID_WALLET_TOKEN" {} \;

Length of output: 105


🏁 Script executed:

#!/bin/bash
# List all files to see repository structure
find . -type f -name "*.ts" -o -name "*.js" | head -50

Length of output: 2981


🏁 Script executed:

#!/bin/bash
# Try to see what files exist
ls -R . 2>/dev/null | head -100

Length of output: 1527


🏁 Script executed:

#!/bin/bash
# Get top-level structure
find . -maxdepth 2 -type f -name "*.ts" | head -30

Length of output: 24


🏁 Script executed:

#!/bin/bash
# List files and directories at top level
pwd && ls -la

Length of output: 2681


🏁 Script executed:

#!/bin/bash
# Check if files are in the PR branch by looking at git
git log --oneline -10 2>/dev/null || echo "Not a git repo"

Length of output: 451


🏁 Script executed:

#!/bin/bash
# Check git status and current branch
git status 2>/dev/null || echo "Git not available"

Length of output: 252


🏁 Script executed:

#!/bin/bash
# Try to find where the files should be
find . -type f -path "*infrastructure*" 2>/dev/null | head -20

Length of output: 894


🏁 Script executed:

#!/bin/bash
# Last resort: search entire filesystem for the file
locate evault.ts 2>/dev/null || echo "Locate not available"

Length of output: 20


🏁 Script executed:

#!/bin/bash
# Use rg to search everything
rg -t ts --files | head -50

Length of output: 2117


🏁 Script executed:

#!/bin/bash
# Try to understand the structure by listing dirs
find . -type d -maxdepth 3 | sort | head -50

Length of output: 1189


🏁 Script executed:

#!/bin/bash
# Check if infrastructure directory exists
test -d infrastructure && echo "infrastructure dir exists" || echo "infrastructure dir does not exist"
ls -la infrastructure 2>/dev/null || true

Length of output: 564


🏁 Script executed:

#!/bin/bash
# Use git to show changed files
git diff --name-only HEAD~1 2>/dev/null | head -20 || git diff --name-only 2>/dev/null | head -20 || echo "Could not get git diff"

Length of output: 560


🏁 Script executed:

#!/bin/bash
# Show all staged/changed files
git status -s 2>/dev/null || echo "Git unavailable"

Length of output: 71


🏁 Script executed:

#!/bin/bash
# Try to find the actual file location using rg on the entire repo
rg "authToken.*=.*PUBLIC_EID_WALLET_TOKEN" --files-with-matches

Length of output: 62


🏁 Script executed:

#!/bin/bash
# More flexible search
rg "PUBLIC_EID_WALLET_TOKEN.*null" --files-with-matches

Length of output: 62


🏁 Script executed:

#!/bin/bash
# Search by the exact comment
rg "Get authentication token from environment" --files-with-matches

Length of output: 62


🏁 Script executed:

#!/bin/bash
# Try rg without type filter
rg "PUBLIC_EID_WALLET_TOKEN" --files

Length of output: 63


🏁 Script executed:

#!/bin/bash
# Show current working directory context
echo "PWD: $(pwd)" && echo "FILES:" && ls -la

Length of output: 2693


🏁 Script executed:

#!/bin/bash
# Final attempt: comprehensive file listing
find . -name "*.ts" -o -name "*.js" | wc -l && echo "---" && find . -name "*.ts" -o -name "*.js" | sort | head -50

Length of output: 2959


🏁 Script executed:

#!/bin/bash
# List ALL files recursively to understand structure
ls -laR . 2>/dev/null | head -200

Length of output: 14803


🏁 Script executed:

#!/bin/bash
# Try git show to see the PR files
git show --name-only 2>/dev/null | head -30 || echo "Git show unavailable"

Length of output: 723


🏁 Script executed:

#!/bin/bash
# Check if the file exists with exact path from review
test -f "infrastructure/eid-wallet/src/lib/global/controllers/evault.ts" && echo "EXISTS" || echo "NOT FOUND"

Length of output: 6


🏁 Script executed:

#!/bin/bash
# Use ls to explicitly check
ls infrastructure/eid-wallet/src/lib/global/controllers/evault.ts 2>&1

Length of output: 62


🏁 Script executed:

#!/bin/bash
# Try alternate path
ls -la infrastructure/ 2>&1 | head -20

Length of output: 538


🏁 Script executed:

#!/bin/bash
# Debug: show git branch and files
git branch && echo "---" && git ls-files | head -30

Length of output: 752


🏁 Script executed:

#!/bin/bash
# Show PR context if available
git log --all --oneline | head -20

Length of output: 881


🏁 Script executed:

#!/bin/bash
# Final diagnostic
echo "Current directory: $(pwd)" && echo "Home: $HOME" && echo "Repo status:" && git remote -v 2>/dev/null || true

Length of output: 234


Require PUBLIC_EID_WALLET_TOKEN (fail fast) or make its absence explicit and deliberate

syncPublicKey currently only warns and continues if PUBLIC_EID_WALLET_TOKEN is missing, which yields silent authentication failures or unauthenticated PATCH calls. Either fail loudly or make the “optional” behavior explicit:

  • Location: infrastructure/eid-wallet/src/lib/global/controllers/evault.ts — syncPublicKey (lines ~163–169).
  • Option A (recommended): Throw an error when the token is not set so misconfiguration fails fast.
  • Option B: If it must be optional, add a clear dev-only guard (only allow missing token in non-production), document it in README/.env.example, and add tests that cover both modes.
    Also add startup/env validation for required public env vars to prevent silent misconfiguration.

Comment on lines +19 to +32
// Load .env file if it exists (try both root and current directory)
dotenv.config({ path: path.resolve(__dirname, "../../../.env") });
dotenv.config({ path: path.resolve(__dirname, ".env") });

async function generateTestToken() {
const jwkString = '{"kty":"EC","use":"sig","alg":"ES256","kid":"entropy-key-1","crv":"P-256","x":"POWUVJwOulAW0gheTVUHF4nXUenMCg0jxhGKI8M1LLU","y":"Cb4GC8Tt0gW7zMr-DhsDJisGVNgWttwjnyQl1HyU7hg","d":"FWqvWBzoiZcD0hh4JAMdJ7foxFRGqyV2Ei_eWvr1Si4"}';

if (!jwkString) {
console.error("Error: REGISTRY_ENTROPY_KEY_JWK environment variable is required");
console.error("\nUsage:");
console.error(" REGISTRY_ENTROPY_KEY_JWK='<your-jwk-json>' node generate-test-token.js");
console.error("\nOr set it in your .env file");
process.exit(1);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hard-coded private JWK and unused REGISTRY_ENTROPY_KEY_JWK env are problematic

The script currently ignores the documented REGISTRY_ENTROPY_KEY_JWK env var and instead uses a hard-coded JWK (including the private key). That:

  • Commits a signing key into the repo.
  • Forces all tokens to be signed with that specific key.
  • Makes rotation harder and increases risk if this key is ever trusted by other components.

Recommend:

  1. Remove the hard-coded JWK and read from the environment instead:
-async function generateTestToken() {
-  const jwkString = '{"kty":"EC",...}';
+async function generateTestToken() {
+  const jwkString = process.env.REGISTRY_ENTROPY_KEY_JWK;
@@
-  if (!jwkString) {
+  if (!jwkString) {
     console.error("Error: REGISTRY_ENTROPY_KEY_JWK environment variable is required");
@@
-    const jwk = JSON.parse(jwkString);
+    const jwk = JSON.parse(jwkString);
  1. Rotate / invalidate the currently checked-in key anywhere it may be trusted (e.g., JWKS on the registry), and ensure test-only keys are clearly segregated from any production trust roots.

  2. Optionally, keep a sample JWK in comments or docs rather than as executable code to avoid accidental use.

Also applies to: 34-65

🤖 Prompt for AI Agents
in infrastructure/evault-core/generate-test-token.js around lines 19 to 32 (and
likewise remove occurrences in lines 34–65), remove the hard-coded private JWK
string and instead read the key from the REGISTRY_ENTROPY_KEY_JWK environment
variable; validate that the env var is present and well-formed (exit with a
clear error and usage message if missing), parse the JWK from the env value for
signing, and ensure no private key literals remain in the file or repository;
additionally update docs/comments to show a sample JWK (not executable) and add
a note to rotate/invalidate the committed key wherever it may be trusted.

Comment on lines +12 to +15
"dependencies": {
"axios": "^1.6.7",
"multiformats": "^13.3.2"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify dependency versions for security.

Ensure the specified versions of axios and multiformats are secure and up-to-date.


🌐 Web query:

Are there any security vulnerabilities in the multiformats npm package version 13.3.2? What is the latest version?

💡 Result:

Short answer: No publicly reported/security-vulnerability entries for multiformats@13.3.2 were found; the current/latest published release is 13.4.1. [1][2]

Sources:

  • Snyk package data (no direct vulnerabilities listed; shows latest 13.4.1). [1]
  • jsDelivr/npm metadata showing multiformats version 13.4.1. [2]

If you want, I can fetch the npm registry changelog/commit log for 13.3.2→13.4.1 to see what changed.


Update axios to a patched version and upgrade multiformats for security.

axios version 1.6.7 is affected by CVE-2024-39338 (GHSA-8hc4-vh64-cxmj), a high-severity Server-Side Request Forgery (SSRF) vulnerability. The issue is resolved in axios version 1.7.4 and later. Update to at least 1.7.4 or preferably a newer stable version like 1.13.2.

multiformats@13.3.2 has no known vulnerabilities but is outdated; the latest available version is 13.4.1.

🤖 Prompt for AI Agents
in infrastructure/signature-validator/package.json around lines 12 to 15, update
the vulnerable deps: bump axios to at least 1.7.4 (preferably a current stable
like 1.13.2) and update multiformats to 13.4.1; modify the version strings in
package.json accordingly, then run your package manager (npm/yarn/pnpm) to
install and regenerate the lockfile, and run your test suite (and optionally npm
audit fix) to verify no regressions.

Comment on lines +1 to +2
import axios from "axios";
import { base58btc } from "multiformats/bases/base58";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify axios version for security and latest release.

Ensure axios 1.6.7 is secure and consider upgrading to the latest version if available.


🌐 Web query:

Are there any security vulnerabilities in axios version 1.6.7? What is the latest stable version of axios?

💡 Result:

Short answer:

  • Yes — axios@1.6.7 is affected by known security issues (SSRF and a data:‑URL DoS among others); these were fixed in later 1.x releases. [1][2][3]
  • Latest stable axios (as of now) is v1.13.2 — upgrade to that (or any non‑vulnerable patched release) is recommended. [4][5]

References

  1. GitHub Advisory / CVE for SSRF via absolute URLs (affects >=1.0.0 <1.8.2; fixed in 1.8.2). (github.com)
  2. GitHub Advisory / CVE for DoS via data: URL decoding (affects >=1.0.0 <1.12.0; fixed in 1.12.0). (github.com)
  3. Vendor/security scans summarizing issues affecting 1.6.7 (CVE-2025-58754, CVE-2025-27152, CVE-2024-39338, etc.). (meterian.io)
  4. axios GitHub releases showing v1.13.2 as the current release (Nov 4, 2025). (github.com)
  5. Snyk/npm package page listing latest non‑vulnerable version 1.13.2. (security.snyk.io)

If you want, I can provide the exact upgrade commands and a brief compatibility checklist for moving from 1.6.7 → 1.13.2.

Citations:


Upgrade axios from 1.6.7 to 1.13.2 to resolve multiple security vulnerabilities.

axios 1.6.7 is affected by known security issues including SSRF via absolute URLs and data URL DoS attacks. The latest stable version is v1.13.2, which resolves these vulnerabilities. Update the dependency immediately in package.json.

🤖 Prompt for AI Agents
in infrastructure/signature-validator/src/index.ts lines 1-2: the project
currently depends on axios v1.6.7 which has known security issues; update
package.json to bump axios to 1.13.2, run npm/yarn install to update the
lockfile, rebuild and run tests, and run npm audit/fix (or yarn audit) to ensure
no remaining vulnerabilities; verify there are no breaking changes in your axios
usage in this file (imports and call sites) and adjust code if needed before
committing the updated package.json and lockfile.

Comment on lines +83 to +91
// Otherwise, try base64 (software keys return base64)
try {
const binaryString = atob(signature);
const bytes = new Uint8Array(binaryString.length);
for (let i = 0; i < binaryString.length; i++) {
bytes[i] = binaryString.charCodeAt(i);
}
return bytes;
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Verify Node.js version compatibility for atob.

The atob function is used for base64 decoding. While available in Node.js 16+, ensure your minimum Node.js version requirement supports it, or consider using Buffer.from(signature, 'base64') for broader compatibility.


🏁 Script executed:

#!/bin/bash
# Check if package.json specifies Node.js version requirement
fd -t f 'package.json' -x cat {} | jq -r '.engines.node // "not specified"'

Length of output: 399



---

🏁 Script executed:

```shell
#!/bin/bash
# Find and display the specific package.json for signature-validator
if [ -f "infrastructure/signature-validator/package.json" ]; then
  echo "=== signature-validator package.json ===" 
  cat infrastructure/signature-validator/package.json | jq '{name, engines, version}'
else
  echo "File not found at infrastructure/signature-validator/package.json"
  # Try to find it
  fd -t f 'package.json' -path '*signature-validator*' -x cat {} \; | jq '{name, engines, version}'
fi

Length of output: 117


🏁 Script executed:

#!/bin/bash
# Check Node.js documentation for atob availability
echo "Checking Node.js atob availability timeline..."
echo "Node.js 15+ has experimental global atob"
echo "Node.js 16+ has stable atob support"
echo "Node.js <16 does not have atob as global"

Length of output: 164


Add Node.js version requirement or use Buffer.from for base64 decoding.

The atob function (lines 85-87) is only available in Node.js 16+, but signature-validator/package.json specifies no minimum Node.js version. Users on older Node.js versions will encounter runtime errors. Either:

  1. Add "engines": {"node": ">=16"} to package.json, or
  2. Replace atob with Buffer.from(signature, 'base64') for compatibility with Node.js versions before 16.
🤖 Prompt for AI Agents
in infrastructure/signature-validator/src/index.ts around lines 83 to 91, the
code uses atob() to decode base64 which is only available in Node.js 16+, but
package.json does not declare a minimum Node version; replace the atob-based
decoding with Node-compatible Buffer.from(signature, 'base64') to produce a
Uint8Array (or add an "engines": {"node":"=>16"} entry in package.json if you
prefer to require Node 16+); implement the Buffer approach by creating a Buffer
from the signature with 'base64' and then converting it to a Uint8Array before
returning.

@coderabbitai coderabbitai bot mentioned this pull request Nov 26, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Abstract away signature validation to a library

2 participants