Compare commits

...

3 Commits

Author SHA1 Message Date
Alex Blank 9f8058223a Merge branch 'dev' of ssh://git.commumedia.org:2222/alex/headquarter into dev 2026-05-29 15:32:38 +02:00
Alex Blank b483a34517 fix: skip read-only mounts in permission fixer and remove redundant ssh_keys manifest mount
- apply_mount_permissions now skips mounts with readonly=true to avoid
  'Read-only file system' warnings on post-start chown/chmod
- Removed the ssh_keys mount from the pi-agent manifest definition;
  instance-level SSH key mounting now handles this exclusively
- Added unit test for read-only mount skipping

Quality gates: pytest (15 passed)
2026-05-29 15:32:00 +02:00
Alex Blank de8c47c81c fix: deep-merge manifest with base to resolve container user UID/GID
- start_instance now deep-merges manifest with base definition before extracting user.uid/user.gid
- The user config is typically defined in the base image (ubuntu-24.04-dev), not the extending manifest
- Add debug logging to verify resolved uid/gid/home_dir
- Add logging to prepare_ssh_key_files for chown success/failure visibility
- Log current process uid when chown fails to diagnose permission issues

Quality gates: pytest 239 passed (6 pre-existing failures), tsc --noEmit clean
2026-05-29 14:49:24 +02:00
6 changed files with 67 additions and 19 deletions
@@ -232,14 +232,6 @@ def upgrade() -> None:
"writable": True,
"owner": "user",
},
{
"name": "ssh_keys",
"target": "/home/user/.ssh",
"source_type": "ssh_key",
"mode": "0700",
"file_mode": "0600",
"readonly": True,
},
{
"name": "pi_state",
"target": "/tmp/.pi/agents",
+16
View File
@@ -1341,11 +1341,27 @@ async def start_instance(
manifest_def = await session.get(ToolDefinitionManifest, tool_type.manifest_id)
if manifest_def:
manifest = dict(manifest_def.manifest)
# Merge with base definition if referenced (user config is often in base)
if manifest_def.base_definition_id:
base_def = await session.get(
ToolDefinitionManifest, manifest_def.base_definition_id
)
if base_def:
manifest = resolve_base(
deep_merge(dict(base_def.manifest), manifest)
)
home_dir = get_manifest_home_dir(manifest)
user_cfg = manifest.get("user")
if user_cfg:
container_uid = user_cfg.get("uid", 0)
container_gid = user_cfg.get("gid", 0)
logger.debug(
"Manifest user resolved for instance %s: uid=%s, gid=%s, home=%s",
instance.id,
container_uid,
container_gid,
home_dir,
)
# Apply selected config profile if any
instance_dir = os.path.dirname(instance.compose_path)
+11
View File
@@ -40,6 +40,17 @@ def apply_mount_permissions(
"error": None,
}
# Skip read-only mounts — their permissions cannot be changed
# post-start because the bind mount is locked.
if mount.get("readonly", False):
logger.debug(
"Skipping permission fix for read-only mount %s (target=%s)",
name,
target,
)
results.append(result)
continue
# Skip if no permission policy defined
if not owner and not mode and not file_mode:
results.append(result)
+17 -4
View File
@@ -1,5 +1,6 @@
"""SSH key service utilities for preparing keys for container use."""
import logging
import os
from pathlib import Path
@@ -7,6 +8,8 @@ from cryptography.fernet import Fernet
from src.config import Settings
logger = logging.getLogger(__name__)
def _get_fernet() -> Fernet:
"""Generate a valid Fernet key from the session secret."""
@@ -75,10 +78,20 @@ def prepare_ssh_key_files(
os.chown(private_key_path, effective_uid, effective_gid)
os.chown(public_key_path, effective_uid, effective_gid)
os.chown(config_path, effective_uid, effective_gid)
except PermissionError:
# API process may not be running as root; permission fixer will
# handle this post-start if the mount is read-write
pass
logger.debug(
"Set SSH key ownership to uid=%s gid=%s for %s",
effective_uid,
effective_gid,
ssh_dir,
)
except PermissionError as exc:
logger.warning(
"Cannot chown SSH keys to uid=%s gid=%s (running as uid=%s): %s",
effective_uid,
effective_gid,
os.getuid(),
exc,
)
return str(ssh_dir)
@@ -64,6 +64,24 @@ class TestApplyMountPermissions:
"find /home/user/.ssh -type f -exec chmod 0600" in file_mode_call[0][1][2]
)
@patch("src.services.permission_fixer._run_in_container")
def test_skips_readonly_mount(self, mock_run) -> None:
mounts = [
{
"name": "ssh_keys",
"target": "/home/user/.ssh",
"readonly": True,
"mode": "0700",
"file_mode": "0600",
},
]
results = apply_mount_permissions("abc123", mounts)
assert len(results) == 1
assert results[0]["mount_name"] == "ssh_keys"
assert results[0]["success"] is True
mock_run.assert_not_called()
@patch("src.services.permission_fixer._run_in_container")
def test_skips_mount_with_no_policy(self, mock_run) -> None:
mounts = [
+5 -7
View File
@@ -13,7 +13,9 @@ class TestPrepareSshKeyFiles:
"""Tests for prepare_ssh_key_files."""
@patch("src.services.ssh_keys._get_fernet")
def test_creates_files_with_default_permissions(self, mock_fernet, tmp_path) -> None:
def test_creates_files_with_default_permissions(
self, mock_fernet, tmp_path
) -> None:
mock_fernet.return_value.decrypt.return_value = b"private-key-content"
ssh_key = MagicMock()
ssh_key.private_key_encrypted = "enc"
@@ -35,9 +37,7 @@ class TestPrepareSshKeyFiles:
ssh_key.public_key = "ssh-ed25519 AAA test@test"
with patch("os.chown") as mock_chown:
ssh_dir = prepare_ssh_key_files(
str(tmp_path), ssh_key, uid=1001, gid=1001
)
ssh_dir = prepare_ssh_key_files(str(tmp_path), ssh_key, uid=1001, gid=1001)
# os.chown is called for the directory and each of the 3 files
assert mock_chown.call_count == 4
@@ -56,8 +56,6 @@ class TestPrepareSshKeyFiles:
with patch("os.chown", side_effect=PermissionError("not allowed")):
# Should not raise
ssh_dir = prepare_ssh_key_files(
str(tmp_path), ssh_key, uid=1001, gid=1001
)
ssh_dir = prepare_ssh_key_files(str(tmp_path), ssh_key, uid=1001, gid=1001)
assert Path(ssh_dir).exists()