[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>
This commit is contained in:
Manish Gupta
2026-08-31 09:53:09 +05:30
parent 124103743e
commit 43f4dc0a6a

View File

@@ -11,6 +11,7 @@ import useSWR from "swr";
// components
import { LogoSpinner } from "@/components/common/logo-spinner";
// helpers
import { isValidNextPath } from "@plane/utils";
import { EPageTypes } from "@/helpers/authentication.helper";
// hooks
import { useWorkspace } from "@/hooks/store/use-workspace";
@@ -24,19 +25,19 @@ type TAuthenticationWrapper = {
pageType?: TPageType;
};
const isValidURL = (url: string): boolean => {
// A prefix-only scheme check (http(s)/ftp) lets an authority-relative
// value like "///example.com/" through: it matches none of those schemes,
// but the browser still resolves a leading "//" against the current
// origin as an authority (host), navigating off-domain. Resolve against
// location.origin and require the result to actually still be same-origin
// instead of pattern-matching the input string.
try {
return new URL(url, location.origin).origin === location.origin;
} catch {
return false;
}
};
// Delegates to the shared isValidNextPath (@plane/utils) instead of a local
// reimplementation. A from-scratch version here previously resolved the
// value against location.origin and required the result to stay
// same-origin — which has 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, e.g. any self-hosted deployment actually
// serving over plain http (verified directly: this bypasses the
// location.origin-based check on an http:// origin, though not on https://,
// since the schemes then differ). isValidNextPath 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.
const isValidURL = isValidNextPath;
export const AuthenticationWrapper = observer(function AuthenticationWrapper(props: TAuthenticationWrapper) {
const pathname = usePathname();