Skip to content

Commit 43f4dc0

Browse files
mguptahubPlane AI
andcommitted
[INFRA-779] delegate isValidURL to the shared isValidNextPath instead of a local reimplementation
Address /code-review findings on the plane-ee port (PR #9286), which apply equally here: the from-scratch location.origin-based check had its own gap — a next_path like "http:evil.com" resolves AS IF relative whenever the input's scheme happens to match the real origin's own scheme. On this repo's real fix that meant any self-hosted deployment actually serving over plain http (not just the EE port's hardcoded-http placeholder-base variant) — verified directly: bypasses the check on an http:// origin, though not on https://, since the schemes then differ. isValidNextPath (@plane/utils, already used by apps/space for this identical purpose) closes this by requiring a literal leading "/" (and rejecting "//") before any URL-based comparison, so it doesn't depend on which scheme the real origin happens to use. Also removes a second, independently-bug-prone implementation of the same check. Co-authored-by: Plane AI <noreply@plane.so>
1 parent 1241037 commit 43f4dc0

1 file changed

Lines changed: 14 additions & 13 deletions

File tree

apps/web/core/lib/wrappers/authentication-wrapper.tsx

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import useSWR from "swr";
1111
// components
1212
import { LogoSpinner } from "@/components/common/logo-spinner";
1313
// helpers
14+
import { isValidNextPath } from "@plane/utils";
1415
import { EPageTypes } from "@/helpers/authentication.helper";
1516
// hooks
1617
import { useWorkspace } from "@/hooks/store/use-workspace";
@@ -24,19 +25,19 @@ type TAuthenticationWrapper = {
2425
pageType?: TPageType;
2526
};
2627

27-
const isValidURL = (url: string): boolean => {
28-
// A prefix-only scheme check (http(s)/ftp) lets an authority-relative
29-
// value like "///example.com/" through: it matches none of those schemes,
30-
// but the browser still resolves a leading "//" against the current
31-
// origin as an authority (host), navigating off-domain. Resolve against
32-
// location.origin and require the result to actually still be same-origin
33-
// instead of pattern-matching the input string.
34-
try {
35-
return new URL(url, location.origin).origin === location.origin;
36-
} catch {
37-
return false;
38-
}
39-
};
28+
// Delegates to the shared isValidNextPath (@plane/utils) instead of a local
29+
// reimplementation. A from-scratch version here previously resolved the
30+
// value against location.origin and required the result to stay
31+
// same-origin — which has its own gap: a next_path like "http:evil.com"
32+
// resolves AS IF relative whenever the input's scheme happens to match the
33+
// real origin's own scheme, e.g. any self-hosted deployment actually
34+
// serving over plain http (verified directly: this bypasses the
35+
// location.origin-based check on an http:// origin, though not on https://,
36+
// since the schemes then differ). isValidNextPath closes this by requiring
37+
// a literal leading "/" (and rejecting "//") before any URL-based
38+
// comparison, so it doesn't depend on which scheme the real origin happens
39+
// to use.
40+
const isValidURL = isValidNextPath;
4041

4142
export const AuthenticationWrapper = observer(function AuthenticationWrapper(props: TAuthenticationWrapper) {
4243
const pathname = usePathname();

0 commit comments

Comments
 (0)