From 9d0de93d3985042a45f2263f4c0fc94f9fcdbbbd Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 16 Jul 2026 14:25:28 -0700 Subject: [PATCH 1/2] Prevent path traversal in control server patch applier apply_code_patch joined the patch path onto the target directory without any containment check, so a model-code or package patch with a path like ../../../etc/cron.d/x (or an absolute path) could write or delete files outside the model/packages directory via the control server's POST /control/patch endpoint. Resolve the joined path and reject any result that escapes the target directory. Legitimate nested paths within the target are unaffected. Fixes #2532 --- .../truss_patch/model_code_patch_applier.py | 21 +++++- .../test_model_container_patch_applier.py | 70 +++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py b/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py index 78ca3a36e..f519cfae7 100644 --- a/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py +++ b/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py @@ -13,9 +13,28 @@ from truss.templates.control.control.helpers.custom_types import Action, Patch +def _resolve_within(base_dir: Path, relative_path: str) -> Path: + """Resolve ``relative_path`` against ``base_dir`` and ensure the result + stays inside ``base_dir``. + + The patch path comes from the request body of the control server's + ``/control/patch`` endpoint, so a value such as ``../../../etc/cron.d/x`` + (or an absolute path, which ``/`` join would leave unchanged) must not be + allowed to write or delete files outside the target directory. + """ + base_resolved = base_dir.resolve() + filepath = (base_dir / relative_path).resolve() + if not filepath.is_relative_to(base_resolved): + raise ValueError( + f"Invalid patch path {relative_path!r}: resolves outside the " + f"target directory {base_resolved}." + ) + return filepath + + def apply_code_patch(relative_dir: Path, patch: Patch, logger: logging.Logger): logger.debug(f"Applying code patch {patch.to_dict()}") - filepath: Path = relative_dir / patch.path + filepath: Path = _resolve_within(relative_dir, patch.path) action = patch.action if action in [Action.ADD, Action.UPDATE]: diff --git a/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py b/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py index 2f56e66de..96095d43e 100644 --- a/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py +++ b/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py @@ -168,6 +168,76 @@ def test_patch_applier_env_var_patch_remove(patch_applier: ModelContainerPatchAp _ = env_var_dict["FOO"] +@pytest.mark.parametrize( + "patch_type, body_cls", + [(PatchType.MODEL_CODE, ModelCodePatch), (PatchType.PACKAGE, PackagePatch)], +) +@pytest.mark.parametrize("action", [Action.ADD, Action.UPDATE]) +def test_patch_applier_rejects_path_traversal_write( + patch_applier: ModelContainerPatchApplier, + truss_container_fs, + patch_type, + body_cls, + action, +): + # A patch path escaping the target directory (e.g. via `..`) must be + # rejected instead of writing outside it (issue #2532). + outside_target = truss_container_fs / "escaped.py" + patch = Patch( + type=patch_type, + body=body_cls(action=action, path="../../escaped.py", content="pwned"), + ) + with pytest.raises(ValueError): + patch_applier(patch, os.environ.copy()) + assert not outside_target.exists() + + +def test_patch_applier_rejects_absolute_path_write( + patch_applier: ModelContainerPatchApplier, truss_container_fs, tmp_path +): + # Joining an absolute path onto the target dir collapses to the absolute + # path, another way to escape; it must be rejected too. + outside_target = tmp_path / "abs_escaped.py" + patch = Patch( + type=PatchType.MODEL_CODE, + body=ModelCodePatch( + action=Action.ADD, path=str(outside_target), content="pwned" + ), + ) + with pytest.raises(ValueError): + patch_applier(patch, os.environ.copy()) + assert not outside_target.exists() + + +def test_patch_applier_rejects_path_traversal_remove( + patch_applier: ModelContainerPatchApplier, truss_container_fs +): + # A REMOVE patch must not be able to delete files outside the target dir. + victim = truss_container_fs / "app" / "config.yaml" + assert victim.exists() + patch = Patch( + type=PatchType.MODEL_CODE, + body=ModelCodePatch(action=Action.REMOVE, path="../config.yaml"), + ) + with pytest.raises(ValueError): + patch_applier(patch, os.environ.copy()) + assert victim.exists() + + +def test_patch_applier_allows_nested_subdirectory_path( + patch_applier: ModelContainerPatchApplier, truss_container_fs +): + # Legitimate nested paths within the target dir must still work. + patch = Patch( + type=PatchType.MODEL_CODE, + body=ModelCodePatch(action=Action.ADD, path="nested/dir/new.py", content="ok"), + ) + patch_applier(patch, os.environ.copy()) + assert ( + truss_container_fs / "app" / "model" / "nested" / "dir" / "new.py" + ).read_text() == "ok" + + def test_patch_applier_external_data_patch_add( patch_applier: ModelContainerPatchApplier, truss_container_fs ): From 2db3c64e5ae51acd5a65e54ec2adeb09afc12b21 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 10 Aug 2026 00:51:06 -0700 Subject: [PATCH 2/2] Return unresolved path from containment check to preserve patching behavior Address review concern about side effects of handing a fully-resolved (qualified) path to the file operations: the containment check now uses resolved copies internally but returns the plain relative_dir / patch.path join, so write/delete behavior and log output during live reload (truss push --watch / truss watch) are byte-for-byte identical to before for legitimate patches, including when the app dir sits behind a symlink. Adds a regression test applying a patch through a symlinked app dir. Co-Authored-By: Claude Fable 5 --- .../truss_patch/model_code_patch_applier.py | 21 ++++++++++------- .../test_model_container_patch_applier.py | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py b/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py index f519cfae7..66cc3b4f2 100644 --- a/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py +++ b/truss/templates/control/control/helpers/truss_patch/model_code_patch_applier.py @@ -13,28 +13,33 @@ from truss.templates.control.control.helpers.custom_types import Action, Patch -def _resolve_within(base_dir: Path, relative_path: str) -> Path: - """Resolve ``relative_path`` against ``base_dir`` and ensure the result - stays inside ``base_dir``. +def _validate_within(base_dir: Path, relative_path: str) -> Path: + """Ensure ``relative_path`` joined onto ``base_dir`` stays inside + ``base_dir``, then return the plain join. The patch path comes from the request body of the control server's ``/control/patch`` endpoint, so a value such as ``../../../etc/cron.d/x`` (or an absolute path, which ``/`` join would leave unchanged) must not be allowed to write or delete files outside the target directory. + + Resolved copies are used only for the containment check; the returned + path is ``base_dir / relative_path`` exactly as before this check was + introduced, so downstream file operations (and their log output) are + unchanged for legitimate patches, including when ``base_dir`` sits + behind a symlink. """ - base_resolved = base_dir.resolve() - filepath = (base_dir / relative_path).resolve() - if not filepath.is_relative_to(base_resolved): + filepath = base_dir / relative_path + if not filepath.resolve().is_relative_to(base_dir.resolve()): raise ValueError( f"Invalid patch path {relative_path!r}: resolves outside the " - f"target directory {base_resolved}." + f"target directory {base_dir}." ) return filepath def apply_code_patch(relative_dir: Path, patch: Patch, logger: logging.Logger): logger.debug(f"Applying code patch {patch.to_dict()}") - filepath: Path = _resolve_within(relative_dir, patch.path) + filepath: Path = _validate_within(relative_dir, patch.path) action = patch.action if action in [Action.ADD, Action.UPDATE]: diff --git a/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py b/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py index 96095d43e..a07786501 100644 --- a/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py +++ b/truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py @@ -238,6 +238,29 @@ def test_patch_applier_allows_nested_subdirectory_path( ).read_text() == "ok" +def test_patch_applier_works_through_symlinked_app_dir(truss_container_fs, tmp_path): + # The containment check must not alter behavior when the inference server + # home is reached via a symlink: the patch is applied through the original + # (unresolved) path, exactly as before the check was introduced. + symlinked_app = tmp_path / "app_link" + symlinked_app.symlink_to(truss_container_fs / "app") + applier = ModelContainerPatchApplier(symlinked_app, mock.Mock()) + patch = Patch( + type=PatchType.MODEL_CODE, + body=ModelCodePatch(action=Action.UPDATE, path="model.py", content="updated"), + ) + applier(patch, os.environ.copy()) + assert (truss_container_fs / "app" / "model" / "model.py").read_text() == "updated" + # Traversal is still rejected through the symlink. + evil = Patch( + type=PatchType.MODEL_CODE, + body=ModelCodePatch(action=Action.ADD, path="../../escaped.py", content="x"), + ) + with pytest.raises(ValueError): + applier(evil, os.environ.copy()) + assert not (truss_container_fs / "escaped.py").exists() + + def test_patch_applier_external_data_patch_add( patch_applier: ModelContainerPatchApplier, truss_container_fs ):