[aviruthen]

Summary

When ModelBuilder.build() packages local code, repack_model copies source_dir, dependencies, and the inference script into the model archive, and tar_and_upload_dir archives them directly. Both follow nested symlinks, so a symlink inside one of these inputs that points elsewhere on the file system gets packaged as a regular file. This change checks that nested symlinks resolve inside the input they belong to before packaging. It also tightens the path checks those inputs already go through.

Changes

sagemaker-core/src/sagemaker/core/common_utils.py

• Add _validate_symlinks_within_root(), which walks a directory and raises ValueError if a nested file or directory symlink resolves outside it. Symlinks that resolve inside the directory, relative or absolute, are still supported.
• _create_or_update_code_dir runs this check on source_directory and on each dependency before copying, and now applies the existing path validation to inference_script.
• Path validation for source_directory, dependencies, the inference script, and code_dir:
  • compares against both the configured and the symlink-resolved form of each restricted path (for example, /etc is /private/etc on macOS)
  • matches whole path components with os.path.commonpath instead of a string prefix
  • adds /proc, /sys, and /dev to the restricted paths

sagemaker-core/src/sagemaker/core/fw_utils.py

• tar_and_upload_dir runs the same symlink check on directory (or script when no directory is given) and on each dependency before creating the archive.

create_tar_file is unchanged. It is shared with model artifact uploads, where symlinks outside the source root are expected (for example, Hugging Face cache snapshots that link into ../../blobs).

Behavior change

A symlink under source_dir or a dependency directory that resolves outside that directory now raises ValueError during packaging. To fix this, copy the target into the directory, or point source_dir at a parent directory that contains both. S3 source_dir inputs are not affected.

Testing

• New unit tests in sagemaker-core/tests/unit/test_common_utils.py cover:
  • nested file and directory symlinks inside and outside the root, relative symlinks, symlinked roots, and sibling directories sharing a name prefix
  • source_dir, dependency directories, and inference_script in _create_or_update_code_dir
  • end-to-end repack_model with local file:// URIs
  • restricted-path matching, including symlinked entries, /etc after resolution, /proc, /sys, /dev, and component-level matching
  • create_tar_file still following symlinks for model uploads
• New tar_and_upload_dir tests in sagemaker-core/tests/unit/test_fw_utils.py cover rejection for source_dir, dependencies, and directory entry points, and packaging of in-root symlinks.
• test_common_utils.py and test_fw_utils.py pass, as do sagemaker-serve test_model_builder_build.py and the sagemaker-mlops workflow repack/model-step tests.
• black, flake8, and pylint pass on the changed files.

[github-actions]

🤖 AI code review

This PR hardens local-code packaging in ModelBuilder.build()/repack_model/tar_and_upload_dir by rejecting nested symlinks that resolve outside the input directory, and tightens the existing sensitive-path denylist (whole-component matching via commonpath, symlink-resolved denylist forms, adding /proc, /sys, /dev). create_tar_file is intentionally left unchanged for model-artifact uploads. The change is well-aligned with prior packaging-side symlink hardening (Source: #5906) and comes with thorough unit tests covering in-root/out-of-root, relative, and symlinked-root cases. I found no correctness, security, or test-coverage issues; the implementation and tests look solid.

_{Reviewed commit 9c65e35. Automated review; verify before acting.}

[github-actions]

🤖 AI code review

This PR hardens local code packaging in the SageMaker SDK v3 by rejecting nested symlinks that resolve outside their source root in _create_or_update_code_dir (repack) and tar_and_upload_dir, and tightens the existing sensitive-path denylist (whole-component matching via commonpath, resolving both configured and symlink-resolved forms, adding /proc, /sys, /dev with a /dev/shm exception). create_tar_file is intentionally left unchanged for model-artifact uploads (HF cache snapshots). The change is well-scoped, aligns with prior precedent (PRs 5906/5418/5419), and is backed by thorough unit tests covering positive and negative cases. I found no correctness bugs; the behavior change (rejecting escaping symlinks under source_dir/dependencies) is documented and intentional.

_{Reviewed commit 6422b71. Automated review; verify before acting.}

[github-actions]

🤖 AI code review

This PR hardens model packaging by rejecting nested symlinks that escape source_dir/dependency roots in _create_or_update_code_dir and tar_and_upload_dir, consolidates sensitive-path matching into _is_sensitive_system_path (component-level commonpath matching + resolved-form checks + /proc, /sys, /dev with a /dev/shm exception), and adds inference_script validation. It follows the precedent set by PR #5906 (reject artifact symlink escapes), #5418 and #5419 (resolved-path denylist validation). The implementation is correct and the test coverage is extensive; create_tar_file is intentionally left following symlinks for model-artifact uploads. I found no blocking issues; one low-severity note below.

_{Reviewed commit 32d09cf. Automated review; verify before acting.}

[github-actions]

🟡 Low: _validate_symlinks_within_root returns early for non-directory paths, so a file dependency (or inference_script) that is itself a symlink pointing outside its root is not caught by the new escape check — only by the sensitive-path denylist. Since create_tar_file uses dereference=True (common_utils.py:507) and shutil.copy2 follows symlinks, such a file would still be dereferenced and packaged. This is arguably intended (the user explicitly named that file), and it is consistent with the PR's stated scope of nested symlinks, but it is worth confirming that a directly-passed symlinked file dependency escaping to an arbitrary location is acceptable, or documenting the limitation.