diff --git a/apps/fileshare/docker-compose.yaml b/apps/fileshare/docker-compose.yaml index 54c2a34..4485083 100644 --- a/apps/fileshare/docker-compose.yaml +++ b/apps/fileshare/docker-compose.yaml @@ -4,6 +4,13 @@ # from the manifest's "volumes" list before this compose file is brought up; # it's referenced as `external: true` here so docker compose doesn't try # to manage its lifecycle. +# +# TODO(image-pin): `:latest` is shaky for production — pin to a digest +# (`dperson/samba@sha256:...`) or a stable tag once we've verified one +# against the upstream registry. For the MVP run we accept the drift +# risk to keep the install reproducible against whatever the upstream +# image happens to be on test day; revisit before any non-developer +# touches this. services: smbd: diff --git a/furtka/cli.py b/furtka/cli.py index c6b3466..764602b 100644 --- a/furtka/cli.py +++ b/furtka/cli.py @@ -55,7 +55,7 @@ def _cmd_app_install(args: argparse.Namespace) -> int: actions = reconciler.reconcile(apps_dir()) for a in actions: print(f" {a.describe()}") - return 0 + return 1 if reconciler.has_errors(actions) else 0 def _cmd_app_remove(args: argparse.Namespace) -> int: @@ -84,7 +84,10 @@ def _cmd_reconcile(args: argparse.Namespace) -> int: print(f" {a.describe()}") if args.dry_run: print("(dry-run — nothing changed)") - return 0 + # Exit non-zero on any per-app failure so systemd marks furtka-reconcile + # red — but only AFTER all apps were attempted, so a broken app doesn't + # hide healthy ones. + return 1 if reconciler.has_errors(actions) else 0 def build_parser() -> argparse.ArgumentParser: diff --git a/furtka/installer.py b/furtka/installer.py index d32e95d..ed9aa38 100644 --- a/furtka/installer.py +++ b/furtka/installer.py @@ -4,11 +4,32 @@ from pathlib import Path from furtka.manifest import ManifestError, load_manifest from furtka.paths import apps_dir, bundled_apps_dir +# Values that an app's .env.example may use as obvious "fill me in" markers. +# If any of these reach the live .env, install refuses — otherwise we'd ship +# an SMB share with password "changeme" out of the box, which is the kind of +# default that ends up screenshotted on Hacker News. +PLACEHOLDER_SECRETS: frozenset[str] = frozenset({"changeme"}) + class InstallError(RuntimeError): pass +def _placeholder_keys(env_path: Path) -> list[str]: + if not env_path.exists(): + return [] + bad: list[str] = [] + for raw in env_path.read_text().splitlines(): + line = raw.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, _, value = line.partition("=") + value = value.strip().strip('"').strip("'") + if value in PLACEHOLDER_SECRETS: + bad.append(key.strip()) + return bad + + def resolve_source(source: str) -> Path: """Resolve a `furtka app install ` arg to a real source folder. @@ -30,8 +51,11 @@ def install_from(src: Path) -> Path: """Copy a validated app folder into /var/lib/furtka/apps//. Preserves an existing .env on upgrade. Bootstraps .env from .env.example - on first install if .env wasn't shipped. - Returns the target folder. + on first install if .env wasn't shipped. Refuses to finish (raises + InstallError) if the resulting .env still has placeholder secrets — the + target folder is left in place so the user can edit and re-run. + + Returns the target folder on success. """ manifest_path = src / "manifest.json" if not manifest_path.exists(): @@ -59,6 +83,19 @@ def install_from(src: Path) -> Path: if not env.exists() and env_example.exists(): shutil.copy2(env_example, env) + # .env carries app secrets — lock to root-only. Done before the placeholder + # check so even the half-installed state is at least not world-readable. + if env.exists(): + env.chmod(0o600) + + bad = _placeholder_keys(env) + if bad: + raise InstallError( + f"{m.name}: {env} still has placeholder values for {', '.join(bad)}. " + f"Edit the file (set real values), then re-run " + f"`furtka app install {m.name}`." + ) + return target diff --git a/furtka/manifest.py b/furtka/manifest.py index b37c371..02502d9 100644 --- a/furtka/manifest.py +++ b/furtka/manifest.py @@ -35,7 +35,14 @@ class Manifest: return f"furtka_{self.name}_{short}" -def load_manifest(path: Path) -> Manifest: +def load_manifest(path: Path, expected_name: str | None = None) -> Manifest: + """Parse and validate a manifest.json. + + `expected_name` is used by the scanner (where the install location's folder + name IS the source of truth and must match the manifest). For loading from + arbitrary source folders during install, leave it None — the manifest's own + `name` field decides the install target. + """ try: raw = json.loads(path.read_text()) except json.JSONDecodeError as e: @@ -50,9 +57,8 @@ def load_manifest(path: Path) -> Manifest: name = raw["name"] if not isinstance(name, str) or not name: raise ManifestError(f"{path}: name must be a non-empty string") - expected = path.parent.name - if name != expected: - raise ManifestError(f"{path}: name {name!r} must equal containing folder {expected!r}") + if expected_name is not None and name != expected_name: + raise ManifestError(f"{path}: name {name!r} must equal {expected_name!r}") volumes = raw["volumes"] if not isinstance(volumes, list) or not all(isinstance(v, str) and v for v in volumes): diff --git a/furtka/reconciler.py b/furtka/reconciler.py index 9dadaa3..c21caf2 100644 --- a/furtka/reconciler.py +++ b/furtka/reconciler.py @@ -18,18 +18,35 @@ class Action: def reconcile(apps_root: Path, dry_run: bool = False) -> list[Action]: + """Walk the apps tree and bring docker into the desired state. + + Failures during one app's reconcile (Docker errors, missing binary, …) are + captured as Action(kind='error', …) and do NOT abort the whole sweep — the + other apps still get reconciled. Callers inspect the returned actions to + decide overall success. + """ actions: list[Action] = [] for result in scan(apps_root): if not result.ok: actions.append(Action("skip", result.path.name, result.error or "")) continue m = result.manifest - for vol_short in m.volumes: - full = m.volume_name(vol_short) - actions.append(Action("ensure_volume", full)) + try: + for vol_short in m.volumes: + full = m.volume_name(vol_short) + actions.append(Action("ensure_volume", full)) + if not dry_run: + dockerops.ensure_volume(full) + actions.append(Action("compose_up", m.name)) if not dry_run: - dockerops.ensure_volume(full) - actions.append(Action("compose_up", m.name)) - if not dry_run: - dockerops.compose_up(result.path, m.name) + dockerops.compose_up(result.path, m.name) + except (dockerops.DockerError, FileNotFoundError, OSError) as e: + # Catch broad enough to cover: docker daemon down, docker binary + # missing on the box, compose file unreadable. Narrow enough that + # programmer errors (KeyError etc.) still surface. + actions.append(Action("error", m.name, str(e))) return actions + + +def has_errors(actions: list[Action]) -> bool: + return any(a.kind == "error" for a in actions) diff --git a/furtka/scanner.py b/furtka/scanner.py index 74800fb..1927970 100644 --- a/furtka/scanner.py +++ b/furtka/scanner.py @@ -27,7 +27,7 @@ def scan(apps_root: Path) -> list[ScanResult]: out.append(ScanResult(path=entry, manifest=None, error="manifest.json missing")) continue try: - m = load_manifest(manifest_path) + m = load_manifest(manifest_path, expected_name=entry.name) except ManifestError as e: out.append(ScanResult(path=entry, manifest=None, error=str(e))) continue diff --git a/tests/test_installer.py b/tests/test_installer.py index 65e01ec..b54211a 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -92,6 +92,20 @@ def test_install_from_rejects_missing_manifest(tmp_path, fake_dirs): installer.install_from(src) +def test_install_from_arbitrary_source_folder_name(tmp_path, fake_dirs): + # Source folder named "downloaded-fileshare-fork-v2" but manifest says + # "fileshare" — install lands at /var/lib/furtka/apps/fileshare/ regardless. + src = _write_app_source( + tmp_path, + "downloaded-fileshare-fork-v2", + VALID_MANIFEST, + env_example="A=real-value", + ) + target = installer.install_from(src) + assert target.name == "fileshare" + assert (target / "manifest.json").exists() + + def test_install_from_rejects_invalid_manifest(tmp_path, fake_dirs): bad = dict(VALID_MANIFEST) del bad["volumes"] @@ -116,3 +130,62 @@ def test_remove_unknown_raises(fake_dirs): def test_bundled_apps_dir_uses_env_override(fake_dirs): _, bundled = fake_dirs assert bundled_apps_dir() == bundled + + +def test_install_refuses_placeholder_password(tmp_path, fake_dirs): + src = _write_app_source( + tmp_path, "fileshare", VALID_MANIFEST, env_example="SMB_PASSWORD=changeme" + ) + with pytest.raises(installer.InstallError, match="placeholder values for SMB_PASSWORD"): + installer.install_from(src) + # Files should still have landed so the user can vim the .env in place. + target = apps_dir() / "fileshare" + assert (target / ".env").exists() + assert (target / "manifest.json").exists() + + +def test_install_succeeds_after_user_edits_env(tmp_path, fake_dirs): + # First run: refuses placeholder. + src = _write_app_source( + tmp_path, "fileshare", VALID_MANIFEST, env_example="SMB_PASSWORD=changeme" + ) + with pytest.raises(installer.InstallError): + installer.install_from(src) + # User edits the live .env to a real secret. + target = apps_dir() / "fileshare" + (target / ".env").write_text("SMB_PASSWORD=hunter2\n") + # Re-run: now succeeds, user .env preserved. + installer.install_from(src) + assert (target / ".env").read_text().strip() == "SMB_PASSWORD=hunter2" + + +def test_install_locks_env_permissions(tmp_path, fake_dirs): + src = _write_app_source( + tmp_path, "fileshare", VALID_MANIFEST, env_example="SMB_PASSWORD=hunter2" + ) + installer.install_from(src) + target = apps_dir() / "fileshare" + mode = (target / ".env").stat().st_mode & 0o777 + assert mode == 0o600, f"expected 0o600 on .env, got {oct(mode)}" + + +def test_placeholder_check_ignores_comments_and_blanks(tmp_path, fake_dirs): + src = _write_app_source( + tmp_path, + "fileshare", + VALID_MANIFEST, + env_example="# default values\n\nSMB_PASSWORD=real-secret\n", + ) + # Should NOT raise — only commented "changeme" mentions, no actual placeholder. + installer.install_from(src) + + +def test_placeholder_check_handles_quoted_values(tmp_path, fake_dirs): + src = _write_app_source( + tmp_path, + "fileshare", + VALID_MANIFEST, + env_example='SMB_PASSWORD="changeme"\n', + ) + with pytest.raises(installer.InstallError, match="placeholder"): + installer.install_from(src) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index f587d7c..5c717ae 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -52,10 +52,20 @@ def test_missing_required_field(tmp_path): load_manifest(path) -def test_name_must_match_folder(tmp_path): +def test_name_must_match_when_expected_name_given(tmp_path): + # Scanner passes expected_name= so /var/lib/furtka/apps/X/ + # can't lie about its own identity. path = _write_app(tmp_path, "wrong-folder", VALID_MANIFEST) - with pytest.raises(ManifestError, match="must equal containing folder"): - load_manifest(path) + with pytest.raises(ManifestError, match="must equal 'wrong-folder'"): + load_manifest(path, expected_name="wrong-folder") + + +def test_name_check_skipped_without_expected_name(tmp_path): + # Installer loads from arbitrary source paths (e.g. /tmp/my-tweaked-app/) + # — the source folder name shouldn't matter, only the manifest's own name. + path = _write_app(tmp_path, "any-folder-name", VALID_MANIFEST) + m = load_manifest(path) + assert m.name == "fileshare" def test_invalid_json(tmp_path): diff --git a/tests/test_reconciler.py b/tests/test_reconciler.py index e102d39..4d2bc39 100644 --- a/tests/test_reconciler.py +++ b/tests/test_reconciler.py @@ -89,3 +89,47 @@ def test_reconcile_skips_broken_manifest(tmp_path, fake_docker): assert skip_actions[0].target == "broken" # Healthy app still got reconciled. assert fake_docker["compose_up"] == [(str(tmp_path / "fileshare"), "fileshare")] + + +def test_reconcile_isolates_per_app_docker_failure(tmp_path, monkeypatch): + """A docker error on one app must not block reconcile of the others.""" + _make_app(tmp_path, "alpha", dict(VALID_MANIFEST, name="alpha", volumes=["a"])) + _make_app(tmp_path, "broken-app", dict(VALID_MANIFEST, name="broken-app", volumes=["b"])) + _make_app(tmp_path, "zulu", dict(VALID_MANIFEST, name="zulu", volumes=["z"])) + + succeeded_compose: list[str] = [] + + def fake_ensure(name): + return True + + def fake_compose_up(app_dir, project): + if project == "broken-app": + raise dockerops.DockerError("simulated daemon failure") + succeeded_compose.append(project) + + monkeypatch.setattr(dockerops, "ensure_volume", fake_ensure) + monkeypatch.setattr(dockerops, "compose_up", fake_compose_up) + + actions = reconciler.reconcile(tmp_path) + error_actions = [a for a in actions if a.kind == "error"] + + assert reconciler.has_errors(actions) + assert len(error_actions) == 1 + assert error_actions[0].target == "broken-app" + # Both healthy apps reconciled despite the broken one in the middle. + assert succeeded_compose == ["alpha", "zulu"] + + +def test_reconcile_isolates_missing_docker_binary(tmp_path, monkeypatch): + """No docker binary on the box still produces a tidy per-app error.""" + _make_app(tmp_path, "fileshare", VALID_MANIFEST) + + def boom(*args, **kwargs): + raise FileNotFoundError("[Errno 2] No such file or directory: 'docker'") + + monkeypatch.setattr(dockerops, "ensure_volume", boom) + actions = reconciler.reconcile(tmp_path) + assert reconciler.has_errors(actions) + error = next(a for a in actions if a.kind == "error") + assert error.target == "fileshare" + assert "docker" in error.detail