Fix env variable issues and session revalidation #1513
Conversation
WalkthroughIntroduces public environment variable handling via a new module, migrates loaders to use getPublicEnvVariables and context-free getSessionTools, refactors root to split server/client loaders and inject window.ENV, updates session management to a cookie/domain-based API, adjusts UI to permissions in Wiki, and tweaks specific links/imports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant R as Remix Root Route
participant Env as window.ENV
participant Pub as publicEnvVariables.ts
participant Sess as SessionContext (ts-api-react)
participant API as DapperTs
rect rgb(245,248,255)
Note over R: SSR
B->>R: Request /
R->>Pub: getPublicEnvVariables(["VITE_API_URL","VITE_COOKIE_DOMAIN",...])
R-->>B: HTML + <script>window.ENV=ENV</script>
end
rect rgb(245,255,245)
Note over B: Hydration
B->>R: clientLoader()
R->>Pub: getSessionTools()
Pub->>Env: read ENV.VITE_API_URL / VITE_COOKIE_DOMAIN
Pub->>Sess: getSessionContext(apiHost, cookieDomain)
Sess->>Sess: read session cookie, init storage
R->>API: new DapperTs({apiHost, sessionId from cookie})
R-->>B: { currentUser, config, ENV }
end
sequenceDiagram
autonumber
participant B as Browser
participant Wiki as /p/tabs/Wiki (clientLoader)
participant Pub as publicEnvVariables.ts
participant Sess as SessionContext
participant API as DapperTs
B->>Wiki: clientLoader(params)
Wiki->>Pub: getSessionTools()
Pub->>Sess: getSessionContext(apiHost, cookieDomain)
Wiki->>API: getWiki(communityId, namespaceId, packageId, slug?)
Wiki->>API: getPackagePermissions(communityId, namespaceId, packageId)
API-->>Wiki: { wiki, permissions }
Wiki-->>B: loader data
Note over B: UI renders "New Page"/edit controls iff permissions.permissions.can_manage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| {wiki.pages.map((page, index) => { | ||
| if (!slug && index === 0) { |
There was a problem hiding this comment.
The current condition !slug && index === 0 has a navigation highlighting issue. It only highlights the first wiki page when no slug is provided, but fails to highlight the appropriate page when a slug is present.
The highlighting logic should:
- Highlight the first page when no slug is provided (
!slug && index === 0) - Highlight the page whose slug matches the current URL slug (
page.slug === slug)
Consider updating the condition to something like:
(!slug && index === 0) || page.slug === slugThis ensures proper navigation state visualization regardless of whether users are viewing the default page or a specific wiki page.
| {wiki.pages.map((page, index) => { | |
| if (!slug && index === 0) { | |
| {wiki.pages.map((page, index) => { | |
| if ((!slug && index === 0) || page.slug === slug) { |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| // sessionid missing but there is a currentUser, session is stale | ||
| if ( | ||
| storedCurrentUser !== null && | ||
| parseCurrentUser(storedCurrentUser).username | ||
| ) { | ||
| _storage.setValue(STALE_KEY, "yes"); | ||
| } | ||
| return false; |
There was a problem hiding this comment.
There appears to be a logical inconsistency in the session validation flow. When a sessionid cookie is missing but a valid currentUser exists in storage, the code marks the session as stale (_storage.setValue(STALE_KEY, "yes")) but then returns false, indicating an invalid session.
This creates a contradiction - marking a session as stale suggests it needs refreshing (which is handled elsewhere in the code), while returning false from sessionValid() indicates the session is completely invalid.
Consider modifying this logic to either:
- Return
truewhen marking the session as stale, allowing the stale flag to trigger revalidation - Or separate the concepts of "stale but recoverable" vs "completely invalid" sessions
This would prevent unnecessary logouts in cases where the session could be refreshed rather than completely invalidated.
| // sessionid missing but there is a currentUser, session is stale | |
| if ( | |
| storedCurrentUser !== null && | |
| parseCurrentUser(storedCurrentUser).username | |
| ) { | |
| _storage.setValue(STALE_KEY, "yes"); | |
| } | |
| return false; | |
| // sessionid missing but there is a currentUser, session is stale | |
| if ( | |
| storedCurrentUser !== null && | |
| parseCurrentUser(storedCurrentUser).username | |
| ) { | |
| _storage.setValue(STALE_KEY, "yes"); | |
| return true; // Return true for stale sessions to allow revalidation | |
| } | |
| return false; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Oksamies
deleted the
09-01-fix_env_variable_issues_and_session_revalidation
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx (1)
50-55: Optional chaining bug on tools.getConfig() resulttools?.getConfig() can evaluate to undefined; subsequent .apiHost/.sessionId access will throw. Chain through the config result or hoist it to a variable.
- apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, + apiHost: tools?.getConfig()?.apiHost, + sessionId: tools?.getConfig()?.sessionId,apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (1)
38-45: Potential undefined access on configtools?.getConfig() may return undefined; current code accesses config.apiHost/sessionId directly and can throw. Either optional-chain or validate config before use.
- const config = tools?.getConfig(); + const config = tools?.getConfig(); + if (!config) { + throw new Response("Failed to initialize session tools", { + status: 500, + }); + } const dapper = new DapperTs(() => { return { - apiHost: config.apiHost, - sessionId: config.sessionId, + apiHost: config.apiHost, + sessionId: config.sessionId, }; });apps/cyberstorm-remix/app/upload/upload.tsx (1)
83-93: Add error handling and fix optional chaining in client loadergetSessionTools() can throw and tools?.getConfig() can be undefined; handle both.
-export async function clientLoader() { - // console.log("clientloader context", getSessionTools(context)); - const tools = getSessionTools(); - const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); - return await dapper.getCommunities(); -} +export async function clientLoader() { + try { + const tools = getSessionTools(); + const config = tools?.getConfig(); + const dapper = new DapperTs(() => ({ + apiHost: config?.apiHost, + sessionId: config?.sessionId, + })); + return await dapper.getCommunities(); + } catch { + throw new Response("Failed to load upload data", { status: 500 }); + } +}apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx (1)
50-55: Optional chaining bug on tools.getConfig() resultProtect against undefined config in client loader.
- apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, + apiHost: tools?.getConfig()?.apiHost, + sessionId: tools?.getConfig()?.sessionId,apps/cyberstorm-remix/app/p/packageListing.tsx (1)
180-195: Avoid Promise.withResolvers; use the original promise (broader compatibility).
Promise.withResolversisn’t universally supported across browsers; also unnecessary here.- let permissionsPromise = undefined; + let permissionsPromise: ReturnType<typeof dapper.getPackagePermissions> | undefined = undefined; const cu = await tools.getSessionCurrentUser(); if (cu.username) { - const wrapperPromise = - Promise.withResolvers< - Awaited<ReturnType<typeof getPackagePermissions>> - >(); - dapper - .getPackagePermissions( - params.communityId, - params.namespaceId, - params.packageId - ) - .then(wrapperPromise.resolve, wrapperPromise.reject); - permissionsPromise = wrapperPromise.promise; + permissionsPromise = dapper.getPackagePermissions( + params.communityId, + params.namespaceId, + params.packageId + ); }Also applies to: 183-194
packages/ts-api-react/src/SessionContext.tsx (1)
69-76: Fix stale flag write: "no" currently coerces to truthy and stores "yes".This flips the intended value.
_storage.setValue(API_HOST_KEY, sessionData.apiHost); - _storage.setValue(COOKIE_DOMAIN_KEY, sessionData.cookieDomain); - _storage.setValue(STALE_KEY, sessionData.stale ? "yes" : "no"); + _storage.setValue(COOKIE_DOMAIN_KEY, sessionData.cookieDomain); + _storage.setValue(STALE_KEY, sessionData.stale);apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (1)
25-26: Handle empty wiki pages to avoid runtime crash.
Accessing wiki.pages[0] without a length check can throw.Consider:
- const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); + if (!wiki.pages.length) { + throw new Error("This package has no wiki pages"); + } + const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id);And mirror the same guard in clientLoader:
- const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); + if (!wiki.pages.length) { + throw new Error("This package has no wiki pages"); + } + const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id);Also applies to: 53-54
🧹 Nitpick comments (31)
apps/cyberstorm-remix/.env (1)
7-9: Reorder key to satisfy dotenv-linter and clarify expected value format.
- Move VITE_COOKIE_DOMAIN above VITE_SITE_URL to silence UnorderedKey.
- Consider documenting expected format (e.g., .thunderstore.io) to avoid cookie scope issues across subdomains.
Apply:
-VITE_SITE_URL= -VITE_API_URL= -VITE_COOKIE_DOMAIN= +VITE_API_URL= +VITE_COOKIE_DOMAIN= +VITE_SITE_URL=apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (1)
162-167: Switching to absolute href may cause full reload; prefer internal navigation when possible.If the upload page is on the same origin, using primitiveType="cyberstormLink" (or a Remix Link) avoids a full page refresh. Also guard against a trailing slash in domain to prevent double “//”.
- primitiveType="link" - href={`${domain}/package/create/`} + // Prefer SPA navigation when ready: + // primitiveType="cyberstormLink" + // linkId="PackageUpload" + primitiveType="link" + href={`${domain.replace(/\/$/, "")}/package/create/`}apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
43-53: Improve error text, fix typo, and avoid shadowing type name.Clearer message with missing keys listed; also rename local var to avoid confusion with the type.
-export function getSessionTools() { - const publicEnvVariables = getPublicEnvVariables([ +export function getSessionTools() { + const env = getPublicEnvVariables([ "VITE_API_URL", "VITE_COOKIE_DOMAIN", ]); - if ( - !publicEnvVariables.VITE_API_URL || - !publicEnvVariables.VITE_COOKIE_DOMAIN - ) { - throw new Error( - "Enviroment variables did not load correctly, please hard refresh page" - ); - } - return getSessionContext( - publicEnvVariables.VITE_API_URL, - publicEnvVariables.VITE_COOKIE_DOMAIN - ); + if (!env.VITE_API_URL || !env.VITE_COOKIE_DOMAIN) { + const missing = [ + !env.VITE_API_URL && "VITE_API_URL", + !env.VITE_COOKIE_DOMAIN && "VITE_COOKIE_DOMAIN", + ] + .filter(Boolean) + .join(", "); + throw new Error( + `Environment variables missing: ${missing}. Try a hard refresh or verify window.ENV injection.` + ); + } + return getSessionContext(env.VITE_API_URL, env.VITE_COOKIE_DOMAIN); }
3-16: Type/style nits.Consider PascalCase for type aliases (PublicEnvVariablesKeys, PublicEnvVariables) for consistency with TS conventions.
apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx (1)
2-3: Coalesce react-router imports (minor)Combine imports to reduce duplication.
-import { useLoaderData } from "react-router"; -import { LoaderFunctionArgs } from "react-router"; +import { LoaderFunctionArgs, useLoaderData } from "react-router";apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (2)
50-56: Broaden error handling (optional)getSessionTools() can throw (e.g., missing env). Consider handling non-ApiError with a user-friendly Response instead of rethrowing.
15-16: Coalesce react-router imports (minor)Tidy up duplicate imports.
-import { LoaderFunctionArgs } from "react-router"; -import { useLoaderData, useOutletContext, useRevalidator } from "react-router"; +import { + LoaderFunctionArgs, + useLoaderData, + useOutletContext, + useRevalidator, +} from "react-router";apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (1)
58-58: Component name mismatch with file (minor)Default export is named Community; consider renaming to TeamSettings for clarity.
-export default function Community() { +export default function TeamSettings() {apps/cyberstorm-remix/app/upload/upload.tsx (1)
34-35: Coalesce react-router imports (minor)Combine into a single import.
-import { MetaFunction } from "react-router"; -import { useLoaderData, useOutletContext } from "react-router"; +import { MetaFunction, useLoaderData, useOutletContext } from "react-router";apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx (1)
1-3: Coalesce react-router imports (minor)Reduce duplicate imports.
-import { LoaderFunctionArgs } from "react-router"; +import { LoaderFunctionArgs } from "react-router"; import { ApiError } from "@thunderstore/thunderstore-api"; -import { useLoaderData } from "react-router"; +import { useLoaderData } from "react-router";Alternative:
-import { LoaderFunctionArgs } from "react-router"; -import { useLoaderData } from "react-router"; +import { LoaderFunctionArgs, useLoaderData } from "react-router";apps/cyberstorm-remix/app/p/packageEdit.tsx (1)
79-122: Streamline client data fetch; verify getConfig usage.
- You re-fetch community/filters/listing/team already fetched server-side. Consider reusing SSR data when hydrated to cut requests.
- Confirm
tools.getConfig()requires no args.Potential parallelization:
- const permissions = await dapper.getPackagePermissions(...); - return { - community: await dapper.getCommunity(...), - communityFilters: await dapper.getCommunityFilters(...), - listing: await dapper.getPackageListingDetails(...), - team: await dapper.getTeamDetails(...), - filters: await dapper.getCommunityFilters(...), - permissions: permissions, - }; + const [permissions, community, communityFilters, listing, team, filters] = + await Promise.all([ + dapper.getPackagePermissions(params.communityId, params.namespaceId, params.packageId), + dapper.getCommunity(params.communityId), + dapper.getCommunityFilters(params.communityId), + dapper.getPackageListingDetails(params.communityId, params.namespaceId, params.packageId), + dapper.getTeamDetails(params.namespaceId), + dapper.getCommunityFilters(params.communityId), + ]); + return { community, communityFilters, listing, team, filters, permissions };Also applies to: 83-89, 101-112
apps/cyberstorm-remix/app/p/team/Team.tsx (2)
86-96: Confirm getConfig arity and handle potential env errors gracefully.
- Verify
tools.getConfig()without args is correct.- Consider catching the env error thrown by
getSessionTools()to return a user-friendly error (instead of crashing navigation).Also applies to: 89-95
45-47: Avoid in-place mutation when sorting sections.
Sorting mutatesfilters.sections; clone to prevent side effects.- const sortedSections = filters.sections.sort( + const sortedSections = [...filters.sections].sort( (a, b) => b.priority - a.priority );Also applies to: 108-110
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
197-207: Optional: parallelize independent requests.
You can shave latency by batching with Promise.all.- return { - community: await dapper.getCommunity(params.communityId), - communityFilters: await dapper.getCommunityFilters(params.communityId), - listing: await dapper.getPackageListingDetails( - params.communityId, - params.namespaceId, - params.packageId - ), - team: await dapper.getTeamDetails(params.namespaceId), - permissions: permissionsPromise, - }; + const [community, communityFilters, listing, team] = await Promise.all([ + dapper.getCommunity(params.communityId), + dapper.getCommunityFilters(params.communityId), + dapper.getPackageListingDetails(params.communityId, params.namespaceId, params.packageId), + dapper.getTeamDetails(params.namespaceId), + ]); + return { community, communityFilters, listing, team, permissions: permissionsPromise };packages/ts-api-react/src/SessionContext.tsx (2)
96-98: DRY: define a SESSION_COOKIE_NAME constant and reuse.Reduces typos and centralizes future changes.
+export const SESSION_COOKIE_NAME = "sessionid"; ... -export const clearCookies = (domain: string) => { - deleteCookie("sessionid", domain); +export const clearCookies = (domain: string) => { + deleteCookie(SESSION_COOKIE_NAME, domain); }; ... -const sessionidCookie = getCookie("sessionid"); +const sessionidCookie = getCookie(SESSION_COOKIE_NAME); ... -const sessionidCookie = getCookie("sessionid"); +const sessionidCookie = getCookie(SESSION_COOKIE_NAME);Also applies to: 126-126, 269-269
27-27: Naming: clarify getConfig(domain) parameter.“domain” here is a fallback apiHost, not a cookie domain; consider renaming to fallbackApiHost for clarity (non-blocking).
Also applies to: 100-103
apps/cyberstorm-remix/app/settings/user/Settings.tsx (1)
10-10: Comment-only change noted.If you later uncomment, ensure SESSION_STORAGE_KEY is imported and the storage manager used matches the new StorageManager API.
apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx (1)
17-26: Do not pass empty sessionId; reuse a single config variable.Minor cleanup and avoids sending blank auth.
export async function clientLoader({ params }: LoaderFunctionArgs) { if (params.namespaceId) { try { - const tools = getSessionTools(); - const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); + const tools = getSessionTools(); + const cfg = tools.getConfig(); + const dapper = new DapperTs(() => ({ + apiHost: cfg.apiHost, + ...(cfg.sessionId ? { sessionId: cfg.sessionId } : {}), + }));apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (3)
39-47: Optional: wrap getSessionTools() for friendlier UX.
It throws when ENV is missing; catching lets you surface a route-scoped message instead of a generic error boundary.-export async function clientLoader({ params }: LoaderFunctionArgs) { +export async function clientLoader({ params }: LoaderFunctionArgs) { if (params.communityId && params.namespaceId && params.packageId) { - const tools = getSessionTools(); + let tools; + try { + tools = getSessionTools(); + } catch { + throw new Error("Environment variables failed to load. Please refresh."); + }
55-59: Parallelize wiki and permission fetches.
Permissions don’t depend on the wiki; shave one round-trip.- const wiki = await dapper.getPackageWiki( - params.namespaceId, - params.packageId - ); - - const permissions = await dapper.getPackagePermissions( + const [wiki, permissions] = await Promise.all([ + dapper.getPackageWiki(params.namespaceId, params.packageId), + dapper.getPackagePermissions( params.communityId, params.namespaceId, params.packageId - ); + ), + ]);Also applies to: 61-67
74-86: Normalize boolean for canManage.
Prevents passing undefined into WikiContent.- canManage={permissions?.permissions.can_manage} + canManage={Boolean(permissions?.permissions?.can_manage)}apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (2)
46-50: Fix nextPage boundary check.
Use length - 1; current check still works due to optional chaining but reads past the array.- if (currentPageIndex === wiki.pages.length) { + if (currentPageIndex >= wiki.pages.length - 1) { nextPage = undefined; } else { nextPage = wiki.pages[currentPageIndex + 1]?.slug; }Repeat the same adjustment in clientLoader.
Also applies to: 99-103
65-72: Optional: handle getSessionTools() thrown errors.
Keeps route-local error messages actionable.(Apply the same try/catch pattern suggested in WikiFirstPage.tsx.)
apps/cyberstorm-remix/app/c/community.tsx (1)
118-126: Optional: defensive getSessionTools() usage.
Catching and rethrowing with a concise message can reduce support churn.- const tools = getSessionTools(); + let tools; + try { + tools = getSessionTools(); + } catch { + throw new Error("Environment variables failed to load. Please refresh."); + }apps/cyberstorm-remix/app/communities/communities.tsx (1)
86-103: Keep order defaulting consistent between server/client loaders.
Server uses SortOptions.Popular as default; mirror that in the client to avoid flicker or query divergence.-export async function clientLoader({ request }: LoaderFunctionArgs) { +export async function clientLoader({ request }: LoaderFunctionArgs) { const tools = getSessionTools(); const dapper = new DapperTs(() => { return { apiHost: tools?.getConfig().apiHost, sessionId: tools?.getConfig().sessionId, }; }); const searchParams = new URL(request.url).searchParams; - const order = searchParams.get("order"); + const order = searchParams.get("order") ?? SortOptions.Popular; const search = searchParams.get("search"); const page = undefined; return await dapper.getCommunities( page, - order ?? SortOptions.Popular, + order, search ?? "" ); }apps/cyberstorm-remix/app/settings/teams/Teams.tsx (1)
26-26: Avoid deep imports from package internals.
Importing from /src can break with packaging; prefer the package root (or re-export).-import { NamespacedStorageManager } from "@thunderstore/ts-api-react"; -import { - setSessionStale, - SESSION_STORAGE_KEY, -} from "@thunderstore/ts-api-react/src/SessionContext"; +import { + NamespacedStorageManager, + setSessionStale, + SESSION_STORAGE_KEY, +} from "@thunderstore/ts-api-react";If these aren’t exported at the root, consider re-exporting them there and updating this import.
Also applies to: 29-31
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
72-85: Drop optional chaining and avoid duplicate getConfig() callsSimplifies types and avoids redundant work.
-export async function clientLoader({ params }: LoaderFunctionArgs) { +export async function clientLoader({ params }: LoaderFunctionArgs) { @@ - const tools = getSessionTools(); - const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); + const tools = getSessionTools(); + const { apiHost, sessionId } = tools.getConfig(); + const dapper = new DapperTs(() => ({ apiHost, sessionId }));apps/cyberstorm-remix/app/p/dependants/Dependants.tsx (1)
96-105: Simplify client config usageRemove optional chaining and cache config once.
- const tools = getSessionTools(); - const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); + const tools = getSessionTools(); + const { apiHost, sessionId } = tools.getConfig(); + const dapper = new DapperTs(() => ({ apiHost, sessionId }));apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (1)
41-49: Tighten client config accessNo need for optional chaining; deref once.
- const tools = getSessionTools(); - const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); + const tools = getSessionTools(); + const { apiHost, sessionId } = tools.getConfig(); + const dapper = new DapperTs(() => ({ apiHost, sessionId }));apps/cyberstorm-remix/app/root.tsx (2)
148-168: Revalidation logic LGTM (minor consistency nit)Logic is sound. Consider using the same storage manager type for both calls for consistency.
- !sessionValid( - new StorageManager(SESSION_STORAGE_KEY), + !sessionValid( + new StorageManager(SESSION_STORAGE_KEY), publicEnvVariables.VITE_API_URL || "", publicEnvVariables.VITE_COOKIE_DOMAIN || "" )
220-226: Inline script: escape JSON and suppress linter with rationaleThis is a controlled injection of allowlisted VITE_* keys, but escape <, >, & to avoid edge cases and add a lint ignore.
Apply:
- <script + {/* biome-ignore lint/security/noDangerouslySetInnerHtml: safe bootstrapping of allowlisted public ENV */} + <script dangerouslySetInnerHTML={{ - __html: `window.ENV = ${JSON.stringify( - data.publicEnvVariables.ENV - )}`, + __html: `window.ENV = ${JSON.stringify( + data.publicEnvVariables.ENV + ) + .replace(/</g, "\\u003c") + .replace(/>/g, "\\u003e") + .replace(/&/g, "\\u0026")}`, }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (29)
apps/cyberstorm-remix/.env(1 hunks)apps/cyberstorm-remix/app/c/community.tsx(3 hunks)apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx(1 hunks)apps/cyberstorm-remix/app/communities/communities.tsx(3 hunks)apps/cyberstorm-remix/app/entry.client.tsx(0 hunks)apps/cyberstorm-remix/app/p/dependants/Dependants.tsx(3 hunks)apps/cyberstorm-remix/app/p/packageEdit.tsx(3 hunks)apps/cyberstorm-remix/app/p/packageListing.tsx(3 hunks)apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx(2 hunks)apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx(2 hunks)apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx(2 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx(2 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx(3 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx(3 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx(4 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx(3 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx(3 hunks)apps/cyberstorm-remix/app/p/team/Team.tsx(3 hunks)apps/cyberstorm-remix/app/root.tsx(7 hunks)apps/cyberstorm-remix/app/settings/teams/Teams.tsx(2 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx(2 hunks)apps/cyberstorm-remix/app/settings/user/Settings.tsx(1 hunks)apps/cyberstorm-remix/app/upload/upload.tsx(3 hunks)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts(1 hunks)packages/ts-api-react/src/SessionContext.tsx(9 hunks)packages/ts-api-react/src/index.ts(0 hunks)
💤 Files with no reviewable changes (2)
- apps/cyberstorm-remix/app/entry.client.tsx
- packages/ts-api-react/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (23)
apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx (2)
apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (1)
clientLoader(35-59)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getSessionTools(37-54)
apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (2)
apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getSessionTools(37-54)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)packages/dapper-ts/src/index.ts (1)
DapperTs(35-86)
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)packages/dapper-ts/src/index.ts (1)
DapperTs(35-86)
apps/cyberstorm-remix/app/c/community.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)
apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (4)
apps/cyberstorm-remix/app/c/community.tsx (1)
clientLoader(118-169)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
clientLoader(32-58)apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (1)
clientLoader(29-52)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getSessionTools(37-54)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getSessionTools(37-54)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)
apps/cyberstorm-remix/app/upload/upload.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (2)
apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (3)
loader(15-39)clientLoader(41-73)Wiki(75-163)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/p/packageEdit.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/p/dependants/Dependants.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)
apps/cyberstorm-remix/app/communities/communities.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)
apps/cyberstorm-remix/app/p/team/Team.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)apps/cyberstorm-remix/app/root.tsx (1)
clientLoader(114-144)
apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)packages/dapper-ts/src/index.ts (1)
DapperTs(35-86)
apps/cyberstorm-remix/app/settings/teams/Teams.tsx (2)
apps/cyberstorm-remix/app/root.tsx (1)
RootLoadersType(146-146)packages/ts-api-react/src/SessionContext.tsx (2)
setSessionStale(78-80)SESSION_STORAGE_KEY(46-46)
apps/cyberstorm-remix/app/root.tsx (4)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)packages/thunderstore-api/src/index.ts (1)
RequestConfig(1-7)packages/ts-api-react/src/SessionContext.tsx (4)
getSessionContext(221-296)sessionValid(121-157)SESSION_STORAGE_KEY(46-46)getSessionStale(82-84)packages/ts-api-react/src/storage.ts (1)
StorageManager(1-69)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
packages/ts-api-react/src/SessionContext.tsx (1)
getSessionContext(221-296)
apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables(13-15)getPublicEnvVariables(17-35)getSessionTools(37-54)packages/dapper-ts/src/index.ts (1)
DapperTs(35-86)
packages/ts-api-react/src/SessionContext.tsx (3)
packages/ts-api-react/src/storage.ts (2)
_storage(37-43)StorageManager(1-69)packages/thunderstore-api/src/index.ts (1)
RequestConfig(1-7)packages/thunderstore-api/src/schemas/objectSchemas.ts (2)
userSchema(314-323)EmptyUser(312-312)
🪛 dotenv-linter (3.3.0)
apps/cyberstorm-remix/.env
[warning] 9-9: [UnorderedKey] The VITE_COOKIE_DOMAIN key should go before the VITE_SITE_URL key
(UnorderedKey)
🪛 ast-grep (0.38.6)
apps/cyberstorm-remix/app/root.tsx
[warning] 220-220: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/cyberstorm-remix/app/root.tsx
[error] 221-221: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Generate visual diffs
- GitHub Check: Test
- GitHub Check: CodeQL
🔇 Additional comments (36)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
22-22: Good permission gating of edit actions.Conditional render based on canManage is clean. Ensure the loader always supplies a boolean to prevent flicker on hydration.
Also applies to: 32-33, 51-69
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (2)
26-26: Import path update looks correct.Migration to the new publicEnv module aligns with the PR direction.
32-36: clientLoader now relies on window.ENV via getSessionTools; verify timing.With the guard added in publicEnvVariables.ts, this should be safe. Without it, clientLoader could throw before ENV injection on initial load.
apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx (1)
92-95: Confirm HTML sanitization path for changelog HTMLdangerouslySetInnerHTML is fine if backend output is sanitized. Please confirm the API guarantees sanitization for changelog HTML to prevent XSS.
apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (1)
29-38: LGTM on session tools migrationSignature change and use of getSessionTools with safe config access look good.
apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx (1)
92-95: Confirm HTML sanitization for readme HTMLPlease verify that backend sanitizes readme HTML to prevent XSS before injecting.
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (3)
28-31: Good move: centralized env/session utilities.
Importing from cyberstorm/security/publicEnvVariables improves consistency and testability.
70-70: Confirm route uses context-less clientLoader signature.
Ensure no callers rely on the older{ context, params }shape.
72-78: Confirm getSessionTools().getConfig() arity.
Other places pass an apiHost intogetConfig(...); here it’s called without args. Verify it returns{ apiHost, sessionId }without a parameter.apps/cyberstorm-remix/app/p/packageEdit.tsx (1)
23-26: Imports look right.
Aligned with the new env/session helpers.apps/cyberstorm-remix/app/p/team/Team.tsx (1)
11-14: Centralized env/session imports LGTM.apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx (2)
9-12: Imports aligned with new helpers.
42-51: Double-check getConfig usage.
Consistency: elsewheregetConfig(apiHost?)is used; heregetConfig()is called bare.Also applies to: 45-51
apps/cyberstorm-remix/app/p/packageListing.tsx (2)
74-77: Consistent centralized imports.
167-176: Verify getConfig call signature.
tools.getConfig()is called without args; confirm that’s intended.Also applies to: 170-176
apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx (1)
11-11: Switch to new getSessionTools import — looks good.Aligns with the new publicEnvVariables module.
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (1)
6-9: Centralized env/session imports look good.
Consistent with the new publicEnvVariables/session-tools pattern.apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (1)
6-9: Env/session import change is consistent.
No issues spotted.apps/cyberstorm-remix/app/c/community.tsx (1)
19-22: Import alignment LGTM.
Matches the new helper module.apps/cyberstorm-remix/app/communities/communities.tsx (1)
27-31: Import updates LGTM.
Consistent with the new pattern.apps/cyberstorm-remix/app/settings/teams/Teams.tsx (2)
33-38: Meta typing update LGTM.
Switch to RootLoadersType aligns with the union of root loaders.
156-156: Session revalidation fix is correct.
Using SESSION_STORAGE_KEY removes the hard-coded "Session" key.apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
10-13: LGTM: centralized env/session importsImporting from cyberstorm/security/publicEnvVariables standardizes env/session access.
apps/cyberstorm-remix/app/p/dependants/Dependants.tsx (1)
16-19: LGTM: env/session helper importsConsistent with the PR’s direction.
apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (3)
6-9: LGTM: env/session helpers importedMatches new infra.
84-99: LGTM: gate “New Page” by can_manageGood permission-based UI control; avoids exposing actions to non-managers.
102-104: LGTM: default-select first page when slug is absentReasonable UX tweak; other pages still render.
apps/cyberstorm-remix/app/root.tsx (9)
10-12: Imports look goodCorrect usage of ShouldRevalidateFunctionArgs and useLoaderData from react-router.
50-50: Window.ENV typingGood to declare ENV; ensures TS safety when reading window.ENV.
92-112: Server loader LGTMPublic ENV filtering to VITE_* looks correct; config without sessionId is fine.
272-276: useRouteLoaderData generic depends on RootLoadersType fixAfter correcting RootLoadersType, this should typecheck. Dapper config binding to apiHost looks correct.
33-44: Verify StorageManager export before refactoring imports
StorageManager isn’t currently re-exported by @thunderstore/ts-api-react’s public API, so importing it from the package root will fail. Confirm that StorageManager—and the other session utilities—are exported in the package’s index; if not, either add those exports upstream or continue using deep imports until they’re exposed.
114-144: Ignore the async-sync and guard recommendations; only correct the typo in the error message.Likely an incorrect or invalid review comment.
232-233: Use VITE_COOKIE_DOMAIN for domain prop
In apps/cyberstorm-remix/app/root.tsx (around line 232), replace- domain={data.publicEnvVariables.ENV.VITE_API_URL || ""} + domain={data.publicEnvVariables.ENV.VITE_COOKIE_DOMAIN || ""}Ensure NavigationWrapper uses this value as the cookie/host domain—not the API base URL—before merging.
176-178: Align hook generic with corrected data type
After fixing RootLoadersType to return the proper loader types, this usage is correct. If you instead choose to use a concrete RootData, update the generic on useLoaderData() accordingly. Unable to verify via automated typecheck—please run your project’s typecheck command (e.g.pnpm typecheckoryarn typecheck) to confirm there are no implicit any leaks.
285-287: Double-checkdomainusage before swapping env var
Consumers ofoutletContext.domaincurrently expect the API base URL (VITE_API_URL) (e.g. by passing it to DapperTs or building fetch calls). Replacing it withVITE_COOKIE_DOMAINwill break all existing API calls. If you need both values, add a newcookieDomainfield instead of reusingdomain.
| const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | ||
| const dapper = new DapperTs(() => { | ||
| return { | ||
| apiHost: process.env.VITE_API_URL, | ||
| apiHost: publicEnvVariables.VITE_API_URL, | ||
| sessionId: undefined, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit VITE_API_URL guard.
Prevents misconfigured apiHost on SSR.
- const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]);
+ const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]);
const dapper = new DapperTs(() => {
return {
- apiHost: publicEnvVariables.VITE_API_URL,
+ apiHost: VITE_API_URL,
sessionId: undefined,
};
});
+ if (!VITE_API_URL) {
+ throw new Error("Missing VITE_API_URL");
+ }📝 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 publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | |
| const dapper = new DapperTs(() => { | |
| return { | |
| apiHost: process.env.VITE_API_URL, | |
| apiHost: publicEnvVariables.VITE_API_URL, | |
| sessionId: undefined, | |
| // Destructure and explicitly guard VITE_API_URL | |
| const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]); | |
| const dapper = new DapperTs(() => { | |
| return { | |
| apiHost: VITE_API_URL, | |
| sessionId: undefined, | |
| }; | |
| }); | |
| if (!VITE_API_URL) { | |
| throw new Error("Missing VITE_API_URL"); | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/community.tsx around lines 67 to 71, there is no
explicit guard for VITE_API_URL so apiHost can be misconfigured during SSR; add
a check after retrieving publicEnvVariables that verifies VITE_API_URL is a
non-empty string (throw a clear error or provide a safe default) and use that
validated value when constructing the DapperTs instance so apiHost is never
undefined on the server.
| const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | ||
| const dapper = new DapperTs(() => { | ||
| return { | ||
| apiHost: process.env.VITE_API_URL, | ||
| apiHost: publicEnvVariables.VITE_API_URL, | ||
| sessionId: undefined, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard VITE_API_URL in loader.
Fail fast with a clear message if missing.
- const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]);
+ const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]);
const dapper = new DapperTs(() => {
return {
- apiHost: publicEnvVariables.VITE_API_URL,
+ apiHost: VITE_API_URL,
sessionId: undefined,
};
});
+ if (!VITE_API_URL) {
+ throw new Error("Missing VITE_API_URL");
+ }📝 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 publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | |
| const dapper = new DapperTs(() => { | |
| return { | |
| apiHost: process.env.VITE_API_URL, | |
| apiHost: publicEnvVariables.VITE_API_URL, | |
| sessionId: undefined, | |
| const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]); | |
| const dapper = new DapperTs(() => { | |
| return { | |
| apiHost: VITE_API_URL, | |
| sessionId: undefined, | |
| }; | |
| }); | |
| if (!VITE_API_URL) { | |
| throw new Error("Missing VITE_API_URL"); | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/communities/communities.tsx around lines 72-76, the
VITE_API_URL from getPublicEnvVariables is used without a guard; update the
loader to validate that VITE_API_URL exists and fail fast with a clear message
if it's missing (e.g., throw a descriptive Error or return a 500 Response)
before constructing the DapperTs instance so the app doesn't proceed with an
undefined apiHost.
| const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | ||
| const dapper = new DapperTs(() => { | ||
| return { | ||
| apiHost: process.env.VITE_API_URL, | ||
| apiHost: publicEnvVariables.VITE_API_URL, | ||
| sessionId: undefined, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
SSR env validation: ensure VITE_API_URL exists
Prevents undefined host usage during SSR loader calls.
- const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]);
- const dapper = new DapperTs(() => {
- return {
- apiHost: publicEnvVariables.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]);
+ if (!VITE_API_URL) {
+ throw new Response("Server misconfiguration: VITE_API_URL missing", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({ apiHost: VITE_API_URL, sessionId: undefined }));📝 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 publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | |
| const dapper = new DapperTs(() => { | |
| return { | |
| apiHost: process.env.VITE_API_URL, | |
| apiHost: publicEnvVariables.VITE_API_URL, | |
| sessionId: undefined, | |
| }; | |
| const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]); | |
| if (!VITE_API_URL) { | |
| throw new Response("Server misconfiguration: VITE_API_URL missing", { status: 500 }); | |
| } | |
| const dapper = new DapperTs(() => ({ apiHost: VITE_API_URL, sessionId: undefined })); |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/dependants/Dependants.tsx around lines 31-36, the
code uses publicEnvVariables.VITE_API_URL directly which can be undefined during
SSR; ensure VITE_API_URL is present before creating DapperTs to avoid undefined
host usage in loader calls. Validate the result of
getPublicEnvVariables(["VITE_API_URL"]) and if VITE_API_URL is missing either
throw a clear error (e.g., `throw new Error("VITE_API_URL must be set in
environment for SSR")`) or provide a safe fallback URL; alternatively perform
the check before instantiating DapperTs and only construct it when the variable
is defined.
| const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | ||
| const dapper = new DapperTs(() => { | ||
| return { | ||
| apiHost: process.env.VITE_API_URL, | ||
| apiHost: publicEnvVariables.VITE_API_URL, | ||
| sessionId: undefined, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit env validation in server loader.
Avoid passing possibly-undefined apiHost to DapperTs.
- const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]);
+ const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]);
+ if (!VITE_API_URL) {
+ throw new Response("Server misconfiguration: VITE_API_URL missing", { status: 500 });
+ }
const dapper = new DapperTs(() => {
return {
- apiHost: publicEnvVariables.VITE_API_URL,
+ apiHost: VITE_API_URL,
sessionId: undefined,
};
});Also applies to: 51-53
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/packageEdit.tsx around lines 48-52 (and also
apply to 51-53), the loader is passing a potentially undefined
publicEnvVariables.VITE_API_URL into the DapperTs constructor; add an explicit
validation step in the server loader that checks publicEnvVariables.VITE_API_URL
and fails fast if missing (throw a 500 Response or Error with a clear message),
then pass the validated non-undefined string into DapperTs (or provide a safe
default if intentional). Ensure the validation runs before creating DapperTs and
update both occurrences mentioned in the comment.
| const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]); | ||
| const dapper = new DapperTs(() => { | ||
| return { | ||
| apiHost: process.env.VITE_API_URL, | ||
| apiHost: publicEnvVariables.VITE_API_URL, | ||
| sessionId: undefined, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add server env guard for VITE_API_URL.
Same rationale as other loaders.
- const publicEnvVariables = getPublicEnvVariables(["VITE_API_URL"]);
+ const { VITE_API_URL } = getPublicEnvVariables(["VITE_API_URL"]);
+ if (!VITE_API_URL) {
+ throw new Response("Server misconfiguration: VITE_API_URL missing", { status: 500 });
+ }
const dapper = new DapperTs(() => {
return {
- apiHost: publicEnvVariables.VITE_API_URL,
+ apiHost: VITE_API_URL,
sessionId: undefined,
};
});Also applies to: 139-141
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/packageListing.tsx around lines 136-142 (and also
for lines 139-141), ensure VITE_API_URL is validated on the server before
constructing DapperTs: check that publicEnvVariables.VITE_API_URL is defined and
non-empty and if not, throw a clear server-side error (or use the existing
server env-guard helper) so the loader fails fast rather than passing undefined
into DapperTs.
| export const getCookie = (cookieName: string) => { | ||
| const cookie = new RegExp(`${cookieName}=([^;]+);`).exec( | ||
| document.cookie[-1] === ";" ? document.cookie : document.cookie + ";" | ||
| )?.[1]; | ||
| return cookie ? cookie : null; | ||
| }; |
There was a problem hiding this comment.
Fix cookie parsing (document.cookie[-1] is always undefined) and avoid brittle RegExp.
Use a safe split-based lookup and handle SSR gracefully.
-export const getCookie = (cookieName: string) => {
- const cookie = new RegExp(`${cookieName}=([^;]+);`).exec(
- document.cookie[-1] === ";" ? document.cookie : document.cookie + ";"
- )?.[1];
- return cookie ? cookie : null;
-};
+export const getCookie = (cookieName: string): string | null => {
+ if (typeof document === "undefined" || !document.cookie) return null;
+ const prefix = `${encodeURIComponent(cookieName)}=`;
+ const pair = document.cookie.split("; ").find((c) => c.startsWith(prefix));
+ return pair ? decodeURIComponent(pair.slice(prefix.length)) : null;
+};📝 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.
| export const getCookie = (cookieName: string) => { | |
| const cookie = new RegExp(`${cookieName}=([^;]+);`).exec( | |
| document.cookie[-1] === ";" ? document.cookie : document.cookie + ";" | |
| )?.[1]; | |
| return cookie ? cookie : null; | |
| }; | |
| export const getCookie = (cookieName: string): string | null => { | |
| if (typeof document === "undefined" || !document.cookie) return null; | |
| const prefix = `${encodeURIComponent(cookieName)}=`; | |
| const pair = document.cookie.split("; ").find((c) => c.startsWith(prefix)); | |
| return pair ? decodeURIComponent(pair.slice(prefix.length)) : null; | |
| }; |
🤖 Prompt for AI Agents
In packages/ts-api-react/src/SessionContext.tsx around lines 55 to 60, the
cookie parsing uses document.cookie[-1] (always undefined) and a brittle RegExp;
replace with a safe implementation that first guards for SSR by returning null
if typeof document === "undefined" or !document.cookie, then split
document.cookie on ';', trim each segment, find the segment that starts with
cookieName + '=', split on '=' to get the value, decodeURIComponent the value
and return it or null if not found; avoid using negative index or fragile regex.
| const deleteCookie = (name: string, domain: string) => { | ||
| const date = new Date(); | ||
| date.setTime(0); | ||
| document.cookie = `${name}=${null}; expires=${date.toUTCString()}; path=/`; | ||
| document.cookie = `${name}=; domain=${domain}; expires=${date.toUTCString()}; path=/`; | ||
| }; |
There was a problem hiding this comment.
Ensure cookie deletion works when domain is empty; don’t emit domain=.
Current code writes an invalid attribute when domain === "", which can fail to clear the cookie.
-const deleteCookie = (name: string, domain: string) => {
- const date = new Date();
- date.setTime(0);
- document.cookie = `${name}=; domain=${domain}; expires=${date.toUTCString()}; path=/`;
-};
+const deleteCookie = (name: string, domain: string) => {
+ const date = new Date(0);
+ const parts = [
+ `${encodeURIComponent(name)}=`,
+ `expires=${date.toUTCString()}`,
+ "path=/",
+ ];
+ if (domain) parts.push(`domain=${domain}`);
+ document.cookie = parts.join("; ");
+};📝 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 deleteCookie = (name: string, domain: string) => { | |
| const date = new Date(); | |
| date.setTime(0); | |
| document.cookie = `${name}=${null}; expires=${date.toUTCString()}; path=/`; | |
| document.cookie = `${name}=; domain=${domain}; expires=${date.toUTCString()}; path=/`; | |
| }; | |
| const deleteCookie = (name: string, domain: string) => { | |
| const date = new Date(0); | |
| const parts = [ | |
| `${encodeURIComponent(name)}=`, | |
| `expires=${date.toUTCString()}`, | |
| "path=/", | |
| ]; | |
| if (domain) parts.push(`domain=${domain}`); | |
| document.cookie = parts.join("; "); | |
| }; |
🤖 Prompt for AI Agents
In packages/ts-api-react/src/SessionContext.tsx around lines 62 to 66, the
deleteCookie function currently emits an invalid "domain=" attribute when domain
is an empty string; update the function to only include the "domain=..." segment
when domain is non-empty (or truthy). Build the cookie string conditionally:
always set name=, expires to a past date and path=/, but append `;
domain=${domain}` only if domain is provided; then assign that constructed
string to document.cookie so cookie deletion works when domain is empty.
| export const getConfig = ( | ||
| _storage: StorageManager, | ||
| domain?: string | ||
| ): RequestConfig => { | ||
| const apiHost = _storage.safeGetValue(API_HOST_KEY); | ||
| const sessionId = _storage.safeGetValue(ID_KEY); | ||
| return { | ||
| // THIS IS NOT KOSHER | ||
| apiHost: apiHost ? apiHost : domain ? domain : "", | ||
| sessionId: sessionId ?? "", | ||
| sessionId: getCookie("sessionid") ?? "", | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid sending empty sessionId; omit the field when missing.
Prevents accidental auth header construction downstream.
export const getConfig = (
_storage: StorageManager,
domain?: string
): RequestConfig => {
const apiHost = _storage.safeGetValue(API_HOST_KEY);
- return {
- // THIS IS NOT KOSHER
- apiHost: apiHost ? apiHost : domain ? domain : "",
- sessionId: getCookie("sessionid") ?? "",
- };
+ const sessionId = getCookie("sessionid") ?? undefined;
+ return {
+ apiHost: apiHost ?? (domain ?? ""),
+ ...(sessionId ? { sessionId } : {}),
+ };
};📝 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.
| export const getConfig = ( | |
| _storage: StorageManager, | |
| domain?: string | |
| ): RequestConfig => { | |
| const apiHost = _storage.safeGetValue(API_HOST_KEY); | |
| const sessionId = _storage.safeGetValue(ID_KEY); | |
| return { | |
| // THIS IS NOT KOSHER | |
| apiHost: apiHost ? apiHost : domain ? domain : "", | |
| sessionId: sessionId ?? "", | |
| sessionId: getCookie("sessionid") ?? "", | |
| }; | |
| export const getConfig = ( | |
| _storage: StorageManager, | |
| domain?: string | |
| ): RequestConfig => { | |
| const apiHost = _storage.safeGetValue(API_HOST_KEY); | |
| const sessionId = getCookie("sessionid") ?? undefined; | |
| return { | |
| apiHost: apiHost ?? (domain ?? ""), | |
| ...(sessionId ? { sessionId } : {}), | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In packages/ts-api-react/src/SessionContext.tsx around lines 100 to 109, the
current getConfig always returns sessionId as an empty string when missing which
can lead to accidental auth header construction downstream; change the function
to only include the sessionId property when a non-empty cookie exists (e.g.,
read cookie into a variable and if truthy include sessionId in the returned
object, otherwise omit the field), and ensure RequestConfig accepts sessionId as
optional so callers handle its absence.
| // sessionid missing but there is a currentUser, session is stale | ||
| if ( | ||
| storedCurrentUser !== null && | ||
| parseCurrentUser(storedCurrentUser).username | ||
| ) { | ||
| _storage.setValue(STALE_KEY, "yes"); | ||
| } | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
sessionValid should not throw on corrupted stored currentUser.
Use safeParse and clean up invalid storage instead of throwing via parseCurrentUser().
-// sessionid missing but there is a currentUser, session is stale
-if (
- storedCurrentUser !== null &&
- parseCurrentUser(storedCurrentUser).username
-) {
- _storage.setValue(STALE_KEY, "yes");
-}
+// sessionid missing but there is a currentUser, session is stale
+if (storedCurrentUser !== null) {
+ const parsed = userSchema.safeParse(storedCurrentUser);
+ if (parsed.success && parsed.data.username) {
+ _storage.setValue(STALE_KEY, "yes");
+ } else {
+ _storage.removeValue(CURRENT_USER_KEY);
+ }
+}📝 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.
| // sessionid missing but there is a currentUser, session is stale | |
| if ( | |
| storedCurrentUser !== null && | |
| parseCurrentUser(storedCurrentUser).username | |
| ) { | |
| _storage.setValue(STALE_KEY, "yes"); | |
| } | |
| return false; | |
| }; | |
| // sessionid missing but there is a currentUser, session is stale | |
| if (storedCurrentUser !== null) { | |
| const parsed = userSchema.safeParse(storedCurrentUser); | |
| if (parsed.success && parsed.data.username) { | |
| _storage.setValue(STALE_KEY, "yes"); | |
| } else { | |
| _storage.removeValue(CURRENT_USER_KEY); | |
| } | |
| } | |
| return false; | |
| }; |
🤖 Prompt for AI Agents
In packages/ts-api-react/src/SessionContext.tsx around lines 149-157, the
current logic calls parseCurrentUser(...) which can throw if storedCurrentUser
is corrupted; replace that call with a non-throwing safeParse (or wrap in
try/catch), and if parsing fails remove/cleanup the stored currentUser from
storage and return false; if safeParse returns an object with a username then
mark STALE_KEY as "yes" as before. Ensure no exception bubbles from corrupted
storage by using safeParse or guarded parsing and deleting the invalid key
before returning.
| const currentUser = _storage.safeGetJsonValue(CURRENT_USER_KEY); | ||
|
|
||
| /** | ||
| * For accessing data API's session id. | ||
| * | ||
| * * isReady: boolean | ||
| * * sessionId?: string | ||
| * * csrfToken?: string | ||
| * * username?: string | ||
| * * setSession: ({sessionId: string, username: string}) => void | ||
| * * clearSession: () => void | ||
| */ | ||
| export const useSession = (): ContextInterface => { | ||
| const contextState = useContext(SessionContext); | ||
|
|
||
| if (contextState === null) { | ||
| throw new Error("useSession must be used within a SessionProvider tag"); | ||
| if (currentUser === null) { | ||
| return emptyCurrentUser; | ||
| } | ||
|
|
||
| return contextState; | ||
| return parseCurrentUser(currentUser); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getSessionCurrentUser: guard against invalid JSON; don’t throw from parse.
Hardening avoids app-breaking errors from malformed localStorage.
- const currentUser = _storage.safeGetJsonValue(CURRENT_USER_KEY);
-
- if (currentUser === null) {
- return emptyCurrentUser;
- }
- return parseCurrentUser(currentUser);
+ const currentUser = _storage.safeGetJsonValue(CURRENT_USER_KEY);
+ if (currentUser === null) return emptyCurrentUser;
+ const parsed = userSchema.safeParse(currentUser);
+ if (!parsed.success) {
+ _storage.removeValue(CURRENT_USER_KEY);
+ return emptyCurrentUser;
+ }
+ return parsed.data;📝 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 currentUser = _storage.safeGetJsonValue(CURRENT_USER_KEY); | |
| /** | |
| * For accessing data API's session id. | |
| * | |
| * * isReady: boolean | |
| * * sessionId?: string | |
| * * csrfToken?: string | |
| * * username?: string | |
| * * setSession: ({sessionId: string, username: string}) => void | |
| * * clearSession: () => void | |
| */ | |
| export const useSession = (): ContextInterface => { | |
| const contextState = useContext(SessionContext); | |
| if (contextState === null) { | |
| throw new Error("useSession must be used within a SessionProvider tag"); | |
| if (currentUser === null) { | |
| return emptyCurrentUser; | |
| } | |
| return contextState; | |
| return parseCurrentUser(currentUser); | |
| }; | |
| const currentUser = _storage.safeGetJsonValue(CURRENT_USER_KEY); | |
| if (currentUser === null) return emptyCurrentUser; | |
| const parsed = userSchema.safeParse(currentUser); | |
| if (!parsed.success) { | |
| _storage.removeValue(CURRENT_USER_KEY); | |
| return emptyCurrentUser; | |
| } | |
| return parsed.data; | |
| }; |
🤖 Prompt for AI Agents
In packages/ts-api-react/src/SessionContext.tsx around lines 213-219, the
current code calls parseCurrentUser on the value returned from
_storage.safeGetJsonValue and can throw if the stored JSON is malformed or not
the expected shape. Change this to validate and guard: if currentUser is null
return emptyCurrentUser; otherwise wrap parsing/validation in a try/catch (or
check typeof/currentUser shape) and on any error or unexpected type return
emptyCurrentUser instead of letting parseCurrentUser throw; keep successful
parse flow unchanged.
Summary by CodeRabbit
New Features
Refactor
Chores