Skip to content

Commit 2b88415

Browse files
Kastier1claude
andcommitted
fix(hosting-cli): restrict gcp deploy env, add --no-interactive, extract constants
Address PR feedback: - Restrict the deploy script's environment to an allowlist of host vars (PATH, HOME, gcloud/docker config, proxy/TLS) plus the explicit deploy overrides. Prevents a tampered or compromised flexgen manifest from exfiltrating unrelated host secrets like AWS_*/GITHUB_TOKEN. - Add --interactive/--no-interactive (default true) so the command works in CI. In non-interactive mode the run prompt is skipped, and an existing Dockerfile errors out unless --overwrite-dockerfile is set. - Extract env-var keys and manifest field names into module-level constants per project convention. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a100bf8 commit 2b88415

2 files changed

Lines changed: 224 additions & 19 deletions

File tree

  • packages/reflex-hosting-cli/src/reflex_cli/v2
  • tests/units/reflex_cli/v2

packages/reflex-hosting-cli/src/reflex_cli/v2/gcp.py

Lines changed: 112 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,60 @@
2626

2727
DOCKERFILE_NAME = "Dockerfile"
2828

29+
# Environment variables passed to the deploy script.
30+
ENV_GCP_PROJECT = "GCP_PROJECT"
31+
ENV_GCP_REGION = "GCP_REGION"
32+
ENV_SERVICE_NAME = "SERVICE_NAME"
33+
ENV_AR_REPO = "AR_REPO"
34+
ENV_VERSION = "VERSION"
35+
36+
# Manifest response field names from flexgen.
37+
FIELD_DOCKERFILE = "dockerfile"
38+
FIELD_DEPLOY_COMMAND = "deploy_command"
39+
40+
# Allowlist of host environment variables forwarded to the deploy script.
41+
# We deliberately exclude things like AWS_*/GITHUB_TOKEN/SSH agent sockets so a
42+
# compromised or tampered manifest cannot exfiltrate unrelated credentials.
43+
DEPLOY_ENV_ALLOWLIST = frozenset({
44+
"PATH",
45+
"HOME",
46+
"USER",
47+
"LOGNAME",
48+
"SHELL",
49+
"TERM",
50+
"LANG",
51+
"LC_ALL",
52+
"LC_CTYPE",
53+
"TMPDIR",
54+
"TEMP",
55+
"TMP",
56+
"XDG_CONFIG_HOME",
57+
# gcloud configuration
58+
"CLOUDSDK_CONFIG",
59+
"CLOUDSDK_ACTIVE_CONFIG_NAME",
60+
"CLOUDSDK_CORE_PROJECT",
61+
"CLOUDSDK_CORE_ACCOUNT",
62+
"CLOUDSDK_AUTH_ACCESS_TOKEN_FILE",
63+
"GOOGLE_APPLICATION_CREDENTIALS",
64+
# docker configuration
65+
"DOCKER_HOST",
66+
"DOCKER_TLS_VERIFY",
67+
"DOCKER_CERT_PATH",
68+
"DOCKER_CONFIG",
69+
"DOCKER_BUILDKIT",
70+
# corporate proxy / TLS trust
71+
"HTTP_PROXY",
72+
"HTTPS_PROXY",
73+
"NO_PROXY",
74+
"http_proxy",
75+
"https_proxy",
76+
"no_proxy",
77+
"SSL_CERT_FILE",
78+
"SSL_CERT_DIR",
79+
"REQUESTS_CA_BUNDLE",
80+
"CURL_CA_BUNDLE",
81+
})
82+
2983

3084
@click.group()
3185
def gcp_cli():
@@ -78,6 +132,12 @@ def gcp_cli():
78132
help="Overwrite an existing Dockerfile without prompting.",
79133
)
80134
@click.option("--token", help="The Reflex authentication token.")
135+
@click.option(
136+
"--interactive/--no-interactive",
137+
is_flag=True,
138+
default=True,
139+
help="Whether to prompt before overwriting the Dockerfile and running the script.",
140+
)
81141
@click.option(
82142
"--dry-run",
83143
is_flag=True,
@@ -99,6 +159,7 @@ def gcp_deploy(
99159
source_dir: str,
100160
overwrite_dockerfile: bool,
101161
token: str | None,
162+
interactive: bool,
102163
dry_run: bool,
103164
loglevel: str,
104165
):
@@ -112,7 +173,7 @@ def gcp_deploy(
112173
console.set_log_level(loglevel)
113174

114175
authenticated_client = hosting.get_authenticated_client(
115-
token=token, interactive=True
176+
token=token, interactive=interactive
116177
)
117178

118179
bash_path = shutil.which("bash")
@@ -154,11 +215,11 @@ def gcp_deploy(
154215

155216
version_value = version_tag or datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S")
156217
deploy_env = {
157-
"GCP_PROJECT": gcp_project,
158-
"GCP_REGION": region,
159-
"SERVICE_NAME": service_name,
160-
"AR_REPO": ar_repo,
161-
"VERSION": version_value,
218+
ENV_GCP_PROJECT: gcp_project,
219+
ENV_GCP_REGION: region,
220+
ENV_SERVICE_NAME: service_name,
221+
ENV_AR_REPO: ar_repo,
222+
ENV_VERSION: version_value,
162223
}
163224

164225
console.info("Received deploy manifest from flexgen.")
@@ -172,6 +233,10 @@ def gcp_deploy(
172233
console.print("─" * 60)
173234
console.print(deploy_script)
174235
console.print("─" * 60)
236+
console.info(
237+
f"The script runs with a restricted env (only {len(DEPLOY_ENV_ALLOWLIST)} "
238+
"allowlisted host variables forwarded plus the deploy variables above)."
239+
)
175240

176241
if dry_run:
177242
console.print("")
@@ -182,13 +247,20 @@ def gcp_deploy(
182247
console.info("Dry run — nothing written or executed.")
183248
return
184249

185-
if not _write_dockerfile(dockerfile_path, dockerfile, overwrite_dockerfile):
250+
if not _write_dockerfile(
251+
dockerfile_path, dockerfile, overwrite_dockerfile, interactive
252+
):
186253
raise click.exceptions.Exit(1)
187254

188-
answer = console.ask("Run the deploy script now?", choices=["y", "n"], default="y")
189-
if answer != "y":
190-
console.warn("Aborted by user. The Dockerfile has been written for later use.")
191-
raise click.exceptions.Exit(1)
255+
if interactive:
256+
answer = console.ask(
257+
"Run the deploy script now?", choices=["y", "n"], default="y"
258+
)
259+
if answer != "y":
260+
console.warn(
261+
"Aborted by user. The Dockerfile has been written for later use."
262+
)
263+
raise click.exceptions.Exit(1)
192264

193265
exit_code = _run_deploy_script(
194266
bash_path=bash_path,
@@ -284,31 +356,44 @@ def _request_manifest(token: str) -> tuple[str, str]:
284356
console.error("Flexgen returned an unexpected response shape.")
285357
raise click.exceptions.Exit(1)
286358

287-
dockerfile = body.get("dockerfile")
288-
deploy_command = body.get("deploy_command")
359+
dockerfile = body.get(FIELD_DOCKERFILE)
360+
deploy_command = body.get(FIELD_DEPLOY_COMMAND)
289361
if not isinstance(dockerfile, str) or not dockerfile.strip():
290-
console.error("Flexgen response is missing a non-empty 'dockerfile' field.")
362+
console.error(
363+
f"Flexgen response is missing a non-empty {FIELD_DOCKERFILE!r} field."
364+
)
291365
raise click.exceptions.Exit(1)
292366
if not isinstance(deploy_command, str) or not deploy_command.strip():
293-
console.error("Flexgen response is missing a non-empty 'deploy_command' field.")
367+
console.error(
368+
f"Flexgen response is missing a non-empty {FIELD_DEPLOY_COMMAND!r} field."
369+
)
294370
raise click.exceptions.Exit(1)
295371

296372
return dockerfile, deploy_command
297373

298374

299-
def _write_dockerfile(path: Path, contents: str, overwrite: bool) -> bool:
300-
"""Write the Dockerfile to disk, prompting before overwriting.
375+
def _write_dockerfile(
376+
path: Path, contents: str, overwrite: bool, interactive: bool
377+
) -> bool:
378+
"""Write the Dockerfile to disk, prompting before overwriting in interactive mode.
301379
302380
Args:
303381
path: Where to write the Dockerfile.
304382
contents: The Dockerfile body.
305383
overwrite: If True, overwrite without prompting.
384+
interactive: If False, never prompt; require `overwrite` when the file exists.
306385
307386
Returns:
308387
True on success, False if the user declined to overwrite or write failed.
309388
310389
"""
311390
if path.exists() and not overwrite:
391+
if not interactive:
392+
console.error(
393+
f"{path} already exists. Pass --overwrite-dockerfile to replace it "
394+
"in non-interactive mode."
395+
)
396+
return False
312397
answer = console.ask(
313398
f"{path} already exists. Overwrite?", choices=["y", "n"], default="n"
314399
)
@@ -335,17 +420,25 @@ def _run_deploy_script(
335420
) -> int:
336421
"""Run the bash deploy script, streaming output to the user's terminal.
337422
423+
The script's environment is restricted to ``DEPLOY_ENV_ALLOWLIST`` (plus the
424+
explicit ``env_overrides``) so unrelated host secrets like ``AWS_*`` or
425+
``GITHUB_TOKEN`` cannot be exfiltrated by a tampered or compromised manifest.
426+
338427
Args:
339428
bash_path: Resolved path to the bash executable.
340429
script: The bash script body received from flexgen.
341430
cwd: Working directory to run the script in.
342-
env_overrides: Environment variables to layer on top of the parent env.
431+
env_overrides: Environment variables required by the deploy script.
343432
344433
Returns:
345434
The exit code of the bash process.
346435
347436
"""
348-
env = os.environ.copy()
437+
env = {
438+
name: value
439+
for name, value in os.environ.items()
440+
if name in DEPLOY_ENV_ALLOWLIST
441+
}
349442
env.update(env_overrides)
350443
try:
351444
result = subprocess.run(

tests/units/reflex_cli/v2/test_gcp.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import os
34
from pathlib import Path
45
from unittest import mock
56

@@ -294,6 +295,117 @@ def test_gcp_deploy_default_version_is_timestamp(mocker: MockFixture, tmp_path:
294295
assert version.replace("-", "").isdigit()
295296

296297

298+
def test_gcp_deploy_no_interactive_skips_run_prompt(
299+
mocker: MockFixture, tmp_path: Path
300+
):
301+
run_mock = _patch_environment(mocker)
302+
_mock_manifest_response(mocker)
303+
304+
result = runner.invoke(
305+
hosting_cli,
306+
[
307+
"gcp",
308+
"deploy",
309+
"--gcp-project",
310+
"p",
311+
"--source",
312+
str(tmp_path),
313+
"--no-interactive",
314+
"--token",
315+
"fake-token",
316+
],
317+
)
318+
319+
assert result.exit_code == 0, result.output
320+
assert (tmp_path / "Dockerfile").read_text() == DOCKERFILE
321+
assert run_mock.call_count == 1
322+
323+
324+
def test_gcp_deploy_no_interactive_refuses_to_overwrite_without_flag(
325+
mocker: MockFixture, tmp_path: Path
326+
):
327+
run_mock = _patch_environment(mocker)
328+
_mock_manifest_response(mocker)
329+
existing = tmp_path / "Dockerfile"
330+
existing.write_text("FROM existing\n")
331+
332+
result = runner.invoke(
333+
hosting_cli,
334+
[
335+
"gcp",
336+
"deploy",
337+
"--gcp-project",
338+
"p",
339+
"--source",
340+
str(tmp_path),
341+
"--no-interactive",
342+
"--token",
343+
"fake-token",
344+
],
345+
)
346+
347+
assert result.exit_code == 1
348+
assert "--overwrite-dockerfile" in result.output
349+
assert existing.read_text() == "FROM existing\n"
350+
assert run_mock.call_count == 0
351+
352+
353+
def test_gcp_deploy_env_is_restricted_to_allowlist(mocker: MockFixture, tmp_path: Path):
354+
"""Verify the script env excludes host secrets and only includes allowlisted vars."""
355+
from reflex_cli.v2 import gcp as gcp_module
356+
357+
mocker.patch(
358+
"reflex_cli.utils.hosting.get_authenticated_client",
359+
return_value=hosting.AuthenticatedClient(token="fake-token", validated_data={}),
360+
)
361+
mocker.patch(
362+
"reflex_cli.v2.gcp.shutil.which", side_effect=lambda n: f"/usr/bin/{n}"
363+
)
364+
mocker.patch(
365+
"reflex_cli.v2.gcp._get_active_gcp_account", return_value="u@example.com"
366+
)
367+
_mock_manifest_response(mocker)
368+
369+
captured: dict[str, dict[str, str]] = {}
370+
371+
def fake_run(*args, **kwargs):
372+
captured["env"] = kwargs["env"]
373+
return mock.MagicMock(returncode=0)
374+
375+
mocker.patch("reflex_cli.v2.gcp.subprocess.run", side_effect=fake_run)
376+
mocker.patch.dict(
377+
os.environ,
378+
{
379+
"PATH": "/usr/bin",
380+
"HOME": "/home/test",
381+
"AWS_SECRET_ACCESS_KEY": "should-not-leak",
382+
"GITHUB_TOKEN": "also-secret",
383+
"DOCKER_HOST": "unix:///var/run/docker.sock",
384+
"MY_RANDOM_VAR": "should-not-leak",
385+
},
386+
clear=True,
387+
)
388+
389+
result = runner.invoke(
390+
hosting_cli,
391+
["gcp", "deploy", "--gcp-project", "p", "--source", str(tmp_path)],
392+
input="y\n",
393+
)
394+
395+
assert result.exit_code == 0, result.output
396+
env = captured["env"]
397+
# Allowlisted host vars are forwarded.
398+
assert env["PATH"] == "/usr/bin"
399+
assert env["HOME"] == "/home/test"
400+
assert env["DOCKER_HOST"] == "unix:///var/run/docker.sock"
401+
# Deploy overrides are present.
402+
assert env[gcp_module.ENV_GCP_PROJECT] == "p"
403+
# Host secrets are NOT forwarded.
404+
assert "AWS_SECRET_ACCESS_KEY" not in env
405+
assert "GITHUB_TOKEN" not in env
406+
assert "MY_RANDOM_VAR" not in env
407+
408+
297409
@pytest.fixture(autouse=True)
298410
def _no_log_level_side_effects(mocker: MockFixture):
299411
mocker.patch("reflex_cli.utils.console.set_log_level")

0 commit comments

Comments
 (0)