From 2857723e47432ddaab317542911bd81974eee6e7 Mon Sep 17 00:00:00 2001
From: Tom <108088199+ThomasJButler@users.noreply.github.com>
Date: Mon, 15 Jun 2026 07:00:11 +0100
Subject: [PATCH] fix(security): restrict API-key encryption key file to 0o600

Lock the API key encryption key file to owner-only permissions on creation and when reading existing keys, with regression coverage for permissions and encryption roundtrip.
---
 src/api_key_manager.py                 | 10 +++++
 tests/test_api_key_file_permissions.py | 51 ++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 tests/test_api_key_file_permissions.py

diff --git a/src/api_key_manager.py b/src/api_key_manager.py
index f0d25ced6..b3cf9a7b6 100644
--- a/src/api_key_manager.py
+++ b/src/api_key_manager.py
@@ -4,6 +4,8 @@ import logging
 from typing import Dict
 from cryptography.fernet import Fernet, InvalidToken
 
+from core.platform_compat import safe_chmod
+
 logger = logging.getLogger(__name__)
 
 class APIKeyManager:
@@ -15,12 +17,20 @@ class APIKeyManager:
     def get_or_create_key(self) -> bytes:
         """Get or create encryption key for API keys"""
         if os.path.exists(self.key_file):
+            # Older versions wrote .key with the process umask (often 0o644,
+            # i.e. group/world-readable). Re-restrict on read so existing
+            # installs heal without needing the key to be regenerated.
+            safe_chmod(self.key_file, 0o600)
             with open(self.key_file, 'rb') as f:
                 return f.read()
         else:
             key = Fernet.generate_key()
             with open(self.key_file, 'wb') as f:
                 f.write(key)
+            # This key decrypts every stored provider credential, so restrict it
+            # to the owner (0o600) — it must not be group/world-readable. No-op
+            # on Windows (files there are ACL-restricted to the user already).
+            safe_chmod(self.key_file, 0o600)
             return key
     
     def encrypt_api_key(self, api_key: str) -> str:
diff --git a/tests/test_api_key_file_permissions.py b/tests/test_api_key_file_permissions.py
new file mode 100644
index 000000000..947e1bcd0
--- /dev/null
+++ b/tests/test_api_key_file_permissions.py
@@ -0,0 +1,51 @@
+"""Regression: the API-key encryption key file (data/.key) must be owner-only
+(0o600).
+
+``APIKeyManager.get_or_create_key`` writes the Fernet key that decrypts *every*
+stored provider credential. Older versions created it with the process umask
+(commonly 0o644 — group/world-readable). It must be locked to the owner, both
+when freshly created and when an older, too-permissive key is read back.
+
+POSIX-only: ``core.platform_compat.safe_chmod`` is a documented no-op on Windows
+(files under the user profile are ACL-restricted), so the mode assertions are
+skipped there.
+"""
+import os
+import stat
+import sys
+
+import pytest
+
+from src.api_key_manager import APIKeyManager
+
+_WINDOWS = sys.platform.startswith("win")
+
+
+def _mode(path: str) -> int:
+    return stat.S_IMODE(os.stat(path).st_mode)
+
+
+@pytest.mark.skipif(_WINDOWS, reason="POSIX permission bits only")
+def test_new_key_file_is_owner_only(tmp_path):
+    mgr = APIKeyManager(str(tmp_path))
+    mgr.get_or_create_key()
+    assert _mode(mgr.key_file) == 0o600, f"expected 0o600, got {oct(_mode(mgr.key_file))}"
+
+
+@pytest.mark.skipif(_WINDOWS, reason="POSIX permission bits only")
+def test_existing_world_readable_key_is_relocked(tmp_path):
+    mgr = APIKeyManager(str(tmp_path))
+    # Simulate a key written by an older version with a permissive umask.
+    with open(mgr.key_file, "wb") as f:
+        f.write(b"x" * 44)
+    os.chmod(mgr.key_file, 0o644)
+    mgr.get_or_create_key()  # existing-file branch should re-lock it
+    assert _mode(mgr.key_file) == 0o600, f"expected re-lock to 0o600, got {oct(_mode(mgr.key_file))}"
+
+
+def test_encrypt_decrypt_roundtrip_still_works(tmp_path):
+    # The permission hardening must not change functional behaviour.
+    mgr = APIKeyManager(str(tmp_path))
+    enc = mgr.encrypt_api_key("sk-secret")
+    assert enc and enc != "sk-secret"
+    assert mgr.decrypt_api_key(enc) == "sk-secret"