Skip to content

security: remove fake credential encryption - #3951

Closed
Tednoob17 wants to merge 1 commit into
angular:mainfrom
Tednoob17:fix/security/credentials
Closed

security: remove fake credential encryption#3951
Tednoob17 wants to merge 1 commit into
angular:mainfrom
Tednoob17:fix/security/credentials

Conversation

@Tednoob17

Copy link
Copy Markdown

Fixes recoverable production credentials (GCP, BrowserStack, SauceLabs) by removing fake AES-256-GCM encryption whose key was derivable from the public org name.

  • Delete committed .data credential files and encrypt.ts
  • Replace bundled JS with minimal ESM scripts that read credentials from env
  • Update action.yml inputs to accept secrets from GitHub Actions
  • Mark required inputs and pass them as env vars in composite action steps
  • Restore BUILD.bazel targets without encrypted blob references
  • Add test verifying no hardcoded keys or encrypted blobs remain

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

Copy link
Copy Markdown

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 refactors the GitHub Actions for Bazel remote execution, BrowserStack, and Sauce Labs to accept credentials directly via action inputs and environment variables, removing the previous embedded encrypted credential files and decryption logic. A new test script, test-credentials.mjs, is also added to verify that no encrypted files or hardcoded crypto constants are reintroduced. Feedback suggests quoting the script paths in the composite action files to prevent word-splitting issues on paths with spaces, and improving the test script to provide a clearer error message if no compiled JS files are found.

main: 'configure-remote.js'
using: composite
steps:
- run: node $GITHUB_ACTION_PATH/configure-remote.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Quote the script path to prevent word splitting and execution failures if the runner's workspace path contains spaces (e.g., on certain self-hosted runners or Windows environments).

    - run: node "$GITHUB_ACTION_PATH/configure-remote.js"

runs:
using: composite
steps:
- run: node $GITHUB_ACTION_PATH/set-browserstack-env.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Quote the script path to prevent word splitting and execution failures if the runner's workspace path contains spaces (e.g., on certain self-hosted runners or Windows environments).

    - run: node "$GITHUB_ACTION_PATH/set-browserstack-env.js"

runs:
using: composite
steps:
- run: node $GITHUB_ACTION_PATH/set-saucelabs-env.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Quote the script path to prevent word splitting and execution failures if the runner's workspace path contains spaces (e.g., on certain self-hosted runners or Windows environments).

    - run: node "$GITHUB_ACTION_PATH/set-saucelabs-env.js"

Comment thread github-actions/test-credentials.mjs Outdated
}
}

const jsFiles = fs.readdirSync(actionDir).filter((f) => f.endsWith('.js'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If no JS files are found in the action directory, the test will fail with a generic 'No process.env usage found' error. Consider explicitly checking if the JS files array is empty and pushing a more descriptive error message to help developers identify that the build outputs are missing.

@Tednoob17 Tednoob17 closed this Sep 2, 2026
@Tednoob17
Tednoob17 force-pushed the fix/security/credentials branch from 6491d8e to 6a54b1f Compare September 2, 2026 18:44
@Tednoob17 Tednoob17 reopened this Sep 2, 2026
@alan-agius4

Copy link
Copy Markdown
Contributor

Forked repos cannot access secrets, which disables RBE for pull requests and breaks existing workflows.

@alan-agius4 alan-agius4 closed this Sep 3, 2026
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.

2 participants