Skip to content

fix: read Bazel RBE auth tag from NGAT env instead of hardcoding#3590

Open
anandppatil wants to merge 1 commit intoangular:mainfrom
anandppatil:fix/remove-hardcoded-gcm-auth-tag
Open

fix: read Bazel RBE auth tag from NGAT env instead of hardcoding#3590
anandppatil wants to merge 1 commit intoangular:mainfrom
anandppatil:fix/remove-hardcoded-gcm-auth-tag

Conversation

@anandppatil
Copy link
Copy Markdown

fix: read Bazel RBE auth tag from NGAT env instead of hardcoding

Problem

The github-actions/bazel/configure-remote/constants.ts file hardcodes the AES-256-GCM authentication tag used to decrypt gcp_token.data:

// Current (vulnerable)
export const at = 'QwbjZ/z+yDtD+XZjKj9Ynw==';

This means all four parameters needed to decrypt the embedded GCP service account credential are publicly available:

Parameter Source Public?
Algorithm constants.tsaes-256-gcm
Key Derived from GITHUB_REPOSITORY_OWNER ("angular")
IV Hardcoded in constants.ts
Auth Tag Hardcoded in constants.ts
Ciphertext gcp_token.data in repo

The credential can be decrypted entirely offline — no secrets, authentication, or internal access required.

Existing Pattern

The SauceLabs and BrowserStack actions in this same repository already handle this correctly by reading the auth tag from a secret environment variable:

// github-actions/saucelabs/constants.ts — correct
export const at = process.env.NGAT!;

// github-actions/browserstack/constants.ts — correct
export const at = process.env.NGAT!;

Fix

This PR applies the same pattern to the Bazel configure-remote action:

- export const at = 'QwbjZ/z+yDtD+XZjKj9Ynw==';
+ export const at = process.env.NGAT!;

Additional Steps Required by Maintainers

  1. Rebuild the checked-in bundle: configure-remote.js needs to be regenerated via the Bazel build after this TypeScript change.
  2. Rotate the exposed credential: The GCP service account key for project internal-200822 should be rotated, since the current gcp_token.data is decryptable with the previously hardcoded auth tag.
  3. Re-encrypt gcp_token.data: After rotation, re-encrypt the new credential using encrypt.ts (which will now produce a new auth tag that should be set as the NGAT secret).
  4. Ensure NGAT is available: Verify that the NGAT secret is passed to workflow steps that invoke configure-remote (it is already available for SauceLabs/BrowserStack steps).

Secondary Concern: AES-GCM Nonce Reuse

The Bazel and SauceLabs integrations share the same (key, IV) pair (angular<<<..., 000003213213123213). Since AES-GCM is a stream cipher mode, reusing the same key+nonce for different plaintexts enables crib-dragging attacks. Ideally each integration should use a unique IV. This is noted for awareness but not addressed in this PR to keep the change minimal and focused.

Related

  • Google Issue Tracker: 495423772

The Bazel configure-remote action hardcodes the AES-256-GCM authentication
tag in constants.ts, while the SauceLabs and BrowserStack actions correctly
read it from the NGAT environment variable. This inconsistency means the
embedded GCP service account credential can be decrypted using only publicly
available parameters.

This change aligns the Bazel action with the existing SauceLabs and
BrowserStack patterns by reading the auth tag from process.env.NGAT.

Note: The checked-in configure-remote.js bundle will need to be rebuilt
by maintainers. The exposed credential should be rotated and gcp_token.data
re-encrypted.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request replaces a hardcoded authentication tag with an environment variable in the cryptographic constants. The review identifies a critical security vulnerability, noting that the change fails to prevent offline decryption because the encryption key is derived from a public value and the initialization vector remains hardcoded. It is recommended to move the encryption key to a secret environment variable and address the nonce reuse issue to ensure proper confidentiality.


export const alg = 'aes-256-gcm';
export const at = 'QwbjZ/z+yDtD+XZjKj9Ynw==';
export const at = process.env.NGAT!;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-critical critical

This change does not actually prevent offline decryption of the credential, which is the primary problem described in the PR. In AES-GCM, the authentication tag (at) is used for integrity and authenticity, but it does not provide confidentiality. Because the encryption key (k) is derived from a public value (GITHUB_REPOSITORY_OWNER) and the initialization vector (iv) is hardcoded, an attacker can still decrypt the gcp_token.data file without the authentication tag. In Node.js, the decipher.update() method returns the decrypted plaintext even if setAuthTag() is not called or if final() fails. To properly secure the credential, the encryption key (k) should be moved to a secret environment variable. Relying on the secrecy of the authentication tag is not an effective security measure for AES-GCM. Additionally, as noted in the PR description, reusing the same (Key, IV) pair across different integrations is a critical vulnerability (nonce reuse) that should be addressed by using unique, secret keys and IVs.

@anandppatil
Copy link
Copy Markdown
Author

Thanks for the review. The observation about AES-GCM's behavior is technically correct — decipher.update() returns plaintext before final() validates the auth tag, so auth tag secrecy alone doesn't provide full confidentiality.
However, a few points of context:

This PR intentionally matches the existing pattern used by github-actions/saucelabs/constants.ts and github-actions/browserstack/constants.ts in this same repository, which both use process.env.NGAT! for the auth tag. If the current pattern is considered insufficient, that's a broader design issue affecting all three integrations — not specific to this patch.
Fixing the key derivation is out of scope for an external contributor. Changing how k is derived would require the maintainers to re-encrypt all credential files (gcp_token.data and equivalents), update the encrypt.ts tooling, and coordinate secret rotation across all consuming repos (angular/angular, angular/components, etc.). That's a maintainer-driven change.
This PR explicitly calls out the deeper issues — the key derivation weakness and the nonce reuse concern are both documented in the PR description so the maintainers can address them holistically.

The goal of this patch is to remove the hardcoded auth tag, which currently makes the Bazel credential trivially recoverable (all four parameters public). Even if the auth tag alone doesn't provide full AES-GCM confidentiality, hardcoding it in source is strictly worse than reading it from a secret, and inconsistent with the rest of the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant