-
Notifications
You must be signed in to change notification settings - Fork 5
Rename window.ENV to reduce clashing with other scripts #1520
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
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 | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -38,7 +38,7 @@ import { | |||||||||||||||||||||||||||||||
} from "@thunderstore/ts-api-react/src/SessionContext"; | ||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||
getPublicEnvVariables, | ||||||||||||||||||||||||||||||||
publicEnvVariables, | ||||||||||||||||||||||||||||||||
publicEnvVariablesType, | ||||||||||||||||||||||||||||||||
} from "cyberstorm/security/publicEnvVariables"; | ||||||||||||||||||||||||||||||||
import { StorageManager } from "@thunderstore/ts-api-react/src/storage"; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
@@ -47,7 +47,7 @@ import { StorageManager } from "@thunderstore/ts-api-react/src/storage"; | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
declare global { | ||||||||||||||||||||||||||||||||
interface Window { | ||||||||||||||||||||||||||||||||
ENV: publicEnvVariables; | ||||||||||||||||||||||||||||||||
NIMBUS_PUBLIC_ENV: publicEnvVariablesType; | ||||||||||||||||||||||||||||||||
Dapper: DapperTs; | ||||||||||||||||||||||||||||||||
nitroAds?: { | ||||||||||||||||||||||||||||||||
createAd: ( | ||||||||||||||||||||||||||||||||
|
@@ -104,9 +104,7 @@ export async function loader() { | |||||||||||||||||||||||||||||||
sessionId: undefined, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
publicEnvVariables: { | ||||||||||||||||||||||||||||||||
ENV: publicEnvVariables, | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
publicEnvVariables: publicEnvVariables, | ||||||||||||||||||||||||||||||||
currentUser: undefined, | ||||||||||||||||||||||||||||||||
config, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
|
@@ -137,9 +135,7 @@ export async function clientLoader() { | |||||||||||||||||||||||||||||||
const currentUser = await sessionTools.getSessionCurrentUser(); | ||||||||||||||||||||||||||||||||
const config = sessionTools.getConfig(publicEnvVariables.VITE_API_URL); | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
publicEnvVariables: { | ||||||||||||||||||||||||||||||||
ENV: publicEnvVariables, | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
publicEnvVariables: publicEnvVariables, | ||||||||||||||||||||||||||||||||
currentUser: currentUser.username ? currentUser : undefined, | ||||||||||||||||||||||||||||||||
config, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
|
@@ -221,8 +217,8 @@ export function Layout({ children }: { children: React.ReactNode }) { | |||||||||||||||||||||||||||||||
<body> | ||||||||||||||||||||||||||||||||
<script | ||||||||||||||||||||||||||||||||
dangerouslySetInnerHTML={{ | ||||||||||||||||||||||||||||||||
__html: `window.ENV = ${JSON.stringify( | ||||||||||||||||||||||||||||||||
data.publicEnvVariables.ENV | ||||||||||||||||||||||||||||||||
__html: `window.NIMBUS_PUBLIC_ENV = ${JSON.stringify( | ||||||||||||||||||||||||||||||||
data.publicEnvVariables | ||||||||||||||||||||||||||||||||
)}`, | ||||||||||||||||||||||||||||||||
Comment on lines
+220
to
222
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. 🛠️ Refactor suggestion Harden the global: make it read-only and resilient; consider CSP Prevent third-party scripts from mutating or overwriting the exposed env by defining it as non-writable + frozen; also don’t overwrite if already set. If you use a CSP, ensure this inline script is nonce’d or hashed. Apply: - __html: `window.NIMBUS_PUBLIC_ENV = ${JSON.stringify(
- data.publicEnvVariables
- )}`,
+ __html: `(function(){if(!("NIMBUS_PUBLIC_ENV" in window)){Object.defineProperty(window,"NIMBUS_PUBLIC_ENV",{value:Object.freeze(${JSON.stringify(
+ data.publicEnvVariables
+ )}),writable:false,configurable:false,enumerable:true});}})();`, Optional (CSP): add a nonce to this script tag consistent with your CSP setup. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||
|
@@ -231,7 +227,7 @@ export function Layout({ children }: { children: React.ReactNode }) { | |||||||||||||||||||||||||||||||
<Toast.Provider toastDuration={10000}> | ||||||||||||||||||||||||||||||||
<TooltipProvider> | ||||||||||||||||||||||||||||||||
<NavigationWrapper | ||||||||||||||||||||||||||||||||
domain={data.publicEnvVariables.ENV.VITE_API_URL || ""} | ||||||||||||||||||||||||||||||||
domain={data.publicEnvVariables.VITE_API_URL || ""} | ||||||||||||||||||||||||||||||||
currentUser={data?.currentUser} | ||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||
<div className="container container--x container--full island"> | ||||||||||||||||||||||||||||||||
|
@@ -274,7 +270,7 @@ function App() { | |||||||||||||||||||||||||||||||
const data = useRouteLoaderData<RootLoadersType>("root"); | ||||||||||||||||||||||||||||||||
const dapper = new DapperTs(() => { | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
apiHost: data?.publicEnvVariables.ENV.VITE_API_URL, | ||||||||||||||||||||||||||||||||
apiHost: data?.publicEnvVariables.VITE_API_URL, | ||||||||||||||||||||||||||||||||
sessionId: data?.config.sessionId, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
|
@@ -284,7 +280,7 @@ function App() { | |||||||||||||||||||||||||||||||
context={{ | ||||||||||||||||||||||||||||||||
currentUser: data?.currentUser, | ||||||||||||||||||||||||||||||||
requestConfig: dapper.config, | ||||||||||||||||||||||||||||||||
domain: data?.publicEnvVariables.ENV.VITE_API_URL, | ||||||||||||||||||||||||||||||||
domain: data?.publicEnvVariables.VITE_API_URL, | ||||||||||||||||||||||||||||||||
dapper: dapper, | ||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,14 +11,14 @@ export type publicEnvVariablesKeys = | |||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
export type PublicPrefix<envVariable extends string> = `VITE_${envVariable}`; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
export type publicEnvVariables = Partial<{ | ||||||||||||||||||||||||||||||||||||||||||||
export type publicEnvVariablesType = Partial<{ | ||||||||||||||||||||||||||||||||||||||||||||
[key in PublicPrefix<publicEnvVariablesKeys>]: string | undefined; | ||||||||||||||||||||||||||||||||||||||||||||
}>; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
export function getPublicEnvVariables( | ||||||||||||||||||||||||||||||||||||||||||||
vars: PublicPrefix<publicEnvVariablesKeys>[] | ||||||||||||||||||||||||||||||||||||||||||||
): publicEnvVariables { | ||||||||||||||||||||||||||||||||||||||||||||
const returnedVars: publicEnvVariables = {}; | ||||||||||||||||||||||||||||||||||||||||||||
): publicEnvVariablesType { | ||||||||||||||||||||||||||||||||||||||||||||
const returnedVars: publicEnvVariablesType = {}; | ||||||||||||||||||||||||||||||||||||||||||||
if (import.meta.env.SSR) { | ||||||||||||||||||||||||||||||||||||||||||||
vars.forEach((envVar) => { | ||||||||||||||||||||||||||||||||||||||||||||
if (envVar.startsWith("VITE_") && envVar in process.env) { | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -27,8 +27,8 @@ export function getPublicEnvVariables( | |||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||
vars.forEach((envVar) => { | ||||||||||||||||||||||||||||||||||||||||||||
if (envVar.startsWith("VITE_") && envVar in window.ENV) { | ||||||||||||||||||||||||||||||||||||||||||||
returnedVars[envVar] = window.ENV[envVar]; | ||||||||||||||||||||||||||||||||||||||||||||
if (envVar.startsWith("VITE_") && envVar in window.NIMBUS_PUBLIC_ENV) { | ||||||||||||||||||||||||||||||||||||||||||||
returnedVars[envVar] = window.NIMBUS_PUBLIC_ENV[envVar]; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
29
to
33
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. 🛠️ Refactor suggestion Defensive access to window.NIMBUS_PUBLIC_ENV to prevent runtime errors If the inline script fails to run (e.g., CSP), Apply: - } else {
- vars.forEach((envVar) => {
- if (envVar.startsWith("VITE_") && envVar in window.NIMBUS_PUBLIC_ENV) {
- returnedVars[envVar] = window.NIMBUS_PUBLIC_ENV[envVar];
- }
- });
- }
+ } else {
+ const publicEnv =
+ typeof window !== "undefined" && (window as any).NIMBUS_PUBLIC_ENV
+ ? ((window as any).NIMBUS_PUBLIC_ENV as Record<string, string | undefined>)
+ : {};
+ vars.forEach((envVar) => {
+ if (
+ envVar.startsWith("VITE_") &&
+ Object.prototype.hasOwnProperty.call(publicEnv, envVar)
+ ) {
+ returnedVars[envVar] = publicEnv[envVar];
+ }
+ });
+ } 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
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.
💡 Verification agent
🧩 Analysis chain
Repo-wide verification to catch stragglers after the rename
Scan for old
window.ENV
and nested.ENV
usages; also ensure the new global is referenced where needed. This avoids runtime issues in un-touched routes/components.Run:
🏁 Script executed:
Length of output: 102
🏁 Script executed:
Length of output: 1739
Fix typo in error messages
Change “Enviroment variables did not load correctly…” to “Environment variables did not load correctly…” in:
🤖 Prompt for AI Agents