[patrickbrandao]

Closes #27

What

• crypto.util.ts: getEncryptionSecret() reads only THINKER_ENCRYPTION_KEY. getDecryptionSecrets() returns [key, PREVIOUS]. BETTER_AUTH_SECRET is no longer read in this file.

• New assertEncryptionKeyConfig() / encryptionKeyConfigProblems() throw a StartupConfigError when:

  • the key is missing;
  • the key is under 32 chars;
  • the key equals BETTER_AUTH_SECRET;
  • THINKER_ENCRYPTION_KEY_PREVIOUS is set but under 32 chars.

  Error messages name the variables, never their values, and include the openssl rand -hex 32 hint.

• main.ts runs the check in bootstrap() before the app and migrations start. A config error exits 1 with Configuration error: ... instead of the migration hint. The legacy-key check now throws the same error type.

• EncryptApiKeys migration and secret-reencryption.service.ts no longer read or mention the fallback.

• docker/docker-compose.yml now requires the key with ${THINKER_ENCRYPTION_KEY:?...}.

• Docs: docker/.env.example, packages/backend/.env.example, DOCKER_README.md (docker run examples now pass the key) and docs/dev/manifest-guide.md.

• CI: the e2e job env and docker-smoke both set a key. Docker-smoke also checks that install.sh writes the key.

• The changeset ships inside 0.1.0 and bumps nothing, like feat(backend)!: rename client-visible identifiers to THINKER_*, thk_ and x-thinker-* #138.

Not done here (out of my scope): docker/install.sh is unchanged, so docker/.env.example keeps the commented # THINKER_ENCRYPTION_KEY= line it rewrites. The issue's switch to an uncommented line and the "--upgrade without the key" abort still belong to devops. The release notes in docs/releases/0.1.0.md (docs-ptbr) also need an entry for this change.

Tests

• Unit tests:
  • the gate in each case (missing, short, equal, short PREVIOUS, valid PREVIOUS, no secret value in any message);
  • no fallback in either function;
  • a known-ciphertext fixture with a rotation through PREVIOUS;
  • the fixture does not decrypt via BETTER_AUTH_SECRET.
• Specs that relied on the fallback now set the key. test/support/unit-env.ts and test/support/e2e-env.ts supply a test key.
• E2E test/encryption-key-required.e2e-spec.ts boots the real src/main.ts with the key missing, short, equal to BETTER_AUTH_SECRET, and with a short PREVIOUS. Each run must exit non-zero with the expected message. It also saves a provider key through the API and checks that it decrypts with the key and not with BETTER_AUTH_SECRET.
• Local results:
  • npm run lint (0 errors), npm run typecheck and npm run build pass for packages/backend;
  • npx jest: 475 suites, 9096 tests passed;
  • TZ=UTC npm run test:e2e:docker: 72 suites, 604 tests passed.

qa-docker is needed (runtime boot behavior changed).

🤖 Generated with Claude Code

[patrickbrandao]

Review of #150: require THINKER_ENCRYPTION_KEY with no BETTER_AUTH_SECRET fallback (#27)

Diff reviewed: git diff origin/main...f32a40f4b8f5e454de307f6ec270beb47b18b767 (29 files).

Summary

• getEncryptionSecret() reads only THINKER_ENCRYPTION_KEY. getDecryptionSecrets() returns the key plus an optional THINKER_ENCRYPTION_KEY_PREVIOUS. BETTER_AUTH_SECRET is never a candidate.
• assertEncryptionKeyConfig runs at boot before the app or its migrations start. It rejects a key that is missing, shorter than 32 characters or equal to BETTER_AUTH_SECRET, and a PREVIOUS value that is set but too short. The error messages never include secret values, and a test checks this.
• StartupConfigError gives operators a clean fatal message in main.ts, and legacy-env.ts now reuses it.
• In migration EncryptApiKeys, only the fallback was removed. The migration destroys no data, and the boot gate guarantees the key is set before it runs.
• Compose uses ${THINKER_ENCRYPTION_KEY:?}. install.sh already generated the key. The smoke and install e2e jobs now check that the key was generated, and the CI e2e env sets it.
• No new outbound destination. No new manifest/MANIFEST_* identifiers. Credentials are not logged.

Tests

Unit coverage in crypto.util.spec.ts: the config check, no fallback, a fixed ciphertext that stays readable, and rotation through PREVIOUS. A new test/encryption-key-required.e2e-spec.ts boots the real process for each refusal case and checks that a provider key is stored under the dedicated key and not under the session secret. Coverage is at the right level.

Findings

should-fix 1: there is no upgrade path for installs that relied on the fallback. In docker/DOCKER_README.md:131-137 and docs/dev/manifest-guide.md, nothing tells an existing operator, including one migrating from Manifest, who never set THINKER_ENCRYPTION_KEY what to do. Failure scenario: on upgrade, Compose refuses to start. The operator generates a fresh key, and every stored provider key and OAuth token silently stops decrypting, because read paths treat a failed decrypt as "not connected". The safe recipe exists: set a new THINKER_ENCRYPTION_KEY, set THINKER_ENCRYPTION_KEY_PREVIOUS=<old BETTER_AUTH_SECRET>, boot, wait for the re-encryption log, then remove PREVIOUS. Fix: document that recipe in the DOCKER_README upgrade section and in the 0.1.0 release notes, and point to it from the boot error message when the key is missing. Recordings written under the old secret stay unreadable, as the README already says.

nit 2: stale comment at docker/install.sh:291-296. It still says "Left unset the backend falls back to BETTER_AUTH_SECRET ... and warns about it on every boot". Update it: the key is now required.

nit 3: the doc comment at packages/backend/src/common/utils/crypto.util.ts:107 and onward could mention that PREVIOUS may legitimately hold the old BETTER_AUTH_SECRET during the upgrade above. This would help the upgrade recipe. Optional.

High-risk criteria that apply

• It changes crypto and key handling: THINKER_ENCRYPTION_KEY and how provider credentials and OAuth tokens are encrypted.
• It changes .github/workflows/ci.yml and .github/workflows/docker-smoke.yml. This is outside the Escopo of CI mínima provisória #38. The edits are small and only add a generated test key, with no change to permissions or secrets usage, but the criterion still applies.

Nothing blocking. qa-docker is required for this runtime change: it should cover boot refusal and a normal boot with both keys set. Should-fix 1 must be fixed or tracked in an issue linked in the PR before merge.

CI on the reviewed SHA: all 19 check runs completed with success (CodeQL, Docker build amd64/arm64, Compose smoke, install.sh e2e, Lint, Typecheck, Build, Unit backend/cli/shared/frontend, Backend e2e UTC and America/Sao_Paulo, Changeset check, Scripts, dependency-review).

Head revisado: f32a40f
Veredito: risco alto (aguardar mantenedor)

[patrickbrandao]

Review should-fix 1 and the nits are tracked in #152.

[patrickbrandao]

qa-docker report for PR #150

Image thinker:qa-q15017227 (arm64, build 1m08s, 429MB). Base images are those named in docker/Dockerfile. Postgres is postgres:16-alpine@sha256:7218...80ea. THINKER_CATALOG_SYNC=false, mock provider only, volumes isolated by override (thinker-qa-q15017227_*), and ports 5432/5433 were not touched.

Step	Result
Build	pass
Boot and health	pass
Check 1a: THINKER_ENCRYPTION_KEY missing	pass: exit 1, "is not set ... required"
Check 1b: key shorter than 32 chars	pass: exit 1, "too short (14 chars)"
Check 1c: key equal to BETTER_AUTH_SECRET	pass: exit 1, "must differ from BETTER_AUTH_SECRET"
Check 1d: no secret value in the messages	pass: grep of the output found 0 matches
Check 2a: provider key stored encrypted	pass: tenant_providers.api_key_encrypted is salt:iv:tag:ct form, no plaintext
Check 2b: proxied request with that key	pass
Check 2c: decrypt with BETTER_AUTH_SECRET	pass: fails with GCM auth error, while THINKER_ENCRYPTION_KEY decrypts it
Check 3: rotation via THINKER_ENCRYPTION_KEY_PREVIOUS	pass: proxy works after rotating, and SecretReencryptionService re-encrypted 1 of 1 under the new key. Without PREVIOUS the old key is unusable ("Failed to decrypt API key" warning), as expected.
Smoke: dashboard HTML, admin setup, agent key, /v1/models	pass
Smoke: /v1/chat/completions, /v1/messages, /v1/responses	pass
Gateway contracts (9 tests)	pass
Backend e2e (test:e2e:docker)	pass on retry: 71/72 suites on the first run, 15 tests failed in proxy.e2e-spec and no-phonehome-egress.e2e-spec with hook timeouts (30s)
e2e rerun of those two suites	pass (37/37)
Teardown	pass: only labeled volumes were removed, none remain

Flaky: the first e2e run took 30 minutes with a host load average above 120, because other QA runs were going at the same time. Both failures were hook timeouts and passed on rerun.

Finding (non-blocking): docker/docker-compose.yml and .env.example do not pass THINKER_ENCRYPTION_KEY_PREVIOUS into the container. Rotation therefore needs a compose override, and I used one. Suggest adding - THINKER_ENCRYPTION_KEY_PREVIOUS=${THINKER_ENCRYPTION_KEY_PREVIOUS:-} to the compose environment. The dashboard <title> still says "Manifest", which is legacy and out of scope.

Tested SHA: f32a40f