From 750684ca36d7a5bd099cfc0bb95eb04f02ec5bf1 Mon Sep 17 00:00:00 2001 From: Rip&Tear <84775494+theCyberTech@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:23:30 +0800 Subject: [PATCH 1/3] fix: enforce owner-only permissions on credential files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Credentials stored at rest were left world-readable on multi-user hosts: - TokenManager._get_secure_storage_path() documented its credential dir as mode 0o700 but created it via mkdir() with default perms (0o755), leaving the Fernet secret.key and encrypted tokens.enc in a traversable dir. - Settings.dump() persisted tool_repository_password (plaintext) to settings.json via open("w"), producing a 0o644 file, and created the config dir at 0o755 — despite the sibling token_manager already writing secrets atomically at 0o600. Fixes: - TokenManager: chmod the credential dir to 0o700 after mkdir (robust against umask and pre-existing dirs). - Settings: write settings.json atomically at 0o600 (mkstemp + chmod + os.replace) and chmod the dedicated config dir to 0o700. The /tmp and cwd fallback parents are deliberately not chmod'd; the 0o600 file mode protects the credential there. Adds regression tests asserting 0o600 files and 0o700 dirs, and that shared fallback dirs are not globally tightened. Co-Authored-By: Claude Opus 4.8 --- lib/cli/tests/test_config.py | 55 +++++++++++++++++++ lib/cli/tests/test_token_manager.py | 48 ++++++++++++++++ lib/crewai-core/src/crewai_core/settings.py | 37 ++++++++++++- .../src/crewai_core/token_manager.py | 6 ++ 4 files changed, 144 insertions(+), 2 deletions(-) diff --git a/lib/cli/tests/test_config.py b/lib/cli/tests/test_config.py index b8e5ba9896..e858ebd889 100644 --- a/lib/cli/tests/test_config.py +++ b/lib/cli/tests/test_config.py @@ -1,5 +1,8 @@ import json +import os import shutil +import stat +import sys import tempfile import unittest from datetime import datetime, timedelta @@ -146,3 +149,55 @@ def test_empty_config_file(self): settings = Settings(config_path=self.config_path) self.assertIsNone(settings.tool_repository_username) + + +class TestSettingsFilePermissions(unittest.TestCase): + """Regression tests: credentials in settings.json must not be world-readable.""" + + def setUp(self): + self.test_dir = Path(tempfile.mkdtemp()) + + def tearDown(self): + shutil.rmtree(self.test_dir, ignore_errors=True) + + @unittest.skipIf(sys.platform == "win32", "POSIX permission semantics") + def test_dump_writes_owner_only_file(self): + config_path = self.test_dir / "settings.json" + old_umask = os.umask(0o022) + try: + settings = Settings( + config_path=config_path, tool_repository_password="hunter2" + ) + settings.dump() + finally: + os.umask(old_umask) + + mode = stat.S_IMODE(config_path.stat().st_mode) + self.assertEqual(mode, 0o600, f"expected 0o600, got {oct(mode)}") + + @unittest.skipIf(sys.platform == "win32", "POSIX permission semantics") + def test_dedicated_config_dir_is_owner_only(self): + config_path = self.test_dir / "crewai" / "settings.json" + old_umask = os.umask(0o022) + try: + Settings(config_path=config_path, tool_repository_username="u") + finally: + os.umask(old_umask) + + mode = stat.S_IMODE(config_path.parent.stat().st_mode) + self.assertEqual(mode, 0o700, f"expected 0o700, got {oct(mode)}") + + @unittest.skipIf(sys.platform == "win32", "POSIX permission semantics") + def test_shared_fallback_dir_is_not_chmodded(self): + """The system temp dir (a fallback parent) must never be globally chmod'd.""" + from crewai_core.settings import _ensure_dir_mode + + tmp_root = Path(tempfile.gettempdir()) + before = stat.S_IMODE(tmp_root.stat().st_mode) + _ensure_dir_mode(tmp_root) + after = stat.S_IMODE(tmp_root.stat().st_mode) + self.assertEqual(before, after) + + +if __name__ == "__main__": + unittest.main() diff --git a/lib/cli/tests/test_token_manager.py b/lib/cli/tests/test_token_manager.py index f5eaead049..50b7fa3ce9 100644 --- a/lib/cli/tests/test_token_manager.py +++ b/lib/cli/tests/test_token_manager.py @@ -1,6 +1,9 @@ """Tests for TokenManager with atomic file operations.""" import json +import os +import stat +import sys import tempfile import unittest from datetime import datetime, timedelta @@ -285,5 +288,50 @@ def test_delete_secure_file_not_exists( tm._delete_secure_file("nonexistent.txt") +class TestSecureStoragePathPermissions(unittest.TestCase): + """Test that the credential directory is created with restrictive permissions.""" + + @unittest.skipIf(sys.platform == "win32", "POSIX permission semantics") + def test_storage_path_is_owner_only(self) -> None: + """The credential directory must be mode 0o700 even under a permissive umask.""" + with tempfile.TemporaryDirectory() as base: + old_umask = os.umask(0o022) + try: + with ( + patch("crewai_core.token_manager.sys.platform", "linux"), + patch( + "crewai_core.token_manager.os.path.expanduser", + return_value=base, + ), + ): + storage_path = TokenManager._get_secure_storage_path() + finally: + os.umask(old_umask) + + self.assertTrue(storage_path.is_dir()) + mode = stat.S_IMODE(storage_path.stat().st_mode) + self.assertEqual(mode, 0o700, f"expected 0o700, got {oct(mode)}") + + @unittest.skipIf(sys.platform == "win32", "POSIX permission semantics") + def test_existing_loose_dir_is_tightened(self) -> None: + """A pre-existing world-traversable directory is corrected to 0o700.""" + with tempfile.TemporaryDirectory() as base: + loose = Path(base) / "crewai" / "credentials" + loose.mkdir(parents=True) + loose.chmod(0o755) + + with ( + patch("crewai_core.token_manager.sys.platform", "linux"), + patch( + "crewai_core.token_manager.os.path.expanduser", + return_value=base, + ), + ): + storage_path = TokenManager._get_secure_storage_path() + + mode = stat.S_IMODE(storage_path.stat().st_mode) + self.assertEqual(mode, 0o700, f"expected 0o700, got {oct(mode)}") + + if __name__ == "__main__": unittest.main() \ No newline at end of file diff --git a/lib/crewai-core/src/crewai_core/settings.py b/lib/crewai-core/src/crewai_core/settings.py index 083a9e2591..f294426c5e 100644 --- a/lib/crewai-core/src/crewai_core/settings.py +++ b/lib/crewai-core/src/crewai_core/settings.py @@ -4,6 +4,7 @@ import json from logging import getLogger +import os from pathlib import Path import tempfile from typing import Any @@ -25,6 +26,37 @@ DEFAULT_CONFIG_PATH = Path.home() / ".config" / "crewai" / "settings.json" +def _ensure_dir_mode(directory: Path) -> None: + """Tighten a dedicated config directory to 0o700. + + Skips directories shared with other users or content (the system temp dir + and the current working directory), which are used as best-effort fallbacks + by :func:`get_writable_config_path` and must not be globally chmod'd. Secret + files written there are still protected by their own 0o600 mode. + """ + try: + shared = {Path(tempfile.gettempdir()).resolve(), Path.cwd().resolve()} + if directory.resolve() in shared: + return + directory.chmod(0o700) + except OSError: + pass + + +def _write_secure_json(path: Path, data: dict[str, Any]) -> None: + """Atomically write ``data`` as JSON to ``path`` with owner-only (0o600) mode.""" + fd, tmp = tempfile.mkstemp(dir=path.parent, prefix=f".{path.name}.") + try: + with os.fdopen(fd, "w") as f: + json.dump(data, f, indent=4) + os.chmod(tmp, 0o600) + os.replace(tmp, path) + except BaseException: + if os.path.exists(tmp): + os.unlink(tmp) + raise + + def get_writable_config_path() -> Path | None: """Find a writable location for the config file with fallback options. @@ -43,6 +75,7 @@ def get_writable_config_path() -> Path | None: for config_path in fallback_paths: try: config_path.parent.mkdir(parents=True, exist_ok=True) + _ensure_dir_mode(config_path.parent) test_file = config_path.parent / ".crewai_write_test" try: test_file.write_text("test") @@ -153,6 +186,7 @@ def __init__(self, config_path: Path | None = None, **data: dict[str, Any]) -> N try: config_path.parent.mkdir(parents=True, exist_ok=True) + _ensure_dir_mode(config_path.parent) except Exception: merged_data = {**data} super().__init__(config_path=Path("/dev/null"), **merged_data) @@ -194,8 +228,7 @@ def dump(self) -> None: existing_data = {} updated_data = {**existing_data, **self.model_dump(exclude_unset=True)} - with self.config_path.open("w") as f: - json.dump(updated_data, f, indent=4) + _write_secure_json(self.config_path, updated_data) except Exception: # noqa: S110 pass diff --git a/lib/crewai-core/src/crewai_core/token_manager.py b/lib/crewai-core/src/crewai_core/token_manager.py index 06f2e7b186..5de4f35f90 100644 --- a/lib/crewai-core/src/crewai_core/token_manager.py +++ b/lib/crewai-core/src/crewai_core/token_manager.py @@ -95,6 +95,12 @@ def _get_secure_storage_path() -> Path: storage_path = Path(base_path) / app_name storage_path.mkdir(parents=True, exist_ok=True) + # Enforce the documented 0o700 mode: mkdir is subject to umask and does + # not adjust the mode of a pre-existing directory, so chmod explicitly. + try: + storage_path.chmod(0o700) + except OSError: + pass return storage_path From 714cd11d043d0cfb650aa46c35603a7498960f20 Mon Sep 17 00:00:00 2001 From: Rip&Tear <84775494+theCyberTech@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:42:16 +0800 Subject: [PATCH 2/3] Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- lib/crewai-core/src/crewai_core/settings.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/crewai-core/src/crewai_core/settings.py b/lib/crewai-core/src/crewai_core/settings.py index f294426c5e..c54f2ddb63 100644 --- a/lib/crewai-core/src/crewai_core/settings.py +++ b/lib/crewai-core/src/crewai_core/settings.py @@ -39,8 +39,12 @@ def _ensure_dir_mode(directory: Path) -> None: if directory.resolve() in shared: return directory.chmod(0o700) - except OSError: - pass + except OSError as e: + logger.debug( + "Could not enforce 0o700 on config directory %s (best-effort): %s", + directory, + e, + ) def _write_secure_json(path: Path, data: dict[str, Any]) -> None: From df4495dc061a4289020b78567f2b954dc3968c06 Mon Sep 17 00:00:00 2001 From: Rip&Tear <84775494+theCyberTech@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:46:26 +0800 Subject: [PATCH 3/3] Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- lib/crewai-core/src/crewai_core/token_manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/crewai-core/src/crewai_core/token_manager.py b/lib/crewai-core/src/crewai_core/token_manager.py index 5de4f35f90..7b2ad46530 100644 --- a/lib/crewai-core/src/crewai_core/token_manager.py +++ b/lib/crewai-core/src/crewai_core/token_manager.py @@ -100,6 +100,8 @@ def _get_secure_storage_path() -> Path: try: storage_path.chmod(0o700) except OSError: + # Best-effort permission hardening only: some platforms/filesystems + # may reject chmod here, and token operations should still proceed. pass return storage_path