fix: make cookie SameSite configurable for cross-origin deployments - #4280
Closed
forecho wants to merge 10 commits into
Closed
fix: make cookie SameSite configurable for cross-origin deployments#4280forecho wants to merge 10 commits into
forecho wants to merge 10 commits into
Conversation
* feat: add deployment automation (CI/CD, systemd, Nginx) - Add backend CI/CD: Go build + deploy via SSH, auto-rollback on failure - Add frontend CI/CD: Cloudflare Pages deployment - Add deploy scripts: first-time setup (Nginx + SSL) and update deploy - Add systemd service, Nginx reverse proxy template, .env.example - Add deployment documentation with scaling guide - Remove unused upstream workflows (Docker, Electron, Gitee sync, etc.) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: improve .env.example with detailed comments Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- Use `|` as sed delimiter to avoid issues with paths containing `/` - Use timestamped backups instead of overwriting single backup file - Use precise regex for port matching to avoid false positives - Add continue-on-error to Slack notification steps (secrets not yet configured) - Parameterize GOARCH in Makefile for ARM support Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The web directory uses bun (bun.lock exists, no package-lock.json). Switched from actions/setup-node + npm ci to oven-sh/setup-bun + bun install. Also removed non-existent type-check step. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
main.go uses `//go:embed web/dist` which requires the frontend build output to exist. Added bun install + build step before Go test and build. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- chown data/logs directories to new-api user after mkdir - chown and chmod .env file so the service can read it Fixes "failed to open log file" crash on startup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Default port 3000 conflicts with existing services on the server. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code uses VITE_REACT_APP_SERVER_URL but CI had VITE_REACT_APP_SERVER (missing _URL suffix), causing frontend to use relative paths instead of the API domain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Frontend at 4aicode.com needs cross-origin access to api.4aicode.com. Allow origins matching 4aicode.com and *.4aicode.pages.dev (preview). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Frontend sends custom header `new-api-user` which was not whitelisted. Use wildcard to avoid future issues with other custom headers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When frontend (e.g. Cloudflare Pages) and backend are on different domains, SameSite=Strict causes browsers to not send session cookies on cross-origin requests, resulting in 401 after login. Add COOKIE_SAME_SITE and COOKIE_SECURE env vars to control cookie policy. Setting COOKIE_SAME_SITE=none automatically enables Secure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughThis PR restructures CI/CD and deployment infrastructure by consolidating Docker-based release workflows into direct Linux server deployment via GitHub Actions, introduces frontend deployment to Cloudflare Pages, and adds comprehensive Bash-based deployment automation with Nginx reverse proxy and systemd service management. Cookie security configuration is externalized to environment variables. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
.github/workflows/deploy-web.yml (2)
14-18: Consider least-privilege token permissions for this workflow.If no step needs PR write operations, drop
pull-requests: writeto reduce token blast radius.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-web.yml around lines 14 - 18, The workflow permissions block grants unnecessary PR write scope; remove the "pull-requests: write" entry (or set it to "none"/"read" as appropriate) from the permissions block so the token follows least-privilege; audit any job/step that needs PR write and only re-add the minimal permission for specific jobs using a job-level permissions override rather than at the top-level.
55-81: Reuse the build artifact indeployinstead of rebuilding.The deploy job currently repeats install+build; using
web-distfrom thebuildjob makes deployments faster and removes drift between jobs.⚡ Suggested workflow simplification
deploy: needs: build runs-on: ubuntu-latest if: github.event_name == 'push' && github.ref == 'refs/heads/main' steps: - - uses: actions/checkout@v4 - - - name: Setup Bun - uses: oven-sh/setup-bun@v2 - - - name: 安装依赖 - working-directory: ./web - run: bun install --frozen-lockfile - - - name: 构建 - working-directory: ./web - run: bun run build + - name: 下载构建产物 + uses: actions/download-artifact@v4 + with: + name: web-dist + path: web/dist - name: 部署到 Cloudflare Pages uses: cloudflare/pages-action@v1 with: apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} projectName: 4aicode directory: web/dist gitHubToken: ${{ secrets.GITHUB_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-web.yml around lines 55 - 81, The deploy job currently re-runs the install/build steps; instead have the build job upload the built output (e.g., artifact name "web-dist") and modify the deploy job (job id deploy) to download that artifact using actions/download-artifact and point the Cloudflare Pages step's directory to the downloaded artifact (e.g., web-dist) rather than running the "安装依赖" and "构建" steps again; update the deploy job to remove bun install/build steps and ensure the Cloudflare Pages step (uses: cloudflare/pages-action@v1) uses the downloaded artifact path as its directory.deploy/nginx-new-api.conf (1)
56-57: SetConnectionconditionally for WebSocket upgrades.Always sending
Connection: upgradeis unnecessary for normal HTTP requests and can interfere with upstream connection handling.🔧 Suggested Nginx pattern
+map $http_upgrade $connection_upgrade { + default upgrade; + '' close; +} + server { ... location / { ... proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; + proxy_set_header Connection $connection_upgrade;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/nginx-new-api.conf` around lines 56 - 57, The config always sets "proxy_set_header Connection \"upgrade\"" which forces Connection: upgrade on all requests; change it to set Connection conditionally by adding a variable (e.g. $connection_upgrade) mapped from $http_upgrade (map $http_upgrade $connection_upgrade { websocket "upgrade"; default ""; }) and then replace the direct directive with "proxy_set_header Connection $connection_upgrade"; update the existing "proxy_set_header Upgrade $http_upgrade;" lines to remain unchanged so WebSocket upgrades keep Upgrade header while normal requests send no Connection: upgrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy-backend.yml:
- Around line 46-48: The test step named "Run tests" currently uses
"continue-on-error: true", which allows downstream "build-and-deploy" to run
despite failing tests; remove "continue-on-error: true" (or set it to false) so
the workflow fails on test failures and ensure the deploy job explicitly depends
on the test job (add "needs: <test_job_id>" to the "build-and-deploy" job) so
deployment only runs when the test job succeeds.
In `@deploy/deploy-all.sh`:
- Around line 140-146: The script currently calls "systemctl reload nginx"
before checking activity which causes set -e to abort when nginx is installed
but stopped; modify the flow around the "systemctl is-active --quiet nginx"
check so that you first test activity and if inactive run "systemctl start
nginx" and "systemctl enable nginx" (using the same commands currently in the
else branch), then run "systemctl reload nginx" only after ensuring nginx is
running; update the block that includes "systemctl reload nginx", "systemctl
is-active --quiet nginx", "systemctl start nginx", and "systemctl enable nginx"
to implement this ordering.
In `@deploy/deploy.sh`:
- Around line 188-191: The cleanup currently runs rm -rf "$DEPLOY_DIR" with
DEPLOY_DIR coming from argv; validate and guard it before deletion by (1)
ensuring DEPLOY_DIR is non-empty and not "/" and not "."; (2) disallow paths
with ".." or absolute root traversal; (3) enforce it is under an expected base
(e.g., DEPLOY_ROOT or a fixed safe directory) via a prefix check (e.g., ensure
DEPLOY_DIR starts with "$DEPLOY_ROOT" or "/tmp/uploads"); and (4) fail loudly
and exit non‑zero if checks fail instead of performing rm -rf; apply these
checks immediately before the rm -rf and use the same DEPLOY_DIR symbol so the
change is localized.
- Around line 64-90: The deploy script stops the service (SERVICE_NAME) and
replaces the binary before validating the environment file, which can leave the
service down if INSTALL_DIR/.env is missing; move the check for the presence and
readability of "$INSTALL_DIR/.env" (and its chown/chmod validation) to before
the systemctl stop $SERVICE_NAME step, and exit with a clear error if the file
is missing or unreadable so the running service is not stopped; keep the
existing chown/chmod lines for "$INSTALL_DIR/.env" and the binary install steps
(cp new-api, chmod +x, chown new-api:new-api) after the env validation.
In `@deploy/nginx-new-api.conf`:
- Around line 34-38: The CORS response uses a dynamic origin via the add_header
directive (add_header Access-Control-Allow-Origin $cors_origin) but doesn't
include a Vary header, which can cause caches/CDNs to serve responses with
incorrect CORS; update the nginx config to emit a Vary: Origin header by adding
an add_header Vary Origin always; (paired with the existing add_header
Access-Control-Allow-Origin $cors_origin) so caches will vary responses by
Origin.
In `@deploy/README.md`:
- Line 17: Update the untyped fenced code blocks in deploy/README.md to include
language identifiers (e.g., ```text or ```bash) so markdownlint MD040 is
satisfied and rendering improves; locate the three untyped fences shown in the
diff (the block containing "浏览器 ...", the single-line "api.4aicode.com →
<hetzner 服务器公网 IP>", and the larger diagram block starting with "Nginx (主节点)"),
replace their opening ``` with ```text (or a more specific language like ```bash
if appropriate) and keep the closing ``` unchanged.
- Line 94: Update the README step that instructs adding Actions secrets by
replacing the hard-coded repository string `richcalls/new-api` with the correct
repository identifier for this project (use the actual owner/repo value or a
placeholder like `OWNER/REPO`), so the Settings → Secrets and variables →
Actions instruction points to the right repo; locate the occurrence of
`richcalls/new-api` in the deploy/README.md and change it accordingly.
In `@main.go`:
- Around line 175-183: Normalize and validate the COOKIE_SAME_SITE and
COOKIE_SECURE env values before comparing: read the env, run strings.TrimSpace
and strings.ToLower, then set sameSite to
http.SameSiteNoneMode/http.SameSiteLaxMode/http.SameSiteStrictMode based on
"none", "lax", "strict" (default to StrictMode when empty) and set secure true
for COOKIE_SECURE values like "true", "1", "yes"; if a non-empty value is
present but not recognized, emit a warning log mentioning COOKIE_SAME_SITE or
COOKIE_SECURE and the provided value so mis-cased or invalid input is visible
(reference variables/symbols: sameSite, secure, COOKIE_SAME_SITE, COOKIE_SECURE,
http.SameSiteNoneMode, http.SameSiteLaxMode).
In `@makefile`:
- Around line 6-9: The build target in the Makefile may fail if the output
directory doesn't exist; update the build recipe for the build target to create
the bin directory before running go build (e.g., run a mkdir -p bin step prior
to the CGO_ENABLED... go build command that writes to -o bin/new-api) so
bin/new-api can be written reliably on a clean workspace.
---
Nitpick comments:
In @.github/workflows/deploy-web.yml:
- Around line 14-18: The workflow permissions block grants unnecessary PR write
scope; remove the "pull-requests: write" entry (or set it to "none"/"read" as
appropriate) from the permissions block so the token follows least-privilege;
audit any job/step that needs PR write and only re-add the minimal permission
for specific jobs using a job-level permissions override rather than at the
top-level.
- Around line 55-81: The deploy job currently re-runs the install/build steps;
instead have the build job upload the built output (e.g., artifact name
"web-dist") and modify the deploy job (job id deploy) to download that artifact
using actions/download-artifact and point the Cloudflare Pages step's directory
to the downloaded artifact (e.g., web-dist) rather than running the "安装依赖" and
"构建" steps again; update the deploy job to remove bun install/build steps and
ensure the Cloudflare Pages step (uses: cloudflare/pages-action@v1) uses the
downloaded artifact path as its directory.
In `@deploy/nginx-new-api.conf`:
- Around line 56-57: The config always sets "proxy_set_header Connection
\"upgrade\"" which forces Connection: upgrade on all requests; change it to set
Connection conditionally by adding a variable (e.g. $connection_upgrade) mapped
from $http_upgrade (map $http_upgrade $connection_upgrade { websocket "upgrade";
default ""; }) and then replace the direct directive with "proxy_set_header
Connection $connection_upgrade"; update the existing "proxy_set_header Upgrade
$http_upgrade;" lines to remain unchanged so WebSocket upgrades keep Upgrade
header while normal requests send no Connection: upgrade.
🪄 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: CHILL
Plan: Pro
Run ID: 939d9355-3efc-4572-874f-04ab1f284553
📒 Files selected for processing (17)
.env.example.github/workflows/deploy-backend.yml.github/workflows/deploy-web.yml.github/workflows/docker-image-alpha.yml.github/workflows/docker-image-arm64.yml.github/workflows/electron-build.yml.github/workflows/pr-check.yml.github/workflows/release.yml.github/workflows/sync-to-gitee.ymldeploy/.env.exampledeploy/README.mddeploy/deploy-all.shdeploy/deploy.shdeploy/new-api.servicedeploy/nginx-new-api.confmain.gomakefile
💤 Files with no reviewable changes (6)
- .github/workflows/pr-check.yml
- .github/workflows/sync-to-gitee.yml
- .github/workflows/electron-build.yml
- .github/workflows/docker-image-arm64.yml
- .github/workflows/docker-image-alpha.yml
- .github/workflows/release.yml
Comment on lines
+46
to
+48
| - name: Run tests | ||
| run: go test ./... -v | ||
| continue-on-error: true |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "deploy-backend.yml" -type fRepository: QuantumNous/new-api
Length of output: 101
🏁 Script executed:
cat -n .github/workflows/deploy-backend.ymlRepository: QuantumNous/new-api
Length of output: 3993
Don't let failed tests still deploy to main.
Line 48 turns the test step into a soft failure, so the build-and-deploy job at line 61 can still publish a backend after go test fails. That removes the only deployment gate in this workflow.
Suggested fix
- name: Run tests
run: go test ./... -v
- continue-on-error: 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.
Suggested change
| - name: Run tests | |
| run: go test ./... -v | |
| continue-on-error: true | |
| - name: Run tests | |
| run: go test ./... -v |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-backend.yml around lines 46 - 48, The test step
named "Run tests" currently uses "continue-on-error: true", which allows
downstream "build-and-deploy" to run despite failing tests; remove
"continue-on-error: true" (or set it to false) so the workflow fails on test
failures and ensure the deploy job explicitly depends on the test job (add
"needs: <test_job_id>" to the "build-and-deploy" job) so deployment only runs
when the test job succeeds.
Comment on lines
+140
to
+146
| systemctl reload nginx | ||
| if systemctl is-active --quiet nginx; then | ||
| echo -e "${GREEN}✅ Nginx 正在运行${NC}" | ||
| else | ||
| systemctl start nginx | ||
| systemctl enable nginx | ||
| echo -e "${GREEN}✅ Nginx 已启动${NC}" |
Contributor
There was a problem hiding this comment.
Start nginx before attempting a reload.
With set -e, Line 140 aborts the script when nginx is installed but currently stopped, so the fallback start path at Lines 141-146 never executes.
Suggested fix
-systemctl reload nginx
-if systemctl is-active --quiet nginx; then
+if systemctl is-active --quiet nginx; then
+ systemctl reload nginx
echo -e "${GREEN}✅ Nginx 正在运行${NC}"
else
systemctl start nginx
systemctl enable nginx
echo -e "${GREEN}✅ Nginx 已启动${NC}"
fi📝 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.
Suggested change
| systemctl reload nginx | |
| if systemctl is-active --quiet nginx; then | |
| echo -e "${GREEN}✅ Nginx 正在运行${NC}" | |
| else | |
| systemctl start nginx | |
| systemctl enable nginx | |
| echo -e "${GREEN}✅ Nginx 已启动${NC}" | |
| if systemctl is-active --quiet nginx; then | |
| systemctl reload nginx | |
| echo -e "${GREEN}✅ Nginx 正在运行${NC}" | |
| else | |
| systemctl start nginx | |
| systemctl enable nginx | |
| echo -e "${GREEN}✅ Nginx 已启动${NC}" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/deploy-all.sh` around lines 140 - 146, The script currently calls
"systemctl reload nginx" before checking activity which causes set -e to abort
when nginx is installed but stopped; modify the flow around the "systemctl
is-active --quiet nginx" check so that you first test activity and if inactive
run "systemctl start nginx" and "systemctl enable nginx" (using the same
commands currently in the else branch), then run "systemctl reload nginx" only
after ensuring nginx is running; update the block that includes "systemctl
reload nginx", "systemctl is-active --quiet nginx", "systemctl start nginx", and
"systemctl enable nginx" to implement this ordering.
Comment on lines
+64
to
+90
| echo -e "${BLUE}2. 停止现有服务...${NC}" | ||
|
|
||
| systemctl stop $SERVICE_NAME 2>/dev/null || true | ||
|
|
||
| echo -e "${BLUE}3. 安装二进制文件...${NC}" | ||
|
|
||
| # 备份旧二进制 | ||
| if [ -f "$INSTALL_DIR/new-api" ]; then | ||
| mv "$INSTALL_DIR/new-api" "$INSTALL_DIR/new-api.backup.$(date +%Y%m%d%H%M%S)" | ||
| echo -e "${YELLOW}已备份旧版本${NC}" | ||
| fi | ||
|
|
||
| # 安装新二进制 | ||
| cp new-api "$INSTALL_DIR/new-api" | ||
| chmod +x "$INSTALL_DIR/new-api" | ||
| chown new-api:new-api "$INSTALL_DIR/new-api" | ||
|
|
||
| echo -e "${GREEN}✅ 二进制文件已安装${NC}" | ||
|
|
||
| # 检查 .env 是否存在，并确保权限正确 | ||
| if [ ! -f "$INSTALL_DIR/.env" ]; then | ||
| echo -e "${RED}❌ 未找到 $INSTALL_DIR/.env，请先上传 .env 文件${NC}" | ||
| echo -e "${YELLOW}参考 deploy/.env.example 创建${NC}" | ||
| exit 1 | ||
| fi | ||
| chown new-api:new-api "$INSTALL_DIR/.env" | ||
| chmod 600 "$INSTALL_DIR/.env" |
Contributor
There was a problem hiding this comment.
Validate .env before taking the service down.
Line 66 stops the unit and Lines 71-79 replace the binary before Line 84 checks /opt/new-api/.env. If that file is missing or unreadable, the deploy exits mid-flight with the previous service already offline.
Suggested fix
echo -e "${BLUE}2. 停止现有服务...${NC}"
+# 先检查 .env，避免中途失败导致服务已停止
+if [ ! -f "$INSTALL_DIR/.env" ]; then
+ echo -e "${RED}❌ 未找到 $INSTALL_DIR/.env，请先上传 .env 文件${NC}"
+ echo -e "${YELLOW}参考 deploy/.env.example 创建${NC}"
+ exit 1
+fi
+chown new-api:new-api "$INSTALL_DIR/.env"
+chmod 600 "$INSTALL_DIR/.env"
+
systemctl stop $SERVICE_NAME 2>/dev/null || true
echo -e "${BLUE}3. 安装二进制文件...${NC}"
@@
-# 检查 .env 是否存在，并确保权限正确
-if [ ! -f "$INSTALL_DIR/.env" ]; then
- echo -e "${RED}❌ 未找到 $INSTALL_DIR/.env，请先上传 .env 文件${NC}"
- echo -e "${YELLOW}参考 deploy/.env.example 创建${NC}"
- exit 1
-fi
-chown new-api:new-api "$INSTALL_DIR/.env"
-chmod 600 "$INSTALL_DIR/.env"
-
echo -e "${BLUE}4. 安装 systemd 服务...${NC}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/deploy.sh` around lines 64 - 90, The deploy script stops the service
(SERVICE_NAME) and replaces the binary before validating the environment file,
which can leave the service down if INSTALL_DIR/.env is missing; move the check
for the presence and readability of "$INSTALL_DIR/.env" (and its chown/chmod
validation) to before the systemctl stop $SERVICE_NAME step, and exit with a
clear error if the file is missing or unreadable so the running service is not
stopped; keep the existing chown/chmod lines for "$INSTALL_DIR/.env" and the
binary install steps (cp new-api, chmod +x, chown new-api:new-api) after the env
validation.
Comment on lines
+188
to
+191
| echo -e "${BLUE}7. 清理部署文件...${NC}" | ||
|
|
||
| cd / | ||
| rm -rf "$DEPLOY_DIR" |
Contributor
There was a problem hiding this comment.
Guard the cleanup target before rm -rf.
DEPLOY_DIR comes straight from argv, and Line 191 deletes it as root. A bad invocation here can wipe an arbitrary directory instead of just the transient upload area.
Suggested fix
echo -e "${BLUE}7. 清理部署文件...${NC}"
cd /
-rm -rf "$DEPLOY_DIR"
+case "$DEPLOY_DIR" in
+ /tmp/new-api-deploy|/tmp/new-api-deploy/*)
+ rm -rf "$DEPLOY_DIR"
+ ;;
+ *)
+ echo -e "${RED}❌ 拒绝删除非预期目录: $DEPLOY_DIR${NC}"
+ exit 1
+ ;;
+esac
echo -e "${GREEN}✅ 清理完成${NC}"📝 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.
Suggested change
| echo -e "${BLUE}7. 清理部署文件...${NC}" | |
| cd / | |
| rm -rf "$DEPLOY_DIR" | |
| echo -e "${BLUE}7. 清理部署文件...${NC}" | |
| cd / | |
| case "$DEPLOY_DIR" in | |
| /tmp/new-api-deploy|/tmp/new-api-deploy/*) | |
| rm -rf "$DEPLOY_DIR" | |
| ;; | |
| *) | |
| echo -e "${RED}❌ 拒绝删除非预期目录: $DEPLOY_DIR${NC}" | |
| exit 1 | |
| ;; | |
| esac | |
| echo -e "${GREEN}✅ 清理完成${NC}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/deploy.sh` around lines 188 - 191, The cleanup currently runs rm -rf
"$DEPLOY_DIR" with DEPLOY_DIR coming from argv; validate and guard it before
deletion by (1) ensuring DEPLOY_DIR is non-empty and not "/" and not "."; (2)
disallow paths with ".." or absolute root traversal; (3) enforce it is under an
expected base (e.g., DEPLOY_ROOT or a fixed safe directory) via a prefix check
(e.g., ensure DEPLOY_DIR starts with "$DEPLOY_ROOT" or "/tmp/uploads"); and (4)
fail loudly and exit non‑zero if checks fail instead of performing rm -rf; apply
these checks immediately before the rm -rf and use the same DEPLOY_DIR symbol so
the change is localized.
Comment on lines
+34
to
+38
| add_header Access-Control-Allow-Origin $cors_origin always; | ||
| add_header Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS, PATCH" always; | ||
| add_header Access-Control-Allow-Headers "*" always; | ||
| add_header Access-Control-Allow-Credentials "true" always; | ||
|
|
Contributor
There was a problem hiding this comment.
Add Vary: Origin for dynamic CORS responses.
Access-Control-Allow-Origin is origin-dependent, but there is no Vary: Origin. In front of shared caches/CDNs, this can serve a response with the wrong CORS policy.
🛡️ Suggested fix
add_header Access-Control-Allow-Origin $cors_origin always;
+ add_header Vary "Origin" always;
add_header Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS, PATCH" always;
add_header Access-Control-Allow-Headers "*" always;
add_header Access-Control-Allow-Credentials "true" always;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/nginx-new-api.conf` around lines 34 - 38, The CORS response uses a
dynamic origin via the add_header directive (add_header
Access-Control-Allow-Origin $cors_origin) but doesn't include a Vary header,
which can cause caches/CDNs to serve responses with incorrect CORS; update the
nginx config to emit a Vary: Origin header by adding an add_header Vary Origin
always; (paired with the existing add_header Access-Control-Allow-Origin
$cors_origin) so caches will vary responses by Origin.
|
|
||
| ## 架构概览 | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
These fences are currently untyped, which triggers markdownlint (MD040) and reduces readability in some renderers.
📝 Suggested markdown fix
-```
+```text
浏览器
...
-```
+```
-```
+```text
api.4aicode.com → <hetzner 服务器公网 IP>
-```
+```
-```
+```text
Nginx (主节点)
...
-```
+```Also applies to: 88-88, 116-116
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/README.md` at line 17, Update the untyped fenced code blocks in
deploy/README.md to include language identifiers (e.g., ```text or ```bash) so
markdownlint MD040 is satisfied and rendering improves; locate the three untyped
fences shown in the diff (the block containing "浏览器 ...", the single-line
"api.4aicode.com → <hetzner 服务器公网 IP>", and the larger diagram block starting
with "Nginx (主节点)"), replace their opening ``` with ```text (or a more specific
language like ```bash if appropriate) and keep the closing ``` unchanged.
|
|
||
| ### 第四步：配置 GitHub Secrets | ||
|
|
||
| 在 `richcalls/new-api` 仓库的 Settings → Secrets and variables → Actions 添加： |
Contributor
There was a problem hiding this comment.
Fix repository name in the Secrets setup step.
Line 94 points to richcalls/new-api; this should match the actual repository to avoid configuring secrets in the wrong place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/README.md` at line 94, Update the README step that instructs adding
Actions secrets by replacing the hard-coded repository string
`richcalls/new-api` with the correct repository identifier for this project (use
the actual owner/repo value or a placeholder like `OWNER/REPO`), so the Settings
→ Secrets and variables → Actions instruction points to the right repo; locate
the occurrence of `richcalls/new-api` in the deploy/README.md and change it
accordingly.
Comment on lines
+175
to
+183
| if os.Getenv("COOKIE_SAME_SITE") == "none" { | ||
| sameSite = http.SameSiteNoneMode | ||
| secure = true // SameSite=None requires Secure=true | ||
| } else if os.Getenv("COOKIE_SAME_SITE") == "lax" { | ||
| sameSite = http.SameSiteLaxMode | ||
| } | ||
| if os.Getenv("COOKIE_SECURE") == "true" { | ||
| secure = true | ||
| } |
Contributor
There was a problem hiding this comment.
Normalize cookie env values and log invalid input.
Line 175 and Line 181 currently require exact lowercase values. Mis-cased values (for example None/TRUE) silently degrade to Strict/false, which can reintroduce post-login 401s in cross-origin setups.
🔧 Suggested hardening
sameSite := http.SameSiteStrictMode
secure := false
- if os.Getenv("COOKIE_SAME_SITE") == "none" {
+ cookieSameSite := strings.ToLower(strings.TrimSpace(os.Getenv("COOKIE_SAME_SITE")))
+ switch cookieSameSite {
+ case "none":
sameSite = http.SameSiteNoneMode
secure = true // SameSite=None requires Secure=true
- } else if os.Getenv("COOKIE_SAME_SITE") == "lax" {
+ case "lax":
sameSite = http.SameSiteLaxMode
+ case "", "strict":
+ // keep default strict
+ default:
+ common.SysLog("invalid COOKIE_SAME_SITE, fallback to strict")
}
- if os.Getenv("COOKIE_SECURE") == "true" {
+ if v, err := strconv.ParseBool(strings.TrimSpace(os.Getenv("COOKIE_SECURE"))); err == nil && v {
secure = true
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.go` around lines 175 - 183, Normalize and validate the COOKIE_SAME_SITE
and COOKIE_SECURE env values before comparing: read the env, run
strings.TrimSpace and strings.ToLower, then set sameSite to
http.SameSiteNoneMode/http.SameSiteLaxMode/http.SameSiteStrictMode based on
"none", "lax", "strict" (default to StrictMode when empty) and set secure true
for COOKIE_SECURE values like "true", "1", "yes"; if a non-empty value is
present but not recognized, emit a warning log mentioning COOKIE_SAME_SITE or
COOKIE_SECURE and the provided value so mis-cased or invalid input is visible
(reference variables/symbols: sameSite, secure, COOKIE_SAME_SITE, COOKIE_SECURE,
http.SameSiteNoneMode, http.SameSiteLaxMode).
Comment on lines
+6
to
+9
| build: | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=$${GOARCH:-amd64} go build \ | ||
| -ldflags "-s -w -X 'github.com/QuantumNous/new-api/common.Version=$$(cat VERSION 2>/dev/null || echo dev)'" \ | ||
| -o bin/new-api |
Contributor
There was a problem hiding this comment.
Ensure bin/ exists before go build -o bin/new-api.
On a clean workspace, Line 9 can fail because bin is missing.
🔧 Suggested fix
build:
+ mkdir -p bin
CGO_ENABLED=0 GOOS=linux GOARCH=$${GOARCH:-amd64} go build \
-ldflags "-s -w -X 'github.com/QuantumNous/new-api/common.Version=$$(cat VERSION 2>/dev/null || echo dev)'" \
-o bin/new-api📝 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.
Suggested change
| build: | |
| CGO_ENABLED=0 GOOS=linux GOARCH=$${GOARCH:-amd64} go build \ | |
| -ldflags "-s -w -X 'github.com/QuantumNous/new-api/common.Version=$$(cat VERSION 2>/dev/null || echo dev)'" \ | |
| -o bin/new-api | |
| build: | |
| mkdir -p bin | |
| CGO_ENABLED=0 GOOS=linux GOARCH=$${GOARCH:-amd64} go build \ | |
| -ldflags "-s -w -X 'github.com/QuantumNous/new-api/common.Version=$$(cat VERSION 2>/dev/null || echo dev)'" \ | |
| -o bin/new-api |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@makefile` around lines 6 - 9, The build target in the Makefile may fail if
the output directory doesn't exist; update the build recipe for the build target
to create the bin directory before running go build (e.g., run a mkdir -p bin
step prior to the CGO_ENABLED... go build command that writes to -o bin/new-api)
so bin/new-api can be written reliably on a clean workspace.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SameSite=Strict导致浏览器不发送 session cookie，登录成功后所有请求返回 401COOKIE_SAME_SITE和COOKIE_SECURE环境变量控制 cookie 策略COOKIE_SAME_SITE=none即可解决Test plan
COOKIE_SAME_SITE=none，验证登录后页面切换不再 401COOKIE_SAME_SITE=lax也能正常工作🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores