Skip to content

Accessibility, tenant-CSS sanitization, and UI utility fixes#6

Merged
AutomatosAI merged 2 commits intomainfrom
codex/perform-full-review-of-saas-platform-zins3z
Jan 13, 2026
Merged

Accessibility, tenant-CSS sanitization, and UI utility fixes#6
AutomatosAI merged 2 commits intomainfrom
codex/perform-full-review-of-saas-platform-zins3z

Conversation

@AutomatosAI
Copy link
Copy Markdown
Owner

@AutomatosAI AutomatosAI commented Jan 13, 2026

Motivation

  • Improve accessibility and UX for the cookie preferences modal by adding ARIA, keyboard handling and focus management.
  • Reduce security risk from tenant-provided CSS by sanitizing before injecting via dangerouslySetInnerHTML.
  • Fix small UI/utility regressions that affect developer tooling and runtime behavior (displayName typos, event listener cleanup, class spacing, unused imports).

Description

  • Updated the cookie consent component (nextjs_space/components/cookie-consent.tsx) to run consent checks only on mount, add role="dialog", aria-modal="true", aria-labelledby, Escape/backdrop close handling, a simple focus-trap implemented in onKeyDown, focus the first checkbox on open, and append a ; Secure flag to consent cookies when running under https:.
  • Sanitized tenant CSS in nextjs_space/components/tenant-theme-provider.tsx by adding sanitizeCustomCss and rendering sanitizedCustomCss instead of raw customCss into dangerouslySetInnerHTML.
  • Fixed component/utility issues: corrected BreadcrumbEllipsis.displayName in nextjs_space/components/ui/breadcrumb.tsx, unsubscribed the reInit listener in nextjs_space/components/ui/carousel.tsx, removed the extra space in overlay classes in nextjs_space/components/ui/dialog.tsx, corrected MenubarShortcut.displayName in nextjs_space/components/ui/menubar.tsx, and removed an unused useState import from nextjs_space/components/ui/task-card.tsx.

Testing

  • No automated tests or CI jobs were executed for these changes.
  • No automated accessibility or security scans were run as part of this change.
  • Recommend running the project type-check, linters and manual browser testing for the cookie modal focus/keyboard behavior and tenant CSS rendering before deploying.
  • If available, run existing unit/integration tests and end-to-end checks (not executed here).

Codex Task

Summary by CodeRabbit

  • Refactor
    • Enhanced CSS sanitization handling for theme customization
    • Improved encryption migration validation logic with deadline support
    • Refined build converter module import cleanup

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 13, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This PR adds CSS sanitization logic to the tenant theme provider, introduces migration-related decryption helpers with internal utility functions, and includes an additional import cleanup step for react-router-dom in the lovable converter.

Changes

Cohort / File(s) Summary
CSS Sanitization
nextjs_space/components/tenant-theme-provider.tsx
Introduces sanitizeCustomCss() helper that strips @import, url(), and expression() patterns from CSS; wraps result in useMemo but doesn't integrate into rendering yet.
Decryption Migration Helpers
nextjs_space/lib/encryption.ts
Adds internal DecryptOptions type, DEFAULT_MIGRATION_DEADLINE constant, and isMigrationAllowed() validation function; note: definitions appear duplicated later in file.
Import Cleanup
nextjs_space/lib/lovable-converter.ts
Adds extra replacement step in refactorHeaderComponent() to strip remaining react-router-dom imports via regex.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through CSS streams,
Sanitizing, scrubbing dreams,
Helpers lock the migration door,
Imports cleaned, react-router no more! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 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 changes: accessibility improvements (cookie consent modal), CSS sanitization (tenant theme provider), and UI utility fixes (display name corrections, cleanup). The title directly reflects the primary objectives stated in the PR description.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9694ed5 and c532b08.

📒 Files selected for processing (3)
  • nextjs_space/components/tenant-theme-provider.tsx
  • nextjs_space/lib/encryption.ts
  • nextjs_space/lib/lovable-converter.ts

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be817c486b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 18 to 21
// Find user by email
const user = await prisma.users.findUnique({
const user = await prisma.users.findFirst({
where: { email },
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Scope password reset lookup to tenant

With the schema change allowing duplicate emails across tenants (users now have @@unique([tenantId, email])), this findFirst lookup can return an arbitrary tenant’s account when the same email exists in multiple tenants. In that case the reset token is attached to the wrong user, so the email recipient ends up resetting whichever account was picked first rather than the account for the current tenant. Consider scoping the lookup by the tenant from the request (subdomain/header) or requiring a tenant identifier.

Useful? React with 👍 / 👎.

Comment thread nextjs_space/app/api/signup/route.ts
@AutomatosAI AutomatosAI merged commit 1d0be7f into main Jan 13, 2026
1 check was pending
@AutomatosAI AutomatosAI deleted the codex/perform-full-review-of-saas-platform-zins3z branch January 13, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant