tools: tighten exec script preflight
This commit is contained in:
parent
ec98564ce6
commit
b18095a8c9
2 changed files with 48 additions and 9 deletions
|
|
@ -113,6 +113,7 @@ var (
|
||||||
}
|
}
|
||||||
|
|
||||||
scriptPreflightEnvVarPattern = regexp.MustCompile(`\$[A-Z_][A-Z0-9_]{1,}`)
|
scriptPreflightEnvVarPattern = regexp.MustCompile(`\$[A-Z_][A-Z0-9_]{1,}`)
|
||||||
|
envAssignmentPattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*=`)
|
||||||
interpreterPipePattern = regexp.MustCompile(
|
interpreterPipePattern = regexp.MustCompile(
|
||||||
`(?i)(?:^|[|;&]\s*)(?:env\s+)?(?:python(?:\d+(?:\.\d+)?)?|node(?:js)?)\b`,
|
`(?i)(?:^|[|;&]\s*)(?:env\s+)?(?:python(?:\d+(?:\.\d+)?)?|node(?:js)?)\b`,
|
||||||
)
|
)
|
||||||
|
|
@ -1155,6 +1156,10 @@ func (t *ExecTool) validateScriptFileForShellBleed(command, cwd string) string {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !shouldScanScriptForShellBleed(absPath) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
if first := scriptPreflightEnvVarPattern.Find(content); len(first) > 0 {
|
if first := scriptPreflightEnvVarPattern.Find(content); len(first) > 0 {
|
||||||
return fmt.Sprintf(
|
return fmt.Sprintf(
|
||||||
"Command blocked by safety guard (exec preflight: detected likely shell variable injection (%s))",
|
"Command blocked by safety guard (exec preflight: detected likely shell variable injection (%s))",
|
||||||
|
|
@ -1180,13 +1185,13 @@ func extractScriptTargetFromCommand(command string) []string {
|
||||||
interpreter := strings.ToLower(filepath.Base(argv[0]))
|
interpreter := strings.ToLower(filepath.Base(argv[0]))
|
||||||
switch {
|
switch {
|
||||||
case isPythonInterpreter(interpreter):
|
case isPythonInterpreter(interpreter):
|
||||||
target := findLastPositionalScriptArg(argv[1:], []string{".py"})
|
target := findFirstPositionalScriptArg(argv[1:], []string{".py"})
|
||||||
if target == "" {
|
if target == "" {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
return []string{target}
|
return []string{target}
|
||||||
case isNodeInterpreter(interpreter):
|
case isNodeInterpreter(interpreter):
|
||||||
target := findLastPositionalScriptArg(argv[1:], []string{".js"})
|
target := findFirstPositionalScriptArg(argv[1:], []string{".js"})
|
||||||
if target == "" {
|
if target == "" {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
@ -1282,7 +1287,7 @@ func stripEnvPrefix(argv []string) []string {
|
||||||
idx++
|
idx++
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if strings.Contains(token, "=") && !strings.HasPrefix(token, "-") && !strings.ContainsAny(token, "/\\") {
|
if envAssignmentPattern.MatchString(token) && !strings.HasPrefix(token, "-") {
|
||||||
idx++
|
idx++
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
@ -1302,7 +1307,7 @@ func isNodeInterpreter(token string) bool {
|
||||||
return token == "node" || token == "nodejs"
|
return token == "node" || token == "nodejs"
|
||||||
}
|
}
|
||||||
|
|
||||||
func findLastPositionalScriptArg(tokens []string, suffixes []string) string {
|
func findFirstPositionalScriptArg(tokens []string, suffixes []string) string {
|
||||||
if len(tokens) == 0 {
|
if len(tokens) == 0 {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
@ -1355,6 +1360,11 @@ func hasScriptSuffix(token string, suffixes []string) bool {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func shouldScanScriptForShellBleed(path string) bool {
|
||||||
|
ext := strings.ToLower(filepath.Ext(path))
|
||||||
|
return ext == ".py" || ext == ".pyw"
|
||||||
|
}
|
||||||
|
|
||||||
func shouldFailClosedInterpreterPreflight(command string) bool {
|
func shouldFailClosedInterpreterPreflight(command string) bool {
|
||||||
trimmed := strings.TrimSpace(command)
|
trimmed := strings.TrimSpace(command)
|
||||||
if trimmed == "" {
|
if trimmed == "" {
|
||||||
|
|
@ -1367,8 +1377,12 @@ func shouldFailClosedInterpreterPreflight(command string) bool {
|
||||||
if interpreterShellWrapperPattern.MatchString(trimmed) {
|
if interpreterShellWrapperPattern.MatchString(trimmed) {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
if interpreterPipePattern.MatchString(trimmed) && strings.ContainsAny(trimmed, "|;&") {
|
if matches := interpreterPipePattern.FindAllStringIndex(trimmed, -1); len(matches) > 0 {
|
||||||
return true
|
for _, match := range matches {
|
||||||
|
if match[0] > 0 {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return false
|
return false
|
||||||
|
|
|
||||||
|
|
@ -686,12 +686,19 @@ func TestShellTool_ScriptPreflight(t *testing.T) {
|
||||||
want string
|
want string
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "quoted script path validates content",
|
name: "quoted python script path validates content",
|
||||||
command: `node "bad.js"`,
|
command: `python "bad.py"`,
|
||||||
fileName: "bad.js",
|
fileName: "bad.py",
|
||||||
content: "const value = $DM_JSON;",
|
content: "const value = $DM_JSON;",
|
||||||
want: "exec preflight: detected likely shell variable injection ($DM_JSON)",
|
want: "exec preflight: detected likely shell variable injection ($DM_JSON)",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "env-prefixed interpreter validates content",
|
||||||
|
command: `env PYTHONPATH=/tmp python bad.py`,
|
||||||
|
fileName: "bad.py",
|
||||||
|
content: "payload = $DM_JSON",
|
||||||
|
want: "exec preflight: detected likely shell variable injection ($DM_JSON)",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "piped interpreter fails closed",
|
name: "piped interpreter fails closed",
|
||||||
command: "cat bad.py | python",
|
command: "cat bad.py | python",
|
||||||
|
|
@ -737,6 +744,24 @@ func TestShellTool_ScriptPreflight(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestShellTool_ScriptPreflight_AllowsDirectChainedInterpreter(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
if err := os.WriteFile(filepath.Join(tmpDir, "good.py"), []byte("print('ok')"), 0o644); err != nil {
|
||||||
|
t.Fatalf("failed to write test script: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tool, err := NewExecTool(tmpDir, false)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unable to configure exec tool: %s", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"action": "run",
|
||||||
|
"command": "python good.py && echo ok",
|
||||||
|
})
|
||||||
|
require.NotContains(t, result.ForLLM, "exec preflight:")
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_URLBypassPrevented verifies that a command cannot bypass the workspace
|
// TestShellTool_URLBypassPrevented verifies that a command cannot bypass the workspace
|
||||||
// sandbox by smuggling a real path after a URL that contains the same //path substring.
|
// sandbox by smuggling a real path after a URL that contains the same //path substring.
|
||||||
// e.g. "echo https://etc/passwd && cat //etc/passwd" must still be blocked.
|
// e.g. "echo https://etc/passwd && cat //etc/passwd" must still be blocked.
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue