chore: deploy API+infra to Oracle Cloud via Terraform, web app to CloudFlare#27
Conversation
…dflare Replaces the SSH-only deploy job (misleadingly named "Deploy to Cloudflare / Remote Server" but never actually touching Cloudflare) with three jobs: - terraform: provisions OCI networking + a VM.Standard.A1.Flex compute instance via Terraform (infra/oci/), with retry-with-backoff on OCI's intermittent "out of host capacity" errors, and cloud-init that installs Docker and fixes Oracle's default iptables rules blocking inbound traffic. - deploy-api: SSHes into the Terraform-provisioned instance and runs docker-compose (base + new docker-compose.prod.yml overlay) to bring up the API, PostGIS, Redis, and OSRM together. - deploy-web: builds the Next.js app for Cloudflare via the OpenNext adapter and deploys it to Workers via wrangler. Also fixes the deploy trigger to include `main` (previously only release/*/release--* branches), adds a Dockerfile for the FastAPI API, and renames apps/web/src/proxy.ts back to middleware.ts with an explicit experimental-edge runtime -- Next.js 16's proxy.ts convention only supports the Node.js runtime, which OpenNext's Cloudflare adapter can't run.
📝 WalkthroughWalkthroughThis PR adds OCI Terraform provisioning, Vault-backed secret distribution, a production API container/compose setup, shared-secret API authentication, Cloudflare/OpenNext web deployment config, and CI jobs that provision and deploy both services. ChangesOCI infrastructure, secrets, and deployment
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions
participant Terraform as Terraform
participant OCIVault as OCI Vault
participant OCIVM as OCI VM
participant DockerCompose as Docker Compose
participant Cloudflare as Cloudflare Worker
GitHubActions->>Terraform: init / plan / apply
Terraform->>OCIVault: create DB secret
Terraform->>OCIVM: provision instance
GitHubActions->>OCIVM: SSH deploy API
OCIVM->>OCIVault: fetch secret bundle
OCIVM->>DockerCompose: rebuild and restart services
GitHubActions->>Cloudflare: build and deploy web app
GitHubActions->>Cloudflare: set API_SHARED_SECRET secret
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.69.3)Trivy execution failed: 2026-07-07T08:08:03Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: cloudformation scan error: fs filter error: fs filter error: walk error open smartylint.json: no such file or directory: open smartylint.json: no such file or directory 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (6)
.github/workflows/push.yml (3)
75-127: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd explicit least-privilege
permissions:to the new jobs.Both jobs rely on default
GITHUB_TOKENpermissions (broader than needed) per static analysis. Since they checkout code and invoke third-party actions, scope this down explicitly.🔒 Proposed fix
terraform: name: Provision OCI Infrastructure needs: [test] + permissions: + contents: read if: |(apply similarly to
deploy-api)Also applies to: 128-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push.yml around lines 75 - 127, The new GitHub Actions jobs are relying on the default GITHUB_TOKEN scope instead of explicit least-privilege access. Add a top-level permissions block to the terraform job, and apply the same restriction to deploy-api, using only the minimal scopes needed for checkout and the actions used in this workflow. Keep the change localized to the job definitions so the permissions are clear and consistent.Source: Linters/SAST tools
216-217: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the globally installed
pnpmversion.
npm install -g pnpminstalls an ad-hoc, unpinned package outside any lockfile, flagged by static analysis. Pin a version (or usepnpm/action-setup) for reproducible, supply-chain-safe builds.🔧 Proposed fix
- - name: Install pnpm - run: npm install -g pnpm + - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: 9🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push.yml around lines 216 - 217, The workflow step that installs pnpm is using an unpinned global npm install, which should be replaced with a versioned setup for reproducible builds. Update the Install pnpm step in the workflow to either install a specific pnpm version or switch to pnpm/action-setup, and keep the change localized to the pnpm install step so the rest of the workflow continues to use the same pnpm invocation.Source: Linters/SAST tools
177-186: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFixed 15s sleep before health check may be flaky.
The compose healthcheck (
docker-compose.prod.yml) allows up tostart_period(20s) + interval(15s)×retries(5)≈ 95s before the container is considered healthy, but this step only waits a flat 15s with a single attempt before failing the deploy.♻️ Proposed fix
- sleep 15 - ssh -o StrictHostKeyChecking=yes "ubuntu@$SERVER_HOST" \ - 'STATUS=$(curl -sf http://localhost:8000/health | python3 -c "import sys,json; print(json.load(sys.stdin)[\"status\"])"); \ - if [ "$STATUS" = "ok" ]; then echo "API health check passed"; else echo "API health check failed (status: $STATUS)"; exit 1; fi' + for i in $(seq 1 10); do + if ssh -o StrictHostKeyChecking=yes "ubuntu@$SERVER_HOST" \ + 'STATUS=$(curl -sf http://localhost:8000/health | python3 -c "import sys,json; print(json.load(sys.stdin)[\"status\"])" 2>/dev/null); [ "$STATUS" = "ok" ]'; then + echo "API health check passed"; exit 0 + fi + echo "Health check attempt $i/10 failed, retrying in 10s..." + sleep 10 + done + echo "API health check failed after retries"; exit 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push.yml around lines 177 - 186, The health verification in the deployment workflow is too eager because the fixed sleep and single curl attempt can fail before the container is actually healthy. Update the Verify deployment health step to wait and retry until the service reports ok, aligning it with the docker-compose.prod.yml healthcheck timing, and keep the logic in the existing ssh/bash block that checks STATUS from /health.apps/api/Dockerfile (2)
10-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the
uvimage tag instead of:latest.Floating tags are mutable and can silently change the toolchain used to build the image between runs, hurting reproducibility and supply-chain integrity.
🔒 Proposed fix
-COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ +COPY --from=ghcr.io/astral-sh/uv:0.9 /uv /uvx /bin/Please confirm the current stable
uvrelease tag to pin to.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/Dockerfile` at line 10, The Dockerfile is using a mutable `ghcr.io/astral-sh/uv:latest` base for the `COPY --from` step, so replace it with a fixed `uv` release tag to keep builds reproducible and stable. Update the `COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/` reference to a конкретe pinned version tag chosen from the current stable `uv` release, and keep the `uv` source image aligned with that exact release going forward.
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCopied app files remain owned by root under the non-root user.
COPY --from=builder /app /appruns beforeUSER appuser(Line 30), so files stay root-owned.appusercan read/execute but not write inside/app(e.g.__pycache__bytecode caching, any future write path).♻️ Proposed fix
-COPY --from=builder /app /app +COPY --from=builder --chown=appuser:appuser /app /app🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/Dockerfile` around lines 26 - 30, The copied application files are still owned by root because the Dockerfile’s COPY --from=builder /app /app runs before USER appuser. Update the Dockerfile so /app is owned by appuser, using COPY with ownership settings or a chown step immediately after copying, and keep USER appuser as the final runtime user. Refer to the COPY --from=builder and USER appuser instructions when making the change..env.example (1)
19-25: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
NEXT_PUBLIC_prefix contradicts the comment's "server-only" premise.The comment argues there's no mixed-content/leak risk because the value is server-side only, but Next.js inlines any
NEXT_PUBLIC_*variable into the client bundle wherever it's referenced — the prefix itself is what would leak the OCI VM's public IP if any future client component reads it. Since usage today is server-only, renaming to a non-NEXT_PUBLIC_variable (e.g.API_URL) would remove this latent risk and better match the intent described in the comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example around lines 19 - 25, The environment variable name in the .env.example entry conflicts with the server-only usage described in the comment: NEXT_PUBLIC_API_URL would be exposed to client bundles if referenced there. Rename this setting to a non-public variable such as API_URL, and update any corresponding reads in the Next.js Route Handlers or other server-side config to use the new name so the intent of server-only access is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/push.yml:
- Around line 112-118: The Terraform plan produced by the Terraform Plan step is
not being used by the apply step, so the workflow recomputes a different plan
instead of applying the saved one. Update the Terraform Apply (retries on OCI
capacity errors) flow and the apply-with-retry.sh script so it accepts the
tfplan file path and passes it through to terraform apply, ensuring the exact
plan generated by Terraform Plan is what gets applied.
- Around line 147-175: The deployment step in the Sync repo and restart services
block always checks out and pulls main, so it ignores the branch that triggered
the workflow. Update the remote git checkout/pull logic in this job to use the
triggering branch name from github.ref_name instead of hardcoding main, and keep
the existing app sync and docker compose restart flow intact.
In `@apps/web/src/middleware.ts`:
- Around line 33-36: The middleware runtime is using the legacy experimental
value, which can fail Next.js 16 validation. Update the middleware configuration
in middleware.ts to use the supported edge runtime instead of the experimental
one, keeping the rest of the next-intl middleware setup unchanged.
In `@docker-compose.prod.yml`:
- Around line 13-14: The API is being exposed directly on port 8000 without TLS
protection. Update the production Compose setup around the ports mapping in
docker-compose.prod.yml and the network ingress rule in infra/oci/network.tf so
that external access goes through TLS termination or a private ingress path, and
only allow port 8000 from trusted/private sources if it must remain open.
In `@infra/oci/backend.tf`:
- Around line 8-20: The S3 backend in terraform configuration still contains the
OCI namespace placeholder in the endpoint, which will break initialization in
CI. Update the backend setup in backend.tf so the endpoint used by the terraform
block is a real OCI Object Storage host, either by replacing the placeholder
with the correct namespace value or by ensuring it is supplied through the same
backend-config path used for the access and secret keys.
In `@infra/oci/cloud-init.yaml.tpl`:
- Line 23: The cloud-init template is using ${VERSION_CODENAME} in a context
that Terraform template rendering interprets as interpolation, so update the
Docker apt source line to escape that shell variable with a literal Terraform
escape in the templatefile content. Fix the cloud-init template expression
around the docker repository echo so the shell still receives VERSION_CODENAME
at runtime, and verify any similar ${...} occurrences in this template are
escaped consistently.
In `@infra/oci/compute.tf`:
- Around line 16-49: The oci_core_instance app resource still allows the legacy
instance metadata endpoint, so update its instance metadata settings to enforce
IMDSv2-only access and disable IMDSv1. Make this change in the oci_core_instance
"app" definition by adding the appropriate metadata/instance options block
alongside the existing metadata and source_details settings, and ensure the
configuration explicitly requires token-based metadata requests.
In `@infra/oci/network.tf`:
- Around line 69-76: The ingress rule in the OCI network configuration is
exposing the FastAPI backend on port 8000 to the public internet without
authentication. Update the network setup around the ingress_security_rules block
so the service is only reachable from a private proxy or restricted source
range, or add app-layer authentication if it must remain reachable; use the
existing network.tf ingress rule and FastAPI backend configuration to locate the
change.
In `@infra/oci/scripts/apply-with-retry.sh`:
- Line 9: The script’s initial directory change in apply-with-retry.sh can fail
silently because set -e is not enabled, causing later terraform commands to run
from the wrong location. Update the cd step near the top of the script to
explicitly check for failure and exit with a clear error before any retry or
terraform logic runs, using the existing script entrypoint and BASH_SOURCE-based
directory resolution as the location to fix.
- Around line 11-12: The backoff timing comment next to MAX_ATTEMPTS and
BASE_DELAY is incorrect because apply_with_retry only sleeps after attempts 1-5
and exits immediately on the final attempt, so the “360” delay and "~21.5 min
worst case" are wrong. Update the inline comment to reflect the actual retry
delays and total worst-case sleep time, and make the matching wording consistent
in the README entry that mentions the worst case. Use the apply_with_retry loop
and the MAX_ATTEMPTS/BASE_DELAY symbols to keep the comment aligned with the
implemented behavior.
In `@infra/oci/variables.tf`:
- Around line 56-60: The ssh_ingress_cidr variable currently defaults to a
public CIDR, which makes the deployed SSH ingress overly permissive when CI does
not override it. Update the variable definition in ssh_ingress_cidr so it is not
open by default: either remove the default and require an explicit
per-environment value, or change the default to a narrowly trusted admin range.
Make sure any references that consume ssh_ingress_cidr continue to use the
explicit value provided by each environment.
---
Nitpick comments:
In @.env.example:
- Around line 19-25: The environment variable name in the .env.example entry
conflicts with the server-only usage described in the comment:
NEXT_PUBLIC_API_URL would be exposed to client bundles if referenced there.
Rename this setting to a non-public variable such as API_URL, and update any
corresponding reads in the Next.js Route Handlers or other server-side config to
use the new name so the intent of server-only access is preserved.
In @.github/workflows/push.yml:
- Around line 75-127: The new GitHub Actions jobs are relying on the default
GITHUB_TOKEN scope instead of explicit least-privilege access. Add a top-level
permissions block to the terraform job, and apply the same restriction to
deploy-api, using only the minimal scopes needed for checkout and the actions
used in this workflow. Keep the change localized to the job definitions so the
permissions are clear and consistent.
- Around line 216-217: The workflow step that installs pnpm is using an unpinned
global npm install, which should be replaced with a versioned setup for
reproducible builds. Update the Install pnpm step in the workflow to either
install a specific pnpm version or switch to pnpm/action-setup, and keep the
change localized to the pnpm install step so the rest of the workflow continues
to use the same pnpm invocation.
- Around line 177-186: The health verification in the deployment workflow is too
eager because the fixed sleep and single curl attempt can fail before the
container is actually healthy. Update the Verify deployment health step to wait
and retry until the service reports ok, aligning it with the
docker-compose.prod.yml healthcheck timing, and keep the logic in the existing
ssh/bash block that checks STATUS from /health.
In `@apps/api/Dockerfile`:
- Line 10: The Dockerfile is using a mutable `ghcr.io/astral-sh/uv:latest` base
for the `COPY --from` step, so replace it with a fixed `uv` release tag to keep
builds reproducible and stable. Update the `COPY
--from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/` reference to a конкретe
pinned version tag chosen from the current stable `uv` release, and keep the
`uv` source image aligned with that exact release going forward.
- Around line 26-30: The copied application files are still owned by root
because the Dockerfile’s COPY --from=builder /app /app runs before USER appuser.
Update the Dockerfile so /app is owned by appuser, using COPY with ownership
settings or a chown step immediately after copying, and keep USER appuser as the
final runtime user. Refer to the COPY --from=builder and USER appuser
instructions when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb3f9e73-5bc3-471e-ae48-e13c42a3a534
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.env.example.github/workflows/push.yml.gitignoreapps/api/.dockerignoreapps/api/Dockerfileapps/api/README.mdapps/web/eslint.config.mjsapps/web/open-next.config.tsapps/web/package.jsonapps/web/src/middleware.tsapps/web/wrangler.jsoncdocker-compose.prod.ymlinfra/oci/README.mdinfra/oci/backend.tfinfra/oci/cloud-init.yaml.tplinfra/oci/compute.tfinfra/oci/network.tfinfra/oci/outputs.tfinfra/oci/providers.tfinfra/oci/scripts/apply-with-retry.shinfra/oci/terraform.tfvars.exampleinfra/oci/variables.tf
| - name: Terraform Plan | ||
| working-directory: infra/oci | ||
| run: terraform plan -input=false -out=tfplan | ||
|
|
||
| - name: Terraform Apply (retries on OCI capacity errors) | ||
| working-directory: infra/oci | ||
| run: ./scripts/apply-with-retry.sh |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Generated Terraform plan is never applied.
terraform plan -out=tfplan (Line 114) produces a plan artifact, but apply-with-retry.sh runs terraform apply with no plan file argument, so it silently recomputes and applies its own plan instead of the one just generated. This defeats the safety guarantee that "what gets applied is exactly what was planned," and wastes the plan step.
🔧 Proposed fix
- name: Terraform Apply (retries on OCI capacity errors)
working-directory: infra/oci
- run: ./scripts/apply-with-retry.sh
+ run: ./scripts/apply-with-retry.sh tfplan(apply-with-retry.sh would need to accept and pass through the plan-file argument to terraform apply.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/push.yml around lines 112 - 118, The Terraform plan
produced by the Terraform Plan step is not being used by the apply step, so the
workflow recomputes a different plan instead of applying the saved one. Update
the Terraform Apply (retries on OCI capacity errors) flow and the
apply-with-retry.sh script so it accepts the tfplan file path and passes it
through to terraform apply, ensuring the exact plan generated by Terraform Plan
is what gets applied.
| - name: Sync repo and restart services | ||
| shell: bash | ||
| env: | ||
| SERVER_HOST: ${{ needs.terraform.outputs.instance_ip }} | ||
| APP_DIR: ${{ secrets.DEPLOY_APP_DIR }} | ||
| run: | | ||
| ssh -o StrictHostKeyChecking=yes "ubuntu@$SERVER_HOST" bash -s <<REMOTE | ||
| set -euo pipefail | ||
|
|
||
| echo "==> Ensuring app directory exists" | ||
| if [ ! -d "$APP_DIR/.git" ]; then | ||
| git clone git@github.com:sprajeesh/location-intelligence.git "$APP_DIR" | ||
| fi | ||
| cd "$APP_DIR" | ||
|
|
||
| echo "==> Pulling latest commits" | ||
| git fetch --all --prune | ||
| git checkout main | ||
| git pull origin main | ||
|
|
||
| echo "==> Rebuilding and restarting all services" | ||
| docker compose -f docker-compose.yml -f docker-compose.prod.yml pull --quiet || true | ||
| docker compose -f docker-compose.yml -f docker-compose.prod.yml up -d --build --remove-orphans | ||
|
|
||
| echo "==> Services status" | ||
| docker compose -f docker-compose.yml -f docker-compose.prod.yml ps | ||
|
|
||
| echo "==> Deploy complete" | ||
| REMOTE |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the workflow trigger and the deploy step in context.
git ls-files .github/workflows/push.yml
wc -l .github/workflows/push.yml
sed -n '1,220p' .github/workflows/push.ymlRepository: sprajeesh/location-intelligence
Length of output: 7857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for related branch/ref handling and repository/SSH deployment setup.
rg -n --hidden --glob '.github/workflows/*.yml' --glob '.github/**' \
'github\.ref_name|github\.ref|release/\*|release--\*|DEPLOY_SSH_KEY|DEPLOY_APP_DIR|git@github\.com:sprajeesh/location-intelligence\.git|github\.repository|git checkout main|git pull origin main|git fetch --all --prune' .Repository: sprajeesh/location-intelligence
Length of output: 1233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect OCI provisioning and VM bootstrap for GitHub SSH access / repo checkout setup.
fd -a 'cloud-init|user-data|startup|init|terraform|oci' infra .
rg -n --hidden --glob 'infra/**' --glob '.github/**' \
'git@github\.com|location-intelligence\.git|DEPLOY_SSH_PUBLIC_KEY|authorized_keys|cloud-init|user_data|user-data|git clone|git pull|ssh-keyscan|known_hosts|github\.repository' .Repository: sprajeesh/location-intelligence
Length of output: 1932
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l infra/oci/cloud-init.yaml.tpl infra/oci/README.md infra/oci/variables.tf infra/oci/compute.tf
sed -n '1,220p' infra/oci/cloud-init.yaml.tpl
printf '\n--- README ---\n'
sed -n '1,220p' infra/oci/README.md
printf '\n--- variables.tf ---\n'
sed -n '1,180p' infra/oci/variables.tf
printf '\n--- compute.tf ---\n'
sed -n '1,120p' infra/oci/compute.tfRepository: sprajeesh/location-intelligence
Length of output: 9574
Use the triggering branch for the remote checkout git checkout main / git pull origin main will always deploy main, so pushes to release/* still roll out the wrong code. Use ${{ github.ref_name }} instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/push.yml around lines 147 - 175, The deployment step in
the Sync repo and restart services block always checks out and pulls main, so it
ignores the branch that triggered the workflow. Update the remote git
checkout/pull logic in this job to use the triggering branch name from
github.ref_name instead of hardcoding main, and keep the existing app sync and
docker compose restart flow intact.
| // Next.js 16 defaults middleware to the Node.js runtime, but the | ||
| // OpenNext Cloudflare adapter only supports Edge middleware. next-intl's | ||
| // middleware has no Node.js-only dependencies, so Edge is safe here. | ||
| runtime: "experimental-edge", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does @opennextjs/cloudflare support Edge runtime middleware, or does it require Node.js runtime, for Next.js 16?
💡 Result:
@opennextjs/cloudflare supports Edge middleware [1][2]. Historically, the adapter has required developers to remove the export const runtime = "edge" configuration from their files because it was designed to run your Next.js application within the Cloudflare Workers runtime (which is already edge-native) rather than the standard Next.js Edge runtime [3][4]. Regarding Next.js 16 and the transition from middleware.ts to proxy.ts: 1. Edge Middleware: @opennextjs/cloudflare supports Edge middleware [1][2]. 2. Proxy (Node.js) Middleware: While previous versions of @opennextjs/cloudflare blocked Node.js-based middleware (proxy.ts) and required it to be converted to Edge middleware, recent updates (including Pull Request #1280) have introduced support for handling proxy.ts/Node.js middleware by bundling it to run in the worker [5]. In summary, you should generally avoid forcing the Next.js "Edge" runtime via configuration, as the adapter is intended to manage the execution environment for you [3][4]. The adapter natively supports Edge middleware, and with the latest updates, it now also supports the Next.js 16 proxy file convention [5].
Citations:
- 1: Improve Next16 support opennextjs/opennextjs-cloudflare#1082
- 2: Next16 support opennextjs/opennextjs-cloudflare#972
- 3: https://opennext.js.org/cloudflare/get-started
- 4: [BUG] Edge runtime causes silent 500 errors instead of build/runtime warning opennextjs/opennextjs-cloudflare#1028
- 5: feat: support Next.js 16 proxy.ts (Node.js middleware) opennextjs/opennextjs-cloudflare#1280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby package versions.
sed -n '1,120p' apps/web/src/middleware.ts
printf '\n--- package.json refs ---\n'
rg -n '"next"|`@opennextjs/cloudflare`|next-intl' apps/web/package.json package.json pnpm-workspace.yaml turbo.json --no-heading || true
# Search for any other runtime overrides or middleware/proxy conventions.
printf '\n--- repo references ---\n'
rg -n 'experimental-edge|runtime:\s*"edge"|runtime:\s*"nodejs"|proxy\.ts|middleware\.ts|export const proxy|export const middleware' apps/web -g '!**/node_modules/**' --no-heading || trueRepository: sprajeesh/location-intelligence
Length of output: 1824
🌐 Web query:
Next.js 16 middleware.ts config runtime experimental-edge supported values and @opennextjs/cloudflare middleware runtime docs
💡 Result:
As of July 2026, the experimental-edge runtime configuration is deprecated in Next.js [1]. The standard runtime for middleware is the Node.js runtime, which has become the default for the newer proxy.ts architecture introduced in Next.js 16 [2][3]. Middleware Configuration and Runtime In Next.js 16, the traditional middleware.ts is maintained for legacy Edge runtime use cases but is deprecated in favor of proxy.ts [3]. For files that still support the runtime configuration (primarily legacy middleware), the supported values are nodejs and edge [1][4]. The experimental-edge value is no longer experimental and has been superseded by edge [1]. Note that proxy.ts files default to the Node.js runtime, and the runtime configuration option is generally not supported or required in these files [5][2]. @opennextjs/cloudflare Middleware Support The @opennextjs/cloudflare adapter has specific constraints regarding these runtimes: - Edge Middleware: Officially supported [6][7]. - Node.js Middleware (proxy.ts / legacy middleware.ts with runtime: 'nodejs'): Historically, this was not supported by the adapter, which would throw an error during build [6][7]. However, recent community-driven development has introduced experimental support for bundling and running a subset of Node.js-compatible middleware logic on Cloudflare Workers [8][9]. This support is specifically intended for logic that does not rely on Node.js native APIs (e.g., filesystem access, subprocesses) [9]. Users should check the latest release notes for @opennextjs/cloudflare, as support for the full proxy.ts architecture remains an evolving area [8][7].
Citations:
- 1: vercel/next.js@84f1e44
- 2: Docs: Contradictory statements about Proxy/Middleware default runtime in proxy.md vercel/next.js#93463
- 3: https://nextjs.org/blog/next-16
- 4: https://vercel.com/docs/routing-middleware/api
- 5: https://nextjs.org/docs/app/api-reference/file-conventions/proxy
- 6: Next16 support opennextjs/opennextjs-cloudflare#972
- 7: Improve Next16 support opennextjs/opennextjs-cloudflare#1082
- 8: feat: support Next.js 16 proxy.ts (Node.js middleware) opennextjs/opennextjs-cloudflare#1280
- 9: feat: support Workers-compatible Node middleware opennextjs/opennextjs-cloudflare#1275
🌐 Web query:
Next.js 16 middleware runtime valid values edge nodejs experimental-edge middleware.ts docs
💡 Result:
In Next.js 16, the middleware.ts file convention has been deprecated and renamed to proxy.ts [1]. Regarding runtime configuration: * The proxy.ts (formerly middleware.ts) file defaults to the Node.js runtime [1][2]. * The runtime configuration option is no longer supported in proxy.ts [2]. Setting export const config = { runtime: '...' } in a proxy.ts file will throw an error [2]. * The Edge runtime is not supported in proxy.ts [1]. If you require the Edge runtime, you must continue using the middleware.ts filename [1]. Next.js 16 retains support for middleware.ts to allow users to maintain Edge-based middleware, while the new proxy.ts convention is intended for Node.js-based request handling [1]. Historical references to experimental-edge for middleware are largely superseded by these version 16 changes [1][3].
Citations:
- 1: https://github.com/vercel/next.js/blob/v16.2.1/docs/01-app/02-guides/upgrading/version-16.mdx
- 2: Docs: Contradictory statements about Proxy/Middleware default runtime in proxy.md vercel/next.js#93463
- 3: vercel/next.js@v14.2.32...v16.0.7
Use runtime: "edge" here
experimental-edge is the legacy value; Next.js 16 expects edge for middleware.ts, and this can fail runtime validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/middleware.ts` around lines 33 - 36, The middleware runtime is
using the legacy experimental value, which can fail Next.js 16 validation.
Update the middleware configuration in middleware.ts to use the supported edge
runtime instead of the experimental one, keeping the rest of the next-intl
middleware setup unchanged.
| resource "oci_core_instance" "app" { | ||
| compartment_id = var.compartment_ocid | ||
| availability_domain = data.oci_identity_availability_domains.ads.availability_domains[0].name | ||
| display_name = "li-demo-app" | ||
| shape = var.instance_shape | ||
|
|
||
| shape_config { | ||
| ocpus = var.instance_ocpus | ||
| memory_in_gbs = var.instance_memory_gbs | ||
| } | ||
|
|
||
| create_vnic_details { | ||
| subnet_id = oci_core_subnet.public.id | ||
| assign_public_ip = true | ||
| } | ||
|
|
||
| source_details { | ||
| source_type = "image" | ||
| source_id = data.oci_core_images.ubuntu_arm.images[0].id | ||
| } | ||
|
|
||
| metadata = { | ||
| ssh_authorized_keys = var.ssh_public_key | ||
| user_data = base64encode(templatefile("${path.module}/cloud-init.yaml.tpl", {})) | ||
| } | ||
|
|
||
| # Avoid destroy/recreate churn if Canonical publishes a newer image after | ||
| # initial provisioning -- we don't want every terraform apply to replace | ||
| # a running instance (and its manually-uploaded OSRM data) just because | ||
| # a newer base image exists. | ||
| lifecycle { | ||
| ignore_changes = [source_details] | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Disable the legacy instance metadata (IMDSv1) endpoint.
The instance leaves the legacy metadata endpoint enabled, which is reachable without token auth and broadens the SSRF/credential-exposure surface. Enforce IMDSv2 only.
🛡️ Proposed hardening
shape = var.instance_shape
+
+ instance_options {
+ are_legacy_imds_endpoints_disabled = true
+ }📝 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.
| resource "oci_core_instance" "app" { | |
| compartment_id = var.compartment_ocid | |
| availability_domain = data.oci_identity_availability_domains.ads.availability_domains[0].name | |
| display_name = "li-demo-app" | |
| shape = var.instance_shape | |
| shape_config { | |
| ocpus = var.instance_ocpus | |
| memory_in_gbs = var.instance_memory_gbs | |
| } | |
| create_vnic_details { | |
| subnet_id = oci_core_subnet.public.id | |
| assign_public_ip = true | |
| } | |
| source_details { | |
| source_type = "image" | |
| source_id = data.oci_core_images.ubuntu_arm.images[0].id | |
| } | |
| metadata = { | |
| ssh_authorized_keys = var.ssh_public_key | |
| user_data = base64encode(templatefile("${path.module}/cloud-init.yaml.tpl", {})) | |
| } | |
| # Avoid destroy/recreate churn if Canonical publishes a newer image after | |
| # initial provisioning -- we don't want every terraform apply to replace | |
| # a running instance (and its manually-uploaded OSRM data) just because | |
| # a newer base image exists. | |
| lifecycle { | |
| ignore_changes = [source_details] | |
| } | |
| } | |
| resource "oci_core_instance" "app" { | |
| compartment_id = var.compartment_ocid | |
| availability_domain = data.oci_identity_availability_domains.ads.availability_domains[0].name | |
| display_name = "li-demo-app" | |
| shape = var.instance_shape | |
| instance_options { | |
| are_legacy_imds_endpoints_disabled = true | |
| } | |
| shape_config { | |
| ocpus = var.instance_ocpus | |
| memory_in_gbs = var.instance_memory_gbs | |
| } | |
| create_vnic_details { | |
| subnet_id = oci_core_subnet.public.id | |
| assign_public_ip = true | |
| } | |
| source_details { | |
| source_type = "image" | |
| source_id = data.oci_core_images.ubuntu_arm.images[0].id | |
| } | |
| metadata = { | |
| ssh_authorized_keys = var.ssh_public_key | |
| user_data = base64encode(templatefile("${path.module}/cloud-init.yaml.tpl", {})) | |
| } | |
| # Avoid destroy/recreate churn if Canonical publishes a newer image after | |
| # initial provisioning -- we don't want every terraform apply to replace | |
| # a running instance (and its manually-uploaded OSRM data) just because | |
| # a newer base image exists. | |
| lifecycle { | |
| ignore_changes = [source_details] | |
| } | |
| } |
🧰 Tools
🪛 Checkov (3.3.2)
[medium] 16-49: Ensure OCI Compute Instance has Legacy MetaData service endpoint disabled
(CKV_OCI_5)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/oci/compute.tf` around lines 16 - 49, The oci_core_instance app
resource still allows the legacy instance metadata endpoint, so update its
instance metadata settings to enforce IMDSv2-only access and disable IMDSv1.
Make this change in the oci_core_instance "app" definition by adding the
appropriate metadata/instance options block alongside the existing metadata and
source_details settings, and ensure the configuration explicitly requires
token-based metadata requests.
Source: Linters/SAST tools
| # first attempt so it never gets masked by the retry loop. | ||
| set -uo pipefail # no -e: we need to inspect terraform's exit code ourselves | ||
|
|
||
| cd "$(dirname "${BASH_SOURCE[0]}")/.." |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard cd against failure.
Since set -e is intentionally not used, a failed cd here won't stop the script — subsequent terraform commands would silently run from the wrong directory.
🔧 Proposed fix
-cd "$(dirname "${BASH_SOURCE[0]}")/.."
+cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 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.
| cd "$(dirname "${BASH_SOURCE[0]}")/.." | |
| cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 1 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 9-9: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/oci/scripts/apply-with-retry.sh` at line 9, The script’s initial
directory change in apply-with-retry.sh can fail silently because set -e is not
enabled, causing later terraform commands to run from the wrong location. Update
the cd step near the top of the script to explicitly check for failure and exit
with a clear error before any retry or terraform logic runs, using the existing
script entrypoint and BASH_SOURCE-based directory resolution as the location to
fix.
Source: Linters/SAST tools
| MAX_ATTEMPTS=6 | ||
| BASE_DELAY=60 # seconds; backoff is BASE_DELAY * attempt: 60,120,180,240,300,360 (~21.5 min worst case) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Worst-case backoff comment is inaccurate.
The loop only sleeps after attempts 1-5 (delays 60,120,180,240,300 → ~15 min total); on the final attempt (attempt == MAX_ATTEMPTS) it exits immediately without sleeping, so the listed "360" delay and "~21.5 min" total never happen. infra/oci/README.md (Line 55, "~21 minutes worst case") states the same incorrect figure and should be corrected together with this comment.
📝 Proposed fix
-BASE_DELAY=60 # seconds; backoff is BASE_DELAY * attempt: 60,120,180,240,300,360 (~21.5 min worst case)
+BASE_DELAY=60 # seconds; backoff applies after attempts 1-5: 60,120,180,240,300 (~15 min worst case; no delay after the final attempt)📝 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.
| MAX_ATTEMPTS=6 | |
| BASE_DELAY=60 # seconds; backoff is BASE_DELAY * attempt: 60,120,180,240,300,360 (~21.5 min worst case) | |
| MAX_ATTEMPTS=6 | |
| BASE_DELAY=60 # seconds; backoff applies after attempts 1-5: 60,120,180,240,300 (~15 min worst case; no delay after the final attempt) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/oci/scripts/apply-with-retry.sh` around lines 11 - 12, The backoff
timing comment next to MAX_ATTEMPTS and BASE_DELAY is incorrect because
apply_with_retry only sleeps after attempts 1-5 and exits immediately on the
final attempt, so the “360” delay and "~21.5 min worst case" are wrong. Update
the inline comment to reflect the actual retry delays and total worst-case sleep
time, and make the matching wording consistent in the README entry that mentions
the worst case. Use the apply_with_retry loop and the MAX_ATTEMPTS/BASE_DELAY
symbols to keep the comment aligned with the implemented behavior.
| variable "ssh_ingress_cidr" { | ||
| description = "CIDR allowed to SSH into the instance. Restrict to a known IP where possible; 0.0.0.0/0 is accepted here only because this is a demo box." | ||
| type = string | ||
| default = "0.0.0.0/0" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not default SSH to the public internet.
CI never overrides ssh_ingress_cidr, so this 0.0.0.0/0 default becomes the deployed ingress rule and exposes the VM broadly. Make the value explicit per environment or narrow the default to a trusted admin range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/oci/variables.tf` around lines 56 - 60, The ssh_ingress_cidr variable
currently defaults to a public CIDR, which makes the deployed SSH ingress overly
permissive when CI does not override it. Update the variable definition in
ssh_ingress_cidr so it is not open by default: either remove the default and
require an explicit per-environment value, or change the default to a narrowly
trusted admin range. Make sure any references that consume ssh_ingress_cidr
continue to use the explicit value provided by each environment.
deploy-api hardcoded `git checkout main` on the remote VM regardless of which branch triggered the run, so pushing a release/* or release--* branch (both valid deploy triggers) would silently deploy main's HEAD instead of the tested commit -- inconsistent with deploy-web, which already builds from the triggering ref via actions/checkout.
The remote git clone/pull ran on the OCI VM itself, not the runner, so the runner's ssh-agent (loaded with DEPLOY_SSH_KEY) never reaches it -- agent forwarding was never enabled, and no deploy key was ever provisioned on the VM. First deploy would fail with "Permission denied (publickey)". The repo is public, so HTTPS needs no credentials at all.
deploy-api never had a mechanism to create the .env file docker-compose needs on the VM (correctly gitignored, so cloning the repo alone doesn't produce one) -- the api service's env_file and postgis's build args would both fail on first deploy. Terraform now generates the Postgres password once (random_password), stores it in a new OCI Vault as the single source of truth, and grants the app VM read access to it via its own instance principal (dynamic group + IAM policy in infra/oci/vault.tf) -- no long-lived credential needs to live in GitHub Secrets or on disk. scripts/fetch-secrets.sh runs on the VM at deploy time, reads the password out of Vault, and writes .env with the correct in-container service hostnames (postgis/ redis/osrm, not localhost -- the containerized api needs those, unlike the host-network .env used for local dev). The key is pinned to protection_mode = "SOFTWARE" so this stays free (HSM-protected key versions are billed past the tenancy's free allowance; software-protected ones and the first 150 Vault secrets per tenancy are Always Free -- this uses 1 of each).
Hardcoded 0.0.0.0/0 for the FastAPI backend's ingress rule with no way to narrow it. Introduces api_ingress_cidr (default unchanged at 0.0.0.0/0) so it can be restricted if the API ever sits behind something with a stable source IP. Left the default open and did not add TLS termination: the only current caller is the Cloudflare Worker running apps/web's Route Handlers, which has no fixed egress IP to allowlist, and terminating TLS without an owned domain would mean a self-signed cert that Node's fetch (and the Workers runtime) would reject by default -- breaking the deployed app. Plain HTTP here is an existing, documented tradeoff (see .env.example and infra/oci/README.md) since the browser never calls this URL directly. A real fix needs either a domain for a Let's-Encrypt-backed reverse proxy, or a client-side change to trust a self-signed cert -- both bigger than this pass; not done here.
…onfig backend.tf hardcoded the S3-compatible endpoint as a literal REPLACE_WITH_OCI_NAMESPACE placeholder, requiring a manual edit that's easy to forget -- if missed, terraform init in CI tries to resolve a host that doesn't exist and fails outright. Backend blocks can't reference variables, but Terraform does support partial backend configuration: omit the argument from the file and supply it via -backend-config at init time, exactly like access_key/ secret_key already are. Moves the endpoint over to that same mechanism (new OCI_S3_ENDPOINT secret) instead of a value someone has to remember to paste in.
… restrict this caller) Port 8000 has to stay open to 0.0.0.0/0 (see the existing api_ingress_cidr comment in network.tf) because the Cloudflare Worker running apps/web has no fixed egress IP to allowlist, and TLS termination isn't viable without an owned domain. Since it can't be closed off at the network layer, adds an app-layer backstop instead: every FastAPI route except /health now requires an X-Internal-Api-Key header matching API_SHARED_SECRET (apps/api/app/api/deps.py). Enforcement is opt-in -- unset (e.g. local dev) skips the check entirely, so nothing existing breaks. The three Next.js BFF Route Handlers that call the API now attach the header via a small helper (apps/web/src/utils/apiAuth.ts). Unlike the DB password this secret isn't Vault-backed -- it's a plain GitHub secret forwarded directly to both sides: fetch-secrets.sh writes it into the VM's .env, and deploy-web attaches it to the Worker via `wrangler secret put` (never NEXT_PUBLIC_*, so it's never in the client bundle). Adds tests/test_auth.py covering enforced/unenforced/health-exempt cases.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
infra/oci/vault.tf (1)
1-17: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winState stores the generated password in plaintext.
random_password.db_password.resultis persisted in the Terraform state file (your OCI Object Storage S3 backend), so the header comment's claim that "no long-lived credential ever needs to be stored ... on disk" holds for the VM/GitHub but not for state. Confirm the state bucket enforces encryption-at-rest and tightly scoped access, since anyone with state read access can recover the DB password.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infra/oci/vault.tf` around lines 1 - 17, The issue is that random_password.db_password.result is still stored in Terraform state, so the current comment overstates where the credential lives. Update the vault.tf guidance around random_password.db_password to acknowledge state exposure and ensure the OCI Object Storage backend is explicitly configured for encryption-at-rest and least-privilege access. Also verify the backend and any state readers are tightly scoped so only trusted identities can read the password from state.apps/web/src/app/api/categories/route.ts (1)
2-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBFF route now adds auth, conflicting with the "thin pass-through, no auth" path guideline.
This (and the other
route.tshandlers) now injectX-Internal-Api-KeyviaapiKeyHeaders(), which is necessary since the backend now requires it, but it contradicts the documented convention that these proxy routes should have "no transformation, caching, or auth." Consider updating the path instruction to explicitly carve out shared-secret header forwarding as an allowed exception, so the documented contract matches the intended architecture.As per path instructions,
apps/web/src/app/api/**/route.tsshould "Keep the Next.js BFF proxy routes as thin pass-through handlers that forward query params and request bodies to FastAPI and relay responses verbatim, with no transformation, caching, or auth."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/categories/route.ts` around lines 2 - 11, The Next.js BFF route handlers like GET in route.ts now intentionally forward the shared-secret header via apiKeyHeaders(), which conflicts with the current “no auth” path guideline. Update the apps/web/src/app/api/**/route.ts instruction to explicitly allow forwarding the internal API key (or other shared-secret headers) while keeping the routes thin pass-through for params, bodies, and responses. Mention the existing GET handler pattern and apiKeyHeaders() so the documented contract matches the implemented proxy behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/app/api/deps.py`:
- Around line 24-25: The shared-secret check in the API dependency uses a normal
string inequality comparison, which should be replaced with a constant-time
comparison. Update the guard around settings.api_shared_secret and
x_internal_api_key to use hmac.compare_digest for the secret check, keeping the
existing HTTPException behavior unchanged. Locate this in the dependency
function that protects the internal API key gate in deps.py.
---
Nitpick comments:
In `@apps/web/src/app/api/categories/route.ts`:
- Around line 2-11: The Next.js BFF route handlers like GET in route.ts now
intentionally forward the shared-secret header via apiKeyHeaders(), which
conflicts with the current “no auth” path guideline. Update the
apps/web/src/app/api/**/route.ts instruction to explicitly allow forwarding the
internal API key (or other shared-secret headers) while keeping the routes thin
pass-through for params, bodies, and responses. Mention the existing GET handler
pattern and apiKeyHeaders() so the documented contract matches the implemented
proxy behavior.
In `@infra/oci/vault.tf`:
- Around line 1-17: The issue is that random_password.db_password.result is
still stored in Terraform state, so the current comment overstates where the
credential lives. Update the vault.tf guidance around
random_password.db_password to acknowledge state exposure and ensure the OCI
Object Storage backend is explicitly configured for encryption-at-rest and
least-privilege access. Also verify the backend and any state readers are
tightly scoped so only trusted identities can read the password from state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 653f52c1-b4fa-42b9-a12c-82ef9e855778
📒 Files selected for processing (20)
.env.example.github/workflows/push.ymlapps/api/app/api/deps.pyapps/api/app/config/settings.pyapps/api/app/main.pyapps/api/tests/test_auth.pyapps/web/src/app/api/categories/route.tsapps/web/src/app/api/location/analyze/route.tsapps/web/src/app/api/search/address/route.tsapps/web/src/utils/apiAuth.tsinfra/oci/README.mdinfra/oci/backend.tfinfra/oci/cloud-init.yaml.tplinfra/oci/network.tfinfra/oci/outputs.tfinfra/oci/providers.tfinfra/oci/terraform.tfvars.exampleinfra/oci/variables.tfinfra/oci/vault.tfscripts/fetch-secrets.sh
✅ Files skipped from review due to trivial changes (3)
- .env.example
- infra/oci/terraform.tfvars.example
- infra/oci/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- infra/oci/variables.tf
- infra/oci/providers.tf
- infra/oci/cloud-init.yaml.tpl
- .github/workflows/push.yml
| if settings.api_shared_secret and x_internal_api_key != settings.api_shared_secret: | ||
| raise HTTPException(status_code=401, detail="Invalid or missing API key") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use constant-time comparison for the shared secret.
!= on strings is not constant-time; since this guard is the sole defense on an internet-reachable port (per the docstring above), prefer hmac.compare_digest to avoid a timing side-channel.
🔒 Proposed fix
+import hmac
+
from fastapi import Depends, Header, HTTPException
...
- if settings.api_shared_secret and x_internal_api_key != settings.api_shared_secret:
+ if settings.api_shared_secret and not hmac.compare_digest(
+ x_internal_api_key or "", settings.api_shared_secret
+ ):
raise HTTPException(status_code=401, detail="Invalid or missing API key")📝 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.
| if settings.api_shared_secret and x_internal_api_key != settings.api_shared_secret: | |
| raise HTTPException(status_code=401, detail="Invalid or missing API key") | |
| import hmac | |
| if settings.api_shared_secret and not hmac.compare_digest( | |
| x_internal_api_key or "", settings.api_shared_secret | |
| ): | |
| raise HTTPException(status_code=401, detail="Invalid or missing API key") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/app/api/deps.py` around lines 24 - 25, The shared-secret check in
the API dependency uses a normal string inequality comparison, which should be
replaced with a constant-time comparison. Update the guard around
settings.api_shared_secret and x_internal_api_key to use hmac.compare_digest for
the secret check, keeping the existing HTTPException behavior unchanged. Locate
this in the dependency function that protects the internal API key gate in
deps.py.
1 participant
Summary
Provision infrastructure in Oracle Cloud and deploy pipelines to deploy API, Web and other dependencies.
Key changes:
(Docker install + iptables fix), and retry-with-backoff for OCI capacity errors
middleware.ts with experimental-edge runtime (required by OpenNext's Cloudflare adapter)
Summary by CodeRabbit