Skip to content

fix: 加密密钥长度校验 + AI 标签同步丢失 (#184)#187

Merged
AmintaCCCP merged 3 commits into
mainfrom
fix/184-encryption-key-and-ai-tags-sync
Jun 2, 2026
Merged

fix: 加密密钥长度校验 + AI 标签同步丢失 (#184)#187
AmintaCCCP merged 3 commits into
mainfrom
fix/184-encryption-key-and-ai-tags-sync

Conversation

@AmintaCCCP

@AmintaCCCP AmintaCCCP commented Jun 1, 2026

Copy link
Copy Markdown
Owner

问题

Closes #184
Related: #166

问题 1:AI 配置同步返回 422
用户设置 ENCRYPTION_KEY 为 32 字符 hex(如 openssl rand -hex 16),Buffer.from(key, hex) 只解出 16 字节,AES-256 要求 32 字节 → 抛 Invalid key length → AI config 加密失败 → 422。

问题 2:AI 标签同步后消失
PUT /api/repositoriesINSERT OR REPLACE 整行替换,前端推送空 ai_tags[])会覆盖后端已有的 AI 标签,导致用户手动恢复的标签在下一次同步后凭空消失。#166 中报告的「同步后自动分类消失」也是同一根因。

修复方案

Fix 1:加密密钥自动规范化 — server/src/config.ts

新增 normalizeEncryptionKey() 函数,对所有来源的 key 做规范化:

输入场景 处理方式 向后兼容
64 字符 hex(正确格式) 原样返回 ✅ 零影响
短 hex(如 32 字符) SHA-256 派生为 32 字节 ⚠️ 日志告警
非 hex(base64、纯文本) SHA-256 派生为 32 字节 ⚠️ 日志告警
超长 hex SHA-256 派生

规范化后的 key 会回写到 .encryption-key 文件,确保后续启动即使无规范化代码也能正常使用。

Fix 2:AI Tags 同步丢失 — server/src/routes/repositories.ts

改为 INSERT ... ON CONFLICT(id) DO UPDATE SET ...,对 AI 相关字段做条件更新:

ai_tags = CASE WHEN excluded.ai_tags IS NOT NULL AND excluded.ai_tags != '[]'
          THEN excluded.ai_tags ELSE repositories.ai_tags END

仅当推送的值非空时才更新,空值保留后端已有数据。受影响字段:ai_summaryai_tagsai_platformscustom_descriptioncustom_tagscustom_categoryanalyzed_atlast_edited

测试

  • 新增 server/tests/services/config.test.ts(10 个用例),覆盖所有 key 格式场景 + 边界条件
  • 全部现有 crypto 测试通过
  • 前后端 TypeScript 类型检查通过

Summary by CodeRabbit

  • Bug Fixes

    • Repository batch updates now preserve more existing field values, preventing unintended data loss when updating records without providing all fields.
  • Improvements

    • Enhanced encryption key handling with improved validation and normalization for increased consistency and robustness.
  • Tests

    • Added comprehensive test coverage for encryption key normalization functionality.

- Add normalizeEncryptionKey() to handle non-standard ENCRYPTION_KEY formats
  (short hex, base64, plain text) via SHA-256 derivation, passing through
  valid 64-char hex keys unchanged for backward compatibility (#184)
- Change PUT /api/repositories from INSERT OR REPLACE to ON CONFLICT DO UPDATE
  with conditional updates for AI metadata fields (ai_tags, ai_summary,
  ai_platforms, custom_tags, etc.) — empty values no longer overwrite existing
  backend data, preventing AI tags from disappearing after sync
- Add unit tests for normalizeEncryptionKey (8 cases)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f051e86c-c0c9-4d54-87eb-212486874e6c

📥 Commits

Reviewing files that changed from the base of the PR and between c5a24c6 and 064c8da.

📒 Files selected for processing (2)
  • server/src/config.ts
  • server/src/routes/repositories.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • server/src/routes/repositories.ts
  • server/src/config.ts

📝 Walkthrough

Walkthrough

This PR normalizes encryption keys into deterministic 64-hex AES-256-ready strings (applied to env and file keys, with file rewrite when changed) and changes the repository bulk upsert to use ON CONFLICT(id) DO UPDATE that preserves existing nullable/empty metadata fields.

Changes

Encryption Key Normalization

Layer / File(s) Summary
Key normalization implementation and integration
server/src/config.ts
Exports normalizeEncryptionKey (trim, accept exact 64-hex, otherwise SHA-256 derive) and applies it in resolveEncryptionKey for both process.env.ENCRYPTION_KEY and data/.encryption-key, rewriting the file with 0o600 when normalization changes the stored value.
Key normalization test suite
server/tests/services/config.test.ts
Vitest suite validating pass-through for valid 64-char hex, SHA-256 derivation for other inputs, trimming, long-hex handling, empty-string determinism, and that normalized values decode to 32-byte Buffers.

Repository Bulk Upsert Preservation

Layer / File(s) Summary
SQL conflict resolution with conditional field retention
server/src/routes/repositories.ts
Replaces INSERT OR REPLACE with INSERT ... ON CONFLICT(id) DO UPDATE and uses conditional logic to retain existing text/JSON/timestamp fields when incoming excluded values are NULL, empty, or JSON '[]', while overwriting flags/counters and other columns from excluded.

Sequence Diagram(s)

sequenceDiagram
  participant Env as process.env.ENCRYPTION_KEY
  participant File as data/.encryption-key
  participant Normalizer as normalizeEncryptionKey
  participant FS as Filesystem

  Env->>Normalizer: pass raw env value
  File->>Normalizer: pass raw file contents
  Normalizer->>Normalizer: trim, hex-check, or SHA-256 derive
  Normalizer-->>Env: return normalized key
  alt file normalized != original
    Normalizer->>FS: write normalized key (mode 0o600)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I nudge each key to sixty-four hex bright,
Trim whitespace, hash where lengths aren't right.
Files get rewritten with permissions tight,
Upserts keep metadata intact at night.
A rabbit hums: secure bytes, take flight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title mixes two distinct fixes (encryption key validation and AI tags sync loss) in Chinese without clear separation, making it harder to scan for the primary focus. Consider splitting into more focused titles or clarifying which is the primary fix, or make the combined title clearer in English for broader team visibility.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Code changes directly address both #184 requirements: normalizeEncryptionKey() ensures AES-256 32-byte compliance, and ON CONFLICT DO UPDATE preserves AI fields when incoming values are null/empty.
Out of Scope Changes check ✅ Passed All changes are scoped to addressing #184: encryption key normalization and AI tag sync preservation in repositories endpoint.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/184-encryption-key-and-ai-tags-sync

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 and usage tips.

- Change too-long hex handling from truncation to SHA-256 derivation
  for consistent behavior across all non-standard key formats
- Write normalized key back to .encryption-key file so future startups
  use the correct format even without normalization code
- Add edge case tests: empty string, 64-char non-hex, SHA-256 derivation
  for too-long hex (10 total tests, all passing)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add JSDoc comments to resolveDataDir, resolveEncryptionKey, loadConfig,
parseJsonColumn, and transformRepo functions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AmintaCCCP AmintaCCCP merged commit deb2848 into main Jun 2, 2026
5 checks passed
@AmintaCCCP AmintaCCCP deleted the fix/184-encryption-key-and-ai-tags-sync branch June 2, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: 后端同步时 AI 配置因加密失败(Invalid key length)导致上传失败并返回 422 错误

1 participant