Skip to content

fix(security): harden public auth rate limits - #6997

Merged
TheodoreSpeaks merged 4 commits into
stagingfrom
fix/share-link-guardrails
Aug 23, 2026
Merged

fix(security): harden public auth rate limits#6997
TheodoreSpeaks merged 4 commits into
stagingfrom
fix/share-link-guardrails

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • resolve client IPs from proxy-appended forwarding chains instead of trusting client-controlled prefixes
  • add shared resource-level throttles for public chat and file authentication while keeping per-client limits
  • enforce stronger new share passwords and use the trusted resolver for audit attribution and public abuse controls

Type of Change

  • Bug fix

Testing

  • bun run lint
  • bun run apps/sim/scripts/check-block-registry.ts origin/staging
  • bun run check:audits
  • 199 focused Vitest tests across app, security, and audit packages
  • app, security, audit, and testing package type checks

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@TheodoreSpeaks
TheodoreSpeaks requested a review from a team as a code owner August 22, 2026 23:30
@gitguardian

gitguardian Bot commented Aug 22, 2026

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 23, 2026 12:01am

Request Review

@cursor

cursor Bot commented Aug 22, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches client-IP attribution, public auth rate limits, and share password policy. Misconfigured AUTH_TRUSTED_PROXIES or unresolved IPs can 429 legitimate traffic or weaken throttling.

Overview
Hardens public authentication against spoofed X-Forwarded-For values and distributed guessing.

Client IPs are now resolved by walking the forwarded chain right-to-left and skipping only AUTH_TRUSTED_PROXIES. getClientIp returns null instead of 'unknown'. Unauthenticated IP buckets fail closed unless the route declares an independent backstop (recipient, resource, or enrollment). Chat/file password, OTP, and SSO flows add per-resource limits (failClosed) and skip the IP bucket when the client cannot be identified.

New chat and file share passwords must be at least 15 characters; login still accepts legacy short secrets. Audit logs and webhook IP allow-lists use the same resolver (no x-real-ip fallback).

Reviewed by Cursor Bugbot for commit 2fd9f2c. Bugbot is set up for automated code reviews on this repo. Configure here.

@TheodoreSpeaks
TheodoreSpeaks force-pushed the fix/share-link-guardrails branch from 4906c7e to 5bf1838 Compare August 22, 2026 23:32
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@TheodoreSpeaks
TheodoreSpeaks force-pushed the fix/share-link-guardrails branch from 5bf1838 to 40d573d Compare August 22, 2026 23:36
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@greptile-apps

greptile-apps Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR hardens public authentication and abuse controls by resolving client addresses through trusted proxy chains, failing closed where no independent throttle exists, and adding resource-level limits alongside client-level limits.

  • Adds shared chat, file-share, enrollment, and recipient backstops while preserving per-client throttling.
  • Strengthens newly created chat and file-share passwords without rejecting legacy passwords at authentication.
  • Aligns audit attribution and authentication configuration with the trusted client-IP resolver.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/core/utils/request.ts Centralizes trusted forwarding-chain resolution and returns null when a safe client identity cannot be established.
packages/security/src/ip.ts Implements and tests trusted-proxy-aware client-IP resolution for forwarded address chains.
apps/sim/lib/core/rate-limiter/route-helpers.ts Separates fail-closed IP limiting from explicit deferral to an independently enforced backstop.
apps/sim/lib/core/security/deployment-auth.ts Adds resource-wide throttling to public password authentication while retaining client-level limits.
apps/sim/app/api/chat/[identifier]/otp/route.ts Adds fail-closed chat-resource and recipient limits around public OTP delivery.
apps/sim/app/api/files/public/[token]/otp/route.ts Adds fail-closed share-resource and recipient limits around public file OTP delivery.
apps/sim/lib/api/contracts/chats.ts Requires stronger newly configured chat passwords while preserving the legacy login contract.
apps/sim/lib/api/contracts/public-shares.ts Applies the stronger password requirement to newly created or updated public file shares.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Resolver as Trusted IP Resolver
  participant ClientLimit as Client-IP Limit
  participant ResourceLimit as Resource/Recipient Limit
  participant Auth as Public Auth Handler
  Client->>Resolver: Public authentication request
  alt Trusted client IP resolved
    Resolver->>ClientLimit: Consume client budget
    ClientLimit-->>Resolver: Allowed
  else Client IP unresolved
    Resolver->>Resolver: Fail closed unless independent backstop exists
  end
  Resolver->>ResourceLimit: Consume shared resource/recipient budget
  ResourceLimit-->>Auth: Allowed
  Auth-->>Client: Authentication response
Loading

Reviews (5): Last reviewed commit: "fix(security): preserve independent rate..." | Re-trigger Greptile

Comment thread apps/sim/lib/core/rate-limiter/route-helpers.ts Outdated
Comment thread apps/sim/app/api/chat/utils.test.ts Outdated
Comment thread apps/sim/lib/core/utils/request.ts
Comment thread apps/sim/app/api/chat/utils.test.ts
Comment thread apps/sim/lib/core/config/env.ts
Comment thread apps/sim/lib/core/rate-limiter/route-helpers.ts Outdated
Comment thread apps/sim/lib/core/security/deployment-auth.ts
Comment thread apps/sim/app/api/chat/utils.test.ts
Comment thread apps/sim/app/api/demo-requests/route.ts Outdated
Comment thread apps/sim/app/api/chat/[identifier]/otp/route.ts
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/core/rate-limiter/route-helpers.ts
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2fd9f2c. Configure here.

@TheodoreSpeaks
TheodoreSpeaks merged commit bbf408b into staging Aug 23, 2026
30 checks passed
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