fix: address PR review — scheme-restricted URLs, CWV default, test coverage

- Replace Joi.string() with relaxedUrl custom validator for BROKEN_INTERNAL_LINKS
  to reject dangerous URI schemes (javascript:, data:, blob:) while allowing
  malformed http/https URLs and relative paths
- Add .default([]) to CWV issues to prevent undefined access by consumers
- Document two CWV data shapes (page-level vs group-type) in schema comment
- Add comment explaining BROKEN_INTERNAL_LINKS divergence from BROKEN_BACKLINKS
- Add tests for: COLOR_CONTRAST patchContent/isCodeChangeAvailable,
  A11Y_ASSISTIVE patchContent/isCodeChangeAvailable, ALT_TEXT hasAltAttribute,
  BROKEN_INTERNAL_LINKS malformed URLs, relative paths, dangerous schemes,
  empty strings, and urlsSuggested
- Rename CWV test descriptions to clarify optionality

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Kanishka

packages/spacecat-shared-data-access/src/models/suggestion/suggestion.data-schemas.js

@@ -26,6 +26,17 @@
 import Joi from 'joi';
 import { OPPORTUNITY_TYPES } from '@adobe/spacecat-shared-utils';
 
+/**
+ * Custom Joi validator that accepts malformed HTTP/HTTPS URLs and relative paths
+ * while rejecting dangerous URI schemes (javascript:, data:, blob:, etc.).
+ * Used for BROKEN_INTERNAL_LINKS where crawled content may contain malformed URLs.
+ */
+const relaxedUrl = Joi.string().min(1).custom((value, helpers) => (
+  /^(https?:\/\/|\/)/i.test(value) ? value : helpers.error('string.uriScheme')
+), 'relaxed URL').messages({
+  'string.uriScheme': '{{#label}} must start with http://, https://, or /',
+});
+
 /**
  * Data schemas configuration per opportunity type.
  *
@@ -127,6 +138,10 @@ export const DATA_SCHEMAS = {
       },
     },
   },
+  // CWV has two implicit data shapes:
+  // 1. Page-level (type='url'): url and issues are present
+  // 2. Group-type (type='group'): url is absent, issues may be populated later via update
+  // Both shapes share the same schema; url and issues are optional to support both.
   [OPPORTUNITY_TYPES.CWV]: {
     schema: Joi.object({
       type: Joi.string().required(),
@@ -148,7 +163,7 @@ export const DATA_SCHEMAS = {
           organic: Joi.number().optional(),
         }).unknown(true),
       ).required(),
-      issues: Joi.array().items(Joi.object()).optional(),
+      issues: Joi.array().items(Joi.object()).optional().default([]),
       jiraLink: Joi.string().uri().allow(null).optional(),
       aggregationKey: Joi.string().allow(null).optional(),
     }).unknown(true),
@@ -379,14 +394,16 @@ export const DATA_SCHEMAS = {
   [OPPORTUNITY_TYPES.BROKEN_INTERNAL_LINKS]: {
     schema: Joi.object({
       // Support both naming conventions (snake_case and camelCase)
-      // URL fields use Joi.string() instead of Joi.string().uri() because
-      // internal-links sometimes keeps URL values as-is, including malformed URLs
-      url_from: Joi.string().optional(),
-      urlFrom: Joi.string().optional(),
-      url_to: Joi.string().optional(),
-      urlTo: Joi.string().optional(),
+      // URL fields use relaxedUrl instead of Joi.string().uri() because
+      // internal-links sometimes keeps URL values as-is, including malformed URLs.
+      // Unlike BROKEN_BACKLINKS, these accept malformed http/https/relative URLs
+      // but reject dangerous schemes (javascript:, data:, blob:, etc.).
+      url_from: relaxedUrl.optional(),
+      urlFrom: relaxedUrl.optional(),
+      url_to: relaxedUrl.optional(),
+      urlTo: relaxedUrl.optional(),
       title: Joi.string().optional(),
-      urlsSuggested: Joi.array().items(Joi.string()).optional(),
+      urlsSuggested: Joi.array().items(relaxedUrl).optional(),
       aiRationale: Joi.string().optional(),
       trafficDomain: Joi.number().optional(),
       priority: Joi.string().optional(),

packages/spacecat-shared-data-access/test/unit/models/suggestion/suggestion.model.test.js

@@ -259,7 +259,7 @@ describe('SuggestionModel', () => {
           expect(() => Suggestion.validateData(suggestionData, 'cwv')).to.not.throw();
         });
 
-        it('passes when issues is missing (issues is optional for update flow)', () => {
+        it('passes when issues is missing (issues is optional)', () => {
           const suggestionData = {
             type: 'url',
             url: 'https://www.example.com/page',
@@ -269,7 +269,7 @@ describe('SuggestionModel', () => {
           expect(() => Suggestion.validateData(suggestionData, 'cwv')).to.not.throw();
         });
 
-        it('passes when group-type has pattern but no url (url is optional for groups)', () => {
+        it('passes when url is missing (url is optional)', () => {
           const suggestionData = {
             type: 'group',
             name: 'Some pages',
@@ -285,6 +285,96 @@ describe('SuggestionModel', () => {
           expect(() => Suggestion.validateData(suggestionData, 'cwv')).to.not.throw();
         });
       });
+
+      describe('COLOR_CONTRAST opportunity type', () => {
+        it('passes with patchContent and isCodeChangeAvailable fields', () => {
+          const data = {
+            url: 'https://example.com/page',
+            issues: [{ wcagLevel: 'AA', severity: 'serious' }],
+            patchContent: 'diff --git a/file.js',
+            isCodeChangeAvailable: true,
+          };
+          expect(() => Suggestion.validateData(data, 'color-contrast')).to.not.throw();
+        });
+      });
+
+      describe('A11Y_ASSISTIVE opportunity type', () => {
+        it('passes with patchContent and isCodeChangeAvailable fields', () => {
+          const data = {
+            url: 'https://example.com/page',
+            issues: [{ wcagLevel: 'A', severity: 'critical' }],
+            patchContent: 'diff --git a/style.css',
+            isCodeChangeAvailable: false,
+          };
+          expect(() => Suggestion.validateData(data, 'a11y-assistive')).to.not.throw();
+        });
+      });
+
+      describe('ALT_TEXT opportunity type', () => {
+        it('passes with hasAltAttribute field in recommendations', () => {
+          const data = {
+            recommendations: [{
+              pageUrl: 'https://example.com/page',
+              imageUrl: 'https://example.com/img.png',
+              isDecorative: false,
+              hasAltAttribute: true,
+            }],
+          };
+          expect(() => Suggestion.validateData(data, 'image-alt-text')).to.not.throw();
+        });
+      });
+
+      describe('BROKEN_INTERNAL_LINKS opportunity type', () => {
+        it('passes with malformed http/https URLs', () => {
+          const data = {
+            url_from: 'https://example.com/page with spaces',
+            url_to: 'https://example.com/broken[link]',
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.not.throw();
+        });
+
+        it('passes with relative URLs', () => {
+          const data = {
+            urlFrom: '/relative/path',
+            urlTo: '/another/path',
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.not.throw();
+        });
+
+        it('rejects dangerous URI schemes (javascript:)', () => {
+          const data = {
+            // eslint-disable-next-line no-script-url
+            url_from: 'javascript:alert(1)',
+            url_to: 'https://example.com',
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.throw();
+        });
+
+        it('rejects dangerous URI schemes (data:)', () => {
+          const data = {
+            url_from: 'https://example.com',
+            url_to: 'data:text/html,<script>alert(1)</script>',
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.throw();
+        });
+
+        it('rejects empty string URLs', () => {
+          const data = {
+            url_from: '',
+            url_to: 'https://example.com',
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.throw();
+        });
+
+        it('passes with urlsSuggested containing relaxed URLs', () => {
+          const data = {
+            url_from: 'https://example.com/from',
+            url_to: 'https://example.com/to',
+            urlsSuggested: ['https://example.com/suggested page', '/relative/suggestion'],
+          };
+          expect(() => Suggestion.validateData(data, 'broken-internal-links')).to.not.throw();
+        });
+      });
     });
   });
 });