Skip to content

使用外部共享 Redis 部署时,若密码包含 /、= 等特殊字符,按 URL 规范只能以百分号编码写入 REDIS_URL(明文会破坏 URL 解析)。当前版本下这样的配置会导致容器启动即崩溃。#1312

Merged
ding113 merged 2 commits into
ding113:devfrom
xx0410:fix/redis-url-userinfo-decode
Jul 6, 2026

Conversation

@xx0410

@xx0410 xx0410 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a container startup crash when REDIS_URL contains percent-encoded userinfo (passwords with /, =, @, etc.) and restores DB-index selection from the URL path for the Bull-backed cleanup and notification queues. Aligns the two queue code paths with the main Redis client (src/lib/redis/client.ts), which already decodes credentials and honors the DB index via ioredis' built-in parseURL.

Related Issues / PRs


问题复现

  1. 配置 REDIS_URL=redis://:pass%2Fword%3D@host:6379/15(密码原文为 pass/word=
  2. 启动服务,日志出现: ERR action=schedule_auto_cleanup_error error=WRONGPASS invalid username-password pair ReplyError: WRONGPASS ... command: { name: 'auth', args: [ 'pass%2Fword%3D' ] } fatal msg=[Lifecycle] unhandledRejection
  3. AUTH 参数里是未解码的 %2F/%3D 原文。
  4. 另外:即使认证通过,URL 中 /15 的 DB 序号也会被忽略,Bull 键全部写入 DB 0。

根因

src/lib/notification/notification-queue.tssrc/lib/log-cleanup/cleanup-queue.tsnew URL() 解析后直接取 url.password/url.username。WHATWG URL 不会自动解码 userinfo,且代码未读取 url.pathname 中的 DB 序号。主客户端(src/lib/redis/client.ts)走 ioredis 内置 parseURL,已正确解码并支持 db,两条路径行为不一致。

改动

两处队列的 URL 解析:decodeURIComponent userinfo + 从 pathname 解析 db 传入 Bull redis options。共 +16/−4 行,无行为面扩张。

文件 变更
src/lib/log-cleanup/cleanup-queue.ts decodeURIComponent userinfo;从 url.pathname 解析 DB 序号
src/lib/notification/notification-queue.ts decodeURIComponent userinfo;从 url.pathname 解析 DB 序号

无新增自动化测试(改动为两处对称的解析逻辑,作者已做实机验证)。

验证

  • 实机复现环境(密码含 /= 的共享 Redis + /15 DB 选择):修复前 AUTH 失败进程崩溃;修复后认证通过、Bull 键落在 DB 15。

Description enhanced by Claude AI

Greptile Summary

Fixes a startup crash on REDIS_URLs with percent-encoded credentials (passwords containing /, =, @, etc.) and adds DB-index selection from the URL path for the two Bull-backed queues. Both queue files previously passed raw url.password/url.username directly to ioredis without decoding, causing AUTH failures; this aligns them with src/lib/redis/client.ts.

  • cleanup-queue.ts and notification-queue.ts: decodeURIComponent is now applied to both url.password and url.username before they are passed to Bull's redis options, and url.pathname is parsed to populate redisQueueOptions.db when a DB index is present (e.g. /15).
  • The changes are symmetric across both files and strictly additive — no control flow, retry, or error-handling logic is altered.

Confidence Score: 5/5

Safe to merge — the two changed files apply a narrow, symmetric fix that closes a real connection failure without touching any logic beyond URL credential parsing.

Both changes are minimal and correct: decodeURIComponent on userinfo fields is the standard fix for WHATWG URL's non-decoding behavior, and reading url.pathname for the DB index matches how the main Redis client already works. Empty-path and non-numeric-path edge cases are handled naturally by the Number.isNaN guard. No control flow, queue behavior, or error-handling paths are altered.

No files require special attention.

Important Files Changed

Filename Overview
src/lib/log-cleanup/cleanup-queue.ts Adds decodeURIComponent for userinfo fields and DB-index parsing from URL path — a targeted, correct fix aligned with how src/lib/redis/client.ts handles the same URL.
src/lib/notification/notification-queue.ts Identical symmetric fix to the cleanup queue — decodes percent-encoded credentials and reads DB index from url.pathname, closing the behavioral gap with the main Redis client.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Env as REDIS_URL env var
    participant URLParser as new URL()
    participant Decode as decodeURIComponent()
    participant BullOpts as redisQueueOptions
    participant Bull as Bull Queue (ioredis)

    Env->>URLParser: "redis://:pass%2Fword%3D@host:6379/15"
    URLParser-->>Decode: "url.password = "pass%2Fword%3D""
    URLParser-->>Decode: "url.username = """
    URLParser-->>BullOpts: "url.pathname = "/15""
    Decode-->>BullOpts: "password = "pass/word=""
    Decode-->>BullOpts: "username = """
    Note over BullOpts: db = parseInt("15", 10) = 15
    BullOpts->>Bull: "{ host, port, password, username, db }"
    Bull-->>Bull: "AUTH pass/word= → OK, SELECT 15"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Env as REDIS_URL env var
    participant URLParser as new URL()
    participant Decode as decodeURIComponent()
    participant BullOpts as redisQueueOptions
    participant Bull as Bull Queue (ioredis)

    Env->>URLParser: "redis://:pass%2Fword%3D@host:6379/15"
    URLParser-->>Decode: "url.password = "pass%2Fword%3D""
    URLParser-->>Decode: "url.username = """
    URLParser-->>BullOpts: "url.pathname = "/15""
    Decode-->>BullOpts: "password = "pass/word=""
    Decode-->>BullOpts: "username = """
    Note over BullOpts: db = parseInt("15", 10) = 15
    BullOpts->>Bull: "{ host, port, password, username, db }"
    Bull-->>Bull: "AUTH pass/word= → OK, SELECT 15"
Loading

Reviews (2): Last reviewed commit: "Merge branch 'dev' into fix/redis-url-us..." | Re-trigger Greptile

 使用外部共享 Redis 部署时,若密码包含 /、= 等特殊字符,按 URL 规范只能以百分号编码写入 REDIS_URL(明文会破坏 URL 解析)。当前版本下这样的配置会导致容器启动即崩溃。

 问题复现

 1. 配置 REDIS_URL=redis://:pass%2Fword%3D@host:6379/15(密码原文为 pass/word=)
 2. 启动服务,日志出现:
 ERR action=schedule_auto_cleanup_error error=WRONGPASS invalid username-password pair
 ReplyError: WRONGPASS ... command: { name: 'auth', args: [ 'pass%2Fword%3D' ] }
 fatal msg=[Lifecycle] unhandledRejection
 2. AUTH 参数里是未解码的 %2F/%3D 原文。
 3. 另外:即使认证通过,URL 中 /15 的 DB 序号也会被忽略,Bull 键全部写入 DB 0。

 根因

 src/lib/notification/notification-queue.ts 与 src/lib/log-cleanup/cleanup-queue.ts 用 new URL() 解析后直接取 url.password/url.username。WHATWG URL 不会自动解码 userinfo,且代码未读取 url.pathname 中的 DB 序号。主客户端(src/lib/redis/client.ts)走 ioredis 内置 parseURL,已正确解码并支持 db,两条路径行为不一致。

 改动

 两处队列的 URL 解析:decodeURIComponent userinfo + 从 pathname 解析 db 传入 Bull redis options。共 +16/−4 行,无行为面扩张。

 验证

 - 实机复现环境(密码含 / 与 = 的共享 Redis + /15 DB 选择):修复前 AUTH 失败进程崩溃;修复后认证通过、Bull 键落在 DB 15。
 - (冒烟测试的具体输出,等本机验证完成后填入)
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

修改了两个Redis队列初始化文件(日志清理队列与通知队列),对REDIS_URL中的用户名和密码增加decodeURIComponent解码处理,并新增从URL pathname解析数据库索引编号并设置redisQueueOptions.db的逻辑。

Changes

Redis URL解析修复

Layer / File(s) Summary
用户名密码解码与DB索引解析
src/lib/log-cleanup/cleanup-queue.ts, src/lib/notification/notification-queue.ts
两处Redis队列配置构建中,url.username/url.password 改为经 decodeURIComponent 解码后赋值,并新增从 url.pathname 解析DB索引、有效时设置 redisQueueOptions.db 的逻辑。

Estimated code review effort: 2 (Simple) | ~10 minutes

Related Issues: 未在提供信息中说明。

Related PRs: 未在提供信息中说明。

Suggested labels: bug, redis

Suggested reviewers: 未在提供信息中说明。

Poem

兔子敲键盘,解码字符不再乱,
密码用户名,各归各位好安然,
路径藏数字,db索引巧提炼,
两处队列同修好,Redis连接更周全。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed 标题准确指向 Redis URL 中特殊字符密码导致启动崩溃的问题,且与实际改动一致。
Description check ✅ Passed 描述详细说明了编码后的 userinfo 解码和 DB 路径解析修复,和变更内容一致。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Jul 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces URL decoding for Redis credentials and supports selecting a database index from the URL path in both the log cleanup and notification queues. The reviewer identified a critical issue where empty username or password strings will cause connection failures on unauthenticated Redis instances due to empty AUTH commands. It is recommended to only assign these options if they are non-empty, and to extract this duplicated parsing logic into a shared utility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/lib/log-cleanup/cleanup-queue.ts
Comment thread src/lib/notification/notification-queue.ts
@github-actions github-actions Bot added the size/XS Extra Small PR (< 50 lines) label Jul 6, 2026
Comment thread src/lib/log-cleanup/cleanup-queue.ts
Comment on lines +193 to +194
redisQueueOptions.password = decodeURIComponent(url.password);
redisQueueOptions.username = decodeURIComponent(url.username); // 传递用户名

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Same decodeURIComponent / malformed-encoding caveat as cleanup-queue

url.password can contain a structurally valid but semantically malformed percent sequence (e.g. %GG) that new URL() accepts but decodeURIComponent rejects with URIError. The outer catch re-throws as "Invalid REDIS_URL format", which is caught further up and only emits a warning — the initialization failure may be non-obvious to operators. Same hardening suggestion as in cleanup-queue.ts.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/notification/notification-queue.ts
Line: 193-194

Comment:
**Same `decodeURIComponent` / malformed-encoding caveat as cleanup-queue**

`url.password` can contain a structurally valid but semantically malformed percent sequence (e.g. `%GG`) that `new URL()` accepts but `decodeURIComponent` rejects with `URIError`. The outer catch re-throws as `"Invalid REDIS_URL format"`, which is caught further up and only emits a warning — the initialization failure may be non-obvious to operators. Same hardening suggestion as in `cleanup-queue.ts`.

How can I resolve this? If you propose a fix, please make it concise.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/lib/log-cleanup/cleanup-queue.ts (1)

49-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

解码与 DB 解析逻辑正确,但与 notification-queue.ts 完全重复。

decodeURIComponent 解码 userinfo、Number.isNaN 守卫下的 pathname DB 解析都正确,且整体位于 try/catch 中,decodeURIComponent 对畸形百分号编码抛出的 URIError 也会被捕获。

这段 Redis URL 解析逻辑与 src/lib/notification/notification-queue.ts(Line 189-199) 几乎逐字重复。建议抽取到一个共享工具函数(例如 parseRedisUrlToOptions(redisUrl, useTls)),避免两处将来出现分叉。

♻️ 建议抽取共享辅助函数
// src/lib/redis/parse-redis-url.ts
export function parseRedisUrlToOptions(
  redisUrl: string,
  useTls: boolean
): Queue.QueueOptions["redis"] {
  const options: Queue.QueueOptions["redis"] = {};
  const url = new URL(redisUrl);
  options.host = url.hostname;
  options.port = parseInt(url.port || "6379", 10);
  options.password = decodeURIComponent(url.password);
  options.username = decodeURIComponent(url.username);
  const dbFromPath = parseInt(url.pathname.slice(1), 10);
  if (!Number.isNaN(dbFromPath)) {
    options.db = dbFromPath;
  }
  if (useTls) {
    const rejectUnauthorized = process.env.REDIS_TLS_REJECT_UNAUTHORIZED !== "false";
    options.tls = {
      host: url.hostname,
      servername: url.hostname,
      rejectUnauthorized,
    };
  }
  return options;
}
🤖 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 `@src/lib/log-cleanup/cleanup-queue.ts` around lines 49 - 56, The Redis URL
parsing in cleanup-queue is duplicated in notification-queue, so extract the
shared logic into a reusable helper such as parseRedisUrlToOptions(redisUrl,
useTls) and have both call sites use it. Move the existing WHATWG URL handling,
decodeURIComponent userinfo decoding, DB parsing with Number.isNaN guard, and
TLS option setup into the shared function so the behavior stays identical while
avoiding future drift.
🤖 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.

Nitpick comments:
In `@src/lib/log-cleanup/cleanup-queue.ts`:
- Around line 49-56: The Redis URL parsing in cleanup-queue is duplicated in
notification-queue, so extract the shared logic into a reusable helper such as
parseRedisUrlToOptions(redisUrl, useTls) and have both call sites use it. Move
the existing WHATWG URL handling, decodeURIComponent userinfo decoding, DB
parsing with Number.isNaN guard, and TLS option setup into the shared function
so the behavior stays identical while avoiding future drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 70496fb3-b366-464b-b94e-8f3169503078

📥 Commits

Reviewing files that changed from the base of the PR and between 92ccb52 and b8d4428.

📒 Files selected for processing (2)
  • src/lib/log-cleanup/cleanup-queue.ts
  • src/lib/notification/notification-queue.ts

@ding113
ding113 changed the base branch from main to dev July 6, 2026 08:10
Comment thread src/lib/log-cleanup/cleanup-queue.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Clean, correct, well-commented fix for a real production crash. The Bull queues in cleanup-queue.ts and notification-queue.ts read url.password/url.username raw (the WHATWG URL API does not percent-decode userinfo) and ignored the /N db index, so a REDIS_URL whose password contains /, =, etc. — only representable via percent-encoding — failed AUTH and crashed the process, and the db index was always dropped. The change aligns these two queue paths with the main client (src/lib/redis/client.ts, which hands the raw URL to ioredis' built-in parseURL). No correctness, security, type, or error-handling defects were found in the new code. Two lower-severity items below.

PR Size: XS

  • Lines changed: 20 (+16 / -4)
  • Files changed: 2

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 1 0
Simplification 0 0 0 1

Critical Issues (Must Fix)

None.

High Priority Issues (Should Fix)

None.

Medium / Low Findings

  • [Tests — Medium] New decodeURIComponent + db-selection logic has no unit coverage, and the same ~14-line parse block is duplicated verbatim in both queue files (this PR had to apply the same fix twice). Recommend extracting a pure buildBullRedisOptions(redisUrl) helper and testing it — this kills both birds. See the inline comment on src/lib/log-cleanup/cleanup-queue.ts. CLAUDE.md requires >=80% coverage for new features, and tests/unit/lib/redis/client.test.ts already tests analogous URL handling for the main client.
  • [Process — Low] CLAUDE.md states PR Target Branch: dev (all pull requests must target the dev branch), but this PR targets main. Worth confirming whether it should be retargeted to dev.

Notes

  • decodeURIComponent can throw URIError on a malformed % sequence, but the new lines sit inside the existing try/catch (cleanup-queue.ts:61 / notification-queue.ts:204), which logs and re-throws Invalid REDIS_URL format — so this is not a silent/uncaught failure. No change needed.
  • Pre-merge, run the CLAUDE.md checklist: bun run build && bun run lint && bun run typecheck && bun run test.

Review Coverage

  • Logic and correctness
  • Security (OWASP Top 10)
  • Error handling
  • Type safety
  • Documentation accuracy
  • Test coverage
  • Code clarity

Automated review by Claude AI

@ding113
ding113 merged commit 2eb21b3 into ding113:dev Jul 6, 2026
2 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Jul 6, 2026
@ding113 ding113 mentioned this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/XS Extra Small PR (< 50 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants