fix(executor): block proxy env var unset commands

[krrish-berri-2]

Summary

• Agent-supplied commands to the executor sandbox could run unset HTTPS_PROXY (or HTTP_PROXY) in the child shell before making outbound requests, silently bypassing the vault MITM proxy
• Added a regex guard in POST /execute that rejects any command matching unset (HTTPS|HTTP)_PROXY with a 403 before execAsync is called

Test plan

• Send POST /execute with cmd: "unset HTTPS_PROXY && curl https://example.com" → expect 403 command blocked: proxy env vars cannot be unset
• Send POST /execute with cmd: "unset https_proxy" → expect 403
• Send normal command (e.g. cmd: "echo hello") → expect 200 with output
• Confirm vault proxy still enforced for normal outbound commands

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR adds a regex guard to POST /execute in the executor harness that blocks commands containing unset HTTPS_PROXY (or HTTP_PROXY) before they reach execAsync, returning a 403. The intent is to prevent agent-supplied commands from silently disabling the vault MITM proxy.

• The guard catches the literal unset HTTPS_PROXY/HTTP_PROXY keyword form but misses at least three other shell idioms that achieve the same effect: env -u HTTPS_PROXY, an inline override (HTTPS_PROXY= curl …), and the bash-flag form unset -v HTTPS_PROXY.
• The regex also carries redundant case alternatives that are fully collapsed by the /i flag, and the predicate function is named like an action (rejectsProxyTamper) rather than a test.

Confidence Score: 2/5

The security guard introduced by this PR can be bypassed via common shell idioms, leaving the vault MITM proxy unenforced for commands that don't use the exact unset VAR keyword form.

The change is a targeted security hardening measure, but the regex it relies on is incomplete — at least three bypass paths (env -u, inline variable assignment, and unset -v) are not blocked. Any of these would let an agent-controlled command route traffic outside the vault proxy, which is exactly the threat this PR is meant to close.

harnesses/executor/src/server.ts — the new PROXY_TAMPER_RE regex and its call-site need to cover all shell proxy-clearing idioms, not just the unset VAR form.

Security Review

• Incomplete proxy-bypass guard in harnesses/executor/src/server.ts: the new regex blocks unset HTTPS_PROXY but does not cover env -u HTTPS_PROXY, HTTPS_PROXY= cmd, or unset -v HTTPS_PROXY, all of which silence the vault MITM proxy without triggering the 403.

Important Files Changed

Filename	Overview
harnesses/executor/src/server.ts	Adds a regex guard to block unset HTTPS_PROXY-style commands, but the pattern misses several common shell bypass paths (env -u, inline HTTPS_PROXY=, and unset -v), leaving the proxy enforcement incomplete.

_{Reviews (1): Last reviewed commit: "fix(executor): block proxy env var unset..." | Re-trigger Greptile}

[greptile-apps]

Multiple shell bypass paths miss the unset keyword entirely

The regex only intercepts the literal unset HTTPS_PROXY form, but a shell command passed to execAsync can clear or override the proxy variable in several other ways that the guard never sees:

• env -u HTTPS_PROXY curl https://example.com — POSIX-standard per-command env removal, no unset keyword
• HTTPS_PROXY= curl https://example.com — inline empty-string override, proxy effectively disabled for that process
• unset -v HTTPS_PROXY — bash flag between the keyword and the variable name breaks the \s+(HTTPS?_PROXY) sequence in the regex

All three forms would pass the check and reach execAsync, silently bypassing the vault MITM proxy.

[greptile-apps]

Redundant regex alternatives

With the /i (case-insensitive) flag already applied, the two alternatives https?_proxy and HTTPS?_PROXY match exactly the same set of strings. The second alternative adds no additional coverage and makes the pattern harder to read.

[greptile-apps]

Misleading predicate name — reads as an action, not a test

A boolean predicate that returns true when a command contains a proxy-tampering pattern should read as a question (isProxyTamper, detectsProxyTamper). The current name rejectsProxyTamper implies the function performs the rejection itself, which is confusing when reading the call-site if (rejectsProxyTamper(cmd)).

Suggested change

	function rejectsProxyTamper(cmd: string): boolean {
	return PROXY_TAMPER_RE.test(cmd);
	}
	function isProxyTamper(cmd: string): boolean {
	return PROXY_TAMPER_RE.test(cmd);
	}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!