Claude/update prod runner script lq2g s - #55
Conversation
**Infrastructure (GitHub Actions)** - Add explicit `permissions: contents: read` to ci.yml and deploy.yml - Remove workflow_dispatch inputs from deploy.yml (SLSA build-output integrity) - Make npm audit always non-blocking (continue-on-error: true) - Fix smoke test ordering: run AFTER npm ci so node_modules exist **SAST** - Bind health server to 127.0.0.1 instead of 0.0.0.0 (CWE-319) **Antipatterns (index.js)** - Use object/array destructuring (lines 447, 3034) - Replace implicit boolean coercion !! with Boolean() (lines 2454, 3617) - Replace ++ / -- with += 1 / -= 1 (lines 4091, 4584, 4656, 4768) - Use const instead of let for non-reassigned report (line 4678) - Add no-op comment in empty catch block (line 1736) - Remove redundant trailing return statement (line 3912) - Extract nested template literals to named variables (32 occurrences) - Move user.pendingAction = null inside runUserMutation to fix race (line 3695) - Inline gw.endTimer assignment with eslint-disable for lint false positive (4806) - Add eslint-disable-next-line for intentional await-in-loop (line 1260) https://claude.ai/code/session_01X3PxGFF5zzKptQwVkjYzzN
notify-telegram.sh: require python3 before sending; removes the sed fallback that produced invalid JSON for multi-line messages (newlines, backslashes not escaped). Exits gracefully when python3 is absent. rollback.sh: recompute PORT and HEALTH_URL from the restored release's .env immediately after cp-a, so the post-rollback health check probes the correct port even when the target release uses a different PORT. smoke-test.sh: make the logs/ else-branch verify actual writability with touch (mirrors the data/ check). Prevents a read-only filesystem from silently passing the logs directory check. https://claude.ai/code/session_01X3PxGFF5zzKptQwVkjYzzN
|
CodeAnt AI is reviewing your PR. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe changes enhance CI/CD security and robustness across multiple components: adding read-only permissions to CI, restructuring the deploy workflow with hardcoded deployment mode and reorganized VPS steps, refactoring core application logic with standardized variable formatting and state migration, requiring Python 3 validation in notification scripts, improving rollback port detection, and validating log directory write permissions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
codeant-ai
Bot
added
the
size:L
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| # The restored release may use a different PORT than the one that was running, | ||
| # so we must re-read it now — before the health check — to probe the right port. | ||
| if [[ -f "${CURRENT_DIR}/.env" ]]; then | ||
| _RESTORED_PORT=$(grep "^PORT=" "${CURRENT_DIR}/.env" 2>/dev/null | cut -d= -f2 | tr -d ' "' || true) |
There was a problem hiding this comment.
Suggestion: The restored release's PORT is parsed from .env using grep/cut/tr without stripping inline comments, so a line like PORT=3000 # app port will produce a malformed HEALTH_URL such as http://127.0.0.1:3000# app port/health, causing the health check to hit the wrong path or fail even when the service is healthy. [logic error]
Severity Level: Major ⚠️
- ❌ Rollback health check can fail on healthy deployments.
- ❌ Telegram alert incorrectly reports rollback health failure.
- ⚠️ Operators may perform unnecessary manual interventions.
- ⚠️ Future automated rollback logic could misinterpret failures.| _RESTORED_PORT=$(grep "^PORT=" "${CURRENT_DIR}/.env" 2>/dev/null | cut -d= -f2 | tr -d ' "' || true) | |
| _RESTORED_PORT=$( | |
| grep -m1 "^PORT=" "${CURRENT_DIR}/.env" 2>/dev/null \ | |
| | cut -d= -f2- \ | |
| | sed 's/[[:space:]]*#.*$//' \ | |
| | tr -d ' "' | |
| || true) |
Steps of Reproduction ✅
1. Confirm rollback entry point: in `package.json:19-20` the npm scripts `"rollback":
"./scripts/rollback.sh"` and `"rollback:list": "./scripts/rollback.sh --list"` show that
operators will trigger `/workspace/Runewager/scripts/rollback.sh` directly in production.
2. Note paths used by rollback: in `scripts/rollback.sh:21-26` the script defines
`BASE_DIR="/var/www/html/Runewager"`, `RELEASES_DIR="${BASE_DIR}/releases"`,
`CURRENT_DIR="${BASE_DIR}/current"`, and initial
`HEALTH_URL="http://127.0.0.1:${PORT}/health"`, so any rollback operates on
`${RELEASES_DIR}/release_*` directories and checks health via this URL.
3. Create or edit a release `.env` with an inline comment on PORT in a release directory,
e.g. `/var/www/html/Runewager/releases/release_123/.env` containing a line `PORT=3000 #
app port`. This is a realistic pattern: `README.md:129-131` instructs operators `cp -n
.env.example .env` then `nano .env`, and `.env.example:11` defines `PORT=3000` without
comments, so admins commonly add comments when customizing.
4. Ensure there are at least two releases so that `scripts/rollback.sh` will copy a
previous release into `CURRENT_DIR` (`scripts/rollback.sh:90-93`), then run a rollback
pointing at the commented-port release: `npm run rollback -- releases/release_123` (which
invokes `./scripts/rollback.sh <release_dir>` as documented at `scripts/rollback.sh:7-8`).
5. During rollback, after copying the target release into `CURRENT_DIR`, the block at
`scripts/rollback.sh:99-109` executes: it reads `${CURRENT_DIR}/.env`, runs `grep "^PORT="
... | cut -d= -f2 | tr -d ' "'`. For the line `PORT=3000 # app port`, `cut` yields `3000 #
app port`, and `tr -d ' "'` strips spaces/quotes but leaves `#` and letters, producing
`_RESTORED_PORT="3000#apput"` (similar malformed value).
6. With `_RESTORED_PORT` non-empty, the script sets `PORT="$_RESTORED_PORT"` and
recomputes `HEALTH_URL="http://127.0.0.1:${PORT}/health"` at
`scripts/rollback.sh:105-106`, resulting in a malformed URL like
`http://127.0.0.1:3000#apput/health` that includes a fragment/garbage in place of a clean
port.
7. The health check loop at `scripts/rollback.sh:123-135` then calls `curl ...
"$HEALTH_URL"`; because the URL is malformed (fragment and junk after the port), curl
either fails to connect properly or hits the wrong effective path, so `HTTP_CODE` remains
non-200 for all attempts even though the bot process (Node health endpoint in
`index.js:1910` and `index.js:4923` which listen on `process.env.PORT || 3000`) is
correctly serving `/health` on port 3000.
8. As a result, the script sets `HEALTH_OK=false` and the block at
`scripts/rollback.sh:137-140` fires, logging `Health check FAILED after rollback. The bot
may be down.` and sending a Telegram notification via `notify()` indicating rollback
health check failure, even though the service is actually healthy and running on the
intended port.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/rollback.sh
**Line:** 103:103
**Comment:**
*Logic Error: The restored release's `PORT` is parsed from `.env` using `grep`/`cut`/`tr` without stripping inline comments, so a line like `PORT=3000 # app port` will produce a malformed `HEALTH_URL` such as `http://127.0.0.1:3000# app port/health`, causing the health check to hit the wrong path or fail even when the service is healthy.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
2 participants
CodeAnt-AI Description
Improve deployment safety, health binding, and Telegram notification robustness
What Changed
Impact
✅ Fewer malformed Telegram notifications✅ Correct health probes after rollback✅ Safer internal metrics exposure💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores