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 388d4b88e [hotfix][java][python] Harden Bash arithmetic evaluation
(#1083)
388d4b88e is described below
commit 388d4b88e3dbd6d7a28d5a16ed386897731c2521
Author: Wenjin Xie <[email protected]>
AuthorDate: Wed Sep 2 09:41:13 2026 +0800
[hotfix][java][python] Harden Bash arithmetic evaluation (#1083)
Generated-by: Codex (GPT-5)
Co-authored-by: Codex <[email protected]>
---
.../agents/plan/tools/bash/BashValidator.java | 14 +++++---
.../flink/agents/plan/tools/bash/BashToolTest.java | 31 ++++++++++++++++
.../agents/plan/tools/bash/BashValidatorTest.java | 29 +++++++++++++--
.../flink_agents/plan/tools/bash/bash_validator.py | 13 +++----
.../plan/tools/bash/tests/test_bash_tool.py | 41 ++++++++++++++++++++++
5 files changed, 115 insertions(+), 13 deletions(-)
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 2d938ecfb..4ca6401bc 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
@@ -56,8 +56,6 @@ public final class BashValidator {
"program",
"command",
"command_name",
- // `export VAR=...`, `readonly`, `declare`, `local`,
`typeset`
- "declaration_command",
"pipeline",
"list",
"redirected_statement",
@@ -76,7 +74,6 @@ public final class BashValidator {
"number",
"simple_expansion", // $VAR
"expansion", // ${VAR}
- "arithmetic_expansion", // $((...))
"binary_expression",
"unary_expression",
"parenthesized_expression",
@@ -143,6 +140,12 @@ public final class BashValidator {
return Optional.of(
"Disallowed shell construct '" + node.getType() + "' in:
'" + snippet + "'");
}
+ TSNode parent = node.getParent();
+ if ("variable_assignment".equals(node.getType())
+ && (parent == null || parent.isNull() ||
!"command".equals(parent.getType()))) {
+ return Optional.of(
+ "Standalone variable assignment without an executable is
not allowed.");
+ }
if ("command".equals(node.getType())) {
Optional<String> err =
validateCommand(node, command, allowedCommands,
allowedScriptDirs, cwd);
@@ -168,8 +171,9 @@ public final class BashValidator {
@Nullable String cwd) {
TSNode nameNode = commandNode.getChildByFieldName("name");
if (nameNode == null || nameNode.isNull()) {
- // Bare variable-assignment parsed as command — nothing to
validate.
- return Optional.empty();
+ // Fail closed for constructs such as bare variable assignments.
Bash can later
+ // reinterpret their values in arithmetic contexts.
+ return Optional.of("Command without an executable is not
allowed.");
}
String executable = nodeText(nameNode, command);
if (allowedCommands.contains(executable)) {
diff --git
a/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashToolTest.java
b/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashToolTest.java
index 3a134923e..76698e6ec 100644
---
a/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashToolTest.java
+++
b/plan/src/test/java/org/apache/flink/agents/plan/tools/bash/BashToolTest.java
@@ -23,12 +23,15 @@ import
org.apache.flink.agents.api.resource.ResourceDescriptor;
import org.apache.flink.agents.api.tools.ToolParameters;
import org.apache.flink.agents.api.tools.ToolResponse;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.nio.file.Path;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
class BashToolTest {
@@ -70,6 +73,34 @@ class BashToolTest {
assertTrue(out.startsWith("Command rejected:"));
}
+ @Test
+ void integerDeclarationCannotReevaluateCommandSubstitution(@TempDir Path
tempDir) {
+ Path marker = tempDir.resolve("integer-declaration-marker");
+ ToolResponse r =
+ tool().call(
+ args(
+ "declare -i VALUE='$(touch " + marker
+ ")'",
+ List.of(),
+ List.of()));
+ String out = (String) r.getResult();
+ assertTrue(out.startsWith("Command rejected:"));
+ assertFalse(marker.toFile().exists());
+ }
+
+ @Test
+ void arithmeticExpansionCannotReevaluateAssignedValue(@TempDir Path
tempDir) {
+ Path marker = tempDir.resolve("arithmetic-expansion-marker");
+ ToolResponse r =
+ tool().call(
+ args(
+ "VALUE='$(touch " + marker + ")'; echo
$((VALUE))",
+ List.of("echo"),
+ List.of()));
+ String out = (String) r.getResult();
+ assertTrue(out.startsWith("Command rejected:"));
+ assertFalse(marker.toFile().exists());
+ }
+
@Test
void successfulCommandWithEmptyOutput() {
ToolResponse r = tool().call(args("true", List.of("true"), List.of()));
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 7ff28f071..6dc967df0 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
@@ -76,10 +76,35 @@ class BashValidatorTest {
}
@Test
- void arithmeticExpansionAllowed() {
+ void arithmeticExpansionRejected() {
+ Optional<String> r =
+ BashValidator.validate("echo $((1+2))", List.of("echo"),
List.of(), null);
+ assertTrue(r.isPresent());
+ assertTrue(r.get().contains("arithmetic_expansion"));
+ }
+
+ @Test
+ void declarationCommandRejected() {
+ Optional<String> r =
+ BashValidator.validate(
+ "declare -i VALUE='1 + 2'", List.of("echo"),
List.of(), null);
+ assertTrue(r.isPresent());
+ assertTrue(r.get().contains("declaration_command"));
+ }
+
+ @Test
+ void standaloneVariableAssignmentRejected() {
+ Optional<String> r =
+ BashValidator.validate("VALUE='1 + 2'", List.of("echo"),
List.of(), null);
+ assertTrue(r.isPresent());
+ assertTrue(r.get().contains("executable"));
+ }
+
+ @Test
+ void variableAssignmentPrefixWithAllowedCommandPasses() {
assertEquals(
Optional.empty(),
- BashValidator.validate("echo $((1+2))", List.of("echo"),
List.of(), null));
+ BashValidator.validate("VALUE=abc echo hi", List.of("echo"),
List.of(), null));
}
@Test
diff --git a/python/flink_agents/plan/tools/bash/bash_validator.py
b/python/flink_agents/plan/tools/bash/bash_validator.py
index d5b9c20f8..c87dea757 100644
--- a/python/flink_agents/plan/tools/bash/bash_validator.py
+++ b/python/flink_agents/plan/tools/bash/bash_validator.py
@@ -47,8 +47,6 @@ _ALLOWED_NAMED = frozenset(
"program",
"command",
"command_name",
- # `export VAR=...`, `readonly`, `declare`, `local`, `typeset`
- "declaration_command",
"pipeline",
"list",
"redirected_statement",
@@ -67,7 +65,6 @@ _ALLOWED_NAMED = frozenset(
"number",
"simple_expansion", # $VAR
"expansion", # ${VAR}
- "arithmetic_expansion", # $((...))
"binary_expression",
"unary_expression",
"parenthesized_expression",
@@ -121,6 +118,10 @@ def _walk(
if node.is_named and node.type not in _ALLOWED_NAMED:
snippet = node.text.decode("utf-8", errors="replace")[:80]
return f"Disallowed shell construct '{node.type}' in: {snippet!r}"
+ if node.type == "variable_assignment" and (
+ node.parent is None or node.parent.type != "command"
+ ):
+ return "Standalone variable assignment without an executable is not
allowed."
if node.type == "command":
err = _validate_command_node(node, allowed_commands,
allowed_script_dirs, cwd)
if err is not None:
@@ -140,9 +141,9 @@ def _validate_command_node(
) -> str | None:
name_node = node.child_by_field_name("name")
if name_node is None:
- # Commands without a resolvable name (edge case, e.g. bare
- # variable-assignment parsed as `command`) — nothing to validate.
- return None
+ # Fail closed for constructs such as bare variable assignments. Bash
+ # can later reinterpret their values in arithmetic contexts.
+ return "Command without an executable is not allowed."
executable = name_node.text.decode("utf-8", errors="replace")
if executable in allowed_commands:
return None
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 e05936a0d..3f6048fec 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
@@ -116,6 +116,11 @@ class TestValidateCommand:
def test_reject_env_prefix_command_not_whitelisted(self) -> None:
assert validate_command("FOO=bar rm -rf /", ["echo"], []) is not None
+ def test_reject_standalone_variable_assignment(self) -> None:
+ error = validate_command("VALUE='1 + 2'", ["echo"], [])
+ assert error is not None
+ assert "executable" in error
+
# -- injection vectors: MUST be rejected ------------------------------
def test_reject_dollar_paren_substitution(self) -> None:
@@ -165,6 +170,16 @@ class TestValidateCommand:
error = validate_command("f() { echo hi; }", ["echo"], [])
assert error is not None
+ def test_reject_declaration_command(self) -> None:
+ error = validate_command("declare -i VALUE='1 + 2'", ["echo"], [])
+ assert error is not None
+ assert "declaration_command" in error
+
+ def test_reject_arithmetic_expansion(self) -> None:
+ error = validate_command("echo $((1 + 2))", ["echo"], [])
+ assert error is not None
+ assert "arithmetic_expansion" in error
+
def test_reject_heredoc(self) -> None:
error = validate_command("cat <<EOF\n$(rm /)\nEOF", ["cat"], [])
assert error is not None
@@ -237,6 +252,32 @@ class TestBashTool:
)
assert "Command rejected" in result
+ def test_reject_substitution_re_evaluated_by_integer_declaration(
+ self, tool: BashTool, tmp_path: Path
+ ) -> None:
+ marker = tmp_path / "integer-declaration-marker"
+ result = tool.call(
+ command=f"declare -i VALUE='$(touch {marker})'",
+ timeout=10,
+ allowed_commands=[],
+ allowed_script_dirs=[],
+ )
+ assert "Command rejected" in result
+ assert not marker.exists()
+
+ def test_reject_substitution_re_evaluated_by_arithmetic_expansion(
+ self, tool: BashTool, tmp_path: Path
+ ) -> None:
+ marker = tmp_path / "arithmetic-expansion-marker"
+ result = tool.call(
+ command=f"VALUE='$(touch {marker})'; echo $((VALUE))",
+ timeout=10,
+ allowed_commands=["echo"],
+ allowed_script_dirs=[],
+ )
+ assert "Command rejected" in result
+ assert not marker.exists()
+
def test_no_allowed_commands_rejects_everything(self, tool: BashTool) ->
None:
result = tool.call(command="echo hello", timeout=10)
assert "Command rejected" in result
