fix: careful handling of file permissions during initialization

It has been found that during cold start (e.g. in docker container),
some permissions are invalid. In order to handle that, some operations
are not guarded with RepositoryPaths.preserve_root guard

In addition, it has been also found that in some cases (e.g. web server
start) migrations are performed on empty repository identifier which may
lead to wrong data (see also 435375721d),
as well as some unexpected results during database operations. In order
to handle that, now all watcher instances have their own databases (and
configurations)
This commit is contained in:
2025-07-10 18:41:23 +03:00
parent 97b906c536
commit c6306631e6
19 changed files with 162 additions and 80 deletions

View File

@ -8,7 +8,7 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -8,7 +8,7 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -8,7 +8,7 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock
@ -62,7 +62,7 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_REPOSITORY_SERVER: http://frontend/repo/$$repo/$$arch

View File

@ -12,7 +12,7 @@ services:
AHRIMAN_PACMAN_MIRROR: https://de.mirror.archlinux32.org/$$arch/$$repo
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -8,8 +8,8 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_POSTSETUP_COMMAND: ahriman --architecture x86_64 --repository another-demo service-setup --build-as-user ahriman --packager 'ahriman bot <ahriman@example.com>'
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_PRESETUP_COMMAND: ahriman --architecture x86_64 --repository another-demo service-setup --build-as-user ahriman --packager 'ahriman bot <ahriman@example.com>'
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -9,7 +9,7 @@ services:
AHRIMAN_OAUTH_CLIENT_SECRET: ${AHRIMAN_OAUTH_CLIENT_SECRET}
AHRIMAN_OUTPUT: console
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: sudo -u ahriman ahriman user-add ${AHRIMAN_OAUTH_USER} -R full -p ""
AHRIMAN_POSTSETUP_COMMAND: sudo -u ahriman ahriman user-add ${AHRIMAN_OAUTH_USER} -R full -p ""
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -6,7 +6,7 @@ services:
environment:
AHRIMAN_DEBUG: yes
AHRIMAN_OUTPUT: console
AHRIMAN_PRESETUP_COMMAND: sudo -u ahriman gpg --import /run/secrets/key
AHRIMAN_POSTSETUP_COMMAND: sudo -u ahriman gpg --import /run/secrets/key
AHRIMAN_REPOSITORY: ahriman-demo
configs:

View File

@ -8,7 +8,7 @@ services:
AHRIMAN_OUTPUT: console
AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD}
AHRIMAN_PORT: 8080
AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full
AHRIMAN_REPOSITORY: ahriman-demo
AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock

View File

@ -72,16 +72,17 @@ class Setup(Handler):
application = Application(repository_id, configuration, report=report)
Setup.configuration_create_makepkg(args.packager, args.makeflags_jobs, application.repository.paths)
Setup.executable_create(application.repository.paths, repository_id)
repository_server = f"file://{application.repository.paths.repository}" if args.server is None else args.server
Setup.configuration_create_devtools(
repository_id, args.from_configuration, args.mirror, args.multilib, repository_server)
Setup.configuration_create_sudo(application.repository.paths, repository_id)
with application.repository.paths.preserve_owner():
Setup.configuration_create_makepkg(args.packager, args.makeflags_jobs, application.repository.paths)
Setup.executable_create(application.repository.paths, repository_id)
repository_server = f"file://{application.repository.paths.repository}" if args.server is None else args.server
Setup.configuration_create_devtools(
repository_id, args.from_configuration, args.mirror, args.multilib, repository_server)
Setup.configuration_create_sudo(application.repository.paths, repository_id)
application.repository.repo.init()
# lazy database sync
application.repository.pacman.handle # pylint: disable=pointless-statement
application.repository.repo.init()
# lazy database sync
application.repository.pacman.handle # pylint: disable=pointless-statement
@staticmethod
def _set_service_setup_parser(root: SubParserAction) -> argparse.ArgumentParser:
@ -280,6 +281,5 @@ class Setup(Handler):
command = Setup.build_command(paths.root, repository_id)
command.unlink(missing_ok=True)
command.symlink_to(Setup.ARCHBUILD_COMMAND_PATH)
paths.chown(command) # we would like to keep owner inside ahriman's home
arguments = [_set_service_setup_parser]

View File

@ -130,8 +130,8 @@ class Pacman(LazyLogging):
return # database for some reason deos not exist
self.logger.info("copy pacman database %s from operating system root to ahriman's home %s", src, dst)
shutil.copy(src, dst)
self.repository_paths.chown(dst)
with self.repository_paths.preserve_owner(dst.parent):
shutil.copy(src, dst)
def database_init(self, handle: Handle, repository: str, architecture: str) -> DB:
"""

View File

@ -203,8 +203,6 @@ def migrate_package_repository(connection: Connection, configuration: Configurat
configuration(Configuration): configuration instance
"""
_, repository_id = configuration.check_loaded()
if repository_id.is_empty:
return # no repository available yet
connection.execute("""update build_queue set repository = :repository""", {"repository": repository_id.id})
connection.execute("""update package_bases set repository = :repository""", {"repository": repository_id.id})

View File

@ -94,9 +94,13 @@ class SQLite(
sqlite3.register_adapter(list, json.dumps)
sqlite3.register_converter("json", json.loads)
if self._configuration.getboolean("settings", "apply_migrations", fallback=True):
if not self._configuration.getboolean("settings", "apply_migrations", fallback=True):
return
if self._repository_id.is_empty:
return # do not perform migration on empty repository identifier (e.g. multirepo command)
with self._repository_paths.preserve_owner():
self.with_connection(lambda connection: Migrations.migrate(connection, self._configuration))
self._repository_paths.chown(self.path)
def package_clear(self, package_base: str, repository_id: RepositoryId | None = None) -> None:
"""

View File

@ -17,6 +17,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
import contextlib
import os
import shutil
@ -221,22 +222,14 @@ class RepositoryPaths(LazyLogging):
stat = path.stat()
return stat.st_uid, stat.st_gid
def cache_for(self, package_base: str) -> Path:
"""
get path to cached PKGBUILD and package sources for the package base
Args:
package_base(str): package base name
Returns:
Path: full path to directory for specified package base cache
"""
return self.cache / package_base
def chown(self, path: Path) -> None:
def _chown(self, path: Path) -> None:
"""
set owner of path recursively (from root) to root owner
Notes:
More likely you don't want to call this method explicitly, consider using :func:`preserve_owner`
as context manager instead
Args:
path(Path): path to be chown
@ -256,6 +249,56 @@ class RepositoryPaths(LazyLogging):
set_owner(path)
path = path.parent
def cache_for(self, package_base: str) -> Path:
"""
get path to cached PKGBUILD and package sources for the package base
Args:
package_base(str): package base name
Returns:
Path: full path to directory for specified package base cache
"""
return self.cache / package_base
@contextlib.contextmanager
def preserve_owner(self, path: Path | None = None) -> Generator[None, None, None]:
"""
perform any action preserving owner for any newly created file or directory
Args:
path(Path | None, optional): use this path as root instead of repository root (Default value = None)
Examples:
This method is designed to use as context manager when you are going to perform operations which might
change filesystem, especially if you are doing it under unsafe flag, e.g.::
>>> with paths.preserve_owner():
>>> paths.tree_create()
Note, however, that this method doesn't handle any exceptions and will eventually interrupt
if there will be any.
"""
path = path or self.root
def walk(root: Path) -> Generator[Path, None, None]:
# basically walk, but skipping some content
for child in root.iterdir():
yield child
if child in (self.chroot.parent,):
yield from child.iterdir() # we only yield top-level in chroot directory
elif child.is_dir():
yield from walk(child)
# get current filesystem and run action
previous_snapshot = set(walk(path))
yield
# get newly created files and directories and chown them
new_entries = set(walk(path)).difference(previous_snapshot)
for entry in new_entries:
self._chown(entry)
def tree_clear(self, package_base: str) -> None:
"""
clear package specific files
@ -274,12 +317,13 @@ class RepositoryPaths(LazyLogging):
"""
if self.repository_id.is_empty:
return # do not even try to create tree in case if no repository id set
for directory in (
self.cache,
self.chroot,
self.packages,
self.pacman,
self.repository,
):
directory.mkdir(mode=0o755, parents=True, exist_ok=True)
self.chown(directory)
with self.preserve_owner():
for directory in (
self.cache,
self.chroot,
self.packages,
self.pacman,
self.repository,
):
directory.mkdir(mode=0o755, parents=True, exist_ok=True)

View File

@ -166,11 +166,16 @@ def setup_server(configuration: Configuration, spawner: Spawn, repositories: lis
# package cache
if not repositories:
raise InitializeError("No repositories configured, exiting")
database = SQLite.load(configuration)
watchers: dict[RepositoryId, Watcher] = {}
configuration_path, _ = configuration.check_loaded()
for repository_id in repositories:
application.logger.info("load repository %s", repository_id)
client = Client.load(repository_id, configuration, database, report=False) # explicitly load local client
# load settings explicitly for architecture if any
repository_configuration = Configuration.from_path(configuration_path, repository_id)
# load database instance, because it holds identifier
database = SQLite.load(repository_configuration)
# explicitly load local client
client = Client.load(repository_id, repository_configuration, database, report=False)
watchers[repository_id] = Watcher(client)
application[WatcherKey] = watchers
# workers cache
@ -179,6 +184,7 @@ def setup_server(configuration: Configuration, spawner: Spawn, repositories: lis
application[SpawnKey] = spawner
application.logger.info("setup authorization")
database = SQLite.load(configuration)
validator = application[AuthKey] = Auth.load(configuration, database)
if validator.enabled:
from ahriman.web.middlewares.auth_handler import setup_auth

View File

@ -58,9 +58,11 @@ def test_run(args: argparse.Namespace, configuration: Configuration, repository:
sudo_configuration_mock = mocker.patch("ahriman.application.handlers.setup.Setup.configuration_create_sudo")
executable_mock = mocker.patch("ahriman.application.handlers.setup.Setup.executable_create")
init_mock = mocker.patch("ahriman.core.alpm.repo.Repo.init")
owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner")
_, repository_id = configuration.check_loaded()
Setup.run(args, repository_id, configuration, report=False)
owner_guard_mock.assert_called_once_with()
ahriman_configuration_mock.assert_called_once_with(args, repository_id, configuration)
devtools_configuration_mock.assert_called_once_with(
repository_id, args.from_configuration, args.mirror, args.multilib, f"file://{repository_paths.repository}")
@ -268,13 +270,11 @@ def test_executable_create(configuration: Configuration, repository_paths: Repos
"""
must create executable
"""
chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown")
symlink_mock = mocker.patch("pathlib.Path.symlink_to")
unlink_mock = mocker.patch("pathlib.Path.unlink")
_, repository_id = configuration.check_loaded()
Setup.executable_create(repository_paths, repository_id)
chown_mock.assert_called_once_with(Setup.build_command(repository_paths.root, repository_id))
symlink_mock.assert_called_once_with(Setup.ARCHBUILD_COMMAND_PATH)
unlink_mock.assert_called_once_with(missing_ok=True)

View File

@ -62,12 +62,12 @@ def test_database_copy(pacman: Pacman, mocker: MockerFixture) -> None:
mocker.patch("pathlib.Path.is_file", autospec=True, side_effect=lambda p: p.is_relative_to(path))
mkdir_mock = mocker.patch("pathlib.Path.mkdir")
copy_mock = mocker.patch("shutil.copy")
chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown")
owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner")
pacman.database_copy(pacman.handle, database, path, use_ahriman_cache=True)
mkdir_mock.assert_called_once_with(mode=0o755, exist_ok=True)
copy_mock.assert_called_once_with(path / "sync" / "core.db", dst_path)
chown_mock.assert_called_once_with(dst_path)
owner_guard_mock.assert_called_once_with(dst_path.parent)
def test_database_copy_skip(pacman: Pacman, mocker: MockerFixture) -> None:

View File

@ -6,7 +6,6 @@ from unittest.mock import call as MockCall
from ahriman.core.configuration import Configuration
from ahriman.core.database.migrations.m011_repository_name import migrate_data, migrate_package_repository, steps
from ahriman.models.repository_id import RepositoryId
def test_migration_repository_name() -> None:
@ -38,13 +37,3 @@ def test_migrate_package_repository(connection: Connection, configuration: Confi
MockCall(pytest.helpers.anyvar(str, strict=True), {"repository": configuration.repository_id.id}),
MockCall(pytest.helpers.anyvar(str, strict=True), {"repository": configuration.repository_id.id}),
])
def test_migrate_package_repository_empty_id(connection: Connection, configuration: Configuration,
mocker: MockerFixture) -> None:
"""
must do nothing on empty repository id
"""
mocker.patch("ahriman.core.configuration.Configuration.check_loaded", return_value=("", RepositoryId("", "")))
migrate_package_repository(connection, configuration)
connection.execute.assert_not_called()

View File

@ -36,6 +36,17 @@ def test_init_skip_migration(database: SQLite, mocker: MockerFixture) -> None:
migrate_schema_mock.assert_not_called()
def test_init_skip_empty_repository(database: SQLite, mocker: MockerFixture) -> None:
"""
must skip migrations if repository identifier is not set
"""
database._repository_id = RepositoryId("", "")
migrate_schema_mock = mocker.patch("ahriman.core.database.migrations.Migrations.migrate")
database.init()
migrate_schema_mock.assert_not_called()
def test_package_clear(database: SQLite, repository_id: RepositoryId, mocker: MockerFixture) -> None:
"""
must clear package data

View File

@ -198,15 +198,6 @@ def test_owner(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None
assert RepositoryPaths.owner(repository_paths.root) == (42, 142)
def test_cache_for(repository_paths: RepositoryPaths, package_ahriman: Package) -> None:
"""
must return correct path for cache directory
"""
path = repository_paths.cache_for(package_ahriman.base)
assert path.name == package_ahriman.base
assert path.parent == repository_paths.cache
def test_chown(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None:
"""
must correctly set owner for the directory
@ -216,7 +207,7 @@ def test_chown(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None
chown_mock = mocker.patch("os.chown")
path = repository_paths.root / "path"
repository_paths.chown(path)
repository_paths._chown(path)
chown_mock.assert_called_once_with(path, 42, 42, follow_symlinks=False)
@ -229,7 +220,7 @@ def test_chown_parent(repository_paths: RepositoryPaths, mocker: MockerFixture)
chown_mock = mocker.patch("os.chown")
path = repository_paths.root / "parent" / "path"
repository_paths.chown(path)
repository_paths._chown(path)
chown_mock.assert_has_calls([
MockCall(path, 42, 42, follow_symlinks=False),
MockCall(path.parent, 42, 42, follow_symlinks=False)
@ -245,7 +236,7 @@ def test_chown_skip(repository_paths: RepositoryPaths, mocker: MockerFixture) ->
chown_mock = mocker.patch("os.chown")
path = repository_paths.root / "path"
repository_paths.chown(path)
repository_paths._chown(path)
chown_mock.assert_not_called()
@ -254,7 +245,46 @@ def test_chown_invalid_path(repository_paths: RepositoryPaths) -> None:
must raise invalid path exception in case if directory outside the root supplied
"""
with pytest.raises(PathError):
repository_paths.chown(repository_paths.root.parent)
repository_paths._chown(repository_paths.root.parent)
def test_cache_for(repository_paths: RepositoryPaths, package_ahriman: Package) -> None:
"""
must return correct path for cache directory
"""
path = repository_paths.cache_for(package_ahriman.base)
assert path.name == package_ahriman.base
assert path.parent == repository_paths.cache
def test_preserve_owner(tmp_path: Path, repository_id: RepositoryId, mocker: MockerFixture) -> None:
"""
must preserve file owner during operations
"""
repository_paths = RepositoryPaths(tmp_path, repository_id)
repository_paths.tree_create()
chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths._chown")
with repository_paths.preserve_owner():
(repository_paths.root / "created1").touch()
(repository_paths.chroot / "created2").touch()
chown_mock.assert_has_calls([MockCall(repository_paths.root / "created1")])
def test_preserve_owner_specific(tmp_path: Path, repository_id: RepositoryId, mocker: MockerFixture) -> None:
"""
must preserve file owner during operations only in specific directory
"""
repository_paths = RepositoryPaths(tmp_path, repository_id)
repository_paths.tree_create()
(repository_paths.root / "content").mkdir()
chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths._chown")
with repository_paths.preserve_owner(repository_paths.root / "content"):
(repository_paths.root / "created1").touch()
(repository_paths.root / "content" / "created2").touch()
(repository_paths.chroot / "created3").touch()
chown_mock.assert_has_calls([MockCall(repository_paths.root / "content" / "created2")])
def test_tree_clear(repository_paths: RepositoryPaths, package_ahriman: Package, mocker: MockerFixture) -> None:
@ -293,11 +323,11 @@ def test_tree_create(repository_paths: RepositoryPaths, mocker: MockerFixture) -
and not callable(getattr(repository_paths, prop))
}
mkdir_mock = mocker.patch("pathlib.Path.mkdir")
chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown")
owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner")
repository_paths.tree_create()
mkdir_mock.assert_has_calls([MockCall(mode=0o755, parents=True, exist_ok=True) for _ in paths], any_order=True)
chown_mock.assert_has_calls([MockCall(pytest.helpers.anyvar(int)) for _ in paths], any_order=True)
owner_guard_mock.assert_called_once_with()
def test_tree_create_skip(mocker: MockerFixture) -> None: