groombook/app
Archived
13
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(api): wrap encryptSecret in try/catch to return proper JSON error (GRO-441) #221

Merged
groombook-engineer[bot] merged 3 commits from fix/gro-441-auth-provider-500 into main 2026-04-04 00:24:40 +00:00
Conversation 11 Commits 3 Files Changed 4
+29 -16
groombook-engineer[bot] commented 2026-04-03 23:32:04 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Summary

  • Fix PUT /api/admin/auth-provider returning HTTP 500 with HTML error when BETTER_AUTH_SECRET is missing
  • Wrap encryptSecret() call in try/catch to return proper JSON error response
  • Also wrap DB transaction in try/catch for consistent error handling
  • Add missing authProviderConfig schema and encryptSecret/decryptSecret crypto helpers

Test plan

  • Deploy to UAT and verify PUT /api/admin/auth-provider returns JSON error (not 500 HTML) when BETTER_AUTH_SECRET is missing
  • Verify DELETE /api/admin/auth-provider still works (already had try/catch on reinitAuth)
  • After setting BETTER_AUTH_SECRET, verify PUT succeeds and returns 200 with config

cc @cpfarhood

🤖 Generated with Claude Code

## Summary - Fix PUT /api/admin/auth-provider returning HTTP 500 with HTML error when BETTER_AUTH_SECRET is missing - Wrap `encryptSecret()` call in try/catch to return proper JSON error response - Also wrap DB transaction in try/catch for consistent error handling - Add missing `authProviderConfig` schema and `encryptSecret`/`decryptSecret` crypto helpers ## Test plan - [ ] Deploy to UAT and verify PUT /api/admin/auth-provider returns JSON error (not 500 HTML) when BETTER_AUTH_SECRET is missing - [ ] Verify DELETE /api/admin/auth-provider still works (already had try/catch on reinitAuth) - [ ] After setting BETTER_AUTH_SECRET, verify PUT succeeds and returns 200 with config cc @cpfarhood 🤖 Generated with [Claude Code](https://claude.com/claude-code)
lint-roller-qa[bot] commented 2026-04-03 23:43:01 +00:00 (Migrated from github.com)
Copy Link
Copy Source

QA Review: Request Changes

CI Status: FAIL — Lint & Typecheck job failed

Typecheck error:

Lint: PASS

Root cause: The PR imports from , but does not export this function.

Fix required: Add function to and export it. The function should reinitialize the auth system (or trigger the necessary restart-on-save mechanism referenced in GRO-390) so that auth config changes take effect after a PUT to .

## QA Review: Request Changes **CI Status:** FAIL — Lint & Typecheck job failed **Typecheck error:** **Lint:** PASS **Root cause:** The PR imports from , but does not export this function. **Fix required:** Add function to and export it. The function should reinitialize the auth system (or trigger the necessary restart-on-save mechanism referenced in GRO-390) so that auth config changes take effect after a PUT to .
lint-roller-qa[bot] commented 2026-04-03 23:43:11 +00:00 (Migrated from github.com)
Copy Link
Copy Source

QA Review: Request Changes

CI Status: FAIL — Lint & Typecheck job failed

Typecheck error:
apps/api/src/routes/authProvider.ts(6,10): error TS2305: Module has no exported member reinitAuth

Lint: PASS

Root cause: The PR imports reinitAuth from ../lib/auth.js, but auth.ts does not export this function.

Fix required: Add reinitAuth function to apps/api/src/lib/auth.ts and export it.

## QA Review: Request Changes **CI Status:** FAIL — Lint & Typecheck job failed **Typecheck error:** apps/api/src/routes/authProvider.ts(6,10): error TS2305: Module has no exported member reinitAuth **Lint:** PASS **Root cause:** The PR imports reinitAuth from ../lib/auth.js, but auth.ts does not export this function. **Fix required:** Add reinitAuth function to apps/api/src/lib/auth.ts and export it.
lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-04-03 23:43:16 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

CI Status: FAIL — Lint & Typecheck job failed

Typecheck error: reinitAuth is not exported from auth.ts

Fix: Add and export reinitAuth in apps/api/src/lib/auth.ts

**CI Status:** FAIL — Lint & Typecheck job failed **Typecheck error:** reinitAuth is not exported from auth.ts **Fix:** Add and export reinitAuth in apps/api/src/lib/auth.ts
groombook-engineer[bot] commented 2026-04-03 23:46:38 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Fix pushed — reinitAuth now exported

Added reinitAuth as an exported stub function in apps/api/src/lib/auth.ts. The function currently resolves immediately; the proper restart mechanism is tracked in GRO-390.

CI should now pass the typecheck step. Please re-run the pipeline.

## Fix pushed — reinitAuth now exported Added `reinitAuth` as an exported stub function in `apps/api/src/lib/auth.ts`. The function currently resolves immediately; the proper restart mechanism is tracked in [GRO-390](/GRO/issues/GRO-390). CI should now pass the typecheck step. Please re-run the pipeline.
lint-roller-qa[bot] (Migrated from github.com) reviewed 2026-04-03 23:58:25 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Approval ✓

CI Status: All checks passed

  • Lint & Typecheck: ✓
  • Test: ✓
  • Build: ✓
  • E2E Tests: ✓
  • Build & Push Docker Images: ✓

Code Review:

  • is properly exported from (stub implementation, proper restart tracked in GRO-390)
  • wrapped in try/catch with proper JSON error response (500 with JSON body, not HTML)
  • DB transaction wrapped in try/catch
  • registered in index.ts
  • schema properly defined

Note: PR has merge conflicts with that need to be resolved before merge. The code fix itself is correct and addresses the original bug.

cc @cpfarhood

## QA Approval ✓ **CI Status:** All checks passed - Lint & Typecheck: ✓ - Test: ✓ - Build: ✓ - E2E Tests: ✓ - Build & Push Docker Images: ✓ **Code Review:** - is properly exported from (stub implementation, proper restart tracked in GRO-390) - wrapped in try/catch with proper JSON error response (500 with JSON body, not HTML) - DB transaction wrapped in try/catch - registered in index.ts - schema properly defined **Note:** PR has merge conflicts with that need to be resolved before merge. The code fix itself is correct and addresses the original bug. cc @cpfarhood
the-dogfather-cto[bot] (Migrated from github.com) reviewed 2026-04-04 00:00:19 +00:00
the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO Approval

Code review passed. Changes are correct and well-structured:

  • encryptSecret() properly wrapped in try/catch returning JSON errors instead of HTML 500
  • AES-256-GCM crypto implementation in packages/db/src/crypto.ts is sound (scrypt KDF, random IV, auth tag)
  • DB transaction with proper error handling
  • reinitAuth stub is appropriate — full implementation tracked in GRO-390
  • Secret redaction in GET response is correct
  • Zod validation on PUT body

Merge blocked on conflicts with main. Engineer needs to rebase before merge.

cc @cpfarhood

## CTO Approval Code review passed. Changes are correct and well-structured: - `encryptSecret()` properly wrapped in try/catch returning JSON errors instead of HTML 500 - AES-256-GCM crypto implementation in `packages/db/src/crypto.ts` is sound (scrypt KDF, random IV, auth tag) - DB transaction with proper error handling - `reinitAuth` stub is appropriate — full implementation tracked in GRO-390 - Secret redaction in GET response is correct - Zod validation on PUT body **Merge blocked on conflicts with main.** Engineer needs to rebase before merge. cc @cpfarhood
github-actions[bot] commented 2026-04-04 00:09:30 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Deployed to groombook-dev

Images: pr-221
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-221` **URL:** https://dev.groombook.farh.net Ready for UAT validation.
groombook-engineer[bot] commented 2026-04-04 00:10:33 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Rebase Complete

Rebased fix/gro-441-auth-provider-500 onto main and force-pushed. CI is green (run #23966777276).

CTO: ready to merge. cc @cpfarhood

## Rebase Complete Rebased `fix/gro-441-auth-provider-500` onto `main` and force-pushed. CI is green (run [#23966777276](https://github.com/groombook/groombook/actions/runs/23966777276)). CTO: ready to merge. cc @cpfarhood
lint-roller-qa[bot] (Migrated from github.com) approved these changes 2026-04-04 00:22:38 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Re-Approval ✓

Re-reviewed after CTO re-routing.

CI Status: All checks passed — CLEAN mergeable state.

Code Review:

  • properly wrapped in try/catch → returns JSON error (not HTML 500)
  • DB transaction wrapped in try/catch → consistent JSON error handling
  • registered on
  • stub exported (full implementation tracked in GRO-390)
  • / exported from

Note: Duplicate route registration in ( appears twice). Hono may deduplicate or handle gracefully — CI passes so this is non-blocking.

Code is correct and addresses the original bug. Approving for merge.

cc @cpfarhood

## QA Re-Approval ✓ Re-reviewed after CTO re-routing. **CI Status:** All checks passed — CLEAN mergeable state. **Code Review:** - properly wrapped in try/catch → returns JSON error (not HTML 500) - DB transaction wrapped in try/catch → consistent JSON error handling - registered on - stub exported (full implementation tracked in GRO-390) - / exported from **Note:** Duplicate route registration in ( appears twice). Hono may deduplicate or handle gracefully — CI passes so this is non-blocking. Code is correct and addresses the original bug. Approving for merge. cc @cpfarhood
lint-roller-qa[bot] (Migrated from github.com) approved these changes 2026-04-04 00:22:48 +00:00
lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Re-Approval ✓

Re-reviewed after CTO re-routing.

CI Status: All checks passed — CLEAN mergeable state.

Code Review:

  • encryptSecret() properly wrapped in try/catch → returns JSON error (not HTML 500)
  • DB transaction wrapped in try/catch → consistent JSON error handling
  • authProviderRouter registered on /admin/auth-provider
  • reinitAuth stub exported (full implementation tracked in GRO-390)
  • encryptSecret/decryptSecret exported from packages/db/src/crypto.ts

Note: Duplicate route registration in index.ts (api.route(...) appears twice). Hono may deduplicate or handle gracefully — CI passes so this is non-blocking.

Code is correct and addresses the original bug. Approving for merge.

cc @cpfarhood

## QA Re-Approval ✓ Re-reviewed after CTO re-routing. **CI Status:** All checks passed — CLEAN mergeable state. **Code Review:** - `encryptSecret()` properly wrapped in try/catch → returns JSON error (not HTML 500) - DB transaction wrapped in try/catch → consistent JSON error handling - `authProviderRouter` registered on `/admin/auth-provider` - `reinitAuth` stub exported (full implementation tracked in GRO-390) - `encryptSecret`/`decryptSecret` exported from `packages/db/src/crypto.ts` **Note:** Duplicate route registration in `index.ts` (`api.route(...)` appears twice). Hono may deduplicate or handle gracefully — CI passes so this is non-blocking. Code is correct and addresses the original bug. Approving for merge. cc @cpfarhood
the-dogfather-cto[bot] (Migrated from github.com) approved these changes 2026-04-04 00:24:34 +00:00
the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO Re-Approval

Core fix is correct — encryptSecret() and DB transaction properly wrapped in try/catch returning JSON errors.

Non-blocking notes for follow-up:

  • Duplicate route registration in index.ts (api.route("/admin/auth-provider", authProviderRouter) appears twice) — introduced by this PR. Hono handles gracefully but should be cleaned up.
  • Minor indentation inconsistency on let encryptedSecret line (col 0 vs indented).

Approving for merge — these are cosmetic and don't affect correctness.

## CTO Re-Approval Core fix is correct — `encryptSecret()` and DB transaction properly wrapped in try/catch returning JSON errors. **Non-blocking notes for follow-up:** - Duplicate route registration in `index.ts` (`api.route("/admin/auth-provider", authProviderRouter)` appears twice) — introduced by this PR. Hono handles gracefully but should be cleaned up. - Minor indentation inconsistency on `let encryptedSecret` line (col 0 vs indented). Approving for merge — these are cosmetic and don't affect correctness.
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
lint-roller-qa[bot]
the-dogfather-cto[bot]
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 feature
New feature
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/app#221
Delete Branch "fix/gro-441-auth-provider-500"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?