Skip to content

Commit 64a6101

Browse files
toderianclaude
andauthored
Feat volume isolation (#391)
* feat: deterministic container naming and stale container guardrail Make container_name = cfg_instance_id (was cfg_instance_id + random suffix). This ensures 1 plugin = 1 container identity, stable across restarts. Add _ensure_no_stale_container() that queries Docker by name and force-removes any existing container before starting a new one. Covers crash recovery where self.container reference is lost but a Docker container still exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add fixed_volume.py module for file-backed volume isolation Standalone module adapted from the volume_isolation PoC. Provides: - FixedVolume dataclass with img_path, mount_path, meta_path properties - provision(): fallocate + mkfs.ext4 -m 0 + losetup + mount (idempotent) - cleanup(): umount + losetup -d (graceful, never raises) - cleanup_stale_mounts(): recovers orphaned loop devices from prior crashes - Size mismatch detection (warns but refuses to resize) - Removes lost+found/ on fresh volumes - All functions accept logger callable (defaults to print) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add FIXED_SIZE_VOLUMES config key and _fixed_volumes instance variable Add FIXED_SIZE_VOLUMES to _CONFIG (defaults to {}) for configuring file-backed, size-limited volumes. Mark VOLUMES as @deprecated in comment. Add _fixed_volumes list to __reset_vars() for tracking provisioned volumes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: deprecate VOLUMES config with warning Add deprecation warning (red) to _configure_volumes() when VOLUMES is non-empty. Existing functionality is unchanged -- dirs still created, permissions still set, self.volumes still populated. Users should migrate to FIXED_SIZE_VOLUMES for size-limited, isolated volumes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: implement _configure_fixed_size_volumes and _cleanup_fixed_size_volumes Add mixin methods to _ContainerUtilsMixin in container_utils.py: _configure_fixed_size_volumes(): - Validates FIXED_SIZE_VOLUMES config entries (SIZE, MOUNTING_POINT required) - Checks for required tools (fallocate, mkfs.ext4, losetup, mount, etc.) - Recovers stale mounts from prior crashes via cleanup_stale_mounts() - Detects orphaned volumes (in meta/ but not in config) and warns - Provisions each volume (idempotent) and populates self.volumes - Cleans up already-provisioned volumes on partial failure _cleanup_fixed_size_volumes(): - Unmounts and detaches loop devices for all provisioned volumes - Continues cleanup even if individual volumes fail - Clears self._fixed_volumes list Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: wire fixed-size volumes into plugin lifecycle Call _configure_fixed_size_volumes() in on_init() and _restart_container() after the existing volume configuration methods. Call _cleanup_fixed_size_volumes() in _stop_container_and_save_logs_to_disk() after stop_container() to free loop devices. Lifecycle order: stop container (release file handles) -> unmount volumes -> detach loop devices -> save logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add unit and integration tests for fixed-size volume isolation 31 tests covering: - FixedVolume dataclass path properties - Size string parsing (K/M/G/T/bytes) - docker_bind_spec format - Tool requirement checking - Image creation (new + existing + size mismatch) - Loop device attachment (reuse + new) - Mount skipping when already mounted - Cleanup graceful error handling - Stale mount recovery (edge node restart case) - Provision full flow (new volume + re-mount) - Mixin _configure_fixed_size_volumes (empty, missing fields, tools, success) - Mixin _cleanup_fixed_size_volumes (empty, multiple, failure continuation) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add comprehensive lifecycle integration tests Extend test infrastructure and add 51 lifecycle integration tests that emulate the edge node environment with mocked Docker client: support.py changes: - Extend _DummyBasePlugin with semaphore stubs, diskapi, tunnel methods - Add make_mock_container() and make_mock_docker_client() helpers - Add make_lifecycle_runner() factory with all __reset_vars attributes, state machine, restart tracking, resource limits, health check state test_container_lifecycle.py tests cover: - Init phase (state, naming, defaults) - First launch (docker run, image check, stale container, name, volumes, env) - Running state (status check, crash detection, normal exit, failure counting) - Restart flow (stop old, start new, state transitions, failure preservation) - Stop and close (docker stop/remove, log saving, graceful cleanup) - Stale container guardrail (remove running/exited, noop, error handling) - Process loop (launch, status check, crash->restart with backoff, paused, restart policy "no", max retries, multiple healthy iterations) - Fixed-size volumes (provision before start, cleanup on stop, reprovision on restart, graceful degradation with missing tools) - VOLUMES deprecation warning - Full end-to-end lifecycle (launch -> run -> crash -> restart -> run -> close) - Multiple crash failure counter accumulation Total: 119 tests (37 config + 31 fixed_volume + 51 lifecycle) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add exponential backoff with jitter for image pull retries When multiple container_app_runner plugins on the same edge node pull images simultaneously, DockerHub rate-limits (429) cause all pulls to fail and retry at the same time (thundering herd). This adds exponential backoff with random jitter so retries spread across time. Design: - Backoff formula: base * 2^(failures-1) + uniform(0, base * 2^(failures-1)) - No max cap -- exponential growth naturally spaces out retries - Jitter ensures each plugin picks a different retry time - 100 max attempts before giving up (configurable, 0 = unlimited) - Integrates with process() loop (no blocking sleep) - On success, all counters reset Config keys: - IMAGE_PULL_MAX_RETRIES: 100 (max attempts, 0=unlimited) - IMAGE_PULL_BACKOFF_BASE: 2 (base delay in seconds) Methods added: - _calculate_image_pull_backoff() - _record_image_pull_failure() - _record_image_pull_success() - _is_image_pull_backoff_active() - _has_exceeded_image_pull_retries() 12 new tests covering backoff behavior, jitter randomness, counter reset, max retries, no-cap growth, and integration with pull method. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: change IMAGE_PULL_BACKOFF_BASE default to 20s, add timing docs Change base delay from 2s to 20s for more practical spacing when DockerHub rate-limits. Add timing table to _calculate_image_pull_backoff docstring showing delay progression at each failure count. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: handle Docker returning string 'None' for container IP Docker daemon can return empty string or string "None" instead of actual None for IPAddress in NetworkSettings. Guard against both cases to avoid using invalid values as container IP. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add FIXED_SIZE_VOLUMES storage validation in deeploy Add backend validation for fixed-size volume storage allocation: - _aggregate_fixed_size_volumes_storage_mb(): sums SIZE values across all FIXED_SIZE_VOLUMES entries in a plugin config - _aggregate_container_resources(): now includes storage aggregation alongside CPU/memory, for both legacy and modern plugin formats - Storage validation in deeploy_check_payment_and_job_owner(): rejects requests where FIXED_SIZE_VOLUMES total exceeds the job type's storage allocation from JOB_TYPE_RESOURCE_SPECS (uses <= not ==, allowing partial allocation) - _validate_fixed_size_volumes(): format validation ensuring each entry has parseable SIZE > 0 and non-empty MOUNTING_POINT Replaces the TODO comment about disk validation with actual implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve volume paths to absolute for losetup/mount commands get_data_folder() can return a relative path. losetup and mount require absolute paths. Use Path.resolve() on the root to ensure all derived paths (img_path, mount_path, meta_path) are absolute. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: ensure loop device nodes exist before losetup In Docker-in-Docker environments, only /dev/loop0-8 may exist as device nodes, and they can all be in use by the host (e.g., snap packages). losetup -f fails with misleading "No such file or directory" when no free device node is available. Add _ensure_loop_device_nodes() that creates /dev/loopN nodes (up to 64) using mknod before calling losetup. Called automatically from attach_loop(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use auto remove for CAR * fix: support fractional size suffixes in _parse_size_to_bytes Accept values like '0.5G' by casting through float before applying the unit multiplier, so FIXED_SIZE_VOLUMES entries smaller than 1G no longer fail provisioning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(car): auto-detect OWNER_UID/GID for FIXED_SIZE_VOLUMES from image USER When a container image has a non-root USER directive, the fixed-size ext4 volume was mounted root:root 755 and the non-root container user got "Permission denied" writing to it. Now, if OWNER_UID / OWNER_GID are not explicitly set in the volume config, we inspect the image and resolve its USER to numeric uid:gid (directly or via an ephemeral getent passwd / cat /etc/passwd lookup). Images that run as root continue to get root-owned mounts unchanged. Also hoists _ensure_image_available() to run before _configure_*_volumes so the image is guaranteed local for introspection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore(car): finish FIXED_SIZE_VOLUMES mixin extraction + unblock sdk COPY - Remove dead _configure_fixed_size_volumes / _cleanup_fixed_size_volumes from _ContainerUtilsMixin; they now live in _FixedSizeVolumesMixin and are composed into ContainerAppRunnerPlugin in the previous commit. - Drop **/ratio1_* from .dockerignore so builds that COPY ./ratio1_sdk (e.g. tvitalii/edge_node:testnet via tools/build-and-push) can see the SDK in the build context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * revert: restore .dockerignore exclude for **/ratio1_* The previous commit dropped this to unblock tools/build-and-push, but it was a local-only concern and shouldn't live in the repo. Builds that need ratio1_sdk in the context can override .dockerignore out-of-band (e.g. tools/build-and-push already patches the file temporarily). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(car): hoist inline imports, use self.np.random for jitter Drop `import random` inside _calculate_pull_backoff and switch to self.np.random.uniform — matches the canonical RNG pattern exposed by BasePlugin (self.np). Hoist Path and fixed_volume imports in fixed_size_volumes_mixin to module level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(car): split backoff into 3 family mixins, introduce mixins/ folder Extract 18 backoff methods from ContainerAppRunnerPlugin into three family-scoped mixins under a new mixins/ package: - _RestartBackoffMixin (7 methods) — container restart backoff - _ImagePullBackoffMixin (5 methods) — image pull backoff with jitter - _TunnelBackoffMixin (6 methods) — per-port tunnel restart backoff Also relocate _FixedSizeVolumesMixin into mixins/ for consistency (git mv preserves blame). _ContainerUtilsMixin stays at its current path. State init and cfg_* declarations remain on the plugin; the mixins contribute behavior only. Plugin file shrinks by ~410 LOC. No behavior change. tests/support.py: inject plugin.np from numpy so the dummy BasePlugin exposes the same RNG surface as production (BasePlugin → _UtilsBaseMixin). This unbreaks the 7 TestImagePullBackoff tests that the prior self.np.random.uniform switch (605f021) silently broke. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(car): remove _get_instance_data_subfolder override, route logs to logs/ BasePluginExecutor now provides _get_instance_data_subfolder returning `pipelines_data/{sid}/{iid}`. Drop the CAR override and the _CONTAINER_APPS_SUBFOLDER constant; stop passing an explicit subfolder to diskapi pickle calls since diskapi auto-routes. Persistent state lands at pipelines_data/{sid}/{iid}/plugin_data/persistent_state.pkl. Container logs now use subfolder='logs' → pipelines_data/{sid}/{iid}/logs/container_logs.pkl. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(car): reroute FILE_VOLUMES under pipelines_data instance folder File volumes now live at `{data_folder}/pipelines_data/{sid}/{iid}/file_volumes/{logical_name}/{filename}` instead of the shared `container_volumes/{instance_id}_*/` directory. The instance_id prefix is dropped because the parent path is already instance-scoped. Legacy VOLUMES (deprecated) keeps using CONTAINER_VOLUMES_PATH untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(car): unit tests for diskapi isolation + update stubs for new paths Adds tests/test_diskapi_isolation.py (21 tests) covering sanitization of pathological stream_id/instance_id, helper methods, pickle save/load auto-routing, flat-path fallback with deprecation warning, cross-plugin isolation warning, tier-1 traversal rejection, restricted-location rejection, and bare-mixin degradation. Tests load diskapi.py directly via importlib to sidestep the package's matplotlib/numpy transitive import. Updates tests/support.py and test_worker_app_runner.py stubs with `_get_instance_data_subfolder`, `_safe_path_component`, and `get_data_folder` so the CAR plugin (which no longer overrides _get_instance_data_subfolder) resolves correctly. Two previously-failing FILE_VOLUMES tests now pass; they patch `plugin.get_data_folder` instead of the removed CONTAINER_VOLUMES_PATH route. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(car): absolutize FILE_VOLUMES base path; bump ver to 2.10.170 `self.get_data_folder()` returns a relative path (logger stores _data_dir un-abspath'd), which Docker rejects for bind mounts. Wrap with os.path.abspath. Bumps ver.py to 2.10.170 so the dAuth version check on devnet passes; we were behind main on this bump. Discovered while running Phase 7 e2e suite (scenario 02 file volumes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tutorials/edge_node_api_test): add diskapi endpoints for e2e coverage Extends EdgeNodeApiTestPlugin with HTTP endpoints that wrap the _DiskAPIMixin save/load methods (pickle / json / dataframe) plus a delete_file helper and a GET /whoami that exposes the resolved per-instance path layout. These endpoints let the project_r1_edge_node diskapi_path_reorg e2e suite exercise the full live production path -- SDK deploy -> FastApiWebAppPlugin -> uvicorn -> diskapi_save_* -> on-disk file -- and assert the new pipelines_data/{sid}/{iid}/plugin_data/ layout, including the tier-1 hard reject of deep `..` traversal attempts sent through a user-controlled `subfolder` parameter. Uses plugin built-in accessors (self.pd, self.os_path, self.diskapi_delete_file) instead of top-level imports so the SECURED-mode safety check (_perform_module_safety_check) accepts this plugin. Version: 0.1.0.0 -> 0.2.0.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * security: add safe_path_component utility and harden volume path construction Add safe_path_component() standalone function in fixed_volume.py that sanitizes a single path component via regex + os.path.realpath containment check. Returns '_' for any input that would escape a parent directory ('', '.', '..', embedded separators, symlinks). Apply it to: - FILE_VOLUMES logical names and filenames (container_utils.py) - FIXED_SIZE_VOLUMES logical names (fixed_size_volumes.py) - Add realpath containment check before mkdir in file volume provisioning - Add __post_init__ validation in FixedVolume dataclass (deepest defense) Previously these paths only used sanitize_name() (a variable-name normalizer that allows '.' and '..' through) or no sanitization at all, allowing a logical_name of '..' to escape one directory level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(car): qualify container name with stream_id + sanitize Previously the container name was just `cfg_instance_id`, and start_container force-removes any existing container with that name. Two plugin instances that happen to share an INSTANCE_ID across different pipelines could stomp each other's live containers. Add _compute_container_name(stream_id, instance_id) that builds safe_path_component(f"{stream_id}_{instance_id}") with a "car_" prefix to guarantee a Docker-valid leading character even when inputs are empty or sanitized down to "_". Tests: - New test_container_app_runner_name.py covers collision, sanitization, traversal, empty-input, and determinism cases. - test_container_lifecycle expectations updated for the new name shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fixed_volume): exact mountpoint match against /proc/mounts Substring `in` matching on /proc/mounts could silently alias sibling paths sharing a prefix: a mount at `.../data2` made `.../data` look mounted, so provisioning skipped the real mount step and Docker bind-mounted the plain host directory instead of the loop-backed filesystem -- invalidating the ENOSPC isolation guarantee. Add `_is_path_mounted()` which parses each /proc/mounts line, unescapes the octal sequences the kernel writes for whitespace/backslashes, and compares the mountpoint exactly. Use it from mount_volume() and cleanup_stale_mounts() in place of the substring check. Tests: - New TestIsPathMounted covers exact match, prefix-sibling false positive, trailing-slash normalization, escaped spaces, malformed lines, and unreadable /proc/mounts. - New test_does_not_alias_prefix_sibling_mount on mount_volume and test_prefix_sibling_does_not_trigger_cleanup on cleanup_stale_mounts cover the end-to-end regression scenarios. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fixed_volumes): reject post-sanitization name collisions safe_path_component() maps any non-word char to `_`, so two configured volumes like `"a/b"` and `"a?b"` both normalize to `"a_b"` and silently alias the same image/meta/mount paths, breaking the isolation guarantee. Reject this at startup: before provisioning, group the configured logical names by their sanitized form and raise ValueError when any bucket contains more than one name. The existing outer try/except around _configure_fixed_size_volumes surfaces this as a clear config error instead of two volumes silently sharing storage. Tests: new test_fixed_size_volumes_mixin.py covers distinct names (no raise), colliding names (raise with both logicals in the message), and empty config (no-op). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(fixed_volumes): resolve image owner from metadata, never run the image Previously ownership auto-detection launched a throwaway container from the user-supplied image to read /etc/passwd. That expanded the execution surface of volume provisioning to user code before the main runtime start path, changing the threat model for something that's conceptually pre-start plumbing. Rewrite _resolve_image_owner to inspect image metadata only via docker_client.images.get. Supported resolutions: - empty / "root" / "0" / "0:0" -> (None, None), root-owned default - "1000" or "1000:2000" -> numeric, used directly - symbolic ("appuser") -> (None, None) + warning; user must set OWNER_UID/OWNER_GID explicitly Delete _lookup_passwd_in_image, _lookup_group_in_image, _run_throwaway -- the user's image is no longer executed during volume provisioning. Tests: new ResolveImageOwnerTests in test_fixed_size_volumes_mixin.py cover each case and assert containers.run is never called. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(car): one-time move-and-cleanup migration for legacy CAR data Pre-refactor the plugin wrote persistent state and container logs to {data_folder}/container_apps/{plugin_id}/. The PR's isolation refactor routes these through diskapi to {data_folder}/pipelines_data/{sid}/{iid}/plugin_data/ by default, but provided no migration -- manually_stopped flags and co-located logs reset silently on upgrade. Add _migrate_legacy_car_data invoked once at the top of __reset_vars: - if container_apps/{plugin_id}/ exists, move each entry into the new auto-routed plugin_data/ dir (destination wins on conflict) - delete the legacy dir and the container_apps/ wrapper when empty - idempotent: absence of the legacy dir is a no-op - failure-tolerant: any exception is logged and startup continues Tests: new test_legacy_car_migration.py covers happy path, legacy absent, destination-conflict (new wins, warning), move failure (no raise, warning), and second-run idempotency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(car): drop auto_remove=True on container run With auto_remove=True, Docker silently destroys exited containers, removing post-mortem observability (`docker ps -a` can no longer show the exited container, its logs, or its exit code) and creating a race with stop_container()'s explicit remove() path. Remove the flag. Crash recovery is still covered: - _ensure_no_stale_container() force-removes any prior container with the same name before each containers.run() call - stop_container() continues to remove on graceful stop Test: new test_container_is_not_run_with_auto_remove asserts the flag is absent from the docker-py run kwargs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: increase initial and max backoff time * fix: logs migration * chore: increment version --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 98974f5 commit 64a6101

18 files changed

Lines changed: 4018 additions & 393 deletions

extensions/business/container_apps/container_app_runner.py

Lines changed: 228 additions & 356 deletions
Large diffs are not rendered by default.

extensions/business/container_apps/container_utils.py

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import os
88
import socket
99

10+
from extensions.business.container_apps.fixed_volume import safe_path_component
11+
1012
# Path for container volumes
1113
CONTAINER_VOLUMES_PATH = "/edge_node/_local_cache/_data/container_volumes"
1214

@@ -611,7 +613,10 @@ def _get_container_ip(self):
611613
self.container.reload()
612614
net_settings = self.container.attrs.get('NetworkSettings', {})
613615
# Try top-level IPAddress first (default bridge network)
614-
container_ip = net_settings.get('IPAddress')
616+
container_ip = net_settings.get('IPAddress') or None
617+
# Docker sometimes returns string 'None' instead of actual None
618+
if container_ip and container_ip.lower() == 'none':
619+
container_ip = None
615620
available_keys = list(net_settings.keys())
616621
networks = net_settings.get('Networks', {})
617622
network_names = list(networks.keys())
@@ -919,9 +924,18 @@ def _set_directory_permissions(self, path, mode=0o777):
919924
def _configure_volumes(self):
920925
"""
921926
Processes the volumes specified in the configuration.
927+
928+
.. deprecated::
929+
VOLUMES is deprecated. Use FIXED_SIZE_VOLUMES for size-limited,
930+
isolated volumes with ENOSPC enforcement.
922931
"""
923932
default_volume_rights = "rw"
924933
if hasattr(self, 'cfg_volumes') and self.cfg_volumes and len(self.cfg_volumes) > 0:
934+
self.P(
935+
"WARNING: VOLUMES is deprecated and will be removed in a future version. "
936+
"Use FIXED_SIZE_VOLUMES instead for size-limited, isolated volumes.",
937+
color='r'
938+
)
925939
os.makedirs(CONTAINER_VOLUMES_PATH, exist_ok=True)
926940
self._set_directory_permissions(CONTAINER_VOLUMES_PATH)
927941
for host_path, container_path in self.cfg_volumes.items():
@@ -956,67 +970,83 @@ def _configure_file_volumes(self):
956970
"""
957971
Processes FILE_VOLUMES configuration to create files with specified content
958972
and mount them into the container.
959-
973+
960974
FILE_VOLUMES format:
961975
{
962976
"logical_name": {
963977
"content": "file content here...",
964978
"mounting_point": "/container/path/to/filename.ext"
965979
}
966980
}
967-
981+
968982
The method will:
969983
1. Extract filename from mounting_point
970-
2. Create a directory under CONTAINER_VOLUMES_PATH
984+
2. Create a directory under
985+
{data_folder}/pipelines_data/{stream_id}/{instance_id}/file_volumes/{logical_name}/
971986
3. Write content to a file with the extracted filename
972987
4. Add volume mapping to self.volumes
973988
"""
974989
default_volume_rights = "rw"
975-
990+
976991
if not hasattr(self, 'cfg_file_volumes') or not self.cfg_file_volumes:
977992
return
978-
993+
979994
if not isinstance(self.cfg_file_volumes, dict):
980995
self.P("FILE_VOLUMES must be a dictionary, skipping file volume configuration", color='r')
981996
return
982-
983-
os.makedirs(CONTAINER_VOLUMES_PATH, exist_ok=True)
984-
self._set_directory_permissions(CONTAINER_VOLUMES_PATH)
985-
997+
998+
# Instance-scoped base: {data_folder}/pipelines_data/{sid}/{iid}/file_volumes/
999+
# `get_data_folder()` can return a relative path (logger stores _data_dir
1000+
# un-abspath'd); Docker bind mounts require absolute paths, so resolve
1001+
# here.
1002+
file_volumes_base = self.os_path.abspath(self.os_path.join(
1003+
self.get_data_folder(),
1004+
self._get_instance_data_subfolder(),
1005+
"file_volumes",
1006+
))
1007+
os.makedirs(file_volumes_base, exist_ok=True)
1008+
self._set_directory_permissions(file_volumes_base)
1009+
9861010
for logical_name, file_config in self.cfg_file_volumes.items():
9871011
try:
9881012
# Validate file_config structure
9891013
if not isinstance(file_config, dict):
9901014
self.P(f"FILE_VOLUMES['{logical_name}'] must be a dict with 'content' and 'mounting_point', skipping", color='r')
9911015
continue
992-
1016+
9931017
content = file_config.get('content')
9941018
mounting_point = file_config.get('mounting_point')
995-
1019+
9961020
if content is None:
9971021
self.P(f"FILE_VOLUMES['{logical_name}'] missing 'content' field, skipping", color='r')
9981022
continue
999-
1023+
10001024
if not mounting_point:
10011025
self.P(f"FILE_VOLUMES['{logical_name}'] missing 'mounting_point' field, skipping", color='r')
10021026
continue
1003-
1004-
# Extract filename from mounting_point
1027+
1028+
# Extract filename from mounting_point and sanitize
10051029
mounting_point = str(mounting_point)
10061030
path_parts = mounting_point.rstrip('/').split('/')
1007-
filename = path_parts[-1]
1008-
1031+
filename = safe_path_component(path_parts[-1])
1032+
10091033
if not filename:
10101034
self.P(f"FILE_VOLUMES['{logical_name}'] could not extract filename from mounting_point '{mounting_point}', skipping", color='r')
10111035
continue
1012-
1013-
# Create sanitized directory for this file volume
1014-
sanitized_name = self.sanitize_name(str(logical_name))
1015-
prefixed_name = f"{self.cfg_instance_id}_{sanitized_name}"
1016-
self.P(f" Processing file volume '{logical_name}' → '{prefixed_name}/{filename}' → container '{mounting_point}'")
1017-
1036+
1037+
# Per-volume directory inside the instance-scoped file_volumes folder.
1038+
# No instance_id prefix needed -- parent path is already instance-scoped.
1039+
sanitized_name = safe_path_component(logical_name)
1040+
self.P(f" Processing file volume '{logical_name}' → '{sanitized_name}/{filename}' → container '{mounting_point}'")
1041+
10181042
# Create host directory
1019-
host_volume_dir = self.os_path.join(CONTAINER_VOLUMES_PATH, prefixed_name)
1043+
host_volume_dir = self.os_path.join(file_volumes_base, sanitized_name)
1044+
# Realpath containment: reject if resolved path escapes file_volumes_base
1045+
real_dir = os.path.realpath(host_volume_dir)
1046+
real_base = os.path.realpath(file_volumes_base)
1047+
if not real_dir.startswith(real_base + os.sep) and real_dir != real_base:
1048+
self.P(f"FILE_VOLUMES['{logical_name}'] path escapes base directory, skipping", color='r')
1049+
continue
10201050
try:
10211051
os.makedirs(host_volume_dir, exist_ok=True)
10221052
except PermissionError as exc:
@@ -1073,8 +1103,6 @@ def _configure_file_volumes(self):
10731103
return
10741104

10751105

1076-
### END NEW CONTAINER MIXIN METHODS ###
1077-
10781106
### COMMON CONTAINER UTILITY METHODS ###
10791107
def _setup_env_and_ports(self):
10801108
"""

0 commit comments

Comments
 (0)