fix: check bucket ownership

[shruti-mantri]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces project ownership verification for Google Cloud Storage buckets to prevent unauthorized deployments, implementing a fallback mechanism with randomized bucket names when a mismatch is detected. It also adds tracking and propagation of the GCS object generation ID throughout the build and deployment processes to ensure source code consistency. The review feedback highlights a potential security bypass in the ownership check when both project numbers are undefined, and suggests optimizing the file upload process by removing redundant file reference calls and updating the corresponding unit tests.

[husainhirani]

Split this cl in two parts.

1. bucket ownership check
2. version generation check

[husainhirani]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces bucket ownership verification and fallback mechanisms to randomized bucket names in ensureStorageBucketExists to prevent conflicts and enhance security. It also updates file upload logic to fetch metadata and adds corresponding unit tests. The review feedback highlights a potential Denial of Service (DoS) vulnerability if an attacker pre-creates a bucket (causing bucket.exists() to throw a 403 error), and identifies bugs where the labels parameter is lost during recursive fallback calls. Additionally, it suggests explicitly passing false instead of undefined for the isFallback parameter in build.js for consistency.

[gemini-code-assist]

Security Vulnerability: Denial of Service (DoS) via Bucket Squatting

Issue:
If a bucket with the name bucketName (e.g., [project-id]-source-bucket or [project-id]_cloudbuild) is pre-created by an attacker in a different GCP project, any call to bucket.exists() will throw a 403 Forbidden error because the current service account does not have permission to access it.

Since bucket.exists() is called outside of a specific try-catch block, this 403 error will propagate to the outer catch block of ensureStorageBucketExists, causing the entire deployment to fail. This allows any malicious actor to permanently block deployments for a project by simply squatting on the expected bucket names.

Remedy:
Wrap the bucket.exists() call in a try-catch block. If a 403 error is caught, treat it as if the bucket exists (exists = true). The subsequent ownership check will then fail (since we cannot read the metadata, or the project number won't match), safely triggering the fallback to a randomized bucket name.

Example implementation:

  let exists = false;
  try {
    const [bucketExists] = await callWithRetry(
      () => bucket.exists(),
      `storage.bucket.exists ${bucketName}`
    );
    exists = bucketExists;
  } catch (err) {
    if (err.code === 403 || err.status === 403) {
      exists = true;
    } else {
      throw err;
    }
  }

[gemini-code-assist]

Bug: labels parameter is lost during fallback recursive call

Issue:
When falling back to a randomized bucket name due to an ownership mismatch, the labels parameter is passed as undefined to the recursive ensureStorageBucketExists call. This means any custom labels intended for the bucket will not be applied to the fallback bucket.

Remedy:
Pass the labels parameter instead of undefined in the recursive call.

Suggested change

	return await ensureStorageBucketExists(
	projectId,
	fallbackBucketName,
	location,
	accessToken,
	undefined,
	true,
	progressCallback
	);
	return await ensureStorageBucketExists(
	projectId,
	fallbackBucketName,
	location,
	accessToken,
	labels,
	true,
	progressCallback
	);

[gemini-code-assist]

Bug: labels parameter is lost during fallback recursive call on creation failure

Issue:
Similar to the ownership mismatch fallback, when createBucket fails because the bucket already exists and we fall back to a randomized bucket name, the labels parameter is passed as undefined. This prevents the custom labels from being applied to the fallback bucket.

Remedy:
Pass the labels parameter instead of undefined in the recursive call.

Suggested change

	return await ensureStorageBucketExists(
	projectId,
	fallbackBucketName,
	location,
	accessToken,
	undefined,
	true,
	progressCallback
	);
	return await ensureStorageBucketExists(
	projectId,
	fallbackBucketName,
	location,
	accessToken,
	labels,
	true,
	progressCallback
	);

[gemini-code-assist]

Improvement: Use explicit false for isFallback parameter

For consistency with other calls to ensureStorageBucketExists across the codebase (e.g., in deployer.js and compose.js), pass false explicitly instead of undefined for the isFallback parameter.

Suggested change

	undefined,
	undefined,
	undefined,
	false,

[husainhirani]

Passing 7 positional arguments is not ideal. Can you raise a followup PR to convert this to optional arguments format?
e.g.

ensureStorageBucketExists({projectId, bucketName, region, accessToken, labels, progressCallback})