Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .changeset/itchy-ways-tell.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
"@clerk/shared": patch
---

Use shared safe environment variable access in Netlify cache handler
Make sure `process` is defined in environment when checking if application is running on Netlify.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay to update the changeset from the previous PR here since we haven't deployed yet?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this should be fine

20 changes: 20 additions & 0 deletions packages/shared/src/__tests__/netlifyCacheHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,24 @@ describe('handleNetlifyCacheInDevInstance', () => {

expect(requestStateHeaders.get('Location')).toBe('https://example.netlify.app');
});

it('should ignore the URL environment variable if it is not a string', () => {
// @ts-expect-error - Random object
process.env.URL = {};
process.env.NETLIFY = 'true';

const requestStateHeaders = new Headers({
Location: 'https://example.netlify.app',
});
const locationHeader = requestStateHeaders.get('Location') || '';

handleNetlifyCacheInDevInstance({
locationHeader,
requestStateHeaders,
publishableKey: mockPublishableKey,
});

const locationUrl = new URL(requestStateHeaders.get('Location') || '');
expect(locationUrl.searchParams.has(CLERK_NETLIFY_CACHE_BUST_PARAM)).toBe(true);
});
});
1 change: 0 additions & 1 deletion packages/shared/src/getEnvVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,5 @@ export const getEnvVariable = (name: string, context?: Record<string, any>): str
} catch {
// This will raise an error in Cloudflare Pages
}

return '';
};
26 changes: 20 additions & 6 deletions packages/shared/src/netlifyCacheHandler.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getEnvVariable } from './getEnvVariable';
/* eslint-disable turbo/no-undeclared-env-vars */
import { isDevelopmentFromPublishableKey } from './keys';

/**
Expand All @@ -11,6 +11,23 @@ import { isDevelopmentFromPublishableKey } from './keys';
*/
export const CLERK_NETLIFY_CACHE_BUST_PARAM = '__clerk_netlify_cache_bust';

/**
* Returns true if running in a Netlify environment.
* Checks for Netlify-specific environment variables in process.env.
* Safe for browser and non-Node environments.
*/
function isNetlifyRuntime(): boolean {
if (typeof process === 'undefined' || !process.env) {
return false;
}

return (
Boolean(process.env.NETLIFY) ||
Boolean(process.env.NETLIFY_FUNCTIONS_TOKEN) ||
(typeof process.env.URL === 'string' && process.env.URL.endsWith('netlify.app'))
);
}

/**
* Prevents infinite redirects in Netlify's functions by adding a cache bust parameter
* to the original redirect URL. This ensures that Netlify doesn't serve a cached response
Expand All @@ -32,12 +49,9 @@ export function handleNetlifyCacheInDevInstance({
requestStateHeaders: Headers;
publishableKey: string;
}) {
const isOnNetlify =
getEnvVariable('NETLIFY') ||
(typeof getEnvVariable('URL') === 'string' && getEnvVariable('URL').endsWith('netlify.app')) ||
Boolean(getEnvVariable('NETLIFY_FUNCTIONS_TOKEN'));

const isOnNetlify = isNetlifyRuntime();
const isDevelopmentInstance = isDevelopmentFromPublishableKey(publishableKey);

if (isOnNetlify && isDevelopmentInstance) {
const hasHandshakeQueryParam = locationHeader.includes('__clerk_handshake');
// If location header is the original URL before the handshake flow, add cache bust param
Expand Down
Loading