Unify path validation in EPs, address security gaps

[yuslepukhin]

This pull request strengthens security around external file paths used by various execution providers (TensorRT, NV TensorRT RTX, QNN, and OpenVINO) by replacing custom, ad-hoc path validation logic with a unified utility function, utils::ValidateExternalDataPath. This function ensures that external data paths do not escape the model directory, effectively preventing directory traversal, absolute path usage, and symlink-based escapes. The update also exposes this validation utility through the provider API for consistent use across providers.

Security improvements for external data path validation:

• Replaced manual checks for absolute paths and .. parent directory traversal in TensorRT, NV TensorRT RTX, QNN, and OpenVINO providers with calls to utils::ValidateExternalDataPath, ensuring paths do not escape the model directory and improving protection against directory traversal and symlink attacks. [1] [2] [3] [4] [5] [6] [7] [8]

Provider API enhancements:

• Added the ValidateExternalDataPath function to the provider API (provider_api.h, provider_interfaces.h) and implemented it in the provider bridge, making the unified validation utility accessible to all providers. [1] [2] [3]

[Copilot]

Pull request overview

This PR centralizes external file path validation for execution providers by routing EP context/cache path checks through utils::ValidateExternalDataPath, and exposes that utility through the provider bridge.

Changes:

• Adds ValidateExternalDataPath to the shared provider API/host bridge.
• Replaces custom absolute/parent traversal checks in TensorRT, NV TensorRT RTX, QNN, and OpenVINO EP paths.
• Updates QNN/OpenVINO/TensorRT cache path handling to validate before constructing filesystem paths.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
onnxruntime/core/session/provider_bridge_ort.cc	Implements provider-host forwarding for the validation utility.
onnxruntime/core/providers/shared_library/provider_interfaces.h	Adds the virtual provider-host API entry.
onnxruntime/core/providers/shared_library/provider_api.h	Adds the inline provider-facing wrapper.
onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc	Uses unified validation for refit ONNX model paths.
onnxruntime/core/providers/tensorrt/onnx_ctx_model_helper.cc	Uses unified validation for TensorRT EP cache paths.
onnxruntime/core/providers/nv_tensorrt_rtx/nv_execution_provider.cc	Uses unified validation for NV TensorRT RTX refit ONNX model paths.
onnxruntime/core/providers/nv_tensorrt_rtx/onnx_ctx_model_helper.cc	Uses unified validation for NV TensorRT RTX EP cache paths.
onnxruntime/core/providers/qnn/qnn_execution_provider.cc	Validates QNN EP cache context values before mapping shared buffers.
onnxruntime/core/providers/qnn/builder/onnx_ctx_model_helper.cc	Replaces QNN-specific path checks with unified validation.
onnxruntime/core/providers/openvino/onnx_ctx_model_helper.cc	Validates OpenVINO EP cache context paths before loading blobs/contexts.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[tianleiwu]

Review Summary

This revision addresses all concerns from prior review rounds by introducing ValidateExternalDataPathFromDir — a directory-accepting overload that eliminates the parent_path() semantic mismatch that allowed callers to accidentally widen the validation root. The refactoring is clean, test coverage is comprehensive (including symlink escapes), and the provider bridge correctly exposes both overloads.

Verdict: Looks good. Two optional suggestions below.

Highlights

• Correct factoring: ValidateExternalDataPathFromDir takes a directory directly; ValidateExternalDataPath remains a thin wrapper for model-file callers.
• QNN properly re-wraps the validator error into INVALID_GRAPH status, preserving downstream expectations.
• OpenVINO uses the model-file overload correctly (passes actual file paths, not directories).
• Provider bridge exposes both functions symmetrically.
• Test coverage for the new FromDir variant covers all key scenarios.

Suggestion: Remaining ad-hoc checks

Lines ~1689–1693 in tensorrt_execution_provider.cc (and ~1140 in nv_execution_provider.cc) still use IsAbsolutePath/IsRelativePathToParentPath with log-only enforcement in the dump_ep_context_model_ path. Consider migrating these to the unified validator in a follow-up for consistency.

[tianleiwu]

Overall this looks good and is safe to merge — the refactor consolidates the per-EP ad-hoc absolute/.. string checks into utils::ValidateExternalDataPath / the new ValidateExternalDataPathFromDir, and actually strengthens the guarantees by resolving symlinks via weak canonicalization instead of substring-matching "..". The reworked double-symlink branch (recomputing the canonical external-data path from the symlink model dir and comparing against the canonical real model dir) is a genuine correctness improvement for the HuggingFace-cache layout. Test coverage for the new overload is solid.

A few non-blocking suggestions (none are correctness/security blockers):

1. Partial unification (out of diff). The engine-cache write paths still use the legacy IsAbsolutePath / IsRelativePathToParentPath helpers (tensorrt_execution_provider.cc ~L1689, nv_execution_provider.cc ~L1141). This is defensible since those targets may not exist yet and the new validator enforces existence, but the two validation styles can drift. Consider a short comment at those sites explaining why they intentionally keep the old check, or a follow-up issue.

2. Wording. ValidateExternalDataPathFromDir hard-codes "External data path escapes model directory.", but it is now also used for EP cache-context directories that aren't model directories. The appended allowed directory: keeps it clear enough; just flagging for a future wording pass.

See inline comments for two small items on the wrapper error message and the QNN empty-attr handling.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

[tianleiwu]

Re-reviewed after the latest commit ("Address test failures"). Both prior inline comments are resolved:

1. Wrapper diagnostic — ValidateExternalDataPath now short-circuits empty/absolute external_data_path (if (external_data_path.empty() || !external_data_path.root_path().empty()) return status;) before composing the model-relative context, so input errors no longer get the misleading "escapes working directory" wording. Covered by the new *_EmptyPathRejected / *_AbsolutePathRejected tests.
2. QNN path validation — the hard ORT_THROW_IF_ERROR in GetCapability is replaced with a logged error + early return result, which is the safer behavior during capability querying. Defense-in-depth is preserved since GetEpContextFromMainNode still validates at load time, so an escaping path is never read.

The added step-by-step doc comments accurately describe the two-pass flow and the HuggingFace double-symlink rationale (no comment drift), and the five new tests close the previously noted coverage gaps (HF symlink layout success, empty/absolute rejection on the symlink path, data-symlink-escapes-real-dir rejection, and non-existent-file rejection). No remaining correctness or security concerns. LGTM.