From 190a7ea1a7d7f5de2dda45dda1f738b00967d54e Mon Sep 17 00:00:00 2001 From: erwin mombay Date: Wed, 7 Feb 2024 13:19:55 -0800 Subject: [PATCH] add additional forbidden term for ampproject.org (#39803) * add ampproject.org presubmit check * fix forbidden ampproject term * temp? * single out individual lines to disable forbidden term * change ampproject.org link to amp.dev * simplify forbidden term regex for CDN URL --- ads/_a4a-config.js | 2 ++ build-system/server/app-index/template.js | 2 +- build-system/test-configs/forbidden-terms.js | 9 +++++---- extensions/amp-access/0.1/amp-access-server-jwt.js | 2 ++ extensions/amp-iframe/0.1/amp-iframe.js | 2 ++ src/3p-frame.js | 1 + src/amp-story-player/amp-story-player-impl.js | 2 +- src/cookies.js | 1 + 8 files changed, 15 insertions(+), 6 deletions(-) diff --git a/ads/_a4a-config.js b/ads/_a4a-config.js index dbfe987a4de4..91c1c16abe8b 100644 --- a/ads/_a4a-config.js +++ b/ads/_a4a-config.js @@ -47,6 +47,8 @@ export function getA4ARegistry() { * @type {!{[key: string]: string}} */ export const signingServerURLs = { + /* eslint-disable local/no-forbidden-terms */ 'google': 'https://cdn.ampproject.org/amp-ad-verifying-keyset.json', 'google-dev': 'https://cdn.ampproject.org/amp-ad-verifying-keyset-dev.json', + /* eslint-enable local/no-forbidden-terms */ }; diff --git a/build-system/server/app-index/template.js b/build-system/server/app-index/template.js index a2fa367186d5..3d3d74f5aa29 100644 --- a/build-system/server/app-index/template.js +++ b/build-system/server/app-index/template.js @@ -81,7 +81,7 @@ function renderTemplate(opt_params = {}) { html`
Built with 💙 by - the AMP Project. + the AMP Project.
`, ]); diff --git a/build-system/test-configs/forbidden-terms.js b/build-system/test-configs/forbidden-terms.js index c4b79832a4e0..9528db0d7b7d 100644 --- a/build-system/test-configs/forbidden-terms.js +++ b/build-system/test-configs/forbidden-terms.js @@ -919,12 +919,15 @@ const forbiddenTermsSrcInclusive = { 'extensions/amp-consent/0.1/consent-state-manager.js', ], }, - '(cdn|3p)\\.ampproject\\.': { + 'ampproject\\.': { message: 'The CDN domain should typically not be hardcoded in source ' + 'code. Use urls from src/config/urls.js instead.', allowlist: [ - 'ads/_a4a-config.js', + // NOTE: Do not allow source code to be allowlisted for this rule. + // explicitly allow individual lines through eslint. The only + // exception is `src/config/urls.js` as it contains the variables + // that need to be referenced by other modules. 'build-system/server/amp4test.js', 'build-system/server/app-index/amphtml-helpers.js', 'build-system/server/app-video-testbench.js', @@ -933,8 +936,6 @@ const forbiddenTermsSrcInclusive = { 'build-system/server/variable-substitution.js', 'build-system/tasks/dist.js', 'build-system/tasks/helpers.js', - 'src/3p-frame.js', - 'src/amp-story-player/amp-story-player-impl.js', 'src/config/urls.js', 'testing/local-amp-chrome-extension/background.js', 'tools/experiments/experiments.js', diff --git a/extensions/amp-access/0.1/amp-access-server-jwt.js b/extensions/amp-access/0.1/amp-access-server-jwt.js index d1f7b2d7ed6b..4e6b8605182a 100644 --- a/extensions/amp-access/0.1/amp-access-server-jwt.js +++ b/extensions/amp-access/0.1/amp-access-server-jwt.js @@ -25,8 +25,10 @@ const TAG = 'amp-access-server-jwt'; /** @const {number} */ const AUTHORIZATION_TIMEOUT = 3000; +/* eslint-disable local/no-forbidden-terms */ /** @const {string} */ const AMP_AUD = 'ampproject.org'; +/* eslint-enable local/no-forbidden-terms */ /** * This class implements server-side authorization protocol with JWT. In this diff --git a/extensions/amp-iframe/0.1/amp-iframe.js b/extensions/amp-iframe/0.1/amp-iframe.js index bdfd39501465..adc262bc65a5 100644 --- a/extensions/amp-iframe/0.1/amp-iframe.js +++ b/extensions/amp-iframe/0.1/amp-iframe.js @@ -155,9 +155,11 @@ export class AmpIframe extends AMP.BaseElement { userAssert( !( endsWith(hostname, `.${urls.thirdPartyFrameHost}`) || + // eslint-disable-next-line local/no-forbidden-terms endsWith(hostname, '.ampproject.org') ), 'amp-iframe does not allow embedding of frames from ' + + // eslint-disable-next-line local/no-forbidden-terms 'ampproject.*: %s', src ); diff --git a/src/3p-frame.js b/src/3p-frame.js index 46976c0d96cb..549d72006d0d 100644 --- a/src/3p-frame.js +++ b/src/3p-frame.js @@ -289,6 +289,7 @@ export function getDevelopmentBootstrapBaseUrl(parentWindow, srcFileBasename) { */ function getAdsLocalhost(win) { let adsUrl = urls.thirdParty; // local dev with a non-localhost server + // eslint-disable-next-line local/no-forbidden-terms if (adsUrl == 'https://3p.ampproject.net') { adsUrl = 'http://ads.localhost'; // local dev with a localhost server } diff --git a/src/amp-story-player/amp-story-player-impl.js b/src/amp-story-player/amp-story-player-impl.js index 84305b72c72f..21a02e458fe2 100644 --- a/src/amp-story-player/amp-story-player-impl.js +++ b/src/amp-story-player/amp-story-player-impl.js @@ -49,7 +49,7 @@ const STORY_POSITION_ENUM = { }; /** @const @type {!Array} */ -const SUPPORTED_CACHES = ['cdn.ampproject.org', 'www.bing-amp.com']; +const SUPPORTED_CACHES = ['cdn.ampproject.org', 'www.bing-amp.com']; // eslint-disable-line local/no-forbidden-terms /** @const @type {!Array} */ const SANDBOX_MIN_LIST = ['allow-top-navigation']; diff --git a/src/cookies.js b/src/cookies.js index 455d223bcfe3..4102c0af76ca 100644 --- a/src/cookies.js +++ b/src/cookies.js @@ -186,6 +186,7 @@ function trySetCookie( // We do not allow setting cookies on the domain that contains both // the cdn. and www. hosts. // Note: we need to allow cdn.ampproject.org in order to optin to experiments + // eslint-disable-next-line local/no-forbidden-terms if (domain == 'ampproject.org') { // Actively delete them. value = 'delete';