Prevent path traversal in control server patch applier (#2532)

[sam-shridhar1950f]

What

Fixes #2532. The control server's apply_code_patch joined the patch path directly onto the target directory with no containment check:

filepath: Path = relative_dir / patch.path

patch.path comes straight from the JSON body of POST /control/patch, so a model-code or package patch with a path like ../../../etc/cron.d/x — or an absolute path, which /-join leaves unchanged — could write (ADD/UPDATE) or delete (REMOVE) files anywhere the process can reach, outside the model/packages directory. Both ModelCodePatch and PackagePatch flow through this function, so both were affected.

As noted on the issue, this endpoint is dev/internal, so this is a defense-in-depth hardening: a patch applier should never touch files outside its target directory, whether the offending path comes from a malicious sender on a reachable control port or simply from buggy tooling that emits an unexpected ... Today the latter silently corrupts or deletes unrelated files instead of failing cleanly.

How

Added _resolve_within(base_dir, relative_path), which resolves the joined path and raises ValueError if the result escapes base_dir (via Path.is_relative_to, available on the project's minimum Python 3.9). apply_code_patch now routes through it. This also rejects absolute-path injection, since an absolute patch.path resolves outside the target. Legitimate nested paths within the target directory are unaffected.

The change is scoped to the reported apply_code_patch sink. The external-data path is already covered by #2486; other sinks in ModelContainerPatchApplier (config write) are left untouched to keep this focused.

Testing

New regression tests in test_model_container_patch_applier.py (all fail on the pre-fix code, pass after):

• ../ traversal on write is rejected and nothing is written — parametrized over ModelCodePatch/PackagePatch × ADD/UPDATE
• absolute-path write is rejected
• ../ traversal on REMOVE is rejected and the outside file survives
• a legitimate nested subdirectory path (nested/dir/new.py) still writes correctly

$ uv run pytest truss/tests/templates/control/control/helpers/test_model_container_patch_applier.py
18 passed

uv run ruff check / ruff format are clean on both files.

[cretz]

Thanks for the contribution! Can you give me some background on how you came across this? Was this an issue you hit or just something you saw browsing open issues?

[sam-shridhar1950f]

Thanks for taking a look! Just something I came across browsing the open issues — I wasn't hitting it in production. The security label on #2532 caught my eye, and since the fix was nicely scoped to a single sink (apply_code_patch) with a clear containment invariant to enforce, it seemed like a good self-contained hardening PR to pick up. Happy to adjust scope or approach however you'd prefer.

[cretz]

I am a bit concerned without having tested this against code patching against the backend (i.e. truss push --watch/truss watch) any side effects it may have giving a qualified path here

[sam-shridhar1950f]

Good catch — the concern was valid: the check previously handed the fully-resolved path to the file operations, which changes what gets written/logged if the app dir sits behind a symlink. Fixed in 2db3c64:

• _validate_within now uses resolved copies only for the containment check and returns the plain relative_dir / patch.path join, so the path handed to mkdir/write/unlink (and the log lines) is byte-for-byte identical to pre-PR behavior for every legitimate patch.
• Added a regression test that applies an UPDATE patch through a symlinked app dir and asserts it lands in the original location (and that traversal is still rejected through the symlink).

To verify against the actual live-reload flow, I ran test_control_truss_apply_patch (the Docker integration test that builds a control truss from the local templates, POSTs a real PatchRequest to /control/patch — the same server-side path truss push --watch/truss watch drives — and asserts the model's predict output changes): 1 passed in 3:46. Also green locally: the patch-applier unit tests (19), the control server endpoint tests (35), and the client-side dir-patch applier tests (68).

[cretz]

Are you a Baseten user and were you able to verify this against the Baseten server side platform by deploying/watching a model?

[sam-shridhar1950f]

Yes — I'm a Baseten user, and I just verified this against the platform. I deployed a dev model from this branch's checkout (truss push --watch) and exercised the live-reload flow end to end:

• UPDATE: edited model.py → Created patch to update model code file: model/model.py → patched successfully → next predict returned the new output (no rebuild, same deployment)
• ADD (nested new dir): created model/helpers/nested_util.py → patched successfully, and the model imported it on reload
• REMOVE (incl. empty-dir cleanup): deleted the file and dir → patched successfully → predict reflected the revert

All three round-tripped cleanly through POST /control/patch on the deployed dev container.

One caveat for full transparency: the dev image's control server is built by the Baseten backend from released truss, so the platform run pins down the exact behavior the released applier has today. As of 2db3c64 the containment check returns the identical unresolved relative_dir / patch.path join for every accepted patch — so for the entire flow verified above, the patched code is behavior-identical by construction. The only behavioral delta is rejecting paths that escape the target dir, and that path is covered by the unit tests plus the local Docker integration test (test_control_truss_apply_patch), which does bake this branch's control server into the container image.

[cretz]

Thanks for the contribution!