Harden Encryption Key Management and Storage by d-oit · Pull Request #356 · d-oit/do-knowledge-studio

d-oit · 2026-06-27T01:15:35Z

This enhancement significantly improves the application's security posture by hardening how encryption keys (used for LLM API keys and the local key-store) are handled.

Key improvements:

Non-Extractable Keys: Encryption keys are now generated/imported with extractable: false. This means even if an attacker manages to execute script in the origin, they cannot extract the raw key bytes from the CryptoKey object.
Secure Key Storage: Keys are stored directly as CryptoKey objects in IndexedDB, leveraging the browser's structured clone algorithm. This replaces the previous pattern of storing keys as JWK strings (in localStorage or IndexedDB), which were easily readable.
API Key Protection: LLM API keys are no longer dependent on a master key stored in localStorage. The master key is now moved to the secure IndexedDB-based crypto store.
Automated Migration: The implementation includes robust, backward-compatible migration logic that automatically moves existing keys from legacy storage to the new secure store upon first access, ensuring no data loss for users.
Verified Security: Added a comprehensive test suite covering core crypto operations and migration scenarios, utilizing fake-indexeddb for reliable verification.

PR created automatically by Jules for task 4796541718176994595 started by @d-oit

- Centralize secure key management in src/lib/crypto.ts using Web Crypto API. - Implement non-extractable CryptoKey storage in IndexedDB (using structured clone). - Migrate LLM API keys encryption from localStorage to secure IndexedDB store. - Migrate key-store master key from JWK string to non-extractable CryptoKey. - Add fake-indexeddb for robust testing of secure storage logic. - Implement backward-compatible migration paths for all existing encrypted data. - Add comprehensive test suites for crypto utilities and migration flows. Co-authored-by: d-oit <6849456+d-oit@users.noreply.github.com>

google-labs-jules · 2026-06-27T01:15:36Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

codacy-production · 2026-06-27T01:16:59Z

Not up to standards ⛔

🔴 Issues 1 high

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results

ErrorProne 1 high

View in Codacy

🟢 Metrics 32 complexity · -2 duplication

Metric Results

Complexity 32

Duplication -2

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

…1718176994595

deepsource-io · 2026-07-02T13:21:40Z

DeepSource Code Review

We reviewed changes in 1f064eb...b7dcdba on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Updated (UTC)	Details
JavaScript	Jul 2, 2026 1:21p.m.	Review ↗
Python	Jul 2, 2026 1:21p.m.	Review ↗
Shell	Jul 2, 2026 1:21p.m.	Review ↗
SQL	Jul 2, 2026 1:21p.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

deepsource-io · 2026-07-02T13:22:15Z

@@ -0,0 +1,53 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';


'vi' is defined but never used

Unused variables are generally considered a code smell and should be avoided.

deepsource-io · 2026-07-02T13:22:15Z

@@ -0,0 +1,103 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';


'afterEach' is defined but never used

Unused variables are generally considered a code smell and should be avoided.

deepsource-io · 2026-07-02T13:22:15Z

@@ -0,0 +1,103 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';


'vi' is defined but never used

Unused variables are generally considered a code smell and should be avoided.

deepsource-io · 2026-07-02T13:22:15Z

+    // Clear the legacy key from IndexedDB if it exists (mocking legacy state)
+    const request = indexedDB.open('dks:key-store', 1);
+    await new Promise((resolve, reject) => {
+      request.onsuccess = (e: any) => {


Unexpected any. Specify a different type

The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.

deepsource-io · 2026-07-02T13:22:15Z

+        }
+      };
+      request.onerror = reject;
+      request.onupgradeneeded = (e: any) => {


Unexpected any. Specify a different type

The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.

deepsource-io · 2026-07-02T13:22:16Z

@@ -0,0 +1,56 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';


'vi' is defined but never used

Unused variables are generally considered a code smell and should be avoided.

deepsource-io · 2026-07-02T13:22:16Z

+    const combined = new Uint8Array(iv.length + encrypted.byteLength);
+    combined.set(iv, 0);
+    combined.set(new Uint8Array(encrypted), iv.length);
+    const legacyEncryptedValue = 'enc:v1:' + btoa(String.fromCharCode(...combined));


Unexpected string concatenation

In ES2015 (ES6), we can use template literals instead of string concatenation.

deepsource-io · 2026-07-02T13:22:16Z

 async function getKey(): Promise<CryptoKey> {
-  const stored = localStorage.getItem(ENCRYPTION_KEY_STORAGE);
-  if (stored) {
-    try {
-      return await crypto.subtle.importKey(
-        'jwk',
-        JSON.parse(stored) as JsonWebKey,
-        { name: 'AES-GCM', length: 256 },
-        true,
-        ['encrypt', 'decrypt'],
-      );
-    } catch (err) {
-      logger.warn('Encryption key is corrupted, generating a new one', err);
-      localStorage.removeItem(ENCRYPTION_KEY_STORAGE);
+  const CRYPTO_KEY_ID = 'dks:llm:encryption-key';
+
+  try {
+    if (await hasKey(CRYPTO_KEY_ID)) {
+      return await getOrCreateKey(CRYPTO_KEY_ID, { extractable: false });
    }
+  } catch (err) {
+    logger.debug('Error checking for key in crypto-store', err);
  }

-  const key = await crypto.subtle.generateKey(
-    { name: 'AES-GCM', length: 256 },
-    true,
-    ['encrypt', 'decrypt'],
-  );
-  const exported = await crypto.subtle.exportKey('jwk', key);
-  localStorage.setItem(ENCRYPTION_KEY_STORAGE, JSON.stringify(exported));
-  return key;
+  try {
+    // Fallback to migration from localStorage
+    const stored = localStorage.getItem(ENCRYPTION_KEY_STORAGE);
+    if (stored) {
+      try {
+        const jwk = JSON.parse(stored) as JsonWebKey;
+        const key = await importAndStoreKey(CRYPTO_KEY_ID, jwk, { extractable: false });
+        localStorage.removeItem(ENCRYPTION_KEY_STORAGE);
+        logger.info('Migrated LLM encryption key to secure storage');
+        return key;
+      } catch (migrationErr) {
+        logger.warn('Failed to migrate LLM encryption key, generating a new one', migrationErr);
+        localStorage.removeItem(ENCRYPTION_KEY_STORAGE);
+      }
+    }
+  } catch (err) {
+    logger.debug('No legacy key found in localStorage', err);
+  }
+
+  return await getOrCreateKey(CRYPTO_KEY_ID, { extractable: false });
 }


Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable

It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

deepsource-io · 2026-07-02T13:22:16Z

+ * Get the AES-GCM encryption key from the secure crypto-store.
+ * Migrates existing legacy keys from localStorage to IndexedDB.
 */
 async function getKey(): Promise<CryptoKey> {


`getKey` has a cyclomatic complexity of 6 with "medium" risk

A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

deepsource-io · 2026-07-02T13:22:16Z

 export async function encryptApiKey(plaintext: string): Promise<string> {
  const key = await getKey();
-  const iv = crypto.getRandomValues(new Uint8Array(12));
-  const encoded = new TextEncoder().encode(plaintext);
-  const encrypted = await crypto.subtle.encrypt(
-    { name: 'AES-GCM', iv },
-    key,
-    encoded,
-  );
-
-  // Combine IV + ciphertext and base64 encode
-  const combined = new Uint8Array(iv.length + encrypted.byteLength);
-  combined.set(iv, 0);
-  combined.set(new Uint8Array(encrypted), iv.length);
-
-  return ENCRYPTED_PREFIX + btoa(String.fromCharCode(...combined));
+  const encrypted = await encrypt(plaintext, key);
+  return ENCRYPTED_PREFIX + encrypted;
 }


Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable

It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

github-actions Bot added config tests Related to automated/manual tests labels Jun 27, 2026

Merge branch 'main' into fix/security-hardening-key-management-479654…

b7dcdba

…1718176994595

deepsource-io Bot reviewed Jul 2, 2026

View reviewed changes

		@@ -0,0 +1,53 @@
		import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';

		@@ -0,0 +1,103 @@
		import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';

		@@ -0,0 +1,56 @@
		import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';

Conversation

d-oit commented Jun 27, 2026

Uh oh!

google-labs-jules Bot commented Jun 27, 2026

Uh oh!

codacy-production Bot commented Jun 27, 2026

Not up to standards ⛔

Uh oh!

deepsource-io Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

'vi' is defined but never used

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

'afterEach' is defined but never used

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

'vi' is defined but never used

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

Unexpected any. Specify a different type

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

Unexpected any. Specify a different type

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

'vi' is defined but never used

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

Unexpected string concatenation

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

`getKey` has a cyclomatic complexity of 6 with "medium" risk

Uh oh!

deepsource-io Bot Jul 2, 2026

Choose a reason for hiding this comment

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

deepsource-io Bot commented Jul 2, 2026 •

edited

Loading