zhfeng commented on PR #25551:
URL: https://github.com/apache/camel/pull/25551#issuecomment-5365155822
## Security Review Comment
Thank you for this well-implemented addition. The design is solid and the
security model is thoughtfully documented. I have a few suggestions to
strengthen the security posture before merge:
---
### 1. Trusted mode must not be the default — confirmed ✅
The PR correctly makes default mode restricted and requires explicit
`createWithHostAccess()` opt-in. No action needed here.
---
### 2. Documentation: warn about `context.stop()` blast radius in trusted
mode
The current docs state that trusted mode "lets Python call public methods
and fields on bound Java objects" and that `context`, `exchange`, and `message`
are available. However, it does not explicitly warn that **`context.stop()` is
callable** — a Python script could shut down the entire Camel application.
**Suggested addition to the `== Trusted host access` section:**
```adoc
[WARNING]
Using trusted mode gives Python scripts the ability to call *any* public
method on the
Camel context, exchange, and message objects. This includes destructive
operations such
as `context.stop()`, `context.getRegistry().bind(...)`, or
`exchange.getContext().getExecutorService(...)`.
Only enable trusted mode when the Python scripts are fully trusted (e.g.
developed by your own team,
not sourced from external input).
```
---
### 3. Documentation: resource file loading attack surface
The implementation supports loading scripts from files (`resource:file:...`,
`resource:classpath:...`). If a route dynamically selects a script path from an
untrusted source (e.g. a message header), an attacker could cause loading of an
arbitrary file.
**Suggested addition near the `== Usage` section or `== Security` section:**
```adoc
[WARNING]
When using `resource:` URIs to load Python scripts from files or classpath,
ensure the script
path is never derived from untrusted input (such as message headers or query
parameters).
A maliciously crafted path could cause the application to load an unexpected
Python script.
```
---
### 4. Documentation: `python.PosixModuleBackend = "java"` is not a security
boundary
The implementation uses `.option("python.PosixModuleBackend", "java")`. This
is documented in the source but not in the docs. It would be helpful to state
explicitly:
```adoc
The GraalPy POSIX module is backed by Java's file/system APIs. File I/O,
process creation,
and socket operations are blocked by `allowIO(false)`. However, this is not
a hard security
sandbox — rely on GraalVM's `HostAccess` restrictions for the actual
security boundary.
```
---
These are all documentation-only suggestions. The implementation is
well-done. The existing security documentation is already above average for a
language component.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
