security: disable $ENV in tool response filters and sync validation compile options (#7208)

lpcox · web-flow · commit 49a79127cc22 · 2026-06-08T08:53:35.000-07:00
Tool response filters compiled without `WithEnvironLoader` could expose
environment variables (including `GITHUB_TOKEN`) if a filter referenced
`$ENV`. Validation also compiled with different options than the runtime
path, meaning `$ENV`-referencing filters would pass validation but
behave differently at runtime.

## Changes

- **`internal/middleware/jqschema.go`** &mdash; Add
`gojq.WithEnvironLoader(func() []string { return nil })` to
`CompileToolResponseFilter`, matching the existing guard on the
`walk_schema` path:
  ```go
  code, err := gojq.Compile(query,
gojq.WithEnvironLoader(func() []string { return nil }), // explicitly
disable $ENV access (defense-in-depth)
  )
  ```

- **`internal/config/validation.go`** &mdash; Apply the same
`WithEnvironLoader` option in `validateToolResponseFilters` so
validation accurately predicts runtime behavior. Previously a filter
using `$ENV` would pass validation but silently return `null`/`{}` for
`$ENV` references at runtime.

- **`internal/middleware/jqschema.go`** &mdash; Improve the multiple-results
error message to guide admins toward the correct fix:
  ```
// Before: "tool response filter returned multiple results, first=...
extra=..."
// After: "tool response filter returned multiple results &mdash; use array
form ([.a, .b]) to combine outputs into a single value; first=...
extra=..."
  ```

- **`internal/middleware/jqschema_coverage_test.go`** &mdash; Add
`TestCompileToolResponseFilter_EnvDisabled` which runs a
`$ENV`-referencing filter through the compiled code object directly and
asserts `$ENV` resolves to `{}`, not the real process environment.
diff --git a/internal/config/validation.go b/internal/config/validation.go
@@ -136,7 +136,9 @@ func validateToolResponseFilters(filters map[string]string, jsonPath string) err
 		if err != nil {
 			return fmt.Errorf("%s.%s contains an invalid jq expression: %w", jsonPath, toolName, err)
 		}
-		if _, err := gojq.Compile(query); err != nil {
+		if _, err := gojq.Compile(query,
+			gojq.WithEnvironLoader(func() []string { return nil }), // match runtime compile options (defense-in-depth)
+		); err != nil {
 			return fmt.Errorf("%s.%s contains an invalid jq expression: %w", jsonPath, toolName, err)
 		}
 	}
diff --git a/internal/middleware/jqschema.go b/internal/middleware/jqschema.go
@@ -253,7 +253,7 @@ func runJqCode(
 
 	if opts.CheckMultipleResults {
 		if extra, ok := iter.Next(); ok {
-			return nil, fmt.Errorf("%s returned multiple results, first=%T extra=%T", errPrefix, v, extra)
+			return nil, fmt.Errorf("%s returned multiple results &mdash; use array form ([.a, .b]) to combine outputs into a single value; first=%T extra=%T", errPrefix, v, extra)
 		}
 	}
 
@@ -279,7 +279,9 @@ func CompileToolResponseFilter(filter string) (*gojq.Code, error) {
 		return nil, fmt.Errorf("failed to parse tool response filter: %w", err)
 	}
 
-	code, err := gojq.Compile(query)
+	code, err := gojq.Compile(query,
+		gojq.WithEnvironLoader(func() []string { return nil }), // explicitly disable $ENV access (defense-in-depth)
+	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compile tool response filter: %w", err)
 	}
diff --git a/internal/middleware/jqschema_coverage_test.go b/internal/middleware/jqschema_coverage_test.go
@@ -246,6 +246,31 @@ func TestCompileToolResponseFilter_ParseError(t *testing.T) {
 	}
 }
 
+// TestCompileToolResponseFilter_EnvDisabled verifies that $ENV access is
+// blocked in compiled tool response filters (defense-in-depth). The compiled
+// code object itself is used directly (via applyToolResponseFilter) to
+// confirm that the WithEnvironLoader option was applied at compile time.
+func TestCompileToolResponseFilter_EnvDisabled(t *testing.T) {
+	// $ENV should resolve to an empty object (not real env vars) when
+	// WithEnvironLoader returns nil.
+	code, err := CompileToolResponseFilter(". + {env: $ENV}")
+	require.NoError(t, err)
+	require.NotNil(t, code)
+
+	// Use the compiled code object directly so we verify the options
+	// applied during CompileToolResponseFilter, not a re-compile.
+	result, runErr := applyToolResponseFilter(context.Background(), code, map[string]interface{}{"a": 1})
+	require.NoError(t, runErr)
+	m, ok := result.(map[string]interface{})
+	require.True(t, ok)
+	// Original input field must be preserved by the merge.
+	assert.Equal(t, 1, m["a"])
+	// $ENV must be an empty object, not the real process environment.
+	env, ok := m["env"].(map[string]interface{})
+	require.True(t, ok, "env should be an object, got: %T", m["env"])
+	assert.Empty(t, env)
+}
+
 // TestApplyToolResponseFilter_ParseError verifies that ApplyToolResponseFilter
 // propagates compile errors from CompileToolResponseFilter.
 func TestApplyToolResponseFilter_ParseError(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,9 @@ func validateToolResponseFilters(filters map[string]string, jsonPath string) err`
`136`	`136`	`if err != nil {`
`137`	`137`	`return fmt.Errorf("%s.%s contains an invalid jq expression: %w", jsonPath, toolName, err)`
`138`	`138`	`}`
`139`		`- if _, err := gojq.Compile(query); err != nil {`
	`139`	`+ if _, err := gojq.Compile(query,`
	`140`	`+ gojq.WithEnvironLoader(func() []string { return nil }), // match runtime compile options (defense-in-depth)`
	`141`	`+ ); err != nil {`
`140`	`142`	`return fmt.Errorf("%s.%s contains an invalid jq expression: %w", jsonPath, toolName, err)`
`141`	`143`	`}`
`142`	`144`	`}`
Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ func runJqCode(`
`253`	`253`
`254`	`254`	`if opts.CheckMultipleResults {`
`255`	`255`	`if extra, ok := iter.Next(); ok {`
`256`		`- return nil, fmt.Errorf("%s returned multiple results, first=%T extra=%T", errPrefix, v, extra)`
	`256`	`+ return nil, fmt.Errorf("%s returned multiple results — use array form ([.a, .b]) to combine outputs into a single value; first=%T extra=%T", errPrefix, v, extra)`
`257`	`257`	`}`
`258`	`258`	`}`
`259`	`259`
`@@ -279,7 +279,9 @@ func CompileToolResponseFilter(filter string) (*gojq.Code, error) {`
`279`	`279`	`return nil, fmt.Errorf("failed to parse tool response filter: %w", err)`
`280`	`280`	`}`
`281`	`281`
`282`		`- code, err := gojq.Compile(query)`
	`282`	`+ code, err := gojq.Compile(query,`
	`283`	`+ gojq.WithEnvironLoader(func() []string { return nil }), // explicitly disable $ENV access (defense-in-depth)`
	`284`	`+ )`
`283`	`285`	`if err != nil {`
`284`	`286`	`return nil, fmt.Errorf("failed to compile tool response filter: %w", err)`
`285`	`287`	`}`