This is an automated email from the ASF dual-hosted git repository.
wenjin272 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/flink-agents.git
The following commit(s) were added to refs/heads/main by this push:
new ce89eadec [plan][java][python] Reject unsafe Bash redirects (#1079)
ce89eadec is described below
commit ce89eadecace583ce1005f96b38ed923130d4e71
Author: hylin <[email protected]>
AuthorDate: Wed Sep 2 15:39:59 2026 +0800
[plan][java][python] Reject unsafe Bash redirects (#1079)
Generated-by: TRAE (proprietary model)
Co-authored-by: linhongyu510 <[email protected]>
---
docs/content/docs/development/skills.md | 2 +
.../docs/get-started/quickstart/skills_agent.md | 1 +
.../flink/agents/plan/tools/bash/BashTool.java | 4 +-
.../agents/plan/tools/bash/BashValidator.java | 58 ++++++++++++++++++++++
.../agents/plan/tools/bash/BashValidatorTest.java | 54 ++++++++++++++++++--
python/flink_agents/plan/tools/bash/bash_tool.py | 3 ++
.../flink_agents/plan/tools/bash/bash_validator.py | 49 +++++++++++++++++-
.../plan/tools/bash/tests/test_bash_tool.py | 50 +++++++++++++++++--
8 files changed, 209 insertions(+), 12 deletions(-)
diff --git a/docs/content/docs/development/skills.md
b/docs/content/docs/development/skills.md
index d07c84ed1..22c067492 100644
--- a/docs/content/docs/development/skills.md
+++ b/docs/content/docs/development/skills.md
@@ -197,6 +197,8 @@ public static ResourceDescriptor mathModel() {
**Key points:**
- `skills` lists the skill names (matching the `name` field in each
`SKILL.md`) the agent may use.
- `allowed_commands` is a whitelist of shell command names the built-in `bash`
tool may execute. Any command not on the list is rejected, so keep it as narrow
as the skills require (for example `echo` and `bc` for arithmetic).
+- The `bash` tool rejects redirects to files by default. File-descriptor
duplication and closure, such as `2>&1` and `2>&-`, remain available.
+- Assignments to environment variables that can change command resolution or
executable loading, including `PATH`, `BASH_ENV`, `ENV`, `SHELLOPTS`, `CDPATH`,
`LD_*`, and `DYLD_*`, are rejected. The command allowlist is a validation
boundary, not a general-purpose operating-system sandbox; do not allow shells,
privilege wrappers, or other commands that can launch arbitrary executables.
- The `load_skill` and `bash` tools are added automatically — you do not
declare them in `tools`. They are added alongside any tools you do declare.
- Make sure the system prompt instructs the agent to load the relevant skill
before acting, for example: *"You must load the skill first and strictly follow
its instructions."* Without this nudge, smaller models may answer directly
instead of consulting the skill.
diff --git a/docs/content/docs/get-started/quickstart/skills_agent.md
b/docs/content/docs/get-started/quickstart/skills_agent.md
index a95a276a9..df7d63196 100644
--- a/docs/content/docs/get-started/quickstart/skills_agent.md
+++ b/docs/content/docs/get-started/quickstart/skills_agent.md
@@ -191,6 +191,7 @@ public class MathAgent extends Agent {
- `@skills`/`@Skills` declares a skill source. `Skills.from_package` (Python)
loads skills bundled inside an installed package by `(package, resource)`;
`Skills.fromClasspath` (Java) loads them from a classpath resource packaged in
the jar.
- A declared skill is exposed to a model only when the model lists it in
`skills`. The `load_skill` and `bash` tools are then added automatically.
- `allowed_commands` whitelists the shell commands the `bash` tool may run —
keep it as narrow as the skill requires.
+- The built-in `bash` tool rejects file redirects and execution-changing
environment assignments. File-descriptor duplication and closure remain
supported; see the [Skills]({{< ref "docs/development/skills" >}})
documentation for the complete security contract.
### Integrate the Agent with Flink
diff --git
a/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashTool.java
b/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashTool.java
index 57f1dc08f..38cad1cf8 100644
--- a/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashTool.java
+++ b/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashTool.java
@@ -42,7 +42,9 @@ import java.util.concurrent.TimeUnit;
*
* <p>Mirrors the Python {@code
flink_agents.plan.tools.bash.bash_tool.BashTool}. The framework
* (e.g. {@code ChatModelAction}) injects {@code allowed_commands} and {@code
allowed_script_dirs}
- * at call time; the model only sees {@code command}, {@code timeout} and
{@code cwd}.
+ * at call time; the model only sees {@code command}, {@code timeout} and
{@code cwd}. File
+ * redirects are rejected except for file-descriptor duplication and closure,
and assignments to
+ * execution-changing environment variables are rejected.
*/
public class BashTool extends Tool {
diff --git
a/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashValidator.java
b/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashValidator.java
index 4ca6401bc..0a6b4ee08 100644
---
a/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashValidator.java
+++
b/plan/src/main/java/org/apache/flink/agents/plan/tools/bash/BashValidator.java
@@ -79,6 +79,12 @@ public final class BashValidator {
"parenthesized_expression",
"array");
+ private static final Set<String> BLOCKED_ENVIRONMENT_VARIABLES =
+ Set.of("PATH", "BASH_ENV", "ENV", "SHELLOPTS", "CDPATH");
+ private static final Set<String> DYNAMIC_LOADER_VARIABLE_PREFIXES =
Set.of("LD_", "DYLD_");
+ private static final Set<String> FD_REDIRECT_OPERATORS = Set.of("<&",
">&");
+ private static final Set<String> FD_CLOSE_OPERATORS = Set.of("<&-", ">&-");
+
private static final Object PARSER_LOCK = new Object();
private static volatile TSParser parser;
@@ -146,6 +152,21 @@ public final class BashValidator {
return Optional.of(
"Standalone variable assignment without an executable is
not allowed.");
}
+ if ("file_redirect".equals(node.getType()) && !isFdOnlyRedirect(node,
command)) {
+ return Optional.of(
+ "File redirects are not allowed; only file-descriptor
duplication and closure "
+ + "are permitted.");
+ }
+ if ("variable_assignment".equals(node.getType())) {
+ TSNode nameNode = node.getChildByFieldName("name");
+ if (nameNode != null && !nameNode.isNull()) {
+ String name = nodeText(nameNode, command);
+ if (isBlockedEnvironmentVariable(name)) {
+ return Optional.of(
+ "Environment variable assignment '" + name + "' is
not allowed.");
+ }
+ }
+ }
if ("command".equals(node.getType())) {
Optional<String> err =
validateCommand(node, command, allowedCommands,
allowedScriptDirs, cwd);
@@ -163,6 +184,43 @@ public final class BashValidator {
return Optional.empty();
}
+ private static boolean isFdOnlyRedirect(TSNode node, String command) {
+ String operator = null;
+ for (int i = 0; i < node.getChildCount(); i++) {
+ TSNode child = node.getChild(i);
+ if (!child.isNamed()) {
+ operator = child.getType();
+ break;
+ }
+ }
+ TSNode destination = node.getChildByFieldName("destination");
+ if (FD_CLOSE_OPERATORS.contains(operator)) {
+ return destination == null || destination.isNull();
+ }
+ if (!FD_REDIRECT_OPERATORS.contains(operator)
+ || destination == null
+ || destination.isNull()) {
+ return false;
+ }
+ if ("number".equals(destination.getType())) {
+ return true;
+ }
+ String destinationText = nodeText(destination, command);
+ return "word".equals(destination.getType()) &&
destinationText.matches("\\d+-");
+ }
+
+ private static boolean isBlockedEnvironmentVariable(String name) {
+ if (BLOCKED_ENVIRONMENT_VARIABLES.contains(name)) {
+ return true;
+ }
+ for (String prefix : DYNAMIC_LOADER_VARIABLE_PREFIXES) {
+ if (name.startsWith(prefix)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private static Optional<String> validateCommand(
TSNode commandNode,
String command,
diff --git
a/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashValidatorTest.java
b/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashValidatorTest.java
index 6dc967df0..3946b4c8b 100644
---
a/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashValidatorTest.java
+++
b/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashValidatorTest.java
@@ -107,6 +107,25 @@ class BashValidatorTest {
BashValidator.validate("VALUE=abc echo hi", List.of("echo"),
List.of(), null));
}
+ @Test
+ void executionChangingVariableAssignmentsRejected() {
+ for (String name :
+ List.of(
+ "PATH",
+ "BASH_ENV",
+ "ENV",
+ "SHELLOPTS",
+ "CDPATH",
+ "LD_PRELOAD",
+ "LD_LIBRARY_PATH",
+ "DYLD_INSERT_LIBRARIES")) {
+ assertEquals(
+ Optional.of("Environment variable assignment '" + name +
"' is not allowed."),
+ BashValidator.validate(
+ name + "=unsafe echo hi", List.of("echo"),
List.of(), null));
+ }
+ }
+
@Test
void commandSubstitutionRejected() {
Optional<String> r =
@@ -132,10 +151,35 @@ class BashValidatorTest {
}
@Test
- void redirectAllowed() {
- // basic redirect of allowed command should pass
- Optional<String> r =
- BashValidator.validate("echo hi > /tmp/x", List.of("echo"),
List.of(), null);
- assertEquals(Optional.empty(), r);
+ void fileRedirectsRejected() {
+ for (String command :
+ List.of(
+ "echo hi > /tmp/out",
+ "echo hi >> /tmp/out",
+ "echo hi 2> /tmp/err",
+ "echo hi < /tmp/in",
+ "echo hi >2",
+ "echo hi >&/tmp/out")) {
+ assertEquals(
+ Optional.of(
+ "File redirects are not allowed; only
file-descriptor duplication and "
+ + "closure are permitted."),
+ BashValidator.validate(command, List.of("echo"),
List.of(), null));
+ }
+ }
+
+ @Test
+ void fileDescriptorOnlyRedirectsAllowed() {
+ for (String command :
+ List.of(
+ "echo hi 2>&1",
+ "echo hi >&2",
+ "echo hi <&0",
+ "echo hi 2>&-",
+ "echo hi 3>&1-")) {
+ assertEquals(
+ Optional.empty(),
+ BashValidator.validate(command, List.of("echo"),
List.of(), null));
+ }
}
}
diff --git a/python/flink_agents/plan/tools/bash/bash_tool.py
b/python/flink_agents/plan/tools/bash/bash_tool.py
index c9fd1c887..40246b62e 100644
--- a/python/flink_agents/plan/tools/bash/bash_tool.py
+++ b/python/flink_agents/plan/tools/bash/bash_tool.py
@@ -67,6 +67,9 @@ class BashTool(Tool):
Safety:
- The first token of each sub-command must be in ``allowed_commands``, or
resolve to a file under one of ``allowed_script_dirs``.
+ - File redirects are rejected; only file-descriptor duplication and closure
+ are allowed.
+ - Assignments to execution-changing environment variables are rejected.
- ``allowed_commands`` and ``allowed_script_dirs`` are injected at call
time by the framework (not visible to the LLM through ``args_schema``).
"""
diff --git a/python/flink_agents/plan/tools/bash/bash_validator.py
b/python/flink_agents/plan/tools/bash/bash_validator.py
index c87dea757..ec28935cc 100644
--- a/python/flink_agents/plan/tools/bash/bash_validator.py
+++ b/python/flink_agents/plan/tools/bash/bash_validator.py
@@ -24,8 +24,9 @@ command to be rejected. Every ``command`` node's name is
checked against the
``allowed_commands`` allowlist or resolved under ``allowed_script_dirs``.
This lets the tool accept natural shell constructs like pipes, ``&&`` / ``||``
-chains and simple redirections while blocking common injection vectors
(``$()``,
-backticks, heredoc bodies containing substitutions, control flow, etc.).
+chains and file-descriptor-only redirections while blocking common injection
+vectors (``$()``, backticks, file redirects, execution-changing environment
+assignments, control flow, etc.).
"""
from __future__ import annotations
@@ -72,6 +73,13 @@ _ALLOWED_NAMED = frozenset(
}
)
+_BLOCKED_ENVIRONMENT_VARIABLES = frozenset(
+ {"PATH", "BASH_ENV", "ENV", "SHELLOPTS", "CDPATH"}
+)
+_DYNAMIC_LOADER_VARIABLE_PREFIXES = ("LD_", "DYLD_")
+_FD_REDIRECT_OPERATORS = frozenset({"<&", ">&"})
+_FD_CLOSE_OPERATORS = frozenset({"<&-", ">&-"})
+
@lru_cache(maxsize=1)
def _get_parser() -> Parser:
@@ -122,6 +130,17 @@ def _walk(
node.parent is None or node.parent.type != "command"
):
return "Standalone variable assignment without an executable is not
allowed."
+ if node.type == "file_redirect" and not _is_fd_only_redirect(node):
+ return (
+ "File redirects are not allowed; only file-descriptor duplication "
+ "and closure are permitted."
+ )
+ if node.type == "variable_assignment":
+ name_node = node.child_by_field_name("name")
+ if name_node is not None:
+ name = name_node.text.decode("utf-8", errors="replace")
+ if _is_blocked_environment_variable(name):
+ return f"Environment variable assignment '{name}' is not
allowed."
if node.type == "command":
err = _validate_command_node(node, allowed_commands,
allowed_script_dirs, cwd)
if err is not None:
@@ -133,6 +152,32 @@ def _walk(
return None
+def _is_fd_only_redirect(node: Node) -> bool:
+ """Return whether a redirect only duplicates or closes a file
descriptor."""
+ operator = next((child.type for child in node.children if not
child.is_named), None)
+ destination = node.child_by_field_name("destination")
+ if operator in _FD_CLOSE_OPERATORS:
+ return destination is None
+ return (
+ operator in _FD_REDIRECT_OPERATORS
+ and destination is not None
+ and (
+ destination.type == "number"
+ or (
+ destination.type == "word"
+ and destination.text.endswith(b"-")
+ and destination.text[:-1].isdigit()
+ )
+ )
+ )
+
+
+def _is_blocked_environment_variable(name: str) -> bool:
+ return name in _BLOCKED_ENVIRONMENT_VARIABLES or name.startswith(
+ _DYNAMIC_LOADER_VARIABLE_PREFIXES
+ )
+
+
def _validate_command_node(
node: Node,
allowed_commands: List[str],
diff --git a/python/flink_agents/plan/tools/bash/tests/test_bash_tool.py
b/python/flink_agents/plan/tools/bash/tests/test_bash_tool.py
index 3f6048fec..58eb8022e 100644
--- a/python/flink_agents/plan/tools/bash/tests/test_bash_tool.py
+++ b/python/flink_agents/plan/tools/bash/tests/test_bash_tool.py
@@ -113,6 +113,24 @@ class TestValidateCommand:
def test_allow_env_prefix(self) -> None:
assert validate_command("FOO=bar echo hi", ["echo"], []) is None
+ @pytest.mark.parametrize(
+ "name",
+ [
+ "PATH",
+ "BASH_ENV",
+ "ENV",
+ "SHELLOPTS",
+ "CDPATH",
+ "LD_PRELOAD",
+ "LD_LIBRARY_PATH",
+ "DYLD_INSERT_LIBRARIES",
+ ],
+ )
+ def test_reject_execution_changing_env_prefix(self, name: str) -> None:
+ assert validate_command(f"{name}=unsafe echo hi", ["echo"], []) == (
+ f"Environment variable assignment '{name}' is not allowed."
+ )
+
def test_reject_env_prefix_command_not_whitelisted(self) -> None:
assert validate_command("FOO=bar rm -rf /", ["echo"], []) is not None
@@ -192,11 +210,35 @@ class TestValidateCommand:
def test_allow_brace_expansion(self) -> None:
assert validate_command("echo {a,b,c}", ["echo"], []) is None
- def test_allow_redirect_to_file(self) -> None:
- assert validate_command("echo hi > /tmp/out", ["echo"], []) is None
+ @pytest.mark.parametrize(
+ "command",
+ [
+ "echo hi > /tmp/out",
+ "echo hi >> /tmp/out",
+ "echo hi 2> /tmp/err",
+ "echo hi < /tmp/in",
+ "echo hi >2",
+ "echo hi >&/tmp/out",
+ ],
+ )
+ def test_reject_file_redirect(self, command: str) -> None:
+ assert validate_command(command, ["echo"], []) == (
+ "File redirects are not allowed; only file-descriptor duplication "
+ "and closure are permitted."
+ )
- def test_allow_stderr_redirect(self) -> None:
- assert validate_command("echo hi 2>&1", ["echo"], []) is None
+ @pytest.mark.parametrize(
+ "command",
+ [
+ "echo hi 2>&1",
+ "echo hi >&2",
+ "echo hi <&0",
+ "echo hi 2>&-",
+ "echo hi 3>&1-",
+ ],
+ )
+ def test_allow_fd_only_redirect(self, command: str) -> None:
+ assert validate_command(command, ["echo"], []) is None
class TestBashTool:
