Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
""" WalkthroughA new GitHub Actions workflow, Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub PR
participant GitHub Actions
participant Azure
participant PR Comment
GitHub PR->>GitHub Actions: PR opened/reopened/synchronized
GitHub Actions->>Azure: Login and setup
GitHub Actions->>Azure: Build & push Docker image
GitHub Actions->>Azure: Create resource group & container app
Azure-->>GitHub Actions: Return deployed API URL
GitHub Actions->>PR Comment: Post deployment details
GitHub PR->>GitHub Actions: PR closed
GitHub Actions->>Azure: Login
GitHub Actions->>Azure: Delete resource group & image
GitHub Actions->>PR Comment: Post cleanup confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
""" Tip You can disable sequence diagrams in the walkthrough.Disable the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
waxb
force-pushed
the
waxb/feat/add-autodev-pr
branch
from
38efcff to
ff5d0e1
Compare
🧹 Brease PR Environment Cleaned UpAll Azure resources for this PR have been deleted:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
.github/workflows/azure-pr-deploy.yml (3)
140-150:--no-waitmay publish misleading “cleanup completed” messages
Deletion happens asynchronously; a user clicking the resource group minutes later will still see it. Either callaz group wait --deletedafter the delete or drop--no-waitso the comment reflects actual state.
152-160: Image cleanup may leave orphan manifests
az acr repository delete --tagremoves only the specific tag; the manifest remains if other tags (e.g.,latest) reference it. Consider--yes --manifestto ensure the blob is removed and storage costs are avoided.
28-128: Trailing whitespace & missing EOF newline flagged by YAML-lint
Clean up trailing spaces and ensure the file ends with a newline to keep lint pipelines green.Also applies to: 167-172
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/azure-pr-deploy.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 167-167: trailing spaces
(trailing-spaces)
[error] 171-171: trailing spaces
(trailing-spaces)
[error] 172-172: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy
- GitHub Check: Analyze (go)
| on: | ||
| pull_request: | ||
| types: [opened, reopened, synchronize, closed] | ||
|
|
There was a problem hiding this comment.
Unrestricted PR trigger exposes Azure resources to untrusted fork code
Running this workflow for every pull_request event allows code from forks to execute arbitrary az * commands with the federated service-principal, giving write access to the subscription. Restrict deployments to trusted actors, e.g.
-on:
- pull_request:
- types: [opened, reopened, synchronize, closed]
+on:
+ pull_request:
+ types: [opened, reopened, synchronize, closed]
+ # Only run when the PR originates from this repo (not forks)
+ branches: [main] # adjust to default branchor gate the jobs behind a protected environment that requires manual approval.
📝 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: | |
| pull_request: | |
| types: [opened, reopened, synchronize, closed] | |
| on: | |
| pull_request: | |
| types: [opened, reopened, synchronize, closed] | |
| # Only run when the PR originates from this repo (not forks) | |
| branches: [main] # adjust to default branch |
🤖 Prompt for AI Agents
In .github/workflows/azure-pr-deploy.yml around lines 3 to 6, the workflow
triggers on all pull_request events, which allows untrusted fork code to run
Azure commands with elevated permissions. To fix this, restrict the workflow
trigger to trusted actors only by adding a condition that checks the pull
request author or use a protected environment with manual approval to gate
deployment jobs. This prevents unauthorized code from executing sensitive Azure
commands.
ff5d0e1 to
4c069bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.github/workflows/azure-pr-deploy.yml (3)
3-6: Workflow runs for untrusted forks – restrict trigger or require an approval
40-46: Service-principal IDs are hard-coded – move to repo secrets
61-73:az group/app env createare not idempotent – handle re-runs gracefully
🧹 Nitpick comments (1)
.github/workflows/azure-pr-deploy.yml (1)
113-131: Minor: trailing whitespace violates YAML linting rulesSeveral comment lines contain stray spaces, causing the YAML-Lint errors reported by CI. Strip them to keep the workflow green.
Also applies to: 172-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/azure-pr-deploy.yml(1 hunks)apps/api/Dockerfile(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 177-177: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (go)
apps/api/Dockerfile
Outdated
| bash \ | ||
| && curl -1sLf 'https://dl.cloudsmith.io/public/infisical/infisical-cli/setup.deb.sh' | bash \ | ||
| && apt-get update && apt-get install -y infisical \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| COPY --from=builder /brease /usr/local/bin/ | ||
| CMD ["brease"] | ||
|
|
||
| # Create startup script that fetches secrets and runs the app | ||
| RUN echo '#!/bin/bash\n\ |
There was a problem hiding this comment.
Avoid piping remote installer to bash; pin & verify Infisical CLI package
curl … | bash executes unauthenticated code during every build, opening the door to supply-chain hijacking.
• Fetch the GPG key, add the signed .deb, or at minimum pin an exact version (infisical=1.7.3-1).
• Validate checksums before install.
- && curl -1sLf 'https://dl.cloudsmith.io/public/infisical/infisical-cli/setup.deb.sh' | bash \
- && apt-get update && apt-get install -y infisical \
+ && curl -fsSL https://dl.cloudsmith.io/public/infisical/infisical-cli/gpg.key | gpg --dearmor -o /usr/share/keyrings/infisical.gpg \
+ && echo "deb [signed-by=/usr/share/keyrings/infisical.gpg] https://dl.cloudsmith.io/public/infisical/infisical-cli/deb/debian bookworm main" \
+ > /etc/apt/sources.list.d/infisical.list \
+ && apt-get update \
+ && apt-get install -y infisical=1.7.3-1 \📝 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.
| # Update package lists and install ca-certificates and Infisical CLI | |
| RUN apt-get clean && \ | |
| rm -rf /var/lib/apt/lists/* && \ | |
| apt-get update --fix-missing && apt-get install -y \ | |
| ca-certificates \ | |
| curl \ | |
| bash \ | |
| && curl -1sLf 'https://dl.cloudsmith.io/public/infisical/infisical-cli/setup.deb.sh' | bash \ | |
| && apt-get update && apt-get install -y infisical \ | |
| && rm -rf /var/lib/apt/lists/* | |
| # Update package lists and install ca-certificates and Infisical CLI | |
| RUN apt-get clean && \ | |
| rm -rf /var/lib/apt/lists/* && \ | |
| apt-get update --fix-missing && apt-get install -y \ | |
| ca-certificates \ | |
| curl \ | |
| bash \ | |
| && curl -fsSL https://dl.cloudsmith.io/public/infisical/infisical-cli/gpg.key \ | |
| | gpg --dearmor -o /usr/share/keyrings/infisical.gpg \ | |
| && echo "deb [signed-by=/usr/share/keyrings/infisical.gpg] \ | |
| https://dl.cloudsmith.io/public/infisical/infisical-cli/deb/debian \ | |
| bookworm main" \ | |
| > /etc/apt/sources.list.d/infisical.list \ | |
| && apt-get update \ | |
| && apt-get install -y infisical=1.7.3-1 \ | |
| && rm -rf /var/lib/apt/lists/* |
🤖 Prompt for AI Agents
In apps/api/Dockerfile around lines 13 to 22, avoid using `curl ... | bash` to
install the Infisical CLI as it executes unauthenticated code. Instead, fetch
and add the GPG key securely, add the Infisical repository with a pinned
version, and validate package checksums before installation. This ensures the
package authenticity and prevents supply-chain attacks.
4c069bb to
459ae0c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
.github/workflows/azure-pr-deploy.yml (5)
3-6: Restrict workflow trigger to trusted PRs or protect via environmentRunning the workflow for all
pull_requestevents lets untrusted-fork code execute privilegedazcommands with your federated service principal. Limit the trigger to internal PRs (or require a protectedenvironment) to prevent supply-chain abuse.on: pull_request: types: [opened, reopened, synchronize, closed] + # Only run when the PR comes from the same repository, not forks + branches: [main] # adjust to default branch + # Alternatively, move the jobs under a protected `environment`
40-46: Hard-coded service-principal IDs violate secret-management policy
client-id,tenant-id, andsubscription-idare committed in plain text. Rotate them immediately and inject via repository/org secrets instead.- client-id: 2bfd5f96-2fa1-44ff-af35-17722c04027f - tenant-id: 760d74d4-e9ad-46f8-bbae-e20bce9596ab - subscription-id: 5533053b-de97-432f-908a-c7018c458532 + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
138-142: Credentials leaked in cleanup job as wellCleanup repeats the same hard-coded IDs. Apply the same secret-based fix here.
- client-id: 2bfd5f96-2fa1-44ff-af35-17722c04027f - tenant-id: 760d74d4-e9ad-46f8-bbae-e20bce9596ab - subscription-id: 5533053b-de97-432f-908a-c7018c458532 + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
88-95: Secrets exposed as plain env-vars & flagged by gitleaks
INFISICAL_CLIENT_SECRET(line 94) and Azure IDs are visible viaaz containerapp revision show, breaching zero-trust. Load them as Container App secrets and reference withsecretref:.- --env-vars \ - PORT=4400 \ - NODE_ENV=staging \ - INFISICAL_PROJECT_ID=642ed4939db25595ac7eb9cd \ - INFISICAL_ENVIRONMENT=staging \ - INFISICAL_CLIENT_ID=cd3cc75e-d3bc-4b42-ba5d-6b68f56afa78 \ - INFISICAL_CLIENT_SECRET=b8791a2f3571a3a24c8d938c627f31490427b7466ef7e335782bd1023d145b9a + --secrets \ + infisicalClientId=${{ secrets.INFISICAL_CLIENT_ID }} \ + infisicalClientSecret=${{ secrets.INFISICAL_CLIENT_SECRET }} \ + --env-vars \ + PORT=4400 \ + NODE_ENV=staging \ + INFISICAL_PROJECT_ID=642ed4939db25595ac7eb9cd \ + INFISICAL_ENVIRONMENT=staging \ + INFISICAL_CLIENT_ID=secretref:infisicalClientId \ + INFISICAL_CLIENT_SECRET=secretref:infisicalClientSecret
61-73: Make resource creation idempotent to support re-runs
az group createandaz containerapp env createfail when resources already exist, breaking retryability. Guard with... show || ... create(or--exists).-az group create \ - --name ${{ env.AZURE_RESOURCE_GROUP }} \ - --location ${{ env.AZURE_LOCATION }} +az group show --name ${{ env.AZURE_RESOURCE_GROUP }} \ + --query name --output tsv 2>/dev/null || \ +az group create \ + --name ${{ env.AZURE_RESOURCE_GROUP }} \ + --location ${{ env.AZURE_LOCATION }} -az containerapp env create \ - --name ${{ env.CONTAINER_APP_ENVIRONMENT }} \ - --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ - --location ${{ env.AZURE_LOCATION }} +az containerapp env show \ + --name ${{ env.CONTAINER_APP_ENVIRONMENT }} \ + --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ + --query name --output tsv 2>/dev/null || \ +az containerapp env create \ + --name ${{ env.CONTAINER_APP_ENVIRONMENT }} \ + --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ + --location ${{ env.AZURE_LOCATION }}
🧹 Nitpick comments (1)
.github/workflows/azure-pr-deploy.yml (1)
28-28: Clean up trailing whitespace and add final newlineMinor YAML-lint violations can be fixed automatically; keeps diffs clean.
- ## 🚀 Brease PR Environment Deployed Successfully!· + ## 🚀 Brease PR Environment Deployed Successfully! ... - > ✅ Cleanup completed successfully.· + > ✅ Cleanup completed successfully. +Also applies to: 112-112, 116-116, 121-121, 126-126, 130-130, 171-171, 175-176
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/azure-pr-deploy.yml(1 hunks)apps/api/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/Dockerfile
🧰 Additional context used
🪛 Gitleaks (8.27.2)
.github/workflows/azure-pr-deploy.yml
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 116-116: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 171-171: trailing spaces
(trailing-spaces)
[error] 175-175: trailing spaces
(trailing-spaces)
[error] 176-176: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy
- GitHub Check: Analyze (go)
waxb
force-pushed
the
waxb/feat/add-autodev-pr
branch
2 times, most recently
from
e7a2954 to
081e364
Compare
waxb
force-pushed
the
waxb/feat/add-autodev-pr
branch
from
081e364 to
75dd475
Compare
waxb
force-pushed
the
waxb/feat/add-autodev-pr
branch
from
1b0e5c1 to
2e424d2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
.github/workflows/azure-pr-deploy.yml (3)
3-6: Restrict workflow trigger to trusted refsRunning on every
pull_request(incl. forks) gives untrusted code write access to the subscription. Limit to internal branches or gate behind a protected environment.
43-46: Service-principal IDs should come from secrets, not codeHard-coding blocks rotation and leaks metadata; use
${{ secrets.* }}instead.
145-147: Same issue in cleanup job — replace with secrets
🧹 Nitpick comments (2)
apps/api/start.sh (1)
5-5: Quote params & place command after---infisical run --projectId=$INFISICAL_PROJECT_ID --env=staging brease +infisical run --projectId="$INFISICAL_PROJECT_ID" --env=staging -- brease.github/workflows/azure-pr-deploy.yml (1)
104-109: Quote vars inaz containerapp show(SC2086)-URL=$(az containerapp show \ - --name ${{ env.API_APP_NAME }} \ - --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ +URL=$(az containerapp show \ + --name "${{ env.API_APP_NAME }}" \ + --resource-group "${{ env.AZURE_RESOURCE_GROUP }}" \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/azure-pr-deploy.yml(1 hunks)apps/api/Dockerfile(1 hunks)apps/api/start.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/Dockerfile
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/azure-pr-deploy.yml
103-103: shellcheck reported issue in this script: SC2086:info:6:28: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 Gitleaks (8.27.2)
.github/workflows/azure-pr-deploy.yml
99-99: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 135-135: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 180-180: trailing spaces
(trailing-spaces)
[error] 181-181: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Shellcheck (0.10.0)
apps/api/start.sh
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy
- GitHub Check: Analyze (go)
| @@ -0,0 +1,5 @@ | |||
| #!/bin/bash | |||
| set -e | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the script with set -euo pipefail
Fail fast on unset variables and pipe errors for more predictable behaviour.
-set -e
+set -euo pipefail🤖 Prompt for AI Agents
In apps/api/start.sh at line 2, replace the current 'set -e' command with 'set
-euo pipefail' to make the script fail fast on unset variables and pipe errors,
improving its robustness and predictability.
| #!/bin/bash | ||
| set -e | ||
| echo "Fetching secrets from Infisical..." | ||
| export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix SC2155 and quote sensitive expansions
Declare then export and always quote to avoid word-splitting / injection.
-export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent)
+INFISICAL_TOKEN=$(infisical login \
+ --method=universal-auth \
+ --client-id="$INFISICAL_CLIENT_ID" \
+ --client-secret="$INFISICAL_CLIENT_SECRET" \
+ --plain --silent)
+export INFISICAL_TOKEN📝 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.
| export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent) | |
| INFISICAL_TOKEN=$(infisical login \ | |
| --method=universal-auth \ | |
| --client-id="$INFISICAL_CLIENT_ID" \ | |
| --client-secret="$INFISICAL_CLIENT_SECRET" \ | |
| --plain --silent) | |
| export INFISICAL_TOKEN |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In apps/api/start.sh at line 4, the export statement combines declaration and
export, which triggers SC2155, and the variable expansion is unquoted, risking
word-splitting or injection. Fix this by first declaring the variable on a
separate line, quoting the command substitution to prevent word-splitting, and
then exporting the variable on the next line.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.github/workflows/azure-pr-deploy.yml (2)
3-6: Workflow still runs on untrusted fork PRsSame concern raised previously: exposing Azure credentials to forked PRs is a high-risk vector. Restrict the trigger or gate with protected environments.
40-46: Service-principal identifiers are hard-codedClient-id / tenant-id / subscription-id must come from repository or org secrets to allow rotation and avoid accidental disclosure.
🧹 Nitpick comments (1)
.github/workflows/azure-pr-deploy.yml (1)
61-66: Make resource-group creation idempotent
az group createexits non-zero only on real errors, but flooding logs with “already exists” is noisy. A quick guard avoids that and saves ~10 s per run:if ! az group exists --name "$AZURE_RESOURCE_GROUP"; then az group create --name "$AZURE_RESOURCE_GROUP" --location "$AZURE_LOCATION" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/azure-pr-deploy.yml(1 hunks)apps/api/Dockerfile(1 hunks)apps/api/start.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/Dockerfile
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/azure-pr-deploy.yml
103-103: shellcheck reported issue in this script: SC2086:info:6:28: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 Gitleaks (8.27.2)
.github/workflows/azure-pr-deploy.yml
99-99: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 135-135: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 180-180: trailing spaces
(trailing-spaces)
[error] 181-181: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Shellcheck (0.10.0)
apps/api/start.sh
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
| #!/bin/bash | ||
| set -e | ||
| echo "Fetching secrets from Infisical..." | ||
| export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote variables & split assignment to avoid masked return value
Capturing the token in the same export statement both masks infisical login’s exit status (SC2155) and leaves expansions un-quoted, which will break if any variable contains whitespace. Recommend:
-export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent)
+INFISICAL_TOKEN="$(infisical login \
+ --method=universal-auth \
+ --client-id="$INFISICAL_CLIENT_ID" \
+ --client-secret="$INFISICAL_CLIENT_SECRET" \
+ --plain --silent)"
+export INFISICAL_TOKENAdds quoting, preserves exit-code, and keeps the script safety guarantees already implied by set -e.
📝 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.
| export INFISICAL_TOKEN=$(infisical login --method=universal-auth --client-id=$INFISICAL_CLIENT_ID --client-secret=$INFISICAL_CLIENT_SECRET --plain --silent) | |
| INFISICAL_TOKEN="$(infisical login \ | |
| --method=universal-auth \ | |
| --client-id="$INFISICAL_CLIENT_ID" \ | |
| --client-secret="$INFISICAL_CLIENT_SECRET" \ | |
| --plain --silent)" | |
| export INFISICAL_TOKEN |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 4-4: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In apps/api/start.sh at line 4, the export statement combines variable
assignment and command substitution without quoting, which masks the exit status
and risks word splitting. To fix this, split the assignment and export into two
separate commands, assign the output of the infisical login command to the
variable with proper double quotes around the command substitution to preserve
whitespace, then export the variable on the next line. This preserves the exit
code and ensures safe handling of the token value.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.github/workflows/azure-pr-deploy.yml (4)
3-6: Unrestricted PR trigger still exposes subscription – restrict or gate.
The workflow continues to run on anypull_requestevent, including from forks, letting untrusted code execute privileged Azure commands. Please scope the trigger to trusted branches / actors or protect the job behind a required-approvalenvironment.on: pull_request: - types: [opened, reopened, synchronize, closed] + types: [opened, reopened, synchronize, closed] + # Only run for PRs raised from this repository (adjust default branch) + branches: [main]
42-45: Service-principal IDs are hard-coded – move to GitHub secrets.
client-id,tenant-id, andsubscription-idmust be injected from${{ secrets.* }}to avoid credential leakage and ease rotation.Same issue exists in both deploy and cleanup jobs.
Also applies to: 129-131
81-85: INFISICAL credentials committed in plain text – immediate secret leak.
BothINFISICAL_CLIENT_IDandINFISICAL_CLIENT_SECRETare public in the repo. Anyone can impersonate the machine identity and exfiltrate all secrets.Move them to Container-App
--secretsand reference withsecretref::- INFISICAL_CLIENT_ID=cd3cc75e-d3bc-4b42-ba5d-6b68f56afa78 \ - INFISICAL_CLIENT_SECRET=b8791a2f3571a3a24c8d938c627f31490427b7466ef7e335782bd1023d145b9a + --secrets \ + infisicalClientId=${{ secrets.INFISICAL_CLIENT_ID }} \ + infisicalClientSecret=${{ secrets.INFISICAL_CLIENT_SECRET }} \ + --env-vars \ + INFISICAL_CLIENT_ID=secretref:infisicalClientId \ + INFISICAL_CLIENT_SECRET=secretref:infisicalClientSecretRotate the leaked credentials immediately.
62-67:az group createis not idempotent – add existence check.
Re-runs fail if the RG already exists (e.g., previous run cancelled). Guard the command:-az group create \ - --name ${{ env.AZURE_RESOURCE_GROUP }} \ - --location ${{ env.AZURE_LOCATION }} +if ! az group exists --name ${{ env.AZURE_RESOURCE_GROUP }}; then + az group create \ + --name ${{ env.AZURE_RESOURCE_GROUP }} \ + --location ${{ env.AZURE_LOCATION }} +fi
🧹 Nitpick comments (1)
.github/workflows/azure-pr-deploy.yml (1)
96-121: Trailing spaces & missing EOF newline flagged by YAMLlint.
Minor but CI may fail. Strip trailing blanks and ensure the file ends with\n.Also applies to: 159-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/azure-pr-deploy.yml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
.github/workflows/azure-pr-deploy.yml
84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 106-106: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 115-115: trailing spaces
(trailing-spaces)
[error] 119-119: trailing spaces
(trailing-spaces)
[error] 160-160: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 165-165: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/azure-pr-deploy.yml (1)
3-6: Restrict PR trigger to trusted sources (repeat finding)
Workflow still runs for everypull_requestevent, so code from forks can execute privileged Azure commands. Please gate the workflow (e.g.branches: [main]or a protectedenvironment) before merging.
🧹 Nitpick comments (1)
.github/workflows/azure-pr-deploy.yml (1)
27-27: Clean up trailing whitespace and missing EOF newline
YAML-lint flags these lines; trimming whitespace avoids noisy CI failures.- +Add a single blank line at the very end of the file as well.
Also applies to: 105-105, 109-109, 113-113, 118-118, 122-122, 163-163, 167-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/azure-pr-deploy.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 105-105: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 113-113: trailing spaces
(trailing-spaces)
[error] 118-118: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 163-163: trailing spaces
(trailing-spaces)
[error] 167-167: trailing spaces
(trailing-spaces)
[error] 168-168: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
.github/workflows/azure-pr-deploy.yml (2)
40-45: Service-principal IDs now pulled from secrets – nice hardening
Hard-coding was removed in favour ofsecrets.*; this closes the credential-leak issue raised earlier.
78-88: Verify whetherINFISICAL_PROJECT_IDshould also be secret-scoped
INFISICAL_PROJECT_IDlooks like an internal identifier. If exposing it publicly has no downside, leave as-is. Otherwise inject through--secretsjust like the client credentials.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/azure-pr-deploy.yml (1)
3-6: Unrestricted PR trigger still exposes Azure subscription to forked PRs
Previous feedback to restrict the workflow trigger (or gate it behind an approved environment) has not been incorporated. Malicious code in a fork can still obtain a federated OIDC token and run arbitraryazcommands with contributor-level privileges.
🧹 Nitpick comments (2)
.github/workflows/azure-pr-deploy.yml (2)
13-16: Separate ACR name from registry FQDN for consistency & DRYness
You hard-codedotincin theaz acr loginstep (l.49-50) whileAZURE_CONTAINER_REGISTRYcontains the FQDNdotinc.azurecr.io. Relying on two different literals is brittle and diverges if the registry name ever changes.env: - AZURE_CONTAINER_REGISTRY: dotinc.azurecr.io + AZURE_ACR_NAME: dotinc + AZURE_CONTAINER_REGISTRY: ${{ env.AZURE_ACR_NAME }}.azurecr.ioand later:
- - name: Log in to Azure Container Registry - run: az acr login --name dotinc + - name: Log in to Azure Container Registry + run: az acr login --name ${{ env.AZURE_ACR_NAME }}
27-27: YAML-lint: trailing whitespace & missing final newline
CI linting already flags these lines. Strip the trailing spaces and ensure the file ends with a single newline to keep the workflow green.- api-url: ${{ steps.get-api-url.outputs.url }}␠ + api-url: ${{ steps.get-api-url.outputs.url }} … - ## 🚀 Brease PR Environment Deployed Successfully!␠ + ## 🚀 Brease PR Environment Deployed Successfully! … - > 💡 This environment will be automatically cleaned up when the PR is merged or closed.␠ + > 💡 This environment will be automatically cleaned up when the PR is merged or closed. … - > ✅ Cleanup completed successfully.␠ + > ✅ Cleanup completed successfully. +Also applies to: 117-117, 121-121, 125-125, 130-130, 134-134, 175-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/azure-pr-deploy.yml(1 hunks)apps/api/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/Dockerfile
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 117-117: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 134-134: trailing spaces
(trailing-spaces)
[error] 175-175: trailing spaces
(trailing-spaces)
[error] 179-179: trailing spaces
(trailing-spaces)
[error] 180-180: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy
|
|
||
| - name: Get API URL | ||
| id: get-api-url | ||
| run: | | ||
| URL=$(az containerapp show \ | ||
| --name ${{ env.API_APP_NAME }} \ | ||
| --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ | ||
| --query properties.configuration.ingress.fqdn \ | ||
| --output tsv) | ||
| echo "url=https://$URL" >> $GITHUB_OUTPUT | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
az containerapp show can race with provisioning – add retry
Immediately querying the FQDN sometimes returns an empty string while DNS & ingress settle. A short retry loop (or az rest --retry-interval) prevents posting an empty URL to the PR comment.
- URL=$(az containerapp show \
+ for i in {1..10}; do
+ URL=$(az containerapp show \
--name ${{ env.API_APP_NAME }} \
--resource-group ${{ env.AZURE_RESOURCE_GROUP }} \
--query properties.configuration.ingress.fqdn \
--output tsv)
+ [ -n "$URL" ] && break
+ sleep 15
+ done
echo "url=https://$URL" >> $GITHUB_OUTPUT📝 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: Get API URL | |
| id: get-api-url | |
| run: | | |
| URL=$(az containerapp show \ | |
| --name ${{ env.API_APP_NAME }} \ | |
| --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ | |
| --query properties.configuration.ingress.fqdn \ | |
| --output tsv) | |
| echo "url=https://$URL" >> $GITHUB_OUTPUT | |
| - name: Get API URL | |
| id: get-api-url | |
| run: | | |
| for i in {1..10}; do | |
| URL=$(az containerapp show \ | |
| --name ${{ env.API_APP_NAME }} \ | |
| --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ | |
| --query properties.configuration.ingress.fqdn \ | |
| --output tsv) | |
| [ -n "$URL" ] && break | |
| sleep 15 | |
| done | |
| echo "url=https://$URL" >> $GITHUB_OUTPUT |
🤖 Prompt for AI Agents
In .github/workflows/azure-pr-deploy.yml around lines 100 to 110, the az
containerapp show command can return an empty FQDN due to provisioning delays.
To fix this, wrap the command in a retry loop that attempts the query multiple
times with a short delay between tries, ensuring the URL is not empty before
echoing it to GITHUB_OUTPUT. This prevents posting an empty URL to the PR
comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/azure-pr-deploy.yml (3)
3-6: Unrestricted PR trigger still exposes Azure resources to forked PRs
The workflow continues to run for everypull_requestevent, giving untrusted fork code write access to the subscription. Restrict the trigger to trusted actors or gate the deploy job behind a protected environment.-on: - pull_request: - types: [opened, reopened, synchronize, closed] +on: + pull_request: + types: [opened, reopened, synchronize, closed] + # Ensure only in-repo PRs can deploy + branches: [main] # adjust to your default branch
74-79:az group createis not idempotent – reruns will fail
On re-runs the command errors if the group already exists. Add an existence check (or--exists) to make the step idempotent.-az group create \ - --name ${{ env.AZURE_RESOURCE_GROUP }} \ - --location ${{ env.AZURE_LOCATION }} +if ! az group exists --name ${{ env.AZURE_RESOURCE_GROUP }}; then + az group create \ + --name ${{ env.AZURE_RESOURCE_GROUP }} \ + --location ${{ env.AZURE_LOCATION }} +fi
90-97: Secrets still injected as plain environment variables
INFISICAL_CLIENT_IDandINFISICAL_CLIENT_SECRETremain readable viaaz containerapp revision show. Move them to Container-App secrets and reference withsecretref:.- --env-vars \ - ... \ - INFISICAL_CLIENT_ID=${{ secrets.INFISICAL_CLIENT_ID }} \ - INFISICAL_CLIENT_SECRET=${{ secrets.INFISICAL_CLIENT_SECRET }} \ + --secrets \ + infisicalClientId=${{ secrets.INFISICAL_CLIENT_ID }} \ + infisicalClientSecret=${{ secrets.INFISICAL_CLIENT_SECRET }} \ + --env-vars \ + ... \ + INFISICAL_CLIENT_ID=secretref:infisicalClientId \ + INFISICAL_CLIENT_SECRET=secretref:infisicalClientSecret \
🧹 Nitpick comments (2)
.github/workflows/azure-pr-deploy.yml (2)
100-106: Single shotaz containerapp showraces with provisioning – add retry
A fresh deployment often returns an empty FQDN. Wrap the query in a small retry loop to avoid posting blank URLs.-URL=$(az containerapp show \ - --name ${{ env.API_APP_NAME }} \ - --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ - --query properties.configuration.ingress.fqdn \ - --output tsv) +for i in {1..10}; do + URL=$(az containerapp show \ + --name ${{ env.API_APP_NAME }} \ + --resource-group ${{ env.AZURE_RESOURCE_GROUP }} \ + --query properties.configuration.ingress.fqdn \ + --output tsv) + [ -n "$URL" ] && break + sleep 15 +done
27-131: Minor style: remove trailing whitespace & add final newline
YAMLlint flags multiple trailing-space errors (lines 27, 114, 118, 122, 127, 131, 172, 176) and a missing newline at EOF. Trimming these keeps the workflow tidy and prevents CI lint failures.- api-url: ${{ steps.get-api-url.outputs.url }}␠ + api-url: ${{ steps.get-api-url.outputs.url }} … - ## 🚀 Brease PR Environment Deployed Successfully!␠ + ## 🚀 Brease PR Environment Deployed Successfully! … - > 💡 This environment will be automatically cleaned up when the PR is merged or closed.␠ + > 💡 This environment will be automatically cleaned up when the PR is merged or closed. +Also applies to: 172-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/azure-pr-deploy.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/azure-pr-deploy.yml
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 118-118: trailing spaces
(trailing-spaces)
[error] 122-122: trailing spaces
(trailing-spaces)
[error] 127-127: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
[error] 172-172: trailing spaces
(trailing-spaces)
[error] 176-176: trailing spaces
(trailing-spaces)
[error] 177-177: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (go)
2 participants
We'd love to see more contributions
Read how you can contribute to the project.
Summary by CodeRabbit
New Features
Chores