From 25acd17f84eef8ac903136c9314d3ab6c5b986f8 Mon Sep 17 00:00:00 2001 From: Jackson Smith <42005615+jackson-asmith@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:28:11 -0400 Subject: [PATCH 1/2] Write .env with 0600 permissions and warn on loose modes The device password is stored in .env as TC_PASSWORD. This ensures the file is created owner-only and repairs a loose mode on rewrite, and adds a doctor warning when an existing .env is group/world-accessible. Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- src/timecapsulesmb/checks/doctor_steps.py | 5 +++- src/timecapsulesmb/core/config.py | 17 +++++++++++++ tests/test_checks.py | 19 +++++++++++++++ tests/test_config.py | 29 +++++++++++++++++++++++ 5 files changed, 70 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ceb5fdc5..5effc2d5 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ At the start of `configure`, the tool first tries to discover your Time Capsule `configure` also checks whether SSH is reachable. If SSH is closed, it enables SSH using the built-in Python 3 ACP client, reboots the device, waits for SSH to come up, and then continues the normal probing flow. If the password is wrong, it asks again instead of writing a broken `.env` file. -The password you enter here is stored locally as `TC_PASSWORD` so the tool can keep using SSH and ACP. The managed Samba runtime reads the current device password on the Time Capsule at boot. In other words, after setup, you normally connect with: +The password you enter here is stored locally as `TC_PASSWORD` so the tool can keep using SSH and ACP. The `.env` file is written with owner-only (`0600`) permissions, and `doctor` warns if it later becomes readable by other users. Do not sync the repo folder to a cloud service after `configure`. The managed Samba runtime reads the current device password on the Time Capsule at boot. In other words, after setup, you normally connect with: - username: `admin` (or any other password) - password: the same Time Capsule password you entered during configuration diff --git a/src/timecapsulesmb/checks/doctor_steps.py b/src/timecapsulesmb/checks/doctor_steps.py index 1a83e36e..6ba0af79 100644 --- a/src/timecapsulesmb/checks/doctor_steps.py +++ b/src/timecapsulesmb/checks/doctor_steps.py @@ -61,7 +61,7 @@ parse_xattr_tdb_paths, ) from timecapsulesmb.checks.smb_targets import doctor_smb_servers -from timecapsulesmb.core.config import AppConfig, DEFAULT_SAMBA_AUTH_USER, validate_app_config +from timecapsulesmb.core.config import AppConfig, DEFAULT_SAMBA_AUTH_USER, env_file_permission_warning, validate_app_config from timecapsulesmb.core.release import CLI_VERSION_CODE, RELEASE_TAG from timecapsulesmb.core.net import endpoint_host from timecapsulesmb.device.compat import is_netbsd4_payload_family, is_netbsd6_payload_family, render_compatibility_message @@ -335,6 +335,9 @@ def _add_config_validation_results( return False add_result(CheckResult("PASS", f"configuration file exists: {config.path}")) + permission_warning = env_file_permission_warning(config.path) + if permission_warning: + add_result(CheckResult("WARN", permission_warning)) validation_errors = validate_app_config(config, profile="doctor") if validation_errors: for error in validation_errors: diff --git a/src/timecapsulesmb/core/config.py b/src/timecapsulesmb/core/config.py index 4f56bdae..71b791c2 100644 --- a/src/timecapsulesmb/core/config.py +++ b/src/timecapsulesmb/core/config.py @@ -648,6 +648,21 @@ def preserved_env_file_values(values: dict[str, str]) -> dict[str, str]: return {key: value for key, value in values.items() if key not in ENV_FILE_OMIT_KEYS} +def env_file_permission_warning(path: Path) -> str | None: + if os.name != "posix": + return None + try: + mode = path.stat().st_mode + except OSError: + return None + if mode & 0o077: + return ( + f"{path} is accessible to other users (mode {mode & 0o777:03o}); " + f"it stores TC_PASSWORD. Run: chmod 600 {path}" + ) + return None + + def write_env_file(path: Path, values: dict[str, str]) -> None: text = render_env_text(values) tmp_name: str | None = None @@ -664,7 +679,9 @@ def write_env_file(path: Path, values: dict[str, str]) -> None: tmp.write(text) tmp.flush() os.fsync(tmp.fileno()) + os.chmod(tmp_name, 0o600) os.replace(tmp_name, path) + os.chmod(path, 0o600) finally: if tmp_name is not None: try: diff --git a/tests/test_checks.py b/tests/test_checks.py index 66788497..d1ab6bc8 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import sys import socket import tempfile @@ -41,6 +42,7 @@ DOCTOR_STARTUP_GRACE_SECONDS, STARTUP_GRACE_DETAIL_KEY, STARTUP_GRACE_MASK, + _add_config_validation_results, _apply_startup_grace, ) from timecapsulesmb.checks.local_tools import check_required_local_tools @@ -478,6 +480,23 @@ def test_run_doctor_checks_stops_invalid_config_before_remote_checks(self) -> No bonjour.assert_not_called() smb_listing.assert_not_called() + @unittest.skipUnless(os.name == "posix", "POSIX file modes only") + def test_config_validation_warns_on_loose_env_permissions(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + env_path = Path(tmp) / ".env" + env_path.write_text("TC_HOST=root@10.0.0.2\nTC_PASSWORD=secret\n") + os.chmod(env_path, 0o644) + config = AppConfig.from_values( + self.valid_doctor_values(), + path=env_path, + exists=True, + file_values=self.valid_doctor_values(), + ) + results: list[CheckResult] = [] + _add_config_validation_results(config, repo_root=REPO_ROOT, add_result=results.append) + warnings = [r for r in results if r.status == "WARN" and "chmod 600" in r.message] + self.assertTrue(warnings) + def test_run_doctor_checks_passes_when_deployed_version_matches_current_cli(self) -> None: run = self.run_doctor_with_mocks( ssh_login=mock.Mock(status="PASS", message="ssh ok"), diff --git a/tests/test_config.py b/tests/test_config.py index d1b23277..b99e9869 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import shlex import tempfile import unittest @@ -20,6 +21,7 @@ ConfigValidationError, build_mdns_device_model_txt, DEFAULTS, + env_file_permission_warning, load_app_config, parse_bool, parse_env_file, @@ -225,6 +227,33 @@ def test_write_env_file_omits_airport_syap(self) -> None: reparsed = parse_env_file(path) self.assertNotIn("TC_AIRPORT_SYAP", reparsed) + @unittest.skipUnless(os.name == "posix", "POSIX file modes only") + def test_write_env_file_is_owner_only(self) -> None: + values = dict(DEFAULTS) + values["TC_PASSWORD"] = "secret" + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / ".env" + path.write_text("stale") + os.chmod(path, 0o644) + write_env_file(path, values) + self.assertEqual(path.stat().st_mode & 0o777, 0o600) + + @unittest.skipUnless(os.name == "posix", "POSIX file modes only") + def test_env_file_permission_warning_flags_loose_modes(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / ".env" + path.write_text("TC_PASSWORD=secret\n") + os.chmod(path, 0o600) + self.assertIsNone(env_file_permission_warning(path)) + os.chmod(path, 0o644) + warning = env_file_permission_warning(path) + self.assertIsNotNone(warning) + self.assertIn("chmod 600", warning or "") + + def test_env_file_permission_warning_missing_file_is_silent(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + self.assertIsNone(env_file_permission_warning(Path(tmp) / "absent.env")) + def test_validate_airport_syap_accepts_known_codes(self) -> None: for value in ("104", "105", "106", "108", "109", "113", "114", "116", "117", "119", "120"): self.assertIsNone(validate_airport_syap(value, "Airport Utility syAP code")) From d07516e5b14cad5d4486ea20c09fdbfe72f53710 Mon Sep 17 00:00:00 2001 From: Jackson Smith <42005615+jackson-asmith@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:12:18 -0400 Subject: [PATCH 2/2] Drop redundant post-replace chmod in write_env_file (review feedback) Addresses review feedback on #252. On POSIX, os.replace preserves the temp file's 0600 mode on the destination inode, so the extra chmod on the final path was redundant and could fail on filesystems (NFS, some shared mounts) that restrict chmod even after a successful write. Co-Authored-By: Claude Opus 4.8 --- src/timecapsulesmb/core/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/timecapsulesmb/core/config.py b/src/timecapsulesmb/core/config.py index 71b791c2..798b8df4 100644 --- a/src/timecapsulesmb/core/config.py +++ b/src/timecapsulesmb/core/config.py @@ -681,7 +681,6 @@ def write_env_file(path: Path, values: dict[str, str]) -> None: os.fsync(tmp.fileno()) os.chmod(tmp_name, 0o600) os.replace(tmp_name, path) - os.chmod(path, 0o600) finally: if tmp_name is not None: try: