fix(security): move newsletter subscription to server-side API route#465
fix(security): move newsletter subscription to server-side API route#465KeenIsHere wants to merge 25 commits into
Conversation
Move transforms to Motion props to avoid Tailwind overrides and respect prefers-reduced-motion.
…aders Add aria-hidden and focusable=false to checkmark SVG for accessibility.
…on guide - Clarify highlighted card is driven by highlighted prop, not position - Remove Framer Motion from browser support (runtime dependency, not browser feature) - Add installation instructions using yarn animata:new - Update animation details to reflect Motion-based approach
- Replace Unix shell syntax (&, wait) with cross-platform concurrently - Install concurrently package for parallel process management - Update .storybook/main.ts with webpack configuration - Add .env.storybook for font optimization settings - Include comprehensive troubleshooting guide This fixes yarn dev failures on Windows and provides workarounds for Google Fonts network connectivity issues.
- Create app/api/newsletter/route.ts to handle Plunk API call server-side - Remove NEXT_PUBLIC_PLUNK_API_KEY from client-side code - Add input validation and proper error handling - Fixes critical security vulnerability: API key was exposed in browser Relates to CDL LO2 - Backend Development (Node.js) Student: Krishna Kumar Gupta | ID: 23056976
📝 WalkthroughWalkthroughThis PR adds four new animated React components (DropdownMenu, AnimatedBackgroundWrapper, AnimatedPricingCards, FeatureGridHoverReveal) with full Storybook stories and documentation, moves newsletter subscriptions to a secure server-side API route, updates Storybook configuration and the development environment to use parallel command execution, and extends the docs sidebar navigation. ChangesNew Animated Components
Development Infrastructure and Newsletter API Security
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
primitive/animated-background-wrapper.tsx (1)
1-299: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winEliminate duplicate implementation and re-export from the canonical animata path.
Keeping two full copies will drift (and this PR already shows divergence across related files). Prefer a thin re-export here.
♻️ Proposed consolidation
-"use client"; - -import type { ReactNode } from "react"; -import { useEffect, useState } from "react"; -import { cn } from "`@/lib/utils`"; -... -export default function AnimatedBackgroundWrapper(...) { - ... -} +"use client"; + +export { default } from "`@/animata/primitive/animated-background-wrapper`"; +export type { AnimatedBackgroundProps } from "`@/animata/primitive/animated-background-wrapper`";As per coding guidelines,
animata/**/*.{tsx,ts}: Components go inanimata/<category>/<name>.tsxwith a matching<name>.stories.tsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@primitive/animated-background-wrapper.tsx` around lines 1 - 299, This file duplicates the full AnimatedBackgroundWrapper implementation; replace it with a thin re-export to the canonical animata component instead: remove the internal implementation (all constants, helper functions, subcomponents, KEYFRAME_STYLES, and the default function AnimatedBackgroundWrapper) and instead import the canonical component and types (e.g., import AnimatedBackgroundWrapper, { AnimatedBackgroundProps } from "animata/…") and re-export it (export { AnimatedBackgroundProps } and export default AnimatedBackgroundWrapper) so this module becomes a simple passthrough to the single source-of-truth.
🧹 Nitpick comments (1)
app/api/newsletter/route.ts (1)
85-85: 💤 Low valueUnused variable:
datais parsed but never used.The
PlunkResponseis read from the response but the value is discarded. Either remove the assignment or use it (e.g., to surface Plunk's success message).Suggested fix
- const data: PlunkResponse = await plunkResponse.json(); + // Optionally consume the body to ensure the response is fully read + await plunkResponse.json();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/newsletter/route.ts` at line 85, The parsed PlunkResponse stored in the local variable data is unused; either remove the assignment to data or use it to validate/surface Plunk's result. Update the code around the plunkResponse.json() call in app/api/newsletter/route.ts: if you need the response, read the value from plunkResponse.json() into data (type PlunkResponse) and use its success/message fields to determine success or include in the API response/logging, otherwise remove the const data: PlunkResponse = await plunkResponse.json(); line entirely to avoid the unused variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.storybook/main.ts:
- Around line 9-10: In .storybook/main.ts fix nextConfigPath to be an absolute
path: import path from "path" at the top and set nextConfigPath using
path.resolve(process.cwd(), "next.config.mjs") instead of the relative
"./next.config.mjs" so `@storybook/nextjs` resolves the repo-root next.config.mjs
correctly (update the nextConfigPath entry in the exported config object).
In `@animata/navigation/dropdown-menu.tsx`:
- Around line 47-54: handleKeyDown currently computes new selectedIndex using
modulo with items.length which will throw/produce NaN when items is empty; add
an early guard at the top of the handleKeyDown function to return immediately if
items.length === 0 (or !items.length) before handling "ArrowDown" / "ArrowUp"
keys so setSelectedIndex never receives an invalid value; keep existing behavior
for "Enter" as needed and reference the handleKeyDown function, items array,
setSelectedIndex and selectedIndex state when making the change.
In `@animata/primitive/animated-background-wrapper.tsx`:
- Around line 63-85: Replace the hardcoded violet/fuchsia/indigo background
classes in the animated background config with the theme accent token: update
the objects whose keyframe values are "animata-aurora-1", "animata-aurora-3",
and "animata-aurora-4" (currently using "bg-violet-500", "bg-indigo-500",
"bg-fuchsia-500") to use the theme accent color (hsl(var(--accent)))
instead—either by swapping the string in their color property to a
Tailwind-compatible accent class (e.g. using bg-[hsl(var(--accent))]) or by
applying the inline backgroundColor style with hsl(var(--accent))). Ensure only
those three objects are changed so the rest (e.g. "animata-aurora-2" with blue)
remain untouched.
In `@animata/section/animated-pricing-cards.tsx`:
- Around line 138-151: The CTA Motion button (motion.button) lacks an explicit
type which may cause it to submit a parent form; update the motion.button
element to include type="button" so clicks don't trigger form submission when
this component (using plan.highlighted and plan.ctaText) is rendered inside a
form.
- Around line 79-84: The entrance animation on motion.article still runs for
reduced-motion users; import and call useReducedMotion from framer-motion and
conditionally disable the animated props: when useReducedMotion() is true,
render initial and whileInView as the final static state (e.g., { opacity: 1, y:
0 }) or omit them/transition (so no movement) instead of the animated values,
and keep the animated values (initial: { opacity: 0, y: 20 }, whileInView: {
opacity: 1, y: 0 }, transition) only when reduced motion is not requested;
update the motion.article usage (initial, whileInView, transition, viewport)
accordingly.
In `@animata/section/feature-grid-hover-reveal.tsx`:
- Around line 121-126: The article element uses onFocus/onBlur handlers tied to
setHovered but is not keyboard-focusable; make the card intentionally focusable
by adding tabIndex={0} to the <article> element so keyboard users can trigger
the onFocus/onBlur handlers (or alternatively remove the focus/blur props and
any docs claiming keyboard support if you prefer not to make it focusable);
update the <article> where onMouseEnter/onMouseLeave/onFocus/onBlur call
setHovered to include tabIndex={0} so accessibility and the focus handlers work
as intended.
In `@app/api/newsletter/route.ts`:
- Around line 54-59: The route handler in app/api/newsletter/route.ts checks
process.env.PLUNK_API_KEY but deploy workflows only set
NEXT_PUBLIC_PLUNK_API_KEY; update deployment to provide PLUNK_API_KEY as a
runtime secret rather than relying on the public var. In code, remove the unused
const data: PlunkResponse = await plunkResponse.json() (or use it) inside the
newsletter handler and ensure non-OK plunkResponse branches return 409 where
appropriate (instead of always 502) so hooks/use-newsletter-subscription.ts can
correctly detect response.status === 409; reference the PLUNK_API_KEY check,
plunkResponse handling, and the PlunkResponse symbol to find the exact spots to
change.
- Around line 75-83: The non-OK Plunk response block currently always returns a
502 and never propagates Plunk's actual status (so route can't return 409);
update the error handling around plunkResponse in app/api/newsletter/route.ts so
that when !plunkResponse.ok you read errorData, attempt to parse it (or inspect
error string) and: if it indicates a duplicate/already-subscribed condition
return NextResponse.json({ error: "Already subscribed" }, { status: 409 }),
otherwise return NextResponse.json({ error: "Subscription service temporarily
unavailable" }, { status: plunkResponse.status || 502 }); keep the console.error
logging of errorData and ensure the branch aligns with
hooks/use-newsletter-subscription.ts which expects a 409 for "You are already
subscribed!".
In `@package.json`:
- Line 7: The dev Storybook port in the package.json "dev" script is
inconsistent with NEXT_PUBLIC_STORYBOOK_URL; update the "dev" script (the
concurrently command that runs "storybook dev -p 6006 --no-open
--debug-webpack") to use the same port as NEXT_PUBLIC_STORYBOOK_URL (change -p
to 6007), or alternatively change NEXT_PUBLIC_STORYBOOK_URL to match the current
script port—ensure both the "dev" script and the NEXT_PUBLIC_STORYBOOK_URL value
reference the identical port.
In `@primitive/animated-background-wrapper.stories.tsx`:
- Line 2: The source-of-truth comment on line 2 contains an incorrect duplicated
segment ("animata/animata/primitive/..."); update the comment string that begins
"Source of truth:" in animated-background-wrapper.stories.tsx to the correct
path without the duplicate segment (e.g., change
"animata/animata/primitive/animated-background-wrapper.stories.tsx" to
"animata/primitive/animated-background-wrapper.stories.tsx") so follow-up edits
are not misled.
---
Outside diff comments:
In `@primitive/animated-background-wrapper.tsx`:
- Around line 1-299: This file duplicates the full AnimatedBackgroundWrapper
implementation; replace it with a thin re-export to the canonical animata
component instead: remove the internal implementation (all constants, helper
functions, subcomponents, KEYFRAME_STYLES, and the default function
AnimatedBackgroundWrapper) and instead import the canonical component and types
(e.g., import AnimatedBackgroundWrapper, { AnimatedBackgroundProps } from
"animata/…") and re-export it (export { AnimatedBackgroundProps } and export
default AnimatedBackgroundWrapper) so this module becomes a simple passthrough
to the single source-of-truth.
---
Nitpick comments:
In `@app/api/newsletter/route.ts`:
- Line 85: The parsed PlunkResponse stored in the local variable data is unused;
either remove the assignment to data or use it to validate/surface Plunk's
result. Update the code around the plunkResponse.json() call in
app/api/newsletter/route.ts: if you need the response, read the value from
plunkResponse.json() into data (type PlunkResponse) and use its success/message
fields to determine success or include in the API response/logging, otherwise
remove the const data: PlunkResponse = await plunkResponse.json(); line entirely
to avoid the unused variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97c1da1a-2659-4fa9-953e-c30b1966ceeb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
.env.development.storybook/main.tsanimata/navigation/dropdown-menu.stories.tsxanimata/navigation/dropdown-menu.tsxanimata/primitive/animated-background-wrapper.stories.tsxanimata/primitive/animated-background-wrapper.tsxanimata/section/animated-pricing-cards.stories.tsxanimata/section/animated-pricing-cards.tsxanimata/section/feature-grid-hover-reveal.stories.tsxanimata/section/feature-grid-hover-reveal.tsxapp/api/newsletter/route.tsconfig/docs.tscontent/docs/navigation/dropdown-menu.mdxcontent/docs/primitive/animated-background-wrapper.mdxcontent/docs/section/animated-pricing-cards.mdxcontent/docs/section/feature-grid-hover-reveal.mdxhooks/use-newsletter-subscription.tspackage.jsonprimitive/animated-background-wrapper.stories.tsxprimitive/animated-background-wrapper.tsx
| nextConfigPath: "./next.config.mjs", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate next.config.mjs files:"
fd -a '^next\.config\.mjs$'
echo
echo "Check whether .storybook/next.config.mjs exists:"
if [ -f .storybook/next.config.mjs ]; then
echo "FOUND: .storybook/next.config.mjs"
else
echo "NOT FOUND: .storybook/next.config.mjs"
fi
echo
echo "Current nextConfigPath setting in .storybook/main.ts:"
rg -n 'nextConfigPath' .storybook/main.ts -C2Repository: codse/animata
Length of output: 378
🌐 Web query:
@storybook/nextjs nextConfigPath relative path resolution base directory
💡 Result:
@storybook/nextjs: nextConfigPath must be given as an absolute path; Storybook will not do relative-path resolution for you. The documented usage is to compute it from your project’s base directory (commonly the current working directory) before passing it. In Storybook’s Next.js framework docs, nextConfigPath is described as “The absolute path to the next.config.js file” and is explicitly positioned as an option for cases where your next.config.js is not in the project root [1]. The examples show resolving it with Node’s path.resolve(process.cwd, 'next.config.js') from your Storybook main config, meaning the effective “base directory” is whatever directory Storybook is run from / what process.cwd is at config evaluation time [1]. So for your question (relative path resolution + base directory): - Use an absolute path. Don’t pass a relative string. - If you want to build the absolute path from a relative location, anchor that relative location to process.cwd (or otherwise to the directory you consider the project root) just like the docs example: path.resolve(process.cwd, 'next.config.js') [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Contents: .storybook/main.ts (top 60 lines)"
sed -n '1,60p' .storybook/main.tsRepository: codse/animata
Length of output: 921
Fix nextConfigPath in .storybook/main.ts to point to the repo-root next.config.mjs (absolute path).
Current code uses nextConfigPath: "./next.config.mjs", which resolves to .storybook/next.config.mjs (file doesn’t exist), and @storybook/nextjs requires nextConfigPath to be an absolute path. Update to something like path.resolve(process.cwd, "next.config.mjs") (add import path from "path").
nextConfigPath: "./next.config.mjs",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.storybook/main.ts around lines 9 - 10, In .storybook/main.ts fix
nextConfigPath to be an absolute path: import path from "path" at the top and
set nextConfigPath using path.resolve(process.cwd(), "next.config.mjs") instead
of the relative "./next.config.mjs" so `@storybook/nextjs` resolves the repo-root
next.config.mjs correctly (update the nextConfigPath entry in the exported
config object).
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === "ArrowDown") { | ||
| e.preventDefault(); | ||
| setSelectedIndex((prev) => (prev + 1) % items.length); | ||
| } else if (e.key === "ArrowUp") { | ||
| e.preventDefault(); | ||
| setSelectedIndex((prev) => (prev - 1 + items.length) % items.length); | ||
| } else if (e.key === "Enter") { |
There was a problem hiding this comment.
Guard keyboard index math when items is empty.
On Line 50 and Line 53, modulo with items.length breaks for [] and can set selectedIndex to NaN. Add an early return before handling Arrow keys when items.length === 0.
Suggested patch
const handleKeyDown = (e: KeyboardEvent) => {
+ if (items.length === 0) {
+ if (e.key === "Escape") {
+ e.preventDefault();
+ setIsOpen(false);
+ triggerRef.current?.focus();
+ }
+ return;
+ }
+
if (e.key === "ArrowDown") {
e.preventDefault();
setSelectedIndex((prev) => (prev + 1) % items.length);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if (e.key === "ArrowDown") { | |
| e.preventDefault(); | |
| setSelectedIndex((prev) => (prev + 1) % items.length); | |
| } else if (e.key === "ArrowUp") { | |
| e.preventDefault(); | |
| setSelectedIndex((prev) => (prev - 1 + items.length) % items.length); | |
| } else if (e.key === "Enter") { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if (items.length === 0) { | |
| if (e.key === "Escape") { | |
| e.preventDefault(); | |
| setIsOpen(false); | |
| triggerRef.current?.focus(); | |
| } | |
| return; | |
| } | |
| if (e.key === "ArrowDown") { | |
| e.preventDefault(); | |
| setSelectedIndex((prev) => (prev + 1) % items.length); | |
| } else if (e.key === "ArrowUp") { | |
| e.preventDefault(); | |
| setSelectedIndex((prev) => (prev - 1 + items.length) % items.length); | |
| } else if (e.key === "Enter") { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animata/navigation/dropdown-menu.tsx` around lines 47 - 54, handleKeyDown
currently computes new selectedIndex using modulo with items.length which will
throw/produce NaN when items is empty; add an early guard at the top of the
handleKeyDown function to return immediately if items.length === 0 (or
!items.length) before handling "ArrowDown" / "ArrowUp" keys so setSelectedIndex
never receives an invalid value; keep existing behavior for "Enter" as needed
and reference the handleKeyDown function, items array, setSelectedIndex and
selectedIndex state when making the change.
| color: "bg-violet-500", | ||
| position: "left-[-10%] top-[-10%]", | ||
| baseOpacity: 0.4, | ||
| }, | ||
| { | ||
| keyframe: "animata-aurora-2", | ||
| duration: "20s", | ||
| color: "bg-blue-500", | ||
| position: "left-[55%] top-[-5%]", | ||
| baseOpacity: 0.35, | ||
| }, | ||
| { | ||
| keyframe: "animata-aurora-3", | ||
| duration: "18s", | ||
| color: "bg-indigo-500", | ||
| position: "left-[20%] top-[40%]", | ||
| baseOpacity: 0.3, | ||
| }, | ||
| { | ||
| keyframe: "animata-aurora-4", | ||
| duration: "22s", | ||
| color: "bg-fuchsia-500", | ||
| position: "left-[-5%] top-[20%]", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace hardcoded violet-family colors with the theme accent token.
Line 63, Line 77, and Line 84 use fixed violet/fuchsia/indigo classes, which breaks theme-token consistency. Use the accent token for purple/violet visuals instead.
🎨 Proposed change
const AURORA_BLOBS: AuroraBlobConfig[] = [
{
keyframe: "animata-aurora-1",
duration: "15s",
- color: "bg-violet-500",
+ color: "bg-[hsl(var(--accent))]",
position: "left-[-10%] top-[-10%]",
baseOpacity: 0.4,
},
@@
{
keyframe: "animata-aurora-3",
duration: "18s",
- color: "bg-indigo-500",
+ color: "bg-[hsl(var(--accent))]",
position: "left-[20%] top-[40%]",
baseOpacity: 0.3,
},
{
keyframe: "animata-aurora-4",
duration: "22s",
- color: "bg-fuchsia-500",
+ color: "bg-[hsl(var(--accent))]",
position: "left-[-5%] top-[20%]",
baseOpacity: 0.25,
},
];As per coding guidelines, **/*.{tsx,ts,css}: Use theme accent color hsl(var(--accent)) for purple/violet elements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| color: "bg-violet-500", | |
| position: "left-[-10%] top-[-10%]", | |
| baseOpacity: 0.4, | |
| }, | |
| { | |
| keyframe: "animata-aurora-2", | |
| duration: "20s", | |
| color: "bg-blue-500", | |
| position: "left-[55%] top-[-5%]", | |
| baseOpacity: 0.35, | |
| }, | |
| { | |
| keyframe: "animata-aurora-3", | |
| duration: "18s", | |
| color: "bg-indigo-500", | |
| position: "left-[20%] top-[40%]", | |
| baseOpacity: 0.3, | |
| }, | |
| { | |
| keyframe: "animata-aurora-4", | |
| duration: "22s", | |
| color: "bg-fuchsia-500", | |
| position: "left-[-5%] top-[20%]", | |
| color: "bg-[hsl(var(--accent))]", | |
| position: "left-[-10%] top-[-10%]", | |
| baseOpacity: 0.4, | |
| }, | |
| { | |
| keyframe: "animata-aurora-2", | |
| duration: "20s", | |
| color: "bg-blue-500", | |
| position: "left-[55%] top-[-5%]", | |
| baseOpacity: 0.35, | |
| }, | |
| { | |
| keyframe: "animata-aurora-3", | |
| duration: "18s", | |
| color: "bg-[hsl(var(--accent))]", | |
| position: "left-[20%] top-[40%]", | |
| baseOpacity: 0.3, | |
| }, | |
| { | |
| keyframe: "animata-aurora-4", | |
| duration: "22s", | |
| color: "bg-[hsl(var(--accent))]", | |
| position: "left-[-5%] top-[20%]", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animata/primitive/animated-background-wrapper.tsx` around lines 63 - 85,
Replace the hardcoded violet/fuchsia/indigo background classes in the animated
background config with the theme accent token: update the objects whose keyframe
values are "animata-aurora-1", "animata-aurora-3", and "animata-aurora-4"
(currently using "bg-violet-500", "bg-indigo-500", "bg-fuchsia-500") to use the
theme accent color (hsl(var(--accent))) instead—either by swapping the string in
their color property to a Tailwind-compatible accent class (e.g. using
bg-[hsl(var(--accent))]) or by applying the inline backgroundColor style with
hsl(var(--accent))). Ensure only those three objects are changed so the rest
(e.g. "animata-aurora-2" with blue) remain untouched.
| <motion.article | ||
| key={plan.name} | ||
| initial={{ opacity: 0, y: 20 }} | ||
| whileInView={{ opacity: 1, y: 0 }} | ||
| transition={{ delay: index * 0.1, duration: 0.4 }} | ||
| viewport={{ once: true, margin: "-100px" }} |
There was a problem hiding this comment.
Entrance animation still runs for reduced-motion users.
initial/whileInView always animates, so reduced-motion users still get scroll-in motion. Please gate these animation props when reduced motion is enabled.
Suggested fix
+import { motion, useReducedMotion } from "motion/react";
...
export default function AnimatedPricingCards({ plans = defaultPlans }: AnimatedPricingCardsProps) {
const safePlans = Array.isArray(plans) ? plans : defaultPlans;
+ const prefersReducedMotion = useReducedMotion();
...
<motion.article
key={plan.name}
- initial={{ opacity: 0, y: 20 }}
- whileInView={{ opacity: 1, y: 0 }}
- transition={{ delay: index * 0.1, duration: 0.4 }}
+ initial={prefersReducedMotion ? false : { opacity: 0, y: 20 }}
+ whileInView={prefersReducedMotion ? { opacity: 1, y: 0 } : { opacity: 1, y: 0 }}
+ transition={prefersReducedMotion ? { duration: 0 } : { delay: index * 0.1, duration: 0.4 }}
viewport={{ once: true, margin: "-100px" }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <motion.article | |
| key={plan.name} | |
| initial={{ opacity: 0, y: 20 }} | |
| whileInView={{ opacity: 1, y: 0 }} | |
| transition={{ delay: index * 0.1, duration: 0.4 }} | |
| viewport={{ once: true, margin: "-100px" }} | |
| <motion.article | |
| key={plan.name} | |
| initial={prefersReducedMotion ? false : { opacity: 0, y: 20 }} | |
| whileInView={prefersReducedMotion ? { opacity: 1, y: 0 } : { opacity: 1, y: 0 }} | |
| transition={prefersReducedMotion ? { duration: 0 } : { delay: index * 0.1, duration: 0.4 }} | |
| viewport={{ once: true, margin: "-100px" }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animata/section/animated-pricing-cards.tsx` around lines 79 - 84, The
entrance animation on motion.article still runs for reduced-motion users; import
and call useReducedMotion from framer-motion and conditionally disable the
animated props: when useReducedMotion() is true, render initial and whileInView
as the final static state (e.g., { opacity: 1, y: 0 }) or omit them/transition
(so no movement) instead of the animated values, and keep the animated values
(initial: { opacity: 0, y: 20 }, whileInView: { opacity: 1, y: 0 }, transition)
only when reduced motion is not requested; update the motion.article usage
(initial, whileInView, transition, viewport) accordingly.
| <motion.button | ||
| whileHover={{ scale: 1.02 }} | ||
| whileTap={{ scale: 0.98 }} | ||
| className={cn( | ||
| "h-11 w-full rounded-lg font-semibold transition-colors duration-200", | ||
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | ||
| "motion-reduce:hover:scale-100", | ||
| plan.highlighted | ||
| ? "bg-primary text-primary-foreground hover:bg-primary/90" | ||
| : "border border-border text-foreground hover:bg-muted", | ||
| )} | ||
| > | ||
| {plan.ctaText || "Get Started"} | ||
| </motion.button> |
There was a problem hiding this comment.
Set CTA button type explicitly.
Without type="button", this can submit a parent form unintentionally.
Suggested fix
<motion.button
+ type="button"
whileHover={{ scale: 1.02 }}
whileTap={{ scale: 0.98 }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <motion.button | |
| whileHover={{ scale: 1.02 }} | |
| whileTap={{ scale: 0.98 }} | |
| className={cn( | |
| "h-11 w-full rounded-lg font-semibold transition-colors duration-200", | |
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | |
| "motion-reduce:hover:scale-100", | |
| plan.highlighted | |
| ? "bg-primary text-primary-foreground hover:bg-primary/90" | |
| : "border border-border text-foreground hover:bg-muted", | |
| )} | |
| > | |
| {plan.ctaText || "Get Started"} | |
| </motion.button> | |
| <motion.button | |
| type="button" | |
| whileHover={{ scale: 1.02 }} | |
| whileTap={{ scale: 0.98 }} | |
| className={cn( | |
| "h-11 w-full rounded-lg font-semibold transition-colors duration-200", | |
| "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | |
| "motion-reduce:hover:scale-100", | |
| plan.highlighted | |
| ? "bg-primary text-primary-foreground hover:bg-primary/90" | |
| : "border border-border text-foreground hover:bg-muted", | |
| )} | |
| > | |
| {plan.ctaText || "Get Started"} | |
| </motion.button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animata/section/animated-pricing-cards.tsx` around lines 138 - 151, The CTA
Motion button (motion.button) lacks an explicit type which may cause it to
submit a parent form; update the motion.button element to include type="button"
so clicks don't trigger form submission when this component (using
plan.highlighted and plan.ctaText) is rendered inside a form.
| <article | ||
| onMouseEnter={() => setHovered(true)} | ||
| onMouseLeave={() => setHovered(false)} | ||
| onFocus={() => setHovered(true)} | ||
| onBlur={() => setHovered(false)} | ||
| className={cn( |
There was a problem hiding this comment.
Focus handlers won’t fire on this non-focusable card.
Line 124 and Line 125 attach focus/blur handlers to <article>, but Line 121 is not focusable, so keyboard users won’t trigger this state. Either add tabIndex={0} intentionally or remove the focus handlers/docs claim.
Suggested patch
return (
<article
+ tabIndex={0}
onMouseEnter={() => setHovered(true)}
onMouseLeave={() => setHovered(false)}
onFocus={() => setHovered(true)}
onBlur={() => setHovered(false)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <article | |
| onMouseEnter={() => setHovered(true)} | |
| onMouseLeave={() => setHovered(false)} | |
| onFocus={() => setHovered(true)} | |
| onBlur={() => setHovered(false)} | |
| className={cn( | |
| <article | |
| tabIndex={0} | |
| onMouseEnter={() => setHovered(true)} | |
| onMouseLeave={() => setHovered(false)} | |
| onFocus={() => setHovered(true)} | |
| onBlur={() => setHovered(false)} | |
| className={cn( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@animata/section/feature-grid-hover-reveal.tsx` around lines 121 - 126, The
article element uses onFocus/onBlur handlers tied to setHovered but is not
keyboard-focusable; make the card intentionally focusable by adding tabIndex={0}
to the <article> element so keyboard users can trigger the onFocus/onBlur
handlers (or alternatively remove the focus/blur props and any docs claiming
keyboard support if you prefer not to make it focusable); update the <article>
where onMouseEnter/onMouseLeave/onFocus/onBlur call setHovered to include
tabIndex={0} so accessibility and the focus handlers work as intended.
| const plunkApiKey = process.env.PLUNK_API_KEY; | ||
|
|
||
| if (!plunkApiKey) { | ||
| console.error("PLUNK_API_KEY environment variable is not set"); | ||
| return NextResponse.json({ error: "Server configuration error" }, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify PLUNK_API_KEY is not set in deploy workflow
rg -n "PLUNK_API_KEY" .github/workflows/ --type yamlRepository: codse/animata
Length of output: 274
🏁 Script executed:
#!/bin/bash
# Locate all uses/config for PLUNK_API_KEY and NEXT_PUBLIC_PLUNK_API_KEY
rg -n "PLUNK_API_KEY|NEXT_PUBLIC_PLUNK_API_KEY" . --type-add 'yml:*.{yml,yaml}' -g'*.{yml,yaml}' -g'*.ts' -g'*.tsx' -g'*.js' -g'*.jsx' -g'*.json' -g'*.md' || true
# Check newsletter API route status/error handling
rg -n "app/api/newsletter/route\.ts|NextResponse\.json|status:|409|502|500" app/api/newsletter/route.ts || true
# Check hook handling for 409
rg -n "use-newsletter-subscription\.ts|409|already subscribed" -S . || trueRepository: codse/animata
Length of output: 10712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant parts of the newsletter API route
nl -ba app/api/newsletter/route.ts | sed -n '1,140p'
# Show relevant part of the newsletter hook (409 handling context)
nl -ba hooks/use-newsletter-subscription.ts | sed -n '1,140p'
# Check other CI/workflow files for PLUNK_API_KEY anywhere
rg -n "PLUNK_API_KEY" . --type-add 'yml:*.{yml,yaml}' -g'*.{yml,yaml}' -S || true
# Double-check workflow env blocks for NEXT_PUBLIC_PLUNK_API_KEY presence
nl -ba .github/workflows/deploy.yml | sed -n '1,120p'
nl -ba .github/workflows/deploy-v3.yml | sed -n '1,120p'Repository: codse/animata
Length of output: 100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of the newsletter API route (use cat -n since nl may be missing)
echo "---- app/api/newsletter/route.ts (1-140) ----"
cat -n app/api/newsletter/route.ts | sed -n '1,140p'
echo "---- hooks/use-newsletter-subscription.ts (1-140) ----"
cat -n hooks/use-newsletter-subscription.ts | sed -n '1,140p'
echo "---- .github/workflows/deploy.yml (1-120) ----"
cat -n .github/workflows/deploy.yml | sed -n '1,120p'
echo "---- .github/workflows/deploy-v3.yml (1-120) ----"
cat -n .github/workflows/deploy-v3.yml | sed -n '1,120p'Repository: codse/animata
Length of output: 12925
PLUNK_API_KEY isn’t set in deploy workflows, causing newsletter API 500s
app/api/newsletter/route.ts requires process.env.PLUNK_API_KEY:
const plunkApiKey = process.env.PLUNK_API_KEY;
if (!plunkApiKey) {
console.error("PLUNK_API_KEY environment variable is not set");
return NextResponse.json({ error: "Server configuration error" }, { status: 500 });
}But both .github/workflows/deploy.yml and deploy-v3.yml only set NEXT_PUBLIC_PLUNK_API_KEY during yarn build—they never set PLUNK_API_KEY. Ensure PLUNK_API_KEY is provided to the deployed server/runtime via a secret/variable distinct from the public one.
app/api/newsletter/route.ts:const data: PlunkResponse = await plunkResponse.json();is unused.app/api/newsletter/route.tsnever returns 409 (all non-OK responses return 502), sohooks/use-newsletter-subscription.ts’sresponse.status === 409branch can’t be reached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/newsletter/route.ts` around lines 54 - 59, The route handler in
app/api/newsletter/route.ts checks process.env.PLUNK_API_KEY but deploy
workflows only set NEXT_PUBLIC_PLUNK_API_KEY; update deployment to provide
PLUNK_API_KEY as a runtime secret rather than relying on the public var. In
code, remove the unused const data: PlunkResponse = await plunkResponse.json()
(or use it) inside the newsletter handler and ensure non-OK plunkResponse
branches return 409 where appropriate (instead of always 502) so
hooks/use-newsletter-subscription.ts can correctly detect response.status ===
409; reference the PLUNK_API_KEY check, plunkResponse handling, and the
PlunkResponse symbol to find the exact spots to change.
| // 5. Handle Plunk API response | ||
| if (!plunkResponse.ok) { | ||
| const errorData = await plunkResponse.text(); | ||
| console.error("Plunk API error:", errorData); | ||
| return NextResponse.json( | ||
| { error: "Subscription service temporarily unavailable" }, | ||
| { status: 502 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Plunk API track endpoint response for duplicate email subscription
💡 Result:
Plunk’s public tracking endpoint POST /v1/track does not provide a special “duplicate email subscription” response. What happens when you “track” an event for an email address: - /v1/track “automatically creates or upserts the contact” and then records the event. [1] - The /v1/track API explicitly says it has “No idempotency: re-tracking the same event creates a new event record.” [1] - Therefore, if you call /v1/track multiple times for the same email+event, you should expect multiple event records; the API isn’t documented to detect duplicates and return a deduped/duplicate-specific response. [1] About subscription state changes: - /v1/track supports a “subscribed” boolean field that sets subscription state for the contact; for existing contacts it updates only when the field is explicitly provided (omitting it preserves current state). [2] - However, even with subscription updates, the endpoint still does not document idempotent behavior for duplicate tracking requests (“No idempotency”). [1] Example response shape (successful request): - The Track Event endpoint example returns { "success": true, "data": { "contact":..., "event":..., "timestamp":... } }. [2] So if your use case is “duplicate email subscription” (e.g., repeated calls trying to resubscribe/subscribe the same email), the docs indicate you may get repeated event records rather than a dedicated duplicate-subscription response.
Citations:
- 1: https://docs.useplunk.com/api-reference/public-api/trackEvent
- 2: https://www.mintlify.com/useplunk/plunk/api-reference/endpoints/track
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant portion of the route
FILE="app/api/newsletter/route.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
nl -ba "$FILE" | sed -n '1,220p' | sed -n '60,140p'
else
echo "Missing file: $FILE" >&2
fi
# Search for Plunk endpoint usage and any 409 handling in the codebase
echo "== Search: Plunk usage =="
rg -n "Plunk|useplunk|/v1/|track|409|already subscribed|subscribed" app || trueRepository: codse/animata
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/api/newsletter/route.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
awk 'NR>=60 && NR<=120 {printf "%d:%s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fi
echo "== Search: Plunk usage =="
rg -n "Plunk|useplunk|/v1/|track|409|already subscribed|subscribed" app || trueRepository: codse/animata
Length of output: 6933
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search: 409 already subscribed handling =="
rg -n "status === 409|409|already subscribed|already subscribed!|already subscribed" app || true
echo "== Search: newsletter signup fetch/status handling =="
rg -n "newsletter|subscrib|v1/newsletter|/api/newsletter|fetch\\(.*newsletter|/newsletter" app || trueRepository: codse/animata
Length of output: 769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate hook file =="
fd -i "use-newsletter-subscription" app || true
fd -i "use-newsletter-subscription" . || trueRepository: codse/animata
Length of output: 119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="./hooks/use-newsletter-subscription.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
awk 'NR>=1 && NR<=220 {printf "%d:%s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fiRepository: codse/animata
Length of output: 3235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="./app/api/newsletter/route.ts"
if [ -f "$FILE" ]; then
echo "== $FILE (full, up to 140 lines) =="
awk 'NR<=160 {printf "%d:%s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fiRepository: codse/animata
Length of output: 3933
Fix unreachable “already subscribed” handling: route never returns 409
app/api/newsletter/route.ts always returns 502 for any non-OK Plunk response and never maps plunkResponse.status, so it can never return 409. Meanwhile hooks/use-newsletter-subscription.ts only shows “You are already subscribed!” when response.status === 409, making that branch unreachable. Also, this route calls POST https://api.useplunk.com/v1/track (which is documented as non-idempotent and doesn’t specify a duplicate-subscription 409), so the UI/status mapping should be aligned with what Plunk actually returns (e.g., by parsing errorData and/or propagating Plunk’s status).
Suggested approach
if (!plunkResponse.ok) {
const errorData = await plunkResponse.text();
console.error("Plunk API error:", errorData);
+ // Check if Plunk indicates already subscribed (adjust condition based on actual Plunk response)
+ if (plunkResponse.status === 409 || errorData.includes("already")) {
+ return NextResponse.json(
+ { error: "Email is already subscribed" },
+ { status: 409 },
+ );
+ }
return NextResponse.json(
{ error: "Subscription service temporarily unavailable" },
{ status: 502 },
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 5. Handle Plunk API response | |
| if (!plunkResponse.ok) { | |
| const errorData = await plunkResponse.text(); | |
| console.error("Plunk API error:", errorData); | |
| return NextResponse.json( | |
| { error: "Subscription service temporarily unavailable" }, | |
| { status: 502 }, | |
| ); | |
| } | |
| // 5. Handle Plunk API response | |
| if (!plunkResponse.ok) { | |
| const errorData = await plunkResponse.text(); | |
| console.error("Plunk API error:", errorData); | |
| // Check if Plunk indicates already subscribed (adjust condition based on actual Plunk response) | |
| if (plunkResponse.status === 409 || errorData.includes("already")) { | |
| return NextResponse.json( | |
| { error: "Email is already subscribed" }, | |
| { status: 409 }, | |
| ); | |
| } | |
| return NextResponse.json( | |
| { error: "Subscription service temporarily unavailable" }, | |
| { status: 502 }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/newsletter/route.ts` around lines 75 - 83, The non-OK Plunk response
block currently always returns a 502 and never propagates Plunk's actual status
(so route can't return 409); update the error handling around plunkResponse in
app/api/newsletter/route.ts so that when !plunkResponse.ok you read errorData,
attempt to parse it (or inspect error string) and: if it indicates a
duplicate/already-subscribed condition return NextResponse.json({ error:
"Already subscribed" }, { status: 409 }), otherwise return NextResponse.json({
error: "Subscription service temporarily unavailable" }, { status:
plunkResponse.status || 502 }); keep the console.error logging of errorData and
ensure the branch aligns with hooks/use-newsletter-subscription.ts which expects
a 409 for "You are already subscribed!".
| "type": "module", | ||
| "scripts": { | ||
| "dev": "velite --watch & next dev & storybook dev -p 6006 --no-open & wait", | ||
| "dev": "concurrently \"velite --watch\" \"next dev\" \"storybook dev -p 6006 --no-open --debug-webpack\"", |
There was a problem hiding this comment.
Dev Storybook port is inconsistent with the configured URL.
NEXT_PUBLIC_STORYBOOK_URL is set to http://localhost:6007, but this script starts Storybook on 6006. Keep them aligned to avoid broken local links.
Suggested fix
- "dev": "concurrently \"velite --watch\" \"next dev\" \"storybook dev -p 6006 --no-open --debug-webpack\"",
+ "dev": "concurrently \"velite --watch\" \"next dev\" \"storybook dev -p 6007 --no-open --debug-webpack\"",
...
- "storybook": "storybook dev -p 6006",
+ "storybook": "storybook dev -p 6007",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 7, The dev Storybook port in the package.json "dev"
script is inconsistent with NEXT_PUBLIC_STORYBOOK_URL; update the "dev" script
(the concurrently command that runs "storybook dev -p 6006 --no-open
--debug-webpack") to use the same port as NEXT_PUBLIC_STORYBOOK_URL (change -p
to 6007), or alternatively change NEXT_PUBLIC_STORYBOOK_URL to match the current
script port—ensure both the "dev" script and the NEXT_PUBLIC_STORYBOOK_URL value
reference the identical port.
| @@ -0,0 +1,2 @@ | |||
| // This file is a leftover at the wrong path. | |||
| // Source of truth: animata/animata/primitive/animated-background-wrapper.stories.tsx | |||
There was a problem hiding this comment.
Fix the source-of-truth path in this note.
Line 2 points to animata/animata/primitive/..., which appears incorrect and can mislead follow-up edits.
✏️ Proposed correction
-// Source of truth: animata/animata/primitive/animated-background-wrapper.stories.tsx
+// Source of truth: animata/primitive/animated-background-wrapper.stories.tsx📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Source of truth: animata/animata/primitive/animated-background-wrapper.stories.tsx | |
| // Source of truth: animata/primitive/animated-background-wrapper.stories.tsx |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@primitive/animated-background-wrapper.stories.tsx` at line 2, The
source-of-truth comment on line 2 contains an incorrect duplicated segment
("animata/animata/primitive/..."); update the comment string that begins "Source
of truth:" in animated-background-wrapper.stories.tsx to the correct path
without the duplicate segment (e.g., change
"animata/animata/primitive/animated-background-wrapper.stories.tsx" to
"animata/primitive/animated-background-wrapper.stories.tsx") so follow-up edits
are not misled.
📚 CDL Learning Outcome Reference
Module: CS6W50 Career Development Learning
Student: Krishna Kumar Gupta | ID: 23056976
Company: Codse | Supervisor: Hari Lamichhane
Internship Period: February 2026 – Present
Learning Outcomes Demonstrated:
🔍 Background
During a self-initiated security audit of the Animata codebase
(May 2026), I identified a critical vulnerability where the
NEXT_PUBLIC_PLUNK_API_KEYwas being exposed directly in thebrowser — visible to any visitor via Chrome DevTools Network tab.
This PR implements the fix by moving the Plunk API call
from the client-side browser to a secure server-side
Next.js API route.
❌ Problem (Before This PR)
Any visitor could:
✅ Solution (After This PR)
📁 Files Changed
New File Created
app/api/newsletter/route.tsFile Updated
hooks/use-newsletter-subscription.tsNEXT_PUBLIC_PLUNK_API_KEYusageAuthorizationheader from client/api/newsletterroute instead🔒 Security Comparison
api.useplunk.comdirectly/api/newsletter(our server)🧪 Testing Done
localhost:3000/api/newsletter500whenPLUNK_API_KEYnot set locally(expected — key is a production secret owned by Codse)
📋 Checklist
NEXT_PUBLIC_prefix on sensitive environment variables🔗 Related
Closes: [Security] NEXT_PUBLIC_PLUNK_API_KEY exposed client-side in newsletter hook
https://github.com/codse/discussion/issues/30[Security] NEXT_PUBLIC_PLUNK_API_KEY exposed client-side in newsletter hook
https://github.com/codse/discussion/issues/30
This fix was identified and implemented by Krishna Kumar Gupta
as part of CDL internship learning activities at Codse (May 2026).
The vulnerability was discovered during a self-initiated full
security audit of the Animata codebase.
Summary by CodeRabbit
New Features
Documentation
Chores