feat: better auth by rebelchris · Pull Request #3685 · dailydotdev/daily-api

rebelchris · 2026-03-11T09:02:21Z

No description provided.

…ter-auth

src/betterAuthBridge.ts

src/routes/betterAuth.ts

rebelchris · 2026-03-11T09:13:47Z

@claude see suggestions above and apply, make sure lint and build are green.

claude · 2026-03-11T09:14:01Z

Claude finished @rebelchris's task in 5m 19s —— View job

Security Fixes Applied

Read src/betterAuthBridge.ts and src/routes/betterAuth.ts
Fix clear-text logging of sensitive env variables (lines 79, 110 in betterAuthBridge.ts)
Fix polynomial regex on uncontrolled data (line 315 in betterAuth.ts)
Add rate limiting to wildcard route handler (line 721 in betterAuth.ts)
Remove debug logging that exposes OTP values
Changes pushed to branch

Summary of changes

src/betterAuthBridge.ts

Removed userId from req.log.warn and req.log.error calls to avoid clear-text logging of sensitive identifiers (CodeQL fix: personalize by publication or by tag feeds #43, feat: push updates to webflow every day #44)

src/routes/betterAuth.ts

Replaced the polynomial-risk email regex (/^[^\s@]+@[^\s@]+\.[^\s@]+$/) with safe string index-based checks that can't backtrack (CodeQL feat: add stackdriver trace agent #50)
Applied rate limiting universally to all /auth/* paths (using strictAuthRateLimiter for sign-up, authRateLimiter for everything else), instead of only sign-up/sign-in paths (CodeQL chore(deps): resolve auditing issues #49)
Removed debug logging blocks that exposed OTP values, verification codes, and full response bodies in plaintext — these were not flagged by the scanner but posed a clear security risk

Note: CodeQL alerts for lines 300, 366, 427, 476 (routes that already call enforceRateLimit) appear to be false positives — the rate limiting was already correctly applied inline in each handler. The scanner likely doesn't trace through the custom helper function.

Lint and build will be verified by CI on the pushed commit.
|

pulumi · 2026-03-11T09:14:46Z

🍹 The Update (preview) for dailydotdev/api/prod (at cdb7ba5) was successful.

✨ Neo Explanation

This is a standard application version rollout deploying a new build across all API services, with accompanying DB and ClickHouse schema migrations. It also introduces a new scheduled job to clean up expired Better Auth sessions.

Root Cause Analysis

A new version of the API application has been built and is being deployed. All services are being updated to run the new container image, and two new one-off migration jobs are being created to upgrade the database and ClickHouse schemas for the new version. Additionally, a brand-new CronJob (clean-expired-better-auth-sessions) is being introduced, indicating the addition of a session cleanup routine for the Better Auth integration.

Dependency Chain

The new container image version cascades uniformly across the entire workload fleet:

7 Deployments (API, background, private, websocket, worker, temporal, personalized digest) are all updated to the new image
34 CronJobs are updated to run the new image on their next scheduled execution
2 old migration Jobs (DB + ClickHouse) tied to the previous version are deleted, and 2 new migration Jobs are created to run schema migrations against the new version before traffic switches over
1 new CronJob (clean-expired-better-auth-sessions) is created from scratch to periodically purge expired authentication sessions

Risk Analysis

No stateful resources (databases, storage buckets, persistent volumes) are being replaced or deleted. The migration jobs are ephemeral one-shot Kubernetes Jobs, which is the standard pattern here. The rolling update strategy on Deployments prevents downtime during the image swap.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tag-recommendations-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-update-views-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
+   vpc-native-api-db-migration-7ec47bfc                       kubernetes:batch/v1:Job        create
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
+   vpc-native-clean-expired-better-auth-sessions-cron         kubernetes:batch/v1:CronJob    create
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-user-posts-analytics-refresh-cron               kubernetes:batch/v1:CronJob    update
+   vpc-native-api-clickhouse-migration-7ec47bfc               kubernetes:batch/v1:Job        create
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-private-deployment                              kubernetes:apps/v1:Deployment  update
-   vpc-native-api-clickhouse-migration-141ec448               kubernetes:batch/v1:Job        delete
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
-   vpc-native-api-db-migration-141ec448                       kubernetes:batch/v1:Job        delete
~   vpc-native-agents-digest-cron                              kubernetes:batch/v1:CronJob    update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
... and 6 other changes

- Remove userId from log calls in betterAuthBridge.ts to prevent clear-text logging of sensitive identifiers (CodeQL 43, 44) - Replace backtracking-prone email regex with safe string checks to prevent ReDoS vulnerability (CodeQL 50) - Apply rate limiting to all paths in wildcard /auth/* handler not just sign-up/sign-in (CodeQL 49) - Remove debug logging that exposed OTP values and verification data in plain text Co-authored-by: Chris Bongers <rebelchris@users.noreply.github.com>

src/routes/betterAuth.ts

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

rebelchris · 2026-03-11T09:37:08Z

@claude which of your concerns/improvements are still valid?

claude · 2026-03-11T09:37:21Z

Claude finished @rebelchris's task in 1m 13s —— View job

Status Review of Previous Concerns

Read current state of src/routes/betterAuth.ts
Read current state of src/betterAuthBridge.ts
Identify which issues remain vs. were resolved

✅ Resolved

CodeQL Alert	Issue	Status
#43 / #44	Clear-text logging of env variables in `betterAuthBridge.ts`	Fixed — no env values are logged anymore
#50	Polynomial regex on user input	Fixed — replaced with index-based string checks at line 315
#49 / #51	Missing rate limit on wildcard `/auth/*` handler	Fixed — `enforceRateLimit` is called at line 538; scanner alerts are false positives (CodeQL doesn't trace through the helper)
#45–#48	Missing rate limit on `/auth/check-email`, `/auth/change-email`, `/auth/verify-change-email`, `/auth/set-password`	Fixed — each calls `enforceRateLimit` in its first line; also false positives
OTP debug logging	Exposed OTP/verification codes in logs	Fixed — no such logging remains

⚠️ Still Valid

Social redirect routes lack rate limiting — the two routes below delegate to socialRedirectHandler, which makes an outbound OAuth HTTP request but never calls enforceRateLimit:

// src/routes/betterAuth.ts:522-528
fastify.get('/auth/sign-in/social', async (request, reply) =>
  socialRedirectHandler(request, reply, 'social sign-in redirect'),
);

fastify.get('/auth/link-social', async (request, reply) =>
  socialRedirectHandler(request, reply, 'social link redirect'),
);

These are registered as specific routes that take precedence over the wildcard /auth/*, so the wildcard's rate limiter does not cover them. An attacker could hammer these endpoints to trigger many OAuth redirects with no throttle.

Fix: Add enforceRateLimit at the top of socialRedirectHandler, or inline it in each handler before delegating.

Fix this →

The remaining CodeQL alerts (#45–#51) are all false positives — rate limiting is correctly applied via the enforceRateLimit helper throughout. The only genuine gap is the social redirect handlers.

rebelchris · 2026-03-11T10:03:46Z

@claude fix this issue about ratelimitting it should apply it.

claude · 2026-03-11T10:04:00Z

Claude finished @rebelchris's task in 44s —— View job

Rate Limiting Fix for Social Redirect Routes

Analyze the issue
Read src/routes/betterAuth.ts to find socialRedirectHandler
Add enforceRateLimit to socialRedirectHandler
Commit and push

Summary

Added enforceRateLimit(request, reply, authRateLimiter) at the top of the try block in socialRedirectHandler (src/routes/betterAuth.ts:483).

The two routes /auth/sign-in/social and /auth/link-social are registered as specific routes that take precedence over the wildcard /auth/* handler, so they bypassed the wildcard's rate limiter. The fix adds rate limiting directly inside socialRedirectHandler so both routes are now protected.
·

Co-authored-by: Chris Bongers <rebelchris@users.noreply.github.com>

capJavert · 2026-03-11T10:21:36Z

src/cookies.ts

+  betterAuthSession: {
+    key: 'better-auth.session_token',


is this naming theirs or? thinking could be named generic but not blocking

It's ours, but guess it needs to be unique to not collide with Kratos for example, think this is fine no?

think I would not name it better auth, its clunky to name 3party products in first party cookies, but yeah its not crucial or if it adds a lot of stuff for you to retest

src/routes/boot.ts

idoshamun · 2026-03-11T13:26:13Z

.infra/Pulumi.adhoc.yaml

    employmentAgreementBucketName: adhoc-daily-api
    brokkrOrigin: http://brokkr-grpc:50051
    mockExternalServices: true
+    betterAuthSecret:


idoshamun · 2026-03-11T13:28:42Z

__mocks__/better-auth-api.ts

@@ -0,0 +1,3 @@
+export const createAuthMiddleware = (


why we have so many new mock files instead of just one with all the helpers?

I think because of how they import they prefer file based overwrites I've had this issue before with feature ones.

idoshamun · 2026-03-11T13:29:58Z

src/common/betterAuth.ts

@@ -0,0 +1,100 @@
+import type { FastifyReply, FastifyRequest } from 'fastify';
+
+export const getClientIp = (request: FastifyRequest): string =>


dont we have this helper?

We actually don't have this with x-forwarded-for test

src/common/betterAuth.ts

idoshamun · 2026-03-11T13:31:20Z

src/common/betterAuth.ts

@@ -0,0 +1,100 @@
+import type { FastifyReply, FastifyRequest } from 'fastify';


I'd just double check we're not introducing existing utilities here

idoshamun · 2026-03-11T13:33:56Z

src/routes/betterAuth.ts

+const isSignUpEmailPath = (request: FastifyRequest): boolean =>
+  request.method === 'POST' && request.url.includes('/auth/sign-up/email');
+
+const isSignInEmailPath = (request: FastifyRequest): boolean =>


doesn't ba should take care of this?

It's for global rate limitting and turnstile though

Which part do you mean exactly?

rebelchris added 9 commits March 6, 2026 15:11

feat: better auth

4d5cd0a

Merge branch 'main' of github.com:dailydotdev/daily-api into feat-bet…

500fb88

…ter-auth

fix: auth secrets

32178cf

fix: version 1.5

4e35c46

fix: turnstile

692e3e5

fix: security and tests

6137ec4

fix: some more fixes for linking

5780cc8

Merge branch 'main' of github.com:dailydotdev/daily-api into feat-bet…

e222b9f

…ter-auth

fix: add mailing

54336ff

rebelchris requested review from capJavert and idoshamun March 11, 2026 09:02

github-advanced-security bot found potential problems Mar 11, 2026

View reviewed changes

Merge branch 'main' into feat-better-auth

e7eb36f

github-advanced-security bot found potential problems Mar 11, 2026

View reviewed changes

src/routes/betterAuth.ts Dismissed Show dismissed Hide dismissed

Potential fix for code scanning alert no. 51: Missing rate limiting

d4fff85

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

fix: add rate limiting to social redirect handler

65009cf

Co-authored-by: Chris Bongers <rebelchris@users.noreply.github.com>

capJavert reviewed Mar 11, 2026

View reviewed changes

src/routes/boot.ts Show resolved Hide resolved

rebelchris added 4 commits March 11, 2026 13:47

fix: feedback PR

4b88500

fix: more cleanup

2816519

Merge branch 'main' into feat-better-auth

c7bf89f

fix: tests

bb47aac

idoshamun reviewed Mar 11, 2026

View reviewed changes

src/common/betterAuth.ts Show resolved Hide resolved

idoshamun reviewed Mar 11, 2026

View reviewed changes

rebelchris added 2 commits March 11, 2026 15:38

fix: feedback

abdf09f

fix: feedback

cdb7ba5

		@@ -0,0 +1,100 @@
		import type { FastifyReply, FastifyRequest } from 'fastify';

		export const getClientIp = (request: FastifyRequest): string =>

Conversation

rebelchris commented Mar 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rebelchris commented Mar 11, 2026

Uh oh!

claude bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Fixes Applied

Summary of changes

Uh oh!

pulumi bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Neo Explanation

Root Cause Analysis

Dependency Chain

Risk Analysis

Resource Changes

Uh oh!

Uh oh!

rebelchris commented Mar 11, 2026

Uh oh!

claude bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status Review of Previous Concerns

✅ Resolved

⚠️ Still Valid

Uh oh!

rebelchris commented Mar 11, 2026

Uh oh!

claude bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate Limiting Fix for Social Redirect Routes

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude bot commented Mar 11, 2026 •

edited

Loading

pulumi bot commented Mar 11, 2026 •

edited

Loading

claude bot commented Mar 11, 2026 •

edited

Loading

claude bot commented Mar 11, 2026 •

edited

Loading