Skip to content

Set the embeddings server url for CDC mixer #5906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
clincoln8 wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Set the embeddings server url for CDC mixer #5906

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a26d853
Conditionally set the embeddings server url in CDC mixer when models …
clincoln8 Jan 17, 2026
dcaef24
fix
clincoln8 Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion build/cdc_services/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ fi

nginx -c /workspace/nginx.conf

MIXER_ARGS=""
if [[ $ENABLE_MODEL == "true" ]]; then
MIXER_ARGS="--embeddings_server_url=localhost:6060"
fi
Comment on lines +61 to +64
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better robustness and to avoid issues with word splitting, it's recommended to use an array for storing optional command-line arguments in bash. This is safer if arguments with spaces are ever needed in the future. This change should be made in conjunction with updating how MIXER_ARGS is used on line 76.

Suggested change
MIXER_ARGS=""
if [[ $ENABLE_MODEL == "true" ]]; then
MIXER_ARGS="--embeddings_server_url=localhost:6060"
fi
MIXER_ARGS=()
if [[ $ENABLE_MODEL == "true" ]]; then
MIXER_ARGS+=(--embeddings_server_url=localhost:6060)
fi


/workspace/bin/mixer \
--use_bigquery=false \
--use_base_bigtable=false \
Expand All @@ -67,7 +72,8 @@ nginx -c /workspace/nginx.conf
--use_sqlite=$USE_SQLITE \
--use_cloudsql=$USE_CLOUDSQL \
--cloudsql_instance=$CLOUDSQL_INSTANCE \
--remote_mixer_domain=$DC_API_ROOT &
--remote_mixer_domain=$DC_API_ROOT \
$MIXER_ARGS &
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

This line introduces a high-severity command injection vulnerability due to the unquoted shell variable $MIXER_ARGS. Unquoted variables can lead to word splitting and arbitrary command execution if an attacker controls environment variables, continuing an insecure pattern with other unquoted variables like $DC_API_ROOT and $CLOUDSQL_INSTANCE. To prevent this and ensure secure argument handling, it is critical to double-quote $MIXER_ARGS (e.g., "$MIXER_ARGS"). If $MIXER_ARGS is intended to be an array of arguments, then "${MIXER_ARGS[@]}" would be the correct and secure way to expand it, ensuring each argument is passed as a separate word. Always double-quote variables in commands as a security best practice.

Suggested change
$MIXER_ARGS &
"$MIXER_ARGS" &


envoy -l warning --config-path /workspace/esp/envoy-config.yaml &

Expand Down
Loading