Skip to content

Commit

Permalink
Check for common permissions issues with scratch area (#765)
Browse files Browse the repository at this point in the history
Fixes #747
  • Loading branch information
callumforrester authored Jan 7, 2025
1 parent 21a6cf5 commit 82b208f
Show file tree
Hide file tree
Showing 10 changed files with 265 additions and 30 deletions.
5 changes: 5 additions & 0 deletions docs/how-to/edit-live.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Blueapi can be configured to install editable Python packages from a chosen dire

scratch:
root: /path/to/my/scratch/directory
# Required GID for the scratch area
required_gid: 12345
repositories:
# Repository for DLS devices
- name: dodal
Expand All @@ -21,6 +23,9 @@ scratch:
remote_url: https://github.com/DiamondLightSource/mx-bluesky.git
```
Note the `required_gid` field, which is useful for stopping blueapi from locking the files it clones
to a particular owner.

## Synchronization

Blueapi will synchronize reality with the configuration if you run
Expand Down
5 changes: 5 additions & 0 deletions src/blueapi/cli/cli.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import json
import logging
import os
import stat
import sys
from functools import wraps
from pathlib import Path
Expand Down Expand Up @@ -39,6 +41,9 @@
def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None:
# if no command is supplied, run with the options passed

# Set umask to DLS standard
os.umask(stat.S_IWOTH)

config_loader = ConfigLoader(ApplicationConfig)
if config is not None:
configs = (config,) if isinstance(config, Path) else config
Expand Down
38 changes: 34 additions & 4 deletions src/blueapi/cli/scratch.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import logging
import os
import stat
import textwrap
from pathlib import Path
from subprocess import Popen

from git import Repo

from blueapi.config import ScratchConfig
from blueapi.utils import get_owner_gid, is_sgid_set

_DEFAULT_INSTALL_TIMEOUT: float = 300.0

Expand All @@ -23,7 +25,7 @@ def setup_scratch(
install_timeout: Timeout for installing packages
"""

_validate_directory(config.root)
_validate_root_directory(config.root, config.required_gid)

logging.info(f"Setting up scratch area: {config.root}")

Expand Down Expand Up @@ -74,9 +76,6 @@ def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> No

_validate_directory(path)

# Set umask to DLS standard
os.umask(stat.S_IWOTH)

logging.info(f"Installing {path}")
process = Popen(
[
Expand All @@ -94,6 +93,37 @@ def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> No
raise RuntimeError(f"Failed to install {path}: Exit Code: {process.returncode}")


def _validate_root_directory(root_path: Path, required_gid: int | None) -> None:
_validate_directory(root_path)

if not is_sgid_set(root_path):
raise PermissionError(
textwrap.dedent(f"""
The scratch area root directory ({root_path}) needs to have the
SGID permission bit set. This allows blueapi to clone
repositories into it while retaining the ability for
other users in an approved group to edit/delete them.
See https://www.redhat.com/en/blog/suid-sgid-sticky-bit for how to
set the SGID bit.
""")
)
elif required_gid is not None and get_owner_gid(root_path) != required_gid:
raise PermissionError(
textwrap.dedent(f"""
The configuration requires that {root_path} be owned by the group with
ID {required_gid}.
You may be able to find this group's name by running the following
in the terminal.
getent group 1000 | cut -d: -f1
You can transfer ownership, if you have sufficient permissions, with the chgrp
command.
""")
)


def _validate_directory(path: Path) -> None:
if not path.exists():
raise KeyError(f"{path}: No such file or directory")
Expand Down
30 changes: 26 additions & 4 deletions src/blueapi/config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import textwrap
from collections.abc import Mapping
from enum import Enum
from functools import cached_property
Expand Down Expand Up @@ -75,13 +76,34 @@ class RestConfig(BlueapiBaseModel):


class ScratchRepository(BlueapiBaseModel):
name: str = "example"
remote_url: str = "https://github.com/example/example.git"
name: str = Field(
description="Unique name for this repository in the scratch directory",
default="example",
)
remote_url: str = Field(
description="URL to clone from",
default="https://github.com/example/example.git",
)


class ScratchConfig(BlueapiBaseModel):
root: Path = Path("/tmp/scratch/blueapi")
repositories: list[ScratchRepository] = Field(default_factory=list)
root: Path = Field(
description="The root directory of the scratch area, all repositories will "
"be cloned under this directory.",
default=Path("/tmp/scratch/blueapi"),
)
required_gid: int | None = Field(
description=textwrap.dedent("""
Required owner GID for the scratch directory. If supplied the setup-scratch
command will check the scratch area ownership and raise an error if it is
not owned by <GID>.
"""),
default=None,
)
repositories: list[ScratchRepository] = Field(
description="Details of repositories to be cloned and imported into blueapi",
default_factory=list,
)


class OIDCConfig(BlueapiBaseModel):
Expand Down
3 changes: 3 additions & 0 deletions src/blueapi/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from .base_model import BlueapiBaseModel, BlueapiModelConfig, BlueapiPlanModelConfig
from .connect_devices import connect_devices
from .file_permissions import get_owner_gid, is_sgid_set
from .invalid_config_error import InvalidConfigError
from .modules import load_module_all
from .serialization import serialize
Expand All @@ -14,4 +15,6 @@
"BlueapiPlanModelConfig",
"InvalidConfigError",
"connect_devices",
"is_sgid_set",
"get_owner_gid",
]
32 changes: 32 additions & 0 deletions src/blueapi/utils/file_permissions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import stat
from pathlib import Path


def is_sgid_set(path: Path) -> bool:
"""Check if the SGID bit is set so that new files created
under a directory owned by a group are owned by that same group.
See https://www.redhat.com/en/blog/suid-sgid-sticky-bit
Args:
path: Path to the file to check
Returns:
bool: True if the SGID bit is set
"""

mask = path.stat().st_mode
return bool(mask & stat.S_ISGID)


def get_owner_gid(path: Path) -> int:
"""Get the GID of the owner of a file
Args:
path: Path to the file to check
Returns:
bool: The GID of the file owner
"""

return path.stat().st_gid
Loading

0 comments on commit 82b208f

Please sign in to comment.