fix(legal): Prevent stored XSS via javascript: URLs in policy revision flow (#114283)

The legal/compliance policy row rendered policy URLs in two unsafe ways:
the "Review" LinkButton used the raw `policy.url` string directly as an
href, and the `showPolicy` popup handler also passed the raw URL to
`window.open`. Because `new URL('javascript:...')` parses successfully,
`safeURL` returned a truthy URL object for javascript: scheme values,
allowing the outer `policy.url && policyUrl` guard to pass. Any billing
user who clicked "Review" on an affected policy would execute arbitrary
JavaScript in the current page's context.

Fixes: - Add an explicit http/https protocol allowlist check after
`safeURL` so that javascript:, data:, and other dangerous schemes are
treated as invalid and produce a null `policyUrl`. - Replace
`policy.url` with `policyUrl.toString()` in the "Review" LinkButton and
`showPolicy` so all three rendering paths (iframe src, window.open, and
anchor href) go through the validated reference. - Add a `validate`
function to `PolicyRevisionSchema`'s url field in the admin form so
non-http(s) URLs are rejected at submission time, preventing malicious
values from being stored in the first place.

Fixes https://linear.app/getsentry/issue/REVENG-38

Author: swartzrock

static/gsAdmin/schemas/policies.spec.tsx

@@ -0,0 +1,54 @@
+import {PolicyRevisionSchema} from 'admin/schemas/policies';
+
+describe('PolicyRevisionSchema', () => {
+  const urlField = PolicyRevisionSchema.find(f => f.name === 'url')!;
+  const validate = urlField.validate!;
+
+  describe('url field validation', () => {
+    it('accepts https URLs', () => {
+      expect(
+        validate({id: 'url', form: {url: 'https://sentry.io/legal/terms/'}})
+      ).toEqual([]);
+    });
+
+    it('accepts http URLs', () => {
+      expect(validate({id: 'url', form: {url: 'http://example.com/policy/'}})).toEqual(
+        []
+      );
+    });
+
+    it('allows an empty value', () => {
+      expect(validate({id: 'url', form: {url: ''}})).toEqual([]);
+      expect(validate({id: 'url', form: {url: undefined}})).toEqual([]);
+    });
+
+    it('rejects javascript: URLs with a protocol error', () => {
+      // Build via concatenation to avoid the no-script-url ESLint rule.
+      const jsUrl = 'javascript' + ':alert(document.domain)';
+      expect(validate({id: 'url', form: {url: jsUrl}})).toEqual([
+        ['url', 'URL must use http or https protocol'],
+      ]);
+    });
+
+    it('rejects data: URLs with a protocol error', () => {
+      expect(
+        validate({
+          id: 'url',
+          form: {url: 'data:text/html,<script>alert(1)</script>'},
+        })
+      ).toEqual([['url', 'URL must use http or https protocol']]);
+    });
+
+    it('rejects vbscript: URLs with a protocol error', () => {
+      expect(validate({id: 'url', form: {url: 'vbscript:msgbox(1)'}})).toEqual([
+        ['url', 'URL must use http or https protocol'],
+      ]);
+    });
+
+    it('rejects non-URL strings with an invalid URL error', () => {
+      expect(validate({id: 'url', form: {url: 'not-a-valid-url'}})).toEqual([
+        ['url', 'Please enter a valid URL'],
+      ]);
+    });
+  });
+});

static/gsAdmin/schemas/policies.tsx

@@ -1,7 +1,7 @@
-import type {FieldObject} from 'sentry/components/forms/types';
+import type {Field} from 'sentry/components/forms/types';
 import {slugify} from 'sentry/utils/slugify';
 
-export const PolicySchema: FieldObject[] = [
+export const PolicySchema: Field[] = [
   {
     name: 'name',
     type: 'string',
@@ -35,7 +35,7 @@ export const PolicySchema: FieldObject[] = [
   },
 ];
 
-export const PolicyRevisionSchema: FieldObject[] = [
+export const PolicyRevisionSchema: Field[] = [
   {
     name: 'version',
     type: 'string',
@@ -51,6 +51,21 @@ export const PolicyRevisionSchema: FieldObject[] = [
     label: 'URL',
     placeholder: 'e.g. https://example.com/terms-of-service/',
     help: 'If the policy is hosted at an external URL, enter it here.',
+    validate: ({id, form}) => {
+      const value = form[id];
+      if (!value) {
+        return [];
+      }
+      try {
+        const parsed = new URL(value);
+        if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+          return [[id, 'URL must use http or https protocol']];
+        }
+      } catch {
+        return [[id, 'Please enter a valid URL']];
+      }
+      return [];
+    },
   },
   {
     name: 'file',

static/gsApp/views/legalAndCompliance/policyRow.spec.tsx

@@ -7,6 +7,15 @@ import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import {PolicyRow} from 'getsentry/views/legalAndCompliance/policyRow';
 
+// Build the javascript: URL via concatenation to avoid the no-script-url ESLint rule,
+// which flags literal javascript: strings even inside test assertions.
+const JS_URL = 'javascript' + ':alert(document.domain)';
+const DANGEROUS_URLS = [
+  JS_URL,
+  'data:text/html,<script>alert(1)</script>',
+  'vbscript:msgbox(1)',
+];
+
 describe('PolicyRow', () => {
   const {organization} = initializeOrg({});
   const subscription = SubscriptionFixture({organization});
@@ -119,4 +128,92 @@ describe('PolicyRow', () => {
     expect(screen.queryByText(/New version available/i)).not.toBeInTheDocument();
     expect(screen.getByRole('button', {name: 'Review and Accept'})).toBeInTheDocument();
   });
+
+  describe('XSS prevention via URL scheme allowlist', () => {
+    it.each(DANGEROUS_URLS)(
+      'does not render a Review button when policy URL is "%s"',
+      dangerousUrl => {
+        const policy = {...policies.pentest!, url: dangerousUrl};
+        render(
+          <PolicyRow
+            policies={{...policies, pentest: policy}}
+            policy={policy}
+            showUpdated={false}
+            subscription={subscription}
+            onAccept={() => {}}
+          />
+        );
+        expect(screen.queryByRole('button', {name: 'Review'})).not.toBeInTheDocument();
+        expect(
+          screen.queryByRole('button', {name: 'Review and Accept'})
+        ).not.toBeInTheDocument();
+      }
+    );
+
+    it.each(DANGEROUS_URLS)(
+      'does not render Review and Accept for billing user when policy URL is "%s"',
+      dangerousUrl => {
+        const orgWithBilling = OrganizationFixture({access: ['org:billing']});
+        // soc-2-bridge-letter: hasSignature=true, slug is not privacy/terms — normally renders "Review and Accept"
+        const policy = {...policies['soc-2-bridge-letter']!, url: dangerousUrl};
+        render(
+          <PolicyRow
+            policies={{...policies, 'soc-2-bridge-letter': policy}}
+            policy={policy}
+            showUpdated={false}
+            subscription={subscription}
+            onAccept={() => {}}
+          />,
+          {organization: orgWithBilling}
+        );
+        expect(
+          screen.queryByRole('button', {name: 'Review and Accept'})
+        ).not.toBeInTheDocument();
+      }
+    );
+
+    it.each(DANGEROUS_URLS)(
+      'does not pass dangerous URL "%s" to window.open',
+      dangerousUrl => {
+        const windowOpenSpy = jest.spyOn(window, 'open').mockReturnValue(null);
+        const orgWithBilling = OrganizationFixture({access: ['org:billing']});
+        const policy = {...policies['soc-2-bridge-letter']!, url: dangerousUrl};
+        render(
+          <PolicyRow
+            policies={{...policies, 'soc-2-bridge-letter': policy}}
+            policy={policy}
+            showUpdated={false}
+            subscription={subscription}
+            onAccept={() => {}}
+          />,
+          {organization: orgWithBilling}
+        );
+        // No button is rendered so showPolicy cannot be triggered
+        expect(
+          screen.queryByRole('button', {name: 'Review and Accept'})
+        ).not.toBeInTheDocument();
+        expect(windowOpenSpy).not.toHaveBeenCalled();
+        windowOpenSpy.mockRestore();
+      }
+    );
+
+    it('renders the Review button with a safe https href for a legitimate URL', () => {
+      const policy = {...policies.terms!, url: 'https://sentry.io/legal/terms/1.0.0/'};
+      render(
+        <PolicyRow
+          policies={{...policies, terms: policy}}
+          policy={policy}
+          showUpdated={false}
+          subscription={subscription}
+          onAccept={() => {}}
+        />
+      );
+      const reviewButton = screen.getByRole('button', {name: 'Review'});
+      expect(reviewButton).toBeInTheDocument();
+      expect(reviewButton).toHaveAttribute(
+        'href',
+        'https://sentry.io/legal/terms/1.0.0/?userCurrentVersion=1.0.0'
+      );
+    });
+  });
 });

static/gsApp/views/legalAndCompliance/policyRow.tsx

@@ -48,7 +48,12 @@ export function PolicyRow({
   const activeSuperUser = isActiveSuperuser();
   const hasBillingAccess = organization.access.includes('org:billing');
 
-  const policyUrl = policy.url ? safeURL(policy.url) : null;
+  const rawPolicyUrl = policy.url ? safeURL(policy.url) : null;
+  // Only allow http/https URLs to prevent javascript: and data: URL injection
+  const policyUrl =
+    rawPolicyUrl?.protocol === 'http:' || rawPolicyUrl?.protocol === 'https:'
+      ? rawPolicyUrl
+      : null;
   // userCurrentVersion filters version select dropdown to only the current version + latest version
   if (policyUrl && policy.consent) {
     policyUrl.searchParams.set('userCurrentVersion', policy.consent.acceptedVersion);
@@ -61,7 +66,7 @@ export function PolicyRow({
     const name = 'sentryPolicy';
     const width = 600;
     const height = 600;
-    const url = policy.url;
+    const url = policyUrl?.toString() ?? null;
 
     // this attempts to center the dialog
     const innerWidth = window.innerWidth
@@ -226,7 +231,7 @@ export function PolicyRow({
               {t('Review and Accept')}
             </Button>
           ) : (
-            <LinkButton size="sm" external href={policy.url}>
+            <LinkButton size="sm" external href={policyUrl.toString()}>
               {t('Review')}
             </LinkButton>
           ))}