Better permission errors for update, compose (#8)

A check is done preemptively to guess if the user has the necessary permissions to do these operations and provides guidance if necessary. This prevents the CLI from crashing later on with unhelpful stack traces.

Fixes #2.

Author: velovix

brainframe/cli/commands/compose.py

@@ -8,21 +8,6 @@
 
 @command("compose")
 def compose():
-    # Check first if the user has permission to interact with Docker
-    if not os_utils.is_root():
-        if not os_utils.currently_in_group("docker"):
-            error_message = i18n.t("compose.docker-bad-permissions") + "\n"
-
-            if os_utils.added_to_group("docker"):
-                # The user is in the group, they just need to restart
-                error_message += i18n.t("compose.restart-for-group-access")
-            else:
-                # The user is not in the group, so they need to either add
-                # themselves or use sudo
-                error_message += i18n.t("compose.retry-with-sudo-or-group")
-
-            print_utils.fail(error_message)
-
     install_path = env_vars.install_path.get()
     docker_compose.assert_installed(install_path)
     docker_compose.run(install_path, sys.argv[2:])

brainframe/cli/docker_compose.py

@@ -1,7 +1,9 @@
 import subprocess
 from pathlib import Path
-from typing import List
+from typing import List, Tuple, cast, TextIO
 import yaml
+import i18n
+import os
 
 from . import os_utils, print_utils, env_vars
 
@@ -14,7 +16,7 @@
 )
 
 
-def assert_installed(install_path: Path):
+def assert_installed(install_path: Path) -> None:
     compose_path = install_path / "docker-compose.yml"
 
     if not compose_path.is_file():
@@ -24,7 +26,9 @@ def assert_installed(install_path: Path):
         )
 
 
-def run(install_path: Path, commands: List[str]):
+def run(install_path: Path, commands: List[str]) -> None:
+    _assert_has_docker_permissions()
+
     compose_path = install_path / "docker-compose.yml"
 
     full_command = ["docker-compose", "--file", str(compose_path)]
@@ -42,7 +46,9 @@ def run(install_path: Path, commands: List[str]):
     os_utils.run(full_command + commands)
 
 
-def download(target: Path, version="latest"):
+def download(target: Path, version: str = "latest") -> None:
+    _assert_has_write_permissions(target.parent)
+
     subdomain, auth_flags, version = check_download_version(version=version)
 
     url = BRAINFRAME_DOCKER_COMPOSE_URL.format(
@@ -58,7 +64,9 @@ def download(target: Path, version="latest"):
         os_utils.give_brainframe_group_rw_access([target])
 
 
-def check_download_version(version="latest"):
+def check_download_version(
+    version: str = "latest",
+) -> Tuple[str, List[str], str]:
     subdomain = ""
     auth_flags = []
 
@@ -87,14 +95,57 @@ def check_download_version(version="latest"):
             stdout=subprocess.PIPE,
             encoding="utf-8",
         )
-        version = result.stdout.readline().strip()
+        # stdout is a file-like object opened in text mode when the encoding
+        # argument is "utf-8"
+        stdout = cast(TextIO, result.stdout)
+        version = stdout.readline().strip()
 
     return subdomain, auth_flags, version
 
 
-def check_existing_version(install_path: Path):
+def check_existing_version(install_path: Path) -> str:
     compose_path = install_path / "docker-compose.yml"
     compose = yaml.load(compose_path.read_text(), Loader=yaml.SafeLoader)
     version = compose["services"]["core"]["image"].split(":")[-1]
     version = "v" + version
     return version
+
+
+def _assert_has_docker_permissions() -> None:
+    """Fails if the user does not have permissions to interact with Docker"""
+    if not (os_utils.is_root() or os_utils.currently_in_group("docker")):
+        error_message = (
+            i18n.t("general.docker-bad-permissions")
+            + "\n"
+            + _group_recommendation_message("docker")
+        )
+
+        print_utils.fail(error_message)
+
+
+def _assert_has_write_permissions(path: Path) -> None:
+    """Fails if the user does not have write access to the given path."""
+    if os.access(path, os.W_OK):
+        return
+
+    error_message = i18n.t("general.file-bad-write-permissions", path=path)
+    error_message += "\n"
+
+    if path.stat().st_gid == os_utils.BRAINFRAME_GROUP_ID:
+        error_message += " " + _group_recommendation_message("brainframe")
+    else:
+        error_message += " " + i18n.t(
+            "general.unexpected-group-for-file", path=path, group="brainframe"
+        )
+
+    print_utils.fail(error_message)
+
+
+def _group_recommendation_message(group: str) -> str:
+    if os_utils.added_to_group("brainframe"):
+        # The user is in the group, they just need to restart
+        return i18n.t("general.restart-for-group-access", group=group)
+    else:
+        # The user is not in the group, so they need to either add
+        # themselves or use sudo
+        return i18n.t("general.retry-as-root-or-group", group=group)

brainframe/cli/translations/compose.en.yml

@@ -12,7 +12,19 @@ en:
   to a custom location, ensure that the %{install_env_var} environment variable
   has been set."
   mkdir-permission-denied: "Permission denied while making directory
-  %{directory}, trying with sudo."
+  %{directory}, trying as root."
   staging-missing-credentials: "The %{username_env_var} and %{password_env_var}
   variables must be set to access staging."
   user-not-root: "This command must be run as root!"
+  docker-bad-permissions: "Your user does not have sufficient privileges to
+  use Docker."
+  restart-for-group-access: "Your user has been added to the \"%{group}\"
+  group but you must restart to apply this change. Alternatively, you can try
+  again as root."
+  retry-as-root-or-group: "Please try again as root or consider adding your
+  user to the \"%{group}\" group."
+  file-bad-write-permissions: "Your user does not have write access to
+  \"%{path}\"."
+  unexpected-group-for-file: "File \"%{path}\" is not owned by the
+  \"%{group}\" group, but probably should be. Please add this file to the group
+  or try again as root."