security: replace unrestricted setattr with allowlist in Python backend

[titaiwangms]

Summary

Fixes a critical security vulnerability in the ONNX Runtime Python backend where user-controlled kwargs were applied to SessionOptions and RunOptions via unrestricted setattr(), allowing arbitrary file overwrites.

Vulnerability

The prepare() method in onnxruntime/python/backend/backend.py iterated over user-controlled kwargs and used setattr() to apply them directly to a SessionOptions instance. The hasattr() check was not a security guard — it returned True for all exposed properties including dangerous ones like optimized_model_filepath.

Attack vector:

onnxruntime.backend.prepare(
    model_path,
    optimized_model_filepath="/etc/passwd",  # overwrites any file with protobuf binary
    graph_optimization_level=onnxruntime.GraphOptimizationLevel.ORT_ENABLE_ALL
)

The same pattern existed in backend_rep.py for RunOptions.

Fix

Replaced the unrestricted hasattr/setattr loop in both files with strict allowlists:

• _ALLOWED_SESSION_OPTIONS (13 safe attrs) in backend.py
• _ALLOWED_RUN_OPTIONS (4 safe attrs) in backend_rep.py

Both SessionOptions and RunOptions use identical validation logic with three outcomes for each kwarg key:

• Allowlisted — Applied via setattr() (e.g. graph_optimization_level, log_severity_level)
• Known-but-blocked (real attribute on the object, but not on allowlist) — Raises RuntimeError (e.g. optimized_model_filepath, terminate)
• Completely unknown (not a property on the object at all) — Silently ignored for forward compatibility (e.g. nonexistent_option_xyz)

Blocked dangerous attributes:

• optimized_model_filepath — triggers Model::Save(), overwrites arbitrary files with protobuf binary
• profile_file_prefix — writes profiling JSON to arbitrary path
• enable_profiling — causes uncontrolled file writes to cwd
• terminate (RunOptions) — denies the current inference call
• training_mode (RunOptions) — silently switches inference behavior in training builds

Tests

Added TestBackendKwargsAllowlist with 13 new test methods covering all exploit vectors (blocked attrs raise RuntimeError), safe allowlisted attrs (accepted), unknown attrs (silently ignored), and end-to-end run_model() paths for both session and run options. All 15 tests pass (13 new + 2 pre-existing in TestBackend), no regressions.

Files Changed

• onnxruntime/python/backend/backend.py
• onnxruntime/python/backend/backend_rep.py
• onnxruntime/test/python/onnxruntime_test_python_backend.py
• .agents/skills/python-kwargs-setattr-security/SKILL.md

[github-actions]

You can commit the suggested changes from lintrunner.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Hardens the ONNX Runtime Python backend by preventing user-controlled kwargs from setting unsafe SessionOptions / RunOptions attributes via unrestricted setattr(), mitigating arbitrary file write and inference disruption vectors.

Changes:

• Added strict allowlists for SessionOptions (raise on known-but-blocked attributes) and RunOptions (silently ignore non-allowlisted keys).
• Updated backend API docstrings to document allowlisted behavior.
• Added regression tests covering blocked attributes, allowed attributes, and ignore behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
onnxruntime/python/backend/backend.py	Introduces _ALLOWED_SESSION_OPTIONS and raises on blocked known SessionOptions attributes.
onnxruntime/python/backend/backend_rep.py	Introduces _ALLOWED_RUN_OPTIONS and ignores all non-allowlisted RunOptions keys.
onnxruntime/test/python/onnxruntime_test_python_backend.py	Adds tests validating allowlist, block/raise semantics, and ignore behavior.
.github/skills/python-kwargs-setattr-security/SKILL.md	Documents the insecure pattern and the repo’s allowlist-based remediation approach.

---

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

[vraspar]

Review: PR #28083 — security: replace unrestricted setattr with allowlist in Python backend

Thanks for the security fix — the core approach (frozenset allowlists replacing unrestricted hasattr/setattr) is correct and I don't see a bypass within the backend kwargs path. A few items to address before merge:

---

1. RunOptions.terminate becomes a silent no-op (behavioral regression)

Before this PR, rep.run(..., terminate=True) and backend.run_model(..., terminate=True) actually worked — terminate is a real, exposed RunOptions attribute. After this PR it's silently swallowed.

The silent-ignore design is driven by run_model() forwarding the same kwargs to both prepare() and rep.run(), but that's an implementation detail that shouldn't dictate the API contract. Consider splitting kwargs in run_model() into session-options vs run-options buckets so each path only receives its own keys. That way RunOptions can raise on blocked attrs (like terminate) instead of silently dropping them.

2. No run_model() end-to-end test

The PR justifies the asymmetric error handling (raise vs. silent ignore) based on run_model() forwarding kwargs to both paths, but no test actually exercises backend.run_model(...). Suggest adding tests for:

• run_model(..., graph_optimization_level=...) — safe SessionOptions kwarg
• run_model(..., only_execute_path_to_fetches=...) — safe RunOptions kwarg
• run_model(..., terminate=True) — whatever the intended behavior ends up being

3. SKILL.md is in the wrong directory

ORT's existing skills live under .agents/skills/ (ort-build, ort-lint, ort-test). This PR adds .github/skills/ which doesn't exist on main. I'd suggest either moving it to .agents/skills/ or dropping it from this PR and adding it separately after maintainer buy-in — it widens a focused security fix with repo-tooling content.

4. Minor: error message content not asserted in tests

The tests check that RuntimeError is raised for blocked attrs, but don't assert on the message content. A quick assertIn("not permitted", str(ctx.exception)) would guard against accidental regressions in the user-facing error message.

5. Consider an allowlist-drift regression test

A test that enumerates the current writable properties on SessionOptions / RunOptions and asserts they equal allowed ∪ blocked would catch future pybind additions that silently fall through without explicit review.

---

Overall this is a solid, well-scoped security fix. Items 1-3 are the actionable ones; 4-5 are nice-to-haves.

[titaiwangms]

Thanks @vraspar — all items addressed in the latest commits:

Item 1 (kwargs split + RunOptions raise): run_model() now splits kwargs into session_kwargs and run_kwargs before passing to prepare() and rep.run() respectively. With kwargs properly separated, backend_rep.py's run() now raises RuntimeError on blocked-but-known RunOptions attrs (same pattern as prepare()) — so terminate=True raises rather than silently drops.

Item 2 (run_model tests): Added 3 new tests: test_run_model_with_safe_session_option, test_run_model_with_safe_run_option, and test_run_model_with_blocked_run_option_raises. Also renamed test_blocked_run_option_terminate_is_silently_ignored → test_blocked_run_option_terminate_raises to match the new behavior.

Item 3 (SKILL.md location): Moved from .github/skills/ to .agents/skills/ to match ORT conventions.

Item 4 (error message assertions): Added assertIn('not permitted', str(ctx.exception)) to the 3 existing blocked-attr tests.

[github-actions]

You can commit the suggested changes from lintrunner.

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[titaiwangms]

The CI failure is irrelevant.

[tianleiwu]

One follow-up design suggestion: the new allowlists appear complete for the direct Python properties on SessionOptions and RunOptions, but they leave no safe path for advanced runtime configuration channels that are not regular properties. CUDA graph is the clearest example: enabling it is an EP provider option (enable_cuda_graph for CUDA EP or trt_cuda_graph_enable for TensorRT EP), and per-run control uses the run config entry gpu_graph_id. If the backend API is expected to remain useful for advanced performance scenarios, consider a second explicit allowlist for safe advanced configuration channels such as curated session_config_entries, run_config_entries, and provider-specific options, instead of limiting support to plain attributes only. That would preserve the security fix without permanently excluding safe features like CUDA graph.

[titaiwangms]

@tianleiwu Thank you for the forward-looking suggestion — this is exactly the kind of design thinking that helps the backend API stay useful beyond the immediate security fix.

I investigated this during the PR development and found that CUDA graph configuration (and advanced EP options in general) uses completely different APIs than the setattr() pattern this PR secures:

• Provider options flow through SessionOptions.provider_options (a dict passed to add_provider_options()), not Python properties
• Session config entries use add_session_config_entry(key, value) — a method call, not a settable attribute
• Run config entries use add_run_config_entry(key, value) — same pattern

Since these are dict/method-based APIs rather than Python properties, the current frozenset + setattr() allowlist pattern cannot cover them. A proper implementation would need:

1. Per-EP allowlists for provider options (each EP has different valid keys with different security implications)
2. Structured dict parameters (not flat kwargs) to distinguish provider_options, session_config_entries, and run_config_entries
3. Careful security auditing of each EP's option set — some EP options can trigger file I/O or network access

This is estimated at 200–400 lines of new code with its own test surface. I'd like to keep this PR focused on closing the immediate setattr() vulnerability and tackle the advanced config channels as a dedicated follow-up PR where each EP's options can be properly audited.

Would that approach work for you?

[Copilot]

Pull request overview

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

---

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

[tianleiwu]

Reviewed the latest head. The allowlist implementation and tests look good to me: the known file-write/inference-control attributes are blocked, unknown kwargs are still ignored where needed for the dual prepare/run forwarding path, and the earlier doc/PR-body mismatches have been addressed.