Skip to content

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

Merged
lpcox merged 4 commits into
mainfrom
copilot/go-fan-go-module-review
Jun 8, 2026
Merged

security: disable $ENV in tool response filters and sync validation compile options#7208
lpcox merged 4 commits into
mainfrom
copilot/go-fan-go-module-review

Conversation

Copilot AI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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 — Add gojq.WithEnvironLoader(func() []string { return nil }) to CompileToolResponseFilter, matching the existing guard on the walk_schema path:

    code, err := gojq.Compile(query,
        gojq.WithEnvironLoader(func() []string { return nil }), // explicitly disable $ENV access (defense-in-depth)
    )
  • internal/config/validation.go — 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 — 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 — use array form ([.a, .b]) to combine outputs into a single value; first=... extra=..."
    
  • internal/middleware/jqschema_coverage_test.go — 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.

Copilot AI linked an issue Jun 8, 2026 that may be closed by this pull request
4 tasks
Copilot AI changed the title [WIP] Review gojq module for JSON processing security: disable $ENV in tool response filters and sync validation compile options Jun 8, 2026
Copilot AI requested a review from lpcox June 8, 2026 14:06
Copilot finished work on behalf of lpcox June 8, 2026 14:06
@lpcox lpcox marked this pull request as ready for review June 8, 2026 14:35
Copilot AI review requested due to automatic review settings June 8, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens jq-based tool response filtering by explicitly disabling $ENV access during filter compilation and ensuring config validation compiles filters with the same options as the runtime path, preventing accidental environment-variable exposure and eliminating validation/runtime behavior mismatches.

Changes:

  • Disable $ENV access when compiling tool response filters via gojq.WithEnvironLoader(...).
  • Align config-time jq filter validation compilation options with runtime compilation behavior.
  • Improve the “multiple results” error message and add a regression test asserting $ENV is blocked.
Show a summary per file
File Description
internal/middleware/jqschema.go Disables $ENV in tool response filter compilation and improves multiple-results guidance error text.
internal/middleware/jqschema_coverage_test.go Adds coverage test to ensure $ENV is blocked in compiled tool response filters.
internal/config/validation.go Makes validation compile jq filters with the same environment-loader restriction as runtime.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread internal/middleware/jqschema_coverage_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 49a7912 into main Jun 8, 2026
16 checks passed
@lpcox lpcox deleted the copilot/go-fan-go-module-review branch June 8, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go-fan] Go Module Review: itchyny/gojq

3 participants