Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"@adobe/spacecat-shared-athena-client": "1.9.11",
"@adobe/spacecat-shared-brand-client": "1.1.40",
"@adobe/spacecat-shared-content-client": "1.8.22",
"@adobe/spacecat-shared-data-access": "^3.44.0",
"@adobe/spacecat-shared-data-access": "^3.45.0",
"@adobe/spacecat-shared-data-access-v2": "npm:@adobe/spacecat-shared-data-access@2.109.0",
"@adobe/spacecat-shared-drs-client": "1.4.2",
"@adobe/spacecat-shared-gpt-client": "1.6.21",
Expand Down
11 changes: 11 additions & 0 deletions src/controllers/sites.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
wwwUrlResolver, resolveWwwUrl, getIsSummitPlgEnabled, CUSTOMER_VISIBLE_TIERS,
} from '../support/utils.js';
import AccessControlUtil from '../support/access-control-util.js';
import { auditTargetURLsPatchGuard } from '../support/audit-target-urls-validation.js';
import { triggerBrandProfileAgent } from '../support/brand-profile-trigger.js';

/**
Expand Down Expand Up @@ -639,6 +640,16 @@ function SitesController(ctx, log, env) {
? Config.toDynamoItem(siteConfig) || {}
: {};
const merged = { ...existingConfig, ...requestBody.config };
const auditTargetURLsResult = auditTargetURLsPatchGuard(
merged,
site.getBaseURL(),
requestBody.config,
badRequest,
);
if (auditTargetURLsResult?.error) return auditTargetURLsResult.error;
if (auditTargetURLsResult?.normalized !== undefined) {
merged.auditTargetURLs = auditTargetURLsResult.normalized;
}
site.setConfig(merged);
updates = true;
}
Expand Down
129 changes: 129 additions & 0 deletions src/support/audit-target-urls-validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright 2026 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

/**
* API validation for audit target URLs: parseable absolute URL, HTTPS only, and hostname
* must match the site base URL hostname when that can be derived.
*/

/**
* @param {string} baseURL
* @returns {string|null}
*/
export function siteHostnameFromBaseURL(baseURL) {
if (!baseURL || typeof baseURL !== 'string') {
return null;
}
try {
return new URL(baseURL).hostname;
} catch {
return null;
}
}

/**
* @param {string} trimmed
* @param {string|null} siteHostname
* @returns {{ ok: true } | { ok: false, error: string }}
*/
export function validateAuditTargetUrlString(trimmed, siteHostname) {
let url;
try {
url = new URL(trimmed);
} catch {
return { ok: false, error: 'Invalid URL' };
}
if (url.protocol !== 'https:') {
return { ok: false, error: 'URL must use HTTPS' };
}
if (siteHostname && url.hostname !== siteHostname) {
Copy link
Copy Markdown
Member

@nitinja nitinja Apr 6, 2026

Choose a reason for hiding this comment

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

Hostname check -> following case comes to mind

Example: In https://www.example.com
Protocol: https://
Hostname: www.example.com

but the page user entered could be blog.example.com/page -> thats sounds like a valid page, but fails this check?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@nitinja , I believe we need to onboard blog.example.com separately, as technically it may be running on a different tech stack, such as EDS vs AEMaaCS, etc.

return {
ok: false,
error: `URL hostname must match the site domain (${siteHostname})`,
};
}
return { ok: true };
}

/**
* Validates config.auditTargetURLs after merge (manual list only).
*
* @param {unknown} auditTargetURLs
* @param {string} siteBaseURL
* @returns {{ ok: true, normalized?: object } | { ok: false, error: string }}
*/
export function validateAuditTargetURLsConfig(auditTargetURLs, siteBaseURL) {
if (auditTargetURLs === undefined) {
return { ok: true };
}
if (auditTargetURLs === null || typeof auditTargetURLs !== 'object' || Array.isArray(auditTargetURLs)) {
return { ok: false, error: 'config.auditTargetURLs must be an object when provided' };
}

const siteHostname = siteHostnameFromBaseURL(siteBaseURL);
const { manual, ...rest } = auditTargetURLs;

if (manual === undefined) {
return { ok: true };
}
if (!Array.isArray(manual)) {
return { ok: false, error: 'config.auditTargetURLs.manual must be an array' };
}

const normalizedManual = [];
for (let i = 0; i < manual.length; i += 1) {
const entry = manual[i];
if (!entry || typeof entry !== 'object' || typeof entry.url !== 'string') {
return {
ok: false,
error: `config.auditTargetURLs.manual[${i}] must be an object with a string "url" property`,
};
}
const trimmed = entry.url.trim();
const result = validateAuditTargetUrlString(trimmed, siteHostname);
if (!result.ok) {
return { ok: false, error: `Invalid audit target URL at index ${i}: ${result.error}` };
}
normalizedManual.push({ url: trimmed });
}

return {
ok: true,
normalized: { ...rest, manual: normalizedManual },
};
}

/**
* When `configPatch` includes `auditTargetURLs`, validates `merged.auditTargetURLs` (HTTPS +
* hostname vs site) and writes normalized `manual` entries back onto `merged`.
*
* We must consult `configPatch`, not only `merged`: after `{ ...existingConfig, ...patch }`,
* `merged.auditTargetURLs` is still present if it existed on the site and the client only
* changed other keys (e.g. `slack`). In that case we must not re-validate or reject legacy
* data until the client explicitly sends `auditTargetURLs` again.
*
* @param {Record<string, unknown>} merged
* @param {string} siteBaseURL
* @param {Record<string, unknown>} configPatch
* @param {(message: string) => unknown} badRequestFn
* @returns {null | { error: unknown } | { normalized?: object }}
* `null` if the patch did not include `auditTargetURLs`; `{ error }` to return from the
* controller; `{ normalized }` or `{}` on success (caller assigns `normalized` to merged).
*/
export function auditTargetURLsPatchGuard(merged, siteBaseURL, configPatch, badRequestFn) {
if (!Object.prototype.hasOwnProperty.call(configPatch, 'auditTargetURLs')) {
return null;
}
const v = validateAuditTargetURLsConfig(merged.auditTargetURLs, siteBaseURL);
if (!v.ok) return { error: badRequestFn(v.error) };
return v.normalized !== undefined ? { normalized: v.normalized } : {};
}
74 changes: 72 additions & 2 deletions test/controllers/sites.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
* governing permissions and limitations under the License.
*/

/* eslint-env mocha */

import { Organization, Site } from '@adobe/spacecat-shared-data-access';
import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js';
import OrganizationSchema from '@adobe/spacecat-shared-data-access/src/models/organization/organization.schema.js';
Expand Down Expand Up @@ -3437,6 +3435,78 @@ describe('Sites Controller', () => {
expect(mergedConfig.handlers).to.deep.equal({ 'meta-tags': { excludedURLs: [] } });
});

describe('auditTargetURLs validation', () => {
it('returns bad request when manual URL hostname does not match site base URL', async () => {
const site = sites[0];
site.getConfig = sandbox.stub().returns(Config({ slack: { channel: '#x' } }));
site.setConfig = sandbox.stub();
site.save = sandbox.stub();

const response = await sitesController.updateSite({
params: { siteId: SITE_IDS[0] },
data: {
config: {
auditTargetURLs: {
manual: [{ url: 'https://example.com/path1' }],
},
},
},
...defaultAuthAttributes,
});

expect(response.status).to.equal(400);
const err = await response.json();
expect(err.message).to.include('site domain (site1.com)');
expect(site.setConfig).to.have.not.been.called;
});

it('accepts manual URLs on the site hostname', async () => {
const site = sites[0];
site.getConfig = sandbox.stub().returns(Config({}));
site.setConfig = sandbox.stub();
site.save = sandbox.stub().resolves(site);

const response = await sitesController.updateSite({
params: { siteId: SITE_IDS[0] },
data: {
config: {
auditTargetURLs: {
manual: [{ url: 'https://site1.com/path1' }, { url: 'https://site1.com/path2' }],
},
},
},
...defaultAuthAttributes,
});

expect(response.status).to.equal(200);
const merged = site.setConfig.firstCall.args[0];
expect(merged.auditTargetURLs.manual).to.deep.equal([
{ url: 'https://site1.com/path1' },
{ url: 'https://site1.com/path2' },
]);
});

it('does not validate auditTargetURLs when key is omitted from config patch', async () => {
const site = sites[0];
const existingConfig = Config({
auditTargetURLs: { manual: [{ url: 'https://wrong.example/' }] },
});
site.getConfig = sandbox.stub().returns(existingConfig);
site.setConfig = sandbox.stub();
site.save = sandbox.stub().resolves(site);

const response = await sitesController.updateSite({
params: { siteId: SITE_IDS[0] },
data: { config: { slack: { channel: '#only' } } },
...defaultAuthAttributes,
});

expect(response.status).to.equal(200);
const merged = site.setConfig.firstCall.args[0];
expect(merged.auditTargetURLs.manual[0].url).to.equal('https://wrong.example/');
});
});

it('allows removing a config key by explicitly setting it to null', async () => {
const site = sites[0];
const existingConfig = Config({
Expand Down
Loading
Loading