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
This commit is contained in:
@@ -1341,11 +1341,25 @@ async def start_instance(
|
|||||||
manifest_def = await session.get(ToolDefinitionManifest, tool_type.manifest_id)
|
manifest_def = await session.get(ToolDefinitionManifest, tool_type.manifest_id)
|
||||||
if manifest_def:
|
if manifest_def:
|
||||||
manifest = dict(manifest_def.manifest)
|
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)
|
home_dir = get_manifest_home_dir(manifest)
|
||||||
user_cfg = manifest.get("user")
|
user_cfg = manifest.get("user")
|
||||||
if user_cfg:
|
if user_cfg:
|
||||||
container_uid = user_cfg.get("uid", 0)
|
container_uid = user_cfg.get("uid", 0)
|
||||||
container_gid = user_cfg.get("gid", 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
|
# Apply selected config profile if any
|
||||||
instance_dir = os.path.dirname(instance.compose_path)
|
instance_dir = os.path.dirname(instance.compose_path)
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
"""SSH key service utilities for preparing keys for container use."""
|
"""SSH key service utilities for preparing keys for container use."""
|
||||||
|
|
||||||
|
import logging
|
||||||
import os
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -7,6 +8,8 @@ from cryptography.fernet import Fernet
|
|||||||
|
|
||||||
from src.config import Settings
|
from src.config import Settings
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
def _get_fernet() -> Fernet:
|
def _get_fernet() -> Fernet:
|
||||||
"""Generate a valid Fernet key from the session secret."""
|
"""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(private_key_path, effective_uid, effective_gid)
|
||||||
os.chown(public_key_path, effective_uid, effective_gid)
|
os.chown(public_key_path, effective_uid, effective_gid)
|
||||||
os.chown(config_path, effective_uid, effective_gid)
|
os.chown(config_path, effective_uid, effective_gid)
|
||||||
except PermissionError:
|
logger.debug(
|
||||||
# API process may not be running as root; permission fixer will
|
"Set SSH key ownership to uid=%s gid=%s for %s",
|
||||||
# handle this post-start if the mount is read-write
|
effective_uid,
|
||||||
pass
|
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)
|
return str(ssh_dir)
|
||||||
|
|
||||||
|
|||||||
@@ -13,7 +13,9 @@ class TestPrepareSshKeyFiles:
|
|||||||
"""Tests for prepare_ssh_key_files."""
|
"""Tests for prepare_ssh_key_files."""
|
||||||
|
|
||||||
@patch("src.services.ssh_keys._get_fernet")
|
@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"
|
mock_fernet.return_value.decrypt.return_value = b"private-key-content"
|
||||||
ssh_key = MagicMock()
|
ssh_key = MagicMock()
|
||||||
ssh_key.private_key_encrypted = "enc"
|
ssh_key.private_key_encrypted = "enc"
|
||||||
@@ -35,9 +37,7 @@ class TestPrepareSshKeyFiles:
|
|||||||
ssh_key.public_key = "ssh-ed25519 AAA test@test"
|
ssh_key.public_key = "ssh-ed25519 AAA test@test"
|
||||||
|
|
||||||
with patch("os.chown") as mock_chown:
|
with patch("os.chown") as mock_chown:
|
||||||
ssh_dir = prepare_ssh_key_files(
|
ssh_dir = prepare_ssh_key_files(str(tmp_path), ssh_key, uid=1001, gid=1001)
|
||||||
str(tmp_path), ssh_key, uid=1001, gid=1001
|
|
||||||
)
|
|
||||||
|
|
||||||
# os.chown is called for the directory and each of the 3 files
|
# os.chown is called for the directory and each of the 3 files
|
||||||
assert mock_chown.call_count == 4
|
assert mock_chown.call_count == 4
|
||||||
@@ -56,8 +56,6 @@ class TestPrepareSshKeyFiles:
|
|||||||
|
|
||||||
with patch("os.chown", side_effect=PermissionError("not allowed")):
|
with patch("os.chown", side_effect=PermissionError("not allowed")):
|
||||||
# Should not raise
|
# Should not raise
|
||||||
ssh_dir = prepare_ssh_key_files(
|
ssh_dir = prepare_ssh_key_files(str(tmp_path), ssh_key, uid=1001, gid=1001)
|
||||||
str(tmp_path), ssh_key, uid=1001, gid=1001
|
|
||||||
)
|
|
||||||
|
|
||||||
assert Path(ssh_dir).exists()
|
assert Path(ssh_dir).exists()
|
||||||
|
|||||||
Reference in New Issue
Block a user