Skip to content

fix(ci): replace dynamic secret access with explicit secret references#4151

Merged
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/fix-ci-secrets-exposure
Apr 14, 2026
Merged

fix(ci): replace dynamic secret access with explicit secret references#4151
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/fix-ci-secrets-exposure

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Replace secrets[matrix.ecr_repo_secret] with explicit conditional expressions referencing only the specific secrets needed (ECR_APP, ECR_MIGRATIONS, ECR_REALTIME)
  • Resolves CodeQL "Excessive Secrets Exposure" warning in build-dev and build-amd64 jobs

Type of Change

  • Bug fix

Testing

Tested manually — verified YAML syntax and conditional logic

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Resolves CodeQL "Excessive Secrets Exposure" warning by replacing
secrets[matrix.ecr_repo_secret] with conditional expressions that
reference only the specific secrets needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Apr 14, 2026 5:07am

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 14, 2026

PR Summary

Medium Risk
Changes GitHub Actions image build/push logic by altering how ECR repo names are resolved; a mistake in the conditional mapping could break CI image publishing.

Overview
CI workflow now resolves ECR repository names without dynamic secret indexing. In .github/workflows/ci.yml, the build-dev and build-amd64 jobs add a Resolve ECR repo name step that maps matrix.ecr_repo_secret to one of secrets.ECR_APP, secrets.ECR_MIGRATIONS, or secrets.ECR_REALTIME.

Build/tag steps are updated to use the resolved output (steps.ecr-repo.outputs.name) instead of secrets[matrix.ecr_repo_secret], reducing secrets exposure flagged by security scanning.

Reviewed by Cursor Bugbot for commit 830454b. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 14, 2026

Greptile Summary

This PR replaces dynamic secret indexing (secrets[matrix.ecr_repo_secret]) with explicit per-secret conditional chains in the build-dev and build-amd64 jobs, resolving a CodeQL "Excessive Secrets Exposure" warning. Both prior review concerns — silent wrong-repo fallthrough and direct secret interpolation inside run: scripts — are fully addressed: an explicit ECR_REALTIME guard and a terminal || '' fallback are in place on both lines, and secrets flow through an env: block rather than direct expression interpolation.

Confidence Score: 5/5

  • This PR is safe to merge — all prior reviewer concerns are fully resolved with no new issues introduced.
  • Both P1 concerns from prior review threads (silent secret fallthrough and direct interpolation inside run scripts) are addressed: an explicit ECR_REALTIME guard and terminal || '' fallback prevent wrong-repo selection, and the env: block pattern is applied consistently to both build-dev and build-amd64. No remaining P0 or P1 findings.
  • No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Replaced dynamic secret access secrets[matrix.ecr_repo_secret] with explicit conditional chains on lines 95 and 168; added env: block for injection, explicit ECR_REALTIME guard, and `

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[matrix.ecr_repo_secret] --> B{== 'ECR_APP'?}
    B -- Yes --> C[secrets.ECR_APP]
    C --> D{non-empty?}
    D -- Yes --> G[ECR_REPO = ECR_APP value]
    D -- No --> E
    B -- No --> E{== 'ECR_MIGRATIONS'?}
    E -- Yes --> F[secrets.ECR_MIGRATIONS]
    F --> H{non-empty?}
    H -- Yes --> I[ECR_REPO = ECR_MIGRATIONS value]
    H -- No --> J
    E -- No --> J{== 'ECR_REALTIME'?}
    J -- Yes --> K[secrets.ECR_REALTIME]
    K --> L{non-empty?}
    L -- Yes --> M[ECR_REPO = ECR_REALTIME value]
    L -- No --> N
    J -- No --> N[ECR_REPO = '' terminal fallback]
    G --> O[echo name=ECR_REPO to GITHUB_OUTPUT]
    I --> O
    M --> O
    N --> O
    O --> P[Build & push Docker image]
    N -.->|empty repo name causes explicit build failure| P
Loading

Reviews (2): Last reviewed commit: "fix(ci): add explicit ECR_REALTIME guard..." | Re-trigger Greptile

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
… injection

- Prevent silent fallthrough to ECR_REALTIME for unrecognized secret keys
- Move build-amd64 secret resolution to env: block matching build-dev pattern
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 830454b. Configure here.

@waleedlatif1 waleedlatif1 merged commit 48d5101 into staging Apr 14, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/fix-ci-secrets-exposure branch April 14, 2026 05:10
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