-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add firebase auto deployment #95
base: main
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@cp-amisha-i has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 50 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA new GitHub Actions workflow file named Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
1-1: workflow is empty
(syntax-check)
@@ -0,0 +1 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow file is empty and needs to be configured for Firebase deployment.
The workflow file requires configuration to automate Firebase deployments.
Here's a template to help you get started:
name: Deploy to Firebase
on:
push:
branches: [ main ] # Adjust branch name as needed
pull_request:
branches: [ main ] # Optional: for testing deployment
jobs:
deploy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: '20' # Adjust version as needed
- name: Install Dependencies
run: npm ci # or yarn install
- name: Build
run: npm run build # Adjust build command as needed
- name: Deploy to Firebase
uses: FirebaseExtended/action-hosting-deploy@v0
with:
repoToken: '${{ secrets.GITHUB_TOKEN }}'
firebaseServiceAccount: '${{ secrets.FIREBASE_SERVICE_ACCOUNT }}'
projectId: your-project-id # Add your Firebase project ID
channelId: live # or preview for PR deployments
You'll need to:
- Configure the branch names for deployment triggers
- Adjust Node.js version and build commands based on your project
- Add your Firebase project ID
- Set up the
FIREBASE_SERVICE_ACCOUNT
secret in your repository settings
Would you like me to help you set this up or create a GitHub issue to track this task?
🧰 Tools
🪛 actionlint (1.7.4)
1-1: workflow is empty
(syntax-check)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/FirebaseDeploy.yml (3)
1-7
: Enhance workflow triggers for better CI/CD practices.Consider these improvements:
- Add pull request trigger to test deployments before merging
- Add path filters to run only when relevant files change
on: push: branches: - main + paths: + - 'Splito/functions/**' + - 'firestore.*' + pull_request: + branches: + - main + paths: + - 'Splito/functions/**' + - 'firestore.*'
10-10
: Optimize runner selection for better performance.Using
ubuntu-latest
instead ofmacos-latest
would be more efficient for Firebase deployments as they don't require macOS-specific features.- runs-on: macos-latest + runs-on: ubuntu-latest
32-36
: Make working directory configuration more maintainable.Consider using a repository variable or environment variable for the functions directory path to make it more maintainable.
+ env: + FUNCTIONS_DIR: Splito/functions - name: Install Dependencies - working-directory: Splito/functions + working-directory: ${{ env.FUNCTIONS_DIR }} run: | npm ci
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
44-44: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
51-51: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
57-57: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/FirebaseDeploy.yml (1)
1-58
: Verify required secrets are configured.Please ensure these secrets are configured in your repository settings:
FIREBASE_SERVICE_JSON
: Base64 encoded Firebase service account JSONFIREBASE_PROJECT_ID
: Your Firebase project ID🧰 Tools
🪛 actionlint (1.7.4)
44-44: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
51-51: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
57-57: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/FirebaseDeploy.yml (3)
3-6
: Add pull request trigger for deployment testing.Consider adding a pull request trigger to test deployments before merging to main. This helps catch deployment issues early.
on: push: branches: - main + pull_request: + branches: + - main
38-38
: Fix typo in step name.There's a typo in the step name: "henticate" should be "Authenticate".
- - name: henticate Firebase using service account + - name: Authenticate Firebase using service account
8-57
: Add post-deployment cleanup and verification.Consider adding these improvements:
- Post-deployment verification to ensure successful deployment
- Cleanup step in case of failures
Add this step after the deployments:
- name: Verify Deployment if: success() env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} run: | # Verify Functions firebase functions:list --project "$FIREBASE_PROJECT_ID" | grep "Status: ACTIVE" || { echo "Function deployment verification failed" exit 1 } # Verify Firestore Rules firebase firestore:rules --project "$FIREBASE_PROJECT_ID" | grep "deployed successfully" || { echo "Firestore rules deployment verification failed" exit 1 } - name: Cleanup if: always() run: | rm -f firebase-service-account.json || true🧰 Tools
🪛 actionlint (1.7.4)
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
50-50: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
56-56: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
50-50: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
56-56: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
.github/workflows/FirebaseDeploy.yml (1)
47-57
:⚠️ Potential issueFix deployment command syntax and add error handling.
The deployment commands have several issues:
- Incorrect syntax in
--only
flag- Missing error handling
- Shell commands lack proper quoting
- name: Deploy Firestore Rules and Indexes env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} run: | - firebase deploy --only firestore:rules, firestore:indexes --project $FIREBASE_PROJECT_ID + firebase deploy --only "firestore:rules,firestore:indexes" --project "$FIREBASE_PROJECT_ID" || { + echo "Failed to deploy Firestore rules and indexes" + exit 1 + } - name: Deploy Cloud Functions env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} run: | - firebase deploy --only functions --project $FIREBASE_PROJECT_ID + firebase deploy --only functions --project "$FIREBASE_PROJECT_ID" || { + echo "Failed to deploy Cloud Functions" + exit 1 + }Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
50-50: shellcheck reported issue in this script: SC2086:info:1:69: Double quote to prevent globbing and word splitting
(shellcheck)
56-56: shellcheck reported issue in this script: SC2086:info:1:44: Double quote to prevent globbing and word splitting
(shellcheck)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/FirebaseDeploy.yml (1)
6-8
: Add workflow-level safeguardsConsider adding the following workflow-level configurations:
- Environment protection
- Timeout settings
- Concurrency control
jobs: deploy: runs-on: ubuntu-latest + environment: production + timeout-minutes: 15 + concurrency: + group: firebase-deploy-${{ github.ref }} + cancel-in-progress: false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
34-34: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
34-34: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/FirebaseDeploy.yml (2)
30-37
:⚠️ Potential issueFix security vulnerabilities in Firebase authentication
Current implementation has several security concerns:
- Service account JSON is written to disk without cleanup
- Missing error handling
- Shell commands lack proper quoting
- name: Authenticate Firebase using the service account env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} FIREBASE_SERVICE_JSON: ${{ secrets.FIREBASE_SERVICE_JSON }} run: | - echo $FIREBASE_SERVICE_JSON | base64 --decode > firebase-service-account.json - firebase auth:activate-service-account --key-file=firebase-service-account.json - firebase use --add --project $FIREBASE_PROJECT_ID + echo "$FIREBASE_SERVICE_JSON" | base64 --decode | firebase auth:activate-service-account --project "$FIREBASE_PROJECT_ID" --json-input + firebase use --add "$FIREBASE_PROJECT_ID" || exit 1Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
34-34: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
34-34: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
39-50
: 🛠️ Refactor suggestionImprove deployment reliability and error handling
The deployment steps need improvements in:
- Error handling
- Working directory consistency
- Deployment verification
- Shell command quoting
- name: Deploy Firestore Rules and Indexes env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} run: | - firebase deploy --only firestore:rules,firestore:indexes --project $FIREBASE_PROJECT_ID + firebase deploy --only "firestore:rules,firestore:indexes" --project "$FIREBASE_PROJECT_ID" || exit 1 + # Verify deployment + firebase firestore:rules --project "$FIREBASE_PROJECT_ID" | grep "deployed successfully" || exit 1 - name: Deploy Cloud Functions env: FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} + working-directory: functions run: | - cd functions - firebase deploy --only functions --project $FIREBASE_PROJECT_ID + firebase deploy --only functions --project "$FIREBASE_PROJECT_ID" || exit 1 + # Verify deployment + firebase functions:list --project "$FIREBASE_PROJECT_ID" | grep "Status: ACTIVE" || exit 1Likely invalid or redundant comment.
🧰 Tools
🪛 actionlint (1.7.4)
42-42: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
48-48: shellcheck reported issue in this script: SC2086:info:2:44: Double quote to prevent globbing and word splitting
(shellcheck)
on: | ||
push: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict deployment triggers to specific branches
Running deployments on every push is risky. Consider limiting deployments to specific branches (e.g., main/master) or tags.
on:
push:
+ branches:
+ - main # or your production branch
+ tags:
+ - 'v*' # optional: deploy on version tags
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
on: | |
push: | |
on: | |
push: | |
branches: | |
- main # or your production branch | |
tags: | |
- 'v*' # optional: deploy on version tags |
.github/workflows/FirebaseDeploy.yml
Outdated
- name: Install Dependencies | ||
working-directory: functions | ||
run: | | ||
cd functions | ||
npm install | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize dependency installation
The current setup has several inefficiencies:
- Redundant directory change when
working-directory
is already set - Using
npm install
instead ofnpm ci
for reproducible builds - Missing dependency caching
- name: Install Dependencies
working-directory: functions
run: |
- cd functions
- npm install
+ npm ci
+
+ - name: Cache dependencies
+ uses: actions/cache@v3
+ with:
+ path: ~/.npm
+ key: npm-${{ hashFiles('functions/package-lock.json') }}
+ restore-keys: npm-
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/FirebaseDeploy.yml (2)
3-5
:⚠️ Potential issueRestrict deployment triggers to specific branches
Running deployments on every push is risky. Consider limiting deployments to specific branches.
on: push: + branches: + - main # or your production branch + paths: + - 'functions/**' + - 'firestore.rules' + - 'firestore.indexes.json'
30-34
: 🛠️ Refactor suggestionUse
npm ci
for reproducible buildsReplace
npm install
withnpm ci
to ensure consistent installations across different environments.- name: Install Dependencies working-directory: functions run: | - npm install + npm ci
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/FirebaseDeploy.yml (3)
6-9
: LGTM! Job configuration follows best practices.Using
ubuntu-latest
is the recommended choice for Firebase deployments.
11-18
: LGTM! Node.js setup is well-configured.Using the latest action versions and Node.js 20 follows best practices.
46-59
: 🛠️ Refactor suggestionImprove deployment steps with error handling and verification
The deployment steps need several improvements:
- Remove redundant directory change
- Add error handling
- Add deployment verification
- name: Deploy Firestore Rules and Indexes env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }} FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} run: | - firebase deploy --only firestore:rules,firestore:indexes + firebase deploy --only "firestore:rules,firestore:indexes" --project "$FIREBASE_PROJECT_ID" || exit 1 + echo "Verifying deployment..." + firebase firestore:rules --project "$FIREBASE_PROJECT_ID" | grep "deployed successfully" || exit 1 - name: Deploy Cloud Functions env: FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }} FIREBASE_PROJECT_ID: ${{ secrets.FIREBASE_PROJECT_ID }} + working-directory: functions run: | - cd functions - firebase deploy --only functions + firebase deploy --only functions --project "$FIREBASE_PROJECT_ID" || exit 1 + echo "Verifying deployment..." + firebase functions:list --project "$FIREBASE_PROJECT_ID" | grep "Status: ACTIVE" || exit 1Likely invalid or redundant comment.
.github/workflows/FirebaseDeploy.yml
Outdated
- name: Authenticate Firebase using the service account | ||
env: | ||
GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | ||
run: | | ||
echo $GOOGLE_APPLICATION_CREDENTIALS | base64 --decode > google-application-credentials.json | ||
export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve security of Firebase authentication
The current implementation has several security concerns:
- Credentials are written to disk without cleanup
- Shell commands lack proper quoting
- Export command masks potential errors
- name: Authenticate Firebase using the service account
env:
GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }}
run: |
- echo $GOOGLE_APPLICATION_CREDENTIALS | base64 --decode > google-application-credentials.json
- export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json
+ CREDENTIALS_FILE=$(mktemp)
+ echo "$GOOGLE_APPLICATION_CREDENTIALS" | base64 --decode > "$CREDENTIALS_FILE"
+ export GOOGLE_APPLICATION_CREDENTIALS="$CREDENTIALS_FILE"
+ trap 'rm -f "$CREDENTIALS_FILE"' EXIT
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Authenticate Firebase using the service account | |
env: | |
GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | |
run: | | |
echo $GOOGLE_APPLICATION_CREDENTIALS | base64 --decode > google-application-credentials.json | |
export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json | |
- name: Authenticate Firebase using the service account | |
env: | |
GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | |
run: | | |
CREDENTIALS_FILE=$(mktemp) | |
echo "$GOOGLE_APPLICATION_CREDENTIALS" | base64 --decode > "$CREDENTIALS_FILE" | |
export GOOGLE_APPLICATION_CREDENTIALS="$CREDENTIALS_FILE" | |
trap 'rm -f "$CREDENTIALS_FILE"' EXIT |
🧰 Tools
🪛 actionlint (1.7.4)
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/FirebaseDeploy.yml (1)
3-5
:⚠️ Potential issueRestrict deployment triggers to specific branches
Running deployments on every push is risky. Consider limiting deployments to specific branches.
on: push: + branches: + - main # or your production branch + tags: + - 'v*' # optional: deploy on version tags
🧹 Nitpick comments (1)
.github/workflows/FirebaseDeploy.yml (1)
53-57
: Improve Cloud Functions deploymentThe deployment step changes directory unnecessarily when
working-directory
could be used.- name: Deploy Cloud Functions + working-directory: functions run: | - cd functions firebase deploy --only functions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/FirebaseDeploy.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/FirebaseDeploy.yml
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
.github/workflows/FirebaseDeploy.yml (3)
6-9
: LGTM! Good choice of runnerUsing
ubuntu-latest
is the correct choice for Firebase deployments, offering good performance and cost efficiency.
30-34
: 🛠️ Refactor suggestionUse
npm ci
instead ofnpm install
For CI/CD environments,
npm ci
is preferred as it:
- Ensures reproducible builds by strictly following package-lock.json
- Is generally faster than npm install
- Removes node_modules before installation
- name: Install Dependencies working-directory: functions run: | - npm install + npm ciLikely invalid or redundant comment.
58-61
: 🛠️ Refactor suggestionRemove redundant cleanup step
This cleanup step should be removed as it:
- Uses a hardcoded path that might not exist
- Is redundant with the trap-based cleanup in the authentication step
- - name: Remove credentials file - if: success() || failure() - run: | - rm google-application-credentials.jsonLikely invalid or redundant comment.
.github/workflows/FirebaseDeploy.yml
Outdated
# - name: Deploy Firestore Rules and Indexes | ||
# run: | | ||
# echo $GOOGLE_APPLICATION_CREDENTIALS | ||
# cat $GOOGLE_APPLICATION_CREDENTIALS | ||
# firebase deploy --only firestore:rules,firestore:indexes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove commented deployment step
The commented-out deployment step is redundant and includes debug statements that could leak sensitive information.
- # - name: Deploy Firestore Rules and Indexes
- # run: |
- # echo $GOOGLE_APPLICATION_CREDENTIALS
- # cat $GOOGLE_APPLICATION_CREDENTIALS
- # firebase deploy --only firestore:rules,firestore:indexes
.github/workflows/FirebaseDeploy.yml
Outdated
- name: Authenticate Firebase using the service account | ||
env: | ||
FIREBASE_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | ||
run: | | ||
echo $FIREBASE_CREDENTIALS | base64 --decode > google-application-credentials.json | ||
export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json | ||
firebase deploy --only firestore:rules,firestore:indexes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve security of Firebase authentication
Current implementation has several security concerns:
- Credentials are written to disk without secure handling
- Shell commands lack proper quoting
- Export command masks potential errors
- name: Authenticate Firebase using the service account
env:
FIREBASE_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }}
run: |
- echo $FIREBASE_CREDENTIALS | base64 --decode > google-application-credentials.json
- export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json
+ CREDENTIALS_FILE=$(mktemp)
+ echo "$FIREBASE_CREDENTIALS" | base64 --decode > "$CREDENTIALS_FILE"
+ CREDENTIALS_PATH="$(pwd)/$CREDENTIALS_FILE"
+ export GOOGLE_APPLICATION_CREDENTIALS="$CREDENTIALS_PATH"
+ trap 'rm -f "$CREDENTIALS_FILE"' EXIT
firebase deploy --only firestore:rules,firestore:indexes
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: Authenticate Firebase using the service account | |
env: | |
FIREBASE_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | |
run: | | |
echo $FIREBASE_CREDENTIALS | base64 --decode > google-application-credentials.json | |
export GOOGLE_APPLICATION_CREDENTIALS=$(pwd)/google-application-credentials.json | |
firebase deploy --only firestore:rules,firestore:indexes | |
- name: Authenticate Firebase using the service account | |
env: | |
FIREBASE_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | |
run: | | |
CREDENTIALS_FILE=$(mktemp) | |
echo "$FIREBASE_CREDENTIALS" | base64 --decode > "$CREDENTIALS_FILE" | |
CREDENTIALS_PATH="$(pwd)/$CREDENTIALS_FILE" | |
export GOOGLE_APPLICATION_CREDENTIALS="$CREDENTIALS_PATH" | |
trap 'rm -f "$CREDENTIALS_FILE"' EXIT | |
firebase deploy --only firestore:rules,firestore:indexes |
🧰 Tools
🪛 actionlint (1.7.4)
42-42: shellcheck reported issue in this script: SC2086:info:1:6: Double quote to prevent globbing and word splitting
(shellcheck)
42-42: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
Summary by CodeRabbit