Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
27 changes: 27 additions & 0 deletions src/controllers/sites.js
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,32 @@ function SitesController(ctx, log, env) {
}
};

const patchPageCitabilityStatus = async (context) => {
const { siteId } = context.params;
const { url, httpStatus } = context.data || {};

Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

patchPageCitabilityStatus updates site-scoped data but does not validate siteId, load the Site, or enforce accessControlUtil.hasAccess(site) (unlike other /sites/:siteId/* handlers in this controller). Without this, callers with the route capability may be able to clear records for sites they don’t belong to. Add the standard siteId validation + Site.findById + org access check before any update logic.

Suggested change
if (!isValidUUID(siteId)) {
return badRequest('Site ID required');
}
const site = await Site.findById(siteId);
if (!site) {
return notFound('Site not found');
}
if (!await accessControlUtil.hasAccess(site)) {
return forbidden('Only users belonging to the organization can update page citability status');
}

Copilot uses AI. Check for mistakes.
if (!hasText(url)) {
return badRequest('url is required');
}

if (httpStatus !== 404) {
return ok({ url, httpStatus, updated: false });
}

const { PageCitability } = dataAccess;
const record = await PageCitability.findByUrl(url);
if (!record) {
return ok({ url, httpStatus, updated: false });
}
Comment on lines +1318 to +1322
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

The route is parameterized by :siteId, but the update is driven by PageCitability.findByUrl(url) with no verification the returned record belongs to that site. This can cause cross-site updates if the same URL can exist under different sites (or if a caller supplies a URL for a different site). Prefer a site-scoped lookup (e.g., findBySiteIdAndUrl(siteId, url)) or validate record.getSiteId() === siteId before mutating/saving.

Copilot uses AI. Check for mistakes.

record.setIsDeployedAtEdge(false);
record.setCitabilityScore(0);
await record.save();

log.info(`Cleared edge deployment status for 404 page: ${url} (site: ${siteId})`);
return ok({ url, httpStatus, updated: true });
};

return {
createSite,
getAll,
Expand All @@ -1317,6 +1343,7 @@ function SitesController(ctx, log, env) {
updateSite,
updateCdnLogsConfig,
getPageCitabilityCounts,
patchPageCitabilityStatus,
getTopPages,
resolveSite,
getBrandProfile,
Expand Down
1 change: 1 addition & 0 deletions src/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ export default function getRouteHandlers(
'GET /sites/:siteId/brand-profile': sitesController.getBrandProfile,
'POST /sites/:siteId/brand-profile': sitesController.triggerBrandProfile,
'GET /sites/:siteId/page-citability/counts': sitesController.getPageCitabilityCounts,
'PATCH /sites/:siteId/page-citability/status': sitesController.patchPageCitabilityStatus,
'GET /sites/:siteId/top-pages': sitesController.getTopPages,
Comment on lines 340 to 344
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

A new public HTTP endpoint was added to the router, but the OpenAPI specs under docs/openapi/ don't appear to be updated in this PR. Please add this route (and its request/response schema) to the OpenAPI entrypoint (e.g., docs/openapi/api.yaml plus a referenced path section), so generated docs stay accurate.

Copilot uses AI. Check for mistakes.
'GET /sites/:siteId/top-pages/:source': sitesController.getTopPages,
'GET /sites/:siteId/top-pages/:source/:geo': sitesController.getTopPages,
Expand Down
1 change: 1 addition & 0 deletions src/routes/required-capabilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ const routeRequiredCapabilities = {
'GET /sites/:siteId': 'site:read',
'PATCH /sites/:siteId': 'site:write',
'PATCH /sites/:siteId/config/cdn-logs': 'site:write',
'PATCH /sites/:siteId/page-citability/status': 'site:write',
'DELETE /sites/:siteId': 'site:write',
'GET /sites/:siteId/bot-blocker': 'site:read',
'GET /sites/:siteId/audits': 'audit:read',
Expand Down
1 change: 1 addition & 0 deletions test/controllers/sites.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ describe('Sites Controller', () => {
'updateSite',
'updateCdnLogsConfig',
'getPageCitabilityCounts',
'patchPageCitabilityStatus',
'getTopPages',
Comment on lines 133 to 137
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

A new mutating endpoint is introduced, but there are no unit tests covering patchPageCitabilityStatus behavior (404 clears fields, non-404 no-op, unknown URL returns updated:false, missing url -> 400, and access denied). Please add controller tests similar to existing SitesController tests; this change currently only updates the exported-function whitelist.

Copilot generated this review using guidance from repository custom instructions.
'getSiteMetricsBySource',
'getPageMetricsBySource',
Expand Down
Loading