Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThis PR upgrades Node.js to 25.8.2 in CI and local config, replaces Node-based builder images with manual Node installs in Debian-based Docker build stages, adds CORS preflight and method restrictions for /relay/* in the Caddy config (proxying only POST to the upstream), and adds tests validating the Caddy relay behaviour and ApiClient relay usage. ChangesBuild and Node Version
Caddy Relay, Tests and Client
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Caddy
participant RelayBackend as Proxy (mTLS)
participant API
Browser->>Caddy: OPTIONS /relay/generate-signature (Origin: https://oullin.io)
Caddy-->>Browser: 204 + Access-Control-Allow-* headers
Browser->>Caddy: POST /relay/generate-signature (Origin: https://oullin.io)
Caddy->>RelayBackend: reverse_proxy to https://proxy:8443 (Host: oullin_proxy_prod) with mTLS
RelayBackend->>API: internal request to API
API-->>RelayBackend: response
RelayBackend-->>Caddy: proxied response
Caddy-->>Browser: proxied response (with CORS headers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 3/5 reviews remaining, refill in 18 minutes and 10 seconds. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile`:
- Around line 26-36: The RUN step that downloads and verifies Node (the shell
block using set -eux and commands referencing TARGETARCH and NODE_VERSION,
including curl, grep and sha256sum) should enable pipefail so any piped command
failures are propagated; update that shell invocation to use set -euo pipefail
(or add a SHELL directive to enable pipefail) so failures in the grep |
sha256sum pipeline (or any other piped command) cause the build to fail fast and
produce clear errors.
In `@docker/Dockerfile.caddy-watcher`:
- Around line 12-22: Summary: The RUN block in Dockerfile.caddy-watcher lacks
pipefail, so failing commands in pipes (curl|grep|sha256sum) may be masked. Fix:
enable pipefail by adding a SHELL directive (e.g., SHELL
["/bin/bash","-o","pipefail","-c"]) near the top of the Dockerfile or prepend
"set -o pipefail;" to the existing RUN invocation that uses TARGETARCH,
NODE_VERSION, curl, grep and sha256sum; ensure the change affects the RUN that
downloads and verifies "node-v${NODE_VERSION}-linux-${node_arch}.tar.xz" so the
sha256sum check will fail the build on validation errors.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: aba9d46a-0509-4e58-9280-1b63daa75ba5
📒 Files selected for processing (8)
.github/workflows/linter.yml.github/workflows/test.yml.nvmrccaddy/WebCaddyfile.internaldocker/Dockerfiledocker/Dockerfile.caddy-watchertests/caddy/web-caddyfile.test.tstests/stores/api/client.test.ts
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a DNS-related issue by updating the Caddy server configuration to correctly resolve an internal proxy service, alongside enhancing its CORS handling for relay paths. It also includes an update to the Node.js version across the project's development environment and Docker builds. Comprehensive new tests have been added to validate both the Caddyfile's behavior and the API client's interaction with the updated relay configuration, ensuring the stability and correctness of these critical infrastructure changes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Node.js version to 25.8.2 and adjusts the Dockerfiles to install the runtime manually on Debian Slim. It also introduces a CORS preflight handler in the Caddy configuration and updates the reverse proxy target. A review comment identifies that CORS headers should be applied to both preflight and actual requests to ensure cross-origin compatibility.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker/Dockerfile (1)
47-47:⚠️ Potential issue | 🟠 MajorAvoid a floating Corepack in the build.
corepack@latestcreates a non-deterministic dependency that can break the pnpm bootstrap without any repo change. Node 22.13.0 bundles Corepack, so remove the explicit install and use it directly. Apply the same fix todocker/Dockerfile.caddy-watcherline 26.Deterministic approach
-RUN npm install -g corepack@latest --force && corepack enable && corepack prepare pnpm@${PNPM_VERSION} --activate && pnpm install --frozen-lockfile +RUN corepack enable \ + && corepack prepare pnpm@${PNPM_VERSION} --activate \ + && pnpm install --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` at line 47, The RUN which installs corepack globally introduces nondeterminism; remove the "npm install -g corepack@latest --force &&" prefix and leave "corepack enable && corepack prepare pnpm@${PNPM_VERSION} --activate && pnpm install --frozen-lockfile" so the image uses the bundled Corepack from Node 22.13.0; apply the same change to the other Dockerfile's analogous RUN line that currently calls corepack prepare pnpm@${PNPM_VERSION}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@caddy/WebCaddyfile.internal`:
- Around line 24-35: The /relay/* proxy block (handle_path /relay/* with rewrite
and reverse_proxy) currently allows any method; restrict it to only POST by
adding a method matcher (e.g., handle `@relay_post` or handle_path with methods
POST) so the reverse_proxy and rewrite apply only for POST requests, and add a
complementary handler for /relay/* that responds 405 Method Not Allowed for all
other methods (non-POST, excluding existing OPTIONS/ preflight handling). Update
the configuration references around handle_path /relay/*, rewrite * /api{path},
and the reverse_proxy block to ensure only POST reaches oullin_proxy_prod and
all other methods return 405.
In `@docker/Dockerfile.caddy-watcher`:
- Around line 14-24: The RUN step currently verifies the Node binary against an
unsigned SHASUMS256.txt; change it to download SHASUMS256.txt.asc and verify its
GPG signature before checksum verification: fetch SHASUMS256.txt.asc (in the
same place you currently fetch SHASUMS256.txt), import or provide the Node.js
public keyring (use a nodejs-keyring.kbx), run gpgv --keyring nodejs-keyring.kbx
--output SHASUMS256.txt SHASUMS256.txt.asc to produce the verified
SHASUMS256.txt, then proceed to grep/sha256sum and tar extraction as before;
apply the same hardening to the identical block used in the other Dockerfile
(same TARGETARCH/TAR download flow that sets node_arch and uses NODE_VERSION).
In `@tests/caddy/web-caddyfile.test.ts`:
- Around line 15-17: The current assertion on the caddyfile variable uses a
loose regex that only ensures Access-Control-Allow-Headers is non-empty; tighten
the test by asserting the exact expected header list (or at minimum the required
header tokens) instead of “[^"]+”. Update the expect(caddyfile).toMatch(...)
assertion that checks Access-Control-Allow-Headers so it matches the concrete
header string you require (or replace it with separate expect checks for each
required token such as Content-Type, Authorization, etc.) to prevent contract
drift.
---
Outside diff comments:
In `@docker/Dockerfile`:
- Line 47: The RUN which installs corepack globally introduces nondeterminism;
remove the "npm install -g corepack@latest --force &&" prefix and leave
"corepack enable && corepack prepare pnpm@${PNPM_VERSION} --activate && pnpm
install --frozen-lockfile" so the image uses the bundled Corepack from Node
22.13.0; apply the same change to the other Dockerfile's analogous RUN line that
currently calls corepack prepare pnpm@${PNPM_VERSION}.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0cade410-0e8e-4c61-8075-26d9810507b7
📒 Files selected for processing (4)
caddy/WebCaddyfile.internaldocker/Dockerfiledocker/Dockerfile.caddy-watchertests/caddy/web-caddyfile.test.ts
1 participant
Summary by CodeRabbit
Chores
Tests