feat(security): encrypt API keys at rest using AES-GCM by d-oit · Pull Request #264 · d-oit/do-knowledge-studio

d-oit · 2026-06-05T14:17:02Z

Summary

Implements issue #238 — encrypts LLM API keys at rest using AES-GCM via Web Crypto API.

Changes

src/lib/llm/encryption.ts (NEW): AES-GCM encryption module. Generates 256-bit key stored as JWK in localStorage. enc:v1: prefix enables migration detection.
src/lib/llm/config.ts: loadConfig/saveConfig are now async. Auto-encrypts on save, auto-decrypts on load, auto-migrates plaintext keys.
src/features/ai/AIHarness.tsx: Async config loading via useEffect on mount.
src/features/ai/useChat.ts: await loadConfig() in sendMessage.
Tests updated: All loadConfig/saveConfig calls are now async.

Security Design

AES-256-GCM with 12-byte random IV per encryption
Encryption key stored as JWK in localStorage (browser-local threat model)
enc:v1: prefix for forward-compatible migration
Legacy plaintext keys auto-migrated on first load

Validation

0 lint errors ✅
310 unit tests ✅
Typecheck clean ✅
Code reviewer approved ✅

Closes #238

- Add src/lib/llm/encryption.ts: AES-GCM encryption via Web Crypto API - Generates 256-bit key stored as JWK in localStorage - encryptApiKey() returns base64 IV+ciphertext with enc:v1: prefix - decryptApiKey() handles both encrypted and legacy plaintext (migration) - isEncrypted() checks prefix for migration detection - Update config.ts: loadConfig/saveConfig are now async - Decrypts all provider API keys on load - Encrypts API keys before persisting - Auto-migrates plaintext keys on first load - Simplify maskApiKey to remove redundant logic - Update AIHarness.tsx: async config loading via useEffect - Update useChat.ts: await loadConfig() - Update all tests to handle async loadConfig/saveConfig Closes #238 Co-authored-by: d-oit <6849456+d-oit@users.noreply.github.com>

deepsource-io · 2026-06-05T14:17:19Z

DeepSource Code Review

We reviewed changes in 55fd03e...3ad35a2 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	Jun 5, 2026 2:17p.m.	Review ↗
Python	Jun 5, 2026 2:17p.m.	Review ↗
Shell	Jun 5, 2026 2:17p.m.	Review ↗
SQL	Jun 5, 2026 2:17p.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-06-05T14:17:52Z

+
+  // Load persisted config (with decrypted keys) on mount
+  useEffect(() => {
+    void loadConfig().then(setConfig);


Expected 'undefined' and instead saw 'void'

The void operator takes an operand and returns undefined. It can be used to ignore the value produced by an expression. However, this can lead to code that is difficult to understand and maintain. Historically, the void operator was used to get a "pure" undefined value, as the undefined variable was mutable prior to ES5.

deepsource-io · 2026-06-05T14:17:52Z

      ),
    };
-    saveConfig(updated);
+    void saveConfig(updated);


Expected 'undefined' and instead saw 'void'

The void operator takes an operand and returns undefined. It can be used to ignore the value produced by an expression. However, this can lead to code that is difficult to understand and maintain. Historically, the void operator was used to get a "pure" undefined value, as the undefined variable was mutable prior to ES5.

deepsource-io · 2026-06-05T14:17:52Z

      ),
    };
-    saveConfig(updated);
+    void saveConfig(updated);


Expected 'undefined' and instead saw 'void'

The void operator takes an operand and returns undefined. It can be used to ignore the value produced by an expression. However, this can lead to code that is difficult to understand and maintain. Historically, the void operator was used to get a "pure" undefined value, as the undefined variable was mutable prior to ES5.

deepsource-io · 2026-06-05T14:17:52Z

    },
  }),
-  saveConfig: vi.fn(),
+  saveConfig: vi.fn().mockResolvedValue(undefined),


Remove redundant `undefined` from function call

When an argument is omitted from a function call, it will default to undefined. It is therefore redundant to explicitly pass an undefined literal as the last argument.

deepsource-io · 2026-06-05T14:17:52Z

-    saveConfig(config);
+    await saveConfig(config);

    const stored = JSON.parse(localStorage.getItem('dks:llm-config')!) as { activeProvider: string; providers: { openrouter: { apiKey: string } } };


Forbidden non-null assertion

Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.

deepsource-io · 2026-06-05T14:17:52Z

 export function maskApiKey(key: string): string {
-  if (!key || key.length < 8) return key ? `...${key.slice(-4)}` : '';
+  if (!key) return '';
  return `...${key.slice(-4)}`;
 }


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-06-05T14:17:52Z

+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 {
+      // Key is corrupted, generate a new one
+      localStorage.removeItem(ENCRYPTION_KEY_STORAGE);
+    }
+  }
+
+  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;
+}


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-06-05T14:17:52Z

+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));
+}


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-06-05T14:17:53Z

+export async function decryptApiKey(encrypted: string): Promise<string> {
+  if (!encrypted.startsWith(ENCRYPTED_PREFIX)) {
+    // Legacy plaintext key — return as-is for migration
+    return encrypted;
+  }
+
+  const key = await getKey();
+  const base64 = encrypted.slice(ENCRYPTED_PREFIX.length);
+  const combined = Uint8Array.from(atob(base64), (c) => c.charCodeAt(0));
+
+  const iv = combined.slice(0, 12);
+  const ciphertext = combined.slice(12);
+
+  const decrypted = await crypto.subtle.decrypt(
+    { name: 'AES-GCM', iv },
+    key,
+    ciphertext,
+  );
+
+  return new TextDecoder().decode(decrypted);
+}


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-06-05T14:17:53Z

+export function isEncrypted(value: string): boolean {
+  return value.startsWith(ENCRYPTED_PREFIX);
+}


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.

codacy-production · 2026-06-05T14:18:08Z

Not up to standards ⛔

🔴 Issues 1 high

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

Results:
1 new issue

Category Results

Security 1 high

View in Codacy

🟢 Metrics 8 complexity · 0 duplication

Metric Results

Complexity 8

Duplication 0

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.}

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

d-oit merged commit eb378d9 into main Jun 5, 2026
20 of 22 checks passed

d-oit deleted the feat/encrypt-api-keys-at-rest branch June 5, 2026 14:17

deepsource-io Bot reviewed Jun 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(security): encrypt API keys at rest using AES-GCM#264

feat(security): encrypt API keys at rest using AES-GCM#264
d-oit merged 1 commit into
mainfrom
feat/encrypt-api-keys-at-rest

d-oit commented Jun 5, 2026

Uh oh!

deepsource-io Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

deepsource-io Bot Jun 5, 2026

Uh oh!

codacy-production Bot commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

d-oit commented Jun 5, 2026

Summary

Changes

Security Design

Validation

Uh oh!

deepsource-io Bot commented Jun 5, 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!

Uh oh!

deepsource-io Bot Jun 5, 2026

Choose a reason for hiding this comment

Expected 'undefined' and instead saw 'void'

Uh oh!

deepsource-io Bot Jun 5, 2026

Choose a reason for hiding this comment

Expected 'undefined' and instead saw 'void'

Uh oh!

deepsource-io Bot Jun 5, 2026

Choose a reason for hiding this comment

Expected 'undefined' and instead saw 'void'

Uh oh!

deepsource-io Bot Jun 5, 2026

Choose a reason for hiding this comment

Remove redundant `undefined` from function call

Uh oh!

deepsource-io Bot Jun 5, 2026

Choose a reason for hiding this comment

Forbidden non-null assertion

Uh oh!

deepsource-io Bot Jun 5, 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 Jun 5, 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 Jun 5, 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 Jun 5, 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 Jun 5, 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!

codacy-production Bot commented Jun 5, 2026

Not up to standards ⛔

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

deepsource-io Bot commented Jun 5, 2026 •

edited

Loading