Skip to content

fix: normalize default SOURCEBOT_ENCRYPTION_KEY to 32 characters#1311

Closed
brendan-kellam wants to merge 3 commits into
mainfrom
brendan/fix-encryption-key
Closed

fix: normalize default SOURCEBOT_ENCRYPTION_KEY to 32 characters#1311
brendan-kellam wants to merge 3 commits into
mainfrom
brendan/fix-encryption-key

Conversation

@brendan-kellam

@brendan-kellam brendan-kellam commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

The default SOURCEBOT_ENCRYPTION_KEY shipped in docker-compose.yml is 33 zeros, but the env validation added in #1305 requires the key to be exactly 32 characters (a 256-bit AES key). This causes startup to fail with the default value.

This is a hacky workaround: the all-zeros default (33 chars) is coerced to 32 chars inside encrypt() and decrypt() in packages/shared/src/crypto.ts, where the key is actually used for AES-256. SOURCEBOT_ENCRYPTION_KEY is now a plain string in the env schema (the strict .length(32) check is removed so the default value loads). Any other key value is used unchanged.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed encryption key validation so the default environment value is normalized to the required length at startup.
    • Updated encryption/decryption to gracefully handle a legacy/incorrect 33-character key format by coercing it into a valid 32-character key before deriving cryptographic keys.

brendan-kellam and others added 2 commits June 16, 2026 19:13
The default SOURCEBOT_ENCRYPTION_KEY in docker-compose is 33 zeros, which
fails the 32-character (AES-256) length validation. Preprocess the value so
the all-zeros default is trimmed to 32 characters before validation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

The PR handles legacy SOURCEBOT_ENCRYPTION_KEY configuration where the default value is 33 zeros instead of the required 32. A new coerceEncryptionKey helper in crypto.ts normalizes this value at runtime in encrypt and decrypt operations. The environment schema is relaxed to accept any string, delegating validation to the crypto layer. A changelog entry documents the fix.

Changes

Encryption Key Normalization

Layer / File(s) Summary
Encryption key coercion in encrypt and decrypt
packages/shared/src/crypto.ts
A coerceEncryptionKey helper converts the legacy 33-zero key to 32 zeros; encrypt and decrypt use the coerced value for AES key derivation.
Environment validation relaxation and documentation
packages/shared/src/env.server.ts, CHANGELOG.md
SOURCEBOT_ENCRYPTION_KEY schema changed from z.string().length(32) to z.string() with updated comment; changelog Fixed entry documents the 33-to-32 character normalization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: normalizing the default SOURCEBOT_ENCRYPTION_KEY from 33 to 32 characters to fix a validation issue.
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.

✏️ 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 brendan/fix-encryption-key

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@packages/shared/src/env.server.ts`:
- Around line 344-355: The SOURCEBOT_ENCRYPTION_KEY preprocessing silently
normalizes the malformed 33-zero key pattern into a valid but cryptographically
weak 32-zero key without alerting the operator. Instead of silently accepting
this weak key, modify the preprocessing function to detect when this pattern is
provided and emit a warning or error message to inform the operator that they
are using a predictable all-zero encryption key and should rotate it
immediately. This ensures security implications are not hidden from deployments
using this migration path.
🪄 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: 8f1a140d-f0f8-4867-9fbd-2eca574d762d

📥 Commits

Reviewing files that changed from the base of the PR and between e30e75e and 1ba8c53.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • packages/shared/src/env.server.ts

Comment thread packages/shared/src/env.server.ts Outdated
Comment on lines +344 to +355
SOURCEBOT_ENCRYPTION_KEY: z.preprocess(
// @hack in our docker-compose.yml, we mistakenly used a
// encryption key with _33_ zeros. As a hacky mechanism to
// fix peoples deployments without requiring them to update
// their encryption key, we look for keys with this pattern
// and coerce them into _32_ zeros.
// @see https://github.com/sourcebot-dev/sourcebot/commit/e30e75e7af96308b3b063bb3aed8369f5b15aa2e
(value) => value === "0".repeat(33) ? "0".repeat(32) : value,
z.string().length(32, {
message: "SOURCEBOT_ENCRYPTION_KEY must be exactly 32 characters (a 256-bit AES key). Generate one with `openssl rand -base64 24`.",
}),
),

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid silently accepting the known weak all-zero encryption key.

This normalization fixes startup, but it also preserves a predictable key ("0".repeat(32)) for deployments that never rotate it, weakening encryption-at-rest guarantees.

Suggested mitigation
         SOURCEBOT_ENCRYPTION_KEY: z.preprocess(
             // `@hack` in our docker-compose.yml, we mistakenly used a
             // encryption key with _33_ zeros. As a hacky mechanism to
             // fix peoples deployments without requiring them to update
             // their encryption key, we look for keys with this pattern
             // and coerce them into _32_ zeros.
             // `@see` https://github.com/sourcebot-dev/sourcebot/commit/e30e75e7af96308b3b063bb3aed8369f5b15aa2e
-            (value) => value === "0".repeat(33) ? "0".repeat(32) : value,
+            (value) => {
+                if (value === "0".repeat(33)) {
+                    console.warn(
+                        "SOURCEBOT_ENCRYPTION_KEY default placeholder detected and normalized. Rotate to a unique 32-character key immediately.",
+                    );
+                    return "0".repeat(32);
+                }
+                return value;
+            },
             z.string().length(32, {
                 message: "SOURCEBOT_ENCRYPTION_KEY must be exactly 32 characters (a 256-bit AES key). Generate one with `openssl rand -base64 24`.",
             }),
         ),

Based on learnings from provided context, packages/shared/src/crypto.ts:18-47 consumes this env var directly as AES/HMAC key material, so predictable defaults materially reduce secrecy.

🤖 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 `@packages/shared/src/env.server.ts` around lines 344 - 355, The
SOURCEBOT_ENCRYPTION_KEY preprocessing silently normalizes the malformed 33-zero
key pattern into a valid but cryptographically weak 32-zero key without alerting
the operator. Instead of silently accepting this weak key, modify the
preprocessing function to detect when this pattern is provided and emit a
warning or error message to inform the operator that they are using a
predictable all-zero encryption key and should rotate it immediately. This
ensures security implications are not hidden from deployments using this
migration path.

Move the 33-zeros -> 32-zeros default key normalization out of the env
schema and into encrypt()/decrypt() in crypto.ts, where the key is
actually used. SOURCEBOT_ENCRYPTION_KEY is now a plain string in the env
schema.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant