Repository navigation
fix(executor): keep only allowlisted keys from the X-LLM-Env header - #220
Merged
Merged
Conversation
ParseLLMEnvHeader now keeps only the keys named in LLMEnvAllowlist, the list the runner already uses to build the header, and drops the rest. The new V(2) line logs the number of dropped keys, never a key name or value. Runners that send keys outside the list keep working, because no receive-side code reads a key outside the list. Signed-off-by: Shreyas Mocherla <shreyas@shreyasm.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approved
No active review issues were identified for the environment-header allowlist change, although tests were not run during this review.
No open issues 🎉
What changed in this PR
Applies the runner’s existing LLMEnvAllowlist when the agent executor reads X-LLM-Env, keeping both sides consistent.
Changes:
- Drops unlisted header keys and logs only the number dropped, not names or values.
- Adds parser and handler coverage to verify that listed keys reach the agent while unlisted keys are excluded.
Tests were not run during this review.
| File | Description |
|---|---|
internal/workflow/executor/exec_handler.go |
Filters decoded header keys against the shared allowlist and logs the dropped count. |
internal/workflow/executor/exec_handler_test.go |
Adds an agent-context recorder and tests allowlist filtering in the parser and handler. |
Review effort: Balanced
patelrit
approved these changes
Nov 3, 2026
patelrit
left a comment
patelrit
left a comment
There was a problem hiding this comment.
Approved: Copilot 🟢 on head, no open threads, CI green, unit tests, and E2E passed on the head commit (run 41691312797).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ParseLLMEnvHeader(internal/workflow/executor/exec_handler.go) now keeps only the keys listed inLLMEnvAllowlistand drops the rest. A V(2) log line reports how many keys were dropped, never their names or values.Why
LLMEnvAllowlist(internal/workflow/executor/exec_client.go) is the list the runner already uses to build theX-LLM-Envheader. The agent executor now applies the same list when it reads the header, so both sides use one list.Compatibility
A runner that sends keys outside the list keeps working, because no receive-side code reads a key outside the list.
Testing
go test ./internal/workflow/executor/passes, andgo veton the package is clean.ParseLLMEnvHeaderspecdecodes a valid headeris nowdecodes a valid header, dropping keys not in LLMEnvAllowlist. It sends a listed and an unlisted key and expects only the listed one, because an unlisted key such as its oldKEYis now dropped.ExecHandlerspec sends a request carrying a listed and an unlisted key and checks that only the listed one reachesagent.GetLLMEnv.exec_handler.go(run withgo test -overlay) and pass with this change.🤖 Generated with Claude Code