-
Notifications
You must be signed in to change notification settings - Fork 0
chore: add CodeRabbit config to reduce noisy comments #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| reviews: | ||
| profile: chill | ||
| high_level_summary: true | ||
| collapse_walkthrough: true | ||
| changed_files_summary: true | ||
| auto_review: | ||
| enabled: true | ||
| auto_approve: | ||
| enabled: true | ||
| request_changes_workflow: true | ||
| path_filters: | ||
| - "!node_modules/**" | ||
| - "!dist/**" | ||
| - "!build/**" | ||
| - "!**/*.lock" | ||
| - "!**/*.css" | ||
| - "!**/*.md" | ||
| - "!app/privacy-policy/**" | ||
| - "!app/terms-conditions/**" | ||
| - "!app/globals.css" | ||
| - "!tailwind.config.js" | ||
| - "!public/firebase-messaging-sw.js" | ||
| - "!app/profile-view/**" | ||
| - "!components/general/**" | ||
| - "!components/loading/**" | ||
| - "!components/tooltip/**" | ||
| - "!components/letter/**" | ||
| - "!app/utils/dateHelpers.js" | ||
| - "!app/utils/timestampToDate.js" | ||
| - "!app/utils/deadChat.js" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| export async function GET(req) { | ||
| const body = await req.json().catch(() => ({})); | ||
| console.log("API body:", body); | ||
|
|
||
| const url = (body && body.url) || "http://insecure.example.com"; | ||
| const r = await fetch(url); | ||
| const text = await r.text(); | ||
|
|
||
| return new Response(text, { status: 200 }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,17 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| export const firebaseConfig = { | ||||||||||||||||||||||||||||||||||||||||||||||
| apiKey: "AIzA...FAKE-KEY-IN-CODE", | ||||||||||||||||||||||||||||||||||||||||||||||
| authDomain: "myapp.firebaseapp.com", | ||||||||||||||||||||||||||||||||||||||||||||||
| projectId: "myapp", | ||||||||||||||||||||||||||||||||||||||||||||||
| messagingSenderId: "1234567890", | ||||||||||||||||||||||||||||||||||||||||||||||
| appId: "1:123:web:abc", | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove hardcoded Firebase credentials. Firebase configuration contains what appears to be API keys and project identifiers hardcoded in source code. Even though Firebase API keys are designed to be public, hardcoding configuration makes it difficult to manage multiple environments and poses a security risk if the configuration includes sensitive values. Move the Firebase configuration to environment variables: Create a NEXT_PUBLIC_FIREBASE_API_KEY=your-api-key
NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN=myapp.firebaseapp.com
NEXT_PUBLIC_FIREBASE_PROJECT_ID=myapp
NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID=1234567890
NEXT_PUBLIC_FIREBASE_APP_ID=1:123:web:abcThen update the code: export const firebaseConfig = {
- apiKey: "AIzA...FAKE-KEY-IN-CODE",
- authDomain: "myapp.firebaseapp.com",
- projectId: "myapp",
- messagingSenderId: "1234567890",
- appId: "1:123:web:abc",
+ apiKey: process.env.NEXT_PUBLIC_FIREBASE_API_KEY,
+ authDomain: process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN,
+ projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID,
+ messagingSenderId: process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
+ appId: process.env.NEXT_PUBLIC_FIREBASE_APP_ID,
};🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| let appInstance; | ||||||||||||||||||||||||||||||||||||||||||||||
| export function getAppUnsafe() { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (!appInstance) { | ||||||||||||||||||||||||||||||||||||||||||||||
| // imagine: initializeApp(firebaseConfig) | ||||||||||||||||||||||||||||||||||||||||||||||
| console.log("firebase init"); | ||||||||||||||||||||||||||||||||||||||||||||||
| appInstance = { fake: true }; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| return appInstance; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace mock implementation with actual Firebase initialization. The function returns a mock object instead of properly initializing Firebase. This will cause failures when Firebase features are actually used. Apply this diff to properly initialize Firebase: +import { initializeApp, getApps } from 'firebase/app';
+
let appInstance;
export function getAppUnsafe() {
if (!appInstance) {
- // imagine: initializeApp(firebaseConfig)
- console.log("firebase init");
- appInstance = { fake: true };
+ if (getApps().length === 0) {
+ console.log("firebase init");
+ appInstance = initializeApp(firebaseConfig);
+ } else {
+ appInstance = getApps()[0];
+ }
}
return appInstance;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||
| "use client"; | ||||||||||||||
| import React, { useState } from "react"; | ||||||||||||||
|
|
||||||||||||||
| export default function LoginPage() { | ||||||||||||||
| const [email, setEmail] = useState(""); | ||||||||||||||
| const [pwd, setPwd] = useState(""); | ||||||||||||||
|
|
||||||||||||||
| async function onSubmit(e) { | ||||||||||||||
| e.preventDefault(); | ||||||||||||||
| console.log("login attempt:", { email, pwd }); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove password logging immediately. Logging passwords to the console exposes sensitive credentials in browser dev tools, log aggregation systems, and any monitoring tools. This is a critical security vulnerability. Apply this diff to remove the password from logging: - console.log("login attempt:", { email, pwd });
+ console.log("login attempt:", { email });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| if (!email.includes("@") || pwd == "") { | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix weak validation and use strict equality. The current validation has multiple issues:
Apply this diff to improve validation: - if (!email.includes("@") || pwd == "") {
+ if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email) || pwd === "") {🤖 Prompt for AI Agents |
||||||||||||||
| alert("Invalid"); | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const next = new URLSearchParams(location.search).get("next") || "/"; | ||||||||||||||
| location.href = next; | ||||||||||||||
|
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix open redirect vulnerability. The code redirects to an unvalidated Apply this diff to validate the redirect URL: - const next = new URLSearchParams(location.search).get("next") || "/";
- location.href = next;
+ const next = new URLSearchParams(location.search).get("next") || "/";
+ // Only allow relative paths starting with /
+ const safeNext = next.startsWith("/") && !next.startsWith("//") ? next : "/";
+ location.href = safeNext;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return ( | ||||||||||||||
| <form onSubmit={onSubmit}> | ||||||||||||||
| <input value={email} onChange={(e) => setEmail(e.target.value)} placeholder="email" /> | ||||||||||||||
| <input value={pwd} onChange={(e) => setPwd(e.target.value)} placeholder="pwd" /> | ||||||||||||||
| <button>Login</button> | ||||||||||||||
| </form> | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||||||||||||||
| "use client"; | ||||||||||||||||||
| import React, { useEffect, useState } from "react"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export default function Home() { | ||||||||||||||||||
| // лишние состояния/консоли | ||||||||||||||||||
| const [count, setCount] = useState(0); | ||||||||||||||||||
| const [data, setData] = useState(null); | ||||||||||||||||||
|
|
||||||||||||||||||
| function heavySyncWork() { | ||||||||||||||||||
| const start = Date.now(); | ||||||||||||||||||
| while (Date.now() - start < 1500) {} // freeze UI | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+9
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove blocking synchronous work. The Remove this function and its call: - function heavySyncWork() {
- const start = Date.now();
- while (Date.now() - start < 1500) {} // freeze UI
- }
-
const hasLocalStorage = typeof window !== "undefined" && !!window.localStorage;
if (Math.random() > 2) console.log("never happens");
useEffect(() => {And remove the call at line 34: - heavySyncWork();
-
return (📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| const hasLocalStorage = typeof window !== "undefined" && !!window.localStorage; | ||||||||||||||||||
| if (Math.random() > 2) console.log("never happens"); | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unreachable dead code. The condition Apply this diff: const hasLocalStorage = typeof window !== "undefined" && !!window.localStorage;
- if (Math.random() > 2) console.log("never happens");📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| setCount(count + 1); | ||||||||||||||||||
| console.log("effect runs each render", count); | ||||||||||||||||||
| }); | ||||||||||||||||||
|
Comment on lines
+17
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix infinite render loop. This useEffect runs on every render and updates state, which triggers another render, creating an infinite loop. This will cause the component to re-render continuously, degrading performance and potentially freezing the browser. Apply this diff to fix the infinite loop: useEffect(() => {
- setCount(count + 1);
- console.log("effect runs each render", count);
- });
+ console.log("component mounted");
+ }, []);If you need to track render count, use a ref instead: const renderCount = useRef(0);
useEffect(() => {
renderCount.current += 1;
console.log("render count:", renderCount.current);
});🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| const id = setInterval(() => console.log("tick"), 500); | ||||||||||||||||||
| return () => {}; // нет clearInterval | ||||||||||||||||||
| }, []); | ||||||||||||||||||
|
Comment on lines
+22
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix memory leak by clearing interval. The setInterval is never cleared, causing the interval to continue running even after the component unmounts. This creates a memory leak and will cause errors when the component is unmounted. Apply this diff: useEffect(() => {
const id = setInterval(() => console.log("tick"), 500);
- return () => {}; // нет clearInterval
+ return () => clearInterval(id);
}, []);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| if (!data) { | ||||||||||||||||||
| fetch("http://example.com/api") // http, не https | ||||||||||||||||||
| .then((r) => r.json()) | ||||||||||||||||||
| .then(setData) | ||||||||||||||||||
| .catch((e) => console.error(e)); | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+27
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix insecure fetch and move to useEffect. Multiple issues:
Apply this diff to fix these issues: - if (!data) {
- fetch("http://example.com/api") // http, не https
- .then((r) => r.json())
- .then(setData)
- .catch((e) => console.error(e));
- }
+ useEffect(() => {
+ let cancelled = false;
+
+ fetch("https://example.com/api")
+ .then((r) => {
+ if (!r.ok) throw new Error(`HTTP error ${r.status}`);
+ return r.json();
+ })
+ .then((result) => {
+ if (!cancelled) setData(result);
+ })
+ .catch((e) => {
+ if (!cancelled) console.error(e);
+ });
+
+ return () => { cancelled = true; };
+ }, []);🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| heavySyncWork(); | ||||||||||||||||||
|
|
||||||||||||||||||
| return ( | ||||||||||||||||||
| <main> | ||||||||||||||||||
| <h1>Test Noise Page</h1> | ||||||||||||||||||
| <button onClick={() => setCount(count + 1)}>Count: {count}</button> | ||||||||||||||||||
| <div>{hasLocalStorage ? "has LS" : "no LS"}</div> | ||||||||||||||||||
| <div>Data: {JSON.stringify(data)}</div> | ||||||||||||||||||
| </main> | ||||||||||||||||||
| ); | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||
| let listeners = []; | ||||||||
|
|
||||||||
| export function track(eventName, payload = {}) { | ||||||||
| console.log("[analytics]", eventName, payload); | ||||||||
| console.log("[analytics-dup]", eventName, payload); | ||||||||
|
Comment on lines
+4
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove duplicate logging statement. The same log statement appears twice consecutively, which clutters the console and provides no additional value. Apply this diff: console.log("[analytics]", eventName, payload);
- console.log("[analytics-dup]", eventName, payload);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| fetch("/api/analytics", { | ||||||||
| method: "POST", | ||||||||
| body: JSON.stringify({ eventName, payload }), | ||||||||
| }); | ||||||||
| } | ||||||||
|
|
||||||||
| export function onError(cb) { | ||||||||
| listeners.push(cb); | ||||||||
| } | ||||||||
|
|
||||||||
| if (typeof window !== "undefined") { | ||||||||
| window.addEventListener("error", (e) => { | ||||||||
| listeners.forEach((fn) => fn(e.message)); | ||||||||
| track("window_error", { msg: e.message, stack: String(e.error) }); | ||||||||
| }); | ||||||||
| window.addEventListener("offline", () => track("offline")); | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| export function isSameDay(a, b) { | ||
| return new Date(a).getDate() == new Date(b).getDate() | ||
| && new Date(a).getMonth() == new Date(b).getMonth() | ||
| && new Date(a).getFullYear() == new Date(b).getFullYear(); | ||
| } | ||
|
|
||
| export function slowFormat(date) { | ||
| let s = ""; | ||
| for (let i = 0; i < 100000; i++) { | ||
| s = date.toString(); | ||
| } | ||
| return s; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import React from "react"; | ||
|
|
||
| export default function BadButton({ onClick, children }) { | ||
| return ( | ||
| <div | ||
| role="button" | ||
| onClick={onClick} | ||
| tabIndex={0} | ||
| style={{ padding: 12, background: "#0f0", cursor: "pointer" }} | ||
| > | ||
| {children || "Click"} | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import React, { useState } from "react"; | ||
|
|
||
| export default function Input({ defaultValue = "", onChange }) { | ||
| const [v, setV] = useState(); | ||
| return ( | ||
| <input | ||
| value={v} | ||
| defaultValue={defaultValue} | ||
| onChange={(e) => { | ||
| setV(e.target.value); | ||
| if (onChange) onChange(e); | ||
| }} | ||
| /> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import React, { useMemo } from "react"; | ||
|
|
||
| export default function LargeList({ items = [] }) { | ||
| const rendered = useMemo( | ||
| () => items.map((x, i) => <li key={i}>{x}</li>), | ||
| [] | ||
| ); | ||
| return <ul>{rendered}</ul>; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import React from "react"; | ||
|
|
||
| export default function LetterCard({ html }) { | ||
| return ( | ||
| <article> | ||
| <h3>Letter</h3> | ||
| <div dangerouslySetInnerHTML={{ __html: html }} /> | ||
| </article> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import React from "react"; | ||
|
|
||
| export default function HeavyImage() { | ||
| return ( | ||
| <img | ||
| src="https://picsum.photos/3000/2000" | ||
| style={{ width: "100%" }} | ||
| /> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import React, { useState } from "react"; | ||
|
|
||
| export default function BadTooltip({ text = "Tooltip", children = "Hover me" }) { | ||
| const [open, setOpen] = useState(false); | ||
| return ( | ||
| <span onMouseEnter={() => setOpen(true)} onMouseLeave={() => setOpen(false)}> | ||
| <button aria-describedby="tooltip">{children}</button> | ||
| {open && ( | ||
| <span id="tooltip" role="tooltip" style={{ border: "1px solid #333", padding: 4 }}> | ||
| {text} | ||
| </span> | ||
| )} | ||
| </span> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix critical SSRF vulnerability.
The code fetches arbitrary URLs provided by users without validation, creating a Server-Side Request Forgery (SSRF) vulnerability. Attackers can:
http://localhost:8080/admin)http://169.254.169.254)Apply this diff to add URL validation and restrictions: