Make cookies access safer (#2694)

GitbookIO · Jan 8, 2025 · 8276ba0 · 8276ba0
1 parent 37d13d8
commit 8276ba0
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 22 deletions.
diff --git a/.changeset/gorgeous-kiwis-check.md b/.changeset/gorgeous-kiwis-check.md
@@ -0,0 +1,5 @@
+---
+'gitbook': patch
+---
+
+Make cookies access safer
diff --git a/packages/gitbook/src/components/Insights/InsightsProvider.tsx b/packages/gitbook/src/components/Insights/InsightsProvider.tsx
@@ -2,10 +2,11 @@
 
 import type * as api from '@gitbook/api';
 import { OpenAPIOperationContextProvider } from '@gitbook/react-openapi';
-import cookies from 'js-cookie';
 import * as React from 'react';
 import { useEventCallback, useDebounceCallback } from 'usehooks-ts';
 
+import * as cookies from '@/lib/cookies';
+
 import { getSession } from './sessions';
 import { getVisitorId } from './visitorId';
 
@@ -197,7 +198,7 @@ export function InsightsProvider(props: InsightsProviderProps) {
         return () => {
             window.removeEventListener('beforeunload', flushEventsSync);
         };
-    }, []);
+    }, [flushEventsSync]);
 
     return (
         <InsightsContext.Provider value={trackEvent}>
@@ -264,7 +265,7 @@ function transformEvents(input: {
         visitorId: input.visitorId,
         userAgent: window.navigator.userAgent,
         language: window.navigator.language,
-        cookies: cookies.get(),
+        cookies: cookies.getAll(),
         referrer: document.referrer || null,
         visitorAuthToken: input.visitorAuthToken ?? null,
     };

diff --git a/packages/gitbook/src/components/Insights/cookies.ts b/packages/gitbook/src/components/Insights/cookies.ts
@@ -1,6 +1,6 @@
 'use client';
 
-import cookies from 'js-cookie';
+import * as cookies from '@/lib/cookies';
 
 const GRANTED_COOKIE = '__gitbook_cookie_granted';
 
@@ -20,21 +20,13 @@ export function setCookiesTracking(enabled: boolean) {
  * Return `undefined` if state is not known.
  */
 export function isCookiesTrackingDisabled() {
-    try {
-        const state = cookies.get(GRANTED_COOKIE);
+    const state = cookies.get(GRANTED_COOKIE);
 
-        if (state === 'yes') {
-            return false;
-        } else if (state === 'no') {
-            return true;
-        }
-
-        return undefined;
-    } catch (error) {
-        // If there is a security error, we consider cookies as disabled
-        if (error instanceof Error && error.name === 'SecurityError') {
-            return true;
-        }
-        throw error;
+    if (state === 'yes') {
+        return false;
+    } else if (state === 'no') {
+        return true;
     }
+
+    return undefined;
 }
diff --git a/packages/gitbook/src/lib/analytics.ts b/packages/gitbook/src/lib/analytics.ts
@@ -1,6 +1,6 @@
 'use client';
 
-import cookies from 'js-cookie';
+import * as cookies from '@/lib/cookies';
 
 const VISITORID_COOKIE = '__session';
 const GRANTED_COOKIE = '__gitbook_cookie_granted';

diff --git a/packages/gitbook/src/lib/cookies.ts b/packages/gitbook/src/lib/cookies.ts
@@ -0,0 +1,29 @@
+import cookies from 'js-cookie';
+
+import { checkIsSecurityError } from './security-error';
+
+export function getAll(): {
+    [key: string]: string;
+} {
+    try {
+        return cookies.get();
+    } catch (error) {
+        if (checkIsSecurityError(error)) {
+            return {};
+        }
+        throw error;
+    }
+}
+
+export function get(name: string): string | undefined {
+    try {
+        return cookies.get(name);
+    } catch (error) {
+        if (checkIsSecurityError(error)) {
+            return undefined;
+        }
+        throw error;
+    }
+}
+
+export const set = cookies.set;
diff --git a/packages/gitbook/src/lib/local-storage.ts b/packages/gitbook/src/lib/local-storage.ts
@@ -1,3 +1,5 @@
+import { checkIsSecurityError } from './security-error';
+
 /**
  * Get an item from local storage safely.
  */
@@ -9,7 +11,7 @@ export function getItem<T>(key: string, defaultValue: T): T {
         }
         return defaultValue;
     } catch (error) {
-        if (error instanceof Error && error.name === 'SecurityError') {
+        if (checkIsSecurityError(error)) {
             return defaultValue;
         }
         throw error;
@@ -25,7 +27,7 @@ export function setItem(key: string, value: unknown) {
             localStorage.setItem(key, JSON.stringify(value));
         }
     } catch (error) {
-        if (error instanceof Error && error.name === 'SecurityError') {
+        if (checkIsSecurityError(error)) {
             return;
         }
         throw error;

diff --git a/packages/gitbook/src/lib/security-error.ts b/packages/gitbook/src/lib/security-error.ts
@@ -0,0 +1,12 @@
+/**
+ * Test if the error is a security error returned by the browser when cookies or local storage are blocked.
+ */
+export function checkIsSecurityError(error: unknown): error is Error {
+    return (
+        error instanceof Error &&
+        // Safari
+        (error.name === 'SecurityError' ||
+            // Firefox
+            error.name === 'NS_ERROR_FAILURE')
+    );
+}